GSoC 2009: L2CAP Enhanced Retransmission mode

GSoC 2009: L2CAP Enhanced Retransmission mode

[Gustavo F. Padovan]

Hi all,

I'm a accepted student for GSoC 2009 to work with BlueZ. I'm a
Computer Engineer student at University of Campinas (Brazil).

In my project I'm going to implement the L2CAP Enhanced Retransmission
mode. That mode will permit retransmission of corrupted or lost
packets improving bluetooth experience on Linux. My mentors are Luiz
Augusto “Vudentz” von Dentz and Marcel Holtmann.

That's my hack for the next four months, and if you wanna see what I'm
doing look at my git repo[1] and/or read my blog[2]. Also I'll send
reports of my work to this list approximately twice a month.

Finally, I wanna thanks all the BlueZ devs for accept me on GSoC, I'm
very glad to work with BlueZ. And I wanna congratulate the other
students accepted on BlueZ. =)

Regards.


[1] http://www.las.ic.unicamp.br/~gustavo/git/
[2] http://padovan.org/


-- 
Gustavo F. Padovan

Computer Engineering Student
Institute of Computing - IC
University of Campinas - Unicamp

email: gustavo@las.ic.unicamp.br
irc: padovan at freenode.net
mobile: +55 19 81030803

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Nathan Holstein]

On Sat, 2009-04-25 at 20:05 -0300, Gustavo F. Padovan wrote:
> Hi all,
> 
> I'm a accepted student for GSoC 2009 to work with BlueZ. I'm a
> Computer Engineer student at University of Campinas (Brazil).
> 
> In my project I'm going to implement the L2CAP Enhanced Retransmission
> mode. That mode will permit retransmission of corrupted or lost
> packets improving bluetooth experience on Linux. My mentors are Luiz
> Augusto “Vudentz” von Dentz and Marcel Holtmann.
> 
> That's my hack for the next four months, and if you wanna see what I'm
> doing look at my git repo[1] and/or read my blog[2]. Also I'll send
> reports of my work to this list approximately twice a month.
> 
> Finally, I wanna thanks all the BlueZ devs for accept me on GSoC, I'm
> very glad to work with BlueZ. And I wanna congratulate the other
> students accepted on BlueZ. =)
> 
> Regards.
> 
> 
> [1] http://www.las.ic.unicamp.br/~gustavo/git/
> [2] http://padovan.org/


	As part of my employers work in medical devices, I've been working on
implementing enhanced L2CAP mode in the kernel.  Since learning of
Gustavo's acceptance into Google's SOC (congratulations!), I've been
cleared to open up the work which I have thus far.
	Because our initial target is interoperability against a specific set
of medical devices from outside vendors, my work thus far has been
targeted against these devices.  Enhanced L2CAP is fairly complex, and
has multiple parts to it.  To get the first round of compatibility
going, I've taken such paths as limiting the local TX window to 1
outstanding SDU.
	Currently, the code is at a state where it successfully negotiates
between enhanced/streaming/basic modes, opens a connection, and begins
sending data.  Much of the work I've done thus far has been splitting
out the send/receive paths for the separate modes.  There have been no
significant changes in and code path, and no need to make any major
changes to the existing data structures.  Major points left to implement
include:

  *  Support for a TX windows > 1
     On both send and receive, the code limits itself to only one
outstanding l2cap SDU at a time.  I have added a send and receive queue
of skbuffs to a socket to support this, however this is unimplemented,
and quite possibly duplicates code elsewhere in the kernel.

  *  Add logic for the retransmission and monitor timers.
     Again, there are stubs in place for these timers, however the code
is unimplemented.  My next step will be basic retransmission support for
lost packets.

  *  Optimize the FCS computation.

  *  Improve the L2CAP state machine
     Currently, the state machine described in section 8.6.5 of the
L2CAP specification is mostly unimplemented.

	Currently, the code is branched off bluetooth-testing from last week.
The tree is available via:

     git clone git://staticfree.info/git/el2cap

I'd like to thank my friend Steve from the MIT Mobile Experience Lab for
graciously hosting the code.  My latest work is in the "retransmission"
branch.  Due to the increase in code size, I've split the files up into
an l2cap sub-directory under net/bluetooth.  I've not yet cloned
Gustavo's repository, but I should be able to rebase the code off that
as necessary.
	If there are any comments on style or implementation, I'd be happy to
address them in the code.  Hopefully, this can be used as base for
Gustavo's SOC work so we can focus upon complete implementation of the
Enhanced L2CAP specification and interoperability with other devices.



   --Nathan Holstein
     Lamprey Networks Inc.
     www.lampreynetworks.com

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Marcel Holtmann]

Hi Nathan,

> > I'm a accepted student for GSoC 2009 to work with BlueZ. I'm a
> > Computer Engineer student at University of Campinas (Brazil).
> > 
> > In my project I'm going to implement the L2CAP Enhanced Retransmission
> > mode. That mode will permit retransmission of corrupted or lost
> > packets improving bluetooth experience on Linux. My mentors are Luiz
> > Augusto “Vudentz” von Dentz and Marcel Holtmann.
> > 
> > That's my hack for the next four months, and if you wanna see what I'm
> > doing look at my git repo[1] and/or read my blog[2]. Also I'll send
> > reports of my work to this list approximately twice a month.
> > 
> > Finally, I wanna thanks all the BlueZ devs for accept me on GSoC, I'm
> > very glad to work with BlueZ. And I wanna congratulate the other
> > students accepted on BlueZ. =)
> > 
> 
> 	As part of my employers work in medical devices, I've been working on
> implementing enhanced L2CAP mode in the kernel.  Since learning of
> Gustavo's acceptance into Google's SOC (congratulations!), I've been
> cleared to open up the work which I have thus far.

that is good news. Thanks for helping out here.

> 	Because our initial target is interoperability against a specific set
> of medical devices from outside vendors, my work thus far has been
> targeted against these devices.  Enhanced L2CAP is fairly complex, and
> has multiple parts to it.  To get the first round of compatibility
> going, I've taken such paths as limiting the local TX window to 1
> outstanding SDU.
> 	Currently, the code is at a state where it successfully negotiates
> between enhanced/streaming/basic modes, opens a connection, and begins
> sending data.  Much of the work I've done thus far has been splitting
> out the send/receive paths for the separate modes.  There have been no
> significant changes in and code path, and no need to make any major
> changes to the existing data structures.  Major points left to implement
> include:
> 
>   *  Support for a TX windows > 1
>      On both send and receive, the code limits itself to only one
> outstanding l2cap SDU at a time.  I have added a send and receive queue
> of skbuffs to a socket to support this, however this is unimplemented,
> and quite possibly duplicates code elsewhere in the kernel.

in the end the ERTM mode should behave like SOCK_STREAM (like what
RFCOMM does at the moment). The Basic mode will stay as SOCK_SEQPACKET
and this way we require one mode or the other.

For testing purpose of course just setting the value in the existing
SOCK_SEQPACKET should be enough. And also that SOCK_SEQPACKET could run
with ETRM without any downside is an option anyway.

>   *  Add logic for the retransmission and monitor timers.
>      Again, there are stubs in place for these timers, however the code
> is unimplemented.  My next step will be basic retransmission support for
> lost packets.

Trying to get lost packets over Bluetooth is kinda hard. So you need
some faulty code for testing this. In theory the HCI layer is reliable.

>   *  Optimize the FCS computation.

The Linux kernel already has the proper FCS implementation that you can
use. It should be CONFIG_CRC_CCITT if I remember correctly.

This needs fixing first since it just plain useless to not use the
kernel supported FCS functions.

>   *  Improve the L2CAP state machine
>      Currently, the state machine described in section 8.6.5 of the
> L2CAP specification is mostly unimplemented.

This is still done on purpose since it doesn't map nicely to the socket
API, but we could improve things here.

> 	Currently, the code is branched off bluetooth-testing from last week.
> The tree is available via:
> 
>      git clone git://staticfree.info/git/el2cap
> 
> I'd like to thank my friend Steve from the MIT Mobile Experience Lab for
> graciously hosting the code.  My latest work is in the "retransmission"
> branch.  Due to the increase in code size, I've split the files up into
> an l2cap sub-directory under net/bluetooth.  I've not yet cloned
> Gustavo's repository, but I should be able to rebase the code off that
> as necessary.
> 	If there are any comments on style or implementation, I'd be happy to
> address them in the code.  Hopefully, this can be used as base for
> Gustavo's SOC work so we can focus upon complete implementation of the
> Enhanced L2CAP specification and interoperability with other devices.

The split into l2cap.c and queue.c and move to a separate directory
looks totally useless to me. I know that l2cap.c is big, but there is no
real separation. You would have to convince why this makes sense.

So we can start adding the initial constant changes quickly to
bluetooth-testing.git since they are easy to review. However they need
cleanup in a few areas since (for example changing the version doesn't
belong here etc.).

Final changes to the feature mask can only be done once the feature
actually works, but we can constify it already. And your unknown feature
is the fixed channel support (from Bluetooth 3.0 specification).

The main development and upstream acceptance will happen in
bluetooth-testing.git repository. Patches in that repository with a
proper detailed commit message can be considered upstream. However I do
insist on them not breaking current behavior and that they are
bisect-able.

Regards

Marcel

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Nathan Holstein]

On Tue, 2009-04-28 at 08:40 -0700, Marcel Holtmann wrote:
> Hi Nathan,
> 
> > > I'm a accepted student for GSoC 2009 to work with BlueZ. I'm a
> > > Computer Engineer student at University of Campinas (Brazil).
> > > 
> > > In my project I'm going to implement the L2CAP Enhanced Retransmission
> > > mode. That mode will permit retransmission of corrupted or lost
> > > packets improving bluetooth experience on Linux. My mentors are Luiz
> > > Augusto “Vudentz” von Dentz and Marcel Holtmann.
> > > 
> > > That's my hack for the next four months, and if you wanna see what I'm
> > > doing look at my git repo[1] and/or read my blog[2]. Also I'll send
> > > reports of my work to this list approximately twice a month.
> > > 
> > > Finally, I wanna thanks all the BlueZ devs for accept me on GSoC, I'm
> > > very glad to work with BlueZ. And I wanna congratulate the other
> > > students accepted on BlueZ. =)
> > > 
> > 
> > 	As part of my employers work in medical devices, I've been working on
> > implementing enhanced L2CAP mode in the kernel.  Since learning of
> > Gustavo's acceptance into Google's SOC (congratulations!), I've been
> > cleared to open up the work which I have thus far.
> 
> that is good news. Thanks for helping out here.
> 
> > 	Because our initial target is interoperability against a specific set
> > of medical devices from outside vendors, my work thus far has been
> > targeted against these devices.  Enhanced L2CAP is fairly complex, and
> > has multiple parts to it.  To get the first round of compatibility
> > going, I've taken such paths as limiting the local TX window to 1
> > outstanding SDU.
> > 	Currently, the code is at a state where it successfully negotiates
> > between enhanced/streaming/basic modes, opens a connection, and begins
> > sending data.  Much of the work I've done thus far has been splitting
> > out the send/receive paths for the separate modes.  There have been no
> > significant changes in and code path, and no need to make any major
> > changes to the existing data structures.  Major points left to implement
> > include:
> > 
> >   *  Support for a TX windows > 1
> >      On both send and receive, the code limits itself to only one
> > outstanding l2cap SDU at a time.  I have added a send and receive queue
> > of skbuffs to a socket to support this, however this is unimplemented,
> > and quite possibly duplicates code elsewhere in the kernel.
> 
> in the end the ERTM mode should behave like SOCK_STREAM (like what
> RFCOMM does at the moment). The Basic mode will stay as SOCK_SEQPACKET
> and this way we require one mode or the other.
> 
> For testing purpose of course just setting the value in the existing
> SOCK_SEQPACKET should be enough. And also that SOCK_SEQPACKET could run
> with ETRM without any downside is an option anyway.

Some of the devices I test against force Basic mode for SDP and ERTM for
HDP.  In my testing while using the kernel negotiation procedure, both
sdptool and our HDP implementation work transparently, using Basic and
ERTM modes respectively.

My vision was to add an ioctl() to allow an application to specify that
it requires a specific mode.  This was due to my understanding of the
recvmsg() man page:

       The  msg_flags  field  in the msghdr is set on return of
       recvmsg().  It can contain several flags:

       MSG_EOR
              indicates end-of-record; the data returned completed a
              record (generally used with sockets of type
              SOCK_SEQPACKET).

The MCAP/HDP profiles require this metadata to function properly.  Can
we tack MSG_EOR onto a SOCK_STREAM socket?  If so, this would be simpler
than requiring an ioctl.

> 
> >   *  Add logic for the retransmission and monitor timers.
> >      Again, there are stubs in place for these timers, however the code
> > is unimplemented.  My next step will be basic retransmission support for
> > lost packets.
> 
> Trying to get lost packets over Bluetooth is kinda hard. So you need
> some faulty code for testing this. In theory the HCI layer is reliable.
> 
> >   *  Optimize the FCS computation.
> 
> The Linux kernel already has the proper FCS implementation that you can
> use. It should be CONFIG_CRC_CCITT if I remember correctly.
> 
> This needs fixing first since it just plain useless to not use the
> kernel supported FCS functions.

Agreed.

> 
> >   *  Improve the L2CAP state machine
> >      Currently, the state machine described in section 8.6.5 of the
> > L2CAP specification is mostly unimplemented.
> 
> This is still done on purpose since it doesn't map nicely to the socket
> API, but we could improve things here.
> 
> > 	Currently, the code is branched off bluetooth-testing from last week.
> > The tree is available via:
> > 
> >      git clone git://staticfree.info/git/el2cap
> > 
> > I'd like to thank my friend Steve from the MIT Mobile Experience Lab for
> > graciously hosting the code.  My latest work is in the "retransmission"
> > branch.  Due to the increase in code size, I've split the files up into
> > an l2cap sub-directory under net/bluetooth.  I've not yet cloned
> > Gustavo's repository, but I should be able to rebase the code off that
> > as necessary.
> > 	If there are any comments on style or implementation, I'd be happy to
> > address them in the code.  Hopefully, this can be used as base for
> > Gustavo's SOC work so we can focus upon complete implementation of the
> > Enhanced L2CAP specification and interoperability with other devices.
> 
> The split into l2cap.c and queue.c and move to a separate directory
> looks totally useless to me. I know that l2cap.c is big, but there is no
> real separation. You would have to convince why this makes sense.

Just trying to keep things manageable.  Currently, I'm not using any of
the code in queue.c anyway, so I'm tempted to just drop it.

> So we can start adding the initial constant changes quickly to
> bluetooth-testing.git since they are easy to review. However they need
> cleanup in a few areas since (for example changing the version doesn't
> belong here etc.).
> 
> Final changes to the feature mask can only be done once the feature
> actually works, but we can constify it already. And your unknown feature
> is the fixed channel support (from Bluetooth 3.0 specification).
> 
> The main development and upstream acceptance will happen in
> bluetooth-testing.git repository. Patches in that repository with a
> proper detailed commit message can be considered upstream. However I do
> insist on them not breaking current behavior and that they are
> bisect-able.


   --Nathan

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Marcel Holtmann]

Hi Nathan,

> > > 	Because our initial target is interoperability against a specific set
> > > of medical devices from outside vendors, my work thus far has been
> > > targeted against these devices.  Enhanced L2CAP is fairly complex, and
> > > has multiple parts to it.  To get the first round of compatibility
> > > going, I've taken such paths as limiting the local TX window to 1
> > > outstanding SDU.
> > > 	Currently, the code is at a state where it successfully negotiates
> > > between enhanced/streaming/basic modes, opens a connection, and begins
> > > sending data.  Much of the work I've done thus far has been splitting
> > > out the send/receive paths for the separate modes.  There have been no
> > > significant changes in and code path, and no need to make any major
> > > changes to the existing data structures.  Major points left to implement
> > > include:
> > > 
> > >   *  Support for a TX windows > 1
> > >      On both send and receive, the code limits itself to only one
> > > outstanding l2cap SDU at a time.  I have added a send and receive queue
> > > of skbuffs to a socket to support this, however this is unimplemented,
> > > and quite possibly duplicates code elsewhere in the kernel.
> > 
> > in the end the ERTM mode should behave like SOCK_STREAM (like what
> > RFCOMM does at the moment). The Basic mode will stay as SOCK_SEQPACKET
> > and this way we require one mode or the other.
> > 
> > For testing purpose of course just setting the value in the existing
> > SOCK_SEQPACKET should be enough. And also that SOCK_SEQPACKET could run
> > with ETRM without any downside is an option anyway.
> 
> Some of the devices I test against force Basic mode for SDP and ERTM for
> HDP.  In my testing while using the kernel negotiation procedure, both
> sdptool and our HDP implementation work transparently, using Basic and
> ERTM modes respectively.

that is fine. So our SDP client/server uses SOCK_SEQPACKET and then for
HDP you just use SOCK_STREAM. Sounds totally sane to me.

The default mode for SOCK_SEQPACKET should be basic mode and for
SOCK_STREAM is is ERTM. Additionally you can use setsockopt() with
L2CAP_OPTIONS to change the mode requirement.

> My vision was to add an ioctl() to allow an application to specify that
> it requires a specific mode.  This was due to my understanding of the
> recvmsg() man page:
> 
>        The  msg_flags  field  in the msghdr is set on return of
>        recvmsg().  It can contain several flags:
> 
>        MSG_EOR
>               indicates end-of-record; the data returned completed a
>               record (generally used with sockets of type
>               SOCK_SEQPACKET).
> 
> The MCAP/HDP profiles require this metadata to function properly.  Can
> we tack MSG_EOR onto a SOCK_STREAM socket?  If so, this would be simpler
> than requiring an ioctl.

As I said, the mode setting part is already there. And using an ioctl()
for these is wrong. You use setsockopt() for these kind of things.

And I think you can add an MSG_EOR on a stream socket. Not sure that it
really is needed. Sounds to me like a messed up detail in the MCAP
specification that would work anyway without extra flags. Have to read
the spec. at some point.

> > > 	Currently, the code is branched off bluetooth-testing from last week.
> > > The tree is available via:
> > > 
> > >      git clone git://staticfree.info/git/el2cap
> > > 
> > > I'd like to thank my friend Steve from the MIT Mobile Experience Lab for
> > > graciously hosting the code.  My latest work is in the "retransmission"
> > > branch.  Due to the increase in code size, I've split the files up into
> > > an l2cap sub-directory under net/bluetooth.  I've not yet cloned
> > > Gustavo's repository, but I should be able to rebase the code off that
> > > as necessary.
> > > 	If there are any comments on style or implementation, I'd be happy to
> > > address them in the code.  Hopefully, this can be used as base for
> > > Gustavo's SOC work so we can focus upon complete implementation of the
> > > Enhanced L2CAP specification and interoperability with other devices.
> > 
> > The split into l2cap.c and queue.c and move to a separate directory
> > looks totally useless to me. I know that l2cap.c is big, but there is no
> > real separation. You would have to convince why this makes sense.
> 
> Just trying to keep things manageable.  Currently, I'm not using any of
> the code in queue.c anyway, so I'm tempted to just drop it.

Just drop it for now. We can change the layout later if it is really
needed. Currently it is just confusing.

Regards

Marcel

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Gustavo F. Padovan]

Hi Nathan,

Your work will be very useful on My GSoC. We can work together on
implementing ERTM. I was just starting to study how to do mode
negotiaton. I'll stop it and go look through your code to understand
it and start coding something.

I think we need a TO DO list of ERTM stuff with the things that left
to implement and the this Marcel suggested. First I think that we need
do changes suggested by Marcel to push the to upstream. After we can
concentrate on the issues left to implement. I'm planning to start
this tonight.

First I need to study your code, then we can discuss it more. I need
to learn many things about l2cap and ERTM yet =3D).

Are you around #bluez on freenode?

Regards.

On Tue, Apr 28, 2009 at 1:49 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Nathan,
>
>> > > =A0 Because our initial target is interoperability against a specifi=
c set
>> > > of medical devices from outside vendors, my work thus far has been
>> > > targeted against these devices. =A0Enhanced L2CAP is fairly complex,=
 and
>> > > has multiple parts to it. =A0To get the first round of compatibility
>> > > going, I've taken such paths as limiting the local TX window to 1
>> > > outstanding SDU.
>> > > =A0 Currently, the code is at a state where it successfully negotiat=
es
>> > > between enhanced/streaming/basic modes, opens a connection, and begi=
ns
>> > > sending data. =A0Much of the work I've done thus far has been splitt=
ing
>> > > out the send/receive paths for the separate modes. =A0There have bee=
n no
>> > > significant changes in and code path, and no need to make any major
>> > > changes to the existing data structures. =A0Major points left to imp=
lement
>> > > include:
>> > >
>> > > =A0 * =A0Support for a TX windows > 1
>> > > =A0 =A0 =A0On both send and receive, the code limits itself to only =
one
>> > > outstanding l2cap SDU at a time. =A0I have added a send and receive =
queue
>> > > of skbuffs to a socket to support this, however this is unimplemente=
d,
>> > > and quite possibly duplicates code elsewhere in the kernel.
>> >
>> > in the end the ERTM mode should behave like SOCK_STREAM (like what
>> > RFCOMM does at the moment). The Basic mode will stay as SOCK_SEQPACKET
>> > and this way we require one mode or the other.
>> >
>> > For testing purpose of course just setting the value in the existing
>> > SOCK_SEQPACKET should be enough. And also that SOCK_SEQPACKET could ru=
n
>> > with ETRM without any downside is an option anyway.
>>
>> Some of the devices I test against force Basic mode for SDP and ERTM for
>> HDP. =A0In my testing while using the kernel negotiation procedure, both
>> sdptool and our HDP implementation work transparently, using Basic and
>> ERTM modes respectively.
>
> that is fine. So our SDP client/server uses SOCK_SEQPACKET and then for
> HDP you just use SOCK_STREAM. Sounds totally sane to me.
>
> The default mode for SOCK_SEQPACKET should be basic mode and for
> SOCK_STREAM is is ERTM. Additionally you can use setsockopt() with
> L2CAP_OPTIONS to change the mode requirement.
>
>> My vision was to add an ioctl() to allow an application to specify that
>> it requires a specific mode. =A0This was due to my understanding of the
>> recvmsg() man page:
>>
>> =A0 =A0 =A0 =A0The =A0msg_flags =A0field =A0in the msghdr is set on retu=
rn of
>> =A0 =A0 =A0 =A0recvmsg(). =A0It can contain several flags:
>>
>> =A0 =A0 =A0 =A0MSG_EOR
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 indicates end-of-record; the data returned c=
ompleted a
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 record (generally used with sockets of type
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 SOCK_SEQPACKET).
>>
>> The MCAP/HDP profiles require this metadata to function properly. =A0Can
>> we tack MSG_EOR onto a SOCK_STREAM socket? =A0If so, this would be simpl=
er
>> than requiring an ioctl.
>
> As I said, the mode setting part is already there. And using an ioctl()
> for these is wrong. You use setsockopt() for these kind of things.
>
> And I think you can add an MSG_EOR on a stream socket. Not sure that it
> really is needed. Sounds to me like a messed up detail in the MCAP
> specification that would work anyway without extra flags. Have to read
> the spec. at some point.
>
>> > > =A0 Currently, the code is branched off bluetooth-testing from last =
week.
>> > > The tree is available via:
>> > >
>> > > =A0 =A0 =A0git clone git://staticfree.info/git/el2cap
>> > >
>> > > I'd like to thank my friend Steve from the MIT Mobile Experience Lab=
 for
>> > > graciously hosting the code. =A0My latest work is in the "retransmis=
sion"
>> > > branch. =A0Due to the increase in code size, I've split the files up=
 into
>> > > an l2cap sub-directory under net/bluetooth. =A0I've not yet cloned
>> > > Gustavo's repository, but I should be able to rebase the code off th=
at
>> > > as necessary.
>> > > =A0 If there are any comments on style or implementation, I'd be hap=
py to
>> > > address them in the code. =A0Hopefully, this can be used as base for
>> > > Gustavo's SOC work so we can focus upon complete implementation of t=
he
>> > > Enhanced L2CAP specification and interoperability with other devices=
.
>> >
>> > The split into l2cap.c and queue.c and move to a separate directory
>> > looks totally useless to me. I know that l2cap.c is big, but there is =
no
>> > real separation. You would have to convince why this makes sense.
>>
>> Just trying to keep things manageable. =A0Currently, I'm not using any o=
f
>> the code in queue.c anyway, so I'm tempted to just drop it.
>
> Just drop it for now. We can change the layout later if it is really
> needed. Currently it is just confusing.
>
> Regards
>
> Marcel
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>



--=20
Gustavo F. Padovan
http://padovan.org

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Gustavo F. Padovan]

Hi Nathan and Marcel,

I'm working on add the initial constant change to upstream. Until now
I have squashed your 6 first commits and have made change on it.

http://www.las.ic.unicamp.br/~gustavo/git/?p=bluetooth-testing.git;a=commit;h=a75b529b5e70ecbbe4dd56527300bf2d4b95de2f

 A little report and some question about it:

1. I Removed the L2CAP features bits that we won't use now. I'm
supposing that we are going to implement ERTM, Streaming mode and FCS
Option.

2. What did you mean with  "__u16    mtu_cnl;" ( first patch, I
think)? connectionless MTU?


3. #define L2CAP_MODE_DEFAULT    L2CAP_MODE_ENH_RETRANS
I think that we can't default this now, because de code goes to
upstream, same for L2CAP_FCS_DEFAULT.

4.
@@ -1711,12 +1715,25 @@ static int l2cap_build_conf_req(struct sock
*sk, void *data)
...
+       if (pi->mode != L2CAP_MODE_BASIC) {
+               rfc.mode = pi->mode;
+               rfc.txwin_size = L2CAP_DEFAULT_RX_WINDOW;
+               rfc.max_transmit = L2CAP_DEFAULT_MAX_RECEIVE;
+               rfc.retrans_timeout =
cpu_to_le16(L2CAP_DEFAULT_RETRANS_TIMEOUT);
+               rfc.monitor_timeout =
cpu_to_le16(L2CAP_DEFAULT_MONITOR_TIMEOUT);
+               rfc.max_pdu_size = cpu_to_le16(L2CAP_DEFAULT_MAX_RX_APDU);
+
+               l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), &rfc);
+       }

That's wrong. pi->mode == L2CAP_MODE_ENH_RETRANS it's better.
txwin_size, max_transmit, retrans_timeout and monitor_timeout should
be 0 in L2CAP_MODE_STREAMING.




Probably my changes broke up all other commit and we will need to
apply then manually, but also we need to revert the l2cap.c split, so
the code will get broke either way.

We need still separate the code in logical and bisectable commits. I
think that we can put a first commit only with the constants
definitions, and then, other commit with all the other mode
negotiation ( just for ERTM or ERTM and streaming mode) and just  then
put the first code for sending and receive packets. What do you think?

Also I propose we implement things by parts. First ERTM, then
Streaming mode and by the end, FCS Option.



On Tue, Apr 28, 2009 at 4:14 PM, Gustavo F. Padovan
<gustavo@las.ic.unicamp.br> wrote:
> Hi Nathan,
>
> Your work will be very useful on My GSoC. We can work together on
> implementing ERTM. I was just starting to study how to do mode
> negotiaton. I'll stop it and go look through your code to understand
> it and start coding something.
>
> I think we need a TO DO list of ERTM stuff with the things that left
> to implement and the this Marcel suggested. First I think that we need
> do changes suggested by Marcel to push the to upstream. After we can
> concentrate on the issues left to implement. I'm planning to start
> this tonight.
>
> First I need to study your code, then we can discuss it more. I need
> to learn many things about l2cap and ERTM yet =).
>
> Are you around #bluez on freenode?
>
> Regards.
>



-- 
Gustavo F. Padovan
http://padovan.org

Re: GSoC 2009: L2CAP Enhanced Retransmission mode

[Marcel Holtmann]

Hi Gustavo,

> I'm working on add the initial constant change to upstream. Until now
> I have squashed your 6 first commits and have made change on it.
> 
> http://www.las.ic.unicamp.br/~gustavo/git/?p=bluetooth-testing.git;a=commit;h=a75b529b5e70ecbbe4dd56527300bf2d4b95de2f

please send a proper inline mail with this patch to the mailing list so
I can comment on it.

>  A little report and some question about it:
> 
> 1. I Removed the L2CAP features bits that we won't use now. I'm
> supposing that we are going to implement ERTM, Streaming mode and FCS
> Option.

The order should be ERTM, FCS Option and then Streaming mode.

> 2. What did you mean with  "__u16    mtu_cnl;" ( first patch, I
> think)? connectionless MTU?

Don't bother with this values. We are not doing connless support.

> 3. #define L2CAP_MODE_DEFAULT    L2CAP_MODE_ENH_RETRANS
> I think that we can't default this now, because de code goes to
> upstream, same for L2CAP_FCS_DEFAULT.

For SOCK_SEQPACKET, the default mode is BASIC, for SOCK_STREAM it will
be ERTM. When using SOCK_SEQPACKET, the socket option L2CAP_OPTIONS need
be used to select ERTM. That option is already present.

> 4.
> @@ -1711,12 +1715,25 @@ static int l2cap_build_conf_req(struct sock
> *sk, void *data)
> ...
> +       if (pi->mode != L2CAP_MODE_BASIC) {
> +               rfc.mode = pi->mode;
> +               rfc.txwin_size = L2CAP_DEFAULT_RX_WINDOW;
> +               rfc.max_transmit = L2CAP_DEFAULT_MAX_RECEIVE;
> +               rfc.retrans_timeout =
> cpu_to_le16(L2CAP_DEFAULT_RETRANS_TIMEOUT);
> +               rfc.monitor_timeout =
> cpu_to_le16(L2CAP_DEFAULT_MONITOR_TIMEOUT);
> +               rfc.max_pdu_size = cpu_to_le16(L2CAP_DEFAULT_MAX_RX_APDU);
> +
> +               l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc), &rfc);
> +       }
> 
> That's wrong. pi->mode == L2CAP_MODE_ENH_RETRANS it's better.
> txwin_size, max_transmit, retrans_timeout and monitor_timeout should
> be 0 in L2CAP_MODE_STREAMING.

Agreed.

Regards

Marcel