Re: RFC: Allow Bluez to select flushable or non-flushable ACL packets with L2CAP_LM_RELIABLE

[Nick Pelly <npelly@google.com>]

On Wed, Jun 16, 2010 at 4:40 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann <marcel@holtmann.org> wr=
ote:
>> Hi Nick,
>>
>>> >>> >> Right now Bluez always requests flushable ACL packets (but does =
not
>>> >>> >> set a flush timeout, so effectively they are non-flushable):
>>> >>> >>
>>> >>> >> However it is desirable to use an ACL flush timeout on A2DP pack=
ets so
>>> >>> >> that if the ACL packets block for some reason then the LM can fl=
ush
>>> >>> >> them to make room for newer packets.
>>> >>> >>
>>> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary f=
lag by
>>> >>> >> default (non-flushable packet), and let userspace request flusha=
ble
>>> >>> >> packets on A2DP L2CAP sockets with the socket option
>>> >>> >> L2CAP_LM_RELIABLE.
>>> >>> >
>>> >>> > the reliable option has a different meaning. It comes back from t=
he old
>>> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP t=
hat had
>>> >>> > to confirm that we can detect malformed packets and report them. =
These
>>> >>> > days it is just fine to drop them.
>>> >>>
>>> >>> Got it, how about introducing
>>> >>>
>>> >>> #define L2CAP_LM_FLUSHABLE 0x0040
>>> >>
>>> >> that l2cap_sock_setsockopt_old() sets this didn't give you a hint th=
at
>>> >> we might wanna deprecate this socket options ;)
>>> >>
>>> >> I need to read up on the flushable stuff, but in the end it deserves=
 its
>>> >> own socket option. Also an ioctl() to actually trigger Enhanced flus=
h
>>> >> might be needed.
>>> >>
>>> >>> struct l2cap_pinfo {
>>> >>> =A0 =A0...
>>> >>> =A0 =A0__u8 flushable;
>>> >>> }
>>> >>
>>> >> Sure. In the long run we need to turn this into a bitmask. We are ju=
st
>>> >> wasting memory here.
>>> >
>>> > Attached is an updated patch, that checks the LMP features bitmask
>>> > before using the new non-flushable packet type.
>>> >
>>> > I am still using L2CAP_LM_FLUSHABLE socket option in
>>> > l2cap_sock_setsockopt_old(), which I don't think you are happy with.
>>> > So how about a new option:
>>> >
>>> > SOL_L2CAP, L2CAP_ACL_FLUSH
>>> > which has a default value of 0, and can be set to 1 to make the ACL
>>> > data sent by this L2CAP socket flushable.
>>> >
>>> > In a later commit we would then add
>>> > SOL_ACL, ACL_FLUSH_TIMEOUT
>>> > That is used to set an automatic flush timeout for the ACL link on a
>>> > L2CAP socket. Note that SOL_ACL is new.
>>> >
>>> > But maybe this is not what you had in mind, so I'm looking for your
>>> > advice before I implement this.
>>>
>>> Attached an updated patch for 2.6.32 kernel. We've been using this
>>> patch successfully on production devices.
>>
>> can see anything wrong with that patch. However we need to use
>> SOL_BLUETOOTH for it of course. So we need to come up with something to
>> make this simple.
>>
>> An additional change I like to see is to use flags for booleans like
>> flushable in the structures. Can you work on changing that.
>>
>> Also do we have decoding support for this in hcidump. It might be nice
>> to include some really simple examples in the commit message.
>>
>> Regards
>
> I would like to play a little bit with this, so is there any missing upda=
tes?

Nope, that is our most recent version.

Nick