2017-06-14 22:12:47

by Ben Greear

[permalink] [raw]
Subject: Question on flushing and deauth frames

ath10k is a bit weird about flushing frames, and at least my variant of the wave-2
firmware does not always put the deauth frame on the air when deleting stations.

It will likely not be much fun to fix this. I was wondering if the code below
from ieee80211_set_disassoc() could (easily?) be made to wait for a tx-status of the deauth frame
instead of trying to force a flush?

/*
* drop any frame before deauth/disassoc, this can be data or
* management frame. Since we are disconnecting, we should not
* insist sending these frames which can take time and delay
* the disconnection and possible the roaming.
*/
if (tx)
ieee80211_flush_queues(local, sdata, true);

/* deauthenticate/disassociate now */
if (tx || frame_buf)
ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid, stype,
reason, tx, frame_buf);

/* flush out frame - make sure the deauth was actually sent */
if (tx)
ieee80211_flush_queues(local, sdata, false);

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


2017-06-15 16:34:06

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames

On Thu, Jun 15, 2017 at 4:32 PM, Ben Greear <[email protected]> wrote:
>
>
> On 06/14/2017 11:18 PM, Emmanuel Grumbach wrote:
>>>
>>> ath10k is a bit weird about flushing frames, and at least my variant of
>>> the wave-2
>>> firmware does not always put the deauth frame on the air when deleting
>>> stations.
>>>
>>> It will likely not be much fun to fix this. I was wondering if the code
>>> below
>>> from ieee80211_set_disassoc() could (easily?) be made to wait for a
>>> tx-status of the deauth frame
>>> instead of trying to force a flush?
>>
>>
>> That's exactly what it does:
>
>
> Well, the end effect is supposed to be the same, at least...
>
>>> /*
>>> * drop any frame before deauth/disassoc, this can be data or
>>> * management frame. Since we are disconnecting, we should not
>>> * insist sending these frames which can take time and delay
>>> * the disconnection and possible the roaming.
>>> */
>>> if (tx)
>>> ieee80211_flush_queues(local, sdata, true);
>>
>>
>> Remove all the pending data frames from the queues to clear the way
>> for the deauth.
>> Note the drop=true
>>
>>>
>>> /* deauthenticate/disassociate now */
>>> if (tx || frame_buf)
>>> ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid,
>>> stype,
>>> reason, tx, frame_buf);
>>>
>>
>> Send the deauth.
>>
>>> /* flush out frame - make sure the deauth was actually sent */
>>> if (tx)
>>> ieee80211_flush_queues(local, sdata, false);
>>>
>>
>> Wait for the deauth (drop=false).
>> The question here is how ath10k implements the flush(, drop=false)
>
>
> Not well, it seems. I don't think it has a good way to flush (drop=false)
> an individual vdev, though probably I could implement it. And, I think that
> the packets in the mac80211 txqs might be difficult to flush.
>
> But, mac80211 gets a txstatus when the deauth is sent, so if it simply
> waited on that, it would not matter how flush is implemented. And, it might
> try to re-send a time or two if getting the deauth through was worth the
> wait and the initial attempt(s) got a false txstatus.
>

Keep in mind that this code is also called when roaming, any
additional operations will slow down the roaming process. Not all the
drivers support tx_status, but I think that this could be implemented
internally in ath10k. In iwlwifi, we just wait until the queues get
empty without dropping the packets.
Of course, I am giving my *non authoritative* opinion :)

2017-06-15 16:47:28

by Ben Greear

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames

On 06/15/2017 09:34 AM, Emmanuel Grumbach wrote:
> On Thu, Jun 15, 2017 at 4:32 PM, Ben Greear <[email protected]> wrote:
>>
>>
>> On 06/14/2017 11:18 PM, Emmanuel Grumbach wrote:
>>>>
>>>> ath10k is a bit weird about flushing frames, and at least my variant of
>>>> the wave-2
>>>> firmware does not always put the deauth frame on the air when deleting
>>>> stations.
>>>>
>>>> It will likely not be much fun to fix this. I was wondering if the code
>>>> below
>>>> from ieee80211_set_disassoc() could (easily?) be made to wait for a
>>>> tx-status of the deauth frame
>>>> instead of trying to force a flush?
>>>
>>>
>>> That's exactly what it does:
>>
>>
>> Well, the end effect is supposed to be the same, at least...
>>
>>>> /*
>>>> * drop any frame before deauth/disassoc, this can be data or
>>>> * management frame. Since we are disconnecting, we should not
>>>> * insist sending these frames which can take time and delay
>>>> * the disconnection and possible the roaming.
>>>> */
>>>> if (tx)
>>>> ieee80211_flush_queues(local, sdata, true);
>>>
>>>
>>> Remove all the pending data frames from the queues to clear the way
>>> for the deauth.
>>> Note the drop=true
>>>
>>>>
>>>> /* deauthenticate/disassociate now */
>>>> if (tx || frame_buf)
>>>> ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid,
>>>> stype,
>>>> reason, tx, frame_buf);
>>>>
>>>
>>> Send the deauth.
>>>
>>>> /* flush out frame - make sure the deauth was actually sent */
>>>> if (tx)
>>>> ieee80211_flush_queues(local, sdata, false);
>>>>
>>>
>>> Wait for the deauth (drop=false).
>>> The question here is how ath10k implements the flush(, drop=false)
>>
>>
>> Not well, it seems. I don't think it has a good way to flush (drop=false)
>> an individual vdev, though probably I could implement it. And, I think that
>> the packets in the mac80211 txqs might be difficult to flush.
>>
>> But, mac80211 gets a txstatus when the deauth is sent, so if it simply
>> waited on that, it would not matter how flush is implemented. And, it might
>> try to re-send a time or two if getting the deauth through was worth the
>> wait and the initial attempt(s) got a false txstatus.
>>
>
> Keep in mind that this code is also called when roaming, any
> additional operations will slow down the roaming process. Not all the

This is a reason to not retransmit, but logically the flush(drop=false) is
supposed to wait for the frame to be transmitted anyway, and at that point,
tx-status should be immediately available.

> drivers support tx_status, but I think that this could be implemented
> internally in ath10k. In iwlwifi, we just wait until the queues get
> empty without dropping the packets.

When you support virtual stations and a fat firmware, that can be tricky to implement.

From your description, are you waiting on a single vdev, or multiple when you flush?
If multiple, then waiting for tx status of just the deauth frame should be faster
on average than waiting for the flush since you don't have to flush the other
vdevs.

Whether drivers support a correct tx-status, I think all of them report *some* tx
status...that is how mac80211 knows it can clean up the frame? That would be enough
to know that the frame at least went through the tx logic and thus was flushed.

> Of course, I am giving my *non authoritative* opinion :)

I appreciate it none the less!

Thanks,
Ben


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2017-06-15 13:32:56

by Ben Greear

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames



On 06/14/2017 11:18 PM, Emmanuel Grumbach wrote:
>> ath10k is a bit weird about flushing frames, and at least my variant of the wave-2
>> firmware does not always put the deauth frame on the air when deleting stations.
>>
>> It will likely not be much fun to fix this. I was wondering if the code below
>> from ieee80211_set_disassoc() could (easily?) be made to wait for a tx-status of the deauth frame
>> instead of trying to force a flush?
>
> That's exactly what it does:

Well, the end effect is supposed to be the same, at least...

>> /*
>> * drop any frame before deauth/disassoc, this can be data or
>> * management frame. Since we are disconnecting, we should not
>> * insist sending these frames which can take time and delay
>> * the disconnection and possible the roaming.
>> */
>> if (tx)
>> ieee80211_flush_queues(local, sdata, true);
>
> Remove all the pending data frames from the queues to clear the way
> for the deauth.
> Note the drop=true
>
>>
>> /* deauthenticate/disassociate now */
>> if (tx || frame_buf)
>> ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid, stype,
>> reason, tx, frame_buf);
>>
>
> Send the deauth.
>
>> /* flush out frame - make sure the deauth was actually sent */
>> if (tx)
>> ieee80211_flush_queues(local, sdata, false);
>>
>
> Wait for the deauth (drop=false).
> The question here is how ath10k implements the flush(, drop=false)

Not well, it seems. I don't think it has a good way to flush (drop=false)
an individual vdev, though probably I could implement it. And, I think that
the packets in the mac80211 txqs might be difficult to flush.

But, mac80211 gets a txstatus when the deauth is sent, so if it simply
waited on that, it would not matter how flush is implemented. And, it might
try to re-send a time or two if getting the deauth through was worth the
wait and the initial attempt(s) got a false txstatus.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2017-06-15 06:18:53

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames

> ath10k is a bit weird about flushing frames, and at least my variant of the wave-2
> firmware does not always put the deauth frame on the air when deleting stations.
>
> It will likely not be much fun to fix this. I was wondering if the code below
> from ieee80211_set_disassoc() could (easily?) be made to wait for a tx-status of the deauth frame
> instead of trying to force a flush?

That's exactly what it does:

>
> /*
> * drop any frame before deauth/disassoc, this can be data or
> * management frame. Since we are disconnecting, we should not
> * insist sending these frames which can take time and delay
> * the disconnection and possible the roaming.
> */
> if (tx)
> ieee80211_flush_queues(local, sdata, true);

Remove all the pending data frames from the queues to clear the way
for the deauth.
Note the drop=true

>
> /* deauthenticate/disassociate now */
> if (tx || frame_buf)
> ieee80211_send_deauth_disassoc(sdata, ifmgd->bssid, stype,
> reason, tx, frame_buf);
>

Send the deauth.

> /* flush out frame - make sure the deauth was actually sent */
> if (tx)
> ieee80211_flush_queues(local, sdata, false);
>

Wait for the deauth (drop=false).
The question here is how ath10k implements the flush(, drop=false)

2017-06-15 17:40:11

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames

>>
>> Keep in mind that this code is also called when roaming, any
>> additional operations will slow down the roaming process. Not all the
>
>
> This is a reason to not retransmit, but logically the flush(drop=false) is
> supposed to wait for the frame to be transmitted anyway, and at that point,
> tx-status should be immediately available.
>

I can't disagree here although waiting for the Tx status can be tricky
for the low level drivers that don't implement tx_status.

>> drivers support tx_status, but I think that this could be implemented
>> internally in ath10k. In iwlwifi, we just wait until the queues get
>> empty without dropping the packets.
>
>
> When you support virtual stations and a fat firmware, that can be tricky to
> implement.
>
> From your description, are you waiting on a single vdev, or multiple when
> you flush?

In the past, we had queues per vdev. So that we had to wait for the
queues of the vdev. Note that Intel is more focused on the BSS
scenario in which vdev = station pretty much. I understand that you
are more focused on the AP scenario, but that shouldn't really delete
stations all that often, right?

> If multiple, then waiting for tx status of just the deauth frame should be
> faster
> on average than waiting for the flush since you don't have to flush the
> other
> vdevs.

True. Well, on Intel devices, we work with Tx queues, and now we have
a Tx queue for each ra/tid. Yes it limits the number of peers we can
support, but as I said, we are less AP mode oriented.

>
> Whether drivers support a correct tx-status, I think all of them report
> *some* tx
> status...that is how mac80211 knows it can clean up the frame? That would
> be enough
> to know that the frame at least went through the tx logic and thus was
> flushed.

It is an option. Maybe mac80211 could pass the pointer to the skb it
is waiting for in the flush(drop=false) call and that would make it
easier for you to wait for it internally in ath10k?

I'll let Johannes decide here :)

>
>> Of course, I am giving my *non authoritative* opinion :)
>
>
> I appreciate it none the less!
>
>
> Thanks,
> Ben
>
>
> --
> Ben Greear <[email protected]>
> Candela Technologies Inc http://www.candelatech.com
>

2017-06-15 17:49:44

by Ben Greear

[permalink] [raw]
Subject: Re: Question on flushing and deauth frames

On 06/15/2017 10:40 AM, Emmanuel Grumbach wrote:
>>>
>>> Keep in mind that this code is also called when roaming, any
>>> additional operations will slow down the roaming process. Not all the
>>
>>
>> This is a reason to not retransmit, but logically the flush(drop=false) is
>> supposed to wait for the frame to be transmitted anyway, and at that point,
>> tx-status should be immediately available.
>>
>
> I can't disagree here although waiting for the Tx status can be tricky
> for the low level drivers that don't implement tx_status.
>
>>> drivers support tx_status, but I think that this could be implemented
>>> internally in ath10k. In iwlwifi, we just wait until the queues get
>>> empty without dropping the packets.
>>
>>
>> When you support virtual stations and a fat firmware, that can be tricky to
>> implement.
>>
>> From your description, are you waiting on a single vdev, or multiple when
>> you flush?
>
> In the past, we had queues per vdev. So that we had to wait for the
> queues of the vdev. Note that Intel is more focused on the BSS
> scenario in which vdev = station pretty much. I understand that you
> are more focused on the AP scenario, but that shouldn't really delete
> stations all that often, right?

My main interest is in emulating lots of station vdevs, like 64 per
NIC with ath10k. So, dis-associating one should have the least impact
possible on the others.

More normal users are typically one vdev per NIC, so it matters less,
though with some of the P2P stuff, maybe it would still be useful.

In AP mode, I am less certain of how this all works, but there may
also be cases there where changing a flush to waiting for a tx-completion
would be useful.

>> If multiple, then waiting for tx status of just the deauth frame should be
>> faster
>> on average than waiting for the flush since you don't have to flush the
>> other
>> vdevs.
>
> True. Well, on Intel devices, we work with Tx queues, and now we have
> a Tx queue for each ra/tid. Yes it limits the number of peers we can
> support, but as I said, we are less AP mode oriented.

ath10k has similar.

>
>>
>> Whether drivers support a correct tx-status, I think all of them report
>> *some* tx
>> status...that is how mac80211 knows it can clean up the frame? That would
>> be enough
>> to know that the frame at least went through the tx logic and thus was
>> flushed.
>
> It is an option. Maybe mac80211 could pass the pointer to the skb it
> is waiting for in the flush(drop=false) call and that would make it
> easier for you to wait for it internally in ath10k?

I think that would not help a great deal. I've got some more critical
things to poke at first, but maybe will attempt a tx-completion wait
instead of a flush and see how that works.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com