> From: Johannes Berg <[email protected]>
> Sent: Tuesday, March 26, 2024 12:15 AM
> To: Brian Norris <[email protected]>; David Lin
> <[email protected]>
> Cc: Francesco Dolcini <[email protected]>; [email protected];
> [email protected]; [email protected]; Pete Hsieh
> <[email protected]>; rafael.beims <[email protected]>;
> Francesco Dolcini <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Fri, 2024-03-22 at 18:06 -0700, Brian Norris wrote:
> > We appreciate it's well tested, but testing is still orthogonal to the
> > architectural questions. Architectural questions are important because
> > they affect the future maintainability of the mainline Linux wireless
> > stack. If the assumption is that *either* a driver is a cfg80211
> > driver (with firmware-MLME, etc.) or a mac80211 driver (with host
> > MLME), then your series is breaking those assumptions.
>
> Maybe, maybe not, actually. The auth command _is_ somewhat special in
> that it mostly hands stuff down from userspace via cfg80211, but does
> require sending frames. As long as you don't have full offload, at least.
>
> The way I see it, the issue here isn't necessarily the fact that this uses the
> auth command (and then requires assoc, of course), but that we see here
> this is "growing" towards a more mac80211-like model, with the code
> duplication (albeit little that it is today) implied by that. To me, it seems like
> the firmware is moving into the "oh we can't do all _that_ in firmware"
> territory, and that brings it closer to mac80211.
>
> At the same time, as you say, mac80211 is doing more and more offload
> capability, so it seems like apart from "today the firmware requires an assoc
> command rather than assoc frame processing in the host", it's actually not
> _that_ far apart any more!
>
> Now that may be an issue in the short term, but I wouldn't be surprised at
> all if desiring to implement FILS and other new features in this space would
> make the driver move to assoc frame processing in the host as well, because
> it's getting more and more complex, just like auth.
>
> At which point - yeah the APIs are still significantly different, but again we'd
> end up implementing something that exists in mac80211 today and taking it
> into mwifiex?
>
Mwifiex is designed based on a "Thick FW" architecture.
As security features become more complicated, this patch adds WPA3 support by offloading to wpa_supplicant/hostapd
With this patch, auth, assoc and key handshakes are handled in wpa_supplicant/hostapd.
Wpa_supplicant communicates peer capability and derived keys to driver/FW via cfg80211 assoc and add_key commands.
The cfg80211 commands are standard. Implementation between driver and firmware is vendor specific. It shall not break any existing architecture.
> > It may be harder to add
> > future additions to the mac80211 stack [*], if we have to add new
> > concerns of a non-mac80211 implementation in the mix.
>
> Not sure that makes a difference for mac80211 in itself, obviously changes in
> this space would then affect mwifiex, but that shouldn't be much of an
> issue.
>
With this patch Mwifiex still a non-mac80211 implementation.
Driver communicates with wpa_supplicant/hostapd via cfg80211
I think how driver/FW communicate each other is proprietary, I don't see a dependency with mac80211 here
> I'm less worried about this individual patch than what it says about the
> direction this driver and firmware are taking, and I fear we'll end up in a
> situation where over time this driver actually gets to a point where it should
> be using mac80211, but because it's such a piece-meal affair (auth frames
> now, etc.) and large architectural change, they'd never actually do that.
>
> To be fair, that might also require firmware API changes in some way. I used
> to think that was something we should never require, but I'm not so sure
> now any more - certainly we've changed our (Intel) FW API in support of
> Linux architecture many times, and overall that's for a better product (on
> Linux at least.)
>
> Also: David, I'd appreciate if you actually took this discussion seriously; so
> far you've not really contributed any technical arguments.
>
> Johannes
I think we are using standard cfg80211 commands. How it's handled between driver/FW is proprietary, it's carefully verified and shall not impact other features or break any architecture.
We do not see a need why driver has to be redesigned based on mac80211. We can keep adding new features using cfg80211.
Hi Johannes and David,
On Fri, Mar 29, 2024 at 10:06:19AM +0000, David Lin wrote:
> > From: Johannes Berg <[email protected]>
> >
> > The way I see it, the issue here isn't necessarily the fact that this uses the
> > auth command (and then requires assoc, of course), but that we see here
> > this is "growing" towards a more mac80211-like model, with the code
> > duplication (albeit little that it is today) implied by that. To me, it seems like
> > the firmware is moving into the "oh we can't do all _that_ in firmware"
> > territory, and that brings it closer to mac80211.
> >
> > At the same time, as you say, mac80211 is doing more and more offload
> > capability, so it seems like apart from "today the firmware requires an assoc
> > command rather than assoc frame processing in the host", it's actually not
> > _that_ far apart any more!
> >
> > Now that may be an issue in the short term, but I wouldn't be surprised at
> > all if desiring to implement FILS and other new features in this space would
> > make the driver move to assoc frame processing in the host as well, because
> > it's getting more and more complex, just like auth.
> >
> > At which point - yeah the APIs are still significantly different, but again we'd
> > end up implementing something that exists in mac80211 today and taking it
> > into mwifiex?
So it seems there's 2 possible sticking points: code duplication, and
overall trend in the specs and implementation that might increase
duplication?
To me, it seems like the duplication is minimal today, or at least, not
much as a result of anything in this patch proposal. There's some
repetition of 802.11 definitions already, but that's probably
orthogonal.
I have less understanding and foresight on the "trend" questions,
although David seems to somewhat agree in his response below -- that NXP
intends to handle modern security features in the host more and more,
which could indeed mean a bit more framing-related duplication.
> Mwifiex is designed based on a "Thick FW" architecture.
> As security features become more complicated, this patch adds WPA3 support by offloading to wpa_supplicant/hostapd
> With this patch, auth, assoc and key handshakes are handled in wpa_supplicant/hostapd.
> Wpa_supplicant communicates peer capability and derived keys to driver/FW via cfg80211 assoc and add_key commands.
> The cfg80211 commands are standard. Implementation between driver and firmware is vendor specific. It shall not break any existing architecture.
>
> > > It may be harder to add
> > > future additions to the mac80211 stack [*], if we have to add new
> > > concerns of a non-mac80211 implementation in the mix.
> >
> > Not sure that makes a difference for mac80211 in itself, obviously changes in
> > this space would then affect mwifiex, but that shouldn't be much of an
> > issue.
> >
>
> With this patch Mwifiex still a non-mac80211 implementation.
> Driver communicates with wpa_supplicant/hostapd via cfg80211
> I think how driver/FW communicate each other is proprietary, I don't see a dependency with mac80211 here
David, I may have pointed in the wrong direction by claiming "conflict"
with mac80211. I believe Johannes's concerns are in code duplication.
Pretty much all other actively-maintained Linux WiFi drivers are based
on mac80211, so that we don't all have to implement the same frame
construction and parsing code. mwifiex is somewhat of an outlier in
actively adding new features while remaining a cfg80211-only driver.
But for myself, I'm not even fully convinced mac80211-style stuff makes
sense here. Even just looking at the auth() stuff in patch 1, this
driver is far from a thin layer that allows mac80211 to handle the
802.11 details. Just look at the 'priv->host_mlme_reg' and
HostCmd_CMD_MGMT_FRAME_REG stuff -- it seems that the simple act of
sending a single 802.11 frame requires opting into some FW-specific
command mask. This feels "thick", like David mentioned above.
> > I'm less worried about this individual patch than what it says about the
> > direction this driver and firmware are taking, and I fear we'll end up in a
> > situation where over time this driver actually gets to a point where it should
> > be using mac80211, but because it's such a piece-meal affair (auth frames
> > now, etc.) and large architectural change, they'd never actually do that.
> >
> > To be fair, that might also require firmware API changes in some way. I used
> > to think that was something we should never require, but I'm not so sure
> > now any more - certainly we've changed our (Intel) FW API in support of
> > Linux architecture many times, and overall that's for a better product (on
> > Linux at least.)
Yeah, I feel like I see hints of stuff (in public driver code, and in
internal discussions with vendors) where things really work best in
mac80211 land if firmware is developed with *some* consideration for the
mac80211 Linux architecture (or else already is a very thin firmware
from the start). I don't feel like Marvell/NXP has that consideration at
all, and so trying to drag its firmware into mac80211 regardless would
bring its own unique pain. But that's just a lightly-educated feeling.
> > Also: David, I'd appreciate if you actually took this discussion seriously; so
> > far you've not really contributed any technical arguments.
>
> I think we are using standard cfg80211 commands. How it's handled
> between driver/FW is proprietary, it's carefully verified and shall
> not impact other features or break any architecture.
David, repeating the "carefully verified" stuff doesn't really help the
subject at hand, and it's not really a technical argument either, when
it's not accompanied by any proofs. Being careful and running a good QA
cycle are excellent and appreciated, but that's not what we're talking
about any more. We're trying to understand what the firmware
architecture is like, and what driver architecture matches your future
development best, with the needs of the Linux community in mind.
(Now, as an example useful argument: if you claimed that implementing a
mac80211 driver with a generalized ieee80211_ops::tx() function would
introduce unvetted combinations of control logic that cannot be
reconciled with your HostCmd protocols, or that QA can't properly vet
that ... well, that would be a start of a technical argument. But it
would require more than a sentence or two to describe why that's
impossible or difficult.)
> We do not see a need why driver has to be redesigned based on
> mac80211. We can keep adding new features using cfg80211.
All in all, other than being a little grumpy about reviewing new
features here, I'm still leaning toward David's statement here -- that I
don't see why it *must* be rewritten toward mac80211. But I still defer
to Johannes, and I'm just trying to be a bit of a referee, or the
proverbial "devil's advocate".
Brian
Hi Johannes and Brian,
I think this patch is used to leverage MLME of wpa_supplicant and hostapd. It won't affect the usage of cfg80211 for mwifiex. I wonder if I can prepare patch v10.
David
> From: Brian Norris <[email protected]>
> Sent: Wednesday, April 3, 2024 1:39 AM
> To: David Lin <[email protected]>; Johannes Berg
> <[email protected]>
> Cc: Johannes Berg <[email protected]>; Francesco Dolcini
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Pete Hsieh <[email protected]>;
> rafael.beims <[email protected]>; Francesco Dolcini
> <[email protected]>
> Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host
> mlme
>
> Hi Johannes and David,
>
> On Fri, Mar 29, 2024 at 10:06:19AM +0000, David Lin wrote:
> > > From: Johannes Berg <[email protected]>
> > >
> > > The way I see it, the issue here isn't necessarily the fact that
> > > this uses the auth command (and then requires assoc, of course), but
> > > that we see here this is "growing" towards a more mac80211-like
> > > model, with the code duplication (albeit little that it is today)
> > > implied by that. To me, it seems like the firmware is moving into the "oh
> we can't do all _that_ in firmware"
> > > territory, and that brings it closer to mac80211.
> > >
> > > At the same time, as you say, mac80211 is doing more and more
> > > offload capability, so it seems like apart from "today the firmware
> > > requires an assoc command rather than assoc frame processing in the
> > > host", it's actually not _that_ far apart any more!
> > >
> > > Now that may be an issue in the short term, but I wouldn't be
> > > surprised at all if desiring to implement FILS and other new
> > > features in this space would make the driver move to assoc frame
> > > processing in the host as well, because it's getting more and more complex,
> just like auth.
> > >
> > > At which point - yeah the APIs are still significantly different,
> > > but again we'd end up implementing something that exists in mac80211
> > > today and taking it into mwifiex?
>
> So it seems there's 2 possible sticking points: code duplication, and overall
> trend in the specs and implementation that might increase duplication?
>
> To me, it seems like the duplication is minimal today, or at least, not much as a
> result of anything in this patch proposal. There's some repetition of 802.11
> definitions already, but that's probably orthogonal.
>
> I have less understanding and foresight on the "trend" questions, although
> David seems to somewhat agree in his response below -- that NXP intends to
> handle modern security features in the host more and more, which could
> indeed mean a bit more framing-related duplication.
>
> > Mwifiex is designed based on a "Thick FW" architecture.
> > As security features become more complicated, this patch adds WPA3
> > support by offloading to wpa_supplicant/hostapd With this patch, auth, assoc
> and key handshakes are handled in wpa_supplicant/hostapd.
> > Wpa_supplicant communicates peer capability and derived keys to
> driver/FW via cfg80211 assoc and add_key commands.
> > The cfg80211 commands are standard. Implementation between driver and
> firmware is vendor specific. It shall not break any existing architecture.
> >
> > > > It may be harder to add
> > > > future additions to the mac80211 stack [*], if we have to add new
> > > > concerns of a non-mac80211 implementation in the mix.
> > >
> > > Not sure that makes a difference for mac80211 in itself, obviously
> > > changes in this space would then affect mwifiex, but that shouldn't
> > > be much of an issue.
> > >
> >
> > With this patch Mwifiex still a non-mac80211 implementation.
> > Driver communicates with wpa_supplicant/hostapd via cfg80211 I think
> > how driver/FW communicate each other is proprietary, I don't see a
> > dependency with mac80211 here
>
> David, I may have pointed in the wrong direction by claiming "conflict"
> with mac80211. I believe Johannes's concerns are in code duplication.
> Pretty much all other actively-maintained Linux WiFi drivers are based on
> mac80211, so that we don't all have to implement the same frame
> construction and parsing code. mwifiex is somewhat of an outlier in actively
> adding new features while remaining a cfg80211-only driver.
>
> But for myself, I'm not even fully convinced mac80211-style stuff makes sense
> here. Even just looking at the auth() stuff in patch 1, this driver is far from a
> thin layer that allows mac80211 to handle the
> 802.11 details. Just look at the 'priv->host_mlme_reg' and
> HostCmd_CMD_MGMT_FRAME_REG stuff -- it seems that the simple act of
> sending a single 802.11 frame requires opting into some FW-specific command
> mask. This feels "thick", like David mentioned above.
>
> > > I'm less worried about this individual patch than what it says about
> > > the direction this driver and firmware are taking, and I fear we'll
> > > end up in a situation where over time this driver actually gets to a
> > > point where it should be using mac80211, but because it's such a
> > > piece-meal affair (auth frames now, etc.) and large architectural change,
> they'd never actually do that.
> > >
> > > To be fair, that might also require firmware API changes in some
> > > way. I used to think that was something we should never require, but
> > > I'm not so sure now any more - certainly we've changed our (Intel)
> > > FW API in support of Linux architecture many times, and overall
> > > that's for a better product (on Linux at least.)
>
> Yeah, I feel like I see hints of stuff (in public driver code, and in internal
> discussions with vendors) where things really work best in
> mac80211 land if firmware is developed with *some* consideration for the
> mac80211 Linux architecture (or else already is a very thin firmware from the
> start). I don't feel like Marvell/NXP has that consideration at all, and so trying
> to drag its firmware into mac80211 regardless would bring its own unique pain.
> But that's just a lightly-educated feeling.
>
> > > Also: David, I'd appreciate if you actually took this discussion
> > > seriously; so far you've not really contributed any technical arguments.
> >
> > I think we are using standard cfg80211 commands. How it's handled
> > between driver/FW is proprietary, it's carefully verified and shall
> > not impact other features or break any architecture.
>
> David, repeating the "carefully verified" stuff doesn't really help the subject at
> hand, and it's not really a technical argument either, when it's not
> accompanied by any proofs. Being careful and running a good QA cycle are
> excellent and appreciated, but that's not what we're talking about any more.
> We're trying to understand what the firmware architecture is like, and what
> driver architecture matches your future development best, with the needs of
> the Linux community in mind.
>
> (Now, as an example useful argument: if you claimed that implementing a
> mac80211 driver with a generalized ieee80211_ops::tx() function would
> introduce unvetted combinations of control logic that cannot be reconciled
> with your HostCmd protocols, or that QA can't properly vet that ... well, that
> would be a start of a technical argument. But it would require more than a
> sentence or two to describe why that's impossible or difficult.)
>
> > We do not see a need why driver has to be redesigned based on
> > mac80211. We can keep adding new features using cfg80211.
>
> All in all, other than being a little grumpy about reviewing new features here,
> I'm still leaning toward David's statement here -- that I don't see why it *must*
> be rewritten toward mac80211. But I still defer to Johannes, and I'm just trying
> to be a bit of a referee, or the proverbial "devil's advocate".
>
> Brian
On Wed, 2024-04-10 at 07:30 +0000, David Lin wrote:
> Hi Johannes and Brian,
>
> I think this patch is used to leverage MLME of wpa_supplicant and hostapd. It won't affect the usage of cfg80211 for mwifiex. I wonder if I can prepare patch v10.
No. That sentence tells me you've _still_ not understood any of the
technical arguments in the thread, you're _still_ arguing with
completely uninteresting arguments. Where before you had "it's well
tested" and "it uses 'standard' APIs" now you're saying "it doesn't
affect anyone else". All of that is obvious, and also completely besides
the point.
Please go back and actually _understand_ the discussion. Also actually
_participate_ in the discussion too, so far you've pretty much only made
empty arguments. Once you've understood the concerns and can explain why
they don't apply, _then_ you can resend the patch.
johannes