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: <4ce75ba96bcfe5cd35f524d881124cb0defdb6aa.camel@sipsolutions.net>
Subject: Re: [RFC 4/4] wifi: mac80211: Handle RU Puncturing information
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Muna Sinada <quic_msinada@quicinc.com>
Cc:     linux-wireless@vger.kernel.org
Date:   Thu, 19 Jan 2023 16:40:46 +0100
In-Reply-To: <1670006154-6092-5-git-send-email-quic_msinada@quicinc.com>
References: <1670006154-6092-1-git-send-email-quic_msinada@quicinc.com>
         <1670006154-6092-5-git-send-email-quic_msinada@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.3 (3.46.3-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2022-12-02 at 10:35 -0800, Muna Sinada wrote:
> Handle RU Puncturing information received from user space.
> RU Puncturing bitmap is initially received during
> ieee80211_change_iface() and stored. During AP chanwidth setting,
> the bitmap is validated. In addition driver is notified of new bitmap
> value.


"initially" is a bit of a problem, right?


> @@ -213,6 +213,12 @@ static int ieee80211_change_iface(struct wiphy *wiph=
y,
>  	struct sta_info *sta;
>  	int ret;
> =20
> +	sdata->vif.bss_conf.ru_punct_bitmap =3D params->ru_punct_bitmap;
> +	sdata->vif.bss_conf.ru_punct_bitmap_supp_he =3D params->ru_punct_bitmap=
_supp_he;

I mean this can happen at any point in time due to the way you've wired
it up in nl80211 (which I'm not happy about), and then ... it just gets
ignored.

> +	if (!sdata->vif.bss_conf.ru_punct_bitmap)
> +		sdata_dbg(sdata, "RU Puncturing Bitmap was not set by user\n");
> +

?

> @@ -1251,6 +1257,8 @@ static int ieee80211_start_ap(struct wiphy *wiphy, =
struct net_device *dev,
>  	prev_beacon_int =3D link_conf->beacon_int;
>  	link_conf->beacon_int =3D params->beacon_interval;
> =20
> +	sdata->vif.bss_conf.ru_punct_bitmap =3D dev->ieee80211_ptr->ru_punct_bi=
tmap;

Why grab only one of them here? In fact why grab them at all?

I think you really need to work on the API here.

Also this completely ignores links ... so it's wrong for that too. Must
use link_conf->...

johannes