[patch] d80211: remove sub-interface mac address policy

[patch] d80211: remove sub-interface mac address policy

[David Kimdon]

[-- Attachment #1: remove_mac_addr_policy.patch --]
[-- Type: text/plain, Size: 2102 bytes --]

Wireless vlan interfaces need to have the same mac address as
other sub interfaces.  Rather than complicate the kernel here by
adding yet another case where uniqueness is not required, remove
the check on mac address uniqueness altogether.

We should not implement a mac address allocation policy here.  It
is difficult to get it right in all cases and does not belong in
the kernel.  It is better to leave this to be implemented as a
userspace policy.

Signed-off-by: David Kimdon <david.kimdon@devicescape.com>

Index: linux-2.6.16/net/d80211/ieee80211.c
===================================================================
--- linux-2.6.16.orig/net/d80211/ieee80211.c
+++ linux-2.6.16/net/d80211/ieee80211.c
@@ -2116,17 +2116,6 @@ static struct net_device_stats *ieee8021
 	return &(sdata->stats);
 }
 
-static inline int identical_mac_addr_allowed(int type1, int type2)
-{
-	return (type1 == IEEE80211_IF_TYPE_MNTR ||
-		type2 == IEEE80211_IF_TYPE_MNTR ||
-		(type1 == IEEE80211_IF_TYPE_AP &&
-		 type2 == IEEE80211_IF_TYPE_WDS) ||
-		(type1 == IEEE80211_IF_TYPE_WDS &&
-		 (type2 == IEEE80211_IF_TYPE_WDS ||
-		  type2 == IEEE80211_IF_TYPE_AP)));
-}
-
 static int ieee80211_master_open(struct net_device *dev)
 {
 	struct ieee80211_local *local = dev->ieee80211_ptr;
@@ -2202,22 +2191,12 @@ static void ieee80211_start_hard_monitor
 
 static int ieee80211_open(struct net_device *dev)
 {
-	struct ieee80211_sub_if_data *sdata, *nsdata;
+	struct ieee80211_sub_if_data *sdata;
 	struct ieee80211_local *local = dev->ieee80211_ptr;
 	struct ieee80211_if_init_conf conf;
 	int res;
 
 	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	list_for_each_entry(nsdata, &local->sub_if_list, list) {
-		struct net_device *ndev = nsdata->dev;
-
-		if (ndev != dev && ndev != local->mdev &&
-		    netif_running(ndev) &&
-		    memcmp(dev->dev_addr, ndev->dev_addr, ETH_ALEN) == 0 &&
-		    !identical_mac_addr_allowed(sdata->type, nsdata->type)) {
-			return -ENOTUNIQ;
-		}
-	}
 	if (sdata->type == IEEE80211_IF_TYPE_WDS &&
 	    memcmp(sdata->u.wds.remote_addr, "\0\0\0\0\0\0", ETH_ALEN) == 0)
 		return -ENOLINK;

--

Re: [patch] d80211: remove sub-interface mac address policy

[Jiri Benc]

On Thu, 14 Sep 2006 07:33:21 -0700, David Kimdon wrote:
> Wireless vlan interfaces need to have the same mac address as
> other sub interfaces.  Rather than complicate the kernel here by
> adding yet another case where uniqueness is not required, remove
> the check on mac address uniqueness altogether.
> 
> We should not implement a mac address allocation policy here.  It
> is difficult to get it right in all cases and does not belong in
> the kernel.  It is better to leave this to be implemented as a
> userspace policy.

I disagree. This is not about policy, this is about prevention of
accidental violation of IEEE 802.11. The only effect of this patch would
be forcing drivers to do that check themselves thus duplicating code.

Actually, I'd like to add support for the common case (based on a flag in
ieee80211_hw) when WDS is _required_ to have the same MAC address as an AP
interface. That will complicate things even more in the stack but save a
lot of duplicated code in drivers.

I agree that getting this to work with wireless vlans is not easy.
Actually, I'm convinced I broke vlan support at several other places too.
It's really hard not to break it if you know nothing about it.

What is the purpose of "wireless vlans"? Is it something Atheros-specific?
Is it documented somewhere? Or is it in some of IEEE 802.11 standards and I
just overlooked it? (In that case I would be more than happy to review the
whole stack and fix it.) Could you send us some pointers?

Thanks,

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [patch] d80211: remove sub-interface mac address policy

[David Kimdon]

On Thu, Sep 21, 2006 at 08:09:39PM +0200, Jiri Benc wrote:
> On Thu, 14 Sep 2006 07:33:21 -0700, David Kimdon wrote:
> > Wireless vlan interfaces need to have the same mac address as
> > other sub interfaces.  Rather than complicate the kernel here by
> > adding yet another case where uniqueness is not required, remove
> > the check on mac address uniqueness altogether.
> > 
> > We should not implement a mac address allocation policy here.  It
> > is difficult to get it right in all cases and does not belong in
> > the kernel.  It is better to leave this to be implemented as a
> > userspace policy.
> 
> I disagree. This is not about policy, this is about prevention of
> accidental violation of IEEE 802.11. The only effect of this patch would
> be forcing drivers to do that check themselves thus duplicating code.

That is fine, I don't feel strongly, I will cook up a patch that fixes
the check for vlan interfaces.

> What is the purpose of "wireless vlans"?

We use wireless vlans to isolate stations to separate multicast
domains within the same bss based on the input of a radius server.  A
stations is bound to a particular wireless vlan interface, all
stations on that wireless vlan share broadcast keys, and the wireless
vlan interface can be bridged to a particular wired vlan.

> Is it something Atheros-specific?
no, it is chip driver agnostic.

> Is it documented somewhere?  Or is it in some of IEEE 802.11
> standards and I just overlooked it? (In that case I would be more
> than happy to review the whole stack and fix it.) Could you send us
> some pointers?

Take a look at http://www.ietf.org/rfc/rfc3580.txt section 3.31.
Tunnel Attributes.  That is what we are processing at a high level to
get vlan assignment.  FWIW, we have code on its way to hostapd cvs
that uses wireless vlans which will show more details of this
particular implementation.

David