Rfcomm qualification

Rfcomm qualification

[Daryl Van Vorst]

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

Marcel,

We ran into another 1.0B problem with rfcomm testing. We didn't notice this
last time because the tests were done slightly differently (without an
automated tester).

The tester acts as a 1.0B device and establishes a connection with the IUT.
The IUT sent credits (pf bit = 1) after the MSC echange. It shouldn't.
Attached is an hcidump of it.

We'll have to get this resolved before we can check the fcon/fcoff mods.

Could you take a look?

Thanks,

-Daryl.

[-- Attachment #2: rfc_bv_10_devB.dat --]
[-- Type: application/octet-stream, Size: 1024 bytes --]

[Bluez-devel] Re: Rfcomm qualification

[Marcel Holtmann]

Hi Daryl,

> We ran into another 1.0B problem with rfcomm testing. We didn't notice this
> last time because the tests were done slightly differently (without an
> automated tester).
> 
> The tester acts as a 1.0B device and establishes a connection with the IUT.
> The IUT sent credits (pf bit = 1) after the MSC echange. It shouldn't.
> Attached is an hcidump of it.

the problem is that the tester don't sends a PN CMD (btw is this really
allowed?) and so we use RFCOMM_MAX_CREDITS at two points (once for the
session and for every DLC). If the PN CMD is present and shows us a 1.0b
device we set credits to zero and don't make use of credit based flow
control. It seems that the default value of credits must be zero and not
RFCOMM_MAX_CREDITS.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: Rfcomm qualification

[Daryl Van Vorst]

Marcel,

Yes, it's ok that the tester doesn't send the PN command. In 1.0B, the PN
command is optional. In the 1.1 spec, part F:1, section 5.5.3 "DLC paramete=
r
negotiation (PN)", it says that it is optional in the TS 07.10 spec, but is
mandatory in BT spec 1.1 and later.

Will making the default value of credits zero break other things?

-Daryl.

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@rvs.uni-bielefeld.de]=20
> Sent: July 29, 2003 4:03 PM
> To: Daryl Van Vorst
> Cc: BlueZ Mailing List
> Subject: Re: Rfcomm qualification
>=20
>=20
> Hi Daryl,
>=20
> > We ran into another 1.0B problem with rfcomm testing. We=20
> didn't notice=20
> > this last time because the tests were done slightly differently=20
> > (without an automated tester).
> >=20
> > The tester acts as a 1.0B device and establishes a=20
> connection with the=20
> > IUT. The IUT sent credits (pf bit =3D 1) after the MSC echange. It=20
> > shouldn't. Attached is an hcidump of it.
>=20
> the problem is that the tester don't sends a PN CMD (btw is=20
> this really
> allowed?) and so we use RFCOMM_MAX_CREDITS at two points=20
> (once for the session and for every DLC). If the PN CMD is=20
> present and shows us a 1.0b device we set credits to zero and=20
> don't make use of credit based flow control. It seems that=20
> the default value of credits must be zero and not RFCOMM_MAX_CREDITS.
>=20
> Regards
>=20
> Marcel
>=20
>=20
>=20

[Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

Hi Daryl,

> Yes, it's ok that the tester doesn't send the PN command. In 1.0B, the PN
> command is optional. In the 1.1 spec, part F:1, section 5.5.3 "DLC parameter
> negotiation (PN)", it says that it is optional in the TS 07.10 spec, but is
> mandatory in BT spec 1.1 and later.

welcome to the bad stories of Bluetooth RFCOMM ;)

> Will making the default value of credits zero break other things?

This is the magic question. I think it will not break anything, but I am
not 100% sure. And I haven't the time to boot up a test machine.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

[Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

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

Hi Daryl,

> Will making the default value of credits zero break other things?

I got some time to test this and it brings our RFCOMM code back to the
behavior of 1.0b. The session wide credits value is useless and the dlc
credits value must be zero in the initial place. But we must make sure
that we request 1.1 credit based flow control for outgoing connections.
The attached patch should solve all problems.

Max, please review the patch to make sure I don't forget anything.

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 2002 bytes --]

diff -Nru a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
--- a/include/net/bluetooth/rfcomm.h	Wed Jul 30 23:33:57 2003
+++ b/include/net/bluetooth/rfcomm.h	Wed Jul 30 23:33:57 2003
@@ -166,9 +166,7 @@
 	atomic_t         refcnt;
 	int              initiator;
 
-	/* Default DLC parameters */
-	uint   mtu;
-	uint   credits;
+	uint             mtu;
 
 	struct list_head dlcs;
 };
diff -Nru a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
--- a/net/bluetooth/rfcomm/core.c	Wed Jul 30 23:33:57 2003
+++ b/net/bluetooth/rfcomm/core.c	Wed Jul 30 23:33:57 2003
@@ -202,7 +202,7 @@
 	d->mtu        = RFCOMM_DEFAULT_MTU;
 	d->v24_sig    = RFCOMM_V24_RTC | RFCOMM_V24_RTR | RFCOMM_V24_DV;
 
-	d->credits    = RFCOMM_MAX_CREDITS;
+	d->credits    = 0;
 	d->rx_credits = RFCOMM_DEFAULT_CREDITS;
 }
 
@@ -310,7 +310,6 @@
 	rfcomm_dlc_link(s, d);
 
 	d->mtu     = s->mtu;
-	d->credits = s->credits;
 
 	if (s->state == BT_CONNECTED)
 		rfcomm_send_pn(s, 1, d);
@@ -474,9 +473,8 @@
 	s->state = state;
 	s->sock  = sock;
 
-	s->mtu     = RFCOMM_DEFAULT_MTU;
-	s->credits = RFCOMM_MAX_CREDITS;
-	
+	s->mtu = RFCOMM_DEFAULT_MTU;
+
 	list_add(&s->list, &session_list);
 
 	/* Do not increment module usage count for listeting sessions.
@@ -746,7 +744,7 @@
 	pn->ack_timer   = 0;
 	pn->max_retrans = 0;
 
-	if (d->credits) {
+	if (cr || d->credits) {
 		pn->flow_ctrl = cr ? 0xf0 : 0xe0;
 		pn->credits = RFCOMM_DEFAULT_CREDITS;
 	} else {
@@ -1140,18 +1138,16 @@
 
 	if (cr) {
 		if (pn->flow_ctrl == 0xf0) {
+			d->credits = RFCOMM_MAX_CREDITS;
 			d->tx_credits = pn->credits;
-		} else {
+		} else
 			set_bit(RFCOMM_TX_THROTTLED, &d->flags);
-			d->credits = 0;
-		}
 	} else {
 		if (pn->flow_ctrl == 0xe0) {
+			d->credits = RFCOMM_MAX_CREDITS;
 			d->tx_credits = pn->credits;
-		} else {
+		} else
 			set_bit(RFCOMM_TX_THROTTLED, &d->flags);
-			d->credits = 0;
-		}
 	}
 
 	d->priority = pn->priority;

Re: [Bluez-devel] RE: Rfcomm qualification

[Max Krasnyansky]

At 02:43 PM 7/30/2003, Marcel Holtmann wrote:
>Hi Daryl,
>
>> Will making the default value of credits zero break other things?
>
>I got some time to test this and it brings our RFCOMM code back to the
>behavior of 1.0b.

>The session wide credits value is useless 
No, it's not. 1.1 spec only mandates send PN for the _first_ DLC
on that session. Which means that values negotiated in PN request are supposed 
to be applied to the subsequent DLCs.  So we still need session wide settings.
We just need to update them when we get PN req/rsp, which don't do.

Check this out:
"
6.5.1 Initial DLC Negotiation
The use of credit based flow control is a session characteristic. Thus, it has to
be negotiated with the PN multiplexor control command (see Section 5.5.3)
before the first DLC is established.
After the first successful negotiation and DLC establishment, all DLCs will be
flow controlled with this scheme. PN negotiation at subsequent DLC establish-ments
is optional, but recommended, since it also establishes initial credit
count values on both sides for both sides.
"

>and the dlc credits value must be zero in the initial place. 
Yep. But it doesn't really matter because we'll simply use session 
value as default.

>But we must make sure that we request 1.1 credit based 
>flow control for outgoing connections.
That's why session has to have non zero value by default.

>The attached patch should solve all problems.
>Max, please review the patch to make sure I don't forget anything.
It's incorrect.

Max

Re: [Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

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

Hi Max,

> >The session wide credits value is useless 
> No, it's not. 1.1 spec only mandates send PN for the _first_ DLC
> on that session. Which means that values negotiated in PN request are supposed 
> to be applied to the subsequent DLCs.  So we still need session wide settings.
> We just need to update them when we get PN req/rsp, which don't do.
> 
> Check this out:
> "
> 6.5.1 Initial DLC Negotiation
> The use of credit based flow control is a session characteristic. Thus, it has to
> be negotiated with the PN multiplexor control command (see Section 5.5.3)
> before the first DLC is established.
> After the first successful negotiation and DLC establishment, all DLCs will be
> flow controlled with this scheme. PN negotiation at subsequent DLC establish-ments
> is optional, but recommended, since it also establishes initial credit
> count values on both sides for both sides.
> "

memory malfunction ;) I thought it was a per dlc value.

The attached patch sets the default values for session and dlc credits
to zero and put in RFCOMM_MAX_CREDITS only if we receive a PN with 1.1
flow control.

What do you think?

Regards

Marcel


[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 1558 bytes --]

diff -Nru a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
--- a/net/bluetooth/rfcomm/core.c	Thu Jul 31 02:41:29 2003
+++ b/net/bluetooth/rfcomm/core.c	Thu Jul 31 02:41:29 2003
@@ -202,7 +202,7 @@
 	d->mtu        = RFCOMM_DEFAULT_MTU;
 	d->v24_sig    = RFCOMM_V24_RTC | RFCOMM_V24_RTR | RFCOMM_V24_DV;
 
-	d->credits    = RFCOMM_MAX_CREDITS;
+	d->credits    = 0;
 	d->rx_credits = RFCOMM_DEFAULT_CREDITS;
 }
 
@@ -475,7 +475,7 @@
 	s->sock  = sock;
 
 	s->mtu     = RFCOMM_DEFAULT_MTU;
-	s->credits = RFCOMM_MAX_CREDITS;
+	s->credits = 0;
 	
 	list_add(&s->list, &session_list);
 
@@ -746,7 +746,7 @@
 	pn->ack_timer   = 0;
 	pn->max_retrans = 0;
 
-	if (d->credits) {
+	if (cr || d->credits) {
 		pn->flow_ctrl = cr ? 0xf0 : 0xe0;
 		pn->credits = RFCOMM_DEFAULT_CREDITS;
 	} else {
@@ -1135,11 +1135,15 @@
 
 static int rfcomm_apply_pn(struct rfcomm_dlc *d, int cr, struct rfcomm_pn *pn)
 {
+	struct rfcomm_session *s = d->session;
+
 	BT_DBG("dlc %p state %ld dlci %d mtu %d fc 0x%x credits %d", 
 			d, d->state, d->dlci, pn->mtu, pn->flow_ctrl, pn->credits);
 
 	if (cr) {
 		if (pn->flow_ctrl == 0xf0) {
+			s->credits = RFCOMM_MAX_CREDITS;
+			d->credits = s->credits;
 			d->tx_credits = pn->credits;
 		} else {
 			set_bit(RFCOMM_TX_THROTTLED, &d->flags);
@@ -1147,6 +1151,8 @@
 		}
 	} else {
 		if (pn->flow_ctrl == 0xe0) {
+			s->credits = RFCOMM_MAX_CREDITS;
+			d->credits = s->credits;
 			d->tx_credits = pn->credits;
 		} else {
 			set_bit(RFCOMM_TX_THROTTLED, &d->flags);

Re: [Bluez-devel] RE: Rfcomm qualification

[Max Krasnyansky]

At 05:48 PM 7/30/2003, Marcel Holtmann wrote:
>Hi Max,
>
>> >The session wide credits value is useless 
>> No, it's not. 1.1 spec only mandates send PN for the _first_ DLC
>> on that session. Which means that values negotiated in PN request are supposed 
>> to be applied to the subsequent DLCs.  So we still need session wide settings.
>> We just need to update them when we get PN req/rsp, which don't do.
>> 
>> Check this out:
>> "
>> 6.5.1 Initial DLC Negotiation
>> The use of credit based flow control is a session characteristic. Thus, it has to
>> be negotiated with the PN multiplexor control command (see Section 5.5.3)
>> before the first DLC is established.
>> After the first successful negotiation and DLC establishment, all DLCs will be
>> flow controlled with this scheme. PN negotiation at subsequent DLC establish-ments
>> is optional, but recommended, since it also establishes initial credit
>> count values on both sides for both sides.
>> "
>
>memory malfunction ;) I thought it was a per dlc value.
>
>The attached patch sets the default values for session and dlc credits
>to zero and put in RFCOMM_MAX_CREDITS only if we receive a PN with 1.1
>flow control.
>
>What do you think?

I don't like this:

@@ -746,7 +746,7 @@
        pn->ack_timer   = 0;
        pn->max_retrans = 0;
 
-       if (d->credits) {
+       if (cr || d->credits) {


This basically means that we'll always request CFC even if the other side
explicitly told us that they don't support it. Which is kinda dumb :).

Here is what I would do (no time for the patch sory).
When session is created set s->credits to -1. Replace above if() statement 
with
        if (s->credits < 0) {

Also rfcomm_apply_pn() needs to set d->credits only once at the end of the function.

Max

Re: [Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

Hi Max,

> >The attached patch sets the default values for session and dlc credits
> >to zero and put in RFCOMM_MAX_CREDITS only if we receive a PN with 1.1
> >flow control.
> >
> >What do you think?
> 
> I don't like this:
> 
> @@ -746,7 +746,7 @@
>         pn->ack_timer   = 0;
>         pn->max_retrans = 0;
>  
> -       if (d->credits) {
> +       if (cr || d->credits) {
> 
> 
> This basically means that we'll always request CFC even if the other side
> explicitly told us that they don't support it. Which is kinda dumb :).

I know exactly what you mean and I don't like it either. But my general
purpose of this patch was not to introduce another "state" of credits.
And btw. credits is a uint at the moment.

What do you thing about using session->flags and once we received a
positive CFC acknowledgment we set a session wide flag?

> Here is what I would do (no time for the patch sory).
> When session is created set s->credits to -1. Replace above if() statement 
> with
>         if (s->credits < 0) {

I think only

	if (s->credits != 0) {

will work in this case ;)

> Also rfcomm_apply_pn() needs to set d->credits only once at the end of the function.

This depends on how strict we want to go with the RFCOMM spec. It says
that if one dlc requests CFC all other dlc's should go with CFC, too. I
like to give the dlc the chance to deactivate or activate CFC even if
session default CFC setting says otherwise. But maybe this feature is
non-sense - I have to think about it in more detail.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] RE: Rfcomm qualification

[Daryl Van Vorst]

Marcel,

Have you two worked out a solution for this one (see message below)?

Should I use the last patch you sent to test?

-Daryl.

> -----Original Message-----
> From: Marcel Holtmann [mailto:marcel@rvs.uni-bielefeld.de] 
> Sent: August 5, 2003 3:04 PM
> To: Max Krasnyansky
> Cc: Daryl Van Vorst; BlueZ Mailing List
> Subject: Re: [Bluez-devel] RE: Rfcomm qualification
> 
> 
> Hi Max,
> 
> > >The attached patch sets the default values for session and dlc 
> > >credits to zero and put in RFCOMM_MAX_CREDITS only if we 
> receive a PN 
> > >with 1.1 flow control.
> > >
> > >What do you think?
> > 
> > I don't like this:
> > 
> > @@ -746,7 +746,7 @@
> >         pn->ack_timer   = 0;
> >         pn->max_retrans = 0;
> >  
> > -       if (d->credits) {
> > +       if (cr || d->credits) {
> > 
> > 
> > This basically means that we'll always request CFC even if 
> the other 
> > side explicitly told us that they don't support it. Which is kinda 
> > dumb :).
> 
> I know exactly what you mean and I don't like it either. But 
> my general purpose of this patch was not to introduce another 
> "state" of credits. And btw. credits is a uint at the moment.
> 
> What do you thing about using session->flags and once we 
> received a positive CFC acknowledgment we set a session wide flag?
> 
> > Here is what I would do (no time for the patch sory).
> > When session is created set s->credits to -1. Replace above if() 
> > statement
> > with
> >         if (s->credits < 0) {
> 
> I think only
> 
> 	if (s->credits != 0) {
> 
> will work in this case ;)
> 
> > Also rfcomm_apply_pn() needs to set d->credits only once at 
> the end of 
> > the function.
> 
> This depends on how strict we want to go with the RFCOMM 
> spec. It says that if one dlc requests CFC all other dlc's 
> should go with CFC, too. I like to give the dlc the chance to 
> deactivate or activate CFC even if session default CFC 
> setting says otherwise. But maybe this feature is non-sense - 
> I have to think about it in more detail.
> 
> Regards
> 
> Marcel
> 
> 
> 

RE: [Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

Hi Daryl,

> Have you two worked out a solution for this one (see message below)?

I am working on another way to solve this problem. The patch in
2.4.18-mh8 should work, but it is not the best solution.

> Should I use the last patch you sent to test?

Give it a try, because it should pass the test.

Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

RE: [Bluez-devel] RE: Rfcomm qualification

[Daryl Van Vorst]

Marcel,

Is this in your 2.4.18-mh8 patch?

-Daryl.

> -----Original Message-----
> From: bluez-devel-admin@lists.sourceforge.net 
> [mailto:bluez-devel-admin@lists.sourceforge.net] On Behalf Of 
> Marcel Holtmann
> Sent: July 30, 2003 5:49 PM
> To: Max Krasnyansky
> Cc: Daryl Van Vorst; BlueZ Mailing List
> Subject: Re: [Bluez-devel] RE: Rfcomm qualification
> 
> 
> Hi Max,
> 
> > >The session wide credits value is useless
> > No, it's not. 1.1 spec only mandates send PN for the _first_ DLC on 
> > that session. Which means that values negotiated in PN request are 
> > supposed to be applied to the subsequent DLCs.  So we still need 
> > session wide settings. We just need to update them when we get PN 
> > req/rsp, which don't do.
> > 
> > Check this out:
> > "
> > 6.5.1 Initial DLC Negotiation
> > The use of credit based flow control is a session characteristic. 
> > Thus, it has to be negotiated with the PN multiplexor 
> control command 
> > (see Section 5.5.3) before the first DLC is established. After the 
> > first successful negotiation and DLC establishment, all 
> DLCs will be 
> > flow controlled with this scheme. PN negotiation at subsequent DLC 
> > establish-ments is optional, but recommended, since it also 
> > establishes initial credit count values on both sides for 
> both sides. 
> > "
> 
> memory malfunction ;) I thought it was a per dlc value.
> 
> The attached patch sets the default values for session and 
> dlc credits to zero and put in RFCOMM_MAX_CREDITS only if we 
> receive a PN with 1.1 flow control.
> 
> What do you think?
> 
> Regards
> 
> Marcel
> 
> 

RE: [Bluez-devel] RE: Rfcomm qualification

[Marcel Holtmann]

Hi Daryl,

> Is this in your 2.4.18-mh8 patch?

both changes are in

	- Set initial value of RFCOMM credits to zero
	- Support for FCon and FCoff flow control commands


Regards

Marcel




-------------------------------------------------------
This SF.Net email sponsored by: Free pre-built ASP.NET sites including
Data Reports, E-commerce, Portals, and Forums are available now.
Download today and enter to win an XBOX or Visual Studio .NET.
http://aspnet.click-url.com/go/psa00100003ave/direct;at.aspnet_072303_01/01
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel