Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Arend Van Spriel]

On Fri, 2013-02-22 at 13:32 +0000, Johannes Berg wrote:
> On Fri, 2013-02-22 at 13:32 +0000, Piotr Haber wrote:
> > > Apart from a few minor technical comments that I'll omit for now, I'm
> > > not sure what value this really has? EAPOL can already be "protected" by
> > > way of knowing when the station is marked authorized, and DHCP is pretty
> > > tricky because it could take "forever", might not be there at all, etc.
> >
> > By "protect" I meant give Wifi a priority over BT so these time sensitive things 
> > can finish quicker/on first try, limiting the possibility of dropping packets because of BT 
> > using the medium.
>
> I know :)
>
> > This is supposed to be temporary and time limited, so if say DHCP finishes in the window 
> > we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
> > treated as usual. 
>
> That's not even documented/implemented, the way I read the patch you'd
> have to set it back manually.

Actually, the timeout is in brcmfmac so it currently relies on common sense of the driver developers (they tend to use it from time to time though).

> > > What application would actually call this? I don't really see how it
> > > could be integrated like that.
> >
> > For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
> > via iw or some other utility able to send nl messages.
>
> See that's the thing, I don't really see the point for EAPOL: you could
> just as well start protecting when the association is done, and end it
> when the station is marked authorized. That will have protected any
> EAPOL (or other protocols for that matter) traffic.

Ah, did we miss some cfg80211 API call that can tell us a station is authorized.

> Similarly, you could give it a certain timeout to protect DHCP traffic.
> I guess the only thing that would seem necessary would be a notification
> of "DHCP done" that would allow you to drop the protection right away.

We discussed this and we could start protecting when we see a BOOTP message, but indeed the end is not that straightforward and would need a "DHCP done" notification. However, if we have that there is little overhead in having a "DHCP start" notification and I would prefer to avoid looking into the sk_buff to check the protocol. The knowledge of DHCP start and done is not a responsibility of the driver.

> > This feature is styled after Android one.
>
> I know, I'm (vaguely) familiar with that.
>
> > There a Wifi state machine tries to "protect" DHCP traffic.
>
> Is there any *reason* for it though? Would it ever call it after the
> connection is fully established?

That obviously depends on the DHCP lease time or a renewal request. So it could be called afterwards as well.

> To me this seems not very well thought out.

Have to admit that it is a bit uninspired to reuse. In Android bcmdhd it is actually done by driver private ioctl, which did seem like a worse idea. Considered doing it entirely in the driver, but decided that it could be beneficial for dhcp clients to use such an interface and (arguably) for supplicant as well.

We'll do some more thinking and see whether there will be a RFC v2 ;-)

Gr. AvS

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Fri, 2013-02-22 at 16:59 +0000, Arend Van Spriel wrote:

> > Similarly, you could give it a certain timeout to protect DHCP traffic.
> > I guess the only thing that would seem necessary would be a notification
> > of "DHCP done" that would allow you to drop the protection right away.
> 
> We discussed this and we could start protecting when we see a BOOTP
> message, but indeed the end is not that straightforward and would need
> a "DHCP done" notification. However, if we have that there is little
> overhead in having a "DHCP start" notification and I would prefer to
> avoid looking into the sk_buff to check the protocol. The knowledge of
> DHCP start and done is not a responsibility of the driver.

Yes, I agree that sniffing the traffic for this is not a good idea.

> > Is there any *reason* for it though? Would it ever call it after the
> > connection is fully established?
> 
> That obviously depends on the DHCP lease time or a renewal request. So
> it could be called afterwards as well.

But is it? You'd usually expect the DHCP renewal to happen before the
lease expires so then it wouldn't be quite that latency sensitive.

> > To me this seems not very well thought out.
> 
> Have to admit that it is a bit uninspired to reuse. In Android bcmdhd
> it is actually done by driver private ioctl, which did seem like a
> worse idea. Considered doing it entirely in the driver, but decided
> that it could be beneficial for dhcp clients to use such an interface
> and (arguably) for supplicant as well.

Yes, although actually even for DHCP you could probably go on things
like "do I have an address configured", maybe. Not really sure. Having
some sort of "DHCP bracketing" would seem quite a bit saner though than
what was proposed now.

johannes

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Adrian Chadd]

... why not tag such traffic with a higher QOS priority and then have
the BT coex module snoop thaT?



Adrian

RE: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Arend Van Spriel]

On Saturday, February 23, 2013 1:30 AM Adrian Chadd wrote:
>
> ... why not tag such traffic with a higher QOS priority and then have
> the BT coex module snoop thaT?
>

In the driver we could inspect each sk_buff and boost priority of any BOOTP packets so that it will end up in the AC_VO fifo and have hardware coexistence handle it further. I consider that more a pragmatic fix, which is not always the worst thing to go for. However, I would prefer a solution is which user-space, ie. dhcp client can control the priority and/or BT coexistence.

Gr. AvS

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Emmanuel Grumbach]

>>
>
> In the driver we could inspect each sk_buff and boost priority of any BOOTP packets so that it will end up in the AC_VO fifo and have hardware coexistence handle it further. I consider that more a pragmatic fix, which is not always the worst thing to go for. However, I would prefer a solution is which user-space, ie. dhcp client can control the priority and/or BT coexistence.
>

I don't think that the "session solution" as proposed in the RFC is
the right one. Because the DHCP could take forever and also because
there might be quite a few non-critical packets right after
connections. There might be a bunch of services that may wake up and
begin to send a whole bunch of things. OTOH, EAPOL and friends *need*
priority, so I guess that the per-packet solution is the right one.
Note that we could think about a flag in the tx_control in which cfg /
mac80211 would let the driver know that this packet is a "high
priority packet" or even better "a connection gating packet". This
list would include managements / EAPOL / DHCP / any other kind of
stuff that look like these. I remember I saw once in the code
something that looked like this but can't find it right now.

FWIW: here is the code that classifies the packets in iwlwifi:

	/* High prio packet (wrt. BT coex) if it is EAPOL, MCAST or MGMT */
	if (info->band == IEEE80211_BAND_2GHZ        &&
	    (skb->protocol == cpu_to_be16(ETH_P_PAE)  ||
	     is_multicast_ether_addr(hdr->addr1)      ||
	     ieee80211_is_back_req(fc)                ||
	     ieee80211_is_mgmt(fc)))
		tx_flags |= TX_CMD_FLG_BT_DIS;

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Sat, 2013-02-23 at 17:47 +0000, Arend Van Spriel wrote:
> On Saturday, February 23, 2013 1:30 AM Adrian Chadd wrote:
> >
> > ... why not tag such traffic with a higher QOS priority and then have
> > the BT coex module snoop thaT?
> >
> 
> In the driver we could inspect each sk_buff and boost priority of any
> BOOTP packets so that it will end up in the AC_VO fifo and have
> hardware coexistence handle it further. I consider that more a
> pragmatic fix, which is not always the worst thing to go for. 

Well, I don't really think that's the best idea. Sniffing the protocol
is clumsy at best.

> However, I would prefer a solution is which user-space, ie. dhcp
> client can control the priority and/or BT coexistence.

It's not just BT coex btw, with multi-channel support it might also be
interesting to not switch channels while DHCP is going on, for example.

johannes

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Adrian Chadd]

On 24 February 2013 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:

>> In the driver we could inspect each sk_buff and boost priority of any
>> BOOTP packets so that it will end up in the AC_VO fifo and have
>> hardware coexistence handle it further. I consider that more a
>> pragmatic fix, which is not always the worst thing to go for.
>
> Well, I don't really think that's the best idea. Sniffing the protocol
> is clumsy at best.

Sure, but it's the kind of thing that commercial devices do in order
to work around real world issues.

So it'd be nice to have a programmatic way to detect things (eg bootp
packets going out) and signal the driver via some side channel to do
things like btcoex weight changing.

But it'd also be nice to do it based on QoS too.

>> However, I would prefer a solution is which user-space, ie. dhcp
>> client can control the priority and/or BT coexistence.
>
> It's not just BT coex btw, with multi-channel support it might also be
> interesting to not switch channels while DHCP is going on, for example.

Right.



Adrian

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Felix Fietkau]

On 2013-02-25 6:08 AM, Adrian Chadd wrote:
> On 24 February 2013 09:28, Johannes Berg <johannes@sipsolutions.net> wrote:
> 
>>> In the driver we could inspect each sk_buff and boost priority of any
>>> BOOTP packets so that it will end up in the AC_VO fifo and have
>>> hardware coexistence handle it further. I consider that more a
>>> pragmatic fix, which is not always the worst thing to go for.
>>
>> Well, I don't really think that's the best idea. Sniffing the protocol
>> is clumsy at best.
> 
> Sure, but it's the kind of thing that commercial devices do in order
> to work around real world issues.
Just because commercial devices do this crap to weasel out of fixing
things properly doesn't mean we have to do the same.

> So it'd be nice to have a programmatic way to detect things (eg bootp
> packets going out) and signal the driver via some side channel to do
> things like btcoex weight changing.
I disagree, that approach clumsy and fragile and it's trying to address
the issue in the wrong place.

Most devices have some kind of connection manager that has a high-level
perspective of when it's fully connected (which includes DHCP/bootp).
Why not just let that connection manager set a sane maximum network
latency value via pm_qos network_latency and derive btcoex weight
changing and multi-channel settings from that?

- Felix

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Arend van Spriel]

On 02/25/13 06:54, Felix Fietkau wrote:
> On 2013-02-25 6:08 AM, Adrian Chadd wrote:
>> On 24 February 2013 09:28, Johannes Berg<johannes@sipsolutions.net>  wrote:
>>
>>>> In the driver we could inspect each sk_buff and boost priority of any
>>>> BOOTP packets so that it will end up in the AC_VO fifo and have
>>>> hardware coexistence handle it further. I consider that more a
>>>> pragmatic fix, which is not always the worst thing to go for.
>>>
>>> Well, I don't really think that's the best idea. Sniffing the protocol
>>> is clumsy at best.
>>
>> Sure, but it's the kind of thing that commercial devices do in order
>> to work around real world issues.
> Just because commercial devices do this crap to weasel out of fixing
> things properly doesn't mean we have to do the same.
>
>> So it'd be nice to have a programmatic way to detect things (eg bootp
>> packets going out) and signal the driver via some side channel to do
>> things like btcoex weight changing.
> I disagree, that approach clumsy and fragile and it's trying to address
> the issue in the wrong place.
>
> Most devices have some kind of connection manager that has a high-level
> perspective of when it's fully connected (which includes DHCP/bootp).

Hi Felix,

I agree with you here. I think the responsibility is indeed to be placed 
at a higher level.

> Why not just let that connection manager set a sane maximum network
> latency value via pm_qos network_latency and derive btcoex weight
> changing and multi-channel settings from that?

Now you are loosing me, but that can be helped. Could you educate me 
here a bit or some pointers to reading material? If there is a proper 
means already in place to communicate the information we might as well 
use it.

> - Felix
>

Regards,
Arend

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:

> Most devices have some kind of connection manager that has a high-level
> perspective of when it's fully connected (which includes DHCP/bootp).
> Why not just let that connection manager set a sane maximum network
> latency value via pm_qos network_latency and derive btcoex weight
> changing and multi-channel settings from that?

Frankly, I don't think that's going to work well. We tried using the
pm_qos framework once and nothing ever used it. Android isn't going to
change to it, so we'd be stuck with hacks like setting pm_qos in
wpa_supplicant which is just as awkward.

Also, what you mostly want isn't really so much a weight but rather a
time-based approach to give it high priority until the connection
handshake completes (we already do for auth/assoc/... until authorized)
so I think using the pm_qos framework to give priority wouldn't work
very well since there'd also be no way to tell when it was "done"

johannes

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Felix Fietkau]

On 2013-02-25 11:25 AM, Johannes Berg wrote:
> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
> 
>> Most devices have some kind of connection manager that has a high-level
>> perspective of when it's fully connected (which includes DHCP/bootp).
>> Why not just let that connection manager set a sane maximum network
>> latency value via pm_qos network_latency and derive btcoex weight
>> changing and multi-channel settings from that?
> 
> Frankly, I don't think that's going to work well. We tried using the
> pm_qos framework once and nothing ever used it. Android isn't going to
> change to it, so we'd be stuck with hacks like setting pm_qos in
> wpa_supplicant which is just as awkward.
If only the connection manager gets changed to use it, that would
already be enough. It doesn't have to be pushed into dhcp clients and
other applications.

> Also, what you mostly want isn't really so much a weight but rather a
> time-based approach to give it high priority until the connection
> handshake completes (we already do for auth/assoc/... until authorized)
> so I think using the pm_qos framework to give priority wouldn't work
> very well since there'd also be no way to tell when it was "done"
Just release the latency requirement in the connection manager once the
handshake is done. It knows...

- Felix

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Dan Williams]

On Mon, 2013-02-25 at 14:07 +0100, Felix Fietkau wrote:
> On 2013-02-25 11:25 AM, Johannes Berg wrote:
> > On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
> > 
> >> Most devices have some kind of connection manager that has a high-level
> >> perspective of when it's fully connected (which includes DHCP/bootp).
> >> Why not just let that connection manager set a sane maximum network
> >> latency value via pm_qos network_latency and derive btcoex weight
> >> changing and multi-channel settings from that?
> > 
> > Frankly, I don't think that's going to work well. We tried using the
> > pm_qos framework once and nothing ever used it. Android isn't going to
> > change to it, so we'd be stuck with hacks like setting pm_qos in
> > wpa_supplicant which is just as awkward.
> If only the connection manager gets changed to use it, that would
> already be enough. It doesn't have to be pushed into dhcp clients and
> other applications.
> 
> > Also, what you mostly want isn't really so much a weight but rather a
> > time-based approach to give it high priority until the connection
> > handshake completes (we already do for auth/assoc/... until authorized)
> > so I think using the pm_qos framework to give priority wouldn't work
> > very well since there'd also be no way to tell when it was "done"
> Just release the latency requirement in the connection manager once the
> handshake is done. It knows...

We also don't know what IP configuration method will get used; whether
it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static.  Only
the connection manager knows that.  Only the connection manager/DHCP
client know when they expect a lease renew operation to start too.
wpa_supplicant doesn't know any of these things either since it doesn't
do anything IP related.

I think the best approach here is to allow the higher layers to hint to
the driver that some operations that are about to start must be "more
reliable".  That includes EAPOL, DHCP, IP autoconfiguration, etc.  Then
when the higher layers know the operation is finished, they can indicate
the operations are done and the driver can go do whatever it wants.

The driver/stack may wish to do any of [set 1Mb rate, block rate
control, change BT coex, turn on microwave protection, whatever] and
that's great, the upper layers don't care about what the driver does,
just that the reliability of the operation is preserved.

Dan

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Arend van Spriel]

On 02/25/13 14:07, Felix Fietkau wrote:
> On 2013-02-25 11:25 AM, Johannes Berg wrote:
>> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
>>
>>> Most devices have some kind of connection manager that has a high-level
>>> perspective of when it's fully connected (which includes DHCP/bootp).
>>> Why not just let that connection manager set a sane maximum network
>>> latency value via pm_qos network_latency and derive btcoex weight
>>> changing and multi-channel settings from that?
>>
>> Frankly, I don't think that's going to work well. We tried using the
>> pm_qos framework once and nothing ever used it. Android isn't going to
>> change to it, so we'd be stuck with hacks like setting pm_qos in
>> wpa_supplicant which is just as awkward.
> If only the connection manager gets changed to use it, that would
> already be enough. It doesn't have to be pushed into dhcp clients and
> other applications.

Reading back some slides about pm_qos it (from Intel OSTC ;-) ) seems to 
be intended to make kernel drivers aware of performance requirements in 
other kernel parts or user-space. Sounds like a match here although it 
is a one-to-many notification framework. Also not exactly what we want 
(I think).

Gr. AvS

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Arend van Spriel]

On 02/27/13 11:27, Dan Williams wrote:
> On Mon, 2013-02-25 at 14:07 +0100, Felix Fietkau wrote:
>> On 2013-02-25 11:25 AM, Johannes Berg wrote:
>>> On Mon, 2013-02-25 at 06:54 +0100, Felix Fietkau wrote:
>>>
>>>> Most devices have some kind of connection manager that has a high-level
>>>> perspective of when it's fully connected (which includes DHCP/bootp).
>>>> Why not just let that connection manager set a sane maximum network
>>>> latency value via pm_qos network_latency and derive btcoex weight
>>>> changing and multi-channel settings from that?
>>>
>>> Frankly, I don't think that's going to work well. We tried using the
>>> pm_qos framework once and nothing ever used it. Android isn't going to
>>> change to it, so we'd be stuck with hacks like setting pm_qos in
>>> wpa_supplicant which is just as awkward.
>> If only the connection manager gets changed to use it, that would
>> already be enough. It doesn't have to be pushed into dhcp clients and
>> other applications.
>>
>>> Also, what you mostly want isn't really so much a weight but rather a
>>> time-based approach to give it high priority until the connection
>>> handshake completes (we already do for auth/assoc/... until authorized)
>>> so I think using the pm_qos framework to give priority wouldn't work
>>> very well since there'd also be no way to tell when it was "done"
>> Just release the latency requirement in the connection manager once the
>> handshake is done. It knows...
>
> We also don't know what IP configuration method will get used; whether
> it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static.  Only
> the connection manager knows that.  Only the connection manager/DHCP
> client know when they expect a lease renew operation to start too.
> wpa_supplicant doesn't know any of these things either since it doesn't
> do anything IP related.
>
> I think the best approach here is to allow the higher layers to hint to
> the driver that some operations that are about to start must be "more
> reliable".  That includes EAPOL, DHCP, IP autoconfiguration, etc.  Then
> when the higher layers know the operation is finished, they can indicate
> the operations are done and the driver can go do whatever it wants.

Indeed the RFC approach was explicit about the scope of this interface 
being BT coex or actually BT coex override. Johannes proposes one 
dedicated to DHCP as a similar interface in Android is used for that 
right now. Abstracting it to "more reliable" mainly avoids renaming it 
when someone comes up with a use-case other than BT coex or DHCP.

> The driver/stack may wish to do any of [set 1Mb rate, block rate
> control, change BT coex, turn on microwave protection, whatever] and
> that's great, the upper layers don't care about what the driver does,
> just that the reliability of the operation is preserved.

It is actually the reliability of the connection, but it may depend on 
what you mean by "operation" here. I think from user-space perpective 
this API is at most a notification to the driver, which *may* result in 
a more reliable protocol exchange in terms of reliability and/or latency.

Regards,
Arend

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Wed, 2013-02-27 at 11:27 +0100, Dan Williams wrote:
> 
> We also don't know what IP configuration method will get used; whether
> it will be IPv6 RA, DHCPv4 or DHCPv6, IPv4 autoconf, or static.  Only
> the connection manager knows that.  Only the connection manager/DHCP
> client know when they expect a lease renew operation to start too.
> wpa_supplicant doesn't know any of these things either since it doesn't
> do anything IP related.

Right, and if it's static you don't want any of this. I don't think
network latency stuff is a good method, even though on the face of it
the idea here is to reduce the initial connection latency.

> I think the best approach here is to allow the higher layers to hint to
> the driver that some operations that are about to start must be "more
> reliable".  That includes EAPOL, DHCP, IP autoconfiguration, etc.  Then
> when the higher layers know the operation is finished, they can indicate
> the operations are done and the driver can go do whatever it wants.

Yeah, I agree.

Note that it's less about being "more reliable" and more about being
"fast" (for some definition of that), in particular in the multi-channel
scenario you'd want to attempt to not switch away from the channel (if
possible) until DHCP finishes, or at least give it a chance to finish
quickly. But for example for static IP configuration you don't need to
and don't necessarily want to block being on that channel just in case
DHCP happens. Similarly with BT Coex of course, though there I guess
it's more about priority.

In any case it's about completing the connection to the network quickly
before being forced to go to powersave. In particular IPv6 might also
require multicast so doing powersave there really hurts (DTIM beacon.)

> The driver/stack may wish to do any of [set 1Mb rate, block rate
> control, change BT coex, turn on microwave protection, whatever] and
> that's great, the upper layers don't care about what the driver does,
> just that the reliability of the operation is preserved.

I'd say "reliability of the operation is _increased_" I guess :-) But
yeah.

johannes

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Piotr Haber]

On 02/27/13 18:44, Arend van Spriel wrote:
>> I think the best approach here is to allow the higher layers to hint to
>> the driver that some operations that are about to start must be "more
>> reliable".  That includes EAPOL, DHCP, IP autoconfiguration, etc.  Then
>> when the higher layers know the operation is finished, they can indicate
>> the operations are done and the driver can go do whatever it wants.
> 
> Indeed the RFC approach was explicit about the scope of this interface being BT coex or actually BT
> coex override. Johannes proposes one dedicated to DHCP as a similar interface in Android is used for
> that right now. Abstracting it to "more reliable" mainly avoids renaming it when someone comes up
> with a use-case other than BT coex or DHCP.
Definitely a more generic interface would be great as there are many driver independent things
that can be done to improve "reliability" (defer scanning, prevent band/channel switching).
But I  think it is a good idea to keep this "reliability boost" time constrained on cfg80211 level,
with userspace signaling that it is done resulting in return to default operation earlier.

Piotr

RE: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Piotr Haber]

> > This is supposed to be temporary and time limited, so if say DHCP finishes in the window
> > we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
> > treated as usual.

> That's not even documented/implemented, the way I read the patch you'd
> have to set it back manually.

You are right, we implement it this way in brcmfmac but it is not documented
in nl80211 change. I thought of DISABLED as a trigger (for timed boost)
and ENABLED as a way to notify that whoever requested it is done.
The interface could be made to reflect this "design"

> > > What application would actually call this? I don't really see how it
> > > could be integrated like that.

> > For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
> > via iw or some other utility able to send nl messages.

> See that's the thing, I don't really see the point for EAPOL: you could
> just as well start protecting when the association is done, and end it
> when the station is marked authorized. That will have protected any
> EAPOL (or other protocols for that matter) traffic.

That is true, this could be totally automatic.

> Similarly, you could give it a certain timeout to protect DHCP traffic.
> I guess the only thing that would seem necessary would be a notification
> of "DHCP done" that would allow you to drop the protection right away.

I guess that is the only reason we need this interface.
Still if we set the "protection" timeout to be reasonable it can do without
it...

> > This feature is styled after Android one.

> I know, I'm (vaguely) familiar with that.

> > There a Wifi state machine tries to "protect" DHCP traffic.

> Is there any *reason* for it though? Would it ever call it after the
> connection is fully established?

As for reason I can't tell, someone found one to implement it in Android :)
It is not used after connection is established.

Piotr 

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Fri, 2013-02-22 at 13:32 +0000, Piotr Haber wrote:

> > Apart from a few minor technical comments that I'll omit for now, I'm
> > not sure what value this really has? EAPOL can already be "protected" by
> > way of knowing when the station is marked authorized, and DHCP is pretty
> > tricky because it could take "forever", might not be there at all, etc.

> By "protect" I meant give Wifi a priority over BT so these time sensitive things 
> can finish quicker/on first try, limiting the possibility of dropping packets because of BT 
> using the medium.

I know :)

> This is supposed to be temporary and time limited, so if say DHCP finishes in the window 
> we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
> treated as usual. 

That's not even documented/implemented, the way I read the patch you'd
have to set it back manually.

> > What application would actually call this? I don't really see how it
> > could be integrated like that.

> For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
> via iw or some other utility able to send nl messages.

See that's the thing, I don't really see the point for EAPOL: you could
just as well start protecting when the association is done, and end it
when the station is marked authorized. That will have protected any
EAPOL (or other protocols for that matter) traffic.

Similarly, you could give it a certain timeout to protect DHCP traffic.
I guess the only thing that would seem necessary would be a notification
of "DHCP done" that would allow you to drop the protection right away.

> This feature is styled after Android one.

I know, I'm (vaguely) familiar with that.

> There a Wifi state machine tries to "protect" DHCP traffic.

Is there any *reason* for it though? Would it ever call it after the
connection is fully established?

To me this seems not very well thought out.

johannes

RE: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Piotr Haber]

>> From: Johannes Berg [johannes@sipsolutions.net]
>> Sent: Friday, February 22, 2013 12:52 PM
>> To: Piotr Haber
>> Cc: linux-wireless@vger.kernel.org
>> Subject: Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

>On Fri, 2013-02-22 at 10:08 +0100, Piotr Haber wrote:
>> Some devices share antenna/analog front end between Wifi
>> and Bluetooth. The hardware coexistence interface allows
>> them to do so, but there are situations when it would
>> be beneficial if software had a way to have influence on it
>> as well. It can be used to protect time sensitive
>> traffic in presence of Bluetooth voice stream,
>> for an example EAPOL handshake or DHCP negotiation.
>>
>> This patch adds new attribute to SET_WIPHY command
>> and a new field in struct wiphy to allow control of the
>> coexistence behavior. Devices that do not share resources
>> with Bluetooth can ignore this parameter.

> Apart from a few minor technical comments that I'll omit for now, I'm
> not sure what value this really has? EAPOL can already be "protected" by
> way of knowing when the station is marked authorized, and DHCP is pretty
> tricky because it could take "forever", might not be there at all, etc.
By "protect" I meant give Wifi a priority over BT so these time sensitive things 
can finish quicker/on first try, limiting the possibility of dropping packets because of BT 
using the medium.
This is supposed to be temporary and time limited, so if say DHCP finishes in the window 
we give it - great, if not the coexistence goes back to default behavior and Wifi traffic is
treated as usual. 

> What application would actually call this? I don't really see how it
> could be integrated like that.
For EAPOL wpa_supplicant might use it. For DHCP it could be used from enter/exit hooks
via iw or some other utility able to send nl messages.

This feature is styled after Android one.
There a Wifi state machine tries to "protect" DHCP traffic.
It uses extra wpa_supplicant command (DRIVER) with linked driver specific library.
Which means there can be only one wifi device and one needs to recompile wpa_supplicant 
to use another.
I wanted to make it more device independent.

Piotr

Re: [PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Johannes Berg]

On Fri, 2013-02-22 at 10:08 +0100, Piotr Haber wrote:
> Some devices share antenna/analog front end between Wifi
> and Bluetooth. The hardware coexistence interface allows
> them to do so, but there are situations when it would
> be beneficial if software had a way to have influence on it
> as well. It can be used to protect time sensitive
> traffic in presence of Bluetooth voice stream,
> for an example EAPOL handshake or DHCP negotiation.
> 
> This patch adds new attribute to SET_WIPHY command
> and a new field in struct wiphy to allow control of the
> coexistence behavior. Devices that do not share resources
> with Bluetooth can ignore this parameter.

Apart from a few minor technical comments that I'll omit for now, I'm
not sure what value this really has? EAPOL can already be "protected" by
way of knowing when the station is marked authorized, and DHCP is pretty
tricky because it could take "forever", might not be there at all, etc.

What application would actually call this? I don't really see how it
could be integrated like that.

johannes

[PATCH 1/2] [RFC] cfg80211: configuration of Bluetooth coexistence mode

[Piotr Haber]

Some devices share antenna/analog front end between Wifi
and Bluetooth. The hardware coexistence interface allows
them to do so, but there are situations when it would
be beneficial if software had a way to have influence on it
as well. It can be used to protect time sensitive
traffic in presence of Bluetooth voice stream,
for an example EAPOL handshake or DHCP negotiation.

This patch adds new attribute to SET_WIPHY command
and a new field in struct wiphy to allow control of the
coexistence behavior. Devices that do not share resources
with Bluetooth can ignore this parameter.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Hante Meuleman <meuleman@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Piotr Haber <phaber@broadcom.com>
---
 include/net/cfg80211.h       |    4 ++++
 include/uapi/linux/nl80211.h |   20 ++++++++++++++++++++
 net/wireless/nl80211.c       |   22 +++++++++++++++++++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 36e076e..3db21be 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1526,6 +1526,7 @@ struct cfg80211_connect_params {
  * @WIPHY_PARAM_FRAG_THRESHOLD: wiphy->frag_threshold has changed
  * @WIPHY_PARAM_RTS_THRESHOLD: wiphy->rts_threshold has changed
  * @WIPHY_PARAM_COVERAGE_CLASS: coverage class changed
+ * @WIPHY_PARAM_BTCOEX: Bluetooth coexistence mode changed
  */
 enum wiphy_params_flags {
 	WIPHY_PARAM_RETRY_SHORT		= 1 << 0,
@@ -1533,6 +1534,7 @@ enum wiphy_params_flags {
 	WIPHY_PARAM_FRAG_THRESHOLD	= 1 << 2,
 	WIPHY_PARAM_RTS_THRESHOLD	= 1 << 3,
 	WIPHY_PARAM_COVERAGE_CLASS	= 1 << 4,
+	WIPHY_PARAM_BTCOEX_MODE		= 1 << 5,
 };
 
 /*
@@ -2322,6 +2324,7 @@ struct wiphy_wowlan_support {
  * @max_sched_scan_ie_len: same as max_scan_ie_len, but for scheduled
  *	scans
  * @coverage_class: current coverage class
+ * @btcoex_mode: Wifi-Bluetooth coexistence mode
  * @fw_version: firmware version for ethtool reporting
  * @hw_version: hardware version for ethtool reporting
  * @max_num_pmkids: maximum number of PMKIDs supported by device
@@ -2401,6 +2404,7 @@ struct wiphy {
 	u32 frag_threshold;
 	u32 rts_threshold;
 	u8 coverage_class;
+	enum nl80211_btcoex_mode btcoex_mode;
 
 	char fw_version[ETHTOOL_BUSINFO_LEN];
 	u32 hw_version;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5b7dbc1..a96e1c9 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1336,6 +1336,9 @@ enum nl80211_commands {
  *	number of MAC addresses that a device can support for MAC
  *	ACL.
  *
+ * @NL80211_ATTR_WIPHY_BTCOEX_MODE: u8 attribute to control
+ *	Wifi - Bluetooth coexistence mode
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1614,6 +1617,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_MAC_ACL_MAX,
 
+	NL80211_ATTR_WIPHY_BTCOEX_MODE,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -3323,4 +3328,19 @@ enum nl80211_acl_policy {
 	NL80211_ACL_POLICY_DENY_UNLESS_LISTED,
 };
 
+/**
+ * enum nl80211_btcoex_mode - Bluetooth coexistence mode
+ *
+ * Control Wifi - Bluetooth coexistence mode,
+ * to be used with %NL80211_ATTR_WIPHY_BTCOEX_MODE.
+ *
+ * @NL80211_BTCOEX_ENABLED: enable coexistence
+ * @NL80211_BTCOEX_DISABLED: disable coexistence, give Wifi
+ *	traffic priority over Bluetooth
+ */
+enum nl80211_btcoex_mode {
+	NL80211_BTCOEX_ENABLED,
+	NL80211_BTCOEX_DISABLED,
+};
+
 #endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b5978ab..f8f99ef 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -367,6 +367,7 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_P2P_OPPPS] = { .type = NLA_U8 },
 	[NL80211_ATTR_ACL_POLICY] = {. type = NLA_U32 },
 	[NL80211_ATTR_MAC_ADDRS] = { .type = NLA_NESTED },
+	[NL80211_ATTR_WIPHY_BTCOEX_MODE] = { .type = NLA_U8},
 };
 
 /* policy for the key attributes */
@@ -914,7 +915,9 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 portid, u32 seq, int flag
 	    nla_put_u16(msg, NL80211_ATTR_MAX_SCHED_SCAN_IE_LEN,
 			dev->wiphy.max_sched_scan_ie_len) ||
 	    nla_put_u8(msg, NL80211_ATTR_MAX_MATCH_SETS,
-		       dev->wiphy.max_match_sets))
+		       dev->wiphy.max_match_sets) ||
+	    nla_put_u8(msg, NL80211_ATTR_WIPHY_BTCOEX_MODE,
+		       dev->wiphy.btcoex_mode))
 		goto nla_put_failure;
 
 	if ((dev->wiphy.flags & WIPHY_FLAG_IBSS_RSN) &&
@@ -1528,6 +1531,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 	u8 retry_short = 0, retry_long = 0;
 	u32 frag_threshold = 0, rts_threshold = 0;
 	u8 coverage_class = 0;
+	enum nl80211_btcoex_mode btcoex_mode = NL80211_BTCOEX_ENABLED;
 
 	/*
 	 * Try to find the wiphy and netdev. Normally this
@@ -1745,10 +1749,22 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 		changed |= WIPHY_PARAM_COVERAGE_CLASS;
 	}
 
+	if (info->attrs[NL80211_ATTR_WIPHY_BTCOEX_MODE]) {
+		btcoex_mode = nla_get_u8(
+			info->attrs[NL80211_ATTR_WIPHY_BTCOEX_MODE]);
+		if (btcoex_mode != NL80211_BTCOEX_ENABLED &&
+		    btcoex_mode != NL80211_BTCOEX_DISABLED) {
+			result = -EINVAL;
+			goto bad_res;
+		}
+		changed |= WIPHY_PARAM_BTCOEX_MODE;
+	}
+
 	if (changed) {
 		u8 old_retry_short, old_retry_long;
 		u32 old_frag_threshold, old_rts_threshold;
 		u8 old_coverage_class;
+		enum nl80211_btcoex_mode old_btcoex_mode;
 
 		if (!rdev->ops->set_wiphy_params) {
 			result = -EOPNOTSUPP;
@@ -1760,6 +1776,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 		old_frag_threshold = rdev->wiphy.frag_threshold;
 		old_rts_threshold = rdev->wiphy.rts_threshold;
 		old_coverage_class = rdev->wiphy.coverage_class;
+		old_btcoex_mode = rdev->wiphy.btcoex_mode;
 
 		if (changed & WIPHY_PARAM_RETRY_SHORT)
 			rdev->wiphy.retry_short = retry_short;
@@ -1771,6 +1788,8 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			rdev->wiphy.rts_threshold = rts_threshold;
 		if (changed & WIPHY_PARAM_COVERAGE_CLASS)
 			rdev->wiphy.coverage_class = coverage_class;
+		if (changed & WIPHY_PARAM_BTCOEX_MODE)
+			rdev->wiphy.btcoex_mode = btcoex_mode;
 
 		result = rdev_set_wiphy_params(rdev, changed);
 		if (result) {
@@ -1779,6 +1798,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
 			rdev->wiphy.frag_threshold = old_frag_threshold;
 			rdev->wiphy.rts_threshold = old_rts_threshold;
 			rdev->wiphy.coverage_class = old_coverage_class;
+			rdev->wiphy.btcoex_mode = old_btcoex_mode;
 		}
 	}
 
-- 
1.7.9.5