On 4/24/2025 12:15 AM, Johannes Berg wrote:
On Wed, 2025-04-23 at 23:21 +0530, Gokul Sivakumar wrote:
+static int brcmf_cfg80211_change_bss(struct wiphy *wiphy, struct net_device *dev,
+ struct bss_parameters *params)
+{
+ struct brcmf_if *ifp = netdev_priv(dev);
+ int ret = 0;
+
+ /* In AP mode, the "ap_isolate" value 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
+ */
+ if (params->ap_isolate >= 0) {
+ ret = brcmf_fil_iovar_int_set(ifp, "ap_isolate", params->ap_isolate);
+ if (ret < 0)
+ brcmf_err("ap_isolate iovar failed: ret=%d\n", ret);
+ }
+
+ return ret;
+}
Seems like a terrible idea to accept any other changes silently without
doing anything at all.
Hi Johannes,
Agree. That would indeed give the wrong impression to user-space.
However, what if the firmware does not support some of them that
user-space actually want to change. Seems like we are missing a feedback
mechanism here to inform user-space about partial failure to apply the
requested parameters?
Looked at other drivers implementing this callback and here are the results:
[wil6210] wil_cfg80211_change_bss(): does exactly the same thing.
[wilc1000] change_bss(): worse! it accepts everything and does nothing.
[rtl8723bs] cfg80211_rtw_change_bss(): same. just an empty callback.
[mac80211] ieee80211_change_bss(): not surprising this looks pretty good
The mac80211 implementation fills a changed bitmask, but that is to
inform the mac80211 driver what configuration changes to look for.
Also, please pay attention to the linux-wireless list. Like, at all. We
started using tree tags months ago, we started using a different subject
prefix _years_ ago.
If this patch means Infineon is (mildly) regaining interest in upstream
wifi development let's not discourage them. I do watch the
linux-wireless list on occasion but I am a bit lost on your remark. What
do you mean by tree tags. You mean the "wifi:" prefix? But then I am
confused about the "subject prefix" remark.
Digging a bit further maybe you are referring to the "Tree labels"
section [1]? I always considered a patch with only [PATCH] as being for
-next implicitly. If it makes maintainer life easier I am happy to
comply and add it explicit ;-)
Regards,
Arend
[1]
https://wireless.docs.kernel.org/en/latest/en/developers/documentation/submittingpatches.html#tree-labels
