RE: [Bluez-devel] Rfcomm use count

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

[-- Attachment #1: Type: text/plain, Size: 3037 bytes --]

Hi Marcel,

I have attached a log for you to look at if you have some time. If not,
maybe just answer my question below. :)

Here's what I see:

One stray socket. I added the socket pointer to the proc output.
Proc/bluetooth/rfcomm:
sk  2C:02:5F:16:05:00 3A:A4:58:16:05:00 9 1 c3b69340

I have a setup where I kill my app after a random time interval while
several devices are connecting, disconnecting, and transfering data, etc.

Here's a brief version of the log:

1. Program gets kill signal (time 15:09:30)
2. HCI devices get shut down
3. Some data remaining for transmission is sent.
4. One listen socket is closed (I think this is the one which is not being
used)
5. Some sockets/dlcs get closed
6. An incomming connection is received (rfcomm_connect_ind is called, and a
socket is created which matches the one in proc)
7. Some more sockets/dlcs get closed
8. The listen (probably the one which is being used) gets closed

I don't see any lines from rfcomm_sock_accept after the rfcomm_connect_ind
line. And rfcomm_sock_release is never called for the new connection.

My knowledge here is limited, and I may be mis-interpreting the log. But it
appears that a socket is allocated for the new connection as long as there
is room in the wait queue. And if that connection is never accepted, is it
the job of rfcomm_sock_cleanup_listen() to deal with it? That is, it is not
the kernel's duty (outside of the rfcomm module) to deal with the allocated
socket?

So, I think, it appears that rfcomm_sock_cleanup_listen isn't working right.

Not sure if rfcomm_sock_accept is still looping or not while this is going
on.

More digging needed...

Thanks,

-Daryl.

> -----Original Message-----
> From: Daryl Van Vorst [mailto:daryl@wideray.com] 
> Sent: September 13, 2004 9:37 AM
> To: 'Marcel Holtmann'
> Subject: RE: [Bluez-devel] Rfcomm use count
> 
> 
> Hi Marcel,
> 
> > I haven't had time to look at your problem in the last two weeks and
> > dealing with ARM related stuff still not fits into my left 
> > free time for
> > the next weeks. Is this behaviour reproduceable on x86 
> machines and do
> > you have a small text program to trigger this effect? And 
> > what I really
> > care about, is this problem also available with a 2.6 kernel?
> 
> Thanks for the response.
> 
> I have not yet tried to reproduce it on an x86. And 
> unfortunately I don't have a good way to trigger the effect. 
> I have spent quite a lot of time trying to come up with a 
> simple way (or any way) to reliably reproduce the problem. 
> Depending on how the next while plays out, I may try to 
> reproduce it on an x86.
> 
> I would like to be working with a 2.6 kernel, but time 
> constraints have prevented moving to it (would require 
> porting drivers). Maybe on an x86 if time permits.
> 
> I'm currently sifting through RFCOMM debug output. When I get 
> something interesting I'll send it along. I should have 
> something shortly.
> 
> -Daryl.
> 
> 

[-- Attachment #2: messages.gz --]
[-- Type: application/x-gzip, Size: 3161 bytes --]

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

[-- Attachment #1: Type: text/plain, Size: 5089 bytes --]

Marcel,

Turned on debugging in af_bluetooth.c, and it may reveal the issue...

This time the socket left in /proc is: c39bb6a0
The syslog messages are attached.

Snippet from /var/log/messages (I added a debug line in rfcomm_sock_accept
to print the error code):
May 28 11:26:06 jack-00000000 syslog.info klogd:
bluez_accept_dequeue_R6c5dc344: parent c3a81060
May 28 11:26:06 jack-00000000 syslog.info klogd: bluez_accept_unlink: sk
c39bb6a0 state 9
May 28 11:26:06 jack-00000000 syslog.info klogd: rfcomm_sock_sendmsg: sock
c399c910, sk c3ad43e0
May 28 11:26:06 jack-00000000 syslog.info klogd: rfcomm_sock_accept: error
-512

So bluez_accept_unlink() is getting called. But if the socket was really
unlinked, it wouldn't be in /proc, would it?

Could the problem be the order of release_sock() and bluez_accept_unlink()
in the second if() in the code snippet from af_bluetooth.c below?
Specifically, does bluez_accept_unlink() need to be called on an unlocked
socket?

Thanks,

-Daryl.

struct sock *bluez_accept_dequeue(struct sock *parent, struct socket
*newsock)
{
	struct list_head *p, *n;
	struct bluez_pinfo *pi;
	struct sock *sk;
	
	BT_DBG("parent %p", parent);

	list_for_each_safe(p, n, &bluez_pi(parent)->accept_q) {
		pi = list_entry(p, struct bluez_pinfo, accept_q);
		sk = bluez_sk(pi);
		
		lock_sock(sk);
		if (sk->state == BT_CLOSED) {
			release_sock(sk);
			bluez_accept_unlink(sk);
			continue;
		}
		
		if (sk->state == BT_CONNECTED || !newsock) {
			bluez_accept_unlink(sk);
			if (newsock)
				sock_graft(sk, newsock);
			release_sock(sk);
			return sk;
		}
		release_sock(sk);
	}
	return NULL;
}


> -----Original Message-----
> From: bluez-devel-admin@lists.sourceforge.net 
> [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of 
> Daryl Van Vorst
> Sent: September 13, 2004 12:06 PM
> To: 'Marcel Holtmann'
> Cc: 'BlueZ Mailing List'
> Subject: RE: [Bluez-devel] Rfcomm use count
> 
> 
> Hi Marcel,
> 
> I have attached a log for you to look at if you have some 
> time. If not,
> maybe just answer my question below. :)
> 
> Here's what I see:
> 
> One stray socket. I added the socket pointer to the proc output.
> Proc/bluetooth/rfcomm:
> sk  2C:02:5F:16:05:00 3A:A4:58:16:05:00 9 1 c3b69340
> 
> I have a setup where I kill my app after a random time interval while
> several devices are connecting, disconnecting, and 
> transfering data, etc.
> 
> Here's a brief version of the log:
> 
> 1. Program gets kill signal (time 15:09:30)
> 2. HCI devices get shut down
> 3. Some data remaining for transmission is sent.
> 4. One listen socket is closed (I think this is the one which 
> is not being
> used)
> 5. Some sockets/dlcs get closed
> 6. An incomming connection is received (rfcomm_connect_ind is 
> called, and a
> socket is created which matches the one in proc)
> 7. Some more sockets/dlcs get closed
> 8. The listen (probably the one which is being used) gets closed
> 
> I don't see any lines from rfcomm_sock_accept after the 
> rfcomm_connect_ind
> line. And rfcomm_sock_release is never called for the new connection.
> 
> My knowledge here is limited, and I may be mis-interpreting 
> the log. But it
> appears that a socket is allocated for the new connection as 
> long as there
> is room in the wait queue. And if that connection is never 
> accepted, is it
> the job of rfcomm_sock_cleanup_listen() to deal with it? That 
> is, it is not
> the kernel's duty (outside of the rfcomm module) to deal with 
> the allocated
> socket?
> 
> So, I think, it appears that rfcomm_sock_cleanup_listen isn't 
> working right.
> 
> Not sure if rfcomm_sock_accept is still looping or not while 
> this is going
> on.
> 
> More digging needed...
> 
> Thanks,
> 
> -Daryl.
> 
> > -----Original Message-----
> > From: Daryl Van Vorst [mailto:daryl@wideray.com] 
> > Sent: September 13, 2004 9:37 AM
> > To: 'Marcel Holtmann'
> > Subject: RE: [Bluez-devel] Rfcomm use count
> > 
> > 
> > Hi Marcel,
> > 
> > > I haven't had time to look at your problem in the last 
> two weeks and
> > > dealing with ARM related stuff still not fits into my left 
> > > free time for
> > > the next weeks. Is this behaviour reproduceable on x86 
> > machines and do
> > > you have a small text program to trigger this effect? And 
> > > what I really
> > > care about, is this problem also available with a 2.6 kernel?
> > 
> > Thanks for the response.
> > 
> > I have not yet tried to reproduce it on an x86. And 
> > unfortunately I don't have a good way to trigger the effect. 
> > I have spent quite a lot of time trying to come up with a 
> > simple way (or any way) to reliably reproduce the problem. 
> > Depending on how the next while plays out, I may try to 
> > reproduce it on an x86.
> > 
> > I would like to be working with a 2.6 kernel, but time 
> > constraints have prevented moving to it (would require 
> > porting drivers). Maybe on an x86 if time permits.
> > 
> > I'm currently sifting through RFCOMM debug output. When I get 
> > something interesting I'll send it along. I should have 
> > something shortly.
> > 
> > -Daryl.
> > 
> > 
> 

[-- Attachment #2: messages.gz --]
[-- Type: application/x-gzip, Size: 1815 bytes --]

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

Marcel,

I just noticed that bluez_accept_unlink() was called with a socked that was
in state BT_CLOSED, so it can't be the second if(). It would have been
called from the first if().

Clearly, I'm on glue.

Still, does the order of bluez_accept_unlink() and sock_release() matter?

-Daryl.

> -----Original Message-----
> From: bluez-devel-admin@lists.sourceforge.net 
> [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of 
> Daryl Van Vorst
> Sent: September 13, 2004 1:49 PM
> To: 'Marcel Holtmann'
> Cc: 'BlueZ Mailing List'
> Subject: RE: [Bluez-devel] Rfcomm use count
> 
> 
> Marcel,
> 
> Turned on debugging in af_bluetooth.c, and it may reveal the issue...
> 
> This time the socket left in /proc is: c39bb6a0
> The syslog messages are attached.
> 
> Snippet from /var/log/messages (I added a debug line in 
> rfcomm_sock_accept
> to print the error code):
> May 28 11:26:06 jack-00000000 syslog.info klogd:
> bluez_accept_dequeue_R6c5dc344: parent c3a81060
> May 28 11:26:06 jack-00000000 syslog.info klogd: 
> bluez_accept_unlink: sk
> c39bb6a0 state 9
> May 28 11:26:06 jack-00000000 syslog.info klogd: 
> rfcomm_sock_sendmsg: sock
> c399c910, sk c3ad43e0
> May 28 11:26:06 jack-00000000 syslog.info klogd: 
> rfcomm_sock_accept: error
> -512
> 
> So bluez_accept_unlink() is getting called. But if the socket 
> was really
> unlinked, it wouldn't be in /proc, would it?
> 
> Could the problem be the order of release_sock() and 
> bluez_accept_unlink()
> in the second if() in the code snippet from af_bluetooth.c below?
> Specifically, does bluez_accept_unlink() need to be called on 
> an unlocked
> socket?
> 
> Thanks,
> 
> -Daryl.
> 
> struct sock *bluez_accept_dequeue(struct sock *parent, struct socket
> *newsock)
> {
> 	struct list_head *p, *n;
> 	struct bluez_pinfo *pi;
> 	struct sock *sk;
> 	
> 	BT_DBG("parent %p", parent);
> 
> 	list_for_each_safe(p, n, &bluez_pi(parent)->accept_q) {
> 		pi = list_entry(p, struct bluez_pinfo, accept_q);
> 		sk = bluez_sk(pi);
> 		
> 		lock_sock(sk);
> 		if (sk->state == BT_CLOSED) {
> 			release_sock(sk);
> 			bluez_accept_unlink(sk);
> 			continue;
> 		}
> 		
> 		if (sk->state == BT_CONNECTED || !newsock) {
> 			bluez_accept_unlink(sk);
> 			if (newsock)
> 				sock_graft(sk, newsock);
> 			release_sock(sk);
> 			return sk;
> 		}
> 		release_sock(sk);
> 	}
> 	return NULL;
> }
> 
> 
> > -----Original Message-----
> > From: bluez-devel-admin@lists.sourceforge.net 
> > [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of 
> > Daryl Van Vorst
> > Sent: September 13, 2004 12:06 PM
> > To: 'Marcel Holtmann'
> > Cc: 'BlueZ Mailing List'
> > Subject: RE: [Bluez-devel] Rfcomm use count
> > 
> > 
> > Hi Marcel,
> > 
> > I have attached a log for you to look at if you have some 
> > time. If not,
> > maybe just answer my question below. :)
> > 
> > Here's what I see:
> > 
> > One stray socket. I added the socket pointer to the proc output.
> > Proc/bluetooth/rfcomm:
> > sk  2C:02:5F:16:05:00 3A:A4:58:16:05:00 9 1 c3b69340
> > 
> > I have a setup where I kill my app after a random time 
> interval while
> > several devices are connecting, disconnecting, and 
> > transfering data, etc.
> > 
> > Here's a brief version of the log:
> > 
> > 1. Program gets kill signal (time 15:09:30)
> > 2. HCI devices get shut down
> > 3. Some data remaining for transmission is sent.
> > 4. One listen socket is closed (I think this is the one which 
> > is not being
> > used)
> > 5. Some sockets/dlcs get closed
> > 6. An incomming connection is received (rfcomm_connect_ind is 
> > called, and a
> > socket is created which matches the one in proc)
> > 7. Some more sockets/dlcs get closed
> > 8. The listen (probably the one which is being used) gets closed
> > 
> > I don't see any lines from rfcomm_sock_accept after the 
> > rfcomm_connect_ind
> > line. And rfcomm_sock_release is never called for the new 
> connection.
> > 
> > My knowledge here is limited, and I may be mis-interpreting 
> > the log. But it
> > appears that a socket is allocated for the new connection as 
> > long as there
> > is room in the wait queue. And if that connection is never 
> > accepted, is it
> > the job of rfcomm_sock_cleanup_listen() to deal with it? That 
> > is, it is not
> > the kernel's duty (outside of the rfcomm module) to deal with 
> > the allocated
> > socket?
> > 
> > So, I think, it appears that rfcomm_sock_cleanup_listen isn't 
> > working right.
> > 
> > Not sure if rfcomm_sock_accept is still looping or not while 
> > this is going
> > on.
> > 
> > More digging needed...
> > 
> > Thanks,
> > 
> > -Daryl.
> > 
> > > -----Original Message-----
> > > From: Daryl Van Vorst [mailto:daryl@wideray.com] 
> > > Sent: September 13, 2004 9:37 AM
> > > To: 'Marcel Holtmann'
> > > Subject: RE: [Bluez-devel] Rfcomm use count
> > > 
> > > 
> > > Hi Marcel,
> > > 
> > > > I haven't had time to look at your problem in the last 
> > two weeks and
> > > > dealing with ARM related stuff still not fits into my left 
> > > > free time for
> > > > the next weeks. Is this behaviour reproduceable on x86 
> > > machines and do
> > > > you have a small text program to trigger this effect? And 
> > > > what I really
> > > > care about, is this problem also available with a 2.6 kernel?
> > > 
> > > Thanks for the response.
> > > 
> > > I have not yet tried to reproduce it on an x86. And 
> > > unfortunately I don't have a good way to trigger the effect. 
> > > I have spent quite a lot of time trying to come up with a 
> > > simple way (or any way) to reliably reproduce the problem. 
> > > Depending on how the next while plays out, I may try to 
> > > reproduce it on an x86.
> > > 
> > > I would like to be working with a 2.6 kernel, but time 
> > > constraints have prevented moving to it (would require 
> > > porting drivers). Maybe on an x86 if time permits.
> > > 
> > > I'm currently sifting through RFCOMM debug output. When I get 
> > > something interesting I'll send it along. I should have 
> > > something shortly.
> > > 
> > > -Daryl.
> > > 
> > > 
> > 
> 

RE: [Bluez-devel] Rfcomm use count

[Marcel Holtmann]

Hi Daryl,

> I just noticed that bluez_accept_unlink() was called with a socked that was
> in state BT_CLOSED, so it can't be the second if(). It would have been
> called from the first if().
> 
> Clearly, I'm on glue.
> 
> Still, does the order of bluez_accept_unlink() and sock_release() matter?

I haven't done a full audit yet, but try to switch the order and see
what happens. From a first look at it it makes sense to only release the
socket when all the work is done, because otherwise lock_sock() makes no
sense.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM. 
Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

Marcel,

That didn't work, but I think I found the problem. Not sure what the =
correct
solution is though.

Rfcomm_sock_cleanup_listen() calls bluez_accept_dequeue() in a loop =
until
bluez_accept_dequeue() has no more sockets to return. For each socket =
that
bluez_accept_dequeue() returns, rfcomm_sock_cleanup_listen() closes and
kills it. Killing it unlinks it from rfcomm_sk_list.

But if the socket is already closed (sk->state =3D=3D BT_CLOSED), then
bluez_accept_dequeue() does not return the socket (it just continues to =
the
next one in the queue). Bluez_accept_dequeue() does unlink it from the
accept queue, but not from rfcomm_sk_list.

I suppose the first question is: Is the bug in cleanup/dequeue, or is =
the
bug elsewhere (i.e.: things shouldn't ever be in quite that state when
cleanup_listen is called)?

Also, should rfcomm_sock_destruct() be called to decrement the use =
count?

-Daryl.

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@holtmann.org]=20
> Sent: September 14, 2004 2:18 AM
> To: Daryl Van Vorst
> Cc: 'BlueZ Mailing List'
> Subject: RE: [Bluez-devel] Rfcomm use count
>=20
>=20
> Hi Daryl,
>=20
> > I just noticed that bluez_accept_unlink() was called with a=20
> socked that was
> > in state BT_CLOSED, so it can't be the second if(). It=20
> would have been
> > called from the first if().
> >=20
> > Clearly, I'm on glue.
> >=20
> > Still, does the order of bluez_accept_unlink() and=20
> sock_release() matter?
>=20
> I haven't done a full audit yet, but try to switch the order and see
> what happens. From a first look at it it makes sense to only=20
> release the
> socket when all the work is done, because otherwise=20
> lock_sock() makes no
> sense.
>=20
> Regards
>=20
> Marcel
>=20
>=20
>=20

[Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

I have a simple way to reproduce at least part of this bug. I don't have =
an
up-to-date x86 machine to try this on, but I suspect you'll see the same
behaviour:

1. Compile and run the attached code on one machine
2. Connect to it from another machine using: rctest -n -P1 <bd_addr>
3. Hit ctrl-c on rctest
4. Hit ctrl-c on bzt (or whatever you called the compiled code)
5. lsmod and look at the rfcomm use count.

I think the problem stems from rfcomm_cleanup_listen() and
bluez_accept_dequeue(). Bluez_accept_dequeue() won't return the socket =
if it
is in the closed state, and so rfcomm_cleanup_listen() can't fully =
cleanup.

And if accept is called before rfcomm_cleanup_listen(), then (I think) =
the
socket will be unlinked from the accept queue (by accept) but not =
killed,
and so also will not get cleaned up.

Things appear to work if you reverse the order of steps 3 and 4.

I'd send you a patch if I had a simple one, but I don't know what the =
best
approach is. On solution may be to make bluez_accept_dequeue() always =
return
the socket regardless of state and then fix anything that calls
bluez_accept_dequeue() to handle the possibility of a closed socket =
being
returned.

-Daryl.

--- test code below ---
// bzt.c

#include <stdio.h>
#include <errno.h>
#include <sys/socket.h>

#include <bluetooth/bluetooth.h>
#include <bluetooth/rfcomm.h>

int main(int argc, char *argv[])
{
	struct sockaddr_rc loc_addr;
	int s;
=09
	if((s =3D socket(PF_BLUETOOTH, SOCK_STREAM, BTPROTO_RFCOMM)) < 0) {
		printf("Can't create server socket:
%s(%d)\n",strerror(errno), errno);
		return -1;
	}
=09
	loc_addr.rc_family =3D AF_BLUETOOTH;
	bacpy(&loc_addr.rc_bdaddr, BDADDR_ANY);
	loc_addr.rc_channel =3D 1;
	if(bind(s,(struct sockaddr *) &loc_addr, sizeof(loc_addr)) < 0) {
		printf("Can't bind %s(%d)\n",strerror(errno), errno);
		return -1;
	}
=09
	if(listen(s,10)) {
		printf("Can't listen %s(%d)\n",strerror(errno),errno);
		return -1;
	}
=09
	printf("Listening...\n");
	while(1) sleep(999);
=09
	return 0;
}



-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> I have a simple way to reproduce at least part of this bug. I don't have an
> up-to-date x86 machine to try this on, but I suspect you'll see the same
> behaviour:
> 
> 1. Compile and run the attached code on one machine
> 2. Connect to it from another machine using: rctest -n -P1 <bd_addr>
> 3. Hit ctrl-c on rctest
> 4. Hit ctrl-c on bzt (or whatever you called the compiled code)
> 5. lsmod and look at the rfcomm use count.
> 
> I think the problem stems from rfcomm_cleanup_listen() and
> bluez_accept_dequeue(). Bluez_accept_dequeue() won't return the socket if it
> is in the closed state, and so rfcomm_cleanup_listen() can't fully cleanup.
> 
> And if accept is called before rfcomm_cleanup_listen(), then (I think) the
> socket will be unlinked from the accept queue (by accept) but not killed,
> and so also will not get cleaned up.
> 
> Things appear to work if you reverse the order of steps 3 and 4.

I can verify that this bug exists even in a 2.6 kernel. There is no need
to execute step 4, because the missing decrementing of the use count is
already there after step 3. And I think the same problem will exists for
the L2CAP module. Please verify this for me.

> I'd send you a patch if I had a simple one, but I don't know what the best
> approach is. On solution may be to make bluez_accept_dequeue() always return
> the socket regardless of state and then fix anything that calls
> bluez_accept_dequeue() to handle the possibility of a closed socket being
> returned.

At the moment I must admit that I have no idea how to fix this in a sane
way. It seems that this bug is in there from the beginning and a wrong
fix can cause unexpected side effects.

I don't think that the problem is in rfcomm_sock_cleanup_listen(),
because the wrong use count is already present after step 3. So when we
close a connected DLC that is not accepted yet, we still have it on the
accept queue then we have a problem. Maybe there is a bug in our state
machine and this is not socket related.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> At the moment I must admit that I have no idea how to fix=20
> this in a sane
> way. It seems that this bug is in there from the beginning and a wrong
> fix can cause unexpected side effects.
>=20
> I don't think that the problem is in rfcomm_sock_cleanup_listen(),
> because the wrong use count is already present after step 3.=20
> So when we
> close a connected DLC that is not accepted yet, we still have=20
> it on the
> accept queue then we have a problem. Maybe there is a bug in our state
> machine and this is not socket related.

Incoming connections must be added to the accept queue (unless I'm =
really
missing something). So the issue is just what to do when the remote side
closes them before accept() gets to them.

Making bluez_accept_dequeue() return sockets regardless of state is a
potential solution. Accept() for rfcomm and l2cap would then need to be
modified to kill already closed sockets. The existing loop in the =
accept()'s
would need to be modified or a new one added to handle
bluez_accept_dequeue() not always returning an open socket.

I may not have been clear about my thoughts on =
rfcomm_sock_cleanup_listen().
If bluez_accept_dequeue() did return sockets regardless of state, then
rfcomm_sock_cleanup_listen() should work (unless calling close on an =
already
closed socket causes trouble). When it calls rfcomm_sock_kill(), =
sock_put()
gets called which calls destruct() which should decrement the use count.

What if bluez_accept_dequeue() called sk->shutdown() on sockets which =
are
already closed in the accept queue?

I'll try out l2cap later for you. We should see the same thing.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > At the moment I must admit that I have no idea how to fix 
> > this in a sane
> > way. It seems that this bug is in there from the beginning and a wrong
> > fix can cause unexpected side effects.
> > 
> > I don't think that the problem is in rfcomm_sock_cleanup_listen(),
> > because the wrong use count is already present after step 3. 
> > So when we
> > close a connected DLC that is not accepted yet, we still have 
> > it on the
> > accept queue then we have a problem. Maybe there is a bug in our state
> > machine and this is not socket related.
> 
> Incoming connections must be added to the accept queue (unless I'm really
> missing something). So the issue is just what to do when the remote side
> closes them before accept() gets to them.

that is correct.

> Making bluez_accept_dequeue() return sockets regardless of state is a
> potential solution. Accept() for rfcomm and l2cap would then need to be
> modified to kill already closed sockets. The existing loop in the accept()'s
> would need to be modified or a new one added to handle
> bluez_accept_dequeue() not always returning an open socket.

I don't like that idea and the problem is not in ...cleanup_listen() and
so I am not sure if it will really help us.

> I may not have been clear about my thoughts on rfcomm_sock_cleanup_listen().
> If bluez_accept_dequeue() did return sockets regardless of state, then
> rfcomm_sock_cleanup_listen() should work (unless calling close on an already
> closed socket causes trouble). When it calls rfcomm_sock_kill(), sock_put()
> gets called which calls destruct() which should decrement the use count.

The ...cleanup_listen() kicks in when you stop the listening server, but
that is far too late. The problematic part already happened. We close a
connection that wasn't accepted. We must fix it there and not afterwards
like somekind of garbage collection. This is the kernel and not some
weird Jave/C++ code.

> What if bluez_accept_dequeue() called sk->shutdown() on sockets which are
> already closed in the accept queue?

I am not sure about the side effects, but this can be a solution. What I
think is that setting the socket to BT_CLOSED is wrong. Maybe using
BT_DISCONN is better, but this involves changing the state machine.

> I'll try out l2cap later for you. We should see the same thing.

I am not sure anymore. Maybe this is only RFCOMM related.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> > Making bluez_accept_dequeue() return sockets regardless of=20
> state is a
> > potential solution. Accept() for rfcomm and l2cap would=20
> then need to be
> > modified to kill already closed sockets. The existing loop=20
> in the accept()'s
> > would need to be modified or a new one added to handle
> > bluez_accept_dequeue() not always returning an open socket.
>=20
> I don't like that idea and the problem is not in=20
> ...cleanup_listen() and
> so I am not sure if it will really help us.

Fair enough. Though I'm talking about changing bluez_accept_dequeue(), =
not
necessarily cleanup_listen().

> > I may not have been clear about my thoughts on=20
> rfcomm_sock_cleanup_listen().
> > If bluez_accept_dequeue() did return sockets regardless of=20
> state, then
> > rfcomm_sock_cleanup_listen() should work (unless calling=20
> close on an already
> > closed socket causes trouble). When it calls=20
> rfcomm_sock_kill(), sock_put()
> > gets called which calls destruct() which should decrement=20
> the use count.
>=20
> The ...cleanup_listen() kicks in when you stop the listening=20
> server, but
> that is far too late. The problematic part already happened.=20
> We close a
> connection that wasn't accepted. We must fix it there and not=20
> afterwards
> like somekind of garbage collection. This is the kernel and not some
> weird Jave/C++ code.

With my suggestion above the condition would be handled by either =
accept()
or cleanup_listen(), whichever is called first. Though I guess you could
still consider that to be garbage collection.

> > What if bluez_accept_dequeue() called sk->shutdown() on=20
> sockets which are
> > already closed in the accept queue?
>=20
> I am not sure about the side effects, but this can be a=20
> solution. What I
> think is that setting the socket to BT_CLOSED is wrong. Maybe using
> BT_DISCONN is better, but this involves changing the state machine.

Calling sk->shutdown() is bluez_accept_dequeue() would be virtually the =
same
timing (i.e. garbage collecting likeness) as the suggestion above =
(returning
the closed socket in ...accept_dequeue). It would happen only when =
accept()
or cleanup_listen() are called.

> > I'll try out l2cap later for you. We should see the same thing.
>=20
> I am not sure anymore. Maybe this is only RFCOMM related.

I sent another e-mail a moment ago...  It doesn't happen with l2cap (or =
at
least I wasn't able to get it to happen).

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > > I may not have been clear about my thoughts on 
> > rfcomm_sock_cleanup_listen().
> > > If bluez_accept_dequeue() did return sockets regardless of 
> > state, then
> > > rfcomm_sock_cleanup_listen() should work (unless calling 
> > close on an already
> > > closed socket causes trouble). When it calls 
> > rfcomm_sock_kill(), sock_put()
> > > gets called which calls destruct() which should decrement 
> > the use count.
> > 
> > The ...cleanup_listen() kicks in when you stop the listening 
> > server, but
> > that is far too late. The problematic part already happened. 
> > We close a
> > connection that wasn't accepted. We must fix it there and not 
> > afterwards
> > like somekind of garbage collection. This is the kernel and not some
> > weird Jave/C++ code.
> 
> With my suggestion above the condition would be handled by either accept()
> or cleanup_listen(), whichever is called first. Though I guess you could
> still consider that to be garbage collection.

when it is handled in ..cleanup_listen() then it gets handled when you
kill the server process. I think this is far too late, because you can
have more than one connection attempts in between. We must handle this
when we close the connection.

> > > What if bluez_accept_dequeue() called sk->shutdown() on 
> > sockets which are
> > > already closed in the accept queue?
> > 
> > I am not sure about the side effects, but this can be a 
> > solution. What I
> > think is that setting the socket to BT_CLOSED is wrong. Maybe using
> > BT_DISCONN is better, but this involves changing the state machine.
> 
> Calling sk->shutdown() is bluez_accept_dequeue() would be virtually the same
> timing (i.e. garbage collecting likeness) as the suggestion above (returning
> the closed socket in ...accept_dequeue). It would happen only when accept()
> or cleanup_listen() are called.

This is correct and so I don't really like this approach. I think there
is a simply logic mistake in the closing path of the RFCOMM code.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

> > With my suggestion above the condition would be handled by=20
> either accept()
> > or cleanup_listen(), whichever is called first. Though I=20
> guess you could
> > still consider that to be garbage collection.
>=20
> when it is handled in ..cleanup_listen() then it gets handled when you
> kill the server process. I think this is far too late, because you can
> have more than one connection attempts in between. We must handle this
> when we close the connection.

Agreed, but if bluez_accept_dequeue() returns closed connections then it =
can
also be handled in accept(). So this would only be an issue in =
situations
where a socket is listening for a long time without there ever being a =
call
to accept().

The topic of cleanup_listen() came up just because the only way it has
appeared for me in normal use was with incomming connections while the
server was being shut down. While the server is running, incomming
connections are returned by accept() before they get closed by the =
remote
side.

It is still unclear to me why I could not bind to the socket (error was
address already in use, 98) when I tried to restart the server. This
behaviour is not seen with the simple test program I sent last week (and =
so
might be unrelated).

> This is correct and so I don't really like this approach. I=20
> think there
> is a simply logic mistake in the closing path of the RFCOMM code.

Could be. Then the closing path of the rfcomm code would need to be able =
to
remove connections from the accept queue. This might be a difficult =
change.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

I can't get it to happen with l2cap. But I did notice that it takes a few
seconds for the use count to go down after the listening program exits.

-Daryl.

> -----Original Message-----
> From: bluez-devel-admin@lists.sourceforge.net 
> [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of 
> Daryl Van Vorst
> Sent: September 20, 2004 10:59 AM
> To: 'Marcel Holtmann'
> Cc: 'BlueZ Mailing List'
> Subject: RE: [Bluez-devel] Rfcomm Use Count
> 
> 
> Hi Marcel,
> 
> > At the moment I must admit that I have no idea how to fix 
> > this in a sane
> > way. It seems that this bug is in there from the beginning 
> and a wrong
> > fix can cause unexpected side effects.
> > 
> > I don't think that the problem is in rfcomm_sock_cleanup_listen(),
> > because the wrong use count is already present after step 3. 
> > So when we
> > close a connected DLC that is not accepted yet, we still have 
> > it on the
> > accept queue then we have a problem. Maybe there is a bug 
> in our state
> > machine and this is not socket related.
> 
> Incoming connections must be added to the accept queue 
> (unless I'm really
> missing something). So the issue is just what to do when the 
> remote side
> closes them before accept() gets to them.
> 
> Making bluez_accept_dequeue() return sockets regardless of state is a
> potential solution. Accept() for rfcomm and l2cap would then 
> need to be
> modified to kill already closed sockets. The existing loop in 
> the accept()'s
> would need to be modified or a new one added to handle
> bluez_accept_dequeue() not always returning an open socket.
> 
> I may not have been clear about my thoughts on 
> rfcomm_sock_cleanup_listen().
> If bluez_accept_dequeue() did return sockets regardless of state, then
> rfcomm_sock_cleanup_listen() should work (unless calling 
> close on an already
> closed socket causes trouble). When it calls 
> rfcomm_sock_kill(), sock_put()
> gets called which calls destruct() which should decrement the 
> use count.
> 
> What if bluez_accept_dequeue() called sk->shutdown() on 
> sockets which are
> already closed in the accept queue?
> 
> I'll try out l2cap later for you. We should see the same thing.
> 
> -Daryl.
> 
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
> Project Admins to receive an Apple iPod Mini FREE for your 
> judgement on
> who ports your project to Linux PPC the best. Sponsored by IBM.
> Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
> 

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> I can't get it to happen with l2cap. But I did notice that it takes a few
> seconds for the use count to go down after the listening program exits.

what do you see when you keep the listing program running and then do
some connects? Do the module reference count increases every time?

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

> > I can't get it to happen with l2cap. But I did notice that 
> it takes a few
> > seconds for the use count to go down after the listening 
> program exits.
> 
> what do you see when you keep the listing program running and then do
> some connects? Do the module reference count increases every time?

Each time I connect with l2test -n, I see the use count go up by 2. When I
hit ctrl-c on l2test, the use count goes down by 1 after a few seconds.
These excess increments (one for each connection) don't go away until the
test program is killed.

So there is a bit of a problem.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > > I can't get it to happen with l2cap. But I did notice that 
> > it takes a few
> > > seconds for the use count to go down after the listening 
> > program exits.
> > 
> > what do you see when you keep the listing program running and then do
> > some connects? Do the module reference count increases every time?
> 
> Each time I connect with l2test -n, I see the use count go up by 2. When I
> hit ctrl-c on l2test, the use count goes down by 1 after a few seconds.
> These excess increments (one for each connection) don't go away until the
> test program is killed.
> 
> So there is a bit of a problem.

then we use some kind of garbage collection already. This is bad and we
shouldn't do so, but right now I don't see the correct fix for this
behaviour.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

> > Each time I connect with l2test -n, I see the use count go 
> up by 2. When I
> > hit ctrl-c on l2test, the use count goes down by 1 after a 
> few seconds.
> > These excess increments (one for each connection) don't go 
> away until the
> > test program is killed.
> > 
> > So there is a bit of a problem.
> 
> then we use some kind of garbage collection already. This is 
> bad and we
> shouldn't do so, but right now I don't see the correct fix for this
> behaviour.

I just tried an experiment where I listen for a while before calling
accept(). I can raise the use cound by connecting, disconnecting, etc (as
before). As soon as accept() is called, the excess use count goes away.

Before accept() is called, the server will start refusing connections after
you fill up the accept queue. The queue is emptied with a single call to
accept().

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> I just tried an experiment where I listen for a while before calling
> accept(). I can raise the use cound by connecting, disconnecting, etc (as
> before). As soon as accept() is called, the excess use count goes away.
> 
> Before accept() is called, the server will start refusing connections after
> you fill up the accept queue. The queue is emptied with a single call to
> accept().

so this means when we disconnect we must ensure that we also remove this
connection from the accept queue if it is pending there.

What I know think is that calling the bt_accept_unlink() function only
from bt_accept_dequeue() is wrong. If bt_sk(sk)->parent is set and we
ran into a disconnect then we must unlink it by ourself. Does this make
sense?

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> > I just tried an experiment where I listen for a while before calling
> > accept(). I can raise the use cound by connecting,=20
> disconnecting, etc (as
> > before). As soon as accept() is called, the excess use=20
> count goes away.
> >=20
> > Before accept() is called, the server will start refusing=20
> connections after
> > you fill up the accept queue. The queue is emptied with a=20
> single call to
> > accept().
>=20
> so this means when we disconnect we must ensure that we also=20
> remove this
> connection from the accept queue if it is pending there.
>=20
> What I know think is that calling the bt_accept_unlink() function only
> from bt_accept_dequeue() is wrong. If bt_sk(sk)->parent is set and we
> ran into a disconnect then we must unlink it by ourself. Does=20
> this make
> sense?

This makes sense to me, but I'm not convinced that it's enough of a =
problem
to warrant major changes. And I'm probably not the best person to ask. =
;)

Do we know how the tcp stack handles this kind of thing? (I had a quick =
look
at a text on sockets and it didn't specifically cover the case of
connections getting closed which are in the queue. But it was clear that =
the
precise behaviour of the queue varies for tcp-ip from unix to unix.)

Most servers (I think) would sit with accept() blocking and then spend a
very brief time handing off the new connection before blocking on =
accept()
again. So this wouldn't cause much trouble.

I suppose this could affect a very simple server which is =
single-threaded
(and so could spend significant time between accept() calls). Or a very =
busy
server. I'm not sure that this is really a major issue for bluetooth =
(the
bandwith and number of real connections is quite limited compared to
tcp-ip).

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1807 bytes --]

Hi Daryl,

> > so this means when we disconnect we must ensure that we also 
> > remove this
> > connection from the accept queue if it is pending there.
> > 
> > What I know think is that calling the bt_accept_unlink() function only
> > from bt_accept_dequeue() is wrong. If bt_sk(sk)->parent is set and we
> > ran into a disconnect then we must unlink it by ourself. Does 
> > this make
> > sense?
> 
> This makes sense to me, but I'm not convinced that it's enough of a problem
> to warrant major changes. And I'm probably not the best person to ask. ;)

for the L2CAP part the attached patch may fixes the problem and let the
module reference count decrease after the disconnect. The patch is only
compile tested. May you wanna give it a chance and run your L2CAP tests
again.

> Do we know how the tcp stack handles this kind of thing? (I had a quick look
> at a text on sockets and it didn't specifically cover the case of
> connections getting closed which are in the queue. But it was clear that the
> precise behaviour of the queue varies for tcp-ip from unix to unix.)
> 
> Most servers (I think) would sit with accept() blocking and then spend a
> very brief time handing off the new connection before blocking on accept()
> again. So this wouldn't cause much trouble.
> 
> I suppose this could affect a very simple server which is single-threaded
> (and so could spend significant time between accept() calls). Or a very busy
> server. I'm not sure that this is really a major issue for bluetooth (the
> bandwith and number of real connections is quite limited compared to
> tcp-ip).

The queueing for the accept case is different and so you can't really
compare Bluetooth with TCP/IP. Maybe we made a design mistake in the
early days of BlueZ that will now hit us very badly.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1437 bytes --]

===== include/net/bluetooth/bluetooth.h 1.18 vs edited =====
--- 1.18/include/net/bluetooth/bluetooth.h	2004-07-04 17:27:14 +02:00
+++ edited/include/net/bluetooth/bluetooth.h	2004-09-21 01:26:06 +02:00
@@ -133,6 +133,7 @@
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk);
+void bt_accept_unlink(struct sock *sk);
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
===== net/bluetooth/af_bluetooth.c 1.36 vs edited =====
--- 1.36/net/bluetooth/af_bluetooth.c	2004-07-13 15:39:15 +02:00
+++ edited/net/bluetooth/af_bluetooth.c	2004-09-21 01:26:17 +02:00
@@ -165,7 +165,7 @@
 }
 EXPORT_SYMBOL(bt_accept_enqueue);
 
-static void bt_accept_unlink(struct sock *sk)
+void bt_accept_unlink(struct sock *sk)
 {
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
@@ -174,6 +174,7 @@
 	bt_sk(sk)->parent = NULL;
 	sock_put(sk);
 }
+EXPORT_SYMBOL(bt_accept_unlink);
 
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 {
===== net/bluetooth/l2cap.c 1.44 vs edited =====
--- 1.44/net/bluetooth/l2cap.c	2004-09-04 12:51:17 +02:00
+++ edited/net/bluetooth/l2cap.c	2004-09-21 01:23:41 +02:00
@@ -1005,9 +1005,10 @@
 	if (err)
 		sk->sk_err = err;
 
-	if (parent)
+	if (parent) {
+		bt_accept_unlink(sk);
 		parent->sk_data_ready(parent, 0);
-	else
+	} else
 		sk->sk_state_change(sk);
 }
 

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> for the L2CAP part the attached patch may fixes the problem 
> and let the
> module reference count decrease after the disconnect. The 
> patch is only
> compile tested. May you wanna give it a chance and run your 
> L2CAP tests
> again.

I gave it a try and it works fine as far as I can tell. I had to apply the
patch manually though (I'm assuming this is because you're working with a
2.6 kernel and I'm not).

I don't see the use count incrementing badly any more, and the listening
socket will continue to accept connections after the number of
connects/disconnects have exceeded the the accept queue.

Does this give you any ideas for rfcomm?

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

Hi Daryl,

> > for the L2CAP part the attached patch may fixes the problem 
> > and let the
> > module reference count decrease after the disconnect. The 
> > patch is only
> > compile tested. May you wanna give it a chance and run your 
> > L2CAP tests
> > again.
> 
> I gave it a try and it works fine as far as I can tell. I had to apply the
> patch manually though (I'm assuming this is because you're working with a
> 2.6 kernel and I'm not).

I will fix 2.6 first, before I will backport it to 2.4.

> I don't see the use count incrementing badly any more, and the listening
> socket will continue to accept connections after the number of
> connects/disconnects have exceeded the the accept queue.

This sounds great and hopefully this patch has no negative side effects
on normal listen/accept calls and outgoing connections.

> Does this give you any ideas for rfcomm?

Attached is the full patch that should also fix the RFCOMM part. This is
again only compile tested, because I had no time to setup a test system.
The L2CAP solution looks very sane to me, but I am not sure about the
RFCOMM part and we may will have a locking problem. This really needs
intensive testing.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1911 bytes --]

===== include/net/bluetooth/bluetooth.h 1.18 vs edited =====
--- 1.18/include/net/bluetooth/bluetooth.h	2004-07-04 17:27:14 +02:00
+++ edited/include/net/bluetooth/bluetooth.h	2004-09-21 01:26:06 +02:00
@@ -133,6 +133,7 @@
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk);
+void bt_accept_unlink(struct sock *sk);
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
===== net/bluetooth/af_bluetooth.c 1.36 vs edited =====
--- 1.36/net/bluetooth/af_bluetooth.c	2004-07-13 15:39:15 +02:00
+++ edited/net/bluetooth/af_bluetooth.c	2004-09-21 01:26:17 +02:00
@@ -165,7 +165,7 @@
 }
 EXPORT_SYMBOL(bt_accept_enqueue);
 
-static void bt_accept_unlink(struct sock *sk)
+void bt_accept_unlink(struct sock *sk)
 {
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
@@ -174,6 +174,7 @@
 	bt_sk(sk)->parent = NULL;
 	sock_put(sk);
 }
+EXPORT_SYMBOL(bt_accept_unlink);
 
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 {
===== net/bluetooth/l2cap.c 1.44 vs edited =====
--- 1.44/net/bluetooth/l2cap.c	2004-09-04 12:51:17 +02:00
+++ edited/net/bluetooth/l2cap.c	2004-09-21 01:23:41 +02:00
@@ -1005,9 +1005,10 @@
 	if (err)
 		sk->sk_err = err;
 
-	if (parent)
+	if (parent) {
+		bt_accept_unlink(sk);
 		parent->sk_data_ready(parent, 0);
-	else
+	} else
 		sk->sk_state_change(sk);
 }
 
===== net/bluetooth/rfcomm/sock.c 1.29 vs edited =====
--- 1.29/net/bluetooth/rfcomm/sock.c	2004-06-04 02:41:47 +02:00
+++ edited/net/bluetooth/rfcomm/sock.c	2004-09-21 02:09:47 +02:00
@@ -104,8 +104,11 @@
 		if (d->state == BT_CONNECTED)
 			rfcomm_session_getaddr(d->session, &bt_sk(sk)->src, NULL);
 		sk->sk_state_change(sk);
-	} else
+	} else {
+		if (d->state == BT_CLOSED)
+			bt_accept_unlink(sk);
 		parent->sk_data_ready(parent, 0);
+	}
 
 	bh_unlock_sock(sk);
 }

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> I will fix 2.6 first, before I will backport it to 2.4.

Sounds good.

> Attached is the full patch that should also fix the RFCOMM 
> part. This is
> again only compile tested, because I had no time to setup a 
> test system.
> The L2CAP solution looks very sane to me, but I am not sure about the
> RFCOMM part and we may will have a locking problem. This really needs
> intensive testing.

Excellent. Thank you very much.

I'll let you know how it works for me.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> > The L2CAP solution looks very sane to me, but I am not sure=20
> about the
> > RFCOMM part and we may will have a locking problem. This=20
> really needs
> > intensive testing.
>=20
> Excellent. Thank you very much.
>=20
> I'll let you know how it works for me.

Looks like the rfcomm part isn't quite there. The accept queue doesn't =
fill
up, but the use count still increases. And /proc/bluetooth/rfcomm still
lists all of the closed sockets.

I added rfcomm_sock_close() and rfcomm_sock_kill() right after your =
addition
to rfcomm_sk_state_change() and it seems to work. The affects of
rfcomm_sock_alloc() (called by ...connect_ind()) needed to be dealt =
with. I
changed it as follows:

--- mh_sock.c	2004-09-21 14:21:36.000000000 -0700
+++ sock.c	2004-09-21 14:21:47.000000000 -0700
@@ -104,8 +104,11 @@
 			rfcomm_session_getaddr(d->session,
&bluez_pi(sk)->src, NULL);
 		sk->state_change(sk);
 	} else {
-		if (d->state =3D=3D BT_CLOSED)
+		if (d->state =3D=3D BT_CLOSED) {
 			bluez_accept_unlink(sk);
+			rfcomm_sock_close(sk);
+			rfcomm_sock_kill(sk);
+		}
 		parent->data_ready(parent, 0);
 	}
=20
I am not sure about the possible negative side effects of this. What do =
you
think?

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> Looks like the rfcomm part isn't quite there. The accept queue doesn't fill
> up, but the use count still increases. And /proc/bluetooth/rfcomm still
> lists all of the closed sockets.
> 
> I added rfcomm_sock_close() and rfcomm_sock_kill() right after your addition
> to rfcomm_sk_state_change() and it seems to work. The affects of
> rfcomm_sock_alloc() (called by ...connect_ind()) needed to be dealt with. I
> changed it as follows:
> 
> --- mh_sock.c	2004-09-21 14:21:36.000000000 -0700
> +++ sock.c	2004-09-21 14:21:47.000000000 -0700
> @@ -104,8 +104,11 @@
>  			rfcomm_session_getaddr(d->session,
> &bluez_pi(sk)->src, NULL);
>  		sk->state_change(sk);
>  	} else {
> -		if (d->state == BT_CLOSED)
> +		if (d->state == BT_CLOSED) {
>  			bluez_accept_unlink(sk);
> +			rfcomm_sock_close(sk);
> +			rfcomm_sock_kill(sk);
> +		}
>  		parent->data_ready(parent, 0);
>  	}
>  
> I am not sure about the possible negative side effects of this. What do you
> think?

what I think is that we have two different bugs here. We may solved the
first one now, which wasn't a module reference count bug. The second now
increases the module reference count and don't decreases it correctly.
Since the RFCOMM implementation is much more complex, because of its
support for TTY and sockets as end user interfaces, we need to find out
if the socket functions are involved or the core/TTY functions.

At the moment I think calling ...sock_close() and ...sock_kill() in that
function is wrong. However I am not 100% sure.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> At the moment I think calling ...sock_close() and ...sock_kill() in that
> function is wrong. However I am not 100% sure.

maybe this one helps:

--- 1.29/net/bluetooth/rfcomm/sock.c    2004-06-04 02:41:47 +02:00
+++ edited/net/bluetooth/rfcomm/sock.c  2004-09-22 00:12:34 +02:00
@@ -104,8 +104,13 @@
                if (d->state == BT_CONNECTED)
                        rfcomm_session_getaddr(d->session, &bt_sk(sk)->src, NULL);
                sk->sk_state_change(sk);
-       } else
+       } else {
+               if (d->state == BT_CLOSED) {
+                       bt_accept_unlink(sk);
+                       sk->sk_zapped = 1;
+               }
                parent->sk_data_ready(parent, 0);
+       }
 
        bh_unlock_sock(sk);
 }

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> maybe this one helps:
>=20
> --- 1.29/net/bluetooth/rfcomm/sock.c    2004-06-04 02:41:47 +02:00
> +++ edited/net/bluetooth/rfcomm/sock.c  2004-09-22 00:12:34 +02:00
> @@ -104,8 +104,13 @@
>                 if (d->state =3D=3D BT_CONNECTED)
>                         rfcomm_session_getaddr(d->session,=20
> &bt_sk(sk)->src, NULL);
>                 sk->sk_state_change(sk);
> -       } else
> +       } else {
> +               if (d->state =3D=3D BT_CLOSED) {
> +                       bt_accept_unlink(sk);
> +                       sk->sk_zapped =3D 1;
> +               }
>                 parent->sk_data_ready(parent, 0);
> +       }
> =20
>         bh_unlock_sock(sk);
>  }

No luck.

Same behaviour as without the close and kill calls.

I noticed in l2cap.c that after l2cap_chan_del() is called (which now =
does
the accept_unlink()), l2cap_sock_kill() is often called which decrements =
its
use count which was incremented in l2cap_sock_alloc() (much like in =
rfcomm).
But in rfcomm, there don't appear to be any calls to ...sock_kill() for
similar circumstances.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > maybe this one helps:
> > 
> > --- 1.29/net/bluetooth/rfcomm/sock.c    2004-06-04 02:41:47 +02:00
> > +++ edited/net/bluetooth/rfcomm/sock.c  2004-09-22 00:12:34 +02:00
> > @@ -104,8 +104,13 @@
> >                 if (d->state == BT_CONNECTED)
> >                         rfcomm_session_getaddr(d->session, 
> > &bt_sk(sk)->src, NULL);
> >                 sk->sk_state_change(sk);
> > -       } else
> > +       } else {
> > +               if (d->state == BT_CLOSED) {
> > +                       bt_accept_unlink(sk);
> > +                       sk->sk_zapped = 1;
> > +               }
> >                 parent->sk_data_ready(parent, 0);
> > +       }
> >  
> >         bh_unlock_sock(sk);
> >  }
> 
> No luck.
> 
> Same behaviour as without the close and kill calls.

the correct approach seems to be:

	sk->sk_zapped = 1;
	rfcomm_sock_kill(sk);

The problem is that rfcomm_sock_kill() must be called on an unlocked
socket and I think that we will deadlock on a SMP machine or get some
NULL pointer dereferences.

> I noticed in l2cap.c that after l2cap_chan_del() is called (which now does
> the accept_unlink()), l2cap_sock_kill() is often called which decrements its
> use count which was incremented in l2cap_sock_alloc() (much like in rfcomm).
> But in rfcomm, there don't appear to be any calls to ...sock_kill() for
> similar circumstances.

That is right and I agree that a ...sock_kill() is missing that calls
sock_put() to decrease the reference count.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Hi Daryl,

> the correct approach seems to be:
> 
> 	sk->sk_zapped = 1;
> 	rfcomm_sock_kill(sk);
> 
> The problem is that rfcomm_sock_kill() must be called on an unlocked
> socket and I think that we will deadlock on a SMP machine or get some
> NULL pointer dereferences.

after more and more thinking about that problem I am almost sure that it
is right to call rfcomm_sock_kill() in the state change function. Anyway
we must do this on an unlocked socket. Here is my proposal for the final
patch, but we need real testing on this so that I can be sure that there
are no side effects. Can anyone test it on SMP or HT systems?

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2199 bytes --]

===== include/net/bluetooth/bluetooth.h 1.18 vs edited =====
--- 1.18/include/net/bluetooth/bluetooth.h	2004-07-04 17:27:14 +02:00
+++ edited/include/net/bluetooth/bluetooth.h	2004-09-21 01:26:06 +02:00
@@ -133,6 +133,7 @@
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
 
 void bt_accept_enqueue(struct sock *parent, struct sock *sk);
+void bt_accept_unlink(struct sock *sk);
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);
 
 /* Skb helpers */
===== net/bluetooth/af_bluetooth.c 1.36 vs edited =====
--- 1.36/net/bluetooth/af_bluetooth.c	2004-07-13 15:39:15 +02:00
+++ edited/net/bluetooth/af_bluetooth.c	2004-09-21 01:26:17 +02:00
@@ -165,7 +165,7 @@
 }
 EXPORT_SYMBOL(bt_accept_enqueue);
 
-static void bt_accept_unlink(struct sock *sk)
+void bt_accept_unlink(struct sock *sk)
 {
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
@@ -174,6 +174,7 @@
 	bt_sk(sk)->parent = NULL;
 	sock_put(sk);
 }
+EXPORT_SYMBOL(bt_accept_unlink);
 
 struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
 {
===== net/bluetooth/l2cap.c 1.44 vs edited =====
--- 1.44/net/bluetooth/l2cap.c	2004-09-04 12:51:17 +02:00
+++ edited/net/bluetooth/l2cap.c	2004-09-21 01:23:41 +02:00
@@ -1005,9 +1005,10 @@
 	if (err)
 		sk->sk_err = err;
 
-	if (parent)
+	if (parent) {
+		bt_accept_unlink(sk);
 		parent->sk_data_ready(parent, 0);
-	else
+	} else
 		sk->sk_state_change(sk);
 }
 
===== net/bluetooth/rfcomm/sock.c 1.29 vs edited =====
--- 1.29/net/bluetooth/rfcomm/sock.c	2004-06-04 02:41:47 +02:00
+++ edited/net/bluetooth/rfcomm/sock.c	2004-09-22 13:26:26 +02:00
@@ -97,17 +97,26 @@
 
 	if (err)
 		sk->sk_err = err;
+
 	sk->sk_state = d->state;
 
 	parent = bt_sk(sk)->parent;
-	if (!parent) {
+	if (parent) {
+		if (d->state == BT_CLOSED) {
+			sk->sk_zapped = 1;
+			bt_accept_unlink(sk);
+		}
+		parent->sk_data_ready(parent, 0);
+	} else {
 		if (d->state == BT_CONNECTED)
 			rfcomm_session_getaddr(d->session, &bt_sk(sk)->src, NULL);
 		sk->sk_state_change(sk);
-	} else
-		parent->sk_data_ready(parent, 0);
+	}
 
 	bh_unlock_sock(sk);
+
+	if (parent && sk->sk_zapped)
+		rfcomm_sock_kill(sk);
 }
 
 /* ---- Socket functions ---- */

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> after more and more thinking about that problem I am almost=20
> sure that it
> is right to call rfcomm_sock_kill() in the state change=20
> function. Anyway
> we must do this on an unlocked socket. Here is my proposal=20
> for the final
> patch, but we need real testing on this so that I can be sure=20
> that there
> are no side effects. Can anyone test it on SMP or HT systems?

I manually applied your patch to my 2.4 kernel, and it seems to work =
fine. I
haven't been able to break it yet. Looks good. I will keep trying.

Does this change make the "if (sk->state =3D=3D BT_CLOSED)" statement in
bluez_accept_dequeue() redundant?

This is important because if somehow a socket is in BT_CLOSED state and =
is
in the accept queue during shutdown, then rfcomm_cleanup_listen() can't
clean it up (because bluez_accept_dequeue() won't return it). I think =
your
changes make it impossible for there to ever be an rfcomm socket (or =
l2cap
socket) in the BT_CLOSED state in the accept queue. If that's true, then
this isn't an issue. Thoughts?

Do you think we've covered the case where there are incomming =
connections
while things are at various stages of being shut down? I think we're ok, =
but
I wanted to bring it up.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> I manually applied your patch to my 2.4 kernel, and it seems to work fine. I
> haven't been able to break it yet. Looks good. I will keep trying.

try to test your normal stuff to make sure that it doesn't break
anything else.

> Does this change make the "if (sk->state == BT_CLOSED)" statement in
> bluez_accept_dequeue() redundant?

I think so. Haven't thought about it so far.

> This is important because if somehow a socket is in BT_CLOSED state and is
> in the accept queue during shutdown, then rfcomm_cleanup_listen() can't
> clean it up (because bluez_accept_dequeue() won't return it). I think your
> changes make it impossible for there to ever be an rfcomm socket (or l2cap
> socket) in the BT_CLOSED state in the accept queue. If that's true, then
> this isn't an issue. Thoughts?

Sorry, but I can't follow your point here. Please explain it again.

> Do you think we've covered the case where there are incomming connections
> while things are at various stages of being shut down? I think we're ok, but
> I wanted to bring it up.

Actually I don't know. In general we should be protected against that
with our locking.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> try to test your normal stuff to make sure that it doesn't break
> anything else.

So far so good. I'm setting up some automated testers to pound on it for =
a
while.

> > Does this change make the "if (sk->state =3D=3D BT_CLOSED)" =
statement in
> > bluez_accept_dequeue() redundant?
>=20
> I think so. Haven't thought about it so far.
>=20
> > This is important because if somehow a socket is in=20
> BT_CLOSED state and is
> > in the accept queue during shutdown, then=20
> rfcomm_cleanup_listen() can't
> > clean it up (because bluez_accept_dequeue() won't return=20
> it). I think your
> > changes make it impossible for there to ever be an rfcomm=20
> socket (or l2cap
> > socket) in the BT_CLOSED state in the accept queue. If=20
> that's true, then
> > this isn't an issue. Thoughts?
>=20
> Sorry, but I can't follow your point here. Please explain it again.

Bluez_accept_dequeue() won't return a socket if it is in the closed =
state.
So rfcomm_cleanup_listen() will never call close and kill on the socket
because ...accept_dequeue() never returns it (it just unlinks it). And =
then
we would have the use count problem. But if there is no possibility of a
socket being in the closed state on the accept queue (for rfcomm at =
least),
then this isn't an issue.

I think this is fixed - so if I'm still talking nonsense, just ignore =
it. ;)

> > Do you think we've covered the case where there are=20
> incomming connections
> > while things are at various stages of being shut down? I=20
> think we're ok, but
> > I wanted to bring it up.
>=20
> Actually I don't know. In general we should be protected against that
> with our locking.

As far as I could see the locking protects against it. But I did not =
spend
enough time on it to fully understand how it works. If there is a =
problem,
my testing should eventually uncover it.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > > Does this change make the "if (sk->state == BT_CLOSED)" statement in
> > > bluez_accept_dequeue() redundant?
> > 
> > I think so. Haven't thought about it so far.
> > 
> > > This is important because if somehow a socket is in 
> > BT_CLOSED state and is
> > > in the accept queue during shutdown, then 
> > rfcomm_cleanup_listen() can't
> > > clean it up (because bluez_accept_dequeue() won't return 
> > it). I think your
> > > changes make it impossible for there to ever be an rfcomm 
> > socket (or l2cap
> > > socket) in the BT_CLOSED state in the accept queue. If 
> > that's true, then
> > > this isn't an issue. Thoughts?
> > 
> > Sorry, but I can't follow your point here. Please explain it again.
> 
> Bluez_accept_dequeue() won't return a socket if it is in the closed state.
> So rfcomm_cleanup_listen() will never call close and kill on the socket
> because ...accept_dequeue() never returns it (it just unlinks it). And then
> we would have the use count problem. But if there is no possibility of a
> socket being in the closed state on the accept queue (for rfcomm at least),
> then this isn't an issue.
> 
> I think this is fixed - so if I'm still talking nonsense, just ignore it. ;)

I understand what you mean and it should be impossible to have an socket
on the accept_q with state BT_CLOSED. May you wanna remove the extra
code in ...accept_dequeue() and only leave a warning message there when
you do your tests. However this extra code should not harm and I don't
see an easy solution for getting the module reference decrementing right
when we have to do it in ...accept_dequeue() for all socket types.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Hi Marcel,

> I understand what you mean and it should be impossible to 
> have an socket
> on the accept_q with state BT_CLOSED. May you wanna remove the extra
> code in ...accept_dequeue() and only leave a warning message 
> there when
> you do your tests. However this extra code should not harm and I don't
> see an easy solution for getting the module reference 
> decrementing right
> when we have to do it in ...accept_dequeue() for all socket types.

I think I'll leave the code as is, unless you're considering removing the
code in accept_dequeue(). If there's a problem, we'll see the use count go
up.

Thanks for working on this...  Looks like you spent some late nights on it.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Daryl,

> > I understand what you mean and it should be impossible to 
> > have an socket
> > on the accept_q with state BT_CLOSED. May you wanna remove the extra
> > code in ...accept_dequeue() and only leave a warning message 
> > there when
> > you do your tests. However this extra code should not harm and I don't
> > see an easy solution for getting the module reference 
> > decrementing right
> > when we have to do it in ...accept_dequeue() for all socket types.
> 
> I think I'll leave the code as is, unless you're considering removing the
> code in accept_dequeue(). If there's a problem, we'll see the use count go
> up.

replace it with a BT_ERR message (or only add one), so that you see if
it gets called in your tests. I think the code is useless, but I am not
100% sure.

> Thanks for working on this...  Looks like you spent some late nights on it.

You are welcome. Yes, I spent some time to understand what is really
going on. Btw someone must look at the SCO code and check if we have the
same problem there.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM.
Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm Use Count

[Daryl Van Vorst]

Marcel,

> replace it with a BT_ERR message (or only add one), so that you see if
> it gets called in your tests. I think the code is useless, 
> but I am not
> 100% sure.

Ok, will do.

> > Thanks for working on this...  Looks like you spent some 
> late nights on it.
> 
> You are welcome. Yes, I spent some time to understand what is really
> going on. Btw someone must look at the SCO code and check if 
> we have the
> same problem there.

I'll keep that in mind if I ever do any SCO stuff... But that's pretty
unlikely at least for now.

-Daryl.

RE: [Bluez-devel] Rfcomm Use Count

[Marcel Holtmann]

Hi Diego,

> I had the same problem here.
> 
> The patch seems to solve it.
> 
> Tested rfcomm on linux 2.6 single processor.
> 
> I tried to apply the fix also to 2.4.x with
> the following patch that compiles.
> 
> I hope it is correct.
> 
> Thanks for your work.

I will do a patch for 2.4 when I resynced every pending 2.6 stuff. At
the moment I am not quite sure that even my 2.6 patch is the best
solution. However it is better then nothing.

Regards

Marcel




-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

[Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

Hi Marcel,

Any suggestions you have regarding the following would be greatly
appreciated.

I have a server program which does the following:

1. Intializes and attaches two modules via serial port (CSR 18.1)
2. Has SPP and OPP servers
3. Does some HCI-level stuff to monitor connections and close =
connections to
misbehaving devices.

The program will initiate rfcomm connections to devices which do not =
allow
role switches.

If I kill the program while several devices are connected (or are in the
process of connecting/disconnecting), sometimes the rfcomm use count is =
left
above 0 (I've seen it as high as 4). Killing the program detaches the
modules (this is verified by hcid's log output). Killing sdpd and hcid
doesn't reduce the use count. No other bluetooth programs are running,
except a kernel thread krfcommd.

Re-starting the program produces the error "Address already in us(98)" =
when
trying to bind the SPP server socket. The only remedy I've found is to
reboot the device.

Could sending an hci_disconnect directly, rather than using close(), be
confusing the stack? I realize this isn't the ideal way of closing
connections... It only gets used if the normal process is taking longer =
than
is desired when a device is forcing the server to be a slave.

The kernel version is 2.4.21-mh10, with various ARM patches (which, as =
far
as I can tell, do not touch any bluetooth related code). I have a very
similar program running on 2.4.18-mh8 and have never noticed this =
problem.

This doesn't seem to be related, but just in case it is: I'm also seeing
"hci_cmd_task: hci0 command tx timeout" errors. I am able to get the =
above
situation to happen without a timeout error.

Thanks,

-Daryl.



-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

Marcel,

I've been able to get the rfcomm use count to stick above 0 using a
single-threaded app which only does RFCOMM socket calls. After killing
hciattach and all bluetooth related programs, I have this:

lsmod:
Module                  Size  Used by    Not tainted
rfcomm                 23268   2  (autoclean)
hci_uart                5656   0  (autoclean)
l2cap                  17028   1  (autoclean) [rfcomm]
bluez                  32392   0  (autoclean) [rfcomm hci_uart l2cap]

cat /proc/bluetooth/rfcomm:
sk  3A:A4:58:16:05:00 26:02:5F:16:05:00 9 4
sk  3A:A4:58:16:05:00 26:02:5F:16:05:00 9 10

This shows, I think, two sockets in the BT_CLOSED state which aren't =
getting
cleaned up. Kernel is 2.4.21-mh10 running on an ARM (with ARM patches, =
which
as far as I can tell don't touch any bluetooth code).

The test program loops, doing the following, in pseudo code:

While(1) {
	1. Connect to remote device via rfcomm
	2. send a few 100 bytes of data
	3. Attempt to receive, but the remote side closes the connection.
	4. creates a socket, binds, listens, and accepts an incomming rfcomm
connection
	5. closes the listening socket (continues to use the socket return
by accept)
	6. sends a few 100 bytes and receives ~100kB.
	7. closes the socket
}

Unfortunately the problem is not easily reproduced. It can take all day =
for
it to happen, with the test program being re-run many times (and stopped
with ctrl-c).

There are two modules connected via serial. On the device two copies of =
this
program are run, each one specifying a local bluetooth device in the =
bind
calls for connect and listen/accept. i.e. each program works with its =
own
local device and does not do anything with the other device.

Any thoughts?

-Daryl.



-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] Rfcomm use count

[Daryl Van Vorst]

One more thing possibly of interest:

cat /proc/bluetooth/l2cap:
00:00:00:00:00:00 00:00:00:00:00:00 4 3 0x0000 0x0000 1024 0 0x0

-Daryl.

> -----Original Message-----
> From: bluez-devel-admin@lists.sourceforge.net=20
> [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of=20
> Daryl Van Vorst
> Sent: September 8, 2004 3:48 PM
> To: 'BlueZ Mailing List'
> Subject: RE: [Bluez-devel] Rfcomm use count
>=20
>=20
> Marcel,
>=20
> I've been able to get the rfcomm use count to stick above 0 using a
> single-threaded app which only does RFCOMM socket calls. After killing
> hciattach and all bluetooth related programs, I have this:
>=20
> lsmod:
> Module                  Size  Used by    Not tainted
> rfcomm                 23268   2  (autoclean)
> hci_uart                5656   0  (autoclean)
> l2cap                  17028   1  (autoclean) [rfcomm]
> bluez                  32392   0  (autoclean) [rfcomm hci_uart l2cap]
>=20
> cat /proc/bluetooth/rfcomm:
> sk  3A:A4:58:16:05:00 26:02:5F:16:05:00 9 4
> sk  3A:A4:58:16:05:00 26:02:5F:16:05:00 9 10
>=20
> This shows, I think, two sockets in the BT_CLOSED state which=20
> aren't getting
> cleaned up. Kernel is 2.4.21-mh10 running on an ARM (with ARM=20
> patches, which
> as far as I can tell don't touch any bluetooth code).
>=20
> The test program loops, doing the following, in pseudo code:
>=20
> While(1) {
> 	1. Connect to remote device via rfcomm
> 	2. send a few 100 bytes of data
> 	3. Attempt to receive, but the remote side closes the=20
> connection.
> 	4. creates a socket, binds, listens, and accepts an=20
> incomming rfcomm
> connection
> 	5. closes the listening socket (continues to use the=20
> socket return
> by accept)
> 	6. sends a few 100 bytes and receives ~100kB.
> 	7. closes the socket
> }
>=20
> Unfortunately the problem is not easily reproduced. It can=20
> take all day for
> it to happen, with the test program being re-run many times=20
> (and stopped
> with ctrl-c).
>=20
> There are two modules connected via serial. On the device two=20
> copies of this
> program are run, each one specifying a local bluetooth device=20
> in the bind
> calls for connect and listen/accept. i.e. each program works=20
> with its own
> local device and does not do anything with the other device.
>=20
> Any thoughts?
>=20
> -Daryl.
>=20
>=20
>=20
> -------------------------------------------------------
> This SF.Net email is sponsored by BEA Weblogic Workshop
> FREE Java Enterprise J2EE developer tools!
> Get your free copy of BEA WebLogic Workshop 8.1 today.
> http://ads.osdn.com/?ad_idP47&alloc_id=10808&op=3Dick
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>=20



-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Rfcomm use count

[Marcel Holtmann]

Hi Daryl,

> Any suggestions you have regarding the following would be greatly
> appreciated.
> 
> I have a server program which does the following:
> 
> 1. Intializes and attaches two modules via serial port (CSR 18.1)
> 2. Has SPP and OPP servers
> 3. Does some HCI-level stuff to monitor connections and close connections to
> misbehaving devices.
> 
> The program will initiate rfcomm connections to devices which do not allow
> role switches.
> 
> If I kill the program while several devices are connected (or are in the
> process of connecting/disconnecting), sometimes the rfcomm use count is left
> above 0 (I've seen it as high as 4). Killing the program detaches the
> modules (this is verified by hcid's log output). Killing sdpd and hcid
> doesn't reduce the use count. No other bluetooth programs are running,
> except a kernel thread krfcommd.
> 
> Re-starting the program produces the error "Address already in us(98)" when
> trying to bind the SPP server socket. The only remedy I've found is to
> reboot the device.
> 
> Could sending an hci_disconnect directly, rather than using close(), be
> confusing the stack? I realize this isn't the ideal way of closing
> connections... It only gets used if the normal process is taking longer than
> is desired when a device is forcing the server to be a slave.
> 
> The kernel version is 2.4.21-mh10, with various ARM patches (which, as far
> as I can tell, do not touch any bluetooth related code). I have a very
> similar program running on 2.4.18-mh8 and have never noticed this problem.
> 
> This doesn't seem to be related, but just in case it is: I'm also seeing
> "hci_cmd_task: hci0 command tx timeout" errors. I am able to get the above
> situation to happen without a timeout error.

I haven't had time to look at your problem in the last two weeks and
dealing with ARM related stuff still not fits into my left free time for
the next weeks. Is this behaviour reproduceable on x86 machines and do
you have a small text program to trigger this effect? And what I really
care about, is this problem also available with a 2.6 kernel?

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170
Project Admins to receive an Apple iPod Mini FREE for your judgement on
who ports your project to Linux PPC the best. Sponsored by IBM. 
Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel