On 7/30/20 6:13 AM, Johannes Berg wrote:
On Thu, 2020-07-30 at 06:05 -0700, Ben Greear wrote:
It might even be that this can only be done subject to driver choice.
I have tested this patch hard for many years with hundreds of station vifs on ath9k radios and
64 station vifs on ath10k radios, probably way harder than anyone else is testing
this sort of thing.
Yeah, I'm sure!
Possibly you are correct about iwlwifi, I've never tested it with multi-interface,
and as you see, have had bad luck on ax200.
Right.
If you'd accept a patch with a new driver flag check (which I can enable for
ath10k and ath9k), then I'll respin it thus.
My order of preference would be something like
1. track per vif whether it was re-added, and skip before it is
If that works, I can certainly get behind it for semantic reasons (the
vif isn't yet active again), although even there I'm not sure how
iwlwifi would behave - but that's something I'd look into and perhaps
even consider a bug there since it shouldn't know about that interface
yet.
Wouldn't that complicate the sdata-in-driver check even more? So it would
be something like is-in-driver-but-not-yet-reconfigured-in-driver?
This sort of state is quite nasty in my experience. Almost better if
we had less state. Driver should already know if it has an object or not,
so redundant adds, or requests from mac80211 for objects the driver does not
have can be handled properly by the driver?
2. If for some reason that doesn't work, add an iteration flag that
controls this, rather than a per-device config?
I wrote this patch probably 5 or so years ago, and since I have fixed most of
the ath10k firmware crashes that would tend to trigger this bug. I think I have no
good way to test any complicated change to this patch.
I am quite certain that ath9k and ath10k are at least not harmed by this patch, and
certainly old ath10k benefited from this. So, I'm comfortable adding a driver
level flag enabled for those two drivers. Changing all the calling locations to
(maybe) add a flag or adding and tracking vif state is too risky to be worth this change I
think.
Thanks,
Ben
johannes
--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc http://www.candelatech.com
