[RFC] b43: multiple MAC addresses support

[RFC] b43: multiple MAC addresses support

[Johannes Berg]

This patch adds support for multiple virtual interfaces (up to 65536)
to b43. This requires modified firmware which is detected by an invalid
day in the firmware timestamp (254), and then the firmware time and the
rest of the date is used for feature bits.

The multi-MAC support works by having a mask of ignored bits, the
modified firmware will mask off these bits before comparing the address
on incoming frames with the address. This, however, means that when you
add three MAC virtual interfaces, the firmware will at least also ACK
frames on a fourth address. The required mask is calculated dynamically
to be the least restrictive mask possible, but you should take care to
allocate your MAC addresses in a way that minimises the mask to avoid
strange behaviour. For example, avoid using addresses 02:00:00:00:00:02
and 02:00:00:00:00:10 together because these two have only 46 bits in
common, use 02:00:00:00:00:12 and 02:00:00:00:00:10 instead as those
have 47 bits in common.

Note that the mask applies only to the lower 16 bits, so the first four
bytes of all addresses need to be identical.

The required tool to patch the firmware will be published separately.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Michael Wu <flamingice@sourmilk.net>
Cc: Jiri Benc <jbenc@suse.cz>
Cc: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/b43/b43.h  |   19 ++-
 drivers/net/wireless/b43/main.c |  200 +++++++++++++++++++++++++++++++++-------
 drivers/net/wireless/b43/xmit.c |    6 -
 3 files changed, 180 insertions(+), 45 deletions(-)

--- everything.orig/drivers/net/wireless/b43/main.c	2007-11-09 01:53:47.158996039 +0100
+++ everything/drivers/net/wireless/b43/main.c	2007-11-09 02:04:31.358978135 +0100
@@ -543,7 +543,21 @@ static void b43_write_mac_bssid_template
 static void b43_upload_card_macaddress(struct b43_wldev *dev)
 {
 	b43_write_mac_bssid_templates(dev);
-	b43_macfilter_set(dev, B43_MACFILTER_SELF, dev->wl->mac_addr);
+
+	if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA) {
+		u16 tmp;
+
+		tmp = ((u16 *)dev->wl->mac_addr)[0];
+		b43_shm_write16(dev, B43_SHM_SHARED, 0x320, cpu_to_le16(tmp));
+		tmp = ((u16 *)dev->wl->mac_addr)[1];
+		b43_shm_write16(dev, B43_SHM_SHARED, 0x322, cpu_to_le16(tmp));
+		tmp = ((u16 *)dev->wl->mac_addr)[2];
+		b43_shm_write16(dev, B43_SHM_SHARED, 0x324, cpu_to_le16(tmp));
+		/* swap here because we always treat the mask as big endian */
+		b43_shm_write16(dev, B43_SHM_SHARED, 0x326,
+				swab16(dev->wl->mac_mask));
+	} else
+		b43_macfilter_set(dev, B43_MACFILTER_SELF, dev->wl->mac_addr);
 }
 
 static void b43_set_slot_time(struct b43_wldev *dev, u16 slot_time)
@@ -1174,18 +1188,18 @@ static void b43_write_probe_resp_plcp(st
 {
 	struct b43_plcp_hdr4 plcp;
 	u32 tmp;
-	__le16 dur;
 
 	plcp.data = 0;
 	b43_generate_plcp_hdr(&plcp, size + FCS_LEN, rate);
-	dur = ieee80211_generic_frame_duration(dev->wl->hw,
-					       dev->wl->if_id, size,
-					       B43_RATE_TO_BASE100KBPS(rate));
 	/* Write PLCP in two parts and timing for packet transfer */
 	tmp = le32_to_cpu(plcp.data);
 	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset, tmp & 0xFFFF);
 	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 2, tmp >> 16);
-	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 6, le16_to_cpu(dur));
+	/*
+	 * XXX: currently, the duration here doesn't matter, but
+	 * when we implement offload it needs to be fixed!
+	 */
+	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 6, 0);
 }
 
 /* Instead of using custom probe response template, this function
@@ -1200,7 +1214,6 @@ static u8 *b43_generate_probe_resp(struc
 	const u8 *src_data;
 	u8 *dest_data;
 	u16 src_size, elem_size, src_pos, dest_pos;
-	__le16 dur;
 	struct ieee80211_hdr *hdr;
 
 	B43_WARN_ON(!dev->cached_beacon);
@@ -1235,10 +1248,11 @@ static u8 *b43_generate_probe_resp(struc
 	/* Set the frame control. */
 	hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
 					 IEEE80211_STYPE_PROBE_RESP);
-	dur = ieee80211_generic_frame_duration(dev->wl->hw,
-					       dev->wl->if_id, *dest_size,
-					       B43_RATE_TO_BASE100KBPS(rate));
-	hdr->duration_id = dur;
+	/*
+	 * XXX: currently, the duration here doesn't matter, but
+	 * when we implement offload it needs to be fixed!
+	 */
+	hdr->duration_id = 0;
 
 	return dest_data;
 }
@@ -1792,11 +1806,23 @@ static int b43_upload_microcode(struct b
 		err = -EOPNOTSUPP;
 		goto out;
 	}
-	b43dbg(dev->wl, "Loading firmware version %u.%u "
-	       "(20%.2i-%.2i-%.2i %.2i:%.2i:%.2i)\n",
-	       fwrev, fwpatch,
-	       (fwdate >> 12) & 0xF, (fwdate >> 8) & 0xF, fwdate & 0xFF,
-	       (fwtime >> 11) & 0x1F, (fwtime >> 5) & 0x3F, fwtime & 0x1F);
+	/* special day 254: modified firmware with feature bits */
+	if ((fwdate & 0xFF) == 0xFE) {
+		b43dbg(dev->wl, "Loading firmware version %u.%u\n",
+		       fwrev, fwpatch);
+		dev->fw.features = fwtime | ((fwdate & 0xFF00) << 8);
+		b43dbg(dev->wl, "This firmware was modified and has the"
+				" added features 0x%06x\n", dev->fw.features);
+		if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA)
+			b43dbg(dev->wl, " * station multi MAC addr support\n");
+	} else
+		b43dbg(dev->wl, "Loading firmware version %u.%u "
+		       "(20%.2i-%.2i-%.2i %.2i:%.2i:%.2i)\n",
+		       fwrev, fwpatch,
+		       (fwdate >> 12) & 0xF, (fwdate >> 8) & 0xF,
+		       fwdate & 0xFF,
+		       (fwtime >> 11) & 0x1F, (fwtime >> 5) & 0x3F,
+		       fwtime & 0x1F);
 
 	dev->fw.rev = fwrev;
 	dev->fw.patch = fwpatch;
@@ -2948,11 +2974,18 @@ static int b43_op_config_interface(struc
 		return -ENODEV;
 	mutex_lock(&wl->mutex);
 	spin_lock_irqsave(&wl->irq_lock, flags);
-	B43_WARN_ON(wl->if_id != if_id);
-	if (conf->bssid)
-		memcpy(wl->bssid, conf->bssid, ETH_ALEN);
-	else
-		memset(wl->bssid, 0, ETH_ALEN);
+
+	/*
+	 * Only take bssid of first iface, we can only adopt time from
+	 * one BSS so arbitrarily choose the first.
+	 */
+	if (wl->if_id == if_id) {
+		if (conf->bssid)
+			memcpy(wl->bssid, conf->bssid, ETH_ALEN);
+		else
+			memset(wl->bssid, 0, ETH_ALEN);
+	}
+
 	if (b43_status(dev) >= B43_STAT_INITIALIZED) {
 		if (b43_is_mode(wl, IEEE80211_IF_TYPE_AP)) {
 			B43_WARN_ON(conf->type != IEEE80211_IF_TYPE_AP);
@@ -3424,6 +3457,71 @@ static int b43_wireless_core_init(struct
 	return err;
 }
 
+struct mac_iter {
+	struct b43_wl *wl;
+	u16 mac_val;
+	bool add;
+	int count;
+};
+
+static void b43_mac_iter(void *data, u8 *mac, int if_id)
+{
+	struct mac_iter *iter = data;
+	u16 val = (mac[4] << 8) | mac[5];
+	u16 cur = (iter->wl->mac_addr[4] << 8) | iter->wl->mac_addr[5];
+
+	/* skip the one that's being removed */
+	if (!iter->add && val == iter->mac_val)
+		return;
+
+	/* take first address we find as base */
+	if (iter->count == 0) {
+		cur = val;
+		iter->wl->mac_addr[4] = cur >> 8;
+		iter->wl->mac_addr[5] = cur;
+	}
+
+	iter->wl->mac_mask |= cur ^ val;
+
+	iter->count++;
+}
+
+static void b43_adjust_mac_addr_mask(struct b43_wl *wl, bool add, u8 *addr)
+{
+	struct mac_iter iter;
+	DECLARE_MAC_BUF(mbuf);
+	u16 cur;
+	u16 val = (addr[4] << 8) | addr[5];
+
+	/*
+	 * Because this is not invoked on adding the first address,
+	 * we do not need to have any special code for that case.
+	 * If it was invoked, we'd have to set cur to val here etc.
+	 * like above in the iter->count == 0 case (because we don't
+	 * see the interface that's being added in the iteration.)
+	 */
+
+	wl->mac_mask = 0;
+
+	iter.wl = wl;
+	iter.add = add;
+	iter.mac_val = val;
+	iter.count = 0;
+	ieee80211_iterate_active_interfaces(wl->hw, b43_mac_iter, &iter);
+
+	/* add the one that's being added */
+	cur = (wl->mac_addr[4] << 8) | wl->mac_addr[5];
+	if (add)
+		wl->mac_mask |= cur ^ val;
+
+	cur = cur & ~wl->mac_mask;
+	wl->mac_addr[4] = cur >> 8;
+	wl->mac_addr[5] = cur;
+
+	b43dbg(wl, "MAC address is %s, mask is 0x%.4x\n",
+	       print_mac(mbuf, wl->mac_addr), wl->mac_mask);
+}
+
 static int b43_op_add_interface(struct ieee80211_hw *hw,
 				struct ieee80211_if_init_conf *conf)
 {
@@ -3441,16 +3539,36 @@ static int b43_op_add_interface(struct i
 		return -EOPNOTSUPP;
 
 	mutex_lock(&wl->mutex);
-	if (wl->operating)
-		goto out_mutex_unlock;
+	dev = wl->current_dev;
 
-	b43dbg(wl, "Adding Interface type %d\n", conf->type);
+	if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA) {
+		if (wl->if_count && conf->type != IEEE80211_IF_TYPE_STA)
+			goto out_mutex_unlock;
+		if (wl->if_count > 0) {
+			if (((u8 *)conf->mac_addr)[0] != wl->mac_addr[0])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[1] != wl->mac_addr[1])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[2] != wl->mac_addr[2])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[3] != wl->mac_addr[3])
+				goto out_mutex_unlock;
+			b43_adjust_mac_addr_mask(wl, 1, conf->mac_addr);
+		}
+	} else {
+		if (wl->if_count)
+			goto out_mutex_unlock;
+	}
 
-	dev = wl->current_dev;
-	wl->operating = 1;
-	wl->if_id = conf->if_id;
-	wl->if_type = conf->type;
-	memcpy(wl->mac_addr, conf->mac_addr, ETH_ALEN);
+	wl->if_count++;
+
+	if (wl->if_count == 1) {
+		wl->if_id = conf->if_id;
+		wl->if_type = conf->type;
+		memcpy(wl->mac_addr, conf->mac_addr, ETH_ALEN);
+	}
+
+	b43dbg(wl, "Adding Interface type %d\n", conf->type);
 
 	spin_lock_irqsave(&wl->irq_lock, flags);
 	b43_adjust_opmode(dev);
@@ -3464,6 +3582,14 @@ static int b43_op_add_interface(struct i
 	return err;
 }
 
+static void b43_ifid_iter(void *data, u8 *mac, int if_id)
+{
+	struct b43_wl *wl = data;
+
+	if (wl->if_id != if_id)
+		wl->if_id = if_id;
+}
+
 static void b43_op_remove_interface(struct ieee80211_hw *hw,
 				    struct ieee80211_if_init_conf *conf)
 {
@@ -3475,14 +3601,22 @@ static void b43_op_remove_interface(stru
 
 	mutex_lock(&wl->mutex);
 
-	B43_WARN_ON(!wl->operating);
-	B43_WARN_ON(wl->if_id != conf->if_id);
+	B43_WARN_ON(!wl->if_count);
+
+	wl->if_count--;
+	if (wl->if_count == 0) {
+		wl->if_type = IEEE80211_IF_TYPE_INVALID;
+		memset(wl->mac_addr, 0, ETH_ALEN);
+	}
+
+	/* arbitrarily adopt another if_id as main if_id */
+	if (conf->if_id == wl->if_id)
+		ieee80211_iterate_active_interfaces(wl->hw, b43_ifid_iter, wl);
 
-	wl->operating = 0;
+	b43_adjust_mac_addr_mask(wl, 0, conf->mac_addr);
 
 	spin_lock_irqsave(&wl->irq_lock, flags);
 	b43_adjust_opmode(dev);
-	memset(wl->mac_addr, 0, ETH_ALEN);
 	b43_upload_card_macaddress(dev);
 	spin_unlock_irqrestore(&wl->irq_lock, flags);
 
--- everything.orig/drivers/net/wireless/b43/b43.h	2007-11-09 01:53:47.248981228 +0100
+++ everything/drivers/net/wireless/b43/b43.h	2007-11-09 02:06:41.818973253 +0100
@@ -601,22 +601,20 @@ struct b43_wl {
 	struct mutex mutex;
 	spinlock_t leds_lock;
 
-	/* We can only have one operating interface (802.11 core)
-	 * at a time. General information about this interface follows.
-	 */
-
-	/* Opaque ID of the operating interface from the ieee80211
+	/*
+	 * Opaque ID of the first operating interface from the ieee80211
 	 * subsystem. Do not modify.
 	 */
 	int if_id;
-	/* The MAC address of the operating interface. */
+	/* The MAC address */
 	u8 mac_addr[ETH_ALEN];
+	u16 mac_mask;
 	/* Current BSSID */
 	u8 bssid[ETH_ALEN];
 	/* Interface type. (IEEE80211_IF_TYPE_XXX) */
 	int if_type;
-	/* Is the card operating in AP, STA or IBSS mode? */
-	bool operating;
+	/* number of open interfaces */
+	int if_count;
 	/* filter flags */
 	unsigned int filter_flags;
 	/* Stats about the wireless interface */
@@ -648,6 +646,9 @@ struct b43_firmware {
 	u16 rev;
 	/* Firmware patchlevel */
 	u16 patch;
+	/* Firmware features (24 bits) */
+#define B43_FW_FEATURE_MULTI_MAC_STA	0x000001
+	u32 features;
 };
 
 /* Device (802.11 core) initialization status. */
@@ -781,7 +782,7 @@ static inline struct b43_wldev *dev_to_b
 /* Is the device operating in a specified mode (IEEE80211_IF_TYPE_XXX). */
 static inline int b43_is_mode(struct b43_wl *wl, int type)
 {
-	return (wl->operating && wl->if_type == type);
+	return (wl->if_count && wl->if_type == type);
 }
 
 static inline u16 b43_read16(struct b43_wldev *dev, u16 offset)
--- everything.orig/drivers/net/wireless/b43/xmit.c	2007-11-09 01:53:47.378984104 +0100
+++ everything/drivers/net/wireless/b43/xmit.c	2007-11-09 01:53:57.698983181 +0100
@@ -221,7 +221,7 @@ static void generate_txhdr_fw4(struct b4
 	} else {
 		int fbrate_base100kbps = B43_RATE_TO_BASE100KBPS(rate_fb);
 		txhdr->dur_fb = ieee80211_generic_frame_duration(dev->wl->hw,
-								 dev->wl->if_id,
+								 txctl->ifindex,
 								 fragment_len,
 								 fbrate_base100kbps);
 	}
@@ -311,7 +311,7 @@ static void generate_txhdr_fw4(struct b4
 		rts_rate_fb_ofdm = b43_is_ofdm_rate(rts_rate_fb);
 
 		if (txctl->flags & IEEE80211_TXCTL_USE_CTS_PROTECT) {
-			ieee80211_ctstoself_get(dev->wl->hw, dev->wl->if_id,
+			ieee80211_ctstoself_get(dev->wl->hw, txctl->ifindex,
 						fragment_data, fragment_len,
 						txctl,
 						(struct ieee80211_cts *)(txhdr->
@@ -319,7 +319,7 @@ static void generate_txhdr_fw4(struct b4
 			mac_ctl |= B43_TX4_MAC_SENDCTS;
 			len = sizeof(struct ieee80211_cts);
 		} else {
-			ieee80211_rts_get(dev->wl->hw, dev->wl->if_id,
+			ieee80211_rts_get(dev->wl->hw, txctl->ifindex,
 					  fragment_data, fragment_len, txctl,
 					  (struct ieee80211_rts *)(txhdr->
 								   rts_frame));

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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

> This patch adds support for multiple virtual interfaces (up to 65536)
> to b43. This requires modified firmware which is detected by an invalid
> day in the firmware timestamp (254), and then the firmware time and the
> rest of the date is used for feature bits.

One thing I forgot to mention is that currently, mac80211 is not able to
associate different virtual stations with the same BSSID. I had a patch
to fix this but because I had so many patches I dropped that one. I'll
try to revive it next week if I get around, basically it consisted of
adding an sdata pointer to the station get routine. The bigger problem
was making the RX path happy about it :)

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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

So as you can see from the code, I only allowed station interfaces for
now. Which is actually pretty useless since you can't use two of them to
associate to the same AP. It also violates the IEEE 802.11 specification
as far as I can tell because we can only adopt the time of one BSS...

This is a good stepping stone, however. I've shown that the firmware can
easily be modified to support address masks, so we can now think about
how multi-BSS AP operation can work.

Right now, I'm thinking we should do it this way:

As for the firmware, I'd disable support for TIM manipulation and make
it take the beacon from the multicast/broadcast frame queue instead of
from the template RAM. Essentially, at beacon time, the firmware would
send the first frame from the mc/bc queue (the beacon) and then all
other frames from the mc/bc queue (the mc/bc traffic) up to the "last
bc/mc frame ID" where we clear the more data bit. No special-casing of
the DTIM period etc, we let the driver handle all that.

We should limit multi-BSS operation to some sane value, say 16, and
require that the mask is the last four bits or less. This way, we can
use the lowest four bits of the mac address as a beacon ID and have
different "last bc/mc frame ID"s for the different beacons to ease
driver operation and make it race-free.

In the driver, for such modified firmware, we rely on mac80211's
software broadcast/multicast queuing feature along with the get_beacon()
function. Every time the firmware indicates that it has sent a beacon,
the driver will queue the next beacon (round-robin between the
interfaces) and the corresponding multicast traffic.

Something I'm not clear on right now is the timing of it all. If there
are say two BSSes, the driver can simply program the hardware to half
the beacon period and then the beacons will alternate. But if there are
seven BSSes, the beacon period might not be divisible by seven so timing
errors will happen and, worse yet, accumulate. Does hostapd somehow
account for this? Or should the beaconing work completely differently,
for example with all N beacons sent in a burst?

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Jouni Malinen]

On Fri, Nov 09, 2007 at 11:52:30AM +0100, Johannes Berg wrote:

> Something I'm not clear on right now is the timing of it all. If there
> are say two BSSes, the driver can simply program the hardware to half
> the beacon period and then the beacons will alternate. But if there are
> seven BSSes, the beacon period might not be divisible by seven so timing
> errors will happen and, worse yet, accumulate. Does hostapd somehow
> account for this? Or should the beaconing work completely differently,
> for example with all N beacons sent in a burst?

You could always modify the beacon period to a suitable number that
would be divisible by whatever number of BSSes there are and/or add
empty slot to move from, say, 7 to 8, and just not send a beacon at some
of the slots. Sending a burst of all beacons at the same time is also a
valid option up to a certain limit on how long a time would be allocated
for this. However, this may have some timing issues when there are
multiple power save buffered broadcast/multicast frames waiting to be
sent out. In general, it would be good to try to send the beacon frames
as close to their correct time as possible to allow power saving
stations to remain asleep most of the time. Some low-level operation
would also need to make sure the beacon timestamp is correct if the
frame gets delayed due to other frames.

I think it would be perfectly fine to try to simply the configuration
and implementation by requiring all BSSes to use the same beacon
interval and allow small changes in the interval, if needed. hostapd
does not have code for this type of modifications. I've done some
experiments with configuring the hardware to use beacon interval divided
by number of BSSes, but ended up using burst of beacon frames instead
(and all BSSes were forced to use the same beacon interval).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC] b43: multiple MAC addresses support

[Kalle Valo]

Jouni Malinen <j@w1.fi> writes:

> On Fri, Nov 09, 2007 at 11:52:30AM +0100, Johannes Berg wrote:
>
>> Something I'm not clear on right now is the timing of it all. If there
>> are say two BSSes, the driver can simply program the hardware to half
>> the beacon period and then the beacons will alternate. But if there are
>> seven BSSes, the beacon period might not be divisible by seven so timing
>> errors will happen and, worse yet, accumulate. Does hostapd somehow
>> account for this? Or should the beaconing work completely differently,
>> for example with all N beacons sent in a burst?
>
> You could always modify the beacon period to a suitable number that
> would be divisible by whatever number of BSSes there are and/or add
> empty slot to move from, say, 7 to 8, and just not send a beacon at some
> of the slots. Sending a burst of all beacons at the same time is also a
> valid option up to a certain limit on how long a time would be allocated
> for this. However, this may have some timing issues when there are
> multiple power save buffered broadcast/multicast frames waiting to be
> sent out. In general, it would be good to try to send the beacon frames
> as close to their correct time as possible to allow power saving
> stations to remain asleep most of the time.

Exactly, it is important for small mobile devices that beacons are
sent at the correct time. For example, I have seen broken APs which
Target Beacon Transmission Time fluctuates and Nokia N800 has high
power consumption because of that.

-- 
Kalle Valo

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


On Mon, 2007-11-12 at 13:14 +0200, Kalle Valo wrote:

> Exactly, it is important for small mobile devices that beacons are
> sent at the correct time. For example, I have seen broken APs which
> Target Beacon Transmission Time fluctuates and Nokia N800 has high
> power consumption because of that.

The N800 and similar devices only wake up at DTIM beacons, right? So
another idea would be to burst beacons but make sure the DTIM beacon
time doesn't fluctuate away from the DTIM TBTT. This would be possible
by changing the order of the beacons and adjusting the DTIM count so
that the beacon with DTIM count zero is always the first.

Noting beacon(BSS,DTIM count) you'd send:

 - at TBTT 123: beacon(A,1), beacon(B,2), beacon(C,3)
 - at TBTT 124: beacon(A,0), beacon(B,1), beacon(C,2)
 - at TBTT 125: beacon(B,0), beacon(C,1), beacon(A,3)
 - at TBTT 126: beacon(C,0), beacon(A,2), beacon(B,3)

etc.

The actual restrictions this imposes are very complicated to describe,
easier to describe and implement are the restrictions

 (a) identical beacon periods
 (b) identical DTIM periods
 (c) DTIM period > number of BSSes

that result in a subset of the possible values being allowed. Slight
relaxations could be allowed, identical DTIM periods are not really
required when it is possible to schedule them in a non-conflicting way,
i.e. (I think) when the sum over all 1/(DTIM period) is less or equal to
one (all fit) and the gcd of all is not one (possible to schedule in a
non-conflicting way).

[An example: You could have four BSSes A-D, with DTIM periods 2, 4, 8
and 16 respectively. The DTIM beacon scheduling would be
 A, B, A, C, A, B, A, D, A, B, A, C, A, B, A, - (repeat)
with all the other beacons at each period sent after the DTIM beacon.

In the empty slot ("-") you could insert another BSS with DTIM period 16
or a multiple of 16.]

[The actual restrictions don't even require using identical beacon
periods as long as there's a not too small value that divides all beacon
periods. This is very hard to describe formally.]

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Mon, 2007-11-12 at 13:14 +0200, Kalle Valo wrote:
>
>> Exactly, it is important for small mobile devices that beacons are
>> sent at the correct time. For example, I have seen broken APs which
>> Target Beacon Transmission Time fluctuates and Nokia N800 has high
>> power consumption because of that.
>
> The N800 and similar devices only wake up at DTIM beacons, right?

At least N800 and N810 do, we don't want to miss any of broadcast and
multicast traffic. But I don't know how other mobile devices wakeup
for beacons.

> So another idea would be to burst beacons but make sure the DTIM
> beacon time doesn't fluctuate away from the DTIM TBTT. This would be
> possible by changing the order of the beacons and adjusting the DTIM
> count so that the beacon with DTIM count zero is always the first.

At least from my point of view that could possible. Of course it feels
like a bit of a hack, but I guess there's no clean way to implement
this.

-- 
Kalle Valo

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> > The N800 and similar devices only wake up at DTIM beacons, right?
> 
> At least N800 and N810 do, we don't want to miss any of broadcast and
> multicast traffic. But I don't know how other mobile devices wakeup
> for beacons.

Well, if they wake up for all beacons they're using too much power
anyway so I'm not concerned.

> At least from my point of view that could possible. Of course it feels
> like a bit of a hack, but I guess there's no clean way to implement
> this.

Well the scheme with distributing the beacons would make it seem a lot
more like it was implemented in different MACs, but that's quite a bit
harder to implement.

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Jouni Malinen]

On Tue, Nov 13, 2007 at 12:26:57PM +0100, Johannes Berg wrote:

> The N800 and similar devices only wake up at DTIM beacons, right? So
> another idea would be to burst beacons but make sure the DTIM beacon
> time doesn't fluctuate away from the DTIM TBTT. This would be possible
> by changing the order of the beacons and adjusting the DTIM count so
> that the beacon with DTIM count zero is always the first.

This could be an improvement for small number of BSSes, but if the DTIM
period starts going up, the delay in sending out multicast/broadcast
frames may not be acceptable anymore depending on application.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


On Sun, 2007-11-18 at 15:02 -0800, Jouni Malinen wrote:
> On Tue, Nov 13, 2007 at 12:26:57PM +0100, Johannes Berg wrote:
> 
> > The N800 and similar devices only wake up at DTIM beacons, right? So
> > another idea would be to burst beacons but make sure the DTIM beacon
> > time doesn't fluctuate away from the DTIM TBTT. This would be possible
> > by changing the order of the beacons and adjusting the DTIM count so
> > that the beacon with DTIM count zero is always the first.
> 
> This could be an improvement for small number of BSSes, but if the DTIM
> period starts going up, the delay in sending out multicast/broadcast
> frames may not be acceptable anymore depending on application.

Well even if you have a larger number of BSSes you can spread the DTIm 0
beacons across the periods so that you have a few but not all DTIM 0
beacons at one time. And that's still an improvement over currently used
schemes.

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> Exactly, it is important for small mobile devices that beacons are
> sent at the correct time. For example, I have seen broken APs which
> Target Beacon Transmission Time fluctuates and Nokia N800 has high
> power consumption because of that.

The APs at university do this, they're Airespace according to the MAC
address and they just burst beacon1,2,3, wait, burst beacon1,2,3 etc.
They also send all DTIM 0 beacons at the same burst so force PS STAs to
wake up at TBTT and wait for the right beacon...

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> You could always modify the beacon period to a suitable number that
> would be divisible by whatever number of BSSes there are and/or add
> empty slot to move from, say, 7 to 8, and just not send a beacon at some
> of the slots.

Hmm. I'd have said that yes, this works, but rereading the spec I'm not
too certain any more. But more below.

> Sending a burst of all beacons at the same time is also a
> valid option up to a certain limit on how long a time would be allocated
> for this. However, this may have some timing issues when there are
> multiple power save buffered broadcast/multicast frames waiting to be
> sent out.

That's the point I wasn't quite sure on, would it be valid to send the
frames in the order beacon1, beacon2, beacon3, mc1, mc1, ..., mc2, ...,
mc3, ...?

[slight reordering of your mail]

> Some low-level operation
> would also need to make sure the beacon timestamp is correct if the
> frame gets delayed due to other frames.

On Broadcom hardware, I'm pretty sure that the transmit engine updates
the timestamp only during transmission so that shouldn't be a problem.

> In general, it would be good to try to send the beacon frames
> as close to their correct time as possible to allow power saving
> stations to remain asleep most of the time. 

Time zero is defined as TBTT, so this means we need to send the frame as
close to TU "Beacon Interval" * N where N is a non-negative number. Is
my interpretation correct here? If so, and because we only have a single
TSF, doesn't this mean that we should send the beacons all together as
quickly as possible? If we tried spreading them out we'd be sending them
at something like "Beacon Interval" * (N + 0.25) which isn't something
we should do, no?

Another thing that I just realised is that I'm not sure whether we
update the TSF in probe responses correctly or at all because as far as
I can tell it's only done for the probe response microcode offload.
We'll have to look into this.

Actually, what we can do is adjust the offset for each of the beacons
differently. So when we're sending a beacon at TSF "interval * (N+.25)"
we can round that up to "interval * (N+1)" by adjusting the TSF write
offset. This means that stations will adopt a TSF that is larger than
ours by a bit. If we'd do the same for probe responses (which should be
fairly easy), would the timing work out? I'm not entirely clear about
CFP operation etc.

Effectively, only one of our BSSes would be sending out our own TSF, the
other ones would be sending something up to say 0.875 of the beacon
interval larger (assuming we limit the number of BSSes to eight). All
STAs would adopt this time, but what influence does that have on other
operation?

> I think it would be perfectly fine to try to simply the configuration
> and implementation by requiring all BSSes to use the same beacon
> interval and allow small changes in the interval, if needed. hostapd
> does not have code for this type of modifications.

I think I can do pretty much whatever we want, including send TSF
offsets as described above. But I'm not entirely clear on the
side-effects of that. If the TSF offsets are possible without
significant side effects then we're pretty much free to do whatever we
want in software (under some constraints), in the microcode I'd probably
simply pull the broadcast/multicast FIFO at the beacon interval and send
whatever software put into it and allow for extended frame control wrt.
timestamping for beacons/probe responses.

> I've done some
> experiments with configuring the hardware to use beacon interval divided
> by number of BSSes, but ended up using burst of beacon frames instead
> (and all BSSes were forced to use the same beacon interval).

Do you remember any reason?

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Jouni Malinen]

On Mon, Nov 12, 2007 at 01:02:31PM +0100, Johannes Berg wrote:

> That's the point I wasn't quite sure on, would it be valid to send the
> frames in the order beacon1, beacon2, beacon3, mc1, mc1, ..., mc2, ...,
> mc3, ...?

Well, the multicast/broadcast frames would be sent after DTIM beacon
using normal frame transmission rules, so this would be possible order.
Though, this might not be that likely if one were to consider a case
where these APs are on separate radios (i.e., multiple STAs contenting
for the medium) should there be large number of buffered frames.

While this may end up being the easiest and cleanest way of implementing
multi-BSS support, this has somewhat unfortunate drawbacks for power
save stations (e.g., STAs associated with the AP1 would need to remain
awake for the extra time needed to send beacon2 and beacon3; and STAs
associated with the AP3 would need to remain awake while mc1..mc2 frames
are sent).


> On Broadcom hardware, I'm pretty sure that the transmit engine updates
> the timestamp only during transmission so that shouldn't be a problem.

OK. This will allow the STAs to remain in sync.

> Time zero is defined as TBTT, so this means we need to send the frame as
> close to TU "Beacon Interval" * N where N is a non-negative number. Is
> my interpretation correct here? If so, and because we only have a single
> TSF, doesn't this mean that we should send the beacons all together as
> quickly as possible? If we tried spreading them out we'd be sending them
> at something like "Beacon Interval" * (N + 0.25) which isn't something
> we should do, no?

TBTT could (and well, should) be separate for each BSS. In other words,
if there are four virtual BSSes using one radio (BSS1 .. BSS4), TBTT
should be different for each. If the beacon frames are send out as a
burst, TBTT2 should be TBTT1 + <time needed to send beacon1> and so on..
Similarly, there should be independent TSF for each BSS. However,
implementing this for virtual BSS (single radio) designs is going to be
quite difficult since the hardware/firmware is unlikely to support
something like this..

Consequently, the only realistic mechanism may end up being to try to
send the beacon frames as close to each other as possible. I'm not sure
whether non-AP STAs would be able to handle offset from the N *
beacon_int TUs time and still sleep properly. In theory, they could try
to sync with previous beacons, but this would be difficult to do since
beacon transmission times can vary a bit. This would be simple if we
would not need to think about power saving, but reality is likely to be
much more complex and the safest solution would to just try to send the
frame as close as possible after the N * beacon_int time.

> Another thing that I just realised is that I'm not sure whether we
> update the TSF in probe responses correctly or at all because as far as
> I can tell it's only done for the probe response microcode offload.
> We'll have to look into this.

This should be done in the same way as for Beacons, i.e., by whatever
low-level functionality is available in the transmitting path.

> Actually, what we can do is adjust the offset for each of the beacons
> differently. So when we're sending a beacon at TSF "interval * (N+.25)"
> we can round that up to "interval * (N+1)" by adjusting the TSF write
> offset. This means that stations will adopt a TSF that is larger than
> ours by a bit. If we'd do the same for probe responses (which should be
> fairly easy), would the timing work out? I'm not entirely clear about
> CFP operation etc.

Where would the TSF be adjusted? Some of these tricks could be doable in
the low-level firmware/microcode (if one is used), but this may be
difficult to do without access to such low-level location.

> > I've done some
> > experiments with configuring the hardware to use beacon interval divided
> > by number of BSSes, but ended up using burst of beacon frames instead
> > (and all BSSes were forced to use the same beacon interval).
> 
> Do you remember any reason?

Nope.. There were some issues in getting the implementation to be
stable, so that may have been the main reason. Setting the beacon
interval and related interrupts to very small values was probably not
expected in the original design.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> Well, the multicast/broadcast frames would be sent after DTIM beacon
> using normal frame transmission rules, so this would be possible order.
> Though, this might not be that likely if one were to consider a case
> where these APs are on separate radios (i.e., multiple STAs contenting
> for the medium) should there be large number of buffered frames.

Right.

> While this may end up being the easiest and cleanest way of implementing
> multi-BSS support, this has somewhat unfortunate drawbacks for power
> save stations (e.g., STAs associated with the AP1 would need to remain
> awake for the extra time needed to send beacon2 and beacon3; and STAs
> associated with the AP3 would need to remain awake while mc1..mc2 frames
> are sent).

Yep. Hence my idea of doing DTIM 0 beacons independently.

> TBTT could (and well, should) be separate for each BSS. In other words,
> if there are four virtual BSSes using one radio (BSS1 .. BSS4), TBTT
> should be different for each. If the beacon frames are send out as a
> burst, TBTT2 should be TBTT1 + <time needed to send beacon1> and so on..
> Similarly, there should be independent TSF for each BSS. However,
> implementing this for virtual BSS (single radio) designs is going to be
> quite difficult since the hardware/firmware is unlikely to support
> something like this..

Well, I'm fairly sure Broadcom hardware would allow this because
apparently it allows subtracting/adding (?) an arbitrary (?) value to
the TSF timestamp as it is written. This could be made host-controlled
so the host keeps track of the offsets. The only requirement would be
that the beacon periods are spread out in equal intervals to allow the
microcode to schedule properly.

> > Another thing that I just realised is that I'm not sure whether we
> > update the TSF in probe responses correctly or at all because as far as
> > I can tell it's only done for the probe response microcode offload.
> > We'll have to look into this.
> 
> This should be done in the same way as for Beacons, i.e., by whatever
> low-level functionality is available in the transmitting path.

Yes. Except as far as I can tell Broadcom hardware/firmware only does it
when the *firmware* responds to probe responses, and we disabled that.

> > Actually, what we can do is adjust the offset for each of the beacons
> > differently. So when we're sending a beacon at TSF "interval * (N+.25)"
> > we can round that up to "interval * (N+1)" by adjusting the TSF write
> > offset. This means that stations will adopt a TSF that is larger than
> > ours by a bit. If we'd do the same for probe responses (which should be
> > fairly easy), would the timing work out? I'm not entirely clear about
> > CFP operation etc.
> 
> Where would the TSF be adjusted? Some of these tricks could be doable in
> the low-level firmware/microcode (if one is used), but this may be
> difficult to do without access to such low-level location.

There is such access as I described above.

I need do tests but right now I have no second wireless card at all.

johannes

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

[RFC] b43: fix multi MAC address hardware crypto support

[Johannes Berg]

Once mac80211 will be able to have the same station associated
to different virtual interfaces (no matter whether as AP or as
station), we may get into trouble with the hardware encryption
offload RX path because we might have the same peer more than
once and hence the RX path doesn't know which key to use. For
this rare case, we disable the hardware RX path.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Hopefully this helps other drivers... :)

 drivers/net/wireless/b43/b43.h  |    4 +-
 drivers/net/wireless/b43/main.c |   77 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 77 insertions(+), 4 deletions(-)

--- everything.orig/drivers/net/wireless/b43/main.c	2007-11-09 10:48:48.474785861 +0100
+++ everything/drivers/net/wireless/b43/main.c	2007-11-09 11:26:41.284781195 +0100
@@ -793,6 +793,8 @@ static int b43_key_write(struct b43_wlde
 {
 	int i;
 	int sta_keys_start;
+	static const u8 zero_mac[ETH_ALEN] = { 0 };
+	const u8 *orig_mac = mac_addr;
 
 	if (key_len > B43_SEC_KEYSIZE)
 		return -EINVAL;
@@ -807,6 +809,20 @@ static int b43_key_write(struct b43_wlde
 			sta_keys_start = 4;
 		else
 			sta_keys_start = 8;
+		/*
+		 * Check if we already have this MAC address. If so, we're
+		 * adding a key for the second station (the in-hardware mac
+		 * address for the first has already been cleared) and thus
+		 * may not write the address to the hardware.
+		 */
+		for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
+			if (!dev->key[i].keyconf)
+				continue;
+			if (!compare_ether_addr(mac_addr, dev->key[i].mac)) {
+				mac_addr = zero_mac;
+				break;
+			}
+		}
 		for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
 			if (!dev->key[i].keyconf) {
 				/* found empty */
@@ -829,6 +845,7 @@ static int b43_key_write(struct b43_wlde
 	}
 	keyconf->hw_key_idx = index;
 	dev->key[index].keyconf = keyconf;
+	memcpy(dev->key[index].mac, orig_mac, ETH_ALEN);
 
 	return 0;
 }
@@ -2895,8 +2912,13 @@ static int b43_op_set_key(struct ieee802
 		if (err)
 			goto out_unlock;
 
-		if (algorithm == B43_SEC_ALGO_WEP40 ||
-		    algorithm == B43_SEC_ALGO_WEP104) {
+		/*
+		 * The USE DEFAULT KEYS bit may never be set when we have
+		 * multiple interfaces.
+		 */
+		if ((dev->wl->if_count < 2) &&
+		    (algorithm == B43_SEC_ALGO_WEP40 ||
+		     algorithm == B43_SEC_ALGO_WEP104)) {
 			b43_hf_write(dev, b43_hf_read(dev) | B43_HF_USEDEFKEYS);
 		} else {
 			b43_hf_write(dev,
@@ -2925,6 +2947,56 @@ out_unlock:
 	return err;
 }
 
+static void b43_op_sta_notify(struct ieee80211_hw *hw, int if_id,
+			      enum sta_notify_cmd cmd, const u8 *addr)
+{
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	struct b43_wldev *dev;
+	unsigned long flags;
+	int sta_keys_start, i;
+	DECLARE_MAC_BUF(mbuf);
+
+	/*
+	 * NB: This function relies (for correctness) on the fact that
+	 *     sta_notify() is always called before set_key() for any
+	 *     given station.
+	 */
+
+	/*
+	 * For now, don't add back a STA to the RX key matching
+	 * when it is removed from the second virtual interface.
+	 */
+	if (cmd != STA_NOTIFY_ADD)
+		return;
+
+	mutex_lock(&wl->mutex);
+	spin_lock_irqsave(&wl->irq_lock, flags);
+
+	dev = wl->current_dev;
+
+	if (b43_new_kidx_api(dev))
+		sta_keys_start = 4;
+	else
+		sta_keys_start = 8;
+	for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
+		if (!dev->key[i].keyconf)
+			continue;
+		if (compare_ether_addr(dev->key[i].mac, addr) == 0) {
+			/*
+			 * Found the same address as is being added,
+			 * so we cannot use hardware crypto for the RX
+			 * path any more.
+			 */
+			keymac_write(dev, i, NULL);
+			b43dbg(wl, "Removed %s from key matching.\n",
+			       print_mac(mbuf, addr));
+		}
+	}
+
+	spin_unlock_irqrestore(&wl->irq_lock, flags);
+	mutex_unlock(&wl->mutex);
+}
+
 static void b43_op_configure_filter(struct ieee80211_hw *hw,
 				    unsigned int changed, unsigned int *fflags,
 				    int mc_count, struct dev_addr_list *mc_list)
@@ -3695,6 +3767,7 @@ static const struct ieee80211_ops b43_hw
 	.config_interface	= b43_op_config_interface,
 	.configure_filter	= b43_op_configure_filter,
 	.set_key		= b43_op_set_key,
+	.sta_notify		= b43_op_sta_notify,
 	.get_stats		= b43_op_get_stats,
 	.get_tx_stats		= b43_op_get_tx_stats,
 	.start			= b43_op_start,
--- everything.orig/drivers/net/wireless/b43/b43.h	2007-11-09 11:12:07.454790147 +0100
+++ everything/drivers/net/wireless/b43/b43.h	2007-11-09 11:13:11.904780978 +0100
@@ -582,10 +582,10 @@ struct b43_stats {
 
 struct b43_key {
 	/* If keyconf is NULL, this key is disabled.
-	 * keyconf is a cookie. Don't derefenrence it outside of the set_key
-	 * path, because b43 doesn't own it. */
+	 * keyconf is a cookie. b43 doesn't own it. */
 	struct ieee80211_key_conf *keyconf;
 	u8 algorithm;
+	u8 mac[ETH_ALEN];
 };
 
 struct b43_wldev;

Re: [RFC] b43: fix multi MAC address hardware crypto support

[Johannes Berg]

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


> +static void b43_op_sta_notify(struct ieee80211_hw *hw, int if_id,
> +			      enum sta_notify_cmd cmd, const u8 *addr)
> +{
> +	struct b43_wl *wl = hw_to_b43_wl(hw);
> +	struct b43_wldev *dev;
> +	unsigned long flags;
> +	int sta_keys_start, i;
> +	DECLARE_MAC_BUF(mbuf);
> +
> +	/*
> +	 * NB: This function relies (for correctness) on the fact that
> +	 *     sta_notify() is always called before set_key() for any
> +	 *     given station.
> +	 */
> +
> +	/*
> +	 * For now, don't add back a STA to the RX key matching
> +	 * when it is removed from the second virtual interface.
> +	 */
> +	if (cmd != STA_NOTIFY_ADD)
> +		return;
> +
> +	mutex_lock(&wl->mutex);

Hm. this is atomic, can't lock the mutex.

johannes

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

[RFC v2] b43: fix multi MAC address hardware crypto support

[Johannes Berg]

Once mac80211 will be able to have the same station associated
to different virtual interfaces (no matter whether as AP or as
station), we may get into trouble with the hardware encryption
offload RX path because we might have the same peer more than
once and hence the RX path doesn't know which key to use. For
this rare case, we disable the hardware RX path.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Fixes two NULL pointer dereferences and the mutex lock thing. No way to
actually test it until I patch mac80211 to support associating to the
same AP with two virtual interfaces...

 drivers/net/wireless/b43/b43.h  |    3 -
 drivers/net/wireless/b43/main.c |   81 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 81 insertions(+), 3 deletions(-)

--- everything.orig/drivers/net/wireless/b43/main.c	2007-11-10 02:12:20.959959690 +0100
+++ everything/drivers/net/wireless/b43/main.c	2007-11-10 02:14:26.719963486 +0100
@@ -792,6 +792,8 @@ static int b43_key_write(struct b43_wlde
 {
 	int i;
 	int sta_keys_start;
+	static const u8 zero_mac[ETH_ALEN] = { 0 };
+	const u8 *orig_mac = mac_addr;
 
 	if (key_len > B43_SEC_KEYSIZE)
 		return -EINVAL;
@@ -806,6 +808,22 @@ static int b43_key_write(struct b43_wlde
 			sta_keys_start = 4;
 		else
 			sta_keys_start = 8;
+		/*
+		 * Check if we already have this MAC address. If so, we're
+		 * adding a key for the second station (the in-hardware mac
+		 * address for the first has already been cleared) and thus
+		 * may not write the address to the hardware.
+		 */
+		for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
+			if (!mac_addr)
+				break;
+			if (!dev->key[i].keyconf)
+				continue;
+			if (!compare_ether_addr(mac_addr, dev->key[i].mac)) {
+				mac_addr = zero_mac;
+				break;
+			}
+		}
 		for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
 			if (!dev->key[i].keyconf) {
 				/* found empty */
@@ -828,6 +846,10 @@ static int b43_key_write(struct b43_wlde
 	}
 	keyconf->hw_key_idx = index;
 	dev->key[index].keyconf = keyconf;
+	if (orig_mac)
+		memcpy(dev->key[index].mac, orig_mac, ETH_ALEN);
+	else
+		memset(dev->key[index].mac, 0xFF, ETH_ALEN);
 
 	return 0;
 }
@@ -2894,8 +2916,13 @@ static int b43_op_set_key(struct ieee802
 		if (err)
 			goto out_unlock;
 
-		if (algorithm == B43_SEC_ALGO_WEP40 ||
-		    algorithm == B43_SEC_ALGO_WEP104) {
+		/*
+		 * The USE DEFAULT KEYS bit may never be set when we have
+		 * multiple interfaces.
+		 */
+		if ((wl->if_count < 2) &&
+		    (algorithm == B43_SEC_ALGO_WEP40 ||
+		     algorithm == B43_SEC_ALGO_WEP104)) {
 			b43_hf_write(dev, b43_hf_read(dev) | B43_HF_USEDEFKEYS);
 		} else {
 			b43_hf_write(dev,
@@ -2924,6 +2951,55 @@ out_unlock:
 	return err;
 }
 
+static void b43_op_sta_notify(struct ieee80211_hw *hw, int if_id,
+			      enum sta_notify_cmd cmd, const u8 *addr)
+{
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	struct b43_wldev *dev;
+	unsigned long flags;
+	int sta_keys_start, i;
+	DECLARE_MAC_BUF(mbuf);
+
+	/*
+	 * NB: This function relies (for correctness) on the fact that
+	 *     sta_notify() is always called before set_key() for any
+	 *     given station.
+	 */
+
+	/*
+	 * For now, don't add back a STA to the RX key matching
+	 * when it is removed from the second virtual interface.
+	 */
+	if (cmd != STA_NOTIFY_ADD)
+		return;
+
+	/* can't lock mutex here */
+	spin_lock_irqsave(&wl->irq_lock, flags);
+
+	dev = wl->current_dev;
+
+	if (b43_new_kidx_api(dev))
+		sta_keys_start = 4;
+	else
+		sta_keys_start = 8;
+	for (i = sta_keys_start; i < dev->max_nr_keys; i++) {
+		if (!dev->key[i].keyconf)
+			continue;
+		if (compare_ether_addr(dev->key[i].mac, addr) == 0) {
+			/*
+			 * Found the same address as is being added,
+			 * so we cannot use hardware crypto for the RX
+			 * path any more.
+			 */
+			keymac_write(dev, i, NULL);
+			b43dbg(wl, "Removed %s from key matching.\n",
+			       print_mac(mbuf, addr));
+		}
+	}
+
+	spin_unlock_irqrestore(&wl->irq_lock, flags);
+}
+
 static void b43_op_configure_filter(struct ieee80211_hw *hw,
 				    unsigned int changed, unsigned int *fflags,
 				    int mc_count, struct dev_addr_list *mc_list)
@@ -3694,6 +3770,7 @@ static const struct ieee80211_ops b43_hw
 	.config_interface	= b43_op_config_interface,
 	.configure_filter	= b43_op_configure_filter,
 	.set_key		= b43_op_set_key,
+	.sta_notify		= b43_op_sta_notify,
 	.get_stats		= b43_op_get_stats,
 	.get_tx_stats		= b43_op_get_tx_stats,
 	.start			= b43_op_start,
--- everything.orig/drivers/net/wireless/b43/b43.h	2007-11-09 22:54:31.489965385 +0100
+++ everything/drivers/net/wireless/b43/b43.h	2007-11-10 02:14:26.719963486 +0100
@@ -582,10 +582,11 @@ struct b43_stats {
 
 struct b43_key {
 	/* If keyconf is NULL, this key is disabled.
-	 * keyconf is a cookie. Don't derefenrence it outside of the set_key
+	 * keyconf is a cookie. Don't dereference it outside of the set_key
 	 * path, because b43 doesn't own it. */
 	struct ieee80211_key_conf *keyconf;
 	u8 algorithm;
+	u8 mac[ETH_ALEN];
 };
 
 struct b43_wldev;

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


On Fri, 2007-11-09 at 02:43 +0100, Johannes Berg wrote:
> This patch adds support for multiple virtual interfaces (up to 65536)
> to b43.

Ok, turns out I need to do more testing, somehow I broke something, it
seems that the RX header lost the "frame is for me" bit or something. Or
maybe mac80211 has a bug.

johannes

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

[RFC v2] b43: multiple MAC addresses support

[Johannes Berg]

This patch adds support for multiple virtual interfaces (up to 65536)
to b43. This requires modified firmware which is detected by an invalid
day in the firmware timestamp (254), and then the firmware time and the
rest of the date is used for feature bits.

The multi-MAC support works by having a mask of ignored bits, the
modified firmware will mask off these bits before comparing the address
on incoming frames with the address. This, however, means that when you
add three MAC virtual interfaces, the firmware will at least also ACK
frames on a fourth address. The required mask is calculated dynamically
to be the least restrictive mask possible, but you should take care to
allocate your MAC addresses in a way that minimises the mask to avoid
strange behaviour. For example, avoid using addresses 02:00:00:00:00:02
and 02:00:00:00:00:10 together because these two have only 46 bits in
common, use 02:00:00:00:00:12 and 02:00:00:00:00:10 instead as those
have 47 bits in common.

Note that the mask applies only to the lower 16 bits, so the first four
bytes of all addresses need to be identical.

The required tool to patch the firmware will be published separately.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Michael Wu <flamingice@sourmilk.net>
Cc: Jiri Benc <jbenc@suse.cz>
Cc: Ben Greear <greearb@candelatech.com>
---
Ok, it works now. This uses registers instead of shm to set the MAC
address. Not that it matters much. Also fixes a few small bugs I think.
But that may have been only in the security fix patch. You can do the
interdiff to find out...

 drivers/net/wireless/b43/b43.h  |   19 ++-
 drivers/net/wireless/b43/main.c |  199 +++++++++++++++++++++++++++++++++-------
 drivers/net/wireless/b43/xmit.c |    6 -
 3 files changed, 179 insertions(+), 45 deletions(-)

--- everything.orig/drivers/net/wireless/b43/main.c	2007-11-09 03:22:39.028987631 +0100
+++ everything/drivers/net/wireless/b43/main.c	2007-11-10 02:12:20.959959690 +0100
@@ -543,7 +543,20 @@ static void b43_write_mac_bssid_template
 static void b43_upload_card_macaddress(struct b43_wldev *dev)
 {
 	b43_write_mac_bssid_templates(dev);
-	b43_macfilter_set(dev, B43_MACFILTER_SELF, dev->wl->mac_addr);
+
+	if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA) {
+		u16 tmp;
+
+		tmp = ((u16 *)dev->wl->mac_addr)[0];
+		b43_shm_write16(dev, B43_SHM_SCRATCH, 60, cpu_to_le16(tmp));
+		tmp = ((u16 *)dev->wl->mac_addr)[1];
+		b43_shm_write16(dev, B43_SHM_SCRATCH, 61, cpu_to_le16(tmp));
+		tmp = ((u16 *)dev->wl->mac_addr)[2];
+		b43_shm_write16(dev, B43_SHM_SCRATCH, 62, cpu_to_le16(tmp));
+		/* swap here because we always treat the mask as big endian */
+		b43_shm_write16(dev, B43_SHM_SCRATCH, 63, swab16(~dev->wl->mac_mask));
+	} else
+		b43_macfilter_set(dev, B43_MACFILTER_SELF, dev->wl->mac_addr);
 }
 
 static void b43_set_slot_time(struct b43_wldev *dev, u16 slot_time)
@@ -1174,18 +1187,18 @@ static void b43_write_probe_resp_plcp(st
 {
 	struct b43_plcp_hdr4 plcp;
 	u32 tmp;
-	__le16 dur;
 
 	plcp.data = 0;
 	b43_generate_plcp_hdr(&plcp, size + FCS_LEN, rate);
-	dur = ieee80211_generic_frame_duration(dev->wl->hw,
-					       dev->wl->if_id, size,
-					       B43_RATE_TO_BASE100KBPS(rate));
 	/* Write PLCP in two parts and timing for packet transfer */
 	tmp = le32_to_cpu(plcp.data);
 	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset, tmp & 0xFFFF);
 	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 2, tmp >> 16);
-	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 6, le16_to_cpu(dur));
+	/*
+	 * XXX: currently, the duration here doesn't matter, but
+	 * when we implement offload it needs to be fixed!
+	 */
+	b43_shm_write16(dev, B43_SHM_SHARED, shm_offset + 6, 0);
 }
 
 /* Instead of using custom probe response template, this function
@@ -1200,7 +1213,6 @@ static u8 *b43_generate_probe_resp(struc
 	const u8 *src_data;
 	u8 *dest_data;
 	u16 src_size, elem_size, src_pos, dest_pos;
-	__le16 dur;
 	struct ieee80211_hdr *hdr;
 
 	B43_WARN_ON(!dev->cached_beacon);
@@ -1235,10 +1247,11 @@ static u8 *b43_generate_probe_resp(struc
 	/* Set the frame control. */
 	hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
 					 IEEE80211_STYPE_PROBE_RESP);
-	dur = ieee80211_generic_frame_duration(dev->wl->hw,
-					       dev->wl->if_id, *dest_size,
-					       B43_RATE_TO_BASE100KBPS(rate));
-	hdr->duration_id = dur;
+	/*
+	 * XXX: currently, the duration here doesn't matter, but
+	 * when we implement offload it needs to be fixed!
+	 */
+	hdr->duration_id = 0;
 
 	return dest_data;
 }
@@ -1792,11 +1805,23 @@ static int b43_upload_microcode(struct b
 		err = -EOPNOTSUPP;
 		goto out;
 	}
-	b43dbg(dev->wl, "Loading firmware version %u.%u "
-	       "(20%.2i-%.2i-%.2i %.2i:%.2i:%.2i)\n",
-	       fwrev, fwpatch,
-	       (fwdate >> 12) & 0xF, (fwdate >> 8) & 0xF, fwdate & 0xFF,
-	       (fwtime >> 11) & 0x1F, (fwtime >> 5) & 0x3F, fwtime & 0x1F);
+	/* special day 254: modified firmware with feature bits */
+	if ((fwdate & 0xFF) == 0xFE) {
+		b43dbg(dev->wl, "Loading firmware version %u.%u\n",
+		       fwrev, fwpatch);
+		dev->fw.features = fwtime | ((fwdate & 0xFF00) << 8);
+		b43dbg(dev->wl, "This firmware was modified and has the"
+				" added features 0x%06x\n", dev->fw.features);
+		if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA)
+			b43dbg(dev->wl, " * station multi MAC addr support\n");
+	} else
+		b43dbg(dev->wl, "Loading firmware version %u.%u "
+		       "(20%.2i-%.2i-%.2i %.2i:%.2i:%.2i)\n",
+		       fwrev, fwpatch,
+		       (fwdate >> 12) & 0xF, (fwdate >> 8) & 0xF,
+		       fwdate & 0xFF,
+		       (fwtime >> 11) & 0x1F, (fwtime >> 5) & 0x3F,
+		       fwtime & 0x1F);
 
 	dev->fw.rev = fwrev;
 	dev->fw.patch = fwpatch;
@@ -2948,11 +2973,18 @@ static int b43_op_config_interface(struc
 		return -ENODEV;
 	mutex_lock(&wl->mutex);
 	spin_lock_irqsave(&wl->irq_lock, flags);
-	B43_WARN_ON(wl->if_id != if_id);
-	if (conf->bssid)
-		memcpy(wl->bssid, conf->bssid, ETH_ALEN);
-	else
-		memset(wl->bssid, 0, ETH_ALEN);
+
+	/*
+	 * Only take bssid of first iface, we can only adopt time from
+	 * one BSS so arbitrarily choose the first.
+	 */
+	if (wl->if_id == if_id) {
+		if (conf->bssid)
+			memcpy(wl->bssid, conf->bssid, ETH_ALEN);
+		else
+			memset(wl->bssid, 0, ETH_ALEN);
+	}
+
 	if (b43_status(dev) >= B43_STAT_INITIALIZED) {
 		if (b43_is_mode(wl, IEEE80211_IF_TYPE_AP)) {
 			B43_WARN_ON(conf->type != IEEE80211_IF_TYPE_AP);
@@ -3424,6 +3456,71 @@ static int b43_wireless_core_init(struct
 	return err;
 }
 
+struct mac_iter {
+	struct b43_wl *wl;
+	u16 mac_val;
+	bool add;
+	int count;
+};
+
+static void b43_mac_iter(void *data, u8 *mac, int if_id)
+{
+	struct mac_iter *iter = data;
+	u16 val = (mac[4] << 8) | mac[5];
+	u16 cur = (iter->wl->mac_addr[4] << 8) | iter->wl->mac_addr[5];
+
+	/* skip the one that's being removed */
+	if (!iter->add && val == iter->mac_val)
+		return;
+
+	/* take first address we find as base */
+	if (iter->count == 0) {
+		cur = val;
+		iter->wl->mac_addr[4] = cur >> 8;
+		iter->wl->mac_addr[5] = cur;
+	}
+
+	iter->wl->mac_mask |= cur ^ val;
+
+	iter->count++;
+}
+
+static void b43_adjust_mac_addr_mask(struct b43_wl *wl, bool add, u8 *addr)
+{
+	struct mac_iter iter;
+	DECLARE_MAC_BUF(mbuf);
+	u16 cur;
+	u16 val = (addr[4] << 8) | addr[5];
+
+	/*
+	 * Because this is not invoked on adding the first address,
+	 * we do not need to have any special code for that case.
+	 * If it was invoked, we'd have to set cur to val here etc.
+	 * like above in the iter->count == 0 case (because we don't
+	 * see the interface that's being added in the iteration.)
+	 */
+
+	wl->mac_mask = 0;
+
+	iter.wl = wl;
+	iter.add = add;
+	iter.mac_val = val;
+	iter.count = 0;
+	ieee80211_iterate_active_interfaces(wl->hw, b43_mac_iter, &iter);
+
+	/* add the one that's being added */
+	cur = (wl->mac_addr[4] << 8) | wl->mac_addr[5];
+	if (add)
+		wl->mac_mask |= cur ^ val;
+
+	cur = cur & ~wl->mac_mask;
+	wl->mac_addr[4] = cur >> 8;
+	wl->mac_addr[5] = cur;
+
+	b43dbg(wl, "MAC address is %s, mask is 0x%.4x\n",
+	       print_mac(mbuf, wl->mac_addr), wl->mac_mask);
+}
+
 static int b43_op_add_interface(struct ieee80211_hw *hw,
 				struct ieee80211_if_init_conf *conf)
 {
@@ -3441,16 +3538,36 @@ static int b43_op_add_interface(struct i
 		return -EOPNOTSUPP;
 
 	mutex_lock(&wl->mutex);
-	if (wl->operating)
-		goto out_mutex_unlock;
+	dev = wl->current_dev;
 
-	b43dbg(wl, "Adding Interface type %d\n", conf->type);
+	if (dev->fw.features & B43_FW_FEATURE_MULTI_MAC_STA) {
+		if (wl->if_count && conf->type != IEEE80211_IF_TYPE_STA)
+			goto out_mutex_unlock;
+		if (wl->if_count > 0) {
+			if (((u8 *)conf->mac_addr)[0] != wl->mac_addr[0])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[1] != wl->mac_addr[1])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[2] != wl->mac_addr[2])
+				goto out_mutex_unlock;
+			if (((u8 *)conf->mac_addr)[3] != wl->mac_addr[3])
+				goto out_mutex_unlock;
+			b43_adjust_mac_addr_mask(wl, 1, conf->mac_addr);
+		}
+	} else {
+		if (wl->if_count)
+			goto out_mutex_unlock;
+	}
 
-	dev = wl->current_dev;
-	wl->operating = 1;
-	wl->if_id = conf->if_id;
-	wl->if_type = conf->type;
-	memcpy(wl->mac_addr, conf->mac_addr, ETH_ALEN);
+	wl->if_count++;
+
+	if (wl->if_count == 1) {
+		wl->if_id = conf->if_id;
+		wl->if_type = conf->type;
+		memcpy(wl->mac_addr, conf->mac_addr, ETH_ALEN);
+	}
+
+	b43dbg(wl, "Adding Interface type %d\n", conf->type);
 
 	spin_lock_irqsave(&wl->irq_lock, flags);
 	b43_adjust_opmode(dev);
@@ -3464,6 +3581,14 @@ static int b43_op_add_interface(struct i
 	return err;
 }
 
+static void b43_ifid_iter(void *data, u8 *mac, int if_id)
+{
+	struct b43_wl *wl = data;
+
+	if (wl->if_id != if_id)
+		wl->if_id = if_id;
+}
+
 static void b43_op_remove_interface(struct ieee80211_hw *hw,
 				    struct ieee80211_if_init_conf *conf)
 {
@@ -3475,14 +3600,22 @@ static void b43_op_remove_interface(stru
 
 	mutex_lock(&wl->mutex);
 
-	B43_WARN_ON(!wl->operating);
-	B43_WARN_ON(wl->if_id != conf->if_id);
+	B43_WARN_ON(!wl->if_count);
+
+	wl->if_count--;
+	if (wl->if_count == 0) {
+		wl->if_type = IEEE80211_IF_TYPE_INVALID;
+		memset(wl->mac_addr, 0, ETH_ALEN);
+	}
+
+	/* arbitrarily adopt another if_id as main if_id */
+	if (conf->if_id == wl->if_id)
+		ieee80211_iterate_active_interfaces(wl->hw, b43_ifid_iter, wl);
 
-	wl->operating = 0;
+	b43_adjust_mac_addr_mask(wl, 0, conf->mac_addr);
 
 	spin_lock_irqsave(&wl->irq_lock, flags);
 	b43_adjust_opmode(dev);
-	memset(wl->mac_addr, 0, ETH_ALEN);
 	b43_upload_card_macaddress(dev);
 	spin_unlock_irqrestore(&wl->irq_lock, flags);
 
--- everything.orig/drivers/net/wireless/b43/b43.h	2007-11-09 03:22:39.098988498 +0100
+++ everything/drivers/net/wireless/b43/b43.h	2007-11-09 22:54:31.489965385 +0100
@@ -601,22 +601,20 @@ struct b43_wl {
 	struct mutex mutex;
 	spinlock_t leds_lock;
 
-	/* We can only have one operating interface (802.11 core)
-	 * at a time. General information about this interface follows.
-	 */
-
-	/* Opaque ID of the operating interface from the ieee80211
+	/*
+	 * Opaque ID of the first operating interface from the ieee80211
 	 * subsystem. Do not modify.
 	 */
 	int if_id;
-	/* The MAC address of the operating interface. */
+	/* The MAC address */
 	u8 mac_addr[ETH_ALEN];
+	u16 mac_mask;
 	/* Current BSSID */
 	u8 bssid[ETH_ALEN];
 	/* Interface type. (IEEE80211_IF_TYPE_XXX) */
 	int if_type;
-	/* Is the card operating in AP, STA or IBSS mode? */
-	bool operating;
+	/* number of open interfaces */
+	int if_count;
 	/* filter flags */
 	unsigned int filter_flags;
 	/* Stats about the wireless interface */
@@ -648,6 +646,9 @@ struct b43_firmware {
 	u16 rev;
 	/* Firmware patchlevel */
 	u16 patch;
+	/* Firmware features (24 bits) */
+#define B43_FW_FEATURE_MULTI_MAC_STA	0x000001
+	u32 features;
 };
 
 /* Device (802.11 core) initialization status. */
@@ -781,7 +782,7 @@ static inline struct b43_wldev *dev_to_b
 /* Is the device operating in a specified mode (IEEE80211_IF_TYPE_XXX). */
 static inline int b43_is_mode(struct b43_wl *wl, int type)
 {
-	return (wl->operating && wl->if_type == type);
+	return (wl->if_count && wl->if_type == type);
 }
 
 static inline u16 b43_read16(struct b43_wldev *dev, u16 offset)
--- everything.orig/drivers/net/wireless/b43/xmit.c	2007-11-09 03:22:39.238983995 +0100
+++ everything/drivers/net/wireless/b43/xmit.c	2007-11-09 22:38:21.219963866 +0100
@@ -221,7 +221,7 @@ static void generate_txhdr_fw4(struct b4
 	} else {
 		int fbrate_base100kbps = B43_RATE_TO_BASE100KBPS(rate_fb);
 		txhdr->dur_fb = ieee80211_generic_frame_duration(dev->wl->hw,
-								 dev->wl->if_id,
+								 txctl->ifindex,
 								 fragment_len,
 								 fbrate_base100kbps);
 	}
@@ -311,7 +311,7 @@ static void generate_txhdr_fw4(struct b4
 		rts_rate_fb_ofdm = b43_is_ofdm_rate(rts_rate_fb);
 
 		if (txctl->flags & IEEE80211_TXCTL_USE_CTS_PROTECT) {
-			ieee80211_ctstoself_get(dev->wl->hw, dev->wl->if_id,
+			ieee80211_ctstoself_get(dev->wl->hw, txctl->ifindex,
 						fragment_data, fragment_len,
 						txctl,
 						(struct ieee80211_cts *)(txhdr->
@@ -319,7 +319,7 @@ static void generate_txhdr_fw4(struct b4
 			mac_ctl |= B43_TX4_MAC_SENDCTS;
 			len = sizeof(struct ieee80211_cts);
 		} else {
-			ieee80211_rts_get(dev->wl->hw, dev->wl->if_id,
+			ieee80211_rts_get(dev->wl->hw, txctl->ifindex,
 					  fragment_data, fragment_len, txctl,
 					  (struct ieee80211_rts *)(txhdr->
 								   rts_frame));

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> The required tool to patch the firmware will be published separately.

http://git.sipsolutions.net/bcom-ucode.git

Requires patched bcm43xx-asm and bcm43xx-dasm so you won't be happy
until Michael does similar changes, I've sent them to him in private
mail.

johannes

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

Re: [RFC] b43: multiple MAC addresses support

[Johannes Berg]

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


> http://git.sipsolutions.net/bcom-ucode.git
> 
> Requires patched bcm43xx-asm and bcm43xx-dasm so you won't be happy
> until Michael does similar changes, I've sent them to him in private
> mail.

And he has committed them. Grab my script above,
http://git.bu3sch.de/git/bcm43xx-asm.git and let me know the test
results ;)

johannes

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