Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5cdc7a91910a803ef531ae04ba330541476c0f18.camel@sipsolutions.net>
Subject: Re: [bug report] wifi: nl80211/mac80211: clarify link ID in control
 port TX
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Tue, 26 Jul 2022 16:42:41 +0200
In-Reply-To: <Yt/16E0ufa0D0Ziq@kili>
References: <Yt/16E0ufa0D0Ziq@kili>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.3 (3.44.3-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

Hi Dan,


On Tue, 2022-07-26 at 17:10 +0300, Dan Carpenter wrote:
>     5885         rcu_read_lock();
>     5886         err =3D ieee80211_lookup_ra_sta(sdata, skb, &sta);
>=20
> For sdata->vif.type =3D=3D NL80211_IFTYPE_MESH_POINT then "sta" can be NU=
LL.
> Smatch doesn't know the value of sdata->vif.type at this point, and even
> if it did, then it doesn't split the return states up with enough
> granularity for that to make a difference.

Right, but that wouldn't matter anyway?

>     5887         if (err) {
>     5888                 rcu_read_unlock();
>     5889                 return err;
>     5890         }
>     5891=20
>     5892         if (!IS_ERR(sta)) {

Since this is all independent of the interface type.

>     5893                 u16 queue =3D __ieee80211_select_queue(sdata, st=
a, skb);
>     5894=20
>     5895                 skb_set_queue_mapping(skb, queue);
>     5896                 skb_get_hash(skb);
>     5897=20
>     5898                 /*
>     5899                  * for MLO STA, the SA should be the AP MLD addr=
ess, but
>     5900                  * the link ID has been selected already
>     5901                  */
> --> 5902                 if (sta->sta.mlo)
>=20
> Which Smatch complains about here.

Makes sense, should be "if (sta && sta->sta.mlo)" then I suppose. I'm on
vacation now, so if you want to send a patch as a reminder that'd be
nice.

I might clean this up later - it's not the first time the strange return
behaviour semantics of ieee80211_lookup_ra_sta() have gotten confusing
...

Thanks!

johannes