| Message ID | 1258490898.21197.42.camel@johannes.local |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Nov 18, 2009 at 07:48, Johannes Berg <johannes@sipsolutions.net> wrote: > --- wireless-testing.orig/net/mac80211/iface.c 2009-11-17 14:20:19.000000000 +0100 > +++ wireless-testing/net/mac80211/iface.c 2009-11-17 17:56:08.000000000 +0100 > @@ -745,6 +745,11 @@ int ieee80211_if_change_type(struct ieee > if (type == sdata->vif.type) > return 0; > > + /* if it's part of a bridge, reject changing type to station/ibss */ > + if (sdata->dev->br_port && (type == NL80211_IFTYPE_ADHOC || > + type == NL80211_IFTYPE_STATION)) > + return -EBUSY; Busy doesn't seem like the right error here ... maybe use -EOPNOTSUPP like the next test? Thanks,
On Tue, 17 Nov 2009 21:48:18 +0100 Johannes Berg <johannes@sipsolutions.net> wrote: > A number of people have tried to add a wireless interface > (in managed mode) to a bridge and then complained that it > doesn't work. It cannot work, however, because in 802.11 > networks all packets need to be acknowledged and as such > need to be sent to the right address. Promiscuous doesn't > help here. The wireless address format used for these > links has only space for three addresses, the > * transmitter, which must be equal to the sender (origin) > * receiver (on the wireless medium), which is the AP in > the case of managed mode > * the recipient (destination), which is on the APs local > network segment > > In an IBSS, it is similar, but the receiver and recipient > must match and the third address is used as the BSSID. > > To avoid such mistakes in the future, disallow adding a > wireless interface to a bridge. > > Felix has recently added a four-address mode to the AP > and client side that can be used (after negotiating that > it is possible, which must happen out-of-band by setting > up both sides) for bridging, so allow that case. > > Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Looks good, maybe true four-address mode support will be available more widely, and this will no longer be an issue. Acked-by: Stephen Hemminger <shemminger@vyatta.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2009-11-17 at 14:42 -0800, Stephen Hemminger wrote: > > Felix has recently added a four-address mode to the AP > > and client side that can be used (after negotiating that > > it is possible, which must happen out-of-band by setting > > up both sides) for bridging, so allow that case. > Looks good, maybe true four-address mode support will be available > more widely, and this will no longer be an issue. Doubt it, it breaks the current 802.11 standard, and I think 802.1X too if you use WPA/RSN. Anyway, thanks. I'll take a look tomorrow if we can do this generically in cfg80211, and then send a [PATCH] either way. johannes
On Wed, 2009-11-18 at 09:42 +1100, Julian Calaby wrote: > On Wed, Nov 18, 2009 at 07:48, Johannes Berg <johannes@sipsolutions.net> wrote: > > --- wireless-testing.orig/net/mac80211/iface.c 2009-11-17 14:20:19.000000000 +0100 > > +++ wireless-testing/net/mac80211/iface.c 2009-11-17 17:56:08.000000000 +0100 > > @@ -745,6 +745,11 @@ int ieee80211_if_change_type(struct ieee > > if (type == sdata->vif.type) > > return 0; > > > > + /* if it's part of a bridge, reject changing type to station/ibss */ > > + if (sdata->dev->br_port && (type == NL80211_IFTYPE_ADHOC || > > + type == NL80211_IFTYPE_STATION)) > > + return -EBUSY; > > Busy doesn't seem like the right error here ... maybe use -EOPNOTSUPP > like the next test? Not sure, it's a temporary error and you can fix it by removing it from the bridge, so it's "busy" in the sense that it is fixed to the current mode or any other bridging mode by being in the bridge ... it's not that it doesn't support the mode. johannes
On Wed, Nov 18, 2009 at 09:46, Johannes Berg <johannes@sipsolutions.net> wrote: > On Wed, 2009-11-18 at 09:42 +1100, Julian Calaby wrote: >> On Wed, Nov 18, 2009 at 07:48, Johannes Berg <johannes@sipsolutions.net> wrote: >> > --- wireless-testing.orig/net/mac80211/iface.c 2009-11-17 14:20:19.000000000 +0100 >> > +++ wireless-testing/net/mac80211/iface.c 2009-11-17 17:56:08.000000000 +0100 >> > @@ -745,6 +745,11 @@ int ieee80211_if_change_type(struct ieee >> > if (type == sdata->vif.type) >> > return 0; >> > >> > + /* if it's part of a bridge, reject changing type to station/ibss */ >> > + if (sdata->dev->br_port && (type == NL80211_IFTYPE_ADHOC || >> > + type == NL80211_IFTYPE_STATION)) >> > + return -EBUSY; >> >> Busy doesn't seem like the right error here ... maybe use -EOPNOTSUPP >> like the next test? > > Not sure, it's a temporary error and you can fix it by removing it from > the bridge, so it's "busy" in the sense that it is fixed to the current > mode or any other bridging mode by being in the bridge ... it's not that > it doesn't support the mode. Arguably the test following this (ensuring that we don't set ad-hoc mode on a non-ad-hoc channel) is equally temporary - i.e. both actions require the user to do something before they'll work again. But then, the test after that - for whether the interface is running - returns -EBUSY - and is just as easy to remedy, so........ Thanks,
> A number of people have tried to add a wireless interface > (in managed mode) to a bridge and then complained that it > doesn't work. It cannot work, however, because in 802.11 [...] > To avoid such mistakes in the future, disallow adding a > wireless interface to a bridge. As someone who's been bitten by this, I fully support this change. Still, it makes me wonder: my broadcom-based home-router using the wl.o driver can be set in "client bridge" mode. How does it work? Stefan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 17, 2009 at 08:59:03PM -0500, Stefan Monnier wrote: > > A number of people have tried to add a wireless interface > > (in managed mode) to a bridge and then complained that it > > doesn't work. It cannot work, however, because in 802.11 > [...] > > To avoid such mistakes in the future, disallow adding a > > wireless interface to a bridge. > > As someone who's been bitten by this, I fully support this change. > Still, it makes me wonder: my broadcom-based home-router using the wl.o > driver can be set in "client bridge" mode. How does it work? If I'm not mistaken, that has a bunch of code embedded in it that among other things can do a layer-2 version of NAT to rewrite the MAC adresses for frames on the air. YMMV... John
On Tue, 2009-11-17 at 21:59 -0500, John W. Linville wrote: > > As someone who's been bitten by this, I fully support this change. > > Still, it makes me wonder: my broadcom-based home-router using the wl.o > > driver can be set in "client bridge" mode. How does it work? > > If I'm not mistaken, that has a bunch of code embedded in it that > among other things can do a layer-2 version of NAT to rewrite the > MAC adresses for frames on the air. Yeah, that's how it works. You can probably achieve the same effect with the ebtable_nat module in ebtables but I've never even attempted to try that. johannes
--- wireless-testing.orig/include/linux/if.h 2009-11-17 14:18:36.000000000 +0100 +++ wireless-testing/include/linux/if.h 2009-11-17 14:19:04.000000000 +0100 @@ -70,6 +70,7 @@ #define IFF_XMIT_DST_RELEASE 0x400 /* dev_hard_start_xmit() is allowed to * release skb->dst */ +#define IFF_DONT_BRIDGE 0x800 /* disallow bridging this ether dev */ #define IF_GET_IFACE 0x0001 /* for querying only */ #define IF_GET_PROTO 0x0002 --- wireless-testing.orig/net/bridge/br_if.c 2009-11-17 14:19:17.000000000 +0100 +++ wireless-testing/net/bridge/br_if.c 2009-11-17 15:07:59.000000000 +0100 @@ -390,6 +390,10 @@ int br_add_if(struct net_bridge *br, str if (dev->br_port != NULL) return -EBUSY; + /* No bridging devices that dislike that (e.g. wireless) */ + if (dev->priv_flags & IFF_DONT_BRIDGE) + return -EOPNOTSUPP; + p = new_nbp(br, dev); if (IS_ERR(p)) return PTR_ERR(p); --- wireless-testing.orig/net/mac80211/cfg.c 2009-11-17 14:21:24.000000000 +0100 +++ wireless-testing/net/mac80211/cfg.c 2009-11-17 14:37:13.000000000 +0100 @@ -106,8 +106,15 @@ static int ieee80211_change_iface(struct params->mesh_id_len, params->mesh_id); - if (params->use_4addr >= 0) + if (params->use_4addr >= 0) { sdata->use_4addr = !!params->use_4addr; + sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE; + + if ((sdata->vif.type == NL80211_IFTYPE_STATION || + sdata->vif.type == NL80211_IFTYPE_ADHOC) && + !sdata->use_4addr) + sdata->dev->priv_flags |= IFF_DONT_BRIDGE; + } if (sdata->vif.type != NL80211_IFTYPE_MONITOR || !flags) return 0; --- wireless-testing.orig/net/mac80211/iface.c 2009-11-17 14:20:19.000000000 +0100 +++ wireless-testing/net/mac80211/iface.c 2009-11-17 17:56:08.000000000 +0100 @@ -745,6 +745,11 @@ int ieee80211_if_change_type(struct ieee if (type == sdata->vif.type) return 0; + /* if it's part of a bridge, reject changing type to station/ibss */ + if (sdata->dev->br_port && (type == NL80211_IFTYPE_ADHOC || + type == NL80211_IFTYPE_STATION)) + return -EBUSY; + /* Setting ad-hoc mode on non-IBSS channel is not supported. */ if (sdata->local->oper_channel->flags & IEEE80211_CHAN_NO_IBSS && type == NL80211_IFTYPE_ADHOC) @@ -769,6 +774,11 @@ int ieee80211_if_change_type(struct ieee sdata->local->hw.conf.channel->band); sdata->drop_unencrypted = 0; sdata->use_4addr = 0; + if (sdata->vif.type == NL80211_IFTYPE_STATION || + sdata->vif.type == NL80211_IFTYPE_ADHOC) + sdata->dev->priv_flags |= IFF_DONT_BRIDGE; + else + sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE; return 0; } @@ -843,8 +853,16 @@ int ieee80211_if_add(struct ieee80211_lo params->mesh_id_len, params->mesh_id); - if (params && params->use_4addr >= 0) - sdata->use_4addr = !!params->use_4addr; + if (sdata->vif.type == NL80211_IFTYPE_STATION || + sdata->vif.type == NL80211_IFTYPE_ADHOC) + sdata->dev->priv_flags |= IFF_DONT_BRIDGE; + else + sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE; + + if (params && params->use_4addr > 0) { + sdata->use_4addr = true; + sdata->dev->priv_flags &= ~IFF_DONT_BRIDGE; + } mutex_lock(&local->iflist_mtx); list_add_tail_rcu(&sdata->list, &local->interfaces);
A number of people have tried to add a wireless interface (in managed mode) to a bridge and then complained that it doesn't work. It cannot work, however, because in 802.11 networks all packets need to be acknowledged and as such need to be sent to the right address. Promiscuous doesn't help here. The wireless address format used for these links has only space for three addresses, the * transmitter, which must be equal to the sender (origin) * receiver (on the wireless medium), which is the AP in the case of managed mode * the recipient (destination), which is on the APs local network segment In an IBSS, it is similar, but the receiver and recipient must match and the third address is used as the BSSID. To avoid such mistakes in the future, disallow adding a wireless interface to a bridge. Felix has recently added a four-address mode to the AP and client side that can be used (after negotiating that it is possible, which must happen out-of-band by setting up both sides) for bridging, so allow that case. Signed-off-by: Johannes Berg <johannes@sipsolutions.net> --- v2: * change error code as requested by Michael * disallow changing wireless mode on a bridged iface Should more (all?) of this be in cfg80211? include/linux/if.h | 1 + net/bridge/br_if.c | 4 ++++ net/mac80211/cfg.c | 9 ++++++++- net/mac80211/iface.c | 22 ++++++++++++++++++++-- 4 files changed, 33 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html