ath10k.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
@ 2020-12-15 17:21 Youghandhar Chintala
  2020-12-15 18:23 ` Ben Greear
  2020-12-17 22:30 ` Brian Norris
  0 siblings, 2 replies; 8+ messages in thread
From: Youghandhar Chintala @ 2020-12-15 17:21 UTC (permalink / raw)
  To: johannes, ath10k
  Cc: kuabhs, netdev, briannorris, linux-wireless, linux-kernel,
	dianders, pillair, kuba, davem, kvalo

From: Rakesh Pillai <pillair@codeaurora.org>

Currently in case of target hardware restart ,we just reconfig and
re-enable the security keys and enable the network queues to start
data traffic back from where it was interrupted.

Many ath10k wifi chipsets have sequence numbers for the data
packets assigned by firmware and the mac sequence number will
restart from zero after target hardware restart leading to mismatch
in the sequence number expected by the remote peer vs the sequence
number of the frame sent by the target firmware.

This mismatch in sequence number will cause out-of-order packets
on the remote peer and all the frames sent by the device are dropped
until we reach the sequence number which was sent before we restarted
the target hardware

In order to fix this, we trigger a disconnect in case of hardware
restart. After this there will be a fresh connection and thereby
avoiding the dropping of frames by remote peer.

The right fix would be to pull the entire data path into the host
which is not feasible or would need lots of complex/inefficient
datapath changes.

Rakesh Pillai (1):
  ath10k: Set wiphy flag to trigger sta disconnect on hardware restart

Youghandhar Chintala (2):
  cfg80211: Add wiphy flag to trigger STA disconnect after hardware
    restart
  mac80211: Add support to trigger sta disconnect on hardware restart

 drivers/net/wireless/ath/ath10k/core.c | 15 +++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |  3 +++
 include/net/cfg80211.h                 |  4 ++++
 net/mac80211/ieee80211_i.h             |  3 +++
 net/mac80211/mlme.c                    |  9 +++++++++
 net/mac80211/util.c                    | 22 +++++++++++++++++++---
 7 files changed, 56 insertions(+), 3 deletions(-)

-- 
2.7.4


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-15 17:21 [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery Youghandhar Chintala
@ 2020-12-15 18:23 ` Ben Greear
  2020-12-16 11:35   ` Rakesh Pillai
  2020-12-17 22:24   ` Brian Norris
  2020-12-17 22:30 ` Brian Norris
  1 sibling, 2 replies; 8+ messages in thread
From: Ben Greear @ 2020-12-15 18:23 UTC (permalink / raw)
  To: Youghandhar Chintala, johannes, ath10k
  Cc: kuabhs, netdev, briannorris, linux-wireless, linux-kernel,
	dianders, pillair, kuba, davem, kvalo

On 12/15/20 9:21 AM, Youghandhar Chintala wrote:
> From: Rakesh Pillai <pillair@codeaurora.org>
> 
> Currently in case of target hardware restart ,we just reconfig and
> re-enable the security keys and enable the network queues to start
> data traffic back from where it was interrupted.

Are there any known mac80211 radios/drivers that *can* support seamless restarts?

If not, then just could always enable this feature in mac80211?

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-15 18:23 ` Ben Greear
@ 2020-12-16 11:35   ` Rakesh Pillai
  2020-12-17 22:24   ` Brian Norris
  1 sibling, 0 replies; 8+ messages in thread
From: Rakesh Pillai @ 2020-12-16 11:35 UTC (permalink / raw)
  To: 'Ben Greear', 'Youghandhar Chintala', johannes, ath10k
  Cc: kuabhs, netdev, briannorris, linux-wireless, linux-kernel,
	dianders, kuba, davem, kvalo


> From: Ben Greear <greearb@candelatech.com>
> 
> On 12/15/20 9:21 AM, Youghandhar Chintala wrote:
> > From: Rakesh Pillai <pillair@codeaurora.org>
> >
> > Currently in case of target hardware restart ,we just reconfig and
> > re-enable the security keys and enable the network queues to start
> > data traffic back from where it was interrupted.
> 
> Are there any known mac80211 radios/drivers that *can* support seamless
> restarts?
> 
> If not, then just could always enable this feature in mac80211?

I am not aware of any mac80211 target which can restart in a seamless manner.
Hence I chose to keep this optional and driver can expose this flag (if needed) based on the hardware capability.

Thanks,
Rakesh Pillai.


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-15 18:23 ` Ben Greear
  2020-12-16 11:35   ` Rakesh Pillai
@ 2020-12-17 22:24   ` Brian Norris
  2020-12-17 22:57     ` Ben Greear
  1 sibling, 1 reply; 8+ messages in thread
From: Brian Norris @ 2020-12-17 22:24 UTC (permalink / raw)
  To: Ben Greear
  Cc: kuabhs, Youghandhar Chintala, netdev, linux-wireless,
	linux-kernel, ath10k, dianders, pillair, kuba, johannes, davem,
	kvalo

On Tue, Dec 15, 2020 at 10:23:33AM -0800, Ben Greear wrote:
> On 12/15/20 9:21 AM, Youghandhar Chintala wrote:
> > From: Rakesh Pillai <pillair@codeaurora.org>
> > 
> > Currently in case of target hardware restart ,we just reconfig and
> > re-enable the security keys and enable the network queues to start
> > data traffic back from where it was interrupted.
> 
> Are there any known mac80211 radios/drivers that *can* support seamless restarts?
> 
> If not, then just could always enable this feature in mac80211?

I'm quite sure that iwlwifi intentionally supports a seamless restart.
From my experience with dealing with user reports, I don't recall any
issues where restart didn't function as expected, unless there was some
deeper underlying failure (e.g., hardware/power failure; driver bugs /
lockups).

I don't have very good stats for ath10k/QCA6174, but it survives
our testing OK and I again don't recall any user-reported complaints in
this area. I'd say this is a weaker example though, as I don't have as
clear of data. (By contrast, ath10k/WCN399x, which Rakesh, et al, are
patching here, does not pass our tests at all, and clearly fails to
recover from "seamless" restarts, as noted in patch 3.)

I'd also note that we don't operate in AP mode -- only STA -- and IIRC
Ben, you've complained about AP mode in the past.

Brian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-15 17:21 [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery Youghandhar Chintala
  2020-12-15 18:23 ` Ben Greear
@ 2020-12-17 22:30 ` Brian Norris
  1 sibling, 0 replies; 8+ messages in thread
From: Brian Norris @ 2020-12-17 22:30 UTC (permalink / raw)
  To: Youghandhar Chintala
  Cc: kuabhs, netdev, linux-wireless, linux-kernel, ath10k, dianders,
	pillair, kuba, johannes, davem, kvalo

On Tue, Dec 15, 2020 at 10:51:13PM +0530, Youghandhar Chintala wrote:
> From: Rakesh Pillai <pillair@codeaurora.org>

I meant to mention in my other reply: the threading on this series is
broken (as in, it doesn't exist). It looks like you're using
git-send-email (good!), but somehow it doesn't have any In-Reply-To or
References (bad!). Did you send all your mail in one invocation, or did
you send them as separate git-send-email commands? Anyway, please
investigate what when wrong so you can get this right in the future.

For one, this affects Patchwork's ability to group patch series (not to
mention everybody who uses a decent mail reader, with proper threading).
See for example the lore archive, which only is threading replies to
this cover letter:

https://lore.kernel.org/linux-wireless/20201215172113.5038-1-youghand@codeaurora.org/

Regards,
Brian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-17 22:24   ` Brian Norris
@ 2020-12-17 22:57     ` Ben Greear
  2020-12-17 23:18       ` Brian Norris
  0 siblings, 1 reply; 8+ messages in thread
From: Ben Greear @ 2020-12-17 22:57 UTC (permalink / raw)
  To: Brian Norris
  Cc: kuabhs, Youghandhar Chintala, netdev, linux-wireless,
	linux-kernel, ath10k, dianders, pillair, kuba, johannes, davem,
	kvalo

On 12/17/20 2:24 PM, Brian Norris wrote:
> On Tue, Dec 15, 2020 at 10:23:33AM -0800, Ben Greear wrote:
>> On 12/15/20 9:21 AM, Youghandhar Chintala wrote:
>>> From: Rakesh Pillai <pillair@codeaurora.org>
>>>
>>> Currently in case of target hardware restart ,we just reconfig and
>>> re-enable the security keys and enable the network queues to start
>>> data traffic back from where it was interrupted.
>>
>> Are there any known mac80211 radios/drivers that *can* support seamless restarts?
>>
>> If not, then just could always enable this feature in mac80211?
> 
> I'm quite sure that iwlwifi intentionally supports a seamless restart.
>  From my experience with dealing with user reports, I don't recall any
> issues where restart didn't function as expected, unless there was some
> deeper underlying failure (e.g., hardware/power failure; driver bugs /
> lockups).
> 
> I don't have very good stats for ath10k/QCA6174, but it survives
> our testing OK and I again don't recall any user-reported complaints in
> this area. I'd say this is a weaker example though, as I don't have as
> clear of data. (By contrast, ath10k/WCN399x, which Rakesh, et al, are
> patching here, does not pass our tests at all, and clearly fails to
> recover from "seamless" restarts, as noted in patch 3.)
> 
> I'd also note that we don't operate in AP mode -- only STA -- and IIRC
> Ben, you've complained about AP mode in the past.

I complain about all sorts of things, but I'm usually running
station mode :)

Do you actually see iwlwifi stations stay associated through
firmware crashes?

Anyway, happy to hear some have seamless recovery, and in that case,
I have no objections to the patch.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
  2020-12-17 22:57     ` Ben Greear
@ 2020-12-17 23:18       ` Brian Norris
  0 siblings, 0 replies; 8+ messages in thread
From: Brian Norris @ 2020-12-17 23:18 UTC (permalink / raw)
  To: Ben Greear
  Cc: kuabhs, Youghandhar Chintala, <netdev@vger.kernel.org>,
	linux-wireless, Linux Kernel, ath10k, Doug Anderson,
	Rakesh Pillai, Jakub Kicinski, Johannes Berg, David S. Miller,
	Kalle Valo

On Thu, Dec 17, 2020 at 2:57 PM Ben Greear <greearb@candelatech.com> wrote:
> On 12/17/20 2:24 PM, Brian Norris wrote:
> > I'd also note that we don't operate in AP mode -- only STA -- and IIRC
> > Ben, you've complained about AP mode in the past.
>
> I complain about all sorts of things, but I'm usually running
> station mode :)

Hehe, fair :) Maybe I'm mixed up.

But I do get the feeling that specifically within the ath10k family,
there are wildly different use cases (mobile, PC, AP) and chips (and
firmware) that tend to go along with them, and that those use cases
get a fairly different population of {developers, testers, reporters}.
So claiming "feature X works" pretty much always has to be couched in
which chips, firmware, and use case. And there's certainly some wisdom
in these sections:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/submittingpatches#hardware_families
https://wireless.wiki.kernel.org/en/users/drivers/ath10k/submittingpatches#tested-on_tag

> Do you actually see iwlwifi stations stay associated through
> firmware crashes?

Yes.

> Anyway, happy to hear some have seamless recovery, and in that case,
> I have no objections to the patch.

OK! I hope I'm not the only one with such results, because then I
still might question my sanity (and test coverage), but that's still
my understanding.

BTW, I haven't yet closely reviewed the patch series myself, but I ACK
the concept.

Brian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
@ 2020-12-15 17:30 Youghandhar Chintala
  0 siblings, 0 replies; 8+ messages in thread
From: Youghandhar Chintala @ 2020-12-15 17:30 UTC (permalink / raw)
  To: johannes, ath10k
  Cc: kuabhs, netdev, briannorris, linux-wireless, linux-kernel,
	dianders, pillair, kuba, davem, kvalo

From: Rakesh Pillai <pillair@codeaurora.org>

Currently in case of target hardware restart ,we just reconfig and
re-enable the security keys and enable the network queues to start
data traffic back from where it was interrupted.

Many ath10k wifi chipsets have sequence numbers for the data
packets assigned by firmware and the mac sequence number will
restart from zero after target hardware restart leading to mismatch
in the sequence number expected by the remote peer vs the sequence
number of the frame sent by the target firmware.

This mismatch in sequence number will cause out-of-order packets
on the remote peer and all the frames sent by the device are dropped
until we reach the sequence number which was sent before we restarted
the target hardware

In order to fix this, we trigger a disconnect in case of hardware
restart. After this there will be a fresh connection and thereby
avoiding the dropping of frames by remote peer.

The right fix would be to pull the entire data path into the host
which is not feasible or would need lots of complex/inefficient
datapath changes.

Rakesh Pillai (1):
  ath10k: Set wiphy flag to trigger sta disconnect on hardware restart

Youghandhar Chintala (2):
  cfg80211: Add wiphy flag to trigger STA disconnect after hardware
    restart
  mac80211: Add support to trigger sta disconnect on hardware restart

 drivers/net/wireless/ath/ath10k/core.c | 15 +++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |  3 +++
 include/net/cfg80211.h                 |  4 ++++
 net/mac80211/ieee80211_i.h             |  3 +++
 net/mac80211/mlme.c                    |  9 +++++++++
 net/mac80211/util.c                    | 22 +++++++++++++++++++---
 7 files changed, 56 insertions(+), 3 deletions(-)

-- 
2.7.4


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-17 23:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 17:21 [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery Youghandhar Chintala
2020-12-15 18:23 ` Ben Greear
2020-12-16 11:35   ` Rakesh Pillai
2020-12-17 22:24   ` Brian Norris
2020-12-17 22:57     ` Ben Greear
2020-12-17 23:18       ` Brian Norris
2020-12-17 22:30 ` Brian Norris
2020-12-15 17:30 Youghandhar Chintala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).