On Wed, Feb 10, 2021 at 10:07:42AM +0100, Janusz Dziedzic wrote: > wt., 9 lut 2021 o 10:26 Janusz Dziedzic <janusz.dziedzic@xxxxxxxxx> napisał(a): > > pon., 8 lut 2021 o 18:30 Raphaël Mélotte <raphael.melotte@xxxxxxx> napisał(a): > > > If 4addr mode is already enabled, the call to enable it a second time > > > may fail. If this happens when roaming, it leads to deauthentication. What is the case where 4ddr mode would have already been enabled? Does this happen every time when there is roaming since multi_ap_set_4addr_mode() did not have this check before? Or are you considering cases where something external to wpa_supplicant is changing this parameter? > > > - if (wpa_drv_set_4addr_mode(wpa_s, 1) < 0) { > > > - wpa_printf(MSG_ERROR, "Failed to set 4addr mode"); > > > - goto fail; > > > + if (wpa_s->enabled_4addr_mode == 0) { > > > + if (wpa_drv_set_4addr_mode(wpa_s, 1) < 0) { > > > + wpa_printf(MSG_ERROR, "Failed to set 4addr mode"); > > > + goto fail; > > > + } > > > + wpa_s->enabled_4addr_mode = 1; > > > } > > > - wpa_s->enabled_4addr_mode = 1; This looks safer to me if it is clear that no external entity is messing up with the kernel parameter. > > Today set_4addr_mode(1) fail with EBUSY if netdev is already in the bridge. > > So, maybe we should fix it in cfg80211? > > janusz@t2:~$ sudo ifconfig wlp1s0 up > > janusz@t2:~$ sudo iw wlp1s0 set 4addr on > > janusz@t2:~$ sudo iw wlp1s0 set 4addr on > > janusz@t2:~$ sudo brctl addbr br0 > > janusz@t2:~$ sudo brctl addif br0 wlp1s0 > > janusz@t2:~$ sudo iw wlp1s0 set 4addr on > > command failed: Device or resource busy (-16) That specific EBUSY seems unnecessary for a no-change operation, but disabling 4addr mode in station interface that is in a bridge should still be prevented. > Did fast patch (didn't check code deeply yet - just check set 4addr > when busy issuey, seems works correctly): > diff --git a/net/wireless/util.c b/net/wireless/util.c > @@ -1027,6 +1027,7 @@ int cfg80211_change_iface(struct > cfg80211_registered_device *rdev, > > /* if it's part of a bridge, reject changing type to station/ibss */ > if (netif_is_bridge_port(dev) && > + (ntype != otype) && > (ntype == NL80211_IFTYPE_ADHOC || > ntype == NL80211_IFTYPE_STATION || > ntype == NL80211_IFTYPE_P2P_CLIENT)) How would that prevent disabling of 4addr mode when the netdev is in a bridge? > For old kernels/cfg80211 maybe we should introduce > wpa_drv_get_4addr_mode() and set only if required? Do we really need that complexity? Isn't the internal tracking with wpa_s->enabled_4addr_mode sufficient? In other words, simply apply this patch as-is.. -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap