[PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Michael Buesch]

[-- Attachment #1: Type: text/plain, Size: 15995 bytes --]

Hi,

Jiri, please review this patch for things that might look
strange to you. :)

--

This replaces the bcm43xx-d80211 virtual interfaces hack by
a correct implementation with support for monitor during oper.

Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h	2006-04-28 16:13:40.000000000 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h	2006-05-01 20:25:31.000000000 +0200
@@ -626,10 +626,34 @@
 	u8 algorithm;
 };
 
+struct bcm43xx_interface {
+	struct list_head list;
+	/* Interface type (IEEE80211_IF_TYPE_XXX). */
+	int type;
+	/* Opaque ID from the ieee80211 subsystem. Do not modify. */
+	int if_id;
+	/* MAC address for this interface. */
+	u8 *mac_addr;
+	/* BSSID (if any). */
+	u8 *bssid;
+};
+
+struct bcm43xx_interface_list {
+	/* Linked list of active interfaces. */
+	struct list_head list;
+	/* Shortcut pointer to the AP interface (if any). */
+	struct bcm43xx_interface *ap_if;
+
+	/* Usage counters of the internal operation modes. */
+	u16 opmode_ap;
+	u16 opmode_adhoc;
+	u16 opmode_monitor;
+	u16 opmode_promisc;
+};
+
 struct bcm43xx_private {
 	struct ieee80211_hw *ieee;
 	struct ieee80211_low_level_stats ieee_stats;
-	int iw_mode;
 
 	struct net_device *net_dev;
 	struct pci_dev *pci_dev;
@@ -653,6 +677,11 @@
 	    short_slot:1,		/* TRUE, if short slot timing is enabled. */
 	    firmware_norelease:1;	/* Do not release the firmware. Used on suspend. */
 
+	/* One physical device can have one or more virtual
+	 * interfaces. This stores and manages the virtual interfaces.
+	 */
+	struct bcm43xx_interface_list interfaces;
+	/* Various statistics about the physical device. */
 	struct bcm43xx_stats stats;
 
 	/* Bus type we are connected to.
@@ -716,8 +745,6 @@
 
 	/* Informational stuff. */
 	char nick[IW_ESSID_MAX_SIZE + 1];
-	u8 bssid[ETH_ALEN];
-	int interfaces;
 
 	/* encryption/decryption */
 	u16 security_offset;
Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_leds.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_leds.c	2006-04-28 16:13:40.000000000 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_leds.c	2006-05-01 18:57:26.000000000 +0200
@@ -217,7 +217,7 @@
 				bcm43xx_led_blink_stop(led, 0);
 			continue;
 		case BCM43xx_LED_APTRANSFER:
-			if (bcm->iw_mode == IW_MODE_MASTER) {
+			if (bcm->interfaces.opmode_ap) {
 				if (transferring) {
 					interval = BCM43xx_LEDBLINK_FAST;
 					turn_on = 1;
Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c	2006-04-28 16:13:40.000000000 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_main.c	2006-05-01 20:25:24.000000000 +0200
@@ -378,18 +378,30 @@
 static void bcm43xx_macfilter_clear(struct bcm43xx_private *bcm,
 				    u16 offset)
 {
-	const u8 zero_addr[ETH_ALEN] = { 0 };
+	static const u8 zero_addr[ETH_ALEN] = { 0 };
 
 	bcm43xx_macfilter_set(bcm, offset, zero_addr);
 }
 
 static void bcm43xx_write_mac_bssid_templates(struct bcm43xx_private *bcm)
 {
+	static const u8 zero_addr[ETH_ALEN] = { 0 };
+	struct bcm43xx_interface *iface;
 	const u8 *mac = (const u8 *)(bcm->net_dev->dev_addr);
-	const u8 *bssid = bcm->bssid;
+	const u8 *bssid = NULL;
 	u8 mac_bssid[ETH_ALEN * 2];
 	int i;
 
+	list_for_each_entry(iface, &bcm->interfaces.list, list) {
+		if (iface->type != IEEE80211_IF_TYPE_MNTR &&
+		    iface->bssid) {
+			bssid = iface->bssid;
+			break;
+		}
+	}
+	if (!bssid)
+		bssid = zero_addr;
+
 	memcpy(mac_bssid, mac, ETH_ALEN);
 	memcpy(mac_bssid + ETH_ALEN, bssid, ETH_ALEN);
 
@@ -1438,15 +1450,15 @@
 
 static void handle_irq_ps(struct bcm43xx_private *bcm)
 {
-	if (bcm->iw_mode == IW_MODE_MASTER) {
+	if (bcm->interfaces.opmode_ap) {
 		///TODO: PS TBTT
 	} else {
 		if (1/*FIXME: the last PSpoll frame was sent successfully */)
 			bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
 	}
-	if (bcm->iw_mode == IW_MODE_ADHOC)
+	bcm->reg124_set_0x4 = 0;
+	if (bcm->interfaces.opmode_adhoc)
 		bcm->reg124_set_0x4 = 1;
-	//FIXME else set to false?
 }
 
 static void handle_irq_reg124(struct bcm43xx_private *bcm)
@@ -1456,7 +1468,6 @@
 	bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS2_BITFIELD,
 			bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS2_BITFIELD)
 			| 0x4);
-	//FIXME: reset reg124_set_0x4 to false?
 }
 
 static void handle_irq_pmq(struct bcm43xx_private *bcm)
@@ -1509,7 +1520,10 @@
 		 * Request the 80211 subsystem to generate a new beacon
 		 * frame and use it as template.
 		 */
-		bcm->cached_beacon = ieee80211_beacon_get(bcm->net_dev, 0, &control);
+		assert(bcm->interfaces.ap_if);
+		bcm->cached_beacon = ieee80211_beacon_get(bcm->net_dev,
+							  bcm->interfaces.ap_if->if_id,
+							  &control);
 		if (unlikely(!bcm->cached_beacon)) {
 			dprintkl(KERN_WARNING PFX "Could not generate beacon template.\n");
 			goto ack;
@@ -1603,7 +1617,7 @@
 	}
 
 	if (reason & BCM43xx_IRQ_BEACON) {
-		if (bcm->iw_mode == IW_MODE_MASTER)
+		if (bcm->interfaces.opmode_ap)
 			handle_irq_beacon(bcm);
 		bcmirq_handled(BCM43xx_IRQ_BEACON);
 	}
@@ -2147,55 +2161,35 @@
 	printkl(KERN_ERR PFX "MAC suspend failed\n");
 }
 
-void bcm43xx_set_iwmode(struct bcm43xx_private *bcm,
-			int iw_mode)
+static void bcm43xx_select_opmodes(struct bcm43xx_private *bcm)
 {
-	struct net_device *net_dev = bcm->net_dev;
 	u32 status;
 	u16 value;
 
-	bcm->iw_mode = iw_mode;
-	if (iw_mode == IW_MODE_MONITOR)
-		net_dev->type = ARPHRD_IEEE80211;
-	else
-		net_dev->type = ARPHRD_ETHER;
-
 	status = bcm43xx_read32(bcm, BCM43xx_MMIO_STATUS_BITFIELD);
-	/* Reset status to infrastructured mode */
-	status &= ~(BCM43xx_SBF_MODE_AP | BCM43xx_SBF_MODE_MONITOR);
+	/* Reset status to default STA mode */
+	status &= ~BCM43xx_SBF_MODE_AP;
+	status &= ~BCM43xx_SBF_MODE_MONITOR;
 	status &= ~BCM43xx_SBF_MODE_PROMISC;
 	status |= BCM43xx_SBF_MODE_NOTADHOC;
 
+	if (bcm->interfaces.opmode_ap)
+		status |= BCM43xx_SBF_MODE_AP;
+	if (bcm->interfaces.opmode_adhoc)
+		status &= ~BCM43xx_SBF_MODE_NOTADHOC;
+	if (bcm->interfaces.opmode_monitor)
+		status |= BCM43xx_SBF_MODE_MONITOR;
+	if (bcm->interfaces.opmode_promisc)
+		status |= BCM43xx_SBF_MODE_PROMISC;
+
 /* FIXME: Always enable promisc mode, until we get the MAC filters working correctly. */
 status |= BCM43xx_SBF_MODE_PROMISC;
 
-	switch (iw_mode) {
-	case IW_MODE_MONITOR:
-		status |= BCM43xx_SBF_MODE_MONITOR;
-		status |= BCM43xx_SBF_MODE_PROMISC;
-		break;
-	case IW_MODE_ADHOC:
-		status &= ~BCM43xx_SBF_MODE_NOTADHOC;
-		break;
-	case IW_MODE_MASTER:
-		status |= BCM43xx_SBF_MODE_AP;
-		break;
-	case IW_MODE_SECOND:
-	case IW_MODE_REPEAT:
-		TODO(); /* TODO */
-		break;
-	case IW_MODE_INFRA:
-		/* nothing to be done here... */
-		break;
-	default:
-		dprintk(KERN_ERR PFX "Unknown mode in set_iwmode: %d\n", iw_mode);
-	}
-	if (net_dev->flags & IFF_PROMISC)
-		status |= BCM43xx_SBF_MODE_PROMISC;
 	bcm43xx_write32(bcm, BCM43xx_MMIO_STATUS_BITFIELD, status);
 
 	value = 0x0002;
-	if (iw_mode != IW_MODE_ADHOC && iw_mode != IW_MODE_MASTER) {
+	if (bcm->interfaces.opmode_adhoc == 0 &&
+	    bcm->interfaces.opmode_ap == 0) {
 		if (bcm->chip_id == 0x4306 && bcm->chip_rev == 3)
 			value = 0x0064;
 		else
@@ -2291,7 +2285,7 @@
 	bcm43xx_shm_write16(bcm, BCM43xx_SHM_SHARED, 0x0074, 0x0000);
 
 	/* Initially set the wireless operation mode. */
-	bcm43xx_set_iwmode(bcm, bcm->iw_mode);
+	bcm43xx_select_opmodes(bcm);
 
 	if (bcm->current_core->rev < 3) {
 		bcm43xx_write16(bcm, 0x060E, 0x0000);
@@ -2599,27 +2593,6 @@
 	return err;
 }
 
-static void bcm43xx_gen_bssid(struct bcm43xx_private *bcm)
-{
-	const u8 *mac = (const u8*)(bcm->net_dev->dev_addr);
-	u8 *bssid = bcm->bssid;
-
-	switch (bcm->iw_mode) {
-	case IW_MODE_ADHOC:
-		random_ether_addr(bssid);
-		break;
-	case IW_MODE_MASTER:
-	case IW_MODE_INFRA:
-	case IW_MODE_REPEAT:
-	case IW_MODE_SECOND:
-	case IW_MODE_MONITOR:
-		memcpy(bssid, mac, ETH_ALEN);
-		break;
-	default:
-		assert(0);
-	}
-}
-
 static void bcm43xx_rate_memory_write(struct bcm43xx_private *bcm,
 				      u16 rate,
 				      int is_ofdm)
@@ -2746,7 +2719,6 @@
 	/* Maximum Contention Window */
 	bcm43xx_shm_write32(bcm, BCM43xx_SHM_WIRELESS, 0x0004, 0x000003ff);
 
-	bcm43xx_gen_bssid(bcm);
 	bcm43xx_write_mac_bssid_templates(bcm);
 
 	if (bcm->current_core->rev >= 5)
@@ -4169,13 +4141,8 @@
 static int bcm43xx_net_open(struct net_device *net_dev)
 {
 	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
-	int res;
 
-	res = bcm43xx_init_board(bcm);
-	if (!res)
-		return res;
-	bcm43xx_set_iwmode(bcm, bcm->iw_mode);
-	return 0;
+	return bcm43xx_init_board(bcm);
 }
 
 static int bcm43xx_net_stop(struct net_device *net_dev)
@@ -4194,33 +4161,170 @@
 				 struct ieee80211_if_init_conf *conf)
 {
 	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
+	unsigned long flags;
+	int err = -ENOBUFS;
+	struct bcm43xx_interface *iface;
 
-	if (bcm->interfaces > 0)
-		return -ENOBUFS;
-	if (conf->type == IEEE80211_IF_TYPE_MNTR) {
-		bcm->iw_mode = IW_MODE_MONITOR;
-	} else {
-		if (memcmp(bcm->net_dev->dev_addr, conf->mac_addr, ETH_ALEN) != 0)
-			return -EADDRNOTAVAIL;
-		if (conf->type == IEEE80211_IF_TYPE_STA)
-			bcm->iw_mode = IW_MODE_INFRA;
-		else if (conf->type == IEEE80211_IF_TYPE_IBSS)
-			bcm->iw_mode = IW_MODE_ADHOC;
-		else if (conf->type == IEEE80211_IF_TYPE_AP)
-			bcm->iw_mode = IW_MODE_MASTER;
-		else
-			return -EOPNOTSUPP;
+	iface = kzalloc(sizeof(*iface), GFP_KERNEL);
+	if (!iface)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&iface->list);
+	iface->type = conf->type;
+	iface->if_id = conf->if_id;
+	iface->mac_addr = conf->mac_addr;
+
+	bcm43xx_lock_mmio(bcm, flags);
+	switch (conf->type) {
+	case IEEE80211_IF_TYPE_AP:
+		/* Cannot run more than one AP or concurrently
+		 * with IBSS mode.
+		 */
+		if (bcm->interfaces.opmode_ap)
+			break;
+		if (bcm->interfaces.opmode_adhoc)
+			break;
+		bcm->interfaces.opmode_ap++;
+		bcm->interfaces.ap_if = iface;
+		err = 0;
+		break;
+	case IEEE80211_IF_TYPE_STA:
+		/* Cannot run STA and AP or IBSS mode concurrently. */
+		if (bcm->interfaces.opmode_ap)
+			break;
+		if (bcm->interfaces.opmode_adhoc)
+			break;
+		err = 0;
+		break;
+	case IEEE80211_IF_TYPE_IBSS:
+		/* Cannot run more than one IBSS or concurrently
+		 * with AP mode.
+		 */
+		if (bcm->interfaces.opmode_ap)
+			break;
+		if (bcm->interfaces.opmode_adhoc)
+			break;
+		bcm->interfaces.opmode_adhoc++;
+		err = 0;
+		break;
+	case IEEE80211_IF_TYPE_MNTR:
+		bcm->interfaces.opmode_monitor++;
+		err = 0;
+		break;
+	case IEEE80211_IF_TYPE_WDS:
+		//TODO: Uhm..., well.
+		break;
+	default:
+		assert(0);
 	}
-	bcm->interfaces++;
-	return 0;
+	if (!err) {
+		list_add_tail(&iface->list, &bcm->interfaces.list);
+		if (bcm->initialized)
+			bcm43xx_select_opmodes(bcm);
+
+		dprintk(KERN_INFO PFX "Virtual interface added "
+				      "(type: 0x%08X, ID: %d, MAC: "
+				      BCM43xx_MACFMT ")\n",
+			conf->type, conf->if_id,
+			BCM43xx_MACARG(conf->mac_addr));
+	}
+
+	bcm43xx_unlock_mmio(bcm, flags);
+
+	if (err)
+		kfree(iface);
+
+	return err;
 }
 
 static void bcm43xx_remove_interface(struct net_device *net_dev,
 				     struct ieee80211_if_init_conf *conf)
 {
 	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
+	unsigned long flags;
+	struct bcm43xx_interface *iface, *tmp_iface;
+
+	bcm43xx_lock_mmio(bcm, flags);
+	list_for_each_entry_safe(iface, tmp_iface, &bcm->interfaces.list, list) {
+		if (iface->if_id != conf->if_id)
+			continue;
+		assert(conf->type == iface->type);
+
+		switch (iface->type) {
+		case IEEE80211_IF_TYPE_AP:
+			bcm->interfaces.opmode_ap--;
+			assert(bcm->interfaces.opmode_ap == 0);
+			bcm->interfaces.ap_if = NULL;
+			break;
+		case IEEE80211_IF_TYPE_STA:
+			break;
+		case IEEE80211_IF_TYPE_IBSS:
+			bcm->interfaces.opmode_adhoc--;
+			assert(bcm->interfaces.opmode_adhoc == 0);
+			break;
+		case IEEE80211_IF_TYPE_MNTR:
+			assert(bcm->interfaces.opmode_monitor > 0);
+			bcm->interfaces.opmode_monitor--;
+			break;
+		case IEEE80211_IF_TYPE_WDS:
+			//TODO: Uhm..., well.
+			break;
+		default:
+			assert(0);
+		}
+		list_del(&iface->list);
+		kfree(iface);
+		if (bcm->initialized)
+			bcm43xx_select_opmodes(bcm);
+
+		dprintk(KERN_INFO PFX "Virtual interface removed "
+				      "(type: 0x%08X, ID: %d, MAC: "
+				      BCM43xx_MACFMT ")\n",
+			conf->type, conf->if_id,
+			BCM43xx_MACARG(conf->mac_addr));
+		break;
+	}
+	bcm43xx_unlock_mmio(bcm, flags);
+}
+
+static int bcm43xx_config_interface(struct net_device *net_dev,
+				    int if_id,
+				    struct ieee80211_if_conf *conf)
+{
+	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
+	unsigned long flags;
+	struct bcm43xx_interface *iface;
+
+	bcm43xx_lock_mmio(bcm, flags);
+	list_for_each_entry(iface, &bcm->interfaces.list, list) {
+		if (iface->if_id == if_id)
+			goto found;
+	}
+	bcm43xx_unlock_mmio(bcm, flags);
+
+	return -ENODEV;
+found:
+	iface->bssid = conf->bssid;
+	//TODO
+
+	return 0;
+}
+
+static void bcm43xx_set_multicast_list(struct net_device *net_dev,
+				       unsigned short netflags,
+				       int mc_count)
+{
+	struct bcm43xx_private *bcm = bcm43xx_priv(net_dev);
+	unsigned long flags;
+	u16 old_promisc;
 
-	bcm->interfaces--;
+	bcm43xx_lock_mmio(bcm, flags);
+	old_promisc = bcm->interfaces.opmode_promisc;
+	bcm->interfaces.opmode_promisc = !!(netflags & IFF_PROMISC);
+	if (bcm->interfaces.opmode_promisc != old_promisc) {
+		if (bcm->initialized)
+			bcm43xx_select_opmodes(bcm);
+	}
+	bcm43xx_unlock_mmio(bcm, flags);
 }
 
 /* Initialization of struct net_device, just after allocation. */
@@ -4240,6 +4344,7 @@
 	int err;
 
 	bcm->ieee = ieee;
+	INIT_LIST_HEAD(&bcm->interfaces.list);
 	bcm->irq_savedstate = BCM43xx_IRQ_INITIAL;
 	bcm->pci_dev = pci_dev;
 	bcm->net_dev = net_dev;
@@ -4294,6 +4399,8 @@
 	ieee->name = KBUILD_MODNAME;
 	ieee->host_gen_beacon = 1;
 	ieee->rx_includes_fcs = 1;
+	ieee->monitor_during_oper = 1;
+	ieee->channel_change_time = 20000;
 	ieee->tx = bcm43xx_net_hard_start_xmit;
 	ieee->open = bcm43xx_net_open;
 	ieee->stop = bcm43xx_net_stop;
@@ -4301,6 +4408,8 @@
 	ieee->remove_interface = bcm43xx_remove_interface;
 	ieee->reset = bcm43xx_net_reset;
 	ieee->config = bcm43xx_net_config;
+	ieee->config_interface = bcm43xx_config_interface;
+	ieee->set_multicast_list = bcm43xx_set_multicast_list;
 //TODO	ieee->set_key = bcm43xx_net_set_key;
 	ieee->get_stats = bcm43xx_net_get_stats;
 	ieee->queues = 1;
Index: wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_phy.c
===================================================================
--- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx_phy.c	2006-04-28 16:13:40.000000000 +0200
+++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx_phy.c	2006-05-01 18:56:17.000000000 +0200
@@ -94,7 +94,7 @@
 		bcm43xx_mac_suspend(bcm);
 		spin_lock(&phy->lock);
 	} else {
-		if (bcm->iw_mode != IW_MODE_MASTER)
+		if (bcm->interfaces.opmode_ap == 0)
 			bcm43xx_power_saving_ctl_bits(bcm, -1, 1);
 	}
 	phy->is_locked = 1;
@@ -111,7 +111,7 @@
 			bcm43xx_mac_enable(bcm);
 		}
 	} else {
-		if (bcm->iw_mode != IW_MODE_MASTER)
+		if (bcm->interfaces.opmode_ap == 0)
 			bcm43xx_power_saving_ctl_bits(bcm, -1, -1);
 	}
 	phy->is_locked = 0;


-- 
Greetings Michael.

[-- Attachment #2: Type: application/pgp-signature, Size: 191 bytes --]

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Jiri Benc]

On Mon, 1 May 2006 21:35:00 +0200, Michael Buesch wrote:
> --- wireless-dev.orig/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h	2006-04-28 16:13:40.000000000 +0200
> +++ wireless-dev/drivers/net/wireless/d80211/bcm43xx/bcm43xx.h	2006-05-01 20:25:31.000000000 +0200
> @@ -626,10 +626,34 @@
>  	u8 algorithm;
>  };
>  
> +struct bcm43xx_interface {
> +	struct list_head list;
> +	/* Interface type (IEEE80211_IF_TYPE_XXX). */
> +	int type;
> +	/* Opaque ID from the ieee80211 subsystem. Do not modify. */
> +	int if_id;
> +	/* MAC address for this interface. */
> +	u8 *mac_addr;
> +	/* BSSID (if any). */
> +	u8 *bssid;
> +};
> +
> +struct bcm43xx_interface_list {
> +	/* Linked list of active interfaces. */
> +	struct list_head list;
> +	/* Shortcut pointer to the AP interface (if any). */
> +	struct bcm43xx_interface *ap_if;
> +
> +	/* Usage counters of the internal operation modes. */
> +	u16 opmode_ap;
> +	u16 opmode_adhoc;
> +	u16 opmode_monitor;
> +	u16 opmode_promisc;
> +};
> +

This is unnecessary. AFAIK bcm43xx hardware doesn't support more than one
interface at a time (not counting monitor interfaces as those are somewhat
special). There is no need for the linked list.

Instead of those structures above, use something like this:

struct bcm43xx_interface {
	/* Interface type (IEEE80211_IF_TYPE_XXX). */
	int type;
	/* MAC address for this interface. */
	u8 *mac_addr;
	/* BSSID (if any). */
	u8 *bssid;
	/* Number of monitor interfaces. */
	int monitors;
	/* STA, IBSS, AP or WDS running? */
	int oper;
}

You need this structure just once per hw card.

In add_interface callback:
- If conf->type is MNTR:
	- If bcm43xx_interface->oper is FALSE and
	  bcm43xx_interface->monitors is 0, initialize the card.
	- If bcm43xx_interface->monitors is 0, switch the card to monitor
	  mode (enable promisc mode, enable receiving of control frames,
	  etc.)
	- Increase bcm43xx_interface->monitors.
- If conf->type != MNTR:
	- If bcm43xx_interface->oper is TRUE, return error.
	- Set bcm43xx_interface->oper to TRUE.
	- Set bcm43xx_interface->type to conf->type.
	- Set hw MAC address according to conf->mac_addr.
	- If bcm43xx_interface->monitors is 0, initialize the card.

In remove_interface callback:
- If conf->type is MNTR:
	- Decrease bcm43xx_interface->monitors.
	- If bcm43xx_interface->monitors is 0, switch the card from monitor
	  mode (disable promisc mode if not forced by set_multicast_list,
	  disable receiving of control frames, etc.)
	- If bcm43xx_interface->oper is FALSE and
	  bcm43xx_interface->monitors is 0, deinitialize the card.
- If conf->type != MNTR:
	- Set bcm43xx_interface->oper to FALSE.
	- If bcm43xx_interface->monitors is 0, deinitialize the card.

In config_interface callback:
- If conf->type is MNTR, please report a bug :-)
- Assume you are configuring the only interface you have running.

> [...]
> +	iface = kzalloc(sizeof(*iface), GFP_KERNEL);
> +	if (!iface)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&iface->list);
> +	iface->type = conf->type;
> +	iface->if_id = conf->if_id;
> +	iface->mac_addr = conf->mac_addr;
> +
> +	bcm43xx_lock_mmio(bcm, flags);
> +	switch (conf->type) {
> +	case IEEE80211_IF_TYPE_AP:
> +		/* Cannot run more than one AP or concurrently
> +		 * with IBSS mode.
> +		 */
> +		if (bcm->interfaces.opmode_ap)
> +			break;
> +		if (bcm->interfaces.opmode_adhoc)
> +			break;
> +		bcm->interfaces.opmode_ap++;
> +		bcm->interfaces.ap_if = iface;
> +		err = 0;
> +		break;
> +	case IEEE80211_IF_TYPE_STA:
> +		/* Cannot run STA and AP or IBSS mode concurrently. */
> +		if (bcm->interfaces.opmode_ap)
> +			break;
> +		if (bcm->interfaces.opmode_adhoc)
> +			break;
> +		err = 0;
> +		break;

"ifconfig ap up ; ifconfig sta up" will succeed, while
"ifconfig sta up ; ifconfig ap up" won't.

Also, you cannot allow multiple STA interfaces, unless you can configure the
card to ack frames destined to several MAC addresses.

Nevertheless, this is all addressed by the solution suggested above.

> +	case IEEE80211_IF_TYPE_IBSS:
> +		/* Cannot run more than one IBSS or concurrently
> +		 * with AP mode.
> +		 */
> +		if (bcm->interfaces.opmode_ap)
> +			break;
> +		if (bcm->interfaces.opmode_adhoc)
> +			break;
> +		bcm->interfaces.opmode_adhoc++;
> +		err = 0;
> +		break;
> +	case IEEE80211_IF_TYPE_MNTR:
> +		bcm->interfaces.opmode_monitor++;
> +		err = 0;
> +		break;
> +	case IEEE80211_IF_TYPE_WDS:
> +		//TODO: Uhm..., well.
> +		break;

Please return more appropriate error code (-EINVAL perhaps?).

> +	default:
> +		assert(0);
>  	}

Thanks,

-- 
Jiri Benc
SUSE Labs

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Marcus Better]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jiri Benc wrote:
> This is unnecessary. AFAIK bcm43xx hardware doesn't support more than one
> interface at a time (not counting monitor interfaces as those are somewhat
> special).

I wonder what exactly is causing this limitation? Why isn't it just a matter
of software support?

Marcus

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)

iD8DBQFEWlUrXjXn6TzcAQkRAoa6AJ97kI8F7Q42orjNQC6HBIXwj3I/DwCgux18
RBkhEcOFBDtQGEHFq2eMxKU=
=3WoL
-----END PGP SIGNATURE-----

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Ivo van Doorn]

[-- Attachment #1: Type: text/plain, Size: 1071 bytes --]

On Thursday 4 May 2006 21:26, Marcus Better wrote:
> Jiri Benc wrote:
> > This is unnecessary. AFAIK bcm43xx hardware doesn't support more than one
> > interface at a time (not counting monitor interfaces as those are somewhat
> > special).
> 
> I wonder what exactly is causing this limitation? Why isn't it just a matter
> of software support?

Hardware limitations probably. For example the Ralink devices.
The MAC address must be written into the registers for correct behaviour.
When multiple virtual interfaces are going to be used, the device
should be working under multiple MAC addresses. While the device
could be tricked into doing that (I believe rt61 and rt73 drivers from Ralink
are making such an attempt) it could give quite some overhead when for
each frame that is being send the driver should sort out from which
virtual interface it came, write the matching MAC address to the register,
and hope that the device will like it when the MAC address is changed so often
and the RX is not being affected because of some hardware filter.

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Jiri Benc]

Thu, 4 May 2006 22:55:58 +0200, Ivo van Doorn pise:
> On Thursday 4 May 2006 21:26, Marcus Better wrote:
> > I wonder what exactly is causing this limitation? Why isn't it just a
> > matter of software support?
> 
> Hardware limitations probably. For example the Ralink devices.
> The MAC address must be written into the registers for correct behaviour.
> When multiple virtual interfaces are going to be used, the device
> should be working under multiple MAC addresses. While the device
> could be tricked into doing that (I believe rt61 and rt73 drivers from
> Ralink are making such an attempt) it could give quite some overhead when
> for each frame that is being send the driver should sort out from which
> virtual interface it came, write the matching MAC address to the register,
> and hope that the device will like it when the MAC address is changed so
> often and the RX is not being affected because of some hardware filter.

The actual problem is with receiving. ACK frames need to be sent exactly
after SIFS interval (10 or 16 microseconds depending on a PHY, +/- 10%)
after last bit of the frame was received. This means it cannot be done in a
software. You need to tell the hardware about MAC address you are using and
hardware will ACK all frames destined to that address (well, actually only
that received with valid FCS, but it doesn't matter). And most hardware
don't allow to specify more MAC addresses.

 Jiri

-- 
Jiri Benc
SUSE Labs

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Ulrich Kunitz]

> The actual problem is with receiving. ACK frames need to be sent exactly
> after SIFS interval (10 or 16 microseconds depending on a PHY, +/- 10%)
> after last bit of the frame was received. This means it cannot be done in a
> software. You need to tell the hardware about MAC address you are using and
> hardware will ACK all frames destined to that address (well, actually only
> that received with valid FCS, but it doesn't matter). And most hardware
> don't allow to specify more MAC addresses.
> 
>  Jiri

Good explanation. ZD1211 allows to specify a second RX address, I
wondered, what this is good for. So for ZD1211 we could support
two virtual interfaces. However in AP mode one virtual device
would have to send the beacons on its own. But there could be also
support for one interface in AP and the other in STA mode.

Uli
-- 
Ulrich Kunitz - kune@deine-taler.de

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]

On Fri, 2006-05-05 at 07:45 +0200, Ulrich Kunitz wrote:
>  But there could be also
> support for one interface in AP and the other in STA mode.

That's actually a much more interesting use case for WDS.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 793 bytes --]

Re: [PATCH] bcm43xx-d80211: proper implementation of virtual interface support

[Jiri Benc]

On Fri, 05 May 2006 12:15:40 +0200, Johannes Berg wrote:
> On Fri, 2006-05-05 at 07:45 +0200, Ulrich Kunitz wrote:
> >  But there could be also
> > support for one interface in AP and the other in STA mode.
> 
> That's actually a much more interesting use case for WDS.

You don't need different MAC address for WDS.

 Jiri

-- 
Jiri Benc
SUSE Labs