Linux-Wireless Archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Brian Norris <briannorris@chromium.org>, David Lin <yu-hao.lin@nxp.com>
Cc: Francesco Dolcini <francesco@dolcini.it>,
	"kvalo@kernel.org" <kvalo@kernel.org>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Pete Hsieh <tsung-hsien.hsieh@nxp.com>,
	 "rafael.beims" <rafael.beims@toradex.com>,
	Francesco Dolcini <francesco.dolcini@toradex.com>
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme
Date: Wed, 10 Apr 2024 09:52:44 +0200	[thread overview]
Message-ID: <533f8c078f28c0e448005154c08f51518d332640.camel@sipsolutions.net> (raw)
In-Reply-To: <ZgxCngq_Rguc4qs8@google.com>

Hi Brian,

> 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.

Agree.

> 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.

We'll see, but I don't _actually_ think this will significantly change
the architecture here.

In any case we can always kick the can further down the road.

> > 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.

Partially, yes, but also architecturally it should fit in.

> 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.

I'd say though that "actively maintained" part really is because full-
MAC devices that are supported are very few now with Broadcom having
essentially dropped out. I suspect there are other full-MAC chips and/or
firmwares on the market, but few, if any, supported upstream. There's no
particular reason this must be the case, it's just the way hardware
architecture seems to be going.

And as you said before, mac80211 is doing more and more offloads too, so
the line ends up blurring from both sides.
Which then again is part of my concern, if we blur the line from both
sides we need to be even more careful to not grow into a parallel zone
too much.

> 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.

I actually agree.


> This feels "thick", like David mentioned above.

This is kind of getting to the core of the thread: I, for one, don't
think he made that argument. He just *claimed*, without much argument
for that claim, that "Mwifiex is designed based on a "Thick FW"
architecture."

> > 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,

Nor does "we are using standard cfg80211 commands", FWIW. That doesn't
mean we need to think the architecture is good. Taking this argument to
the extreme, it'd be entirely possible to put a (modified) copy of
mac80211 into a driver and claim "we are using standard cfg80211
commands". It's a non-argument.

And really here we get to the core of the issue: Brian, you have now
mostly done the actual _technical_ analysis (and defence) of this patch
that I was waiting for _David_ to do.

johannes

  parent reply	other threads:[~2024-04-10  7:52 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-06  2:00 [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme David Lin
2024-03-06  2:00 ` [PATCH v9 1/2] wifi: mwifiex: add host mlme for client mode David Lin
2024-03-16  0:00   ` Brian Norris
2024-03-18  2:00     ` [EXT] " David Lin
2024-03-18 11:41       ` Francesco Dolcini
2024-03-19  1:36         ` [EXT] " David Lin
2024-03-06  2:00 ` [PATCH v9 2/2] wifi: mwifiex: add host mlme for AP mode David Lin
2024-03-16  0:45   ` Brian Norris
2024-03-18  2:04     ` [EXT] " David Lin
2024-04-18  3:37       ` David Lin
2024-03-15  9:49 ` [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme Francesco Dolcini
2024-03-15 23:59   ` Brian Norris
2024-03-18 11:28     ` Francesco Dolcini
2024-03-19 10:33       ` Kalle Valo
2024-03-20 21:06         ` Brian Norris
2024-03-16  0:49   ` Brian Norris
2024-03-19 12:12     ` Johannes Berg
2024-03-20  0:59       ` [EXT] " David Lin
2024-03-20  1:10         ` David Lin
2024-03-20  9:12           ` Johannes Berg
2024-03-20 21:50             ` Brian Norris
2024-03-21  4:07               ` David Lin
2024-03-23  1:06                 ` Brian Norris
2024-03-25 16:15                   ` Johannes Berg
2024-03-29 10:06                     ` David Lin
2024-04-02 17:38                       ` Brian Norris
2024-04-10  7:30                         ` David Lin
2024-04-10  7:55                           ` Johannes Berg
2024-04-10 10:33                             ` David Lin
2024-04-10 17:56                               ` Johannes Berg
2024-04-11  7:57                                 ` David Lin
2024-04-11  8:02                                   ` Johannes Berg
2024-04-10  7:52                         ` Johannes Berg [this message]
2024-03-25 15:58               ` Johannes Berg
2024-03-29  9:58                 ` David Lin
2024-03-18  9:24   ` Kalle Valo
2024-03-19  1:40     ` [EXT] " David Lin
2024-03-20 21:28     ` Brian Norris
2024-03-21  2:14       ` [EXT] " David Lin
2024-03-16  0:07 ` Brian Norris
2024-03-18  2:20   ` [EXT] " David Lin
2024-03-18 11:45     ` Francesco Dolcini
2024-03-20 21:13     ` Brian Norris
2024-03-21  2:12       ` David Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=533f8c078f28c0e448005154c08f51518d332640.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=briannorris@chromium.org \
    --cc=francesco.dolcini@toradex.com \
    --cc=francesco@dolcini.it \
    --cc=kvalo@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=rafael.beims@toradex.com \
    --cc=tsung-hsien.hsieh@nxp.com \
    --cc=yu-hao.lin@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).