2024-05-23 00:51:24

by Brian Norris

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v8 1/2] wifi: mwifiex: add host mlme for client mode

Necromancing some v8 comments here, since I realized I had the same
question on my last pass at v10:

On Mon, Mar 4, 2024 at 11:26 PM David Lin <[email protected]> wrote:
> > From: David Lin
> > Sent: Thursday, February 29, 2024 11:53 AM
> >
> > > From: Francesco Dolcini <[email protected]>
> > > Sent: Wednesday, February 28, 2024 1:53 AM
> > >
> > > On Fri, Dec 22, 2023 at 11:21:22AM +0800, David Lin wrote:
> > > > +static int
> > > > +mwifiex_cfg80211_probe_client(struct wiphy *wiphy,
> > > > + struct net_device *dev, const u8 *peer,
> > > > + u64 *cookie) {
> > > > + return -1;
> > >
> > > See my following comment on this

> > > > + mwifiex_cfg80211_ops.probe_client =
> > > > + mwifiex_cfg80211_probe_client;
> > >
> > > Can you omit this one? You should get `-EOPNOTSUPP` for free with
> > > probe_client set to NULL. Am I wrong?
> > >
> >
> > Yes. You are right. Remove in patch v9.
>
> This function must be hooked, otherwise AP mode can't work. I will hook this function as before but return -EOPNOTSUPP instead.

You mean, hostapd doesn't like it? That seems like hostapd's problem,
because the actual effect is the same -- the feature isn't supported,
and will always throw an error. The kernel is lying if it adds
NL80211_CMD_PROBE_CLIENT into the wiphy command feature list.

Can you point at what specifically goes wrong in hostapd, and see if
we can find a way to fix it in the right place -- in hostapd? I don't
really hack on hostapd much, but it looks like it comes down to the
drv->use_monitor flag? Notably, that has a comment:

# src/drivers/driver_nl80211_capa.c ~ line 1424
/*
* If poll command and tx status are supported, mac80211 is new enough
* to have everything we need to not need monitor interfaces.
*/

So, you're actively subverting something that hostapd is trying to
learn about mac80211. That sounds double wrong.

At the very least, if we're going to include hacks like this in the
kernel, we should probably have a comment in there, so that future
readers don't keep stubbing their toes on the same question. Or worse,
removing the seemingly useless function and breaking hostapd. But
ideally, we'd drop this hack and fix hostapd.

Brian