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 packets so
that if the ACL packets block for some reason then the LM can flush
them to make room for newer packets.
Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
default (non-flushable packet), and let userspace request flushable
packets on A2DP L2CAP sockets with the socket option
L2CAP_LM_RELIABLE.
Make sense? Any other suggestions?
Attached is a rough (untested) patch of what I had in mind.
See core spec 2.E.5.4.2 for reference.
Nick
Hi Marcel,
On Fri, Dec 18, 2009, Marcel Holtmann wrote:
> > > are you expecting to change this frequently and from different parts of
> > > the code during the lifetime of a socket. I just don't see that
> > > happening at all actually. Either you create a "flushable" socket or you
> > > don't. Fill me in on how you wanna actually use this feature.
> >
> > My use case is just for A2DP. I turn on flushing with a timeout of say
> > 160ms just before starting streaming of A2DP data, and turn it off
> > when I finish. This is not a problem with either API proposal.
>
> I count this as creating socket, setting flushable and then using it.
> Then closing it. And especially in A2DP case where the media socket is
> brought up and taken down a lot that is a proper usage. However I do
> expect that each socket should not change from flushable to
> non-flushable in mid term usage. While potentially possible it don't see
> its usage at all.
I hope you remember that the A2DP media transport socket is kept open
much longer periods than e.g. the SCO socket in HFP/HSP since suspending
an A2DP stream only means stopping sending data but still keeping the
media socket open. Only when the configured codec parameters aren't
suitable for the audio data would you close, reconfigure and recreate
the media socket. So is it ok to keep the flushable setting even while
the A2DP stream is suspended?
Johan
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 packets so
> >> >> > > > >> >> >> that if the ACL packets block for some reason
> >> >> then the LM can flush
> >> >> > > > >> >> >> them to make room for newer packets.
> >> >> > > > >> >> >>
> >> >> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL
> >> >> packet boundary flag by
> >> >> > > > >> >> >> default (non-flushable packet), and let
> >> >> userspace request flushable
> >> >> > > > >> >> >> packets on A2DP L2CAP sockets with the socket
> >> >> option
> >> >> > > > >> >> >> L2CAP_LM_RELIABLE.
> >> >> > > > >> >> >
> >> >> > > > >> >> > the reliable option has a different meaning. It
> >> >> comes back from the old
> >> >> > > > >> >> > Bluetooth 1.1 qualification days where we had to
> >> >> tests on L2CAP that 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 that
> >> >> > > > >> > 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 flush
> >> >> > > > >> > might be needed.
> >> >> > > > >> >
> >> >> > > > >> >> struct l2cap_pinfo {
> >> >> > > > >> >> ...
> >> >> > > > >> >> __u8 flushable;
> >> >> > > > >> >> }
> >> >> > > > >> >
> >> >> > > > >> > Sure. In the long run we need to turn this into a
> >> >> bitmask. We are just
> >> >> > > > >> > 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.
> >> >> > > >
> >> >> > > > Was this proposal ok?
> >> >> > >
> >> >> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> >> >> > >
> >> >> > > > >> 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.
> >> >> > > > >
> >> >> > > > > can I stop you right here (without even looking at the
> >> >> patch). We do
> >> >> > > > > have the generic SOL_BLUETOOTH that you should be
> >> >> using. So adding
> >> >> > > > > SOL_ACL is not a viable option at all.
> >> >> > > >
> >> >> > > > This would be in a later patch, and SOL_BLUETOOTH,
> >> >> ACL_FLUSH_TIMEOUT
> >> >> > > > is fine too, or whatever you prefer.
> >> >> > >
> >> >> > > Why not just use BT_FLUSHABLE and have it always take a
> >> >> timeout option
> >> >> > > and then 0 means not flushable. And advantage of having it
> >> >> separated?
> >> >> >
> >> >> > I think keeping them separate makes it clear that the flush
> >> >> timeout is
> >> >> > global for a given ACL link, whereas the
> >> >> flushable/non-flushable
> >> >> > boolean is specific to a L2CAP channel. (Which is why I
> >> >> suggested
> >> >> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT
> >> >> option -
> >> >> > since this option applies at the ACL level in the stack).
> >> >> >
> >> >> > A specific advantage of this is that flushable packets can
> >> >> be enabled
> >> >> > without over-writing a previous flush timeout that was set
> >> >> on a
> >> >> > different L2CAP socket on the same ACL link. I guess this
> >> >> can also be
> >> >> > achieved with getsockopt() but that is racy.
> >> >>
> >> >>
> >> >> I am talking here about Enhanced Flush support and that would
> >> >> happen on
> >> >> a per ACL handle basis. So it actually almost applies on a per
> >> >> L2CAP
> >> >> socket level. Only exception is if you establish two or more
> >> >> L2CAP
> >> >> connections to the same remote device and set them all to
> >> >> flushable.
> >> >> Then of course all of them will be flushed. So strictly
> >> >> speaking it
> >> >> might be an ACL link feature, but we don't wanna use it that
> >> >> way. And in
> >> >> practice you won't have multiple concurrent flushable L2CAP
> >> >> connections
> >> >> to one remote device anyway.
> >> >>
> >> >>
> >> >> I agree that having 2 flush-able L2CAP channels to the same device
> >> >> would probably not be common. But who knows what new profiles the
> >> >> Bluetooth SIG will come up with that might also benefit from
> >> >> flush-able ACL data. And if a use-case comes up, then your proposed
> >> >> API will require programmers to write a racy getsockopt/setsockopt if
> >> >> they want to turn on flushing on one l2cap connection without
> >> >> affecting the ACL flush timeout set by another connection. Building
> >> >> race conditions into an API seems like a sub-optimal design choice.
> >> >
> >> > are you expecting to change this frequently and from different parts of
> >> > the code during the lifetime of a socket. I just don't see that
> >> > happening at all actually. Either you create a "flushable" socket or you
> >> > don't. Fill me in on how you wanna actually use this feature.
> >>
> >> My use case is just for A2DP. I turn on flushing with a timeout of say
> >> 160ms just before starting streaming of A2DP data, and turn it off
> >> when I finish. This is not a problem with either API proposal.
> >
> > I count this as creating socket, setting flushable and then using it.
> > Then closing it. And especially in A2DP case where the media socket is
> > brought up and taken down a lot that is a proper usage. However I do
> > expect that each socket should not change from flushable to
> > non-flushable in mid term usage. While potentially possible it don't see
> > its usage at all.
> >
> > So we could even force the flushable option into non-changeable after
> > the socket has been connected. Like changing the MTU afterwards makes no
> > sense.
> >
> >> Where it becomes a problem is if there is a reason to have two
> >> flush-able L2CAP connections to the same host. With your API proposal,
> >> the second connection has no way of turning on flushing without
> >> over-writing the flush timeout set by the first socket. You could
> >> implement another API to read the current flush timeout, and have the
> >> second socket read that API, but thats racy.
> >>
> >> If this is not a use-case you care about, then ok. But I just want to
> >> point out that this is a problem that will be baked into the API - and
> >> will require ugly workarounds in userspace as soon as someone requires
> >> 2 flushable L2CAP connections to one host. Given the rate at which
> >> Bluetooth changes and new profiles and use cases are added I would not
> >> be so quick to dismiss this use case.
> >
> > So my idea would actually be that every socket can has its own flush
> > timeout, but the core than picks the time to actually do the flushing of
> > packets. Also we can not have one socket change a socket option of
> > another one. It is a per socket option and not a global one.
>
> I think you are confused. This patch does not implement HCI Enhance
> Flush Command. The flush timeout that I am referring to is passed to
> the Bluetooth Chipset with the HCI Write Automatic Flush Timeout
> command. Which is why it is global for the ACL link.
I was clearly talking about Enhanced Flush support and not the automatic
flush timeout. The automatic flush timeout should clearly not be a L2CAP
socket option. That would be just wrong. It would be great if they had a
default variant so we can just make this a main.conf option, but that is
out of the question.
So personally I think using the automatic flush timeout would be not a
really good choice for us. Just implemented Enhanced Flush would allow
us to achieve exactly the same with a lot of more control over what is
going on.
Regards
Marcel
> Since the flushable is only useful for the time in between the packet is =
hold in
> the Bluetooth chip buffers and hasn't been transmitted over the air.
> Once the packet is on the air, there is nothing to flush anymore. And
> with L2CAP ERTM this all becomes obsolete since we can flush at any time
> anyway. The retransmission takes care of any accidental flush.
Marcel, by the time L2CAP ERTM will get in to real life products whole blob=
of new surprises/problems will show up:)
What Nick did - improves A2DP audio quality rather dramatically especially=
for (low performance) embedded devices working with real life Headsets. Cu=
rrent BlueZ A2DP is good proof of concept, but to make it usable quite few=
things are needed. One of them is this RFC.=20
Thanks!
Oleg.
=20
________________________________________
From: [email protected] [[email protected]=
nel.org] On Behalf Of Marcel Holtmann [[email protected]]
Sent: Friday, December 18, 2009 8:05 PM
To: Nick Pelly
Cc: [email protected]
Subject: Re: RFC: Allow Bluez to select flushable or non-flushable ACL pack=
ets with L2CAP_LM_RELIABLE
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 packets so
> >> > > > >> >> >> that if the ACL packets block for some reason
> >> then the LM can flush
> >> > > > >> >> >> them to make room for newer packets.
> >> > > > >> >> >>
> >> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL
> >> packet boundary flag by
> >> > > > >> >> >> default (non-flushable packet), and let
> >> userspace request flushable
> >> > > > >> >> >> packets on A2DP L2CAP sockets with the socket
> >> option
> >> > > > >> >> >> L2CAP_LM_RELIABLE.
> >> > > > >> >> >
> >> > > > >> >> > the reliable option has a different meaning. It
> >> comes back from the old
> >> > > > >> >> > Bluetooth 1.1 qualification days where we had to
> >> tests on L2CAP that 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 that
> >> > > > >> > 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 flush
> >> > > > >> > might be needed.
> >> > > > >> >
> >> > > > >> >> struct l2cap_pinfo {
> >> > > > >> >> ...
> >> > > > >> >> __u8 flushable;
> >> > > > >> >> }
> >> > > > >> >
> >> > > > >> > Sure. In the long run we need to turn this into a
> >> bitmask. We are just
> >> > > > >> > 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.
> >> > > >
> >> > > > Was this proposal ok?
> >> > >
> >> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> >> > >
> >> > > > >> 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.
> >> > > > >
> >> > > > > can I stop you right here (without even looking at the
> >> patch). We do
> >> > > > > have the generic SOL_BLUETOOTH that you should be
> >> using. So adding
> >> > > > > SOL_ACL is not a viable option at all.
> >> > > >
> >> > > > This would be in a later patch, and SOL_BLUETOOTH,
> >> ACL_FLUSH_TIMEOUT
> >> > > > is fine too, or whatever you prefer.
> >> > >
> >> > > Why not just use BT_FLUSHABLE and have it always take a
> >> timeout option
> >> > > and then 0 means not flushable. And advantage of having it
> >> separated?
> >> >
> >> > I think keeping them separate makes it clear that the flush
> >> timeout is
> >> > global for a given ACL link, whereas the
> >> flushable/non-flushable
> >> > boolean is specific to a L2CAP channel. (Which is why I
> >> suggested
> >> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT
> >> option -
> >> > since this option applies at the ACL level in the stack).
> >> >
> >> > A specific advantage of this is that flushable packets can
> >> be enabled
> >> > without over-writing a previous flush timeout that was set
> >> on a
> >> > different L2CAP socket on the same ACL link. I guess this
> >> can also be
> >> > achieved with getsockopt() but that is racy.
> >>
> >>
> >> I am talking here about Enhanced Flush support and that would
> >> happen on
> >> a per ACL handle basis. So it actually almost applies on a per
> >> L2CAP
> >> socket level. Only exception is if you establish two or more
> >> L2CAP
> >> connections to the same remote device and set them all to
> >> flushable.
> >> Then of course all of them will be flushed. So strictly
> >> speaking it
> >> might be an ACL link feature, but we don't wanna use it that
> >> way. And in
> >> practice you won't have multiple concurrent flushable L2CAP
> >> connections
> >> to one remote device anyway.
> >>
> >>
> >> I agree that having 2 flush-able L2CAP channels to the same device
> >> would probably not be common. But who knows what new profiles the
> >> Bluetooth SIG will come up with that might also benefit from
> >> flush-able ACL data. And if a use-case comes up, then your proposed
> >> API will require programmers to write a racy getsockopt/setsockopt if
> >> they want to turn on flushing on one l2cap connection without
> >> affecting the ACL flush timeout set by another connection. Building
> >> race conditions into an API seems like a sub-optimal design choice.
> >
> > are you expecting to change this frequently and from different parts of
> > the code during the lifetime of a socket. I just don't see that
> > happening at all actually. Either you create a "flushable" socket or yo=
u
> > don't. Fill me in on how you wanna actually use this feature.
>
> My use case is just for A2DP. I turn on flushing with a timeout of say
> 160ms just before starting streaming of A2DP data, and turn it off
> when I finish. This is not a problem with either API proposal.
I count this as creating socket, setting flushable and then using it.
Then closing it. And especially in A2DP case where the media socket is
brought up and taken down a lot that is a proper usage. However I do
expect that each socket should not change from flushable to
non-flushable in mid term usage. While potentially possible it don't see
its usage at all.
So we could even force the flushable option into non-changeable after
the socket has been connected. Like changing the MTU afterwards makes no
sense.
> Where it becomes a problem is if there is a reason to have two
> flush-able L2CAP connections to the same host. With your API proposal,
> the second connection has no way of turning on flushing without
> over-writing the flush timeout set by the first socket. You could
> implement another API to read the current flush timeout, and have the
> second socket read that API, but thats racy.
>
> If this is not a use-case you care about, then ok. But I just want to
> point out that this is a problem that will be baked into the API - and
> will require ugly workarounds in userspace as soon as someone requires
> 2 flushable L2CAP connections to one host. Given the rate at which
> Bluetooth changes and new profiles and use cases are added I would not
> be so quick to dismiss this use case.
So my idea would actually be that every socket can has its own flush
timeout, but the core than picks the time to actually do the flushing of
packets. Also we can not have one socket change a socket option of
another one. It is a per socket option and not a global one.
On other possible way would be to use CMSG details to inform sockets
about flushable packets. We have to see how useful that is. Since the
flushable is only useful for the time in between the packet is hold in
the Bluetooth chip buffers and hasn't been transmitted over the air.
Once the packet is on the air, there is nothing to flush anymore. And
with L2CAP ERTM this all becomes obsolete since we can flush at any time
anyway. The retransmission takes care of any accidental flush.
Regards
Marcel
On Fri, Dec 18, 2009 at 6:05 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Nick,
>
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> Right now Bluez always requests flusha=
ble ACL
>> >> =A0 =A0 =A0 =A0 packets (but does not
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> set a flush timeout, so effectively th=
ey are
>> >> =A0 =A0 =A0 =A0 non-flushable):
>> >> =A0 =A0 =A0 =A0 > > > >> >> >>
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> However it is desirable to use an ACL =
flush
>> >> =A0 =A0 =A0 =A0 timeout on A2DP packets so
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> that if the ACL packets block for some=
reason
>> >> =A0 =A0 =A0 =A0 then the LM can flush
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> them to make room for newer packets.
>> >> =A0 =A0 =A0 =A0 > > > >> >> >>
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> Is it reasonable for Bluez to use the =
0x00 ACL
>> >> =A0 =A0 =A0 =A0 packet boundary flag by
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> default (non-flushable packet), and le=
t
>> >> =A0 =A0 =A0 =A0 userspace request flushable
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> packets on A2DP L2CAP sockets with the=
socket
>> >> =A0 =A0 =A0 =A0 option
>> >> =A0 =A0 =A0 =A0 > > > >> >> >> L2CAP_LM_RELIABLE.
>> >> =A0 =A0 =A0 =A0 > > > >> >> >
>> >> =A0 =A0 =A0 =A0 > > > >> >> > the reliable option has a different mea=
ning. It
>> >> =A0 =A0 =A0 =A0 comes back from the old
>> >> =A0 =A0 =A0 =A0 > > > >> >> > Bluetooth 1.1 qualification days where =
we had to
>> >> =A0 =A0 =A0 =A0 tests on L2CAP that had
>> >> =A0 =A0 =A0 =A0 > > > >> >> > to confirm that we can detect malformed=
packets
>> >> =A0 =A0 =A0 =A0 and report them. These
>> >> =A0 =A0 =A0 =A0 > > > >> >> > days it is just fine to drop them.
>> >> =A0 =A0 =A0 =A0 > > > >> >>
>> >> =A0 =A0 =A0 =A0 > > > >> >> Got it, how about introducing
>> >> =A0 =A0 =A0 =A0 > > > >> >>
>> >> =A0 =A0 =A0 =A0 > > > >> >> #define L2CAP_LM_FLUSHABLE 0x0040
>> >> =A0 =A0 =A0 =A0 > > > >> >
>> >> =A0 =A0 =A0 =A0 > > > >> > that l2cap_sock_setsockopt_old() sets this=
didn't
>> >> =A0 =A0 =A0 =A0 give you a hint that
>> >> =A0 =A0 =A0 =A0 > > > >> > we might wanna deprecate this socket optio=
ns ;)
>> >> =A0 =A0 =A0 =A0 > > > >> >
>> >> =A0 =A0 =A0 =A0 > > > >> > I need to read up on the flushable stuff, =
but in
>> >> =A0 =A0 =A0 =A0 the end it deserves its
>> >> =A0 =A0 =A0 =A0 > > > >> > own socket option. Also an ioctl() to actu=
ally
>> >> =A0 =A0 =A0 =A0 trigger Enhanced flush
>> >> =A0 =A0 =A0 =A0 > > > >> > might be needed.
>> >> =A0 =A0 =A0 =A0 > > > >> >
>> >> =A0 =A0 =A0 =A0 > > > >> >> struct l2cap_pinfo {
>> >> =A0 =A0 =A0 =A0 > > > >> >> =A0 =A0...
>> >> =A0 =A0 =A0 =A0 > > > >> >> =A0 =A0__u8 flushable;
>> >> =A0 =A0 =A0 =A0 > > > >> >> }
>> >> =A0 =A0 =A0 =A0 > > > >> >
>> >> =A0 =A0 =A0 =A0 > > > >> > Sure. In the long run we need to turn this=
into a
>> >> =A0 =A0 =A0 =A0 bitmask. We are just
>> >> =A0 =A0 =A0 =A0 > > > >> > wasting memory here.
>> >> =A0 =A0 =A0 =A0 > > > >>
>> >> =A0 =A0 =A0 =A0 > > > >> Attached is an updated patch, that checks th=
e LMP
>> >> =A0 =A0 =A0 =A0 features bitmask
>> >> =A0 =A0 =A0 =A0 > > > >> before using the new non-flushable packet ty=
pe.
>> >> =A0 =A0 =A0 =A0 > > > >>
>> >> =A0 =A0 =A0 =A0 > > > >> I am still using L2CAP_LM_FLUSHABLE socket o=
ption in
>> >> =A0 =A0 =A0 =A0 > > > >> l2cap_sock_setsockopt_old(), which I don't t=
hink you
>> >> =A0 =A0 =A0 =A0 are happy with.
>> >> =A0 =A0 =A0 =A0 > > > >> So how about a new option:
>> >> =A0 =A0 =A0 =A0 > > > >>
>> >> =A0 =A0 =A0 =A0 > > > >> SOL_L2CAP, L2CAP_ACL_FLUSH
>> >> =A0 =A0 =A0 =A0 > > > >> which has a default value of 0, and can be s=
et to 1
>> >> =A0 =A0 =A0 =A0 to make the ACL
>> >> =A0 =A0 =A0 =A0 > > > >> data sent by this L2CAP socket flushable.
>> >> =A0 =A0 =A0 =A0 > > >
>> >> =A0 =A0 =A0 =A0 > > > Was this proposal ok?
>> >> =A0 =A0 =A0 =A0 > >
>> >> =A0 =A0 =A0 =A0 > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for t=
his.
>> >> =A0 =A0 =A0 =A0 > >
>> >> =A0 =A0 =A0 =A0 > > > >> In a later commit we would then add
>> >> =A0 =A0 =A0 =A0 > > > >> SOL_ACL, ACL_FLUSH_TIMEOUT
>> >> =A0 =A0 =A0 =A0 > > > >> That is used to set an automatic flush timeo=
ut for
>> >> =A0 =A0 =A0 =A0 the ACL link on a
>> >> =A0 =A0 =A0 =A0 > > > >> L2CAP socket. Note that SOL_ACL is new.
>> >> =A0 =A0 =A0 =A0 > > > >
>> >> =A0 =A0 =A0 =A0 > > > > can I stop you right here (without even looki=
ng at the
>> >> =A0 =A0 =A0 =A0 patch). We do
>> >> =A0 =A0 =A0 =A0 > > > > have the generic SOL_BLUETOOTH that you shoul=
d be
>> >> =A0 =A0 =A0 =A0 using. So adding
>> >> =A0 =A0 =A0 =A0 > > > > SOL_ACL is not a viable option at all.
>> >> =A0 =A0 =A0 =A0 > > >
>> >> =A0 =A0 =A0 =A0 > > > This would be in a later patch, and SOL_BLUETOO=
TH,
>> >> =A0 =A0 =A0 =A0 ACL_FLUSH_TIMEOUT
>> >> =A0 =A0 =A0 =A0 > > > is fine too, or whatever you prefer.
>> >> =A0 =A0 =A0 =A0 > >
>> >> =A0 =A0 =A0 =A0 > > Why not just use BT_FLUSHABLE and have it always =
take a
>> >> =A0 =A0 =A0 =A0 timeout option
>> >> =A0 =A0 =A0 =A0 > > and then 0 means not flushable. And advantage of =
having it
>> >> =A0 =A0 =A0 =A0 separated?
>> >> =A0 =A0 =A0 =A0 >
>> >> =A0 =A0 =A0 =A0 > I think keeping them separate makes it clear that t=
he flush
>> >> =A0 =A0 =A0 =A0 timeout is
>> >> =A0 =A0 =A0 =A0 > global for a given ACL link, whereas the
>> >> =A0 =A0 =A0 =A0 flushable/non-flushable
>> >> =A0 =A0 =A0 =A0 > boolean is specific to a L2CAP channel. (Which is w=
hy I
>> >> =A0 =A0 =A0 =A0 suggested
>> >> =A0 =A0 =A0 =A0 > introducing a new level SOL_ACL for the ACL_FLUSH_T=
IMEOUT
>> >> =A0 =A0 =A0 =A0 option -
>> >> =A0 =A0 =A0 =A0 > since this option applies at the ACL level in the s=
tack).
>> >> =A0 =A0 =A0 =A0 >
>> >> =A0 =A0 =A0 =A0 > A specific advantage of this is that flushable pack=
ets can
>> >> =A0 =A0 =A0 =A0 be enabled
>> >> =A0 =A0 =A0 =A0 > without over-writing a previous flush timeout that =
was set
>> >> =A0 =A0 =A0 =A0 on a
>> >> =A0 =A0 =A0 =A0 > different L2CAP socket on the same ACL link. I gues=
s this
>> >> =A0 =A0 =A0 =A0 can also be
>> >> =A0 =A0 =A0 =A0 > achieved with getsockopt() but that is racy.
>> >>
>> >>
>> >> =A0 =A0 =A0 =A0 I am talking here about Enhanced Flush support and th=
at would
>> >> =A0 =A0 =A0 =A0 happen on
>> >> =A0 =A0 =A0 =A0 a per ACL handle basis. So it actually almost applies=
on a per
>> >> =A0 =A0 =A0 =A0 L2CAP
>> >> =A0 =A0 =A0 =A0 socket level. Only exception is if you establish two =
or more
>> >> =A0 =A0 =A0 =A0 L2CAP
>> >> =A0 =A0 =A0 =A0 connections to the same remote device and set them al=
l to
>> >> =A0 =A0 =A0 =A0 flushable.
>> >> =A0 =A0 =A0 =A0 Then of course all of them will be flushed. So strict=
ly
>> >> =A0 =A0 =A0 =A0 speaking it
>> >> =A0 =A0 =A0 =A0 might be an ACL link feature, but we don't wanna use =
it that
>> >> =A0 =A0 =A0 =A0 way. And in
>> >> =A0 =A0 =A0 =A0 practice you won't have multiple concurrent flushable=
L2CAP
>> >> =A0 =A0 =A0 =A0 connections
>> >> =A0 =A0 =A0 =A0 to one remote device anyway.
>> >>
>> >>
>> >> I agree that having 2 flush-able L2CAP channels to the same device
>> >> would probably not be common. But who knows what new profiles the
>> >> Bluetooth SIG will come up with that might also benefit from
>> >> flush-able ACL data. And if a use-case comes up, then your proposed
>> >> API will require programmers to write a racy getsockopt/setsockopt if
>> >> they want to turn on flushing on one l2cap connection without
>> >> affecting the ACL flush timeout set by another connection. Building
>> >> race conditions into an API seems like a sub-optimal design choice.
>> >
>> > are you expecting to change this frequently and from different parts o=
f
>> > the code during the lifetime of a socket. I just don't see that
>> > happening at all actually. Either you create a "flushable" socket or y=
ou
>> > don't. Fill me in on how you wanna actually use this feature.
>>
>> My use case is just for A2DP. I turn on flushing with a timeout of say
>> 160ms just before starting streaming of A2DP data, and turn it off
>> when I finish. This is not a problem with either API proposal.
>
> I count this as creating socket, setting flushable and then using it.
> Then closing it. And especially in A2DP case where the media socket is
> brought up and taken down a lot that is a proper usage. However I do
> expect that each socket should not change from flushable to
> non-flushable in mid term usage. While potentially possible it don't see
> its usage at all.
>
> So we could even force the flushable option into non-changeable after
> the socket has been connected. Like changing the MTU afterwards makes no
> sense.
>
>> Where it becomes a problem is if there is a reason to have two
>> flush-able L2CAP connections to the same host. With your API proposal,
>> the second connection has no way of turning on flushing without
>> over-writing the flush timeout set by the first socket. You could
>> implement another API to read the current flush timeout, and have the
>> second socket read that API, but thats racy.
>>
>> If this is not a use-case you care about, then ok. But I just want to
>> point out that this is a problem that will be baked into the API - and
>> will require ugly workarounds in userspace as soon as someone requires
>> 2 flushable L2CAP connections to one host. Given the rate at which
>> Bluetooth changes and new profiles and use cases are added I would not
>> be so quick to dismiss this use case.
>
> So my idea would actually be that every socket can has its own flush
> timeout, but the core than picks the time to actually do the flushing of
> packets. Also we can not have one socket change a socket option of
> another one. It is a per socket option and not a global one.
I think you are confused. This patch does not implement HCI Enhance
Flush Command. The flush timeout that I am referring to is passed to
the Bluetooth Chipset with the HCI Write Automatic Flush Timeout
command. Which is why it is global for the ACL link.
>
> On other possible way would be to use CMSG details to inform sockets
> about flushable packets. We have to see how useful that is. Since the
> flushable is only useful for the time in between the packet is hold in
> the Bluetooth chip buffers and hasn't been transmitted over the air.
> Once the packet is on the air, there is nothing to flush anymore. And
> with L2CAP ERTM this all becomes obsolete since we can flush at any time
> anyway. The retransmission takes care of any accidental flush.
>
> Regards
>
> Marcel
>
>
>
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 packets so
> >> > > > >> >> >> that if the ACL packets block for some reason
> >> then the LM can flush
> >> > > > >> >> >> them to make room for newer packets.
> >> > > > >> >> >>
> >> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL
> >> packet boundary flag by
> >> > > > >> >> >> default (non-flushable packet), and let
> >> userspace request flushable
> >> > > > >> >> >> packets on A2DP L2CAP sockets with the socket
> >> option
> >> > > > >> >> >> L2CAP_LM_RELIABLE.
> >> > > > >> >> >
> >> > > > >> >> > the reliable option has a different meaning. It
> >> comes back from the old
> >> > > > >> >> > Bluetooth 1.1 qualification days where we had to
> >> tests on L2CAP that 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 that
> >> > > > >> > 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 flush
> >> > > > >> > might be needed.
> >> > > > >> >
> >> > > > >> >> struct l2cap_pinfo {
> >> > > > >> >> ...
> >> > > > >> >> __u8 flushable;
> >> > > > >> >> }
> >> > > > >> >
> >> > > > >> > Sure. In the long run we need to turn this into a
> >> bitmask. We are just
> >> > > > >> > 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.
> >> > > >
> >> > > > Was this proposal ok?
> >> > >
> >> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> >> > >
> >> > > > >> 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.
> >> > > > >
> >> > > > > can I stop you right here (without even looking at the
> >> patch). We do
> >> > > > > have the generic SOL_BLUETOOTH that you should be
> >> using. So adding
> >> > > > > SOL_ACL is not a viable option at all.
> >> > > >
> >> > > > This would be in a later patch, and SOL_BLUETOOTH,
> >> ACL_FLUSH_TIMEOUT
> >> > > > is fine too, or whatever you prefer.
> >> > >
> >> > > Why not just use BT_FLUSHABLE and have it always take a
> >> timeout option
> >> > > and then 0 means not flushable. And advantage of having it
> >> separated?
> >> >
> >> > I think keeping them separate makes it clear that the flush
> >> timeout is
> >> > global for a given ACL link, whereas the
> >> flushable/non-flushable
> >> > boolean is specific to a L2CAP channel. (Which is why I
> >> suggested
> >> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT
> >> option -
> >> > since this option applies at the ACL level in the stack).
> >> >
> >> > A specific advantage of this is that flushable packets can
> >> be enabled
> >> > without over-writing a previous flush timeout that was set
> >> on a
> >> > different L2CAP socket on the same ACL link. I guess this
> >> can also be
> >> > achieved with getsockopt() but that is racy.
> >>
> >>
> >> I am talking here about Enhanced Flush support and that would
> >> happen on
> >> a per ACL handle basis. So it actually almost applies on a per
> >> L2CAP
> >> socket level. Only exception is if you establish two or more
> >> L2CAP
> >> connections to the same remote device and set them all to
> >> flushable.
> >> Then of course all of them will be flushed. So strictly
> >> speaking it
> >> might be an ACL link feature, but we don't wanna use it that
> >> way. And in
> >> practice you won't have multiple concurrent flushable L2CAP
> >> connections
> >> to one remote device anyway.
> >>
> >>
> >> I agree that having 2 flush-able L2CAP channels to the same device
> >> would probably not be common. But who knows what new profiles the
> >> Bluetooth SIG will come up with that might also benefit from
> >> flush-able ACL data. And if a use-case comes up, then your proposed
> >> API will require programmers to write a racy getsockopt/setsockopt if
> >> they want to turn on flushing on one l2cap connection without
> >> affecting the ACL flush timeout set by another connection. Building
> >> race conditions into an API seems like a sub-optimal design choice.
> >
> > are you expecting to change this frequently and from different parts of
> > the code during the lifetime of a socket. I just don't see that
> > happening at all actually. Either you create a "flushable" socket or you
> > don't. Fill me in on how you wanna actually use this feature.
>
> My use case is just for A2DP. I turn on flushing with a timeout of say
> 160ms just before starting streaming of A2DP data, and turn it off
> when I finish. This is not a problem with either API proposal.
I count this as creating socket, setting flushable and then using it.
Then closing it. And especially in A2DP case where the media socket is
brought up and taken down a lot that is a proper usage. However I do
expect that each socket should not change from flushable to
non-flushable in mid term usage. While potentially possible it don't see
its usage at all.
So we could even force the flushable option into non-changeable after
the socket has been connected. Like changing the MTU afterwards makes no
sense.
> Where it becomes a problem is if there is a reason to have two
> flush-able L2CAP connections to the same host. With your API proposal,
> the second connection has no way of turning on flushing without
> over-writing the flush timeout set by the first socket. You could
> implement another API to read the current flush timeout, and have the
> second socket read that API, but thats racy.
>
> If this is not a use-case you care about, then ok. But I just want to
> point out that this is a problem that will be baked into the API - and
> will require ugly workarounds in userspace as soon as someone requires
> 2 flushable L2CAP connections to one host. Given the rate at which
> Bluetooth changes and new profiles and use cases are added I would not
> be so quick to dismiss this use case.
So my idea would actually be that every socket can has its own flush
timeout, but the core than picks the time to actually do the flushing of
packets. Also we can not have one socket change a socket option of
another one. It is a per socket option and not a global one.
On other possible way would be to use CMSG details to inform sockets
about flushable packets. We have to see how useful that is. Since the
flushable is only useful for the time in between the packet is hold in
the Bluetooth chip buffers and hasn't been transmitted over the air.
Once the packet is on the air, there is nothing to flush anymore. And
with L2CAP ERTM this all becomes obsolete since we can flush at any time
anyway. The retransmission takes care of any accidental flush.
Regards
Marcel
On Fri, Dec 18, 2009 at 4:26 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Nick,
>
>> =A0 =A0 =A0 =A0 > > > >> >> >> Right now Bluez always requests flushable=
ACL
>> =A0 =A0 =A0 =A0 packets (but does not
>> =A0 =A0 =A0 =A0 > > > >> >> >> set a flush timeout, so effectively they =
are
>> =A0 =A0 =A0 =A0 non-flushable):
>> =A0 =A0 =A0 =A0 > > > >> >> >>
>> =A0 =A0 =A0 =A0 > > > >> >> >> However it is desirable to use an ACL flu=
sh
>> =A0 =A0 =A0 =A0 timeout on A2DP packets so
>> =A0 =A0 =A0 =A0 > > > >> >> >> that if the ACL packets block for some re=
ason
>> =A0 =A0 =A0 =A0 then the LM can flush
>> =A0 =A0 =A0 =A0 > > > >> >> >> them to make room for newer packets.
>> =A0 =A0 =A0 =A0 > > > >> >> >>
>> =A0 =A0 =A0 =A0 > > > >> >> >> Is it reasonable for Bluez to use the 0x0=
0 ACL
>> =A0 =A0 =A0 =A0 packet boundary flag by
>> =A0 =A0 =A0 =A0 > > > >> >> >> default (non-flushable packet), and let
>> =A0 =A0 =A0 =A0 userspace request flushable
>> =A0 =A0 =A0 =A0 > > > >> >> >> packets on A2DP L2CAP sockets with the so=
cket
>> =A0 =A0 =A0 =A0 option
>> =A0 =A0 =A0 =A0 > > > >> >> >> L2CAP_LM_RELIABLE.
>> =A0 =A0 =A0 =A0 > > > >> >> >
>> =A0 =A0 =A0 =A0 > > > >> >> > the reliable option has a different meanin=
g. It
>> =A0 =A0 =A0 =A0 comes back from the old
>> =A0 =A0 =A0 =A0 > > > >> >> > Bluetooth 1.1 qualification days where we =
had to
>> =A0 =A0 =A0 =A0 tests on L2CAP that had
>> =A0 =A0 =A0 =A0 > > > >> >> > to confirm that we can detect malformed pa=
ckets
>> =A0 =A0 =A0 =A0 and report them. These
>> =A0 =A0 =A0 =A0 > > > >> >> > days it is just fine to drop them.
>> =A0 =A0 =A0 =A0 > > > >> >>
>> =A0 =A0 =A0 =A0 > > > >> >> Got it, how about introducing
>> =A0 =A0 =A0 =A0 > > > >> >>
>> =A0 =A0 =A0 =A0 > > > >> >> #define L2CAP_LM_FLUSHABLE 0x0040
>> =A0 =A0 =A0 =A0 > > > >> >
>> =A0 =A0 =A0 =A0 > > > >> > that l2cap_sock_setsockopt_old() sets this di=
dn't
>> =A0 =A0 =A0 =A0 give you a hint that
>> =A0 =A0 =A0 =A0 > > > >> > we might wanna deprecate this socket options =
;)
>> =A0 =A0 =A0 =A0 > > > >> >
>> =A0 =A0 =A0 =A0 > > > >> > I need to read up on the flushable stuff, but=
in
>> =A0 =A0 =A0 =A0 the end it deserves its
>> =A0 =A0 =A0 =A0 > > > >> > own socket option. Also an ioctl() to actuall=
y
>> =A0 =A0 =A0 =A0 trigger Enhanced flush
>> =A0 =A0 =A0 =A0 > > > >> > might be needed.
>> =A0 =A0 =A0 =A0 > > > >> >
>> =A0 =A0 =A0 =A0 > > > >> >> struct l2cap_pinfo {
>> =A0 =A0 =A0 =A0 > > > >> >> =A0 =A0...
>> =A0 =A0 =A0 =A0 > > > >> >> =A0 =A0__u8 flushable;
>> =A0 =A0 =A0 =A0 > > > >> >> }
>> =A0 =A0 =A0 =A0 > > > >> >
>> =A0 =A0 =A0 =A0 > > > >> > Sure. In the long run we need to turn this in=
to a
>> =A0 =A0 =A0 =A0 bitmask. We are just
>> =A0 =A0 =A0 =A0 > > > >> > wasting memory here.
>> =A0 =A0 =A0 =A0 > > > >>
>> =A0 =A0 =A0 =A0 > > > >> Attached is an updated patch, that checks the L=
MP
>> =A0 =A0 =A0 =A0 features bitmask
>> =A0 =A0 =A0 =A0 > > > >> before using the new non-flushable packet type.
>> =A0 =A0 =A0 =A0 > > > >>
>> =A0 =A0 =A0 =A0 > > > >> I am still using L2CAP_LM_FLUSHABLE socket opti=
on in
>> =A0 =A0 =A0 =A0 > > > >> l2cap_sock_setsockopt_old(), which I don't thin=
k you
>> =A0 =A0 =A0 =A0 are happy with.
>> =A0 =A0 =A0 =A0 > > > >> So how about a new option:
>> =A0 =A0 =A0 =A0 > > > >>
>> =A0 =A0 =A0 =A0 > > > >> SOL_L2CAP, L2CAP_ACL_FLUSH
>> =A0 =A0 =A0 =A0 > > > >> which has a default value of 0, and can be set =
to 1
>> =A0 =A0 =A0 =A0 to make the ACL
>> =A0 =A0 =A0 =A0 > > > >> data sent by this L2CAP socket flushable.
>> =A0 =A0 =A0 =A0 > > >
>> =A0 =A0 =A0 =A0 > > > Was this proposal ok?
>> =A0 =A0 =A0 =A0 > >
>> =A0 =A0 =A0 =A0 > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this=
.
>> =A0 =A0 =A0 =A0 > >
>> =A0 =A0 =A0 =A0 > > > >> In a later commit we would then add
>> =A0 =A0 =A0 =A0 > > > >> SOL_ACL, ACL_FLUSH_TIMEOUT
>> =A0 =A0 =A0 =A0 > > > >> That is used to set an automatic flush timeout =
for
>> =A0 =A0 =A0 =A0 the ACL link on a
>> =A0 =A0 =A0 =A0 > > > >> L2CAP socket. Note that SOL_ACL is new.
>> =A0 =A0 =A0 =A0 > > > >
>> =A0 =A0 =A0 =A0 > > > > can I stop you right here (without even looking =
at the
>> =A0 =A0 =A0 =A0 patch). We do
>> =A0 =A0 =A0 =A0 > > > > have the generic SOL_BLUETOOTH that you should b=
e
>> =A0 =A0 =A0 =A0 using. So adding
>> =A0 =A0 =A0 =A0 > > > > SOL_ACL is not a viable option at all.
>> =A0 =A0 =A0 =A0 > > >
>> =A0 =A0 =A0 =A0 > > > This would be in a later patch, and SOL_BLUETOOTH,
>> =A0 =A0 =A0 =A0 ACL_FLUSH_TIMEOUT
>> =A0 =A0 =A0 =A0 > > > is fine too, or whatever you prefer.
>> =A0 =A0 =A0 =A0 > >
>> =A0 =A0 =A0 =A0 > > Why not just use BT_FLUSHABLE and have it always tak=
e a
>> =A0 =A0 =A0 =A0 timeout option
>> =A0 =A0 =A0 =A0 > > and then 0 means not flushable. And advantage of hav=
ing it
>> =A0 =A0 =A0 =A0 separated?
>> =A0 =A0 =A0 =A0 >
>> =A0 =A0 =A0 =A0 > I think keeping them separate makes it clear that the =
flush
>> =A0 =A0 =A0 =A0 timeout is
>> =A0 =A0 =A0 =A0 > global for a given ACL link, whereas the
>> =A0 =A0 =A0 =A0 flushable/non-flushable
>> =A0 =A0 =A0 =A0 > boolean is specific to a L2CAP channel. (Which is why =
I
>> =A0 =A0 =A0 =A0 suggested
>> =A0 =A0 =A0 =A0 > introducing a new level SOL_ACL for the ACL_FLUSH_TIME=
OUT
>> =A0 =A0 =A0 =A0 option -
>> =A0 =A0 =A0 =A0 > since this option applies at the ACL level in the stac=
k).
>> =A0 =A0 =A0 =A0 >
>> =A0 =A0 =A0 =A0 > A specific advantage of this is that flushable packets=
can
>> =A0 =A0 =A0 =A0 be enabled
>> =A0 =A0 =A0 =A0 > without over-writing a previous flush timeout that was=
set
>> =A0 =A0 =A0 =A0 on a
>> =A0 =A0 =A0 =A0 > different L2CAP socket on the same ACL link. I guess t=
his
>> =A0 =A0 =A0 =A0 can also be
>> =A0 =A0 =A0 =A0 > achieved with getsockopt() but that is racy.
>>
>>
>> =A0 =A0 =A0 =A0 I am talking here about Enhanced Flush support and that =
would
>> =A0 =A0 =A0 =A0 happen on
>> =A0 =A0 =A0 =A0 a per ACL handle basis. So it actually almost applies on=
a per
>> =A0 =A0 =A0 =A0 L2CAP
>> =A0 =A0 =A0 =A0 socket level. Only exception is if you establish two or =
more
>> =A0 =A0 =A0 =A0 L2CAP
>> =A0 =A0 =A0 =A0 connections to the same remote device and set them all t=
o
>> =A0 =A0 =A0 =A0 flushable.
>> =A0 =A0 =A0 =A0 Then of course all of them will be flushed. So strictly
>> =A0 =A0 =A0 =A0 speaking it
>> =A0 =A0 =A0 =A0 might be an ACL link feature, but we don't wanna use it =
that
>> =A0 =A0 =A0 =A0 way. And in
>> =A0 =A0 =A0 =A0 practice you won't have multiple concurrent flushable L2=
CAP
>> =A0 =A0 =A0 =A0 connections
>> =A0 =A0 =A0 =A0 to one remote device anyway.
>>
>>
>> I agree that having 2 flush-able L2CAP channels to the same device
>> would probably not be common. But who knows what new profiles the
>> Bluetooth SIG will come up with that might also benefit from
>> flush-able ACL data. And if a use-case comes up, then your proposed
>> API will require programmers to write a racy getsockopt/setsockopt if
>> they want to turn on flushing on one l2cap connection without
>> affecting the ACL flush timeout set by another connection. Building
>> race conditions into an API seems like a sub-optimal design choice.
>
> are you expecting to change this frequently and from different parts of
> the code during the lifetime of a socket. I just don't see that
> happening at all actually. Either you create a "flushable" socket or you
> don't. Fill me in on how you wanna actually use this feature.
My use case is just for A2DP. I turn on flushing with a timeout of say
160ms just before starting streaming of A2DP data, and turn it off
when I finish. This is not a problem with either API proposal.
Where it becomes a problem is if there is a reason to have two
flush-able L2CAP connections to the same host. With your API proposal,
the second connection has no way of turning on flushing without
over-writing the flush timeout set by the first socket. You could
implement another API to read the current flush timeout, and have the
second socket read that API, but thats racy.
If this is not a use-case you care about, then ok. But I just want to
point out that this is a problem that will be baked into the API - and
will require ugly workarounds in userspace as soon as someone requires
2 flushable L2CAP connections to one host. Given the rate at which
Bluetooth changes and new profiles and use cases are added I would not
be so quick to dismiss this use case.
>
>> But its not worth arguing over. SOL_BLUETOOTH, BT_FLUSHABLE is fine
>> (or BT_FLUSH_TIMEOUT instead).
>
> I would call it BT_FLUSHABLE as of now. Since that is how the
> specification calls it. However I do have to refresh my memory with the
> actual details. I haven't read that part of the specification in a long
> time.
>
> Regards
>
> Marcel
>
>
>
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 packets so
> > > > >> >> >> that if the ACL packets block for some reason
> then the LM can flush
> > > > >> >> >> them to make room for newer packets.
> > > > >> >> >>
> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL
> packet boundary flag by
> > > > >> >> >> default (non-flushable packet), and let
> userspace request flushable
> > > > >> >> >> packets on A2DP L2CAP sockets with the socket
> option
> > > > >> >> >> L2CAP_LM_RELIABLE.
> > > > >> >> >
> > > > >> >> > the reliable option has a different meaning. It
> comes back from the old
> > > > >> >> > Bluetooth 1.1 qualification days where we had to
> tests on L2CAP that 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 that
> > > > >> > 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 flush
> > > > >> > might be needed.
> > > > >> >
> > > > >> >> struct l2cap_pinfo {
> > > > >> >> ...
> > > > >> >> __u8 flushable;
> > > > >> >> }
> > > > >> >
> > > > >> > Sure. In the long run we need to turn this into a
> bitmask. We are just
> > > > >> > 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.
> > > >
> > > > Was this proposal ok?
> > >
> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> > >
> > > > >> 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.
> > > > >
> > > > > can I stop you right here (without even looking at the
> patch). We do
> > > > > have the generic SOL_BLUETOOTH that you should be
> using. So adding
> > > > > SOL_ACL is not a viable option at all.
> > > >
> > > > This would be in a later patch, and SOL_BLUETOOTH,
> ACL_FLUSH_TIMEOUT
> > > > is fine too, or whatever you prefer.
> > >
> > > Why not just use BT_FLUSHABLE and have it always take a
> timeout option
> > > and then 0 means not flushable. And advantage of having it
> separated?
> >
> > I think keeping them separate makes it clear that the flush
> timeout is
> > global for a given ACL link, whereas the
> flushable/non-flushable
> > boolean is specific to a L2CAP channel. (Which is why I
> suggested
> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT
> option -
> > since this option applies at the ACL level in the stack).
> >
> > A specific advantage of this is that flushable packets can
> be enabled
> > without over-writing a previous flush timeout that was set
> on a
> > different L2CAP socket on the same ACL link. I guess this
> can also be
> > achieved with getsockopt() but that is racy.
>
>
> I am talking here about Enhanced Flush support and that would
> happen on
> a per ACL handle basis. So it actually almost applies on a per
> L2CAP
> socket level. Only exception is if you establish two or more
> L2CAP
> connections to the same remote device and set them all to
> flushable.
> Then of course all of them will be flushed. So strictly
> speaking it
> might be an ACL link feature, but we don't wanna use it that
> way. And in
> practice you won't have multiple concurrent flushable L2CAP
> connections
> to one remote device anyway.
>
>
> I agree that having 2 flush-able L2CAP channels to the same device
> would probably not be common. But who knows what new profiles the
> Bluetooth SIG will come up with that might also benefit from
> flush-able ACL data. And if a use-case comes up, then your proposed
> API will require programmers to write a racy getsockopt/setsockopt if
> they want to turn on flushing on one l2cap connection without
> affecting the ACL flush timeout set by another connection. Building
> race conditions into an API seems like a sub-optimal design choice.
are you expecting to change this frequently and from different parts of
the code during the lifetime of a socket. I just don't see that
happening at all actually. Either you create a "flushable" socket or you
don't. Fill me in on how you wanna actually use this feature.
> But its not worth arguing over. SOL_BLUETOOTH, BT_FLUSHABLE is fine
> (or BT_FLUSH_TIMEOUT instead).
I would call it BT_FLUSHABLE as of now. Since that is how the
specification calls it. However I do have to refresh my memory with the
actual details. I haven't read that part of the specification in a long
time.
Regards
Marcel
Hi Marcel,
On Fri, Dec 18, 2009 at 3:50 PM, Marcel Holtmann <[email protected]> wrot=
e:
>
> 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 A2D=
P packets so
> > > > >> >> >> that if the ACL packets block for some reason then the LM =
can flush
> > > > >> >> >> them to make room for newer packets.
> > > > >> >> >>
> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boun=
dary flag by
> > > > >> >> >> default (non-flushable packet), and let userspace request =
flushable
> > > > >> >> >> packets on A2DP L2CAP sockets with the socket option
> > > > >> >> >> L2CAP_LM_RELIABLE.
> > > > >> >> >
> > > > >> >> > the reliable option has a different meaning. It comes back =
from the old
> > > > >> >> > Bluetooth 1.1 qualification days where we had to tests on L=
2CAP that 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 h=
int that
> > > > >> > we might wanna deprecate this socket options ;)
> > > > >> >
> > > > >> > I need to read up on the flushable stuff, but in the end it de=
serves its
> > > > >> > own socket option. Also an ioctl() to actually trigger Enhance=
d flush
> > > > >> > 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 just
> > > > >> > wasting memory here.
> > > > >>
> > > > >> Attached is an updated patch, that checks the LMP features bitma=
sk
> > > > >> 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 w=
ith.
> > > > >> 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.
> > > >
> > > > Was this proposal ok?
> > >
> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> > >
> > > > >> 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.
> > > > >
> > > > > can I stop you right here (without even looking at the patch). We=
do
> > > > > have the generic SOL_BLUETOOTH that you should be using. So addin=
g
> > > > > SOL_ACL is not a viable option at all.
> > > >
> > > > This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOU=
T
> > > > is fine too, or whatever you prefer.
> > >
> > > Why not just use BT_FLUSHABLE and have it always take a timeout optio=
n
> > > and then 0 means not flushable. And advantage of having it separated?
> >
> > I think keeping them separate makes it clear that the flush timeout is
> > global for a given ACL link, whereas the flushable/non-flushable
> > boolean is specific to a L2CAP channel. (Which is why I suggested
> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT option -
> > since this option applies at the ACL level in the stack).
> >
> > A specific advantage of this is that flushable packets can be enabled
> > without over-writing a previous flush timeout that was set on a
> > different L2CAP socket on the same ACL link. I guess this can also be
> > achieved with getsockopt() but that is racy.
>
> I am talking here about Enhanced Flush support and that would happen on
> a per ACL handle basis. So it actually almost applies on a per L2CAP
> socket level. Only exception is if you establish two or more L2CAP
> connections to the same remote device and set them all to flushable.
> Then of course all of them will be flushed. So strictly speaking it
> might be an ACL link feature, but we don't wanna use it that way. And in
> practice you won't have multiple concurrent flushable L2CAP connections
> to one remote device anyway.
>
I agree that having 2=A0flush-able=A0L2CAP channels to the same device
would probably not be common. But who knows what new profiles the
Bluetooth SIG will come up with that might also benefit from
flush-able=A0ACL data. And when this happens, with your proposed API,
programmers will not be able to avoid a race condition between
getsockopt() then setsockopt() if they want to turn on flushing
without changing the ACL flush timeout.
But its not worth arguing over.=A0SOL_BLUETOOTH, BT_FLUSHABLE is fine
(or BT_FLUSH_TIMEOUT instead).
I won't be doing development for a few weeks over the holidays, so I
can not update this patch for a while.
Nick
On Fri, Dec 18, 2009 at 3:50 PM, Marcel Holtmann <[email protected]>wrote:
> 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
> packets so
> > > > >> >> >> that if the ACL packets block for some reason then the LM
> can flush
> > > > >> >> >> them to make room for newer packets.
> > > > >> >> >>
> > > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet
> boundary flag by
> > > > >> >> >> default (non-flushable packet), and let userspace request
> flushable
> > > > >> >> >> packets on A2DP L2CAP sockets with the socket option
> > > > >> >> >> L2CAP_LM_RELIABLE.
> > > > >> >> >
> > > > >> >> > the reliable option has a different meaning. It comes back
> from the old
> > > > >> >> > Bluetooth 1.1 qualification days where we had to tests on
> L2CAP that 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 that
> > > > >> > 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
> flush
> > > > >> > might be needed.
> > > > >> >
> > > > >> >> struct l2cap_pinfo {
> > > > >> >> ...
> > > > >> >> __u8 flushable;
> > > > >> >> }
> > > > >> >
> > > > >> > Sure. In the long run we need to turn this into a bitmask. We
> are just
> > > > >> > 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.
> > > >
> > > > Was this proposal ok?
> > >
> > > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> > >
> > > > >> 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.
> > > > >
> > > > > can I stop you right here (without even looking at the patch). We
> do
> > > > > have the generic SOL_BLUETOOTH that you should be using. So adding
> > > > > SOL_ACL is not a viable option at all.
> > > >
> > > > This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOUT
> > > > is fine too, or whatever you prefer.
> > >
> > > Why not just use BT_FLUSHABLE and have it always take a timeout option
> > > and then 0 means not flushable. And advantage of having it separated?
> >
> > I think keeping them separate makes it clear that the flush timeout is
> > global for a given ACL link, whereas the flushable/non-flushable
> > boolean is specific to a L2CAP channel. (Which is why I suggested
> > introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT option -
> > since this option applies at the ACL level in the stack).
> >
> > A specific advantage of this is that flushable packets can be enabled
> > without over-writing a previous flush timeout that was set on a
> > different L2CAP socket on the same ACL link. I guess this can also be
> > achieved with getsockopt() but that is racy.
>
> I am talking here about Enhanced Flush support and that would happen on
> a per ACL handle basis. So it actually almost applies on a per L2CAP
> socket level. Only exception is if you establish two or more L2CAP
> connections to the same remote device and set them all to flushable.
> Then of course all of them will be flushed. So strictly speaking it
> might be an ACL link feature, but we don't wanna use it that way. And in
> practice you won't have multiple concurrent flushable L2CAP connections
> to one remote device anyway.
>
I agree that having 2 flush-able L2CAP channels to the same device would
probably not be common. But who knows what new profiles the Bluetooth SIG
will come up with that might also benefit from flush-able ACL data. And if a
use-case comes up, then your proposed API will require programmers to write
a racy getsockopt/setsockopt if they want to turn on flushing on one l2cap
connection without affecting the ACL flush timeout set by another
connection. Building race conditions into an API seems like a sub-optimal
design choice.
But its not worth arguing over. SOL_BLUETOOTH, BT_FLUSHABLE is fine (or
BT_FLUSH_TIMEOUT instead).
I won't be doing development for a few weeks over the holidays, so I can not
update this patch for a while.
Nick
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 packets so
> > > >> >> >> that if the ACL packets block for some reason then the LM can flush
> > > >> >> >> them to make room for newer packets.
> > > >> >> >>
> > > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> > > >> >> >> default (non-flushable packet), and let userspace request flushable
> > > >> >> >> packets on A2DP L2CAP sockets with the socket option
> > > >> >> >> L2CAP_LM_RELIABLE.
> > > >> >> >
> > > >> >> > the reliable option has a different meaning. It comes back from the old
> > > >> >> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
> > > >> > 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 flush
> > > >> > might be needed.
> > > >> >
> > > >> >> struct l2cap_pinfo {
> > > >> >> ...
> > > >> >> __u8 flushable;
> > > >> >> }
> > > >> >
> > > >> > Sure. In the long run we need to turn this into a bitmask. We are just
> > > >> > 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.
> > >
> > > Was this proposal ok?
> >
> > Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> >
> > > >> 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.
> > > >
> > > > can I stop you right here (without even looking at the patch). We do
> > > > have the generic SOL_BLUETOOTH that you should be using. So adding
> > > > SOL_ACL is not a viable option at all.
> > >
> > > This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOUT
> > > is fine too, or whatever you prefer.
> >
> > Why not just use BT_FLUSHABLE and have it always take a timeout option
> > and then 0 means not flushable. And advantage of having it separated?
>
> I think keeping them separate makes it clear that the flush timeout is
> global for a given ACL link, whereas the flushable/non-flushable
> boolean is specific to a L2CAP channel. (Which is why I suggested
> introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT option -
> since this option applies at the ACL level in the stack).
>
> A specific advantage of this is that flushable packets can be enabled
> without over-writing a previous flush timeout that was set on a
> different L2CAP socket on the same ACL link. I guess this can also be
> achieved with getsockopt() but that is racy.
I am talking here about Enhanced Flush support and that would happen on
a per ACL handle basis. So it actually almost applies on a per L2CAP
socket level. Only exception is if you establish two or more L2CAP
connections to the same remote device and set them all to flushable.
Then of course all of them will be flushed. So strictly speaking it
might be an ACL link feature, but we don't wanna use it that way. And in
practice you won't have multiple concurrent flushable L2CAP connections
to one remote device anyway.
Regards
Marcel
On Fri, Dec 18, 2009 at 3:05 PM, Marcel Holtmann <[email protected]> wrot=
e:
>
> Hi Nick,
>
> > >> >> >> Right now Bluez always requests flushable ACL packets (but doe=
s not
> > >> >> >> set a flush timeout, so effectively they are non-flushable):
> > >> >> >>
> > >> >> >> However it is desirable to use an ACL flush timeout on A2DP pa=
ckets so
> > >> >> >> that if the ACL packets block for some reason then the LM can =
flush
> > >> >> >> them to make room for newer packets.
> > >> >> >>
> > >> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary=
flag by
> > >> >> >> default (non-flushable packet), and let userspace request flus=
hable
> > >> >> >> packets on A2DP L2CAP sockets with the socket option
> > >> >> >> L2CAP_LM_RELIABLE.
> > >> >> >
> > >> >> > the reliable option has a different meaning. It comes back from=
the old
> > >> >> > Bluetooth 1.1 qualification days where we had to tests on L2CAP=
that 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 =
that
> > >> > we might wanna deprecate this socket options ;)
> > >> >
> > >> > I need to read up on the flushable stuff, but in the end it deserv=
es its
> > >> > own socket option. Also an ioctl() to actually trigger Enhanced fl=
ush
> > >> > 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 =
just
> > >> > 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.
> >
> > Was this proposal ok?
>
> Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
>
> > >> 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.
> > >
> > > can I stop you right here (without even looking at the patch). We do
> > > have the generic SOL_BLUETOOTH that you should be using. So adding
> > > SOL_ACL is not a viable option at all.
> >
> > This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOUT
> > is fine too, or whatever you prefer.
>
> Why not just use BT_FLUSHABLE and have it always take a timeout option
> and then 0 means not flushable. And advantage of having it separated?
I think keeping them=A0separate=A0makes it clear that the flush timeout is
global for a given ACL link, whereas the flushable/non-flushable
boolean is specific to a L2CAP channel. (Which is why I suggested
introducing a new level SOL_ACL for the ACL_FLUSH_TIMEOUT option -
since this option applies at the ACL level in the stack).
A specific advantage of this is that flushable packets can be enabled
without over-writing a previous flush timeout that was set on a
different L2CAP socket on the same ACL link. I guess this can also be
achieved with getsockopt() but that is racy.
> Regards
>
> Marcel
>
>
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 packets so
> >> >> >> that if the ACL packets block for some reason then the LM can flush
> >> >> >> them to make room for newer packets.
> >> >> >>
> >> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> >> >> >> default (non-flushable packet), and let userspace request flushable
> >> >> >> packets on A2DP L2CAP sockets with the socket option
> >> >> >> L2CAP_LM_RELIABLE.
> >> >> >
> >> >> > the reliable option has a different meaning. It comes back from the old
> >> >> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
> >> > 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 flush
> >> > might be needed.
> >> >
> >> >> struct l2cap_pinfo {
> >> >> ...
> >> >> __u8 flushable;
> >> >> }
> >> >
> >> > Sure. In the long run we need to turn this into a bitmask. We are just
> >> > 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.
>
> Was this proposal ok?
Even SOL_L2CAP goes away. Use SOL_BLUETOOTH for this.
> >> 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.
> >
> > can I stop you right here (without even looking at the patch). We do
> > have the generic SOL_BLUETOOTH that you should be using. So adding
> > SOL_ACL is not a viable option at all.
>
> This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOUT
> is fine too, or whatever you prefer.
Why not just use BT_FLUSHABLE and have it always take a timeout option
and then 0 means not flushable. And advantage of having it separated?
Regards
Marcel
On Wed, Dec 16, 2009 at 3:36 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Nick,
>
>> >> >> Right now Bluez always requests flushable ACL packets (but does no=
t
>> >> >> set a flush timeout, so effectively they are non-flushable):
>> >> >>
>> >> >> However it is desirable to use an ACL flush timeout on A2DP packet=
s so
>> >> >> that if the ACL packets block for some reason then the LM can flus=
h
>> >> >> them to make room for newer packets.
>> >> >>
>> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary fla=
g by
>> >> >> default (non-flushable packet), and let userspace request flushabl=
e
>> >> >> packets on A2DP L2CAP sockets with the socket option
>> >> >> L2CAP_LM_RELIABLE.
>> >> >
>> >> > the reliable option has a different meaning. It comes back from the=
old
>> >> > Bluetooth 1.1 qualification days where we had to tests on L2CAP tha=
t had
>> >> > to confirm that we can detect malformed packets and report them. Th=
ese
>> >> > 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 that
>> > we might wanna deprecate this socket options ;)
>> >
>> > I need to read up on the flushable stuff, but in the end it deserves i=
ts
>> > own socket option. Also an ioctl() to actually trigger Enhanced flush
>> > 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 just
>> > 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.
Was this proposal ok?
>> 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.
>
> can I stop you right here (without even looking at the patch). We do
> have the generic SOL_BLUETOOTH that you should be using. So adding
> SOL_ACL is not a viable option at all.
This would be in a later patch, and SOL_BLUETOOTH, ACL_FLUSH_TIMEOUT
is fine too, or whatever you prefer.
Nick
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 packets so
> >> >> that if the ACL packets block for some reason then the LM can flush
> >> >> them to make room for newer packets.
> >> >>
> >> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> >> >> default (non-flushable packet), and let userspace request flushable
> >> >> packets on A2DP L2CAP sockets with the socket option
> >> >> L2CAP_LM_RELIABLE.
> >> >
> >> > the reliable option has a different meaning. It comes back from the old
> >> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
> > 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 flush
> > might be needed.
> >
> >> struct l2cap_pinfo {
> >> ...
> >> __u8 flushable;
> >> }
> >
> > Sure. In the long run we need to turn this into a bitmask. We are just
> > 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.
can I stop you right here (without even looking at the patch). We do
have the generic SOL_BLUETOOTH that you should be using. So adding
SOL_ACL is not a viable option at all.
Remember that it would also be nice if this works with RFCOMM sockets.
Even if the usefulness is limited for these sockets.
Regards
Marcel
Hi Marcel,
On Thu, Dec 10, 2009 at 2:03 PM, Marcel Holtmann <[email protected]> wrote:
> 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 packets so
>> >> that if the ACL packets block for some reason then the LM can flush
>> >> them to make room for newer packets.
>> >>
>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>> >> default (non-flushable packet), and let userspace request flushable
>> >> packets on A2DP L2CAP sockets with the socket option
>> >> L2CAP_LM_RELIABLE.
>> >
>> > the reliable option has a different meaning. It comes back from the old
>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
> 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 flush
> might be needed.
>
>> struct l2cap_pinfo {
>> ? ?...
>> ? ?__u8 flushable;
>> }
>
> Sure. In the long run we need to turn this into a bitmask. We are just
> 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.
Nick
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 packets so
> >> that if the ACL packets block for some reason then the LM can flush
> >> them to make room for newer packets.
> >>
> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> >> default (non-flushable packet), and let userspace request flushable
> >> packets on A2DP L2CAP sockets with the socket option
> >> L2CAP_LM_RELIABLE.
> >
> > the reliable option has a different meaning. It comes back from the old
> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
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 flush
might be needed.
> struct l2cap_pinfo {
> ...
> __u8 flushable;
> }
Sure. In the long run we need to turn this into a bitmask. We are just
wasting memory here.
Regards
Marcel
Hi Marcel,
On Tue, Dec 8, 2009 at 9:06 PM, Marcel Holtmann <[email protected]> wrote:
> 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 packets so
>> that if the ACL packets block for some reason then the LM can flush
>> them to make room for newer packets.
>>
>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>> default (non-flushable packet), and let userspace request flushable
>> packets on A2DP L2CAP sockets with the socket option
>> L2CAP_LM_RELIABLE.
>
> the reliable option has a different meaning. It comes back from the old
> Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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
struct l2cap_pinfo {
...
__u8 flushable;
}
> You can not just blindly use ACL_START_FLUSHABLE without proper version
> checking. This feature has been introduced later and will not work with
> all stacks out there. Please go through the different spec. versions to
> see the difference.
Will do.
Nick
On Tue, Dec 8, 2009 at 9:06 PM, Marcel Holtmann <[email protected]> wrote:
> 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 packets so
> > that if the ACL packets block for some reason then the LM can flush
> > them to make room for newer packets.
> >
> > Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> > default (non-flushable packet), and let userspace request flushable
> > packets on A2DP L2CAP sockets with the socket option
> > L2CAP_LM_RELIABLE.
>
> the reliable option has a different meaning. It comes back from the old
> Bluetooth 1.1 qualification days where we had to tests on L2CAP that had
> to confirm that we can detect malformed packets and report them. These
> days it is just fine to drop them.
>
Ok, how about introducing
#define L2CAP_LM_FLUSHABLE 0x0040
struct l2cap_pinfo {
...
__u8 flushable;
}
> You can not just blindly use ACL_START_FLUSHABLE without proper version
> checking. This feature has been introduced later and will not work with
> all stacks out there. Please go through the different spec. versions to
> see the difference.
>
Will do.
Nick
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 packets so
> that if the ACL packets block for some reason then the LM can flush
> them to make room for newer packets.
>
> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> default (non-flushable packet), and let userspace request flushable
> packets on A2DP L2CAP sockets with the socket option
> L2CAP_LM_RELIABLE.
the reliable option has a different meaning. It comes back from the old
Bluetooth 1.1 qualification days where we had to tests on L2CAP that had
to confirm that we can detect malformed packets and report them. These
days it is just fine to drop them.
You can not just blindly use ACL_START_FLUSHABLE without proper version
checking. This feature has been introduced later and will not work with
all stacks out there. Please go through the different spec. versions to
see the difference.
Regards
Marcel
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 packets so
> >>> >> that if the ACL packets block for some reason then the LM can flush
> >>> >> them to make room for newer packets.
> >>> >>
> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
> >>> >> default (non-flushable packet), and let userspace request flushable
> >>> >> packets on A2DP L2CAP sockets with the socket option
> >>> >> L2CAP_LM_RELIABLE.
> >>> >
> >>> > the reliable option has a different meaning. It comes back from the old
> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
> >> 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 flush
> >> might be needed.
> >>
> >>> struct l2cap_pinfo {
> >>> ...
> >>> __u8 flushable;
> >>> }
> >>
> >> Sure. In the long run we need to turn this into a bitmask. We are just
> >> 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
Marcel
On Wed, Dec 16, 2009 at 1:59 PM, Nick Pelly <[email protected]> wrote:
> Hi Marcel,
>
> On Thu, Dec 10, 2009 at 2:03 PM, Marcel Holtmann <[email protected]> wrote:
>> 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 packets so
>>> >> that if the ACL packets block for some reason then the LM can flush
>>> >> them to make room for newer packets.
>>> >>
>>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>>> >> default (non-flushable packet), and let userspace request flushable
>>> >> packets on A2DP L2CAP sockets with the socket option
>>> >> L2CAP_LM_RELIABLE.
>>> >
>>> > the reliable option has a different meaning. It comes back from the old
>>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
>> 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 flush
>> might be needed.
>>
>>> struct l2cap_pinfo {
>>> ? ?...
>>> ? ?__u8 flushable;
>>> }
>>
>> Sure. In the long run we need to turn this into a bitmask. We are just
>> 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.
Nick
Hi,
On 6/16/2010 9:56 PM, Nick Pelly wrote:
> On Wed, Jun 16, 2010 at 8:14 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi,
>>
>> On Wed, Jun 16, 2010 at 3:04 PM, Suraj<[email protected]> wrote:
>>> Hi Luis,
>>>
>>> On 6/16/2010 5:10 PM, Luiz Augusto von Dentz wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]>
>>>> wrote:
>>>>>
>>>>> 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 packets
>>>>>>>>>>> so
>>>>>>>>>>> that if the ACL packets block for some reason then the LM can flush
>>>>>>>>>>> them to make room for newer packets.
>>>>>>>>>>>
>>>>>>>>>>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag
>>>>>>>>>>> by
>>>>>>>>>>> default (non-flushable packet), and let userspace request flushable
>>>>>>>>>>> packets on A2DP L2CAP sockets with the socket option
>>>>>>>>>>> L2CAP_LM_RELIABLE.
>>>>>>>>>>
>>>>>>>>>> the reliable option has a different meaning. It comes back from the
>>>>>>>>>> old
>>>>>>>>>> Bluetooth 1.1 qualification days where we had to tests on L2CAP that
>>>>>>>>>> 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 that
>>>>>>>> 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 flush
>>>>>>>> might be needed.
>>>>>>>>
>>>>>>>>> struct l2cap_pinfo {
>>>>>>>>> ...
>>>>>>>>> __u8 flushable;
>>>>>>>>> }
>>>>>>>>
>>>>>>>> Sure. In the long run we need to turn this into a bitmask. We are just
>>>>>>>> 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
>>>> updates?
>>>>
>>> This is not exactly something related to your question, but there is another
>>> side effect for the current implementation.
>>>
>>> Assume you have 2 ACL links, FTP and A2DP. A2DP streaming and FTP doing FTP
>>> Put.
>>> When the A2DP packets start blocking, it effectively start blocking the
>>> packets available for FTP too. But, the host has no idea about it and keep
>>> pumping in A2DP data until all available buffers are blocked. Effectively
>>> blocking both A2DP and FTP.
>>>
>>> So at the user level you will see your FTP connection stalling as long your
>>> A2DP connection is stalled (out of range). FTP will restart as soon as A2DP
>>> comes back in range.
>>>
>>> I had raised this issue sometime before, but could not follow it up.
>>
>> I got the impression that we can still control which packets are
>> Automatically-Flushable and which are not, so even thought we set the
>> timeout in a per ACL link fashion we can still mark which packets
>> should be flushable in a per socket basis.
>>
>> Is that correct, Nick?
>
> Yes, as Suraj explains, my patch will solve the problem of a stalled
> A2DP link preventing FTP packets on another ACL - because the A2DP
> packets will be marked flushable.
>
> However its worth bringing up that my patch does not solve the reverse
> problem - if an FTP link is stalled then all ACL packets will be
> temporarily stalled - because you would not normally mark FTP packets
> as flushable. For this you would need the kernel to set a high level
> watermark on a per-ACL-link or per-socket basis, so that a single ACL
> link or a single socket can not use all available ACL buffers
> regardless of whether they are flushable or not.
>
> Nick
Exactly, this was the problem I raised sometime before.
Attached is a patch that works for me.
Not sure if this is the best solution. I can send a formal patch if this
is worth a try.
Regards
Suraj
On Wed, Jun 16, 2010 at 8:14 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Wed, Jun 16, 2010 at 3:04 PM, Suraj <[email protected]> wrote:
>> Hi Luis,
>>
>> On 6/16/2010 5:10 PM, Luiz Augusto von Dentz wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]>
>>> =A0wrote:
>>>>
>>>> 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 t=
hat
>>>>>>> we might wanna deprecate this socket options ;)
>>>>>>>
>>>>>>> I need to read up on the flushable stuff, but in the end it deserve=
s
>>>>>>> its
>>>>>>> own socket option. Also an ioctl() to actually trigger Enhanced flu=
sh
>>>>>>> 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 j=
ust
>>>>>>> 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 t=
o
>>>> 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
>>> updates?
>>>
>> This is not exactly something related to your question, but there is ano=
ther
>> side effect for the current implementation.
>>
>> Assume you have 2 ACL links, FTP and A2DP. A2DP streaming and FTP doing =
FTP
>> Put.
>> When the A2DP packets start blocking, it effectively start blocking the
>> packets available for FTP too. But, the host has no idea about it and ke=
ep
>> pumping in A2DP data until all available buffers are blocked. Effectivel=
y
>> blocking both A2DP and FTP.
>>
>> So at the user level you will see your FTP connection stalling as long y=
our
>> A2DP connection is stalled (out of range). FTP will restart as soon as A=
2DP
>> comes back in range.
>>
>> I had raised this issue sometime before, but could not follow it up.
>
> I got the impression that we can still control which packets are
> Automatically-Flushable and which are not, so even thought we set the
> timeout in a per ACL link fashion we can still mark which packets
> should be flushable in a per socket basis.
>
> Is that correct, Nick?
Yes, as Suraj explains, my patch will solve the problem of a stalled
A2DP link preventing FTP packets on another ACL - because the A2DP
packets will be marked flushable.
However its worth bringing up that my patch does not solve the reverse
problem - if an FTP link is stalled then all ACL packets will be
temporarily stalled - because you would not normally mark FTP packets
as flushable. For this you would need the kernel to set a high level
watermark on a per-ACL-link or per-socket basis, so that a single ACL
link or a single socket can not use all available ACL buffers
regardless of whether they are flushable or not.
Nick
Hi,
On 6/16/2010 8:44 PM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Wed, Jun 16, 2010 at 3:04 PM, Suraj<[email protected]> wrote:
>> Hi Luis,
>>
>> On 6/16/2010 5:10 PM, Luiz Augusto von Dentz wrote:
>>>
>>> Hi,
>>>
>>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]>
>>> wrote:
>>>>
>>>> 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 packets
>>>>>>>>>> so
>>>>>>>>>> that if the ACL packets block for some reason then the LM can flush
>>>>>>>>>> them to make room for newer packets.
>>>>>>>>>>
>>>>>>>>>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag
>>>>>>>>>> by
>>>>>>>>>> default (non-flushable packet), and let userspace request flushable
>>>>>>>>>> packets on A2DP L2CAP sockets with the socket option
>>>>>>>>>> L2CAP_LM_RELIABLE.
>>>>>>>>>
>>>>>>>>> the reliable option has a different meaning. It comes back from the
>>>>>>>>> old
>>>>>>>>> Bluetooth 1.1 qualification days where we had to tests on L2CAP that
>>>>>>>>> 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 that
>>>>>>> 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 flush
>>>>>>> might be needed.
>>>>>>>
>>>>>>>> struct l2cap_pinfo {
>>>>>>>> ...
>>>>>>>> __u8 flushable;
>>>>>>>> }
>>>>>>>
>>>>>>> Sure. In the long run we need to turn this into a bitmask. We are just
>>>>>>> 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
>>> updates?
>>>
>> This is not exactly something related to your question, but there is another
>> side effect for the current implementation.
>>
>> Assume you have 2 ACL links, FTP and A2DP. A2DP streaming and FTP doing FTP
>> Put.
>> When the A2DP packets start blocking, it effectively start blocking the
>> packets available for FTP too. But, the host has no idea about it and keep
>> pumping in A2DP data until all available buffers are blocked. Effectively
>> blocking both A2DP and FTP.
>>
>> So at the user level you will see your FTP connection stalling as long your
>> A2DP connection is stalled (out of range). FTP will restart as soon as A2DP
>> comes back in range.
>>
>> I had raised this issue sometime before, but could not follow it up.
>
> I got the impression that we can still control which packets are
> Automatically-Flushable and which are not, so even thought we set the
> timeout in a per ACL link fashion we can still mark which packets
> should be flushable in a per socket basis.
>
> Is that correct, Nick?
>
Yes, it is possible to flush packets selectively. But the buffer
management is between Host and Controller, not per ACL link.
The root cause of the issue is that A2DP packets are not at all flushed
and keeps clogging the available buffers.
So, Implementing option to actually flush A2DP packet should solve the
above mentioned problem.
Regards
Suraj
Hi,
On Wed, Jun 16, 2010 at 3:04 PM, Suraj <[email protected]> wrote:
> Hi Luis,
>
> On 6/16/2010 5:10 PM, Luiz Augusto von Dentz wrote:
>>
>> Hi,
>>
>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]>
>> =A0wrote:
>>>
>>> Hi Nick,
>>>
>>>>>>>>> Right now Bluez always requests flushable ACL packets (but does n=
ot
>>>>>>>>> set a flush timeout, so effectively they are non-flushable):
>>>>>>>>>
>>>>>>>>> However it is desirable to use an ACL flush timeout on A2DP packe=
ts
>>>>>>>>> so
>>>>>>>>> that if the ACL packets block for some reason then the LM can flu=
sh
>>>>>>>>> them to make room for newer packets.
>>>>>>>>>
>>>>>>>>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary fl=
ag
>>>>>>>>> by
>>>>>>>>> default (non-flushable packet), and let userspace request flushab=
le
>>>>>>>>> packets on A2DP L2CAP sockets with the socket option
>>>>>>>>> L2CAP_LM_RELIABLE.
>>>>>>>>
>>>>>>>> the reliable option has a different meaning. It comes back from th=
e
>>>>>>>> old
>>>>>>>> Bluetooth 1.1 qualification days where we had to tests on L2CAP th=
at
>>>>>>>> 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
>> updates?
>>
> This is not exactly something related to your question, but there is anot=
her
> side effect for the current implementation.
>
> Assume you have 2 ACL links, FTP and A2DP. A2DP streaming and FTP doing F=
TP
> Put.
> When the A2DP packets start blocking, it effectively start blocking the
> packets available for FTP too. But, the host has no idea about it and kee=
p
> pumping in A2DP data until all available buffers are blocked. Effectively
> blocking both A2DP and FTP.
>
> So at the user level you will see your FTP connection stalling as long yo=
ur
> A2DP connection is stalled (out of range). FTP will restart as soon as A2=
DP
> comes back in range.
>
> I had raised this issue sometime before, but could not follow it up.
I got the impression that we can still control which packets are
Automatically-Flushable and which are not, so even thought we set the
timeout in a per ACL link fashion we can still mark which packets
should be flushable in a per socket basis.
Is that correct, Nick?
--=20
Luiz Augusto von Dentz
Computer Engineer
On Wed, Jun 16, 2010 at 4:40 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi,
>
> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann <[email protected]> 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
Hi Luis,
On 6/16/2010 5:10 PM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]> wrote:
>> 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 packets so
>>>>>>>> that if the ACL packets block for some reason then the LM can flush
>>>>>>>> them to make room for newer packets.
>>>>>>>>
>>>>>>>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>>>>>>>> default (non-flushable packet), and let userspace request flushable
>>>>>>>> packets on A2DP L2CAP sockets with the socket option
>>>>>>>> L2CAP_LM_RELIABLE.
>>>>>>>
>>>>>>> the reliable option has a different meaning. It comes back from the old
>>>>>>> Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
>>>>> 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 flush
>>>>> might be needed.
>>>>>
>>>>>> struct l2cap_pinfo {
>>>>>> ...
>>>>>> __u8 flushable;
>>>>>> }
>>>>>
>>>>> Sure. In the long run we need to turn this into a bitmask. We are just
>>>>> 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 updates?
>
This is not exactly something related to your question, but there is
another side effect for the current implementation.
Assume you have 2 ACL links, FTP and A2DP. A2DP streaming and FTP doing
FTP Put.
When the A2DP packets start blocking, it effectively start blocking the
packets available for FTP too. But, the host has no idea about it and
keep pumping in A2DP data until all available buffers are blocked.
Effectively blocking both A2DP and FTP.
So at the user level you will see your FTP connection stalling as long
your A2DP connection is stalled (out of range). FTP will restart as soon
as A2DP comes back in range.
I had raised this issue sometime before, but could not follow it up.
Regards
Suraj
Hi,
On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Nick,
>
>> >>> >> Right now Bluez always requests flushable ACL packets (but does n=
ot
>> >>> >> set a flush timeout, so effectively they are non-flushable):
>> >>> >>
>> >>> >> However it is desirable to use an ACL flush timeout on A2DP packe=
ts so
>> >>> >> that if the ACL packets block for some reason then the LM can flu=
sh
>> >>> >> them to make room for newer packets.
>> >>> >>
>> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary fl=
ag by
>> >>> >> default (non-flushable packet), and let userspace request flushab=
le
>> >>> >> packets on A2DP L2CAP sockets with the socket option
>> >>> >> L2CAP_LM_RELIABLE.
>> >>> >
>> >>> > the reliable option has a different meaning. It comes back from th=
e old
>> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP th=
at had
>> >>> > to confirm that we can detect malformed packets and report them. T=
hese
>> >>> > 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 tha=
t
>> >> 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 flush
>> >> 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 jus=
t
>> >> 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 update=
s?
--=20
Luiz Augusto von Dentz
Computer Engineer
Hi,
On 12/9/2010 10:25 PM, Nick Pelly wrote:
> On Thu, Dec 9, 2010 at 2:37 AM, Andrei Emeltchenko
> <[email protected]> wrote:
>>
>> Hi All,
>>
>> On Wed, Jun 16, 2010 at 5:15 PM, Nick Pelly<[email protected]> wrote:
>>> On Wed, Jun 16, 2010 at 4:40 AM, Luiz Augusto von Dentz
>>> <[email protected]> wrote:
>>>> Hi,
>>>>
>>>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann<[email protected]> wrote:
>>>>> 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 packets so
>>>>>>>>>>> that if the ACL packets block for some reason then the LM can flush
>>>>>>>>>>> them to make room for newer packets.
>>>>>>>>>>>
>>>>>>>>>>> Is it reasonable for Bluez to use the 0x00 ACL packet boundary flag by
>>>>>>>>>>> default (non-flushable packet), and let userspace request flushable
>>>>>>>>>>> packets on A2DP L2CAP sockets with the socket option
>>>>>>>>>>> L2CAP_LM_RELIABLE.
>>>>>>>>>>
>>>>>>>>>> the reliable option has a different meaning. It comes back from the old
>>>>>>>>>> Bluetooth 1.1 qualification days where we had to tests on L2CAP that 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 that
>>>>>>>> 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 flush
>>>>>>>> might be needed.
>>>>>>>>
>>>>>>>>> struct l2cap_pinfo {
>>>>>>>>> ...
>>>>>>>>> __u8 flushable;
>>>>>>>>> }
>>>>>>>>
>>>>>>>> Sure. In the long run we need to turn this into a bitmask. We are just
>>>>>>>> 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.
>>
>> Nick are you going to take Marcel comments? Otherwise I could take
>> care about the patch as it seems that it might help in some
>> situations.
>
> I'm not actively working on this patch.
>
>>>>> 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.
>>
>> At least wireshark which I use understands those packets.
>>
>>>> I would like to play a little bit with this, so is there any missing updates?
>>>
>>> Nope, that is our most recent version.
>>
>> Nick, do you know headset which could help to hear the real
>> difference? I was trying to use Sony DR-BT22 headset which has some
>> issues with A2DP but the solution did not help much.
>
> It becomes essential in non-ideal radio bandwidth conditions such as
> single antenna wifi co-existence. We also had some headsets that
> exacerbated the problem (presumably they had less logic to 'catch-up'
> through late packets) but I can't remember off hand.
I think you should be able to reproduce the same issue using any A2DP
headset.
Use a CSR dongle as local controller and start streaming A2DP data.
Parallel to that initiate an FTP connection to another device.
Take the headset out of range, or keep it in a shield box.
You should be able to see that the FTP link also stalls as the A2DP
packets are stalled in controller without getting flushed.
>
> Nick
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards
Suraj
On Thu, Dec 9, 2010 at 2:37 AM, Andrei Emeltchenko
<[email protected]> wrote:
>
> Hi All,
>
> On Wed, Jun 16, 2010 at 5:15 PM, Nick Pelly <[email protected]> wrote:
> > On Wed, Jun 16, 2010 at 4:40 AM, Luiz Augusto von Dentz
> > <[email protected]> wrote:
> >> Hi,
> >>
> >> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann <[email protected]>=
wrote:
> >>> Hi Nick,
> >>>
> >>>> >>> >> Right now Bluez always requests flushable ACL packets (but do=
es not
> >>>> >>> >> set a flush timeout, so effectively they are non-flushable):
> >>>> >>> >>
> >>>> >>> >> However it is desirable to use an ACL flush timeout on A2DP p=
ackets so
> >>>> >>> >> that if the ACL packets block for some reason then the LM can=
flush
> >>>> >>> >> them to make room for newer packets.
> >>>> >>> >>
> >>>> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundar=
y flag by
> >>>> >>> >> default (non-flushable packet), and let userspace request flu=
shable
> >>>> >>> >> packets on A2DP L2CAP sockets with the socket option
> >>>> >>> >> L2CAP_LM_RELIABLE.
> >>>> >>> >
> >>>> >>> > the reliable option has a different meaning. It comes back fro=
m the old
> >>>> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CA=
P that had
> >>>> >>> > to confirm that we can detect malformed packets and report the=
m. 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=
that
> >>>> >> we might wanna deprecate this socket options ;)
> >>>> >>
> >>>> >> I need to read up on the flushable stuff, but in the end it deser=
ves its
> >>>> >> own socket option. Also an ioctl() to actually trigger Enhanced f=
lush
> >>>> >> 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=
just
> >>>> >> 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 wit=
h.
> >>>> > 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 AC=
L
> >>>> > 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 you=
r
> >>>> > 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.
>
> Nick are you going to take Marcel comments? Otherwise I could take
> care about the patch as it seems that it might help in some
> situations.
I'm not actively working on this patch.
> >>> 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 nic=
e
> >>> to include some really simple examples in the commit message.
>
> At least wireshark which I use understands those packets.
>
> >> I would like to play a little bit with this, so is there any missing u=
pdates?
> >
> > Nope, that is our most recent version.
>
> Nick, do you know headset which could help to hear the real
> difference? I was trying to use Sony DR-BT22 headset which has some
> issues with A2DP but the solution did not help much.
It becomes essential in non-ideal radio bandwidth conditions such as
single antenna wifi co-existence. We also had some headsets that
exacerbated the problem (presumably they had less logic to 'catch-up'
through late packets) but I can't remember off hand.
Nick
Hi All,
On Wed, Jun 16, 2010 at 5:15 PM, Nick Pelly <[email protected]> wrote:
> On Wed, Jun 16, 2010 at 4:40 AM, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi,
>>
>> On Tue, Mar 9, 2010 at 11:45 PM, Marcel Holtmann <[email protected]> w=
rote:
>>> 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 pac=
kets so
>>>> >>> >> that if the ACL packets block for some reason then the LM can f=
lush
>>>> >>> >> them to make room for newer packets.
>>>> >>> >>
>>>> >>> >> Is it reasonable for Bluez to use the 0x00 ACL packet boundary =
flag by
>>>> >>> >> default (non-flushable packet), and let userspace request flush=
able
>>>> >>> >> packets on A2DP L2CAP sockets with the socket option
>>>> >>> >> L2CAP_LM_RELIABLE.
>>>> >>> >
>>>> >>> > the reliable option has a different meaning. It comes back from =
the old
>>>> >>> > Bluetooth 1.1 qualification days where we had to tests on L2CAP =
that 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 t=
hat
>>>> >> we might wanna deprecate this socket options ;)
>>>> >>
>>>> >> I need to read up on the flushable stuff, but in the end it deserve=
s its
>>>> >> own socket option. Also an ioctl() to actually trigger Enhanced flu=
sh
>>>> >> 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 j=
ust
>>>> >> 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.
Nick are you going to take Marcel comments? Otherwise I could take
care about the patch as it seems that it might help in some
situations.
>>> 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.
At least wireshark which I use understands those packets.
>> I would like to play a little bit with this, so is there any missing upd=
ates?
>
> Nope, that is our most recent version.
Nick, do you know headset which could help to hear the real
difference? I was trying to use Sony DR-BT22 headset which has some
issues with A2DP but the solution did not help much.
Regards,
Andrei