2020-06-12 10:38:54

by Pali Rohár

[permalink] [raw]
Subject: mwifiex: Initialization of FW and net interface

Hello!

I was looking at mwifiex code which initialize card firmware and linux
network interface and I think that there are some issues with this code
path.

There is a function mwifiex_sta_init_cmd() which basically doing two
different things:

* initial card firmware initialization
* configuration of interface parameters via card firmware

That function has following definition:

int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init);

'first_sta' and 'init' is FALSE when doing just configuration of
interface parameters (by cfg80211 callbacks).

'init' is TRUE when doing initial card firmware initialization and it is
called when mwifiex driver is doing setup of card. But this function is
called with 'init' set to TRUE multiple times when card driver was
configured to create multiple linux network interfaces. In this case
'first_sta' is TRUE only for the first call of this function.

And now the first suspicious thing is: Why mwifiex driver calls that
initial card firmware initialization multiple times when network
interfaces are created during driver setup, and not when they are
created later by "iw phy phyX interface add ..."?

Next, looking at code of that function mwifiex_sta_init_cmd() it looks
like that all commands send to firmware are "global" and affects all
existing mwifiex network interfaces. Why then it is needed to call this
function when creating a new interface? (E.g. second bssid for AP mode).

Also if it really affects all existing interfaces, it means that
creating a new interface changes configured cfg80211 parameters of all
existing interfaces to some default values.

This also affects power save settings which I described in previous email:
https://lore.kernel.org/linux-wireless/20200609111544.v7u5ort3yk4s7coy@pali/

And the last and the most suspicious thing in that mwifiex_sta_init_cmd
function is that some AP related code is executed only during initial
card firmware initialization and only if initial interface is AP mode
('init' = TRUE, 'first_sta' = TRUE, mode = 'AP').

Seems that driver behaves differently if interfaces are created by
standard 'iw phy phyX interface add ..." command (via cfg80211 layer)
and differently if interfaces are created automatically during driver
setup function.

Are there any reasons for these differences? And what to do with the
fact that most firmware commands which affects all interfaces and not
just one which is initializing?

These issues which I described makes it hard for me to understand what
is driver really doing and what should be correct behavior.

By the way, do you have documentation for firmware commands?


2021-07-17 13:50:34

by Pali Rohár

[permalink] [raw]
Subject: Re: mwifiex: Initialization of FW and net interface

[+cc: Maximilian and Jonas]

Hello Sharvari! I'm forwarding to you this email about initialization
issues in mwifiex driver. Could you as a new NXP maintainer of mwifiex
driver look at it?

On Friday 12 June 2020 12:35:01 Pali Rohár wrote:
> Hello!
>
> I was looking at mwifiex code which initialize card firmware and linux
> network interface and I think that there are some issues with this code
> path.
>
> There is a function mwifiex_sta_init_cmd() which basically doing two
> different things:
>
> * initial card firmware initialization
> * configuration of interface parameters via card firmware
>
> That function has following definition:
>
> int mwifiex_sta_init_cmd(struct mwifiex_private *priv, u8 first_sta, bool init);
>
> 'first_sta' and 'init' is FALSE when doing just configuration of
> interface parameters (by cfg80211 callbacks).
>
> 'init' is TRUE when doing initial card firmware initialization and it is
> called when mwifiex driver is doing setup of card. But this function is
> called with 'init' set to TRUE multiple times when card driver was
> configured to create multiple linux network interfaces. In this case
> 'first_sta' is TRUE only for the first call of this function.
>
> And now the first suspicious thing is: Why mwifiex driver calls that
> initial card firmware initialization multiple times when network
> interfaces are created during driver setup, and not when they are
> created later by "iw phy phyX interface add ..."?
>
> Next, looking at code of that function mwifiex_sta_init_cmd() it looks
> like that all commands send to firmware are "global" and affects all
> existing mwifiex network interfaces. Why then it is needed to call this
> function when creating a new interface? (E.g. second bssid for AP mode).
>
> Also if it really affects all existing interfaces, it means that
> creating a new interface changes configured cfg80211 parameters of all
> existing interfaces to some default values.
>
> This also affects power save settings which I described in previous email:
> https://lore.kernel.org/linux-wireless/20200609111544.v7u5ort3yk4s7coy@pali/
>
> And the last and the most suspicious thing in that mwifiex_sta_init_cmd
> function is that some AP related code is executed only during initial
> card firmware initialization and only if initial interface is AP mode
> ('init' = TRUE, 'first_sta' = TRUE, mode = 'AP').
>
> Seems that driver behaves differently if interfaces are created by
> standard 'iw phy phyX interface add ..." command (via cfg80211 layer)
> and differently if interfaces are created automatically during driver
> setup function.
>
> Are there any reasons for these differences? And what to do with the
> fact that most firmware commands which affects all interfaces and not
> just one which is initializing?
>
> These issues which I described makes it hard for me to understand what
> is driver really doing and what should be correct behavior.
>
> By the way, do you have documentation for firmware commands?