2024-04-10 17:56:55

by Johannes Berg

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

On Wed, 2024-04-10 at 10:33 +0000, David Lin wrote:
>
>
> Take Rx data path as an example,
> In current FW, BA stream setup and de-ampdu are handled in FW. Packet is converted to 802.3 before passing to host. Ampdu reordering is handled in host driver (Mwifiex) due to memory consideration. We used to work on a driver that uses RX_FLAG_8023. It was on an older Wi-Fi part which has more memory and powerful processor. But with this chip buffer required for reordering doesn’t fit FW memory.
>
> Ampdu reordering needs MAC 802.11 header, FW keeps the MAC header in packet descriptor since packet already 802.3 when arrive at driver. As packet descriptor only accessible in the driver, Mwifiex handles the reordering instead of using mac80211 reordering.
>
> With current FW design, to apply mac802.11 we either change FW to pass packet in 802.11 format or driver needs to do the conversion back to 802.11 (which I think doesn’t make sense)
>
> Given existing FW design, we think it’s difficult to apply mac80211.

Hmm, I don't think so? If you have a mac80211 driver with RX_FLAG_8023,
then of course mac80211 cannot do reordering, and you have to do it
somewhere below. But that doesn't mean you necessarily _have_ to do it
in hardware/firmware? You could do it in the driver, which you also have
to do in a cfg80211 driver anyway, and that's OK. Due to usage of RSS,
iwlwifi/mvm even does it internally, although it doesn't even have
RX_FLAG_8023.

But that goes into the direction I was talking about: the boundaries are
getting fuzzier all the time, because offloads happen and get supported
in mac80211. So if you have offloads for header conversion and only get
some reordering data "out of band", then you have to handle that in the
driver. Using mac80211 doesn't mean you have to use _all_ of it.
Originally, mac80211 didn't even have RX_FLAG_8023 after all. But
obviously adding something like that also requires more upstream
engagement :)

I think the question is about the design, but I also think the
differences in the association process are much more fundamental, and
you _don't_ (seem to) handle that in the way mac80211 does/expects,
though you also don't seem to handle it in the way most other full-MAC
devices do (which [can] offload even BSS selection.)

The thing I want you to understand is the relative architecture and how
your work fits into this. I'm not telling you have to change it right
now or do this work differently, I just want to make sure you actually
understand it. The above argument goes some way, showing you've actually
looked at the differences mac80211 would imply, where before I felt you
were pretty much handwaving it away as if irrelevant to the discussion.

johannes


2024-04-11 07:57:47

by David Lin

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v9 0/2] wifi: mwifiex: add code to support host mlme

> From: Johannes Berg <[email protected]>
> Sent: Thursday, April 11, 2024 1:57 AM
> To: David Lin <[email protected]>; Brian Norris
> <[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
>
> On Wed, 2024-04-10 at 10:33 +0000, David Lin wrote:
> >
> >
> > Take Rx data path as an example,
> > In current FW, BA stream setup and de-ampdu are handled in FW. Packet is
> converted to 802.3 before passing to host. Ampdu reordering is handled in host
> driver (Mwifiex) due to memory consideration. We used to work on a driver
> that uses RX_FLAG_8023. It was on an older Wi-Fi part which has more
> memory and powerful processor. But with this chip buffer required for
> reordering doesn’t fit FW memory.
> >
> > Ampdu reordering needs MAC 802.11 header, FW keeps the MAC header in
> packet descriptor since packet already 802.3 when arrive at driver. As packet
> descriptor only accessible in the driver, Mwifiex handles the reordering instead
> of using mac80211 reordering.
> >
> > With current FW design, to apply mac802.11 we either change FW to pass
> > packet in 802.11 format or driver needs to do the conversion back to
> > 802.11 (which I think doesn’t make sense)
> >
> > Given existing FW design, we think it’s difficult to apply mac80211.
>
> Hmm, I don't think so? If you have a mac80211 driver with RX_FLAG_8023,
> then of course mac80211 cannot do reordering, and you have to do it
> somewhere below. But that doesn't mean you necessarily _have_ to do it in
> hardware/firmware? You could do it in the driver, which you also have to do in
> a cfg80211 driver anyway, and that's OK. Due to usage of RSS, iwlwifi/mvm
> even does it internally, although it doesn't even have RX_FLAG_8023.
>
> But that goes into the direction I was talking about: the boundaries are getting
> fuzzier all the time, because offloads happen and get supported in mac80211.
> So if you have offloads for header conversion and only get some reordering
> data "out of band", then you have to handle that in the driver. Using mac80211
> doesn't mean you have to use _all_ of it.

Following jobs are done by FW:

1. MAC 802.11 Rx processes except for BA buffering/reordering and AMSDU.
2. Convert 802.11 frame to 802.3 frame.
3. Embedded MAC 802.11 header information to Rx descriptor for driver to do BA buffering/reordering.

If this FW wants to leverage mac80211:

1. Use 802.11 frame:
Driver should restructure 802.11 frame and mac80211 will redo jobs done by FW. I think this does not make sense.
2. Use 802.3 frame:
Driver still needs to do BA buffering/reordering (mac80211 can help with AMSDU with flag IEEE80211_RX_AMSDU,
but cfg80211 also supports function ieee80211_amsdu_to_8023s() to help this job).
If this is the case, it becomes redundant to pass 802.3 frame to kernel stack via mac80211.

So I think cfg80211 will be more suitable for existing FW.

> Originally, mac80211 didn't even have RX_FLAG_8023 after all. But obviously
> adding something like that also requires more upstream engagement :)
>

Yes, we requested 802.3 fast forwarding of mac80211 due to our previous fully Rx offload FW with very powerful WiFi SoC.

> I think the question is about the design, but I also think the differences in the
> association process are much more fundamental, and you _don't_ (seem to)
> handle that in the way mac80211 does/expects, though you also don't seem to
> handle it in the way most other full-MAC devices do (which [can] offload even
> BSS selection.)
>

FW only supports add station command for AP mode. Association command is used to request and add client station to FW.
FW will help to connect to AP and reply result to driver.
This is another reason that we need to use cfg80211 instead of mac80211. For mac80211, management frames are passed
through ieee80211_ops.tx and station is added via ieee80211_ops.sta_add.
This way can't work with FW for client mode, FW can't not be bypassed for association process for client mode.

> The thing I want you to understand is the relative architecture and how your
> work fits into this. I'm not telling you have to change it right now or do this
> work differently, I just want to make sure you actually understand it. The above
> argument goes some way, showing you've actually looked at the differences
> mac80211 would imply, where before I felt you were pretty much handwaving
> it away as if irrelevant to the discussion.
>
> johannes

Thanks for your comments and suggestions. I wonder can I prepare patch v10 to let this patch be accepted?

David