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: <103f62efc89d86773fbd07729326f3ba08f4ea23.camel@sipsolutions.net>
Subject: Re: [RFC 2/2] wifi: mac80211: use link_id from ieee80211_rx_status
 to retrieve rx link
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Tue, 09 Aug 2022 20:15:20 +0200
In-Reply-To: <20220802065019.20791-3-quic_vthiagar@quicinc.com>
References: <20220802065019.20791-1-quic_vthiagar@quicinc.com>
         <20220802065019.20791-3-quic_vthiagar@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-1.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Tue, 2022-08-02 at 12:20 +0530, Vasanthakumar Thiagarajan wrote:
> Fill rx.link with respective data_link from the reported link_id
> in rx_status. Any link_id > 15 is invalid. Non-MLO connections
> can use either 0 or 15 as the link_id. Please note that link_id
> 0 is used with non-MLO connections to avoid changes in the
> drivers not supporting MLO. For a 802.11 MLD address translated
> frame, driver must report the right link_id for the
> frame to get processed. When processing 802.3 frame format,
> link_id is not that critical, used only with stats update.
> In such case, all the stats will be updated for the deflink.

I think it might be worth splitting this patch a bit different - putting
some parts into the first patch already (that fill rx.link_id), and
keeping some others (statistics etc.) in this patch, which is then more
related to statistics?

>=20
>  	if (rx->link_id >=3D 0) {
> -		link =3D rcu_dereference(rx->sdata->link[rx->link_id]);
> -
> +		link =3D  rcu_dereference(sdata->link[rx->link_id]);

that has some spurious whitespace changes

> +	/*
> +	 * TODO: In MLO, should the frame be dropped if the right link_id is no=
t
> +	 * available? Or may be it is fine in the current form to proceed with
> +	 * the frame processing because with frame being in 802.3 format,
> +	 * link_id is used only for stats purpose and updating the stats on
> +	 * the deflink is fine?
> +	 */
> +	if (pubsta->mlo && status->link_id !=3D IEEE80211_LINK_UNSPECIFIED)
> +		rx.link_id =3D status->link_id;

If the driver *does* give a link ID, it better be valid?

OTOH, could there be races, e.g. while disabling a link?

johannes