From: Gokul Sivakumar <gokulkumar.sivakumar@infineon.com>
To: Johannes Berg <johannes@sipsolutions.net>,
Arend Van Spriel <arend.vanspriel@broadcom.com>
Cc: Ajay Singh <ajay.kathat@microchip.com>,
Claudiu Beznea <claudiu.beznea@tuxon.dev>,
<linux-wireless@vger.kernel.org>, <brcm80211@lists.linux.dev>,
<wlan-kernel-dev-list@infineon.com>
Subject: Re: [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers
Date: Tue, 3 Jun 2025 19:37:24 +0530 [thread overview]
Message-ID: <aD8BaLWyVM5SRSCx@ISCN5CG14747PP.infineon.com> (raw)
In-Reply-To: <19735f0a5c8.279b.9b12b7fc0a3841636cfb5e919b41b954@broadcom.com>
On 06/03, Arend Van Spriel wrote:
> On June 3, 2025 2:29:54 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> > On Tue, 2025-06-03 at 17:42 +0530, Gokul Sivakumar wrote:
> > > >
> > > > > In AP mode, the "ap_isolate" value from the cfg80211 layer represents,
> > > > > 0 = allow low-level bridging of frames between associated stations
> > > > > 1 = restrict low-level bridging of frames to isolate associated stations
> > > > > -1 = do not change existing setting
> > > >
> > > > Is that -1 still true? Seems like now it should just be that
> > > > CFG80211_BSS_PARAM_CHANGED_AP_ISOLATE isn't set and then you don't touch
> > > > it?
> >
> > > Kindly note that the driver is rejecting the SET_BSS operation if an
> > > unsupported AP BSS param is passed by userspace, while the opposite is not
> > > true. ie. the operation would not be rejected by the driver, when a
> > > supported AP BSS param is not passed by the userspace.
> >
> > Sure. That's a different question though.
> >
> > > So yes, the significance of "-1" still holds true, because if suppose the
> > > userspace skipped this param in the SET_BSS request, the driver when
> > > receiving the request will have the ap_isolate param with the default
> > > value "-1". The driver is checking if the param value is >=0 before
> > > proceeding with handling it. And will ignore the param, only if it is -1.
> >
> > Why though? It seems odd. Basically in this case cfg80211 is saying "I'm
> > not changing this parameter" but then anyway you check it's value, why?
yes, if the driver individually checks for the CFG80211_BSS_PARAM_CHANGED_*
flag before handling the corresponding AP BSS param, then the check for the
param value as -1 is not necessary.
> > While we're changing all the drivers, it would seem better to me to just
> > change them in a way that they don't look at the value if the
> > corresponding change flag isn't set?
your suggested approach is fine, will update this and submit a v3 patchset.
> I do have a couple of patches prepped for this, but did want to run few
> tests before sending it out. Now that this is submitted I am not sure what
> to do with that. I can send it as RFC so the idea is clear.
Ok. I have submitted this patchset based on the suggestions in our v1 patch
review discussion, to address johannes's comment using the newly introduced
CHANGED_* flags which are helpful to the drivers.
Gokul
prev parent reply other threads:[~2025-06-03 14:07 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-03 10:32 [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 1/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to driver Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 2/5] wifi: brcmfmac: support AP isolation to restrict reachability between stations Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 3/5] wifi: wil6210: reject SET_BSS if any changed AP BSS param is not supported Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 4/5] wifi: wilc1000: " Gokul Sivakumar
2025-06-03 10:32 ` [PATCH wireless-next v2 5/5] wifi: mac80211: " Gokul Sivakumar
2025-06-03 10:56 ` [PATCH wireless-next v2 0/5] wifi: cfg80211: Add support to indicate changed AP BSS parameters to drivers Johannes Berg
2025-06-03 12:12 ` Gokul Sivakumar
2025-06-03 12:29 ` Johannes Berg
2025-06-03 13:17 ` Arend Van Spriel
2025-06-03 14:07 ` Gokul Sivakumar [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aD8BaLWyVM5SRSCx@ISCN5CG14747PP.infineon.com \
--to=gokulkumar.sivakumar@infineon.com \
--cc=ajay.kathat@microchip.com \
--cc=arend.vanspriel@broadcom.com \
--cc=brcm80211@lists.linux.dev \
--cc=claudiu.beznea@tuxon.dev \
--cc=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=wlan-kernel-dev-list@infineon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox