[PATCH 1/3] mac80211: better definition of mactime

[PATCH 1/3] mac80211: better definition of mactime

[Bruno Randolf]

define mactime as the time when the first data symbol arrived at the HW. the
old definition was questionable because 802.11 defines timestamp only for
beacon and probe response frames, and there it means the timestamp field.

a stricter definition of mactime is necessary for correct merging of IBSS.

note that it is up to the driver to convert whatever its hardware returns to
this definition. unfortunately we don't know for example when atheros hardware
takes its rx timestamp exactly :(

Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---

 include/net/mac80211.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)


diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 460da54..1b807f4 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -295,7 +295,8 @@ enum mac80211_rx_flags {
  * The low-level driver should provide this information (the subset
  * supported by hardware) to the 802.11 code with each received
  * frame.
- * @mactime: MAC timestamp as defined by 802.11
+ * @mactime: value in microseconds of the 64-bit Time Synchronization Function
+ * 	(TSF) timer when the first data symbol (MPDU) arrived at the hardware.
  * @band: the active band when this frame was received
  * @freq: frequency the radio was tuned to when receiving this frame, in MHz
  * @ssi: signal strength when receiving this frame

[PATCH 2/3] mac80211: move function ieee80211_sta_join_ibss()

[Bruno Randolf]

this moves ieee80211_sta_join_ibss() up for the next patch (ibss merge).
---

 net/mac80211/ieee80211_sta.c |  317 +++++++++++++++++++++---------------------
 1 files changed, 159 insertions(+), 158 deletions(-)


diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 2c7b8be..72d7a86 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -2030,6 +2030,165 @@ void ieee80211_rx_bss_list_deinit(struct net_device *dev)
 }
 
 
+static int ieee80211_sta_join_ibss(struct net_device *dev,
+				   struct ieee80211_if_sta *ifsta,
+				   struct ieee80211_sta_bss *bss)
+{
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	int res, rates, i, j;
+	struct sk_buff *skb;
+	struct ieee80211_mgmt *mgmt;
+	struct ieee80211_tx_control control;
+	struct rate_selection ratesel;
+	u8 *pos;
+	struct ieee80211_sub_if_data *sdata;
+	struct ieee80211_supported_band *sband;
+
+	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
+
+	/* Remove possible STA entries from other IBSS networks. */
+	sta_info_flush(local, NULL);
+
+	if (local->ops->reset_tsf) {
+		/* Reset own TSF to allow time synchronization work. */
+		local->ops->reset_tsf(local_to_hw(local));
+	}
+	memcpy(ifsta->bssid, bss->bssid, ETH_ALEN);
+	res = ieee80211_if_config(dev);
+	if (res)
+		return res;
+
+	local->hw.conf.beacon_int = bss->beacon_int >= 10 ? bss->beacon_int : 10;
+
+	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	sdata->drop_unencrypted = bss->capability &
+		WLAN_CAPABILITY_PRIVACY ? 1 : 0;
+
+	res = ieee80211_set_freq(local, bss->freq);
+
+	if (local->oper_channel->flags & IEEE80211_CHAN_NO_IBSS) {
+		printk(KERN_DEBUG "%s: IBSS not allowed on frequency "
+		       "%d MHz\n", dev->name, local->oper_channel->center_freq);
+		return -1;
+	}
+
+	/* Set beacon template */
+	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 400);
+	do {
+		if (!skb)
+			break;
+
+		skb_reserve(skb, local->hw.extra_tx_headroom);
+
+		mgmt = (struct ieee80211_mgmt *)
+			skb_put(skb, 24 + sizeof(mgmt->u.beacon));
+		memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
+		mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
+						   IEEE80211_STYPE_BEACON);
+		memset(mgmt->da, 0xff, ETH_ALEN);
+		memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
+		memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
+		mgmt->u.beacon.beacon_int =
+			cpu_to_le16(local->hw.conf.beacon_int);
+		mgmt->u.beacon.capab_info = cpu_to_le16(bss->capability);
+
+		pos = skb_put(skb, 2 + ifsta->ssid_len);
+		*pos++ = WLAN_EID_SSID;
+		*pos++ = ifsta->ssid_len;
+		memcpy(pos, ifsta->ssid, ifsta->ssid_len);
+
+		rates = bss->supp_rates_len;
+		if (rates > 8)
+			rates = 8;
+		pos = skb_put(skb, 2 + rates);
+		*pos++ = WLAN_EID_SUPP_RATES;
+		*pos++ = rates;
+		memcpy(pos, bss->supp_rates, rates);
+
+		if (bss->band == IEEE80211_BAND_2GHZ) {
+			pos = skb_put(skb, 2 + 1);
+			*pos++ = WLAN_EID_DS_PARAMS;
+			*pos++ = 1;
+			*pos++ = ieee80211_frequency_to_channel(bss->freq);
+		}
+
+		pos = skb_put(skb, 2 + 2);
+		*pos++ = WLAN_EID_IBSS_PARAMS;
+		*pos++ = 2;
+		/* FIX: set ATIM window based on scan results */
+		*pos++ = 0;
+		*pos++ = 0;
+
+		if (bss->supp_rates_len > 8) {
+			rates = bss->supp_rates_len - 8;
+			pos = skb_put(skb, 2 + rates);
+			*pos++ = WLAN_EID_EXT_SUPP_RATES;
+			*pos++ = rates;
+			memcpy(pos, &bss->supp_rates[8], rates);
+		}
+
+		memset(&control, 0, sizeof(control));
+		rate_control_get_rate(dev, sband, skb, &ratesel);
+		if (!ratesel.rate) {
+			printk(KERN_DEBUG "%s: Failed to determine TX rate "
+			       "for IBSS beacon\n", dev->name);
+			break;
+		}
+		control.vif = &sdata->vif;
+		control.tx_rate = ratesel.rate;
+		if (sdata->bss_conf.use_short_preamble &&
+		    ratesel.rate->flags & IEEE80211_RATE_SHORT_PREAMBLE)
+			control.flags |= IEEE80211_TXCTL_SHORT_PREAMBLE;
+		control.antenna_sel_tx = local->hw.conf.antenna_sel_tx;
+		control.flags |= IEEE80211_TXCTL_NO_ACK;
+		control.retry_limit = 1;
+
+		ifsta->probe_resp = skb_copy(skb, GFP_ATOMIC);
+		if (ifsta->probe_resp) {
+			mgmt = (struct ieee80211_mgmt *)
+				ifsta->probe_resp->data;
+			mgmt->frame_control =
+				IEEE80211_FC(IEEE80211_FTYPE_MGMT,
+					     IEEE80211_STYPE_PROBE_RESP);
+		} else {
+			printk(KERN_DEBUG "%s: Could not allocate ProbeResp "
+			       "template for IBSS\n", dev->name);
+		}
+
+		if (local->ops->beacon_update &&
+		    local->ops->beacon_update(local_to_hw(local),
+					     skb, &control) == 0) {
+			printk(KERN_DEBUG "%s: Configured IBSS beacon "
+			       "template\n", dev->name);
+			skb = NULL;
+		}
+
+		rates = 0;
+		sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
+		for (i = 0; i < bss->supp_rates_len; i++) {
+			int bitrate = (bss->supp_rates[i] & 0x7f) * 5;
+			for (j = 0; j < sband->n_bitrates; j++)
+				if (sband->bitrates[j].bitrate == bitrate)
+					rates |= BIT(j);
+		}
+		ifsta->supp_rates_bits[local->hw.conf.channel->band] = rates;
+	} while (0);
+
+	if (skb) {
+		printk(KERN_DEBUG "%s: Failed to configure IBSS beacon "
+		       "template\n", dev->name);
+		dev_kfree_skb(skb);
+	}
+
+	ifsta->state = IEEE80211_IBSS_JOINED;
+	mod_timer(&ifsta->timer, jiffies + IEEE80211_IBSS_MERGE_INTERVAL);
+
+	ieee80211_rx_bss_put(dev, bss);
+
+	return res;
+}
+
+
 static void ieee80211_rx_bss_info(struct net_device *dev,
 				  struct ieee80211_mgmt *mgmt,
 				  size_t len,
@@ -2896,164 +3055,6 @@ static int ieee80211_sta_config_auth(struct net_device *dev,
 	return -1;
 }
 
-static int ieee80211_sta_join_ibss(struct net_device *dev,
-				   struct ieee80211_if_sta *ifsta,
-				   struct ieee80211_sta_bss *bss)
-{
-	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
-	int res, rates, i, j;
-	struct sk_buff *skb;
-	struct ieee80211_mgmt *mgmt;
-	struct ieee80211_tx_control control;
-	struct rate_selection ratesel;
-	u8 *pos;
-	struct ieee80211_sub_if_data *sdata;
-	struct ieee80211_supported_band *sband;
-
-	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
-
-	/* Remove possible STA entries from other IBSS networks. */
-	sta_info_flush(local, NULL);
-
-	if (local->ops->reset_tsf) {
-		/* Reset own TSF to allow time synchronization work. */
-		local->ops->reset_tsf(local_to_hw(local));
-	}
-	memcpy(ifsta->bssid, bss->bssid, ETH_ALEN);
-	res = ieee80211_if_config(dev);
-	if (res)
-		return res;
-
-	local->hw.conf.beacon_int = bss->beacon_int >= 10 ? bss->beacon_int : 10;
-
-	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	sdata->drop_unencrypted = bss->capability &
-		WLAN_CAPABILITY_PRIVACY ? 1 : 0;
-
-	res = ieee80211_set_freq(local, bss->freq);
-
-	if (local->oper_channel->flags & IEEE80211_CHAN_NO_IBSS) {
-		printk(KERN_DEBUG "%s: IBSS not allowed on frequency "
-		       "%d MHz\n", dev->name, local->oper_channel->center_freq);
-		return -1;
-	}
-
-	/* Set beacon template based on scan results */
-	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 400);
-	do {
-		if (!skb)
-			break;
-
-		skb_reserve(skb, local->hw.extra_tx_headroom);
-
-		mgmt = (struct ieee80211_mgmt *)
-			skb_put(skb, 24 + sizeof(mgmt->u.beacon));
-		memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
-		mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
-						   IEEE80211_STYPE_BEACON);
-		memset(mgmt->da, 0xff, ETH_ALEN);
-		memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
-		memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
-		mgmt->u.beacon.beacon_int =
-			cpu_to_le16(local->hw.conf.beacon_int);
-		mgmt->u.beacon.capab_info = cpu_to_le16(bss->capability);
-
-		pos = skb_put(skb, 2 + ifsta->ssid_len);
-		*pos++ = WLAN_EID_SSID;
-		*pos++ = ifsta->ssid_len;
-		memcpy(pos, ifsta->ssid, ifsta->ssid_len);
-
-		rates = bss->supp_rates_len;
-		if (rates > 8)
-			rates = 8;
-		pos = skb_put(skb, 2 + rates);
-		*pos++ = WLAN_EID_SUPP_RATES;
-		*pos++ = rates;
-		memcpy(pos, bss->supp_rates, rates);
-
-		if (bss->band == IEEE80211_BAND_2GHZ) {
-			pos = skb_put(skb, 2 + 1);
-			*pos++ = WLAN_EID_DS_PARAMS;
-			*pos++ = 1;
-			*pos++ = ieee80211_frequency_to_channel(bss->freq);
-		}
-
-		pos = skb_put(skb, 2 + 2);
-		*pos++ = WLAN_EID_IBSS_PARAMS;
-		*pos++ = 2;
-		/* FIX: set ATIM window based on scan results */
-		*pos++ = 0;
-		*pos++ = 0;
-
-		if (bss->supp_rates_len > 8) {
-			rates = bss->supp_rates_len - 8;
-			pos = skb_put(skb, 2 + rates);
-			*pos++ = WLAN_EID_EXT_SUPP_RATES;
-			*pos++ = rates;
-			memcpy(pos, &bss->supp_rates[8], rates);
-		}
-
-		memset(&control, 0, sizeof(control));
-		rate_control_get_rate(dev, sband, skb, &ratesel);
-		if (!ratesel.rate) {
-			printk(KERN_DEBUG "%s: Failed to determine TX rate "
-			       "for IBSS beacon\n", dev->name);
-			break;
-		}
-		control.vif = &sdata->vif;
-		control.tx_rate = ratesel.rate;
-		if (sdata->bss_conf.use_short_preamble &&
-		    ratesel.rate->flags & IEEE80211_RATE_SHORT_PREAMBLE)
-			control.flags |= IEEE80211_TXCTL_SHORT_PREAMBLE;
-		control.antenna_sel_tx = local->hw.conf.antenna_sel_tx;
-		control.flags |= IEEE80211_TXCTL_NO_ACK;
-		control.retry_limit = 1;
-
-		ifsta->probe_resp = skb_copy(skb, GFP_ATOMIC);
-		if (ifsta->probe_resp) {
-			mgmt = (struct ieee80211_mgmt *)
-				ifsta->probe_resp->data;
-			mgmt->frame_control =
-				IEEE80211_FC(IEEE80211_FTYPE_MGMT,
-					     IEEE80211_STYPE_PROBE_RESP);
-		} else {
-			printk(KERN_DEBUG "%s: Could not allocate ProbeResp "
-			       "template for IBSS\n", dev->name);
-		}
-
-		if (local->ops->beacon_update &&
-		    local->ops->beacon_update(local_to_hw(local),
-					     skb, &control) == 0) {
-			printk(KERN_DEBUG "%s: Configured IBSS beacon "
-			       "template based on scan results\n", dev->name);
-			skb = NULL;
-		}
-
-		rates = 0;
-		sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
-		for (i = 0; i < bss->supp_rates_len; i++) {
-			int bitrate = (bss->supp_rates[i] & 0x7f) * 5;
-			for (j = 0; j < sband->n_bitrates; j++)
-				if (sband->bitrates[j].bitrate == bitrate)
-					rates |= BIT(j);
-		}
-		ifsta->supp_rates_bits[local->hw.conf.channel->band] = rates;
-	} while (0);
-
-	if (skb) {
-		printk(KERN_DEBUG "%s: Failed to configure IBSS beacon "
-		       "template\n", dev->name);
-		dev_kfree_skb(skb);
-	}
-
-	ifsta->state = IEEE80211_IBSS_JOINED;
-	mod_timer(&ifsta->timer, jiffies + IEEE80211_IBSS_MERGE_INTERVAL);
-
-	ieee80211_rx_bss_put(dev, bss);
-
-	return res;
-}
-
 
 static int ieee80211_sta_create_ibss(struct net_device *dev,
 				     struct ieee80211_if_sta *ifsta)

Re: [PATCH 2/3] mac80211: move function ieee80211_sta_join_ibss()

[Johannes Berg]

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


On Fri, 2008-02-15 at 16:01 +0900, Bruno Randolf wrote:
> this moves ieee80211_sta_join_ibss() up for the next patch (ibss merge).

Dunno if it needs sign-off, but

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> 
>  net/mac80211/ieee80211_sta.c |  317 +++++++++++++++++++++---------------------
>  1 files changed, 159 insertions(+), 158 deletions(-)
> 
> 
> diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
> index 2c7b8be..72d7a86 100644
> --- a/net/mac80211/ieee80211_sta.c
> +++ b/net/mac80211/ieee80211_sta.c
> @@ -2030,6 +2030,165 @@ void ieee80211_rx_bss_list_deinit(struct net_device *dev)
>  }
>  
> 
> +static int ieee80211_sta_join_ibss(struct net_device *dev,
> +				   struct ieee80211_if_sta *ifsta,
> +				   struct ieee80211_sta_bss *bss)
> +{
> +	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> +	int res, rates, i, j;
> +	struct sk_buff *skb;
> +	struct ieee80211_mgmt *mgmt;
> +	struct ieee80211_tx_control control;
> +	struct rate_selection ratesel;
> +	u8 *pos;
> +	struct ieee80211_sub_if_data *sdata;
> +	struct ieee80211_supported_band *sband;
> +
> +	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
> +
> +	/* Remove possible STA entries from other IBSS networks. */
> +	sta_info_flush(local, NULL);
> +
> +	if (local->ops->reset_tsf) {
> +		/* Reset own TSF to allow time synchronization work. */
> +		local->ops->reset_tsf(local_to_hw(local));
> +	}
> +	memcpy(ifsta->bssid, bss->bssid, ETH_ALEN);
> +	res = ieee80211_if_config(dev);
> +	if (res)
> +		return res;
> +
> +	local->hw.conf.beacon_int = bss->beacon_int >= 10 ? bss->beacon_int : 10;
> +
> +	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +	sdata->drop_unencrypted = bss->capability &
> +		WLAN_CAPABILITY_PRIVACY ? 1 : 0;
> +
> +	res = ieee80211_set_freq(local, bss->freq);
> +
> +	if (local->oper_channel->flags & IEEE80211_CHAN_NO_IBSS) {
> +		printk(KERN_DEBUG "%s: IBSS not allowed on frequency "
> +		       "%d MHz\n", dev->name, local->oper_channel->center_freq);
> +		return -1;
> +	}
> +
> +	/* Set beacon template */
> +	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 400);
> +	do {
> +		if (!skb)
> +			break;
> +
> +		skb_reserve(skb, local->hw.extra_tx_headroom);
> +
> +		mgmt = (struct ieee80211_mgmt *)
> +			skb_put(skb, 24 + sizeof(mgmt->u.beacon));
> +		memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
> +		mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
> +						   IEEE80211_STYPE_BEACON);
> +		memset(mgmt->da, 0xff, ETH_ALEN);
> +		memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
> +		memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
> +		mgmt->u.beacon.beacon_int =
> +			cpu_to_le16(local->hw.conf.beacon_int);
> +		mgmt->u.beacon.capab_info = cpu_to_le16(bss->capability);
> +
> +		pos = skb_put(skb, 2 + ifsta->ssid_len);
> +		*pos++ = WLAN_EID_SSID;
> +		*pos++ = ifsta->ssid_len;
> +		memcpy(pos, ifsta->ssid, ifsta->ssid_len);
> +
> +		rates = bss->supp_rates_len;
> +		if (rates > 8)
> +			rates = 8;
> +		pos = skb_put(skb, 2 + rates);
> +		*pos++ = WLAN_EID_SUPP_RATES;
> +		*pos++ = rates;
> +		memcpy(pos, bss->supp_rates, rates);
> +
> +		if (bss->band == IEEE80211_BAND_2GHZ) {
> +			pos = skb_put(skb, 2 + 1);
> +			*pos++ = WLAN_EID_DS_PARAMS;
> +			*pos++ = 1;
> +			*pos++ = ieee80211_frequency_to_channel(bss->freq);
> +		}
> +
> +		pos = skb_put(skb, 2 + 2);
> +		*pos++ = WLAN_EID_IBSS_PARAMS;
> +		*pos++ = 2;
> +		/* FIX: set ATIM window based on scan results */
> +		*pos++ = 0;
> +		*pos++ = 0;
> +
> +		if (bss->supp_rates_len > 8) {
> +			rates = bss->supp_rates_len - 8;
> +			pos = skb_put(skb, 2 + rates);
> +			*pos++ = WLAN_EID_EXT_SUPP_RATES;
> +			*pos++ = rates;
> +			memcpy(pos, &bss->supp_rates[8], rates);
> +		}
> +
> +		memset(&control, 0, sizeof(control));
> +		rate_control_get_rate(dev, sband, skb, &ratesel);
> +		if (!ratesel.rate) {
> +			printk(KERN_DEBUG "%s: Failed to determine TX rate "
> +			       "for IBSS beacon\n", dev->name);
> +			break;
> +		}
> +		control.vif = &sdata->vif;
> +		control.tx_rate = ratesel.rate;
> +		if (sdata->bss_conf.use_short_preamble &&
> +		    ratesel.rate->flags & IEEE80211_RATE_SHORT_PREAMBLE)
> +			control.flags |= IEEE80211_TXCTL_SHORT_PREAMBLE;
> +		control.antenna_sel_tx = local->hw.conf.antenna_sel_tx;
> +		control.flags |= IEEE80211_TXCTL_NO_ACK;
> +		control.retry_limit = 1;
> +
> +		ifsta->probe_resp = skb_copy(skb, GFP_ATOMIC);
> +		if (ifsta->probe_resp) {
> +			mgmt = (struct ieee80211_mgmt *)
> +				ifsta->probe_resp->data;
> +			mgmt->frame_control =
> +				IEEE80211_FC(IEEE80211_FTYPE_MGMT,
> +					     IEEE80211_STYPE_PROBE_RESP);
> +		} else {
> +			printk(KERN_DEBUG "%s: Could not allocate ProbeResp "
> +			       "template for IBSS\n", dev->name);
> +		}
> +
> +		if (local->ops->beacon_update &&
> +		    local->ops->beacon_update(local_to_hw(local),
> +					     skb, &control) == 0) {
> +			printk(KERN_DEBUG "%s: Configured IBSS beacon "
> +			       "template\n", dev->name);
> +			skb = NULL;
> +		}
> +
> +		rates = 0;
> +		sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
> +		for (i = 0; i < bss->supp_rates_len; i++) {
> +			int bitrate = (bss->supp_rates[i] & 0x7f) * 5;
> +			for (j = 0; j < sband->n_bitrates; j++)
> +				if (sband->bitrates[j].bitrate == bitrate)
> +					rates |= BIT(j);
> +		}
> +		ifsta->supp_rates_bits[local->hw.conf.channel->band] = rates;
> +	} while (0);
> +
> +	if (skb) {
> +		printk(KERN_DEBUG "%s: Failed to configure IBSS beacon "
> +		       "template\n", dev->name);
> +		dev_kfree_skb(skb);
> +	}
> +
> +	ifsta->state = IEEE80211_IBSS_JOINED;
> +	mod_timer(&ifsta->timer, jiffies + IEEE80211_IBSS_MERGE_INTERVAL);
> +
> +	ieee80211_rx_bss_put(dev, bss);
> +
> +	return res;
> +}
> +
> +
>  static void ieee80211_rx_bss_info(struct net_device *dev,
>  				  struct ieee80211_mgmt *mgmt,
>  				  size_t len,
> @@ -2896,164 +3055,6 @@ static int ieee80211_sta_config_auth(struct net_device *dev,
>  	return -1;
>  }
>  
> -static int ieee80211_sta_join_ibss(struct net_device *dev,
> -				   struct ieee80211_if_sta *ifsta,
> -				   struct ieee80211_sta_bss *bss)
> -{
> -	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
> -	int res, rates, i, j;
> -	struct sk_buff *skb;
> -	struct ieee80211_mgmt *mgmt;
> -	struct ieee80211_tx_control control;
> -	struct rate_selection ratesel;
> -	u8 *pos;
> -	struct ieee80211_sub_if_data *sdata;
> -	struct ieee80211_supported_band *sband;
> -
> -	sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
> -
> -	/* Remove possible STA entries from other IBSS networks. */
> -	sta_info_flush(local, NULL);
> -
> -	if (local->ops->reset_tsf) {
> -		/* Reset own TSF to allow time synchronization work. */
> -		local->ops->reset_tsf(local_to_hw(local));
> -	}
> -	memcpy(ifsta->bssid, bss->bssid, ETH_ALEN);
> -	res = ieee80211_if_config(dev);
> -	if (res)
> -		return res;
> -
> -	local->hw.conf.beacon_int = bss->beacon_int >= 10 ? bss->beacon_int : 10;
> -
> -	sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> -	sdata->drop_unencrypted = bss->capability &
> -		WLAN_CAPABILITY_PRIVACY ? 1 : 0;
> -
> -	res = ieee80211_set_freq(local, bss->freq);
> -
> -	if (local->oper_channel->flags & IEEE80211_CHAN_NO_IBSS) {
> -		printk(KERN_DEBUG "%s: IBSS not allowed on frequency "
> -		       "%d MHz\n", dev->name, local->oper_channel->center_freq);
> -		return -1;
> -	}
> -
> -	/* Set beacon template based on scan results */
> -	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 400);
> -	do {
> -		if (!skb)
> -			break;
> -
> -		skb_reserve(skb, local->hw.extra_tx_headroom);
> -
> -		mgmt = (struct ieee80211_mgmt *)
> -			skb_put(skb, 24 + sizeof(mgmt->u.beacon));
> -		memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
> -		mgmt->frame_control = IEEE80211_FC(IEEE80211_FTYPE_MGMT,
> -						   IEEE80211_STYPE_BEACON);
> -		memset(mgmt->da, 0xff, ETH_ALEN);
> -		memcpy(mgmt->sa, dev->dev_addr, ETH_ALEN);
> -		memcpy(mgmt->bssid, ifsta->bssid, ETH_ALEN);
> -		mgmt->u.beacon.beacon_int =
> -			cpu_to_le16(local->hw.conf.beacon_int);
> -		mgmt->u.beacon.capab_info = cpu_to_le16(bss->capability);
> -
> -		pos = skb_put(skb, 2 + ifsta->ssid_len);
> -		*pos++ = WLAN_EID_SSID;
> -		*pos++ = ifsta->ssid_len;
> -		memcpy(pos, ifsta->ssid, ifsta->ssid_len);
> -
> -		rates = bss->supp_rates_len;
> -		if (rates > 8)
> -			rates = 8;
> -		pos = skb_put(skb, 2 + rates);
> -		*pos++ = WLAN_EID_SUPP_RATES;
> -		*pos++ = rates;
> -		memcpy(pos, bss->supp_rates, rates);
> -
> -		if (bss->band == IEEE80211_BAND_2GHZ) {
> -			pos = skb_put(skb, 2 + 1);
> -			*pos++ = WLAN_EID_DS_PARAMS;
> -			*pos++ = 1;
> -			*pos++ = ieee80211_frequency_to_channel(bss->freq);
> -		}
> -
> -		pos = skb_put(skb, 2 + 2);
> -		*pos++ = WLAN_EID_IBSS_PARAMS;
> -		*pos++ = 2;
> -		/* FIX: set ATIM window based on scan results */
> -		*pos++ = 0;
> -		*pos++ = 0;
> -
> -		if (bss->supp_rates_len > 8) {
> -			rates = bss->supp_rates_len - 8;
> -			pos = skb_put(skb, 2 + rates);
> -			*pos++ = WLAN_EID_EXT_SUPP_RATES;
> -			*pos++ = rates;
> -			memcpy(pos, &bss->supp_rates[8], rates);
> -		}
> -
> -		memset(&control, 0, sizeof(control));
> -		rate_control_get_rate(dev, sband, skb, &ratesel);
> -		if (!ratesel.rate) {
> -			printk(KERN_DEBUG "%s: Failed to determine TX rate "
> -			       "for IBSS beacon\n", dev->name);
> -			break;
> -		}
> -		control.vif = &sdata->vif;
> -		control.tx_rate = ratesel.rate;
> -		if (sdata->bss_conf.use_short_preamble &&
> -		    ratesel.rate->flags & IEEE80211_RATE_SHORT_PREAMBLE)
> -			control.flags |= IEEE80211_TXCTL_SHORT_PREAMBLE;
> -		control.antenna_sel_tx = local->hw.conf.antenna_sel_tx;
> -		control.flags |= IEEE80211_TXCTL_NO_ACK;
> -		control.retry_limit = 1;
> -
> -		ifsta->probe_resp = skb_copy(skb, GFP_ATOMIC);
> -		if (ifsta->probe_resp) {
> -			mgmt = (struct ieee80211_mgmt *)
> -				ifsta->probe_resp->data;
> -			mgmt->frame_control =
> -				IEEE80211_FC(IEEE80211_FTYPE_MGMT,
> -					     IEEE80211_STYPE_PROBE_RESP);
> -		} else {
> -			printk(KERN_DEBUG "%s: Could not allocate ProbeResp "
> -			       "template for IBSS\n", dev->name);
> -		}
> -
> -		if (local->ops->beacon_update &&
> -		    local->ops->beacon_update(local_to_hw(local),
> -					     skb, &control) == 0) {
> -			printk(KERN_DEBUG "%s: Configured IBSS beacon "
> -			       "template based on scan results\n", dev->name);
> -			skb = NULL;
> -		}
> -
> -		rates = 0;
> -		sband = local->hw.wiphy->bands[local->hw.conf.channel->band];
> -		for (i = 0; i < bss->supp_rates_len; i++) {
> -			int bitrate = (bss->supp_rates[i] & 0x7f) * 5;
> -			for (j = 0; j < sband->n_bitrates; j++)
> -				if (sband->bitrates[j].bitrate == bitrate)
> -					rates |= BIT(j);
> -		}
> -		ifsta->supp_rates_bits[local->hw.conf.channel->band] = rates;
> -	} while (0);
> -
> -	if (skb) {
> -		printk(KERN_DEBUG "%s: Failed to configure IBSS beacon "
> -		       "template\n", dev->name);
> -		dev_kfree_skb(skb);
> -	}
> -
> -	ifsta->state = IEEE80211_IBSS_JOINED;
> -	mod_timer(&ifsta->timer, jiffies + IEEE80211_IBSS_MERGE_INTERVAL);
> -
> -	ieee80211_rx_bss_put(dev, bss);
> -
> -	return res;
> -}
> -
>  
>  static int ieee80211_sta_create_ibss(struct net_device *dev,
>  				     struct ieee80211_if_sta *ifsta)
> 
> 

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

[PATCH 3/3] mac80211: enable IBSS merging

[Bruno Randolf]

note: ieee80211_get_bitrate() might be better places elsewhere, but i d=
on't
know where it would fit.

enable IBSS cell merging. if an IBSS beacon with the same channel, same=
 ESSID
and a TSF higher than the local TSF (mactime) is received, we have to j=
oin its
BSSID.  while this might not be immediately apparent from reading the 8=
02.11
standard it is compliant and necessary to make IBSS mode functional in =
many
cases. most drivers have a similar behaviour.

* move the relevant code section (previously only containing debug code=
) down
to the end of the function, so we can reuse the bss structure.

* we have to compare the mactime (TSF at the time of packet receive) ra=
ther
than the current TSF. since some drivers are not able to give a reliabl=
e
mactime we fall back to use the current TSF, which will be enough to ca=
tch most
(but not all) cases where an IBSS merge is necessary.

* in IBSS mode we want to allow beacons to override probe response info=
 so we
can correctly do merges.

* we don't only configure beacons based on scan results, so change that
message.

* to enable this we have to let all beacons thru in IBSS mode, even if =
they
have a different BSSID.

Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---

 include/net/mac80211.h       |    3 +
 net/mac80211/ieee80211_sta.c |  107 +++++++++++++++++++++++++++++++---=
--------
 net/mac80211/rx.c            |    5 ++
 3 files changed, 84 insertions(+), 31 deletions(-)


diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1b807f4..c634607 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -276,7 +276,8 @@ struct ieee80211_tx_control {
  * @RX_FLAG_FAILED_PLCP_CRC: Set this flag if the PCLP check failed on
  *	the frame.
  * @RX_FLAG_TSFT: The timestamp passed in the RX status (@mactime fiel=
d)
- *	is valid.
+ *	is valid. This is useful in monitor mode and necessary for beacon f=
rames
+ *	to enable IBSS merging.
  */
 enum mac80211_rx_flags {
 	RX_FLAG_MMIC_ERROR	=3D 1<<0,
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.=
c
index 72d7a86..8b24d1f 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -2189,6 +2189,19 @@ static int ieee80211_sta_join_ibss(struct net_de=
vice *dev,
 }
=20
=20
+static inline int ieee80211_get_bitrate(struct wiphy *phy,
+					enum ieee80211_band band, int rate_idx)
+{
+	struct ieee80211_supported_band *sband;
+	if (band > 0 && band < IEEE80211_NUM_BANDS) {
+		sband =3D phy->bands[band];
+		if (rate_idx > 0 && rate_idx < sband->n_bitrates)
+			return sband->bitrates[rate_idx].bitrate;
+	}
+	return 0;
+}
+
+
 static void ieee80211_rx_bss_info(struct net_device *dev,
 				  struct ieee80211_mgmt *mgmt,
 				  size_t len,
@@ -2202,7 +2215,7 @@ static void ieee80211_rx_bss_info(struct net_devi=
ce *dev,
 	struct ieee80211_sta_bss *bss;
 	struct sta_info *sta;
 	struct ieee80211_sub_if_data *sdata =3D IEEE80211_DEV_TO_SUB_IF(dev);
-	u64 timestamp;
+	u64 beacon_timestamp, rx_timestamp;
 	DECLARE_MAC_BUF(mac);
 	DECLARE_MAC_BUF(mac2);
=20
@@ -2219,31 +2232,7 @@ static void ieee80211_rx_bss_info(struct net_dev=
ice *dev,
 	if (baselen > len)
 		return;
=20
-	timestamp =3D le64_to_cpu(mgmt->u.beacon.timestamp);
-
-	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
-	    memcmp(mgmt->bssid, sdata->u.sta.bssid, ETH_ALEN) =3D=3D 0) {
-#ifdef CONFIG_MAC80211_IBSS_DEBUG
-		static unsigned long last_tsf_debug =3D 0;
-		u64 tsf;
-		if (local->ops->get_tsf)
-			tsf =3D local->ops->get_tsf(local_to_hw(local));
-		else
-			tsf =3D -1LLU;
-		if (time_after(jiffies, last_tsf_debug + 5 * HZ)) {
-			printk(KERN_DEBUG "RX beacon SA=3D%s BSSID=3D"
-			       "%s TSF=3D0x%llx BCN=3D0x%llx diff=3D%lld "
-			       "@%lu\n",
-			       print_mac(mac, mgmt->sa), print_mac(mac2, mgmt->bssid),
-			       (unsigned long long)tsf,
-			       (unsigned long long)timestamp,
-			       (unsigned long long)(tsf - timestamp),
-			       jiffies);
-			last_tsf_debug =3D jiffies;
-		}
-#endif /* CONFIG_MAC80211_IBSS_DEBUG */
-	}
-
+	beacon_timestamp =3D le64_to_cpu(mgmt->u.beacon.timestamp);
 	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems=
);
=20
 	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && elems.supp_rates=
 &&
@@ -2329,8 +2318,10 @@ static void ieee80211_rx_bss_info(struct net_dev=
ice *dev,
=20
 	bss->band =3D rx_status->band;
=20
-	if (bss->probe_resp && beacon) {
-		/* Do not allow beacon to override data from Probe Response. */
+	if (sdata->vif.type !=3D IEEE80211_IF_TYPE_IBSS &&
+	    bss->probe_resp && beacon) {
+		/* STA mode:
+		 * Do not allow beacon to override data from Probe Response. */
 		ieee80211_rx_bss_put(dev, bss);
 		return;
 	}
@@ -2427,13 +2418,71 @@ static void ieee80211_rx_bss_info(struct net_de=
vice *dev,
 		bss->ht_ie_len =3D 0;
 	}
=20
-	bss->timestamp =3D timestamp;
+	bss->timestamp =3D beacon_timestamp;
 	bss->last_update =3D jiffies;
 	bss->rssi =3D rx_status->ssi;
 	bss->signal =3D rx_status->signal;
 	bss->noise =3D rx_status->noise;
 	if (!beacon)
 		bss->probe_resp++;
+
+	/* check if we need to merge IBSS */
+	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
+	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
+	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
+	    bss->freq =3D=3D local->oper_channel->center_freq &&
+	    elems.ssid_len =3D=3D sdata->u.sta.ssid_len &&
+	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) =3D=3D=
 0) {
+		if (rx_status->flag & RX_FLAG_TSFT) {
+			/* in order for correct IBSS merging we need mactime
+			 *
+			 * since mactime is defined as the time the first data
+			 * symbol of the frame hits the PHY, and the timestamp
+			 * of the beacon is defined as "the time that the data
+			 * symbol containing the first bit of the timestamp is
+			 * transmitted to the PHY plus the transmitting STA=E2=80=99s
+			 * delays through its local PHY from the MAC-PHY
+			 * interface to its interface with the WM"
+			 * (802.11 11.1.2) - equals the time this bit arrives at
+			 * the receiver - we have to take into account the
+			 * offset between the two.
+			 * e.g: at 1 MBit that means mactime is 192 usec earlier
+			 * (=3D24 bytes * 8 usecs/byte) than the beacon timestamp.
+			 */
+			int rate =3D ieee80211_get_bitrate(local->hw.wiphy,
+					rx_status->band, rx_status->rate_idx);
+			if (rate)
+				rx_timestamp =3D rx_status->mactime +
+						(24 * 8 * 10 / rate);
+			else
+				rx_timestamp =3D rx_status->mactime;
+		} else if (local && local->ops && local->ops->get_tsf)
+			/* second best option: get current TSF */
+			rx_timestamp =3D local->ops->get_tsf(local_to_hw(local));
+		else
+			/* can't merge without knowing the TSF */
+			rx_timestamp =3D -1LLU;
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+		printk(KERN_DEBUG "RX beacon SA=3D%s BSSID=3D"
+		       "%s TSF=3D0x%llx BCN=3D0x%llx diff=3D%lld @%lu\n",
+		       print_mac(mac, mgmt->sa),
+		       print_mac(mac2, mgmt->bssid),
+		       (unsigned long long)rx_timestamp,
+		       (unsigned long long)beacon_timestamp,
+		       (unsigned long long)(rx_timestamp - beacon_timestamp),
+		       jiffies);
+#endif /* CONFIG_MAC80211_IBSS_DEBUG */
+		if (beacon_timestamp > rx_timestamp) {
+			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
+				printk(KERN_DEBUG "%s: beacon TSF higher than "
+				       "local TSF - IBSS merge with BSSID %s\n",
+				       dev->name, print_mac(mac, mgmt->bssid));
+			ieee80211_sta_join_ibss(dev, &sdata->u.sta, bss);
+			ieee80211_ibss_add_sta(dev, NULL,
+					       mgmt->bssid, mgmt->sa);
+		}
+	}
+
 	ieee80211_rx_bss_put(dev, bss);
 }
=20
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 943ba90..5dfe6f2 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1694,7 +1694,10 @@ static int prepare_for_handlers(struct ieee80211=
_sub_if_data *sdata,
 	case IEEE80211_IF_TYPE_IBSS:
 		if (!bssid)
 			return 0;
-		if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
+		if ((rx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_MGMT &&
+		    (rx->fc & IEEE80211_FCTL_STYPE) =3D=3D IEEE80211_STYPE_BEACON)
+			return 1;
+		else if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
 			if (!(rx->flags & IEEE80211_TXRXD_RXIN_SCAN))
 				return 0;
 			rx->flags &=3D ~IEEE80211_TXRXD_RXRA_MATCH;

-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] mac80211: enable IBSS merging

[Johannes Berg]

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


> +static inline int ieee80211_get_bitrate(struct wiphy *phy,
> +					enum ieee80211_band band, int rate_idx)
> +{
> +	struct ieee80211_supported_band *sband;
> +	if (band > 0 && band < IEEE80211_NUM_BANDS) {
> +		sband = phy->bands[band];
> +		if (rate_idx > 0 && rate_idx < sband->n_bitrates)
> +			return sband->bitrates[rate_idx].bitrate;
> +	}
> +	return 0;
> +}

You don't need to do this. In the rx info (struct ieee80211_txrx_data
rx) we already have the bitrate info available (rx->u.rx.rate), you
might not have access to it but __ieee80211_rx() refuses packets with an
invalid rate or band. Hence, you can leave out *all* the sanity checks
and just do

phy->bands[band]->bitrates[rate_idx]

which doesn't need an inline function.

johannes

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

[PATCH] mac80211: enable IBSS merging

[Bruno Randolf]

enable IBSS cell merging. if an IBSS beacon with the same channel, same=
 ESSID
and a TSF higher than the local TSF (mactime) is received, we have to j=
oin its
BSSID. while this might not be immediately apparent from reading the 80=
2.11
standard it is compliant and necessary to make IBSS mode functional in =
many
cases. most drivers have a similar behaviour.

* move the relevant code section (previously only containing debug code=
) down
to the end of the function, so we can reuse the bss structure.

* we have to compare the mactime (TSF at the time of packet receive) ra=
ther
than the current TSF. since mactime is defined as the time the first da=
ta
symbol arrived we add the time until byte 24 where the timestamp reside=
s, since
this is how the beacon timestamp is defined. as some some drivers are n=
ot able
to give a reliable mactime we fall back to use the current TSF, which w=
ill be
enough to catch most (but not all) cases where an IBSS merge is necessa=
ry.

* in IBSS mode we want to allow beacons to override probe response info=
 so we
can correctly do merges.

* we don't only configure beacons based on scan results, so change that
message.

* to enable this we have to let all beacons thru in IBSS mode, even if =
they
have a different BSSID.

Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---

 include/net/mac80211.h       |    3 +
 net/mac80211/ieee80211_sta.c |   90 ++++++++++++++++++++++++++++------=
--------
 net/mac80211/rx.c            |    5 ++
 3 files changed, 67 insertions(+), 31 deletions(-)


diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1b807f4..c634607 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -276,7 +276,8 @@ struct ieee80211_tx_control {
  * @RX_FLAG_FAILED_PLCP_CRC: Set this flag if the PCLP check failed on
  *	the frame.
  * @RX_FLAG_TSFT: The timestamp passed in the RX status (@mactime fiel=
d)
- *	is valid.
+ *	is valid. This is useful in monitor mode and necessary for beacon f=
rames
+ *	to enable IBSS merging.
  */
 enum mac80211_rx_flags {
 	RX_FLAG_MMIC_ERROR	=3D 1<<0,
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.=
c
index 72d7a86..fb96b51 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -2202,7 +2202,7 @@ static void ieee80211_rx_bss_info(struct net_devi=
ce *dev,
 	struct ieee80211_sta_bss *bss;
 	struct sta_info *sta;
 	struct ieee80211_sub_if_data *sdata =3D IEEE80211_DEV_TO_SUB_IF(dev);
-	u64 timestamp;
+	u64 beacon_timestamp, rx_timestamp;
 	DECLARE_MAC_BUF(mac);
 	DECLARE_MAC_BUF(mac2);
=20
@@ -2219,31 +2219,7 @@ static void ieee80211_rx_bss_info(struct net_dev=
ice *dev,
 	if (baselen > len)
 		return;
=20
-	timestamp =3D le64_to_cpu(mgmt->u.beacon.timestamp);
-
-	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
-	    memcmp(mgmt->bssid, sdata->u.sta.bssid, ETH_ALEN) =3D=3D 0) {
-#ifdef CONFIG_MAC80211_IBSS_DEBUG
-		static unsigned long last_tsf_debug =3D 0;
-		u64 tsf;
-		if (local->ops->get_tsf)
-			tsf =3D local->ops->get_tsf(local_to_hw(local));
-		else
-			tsf =3D -1LLU;
-		if (time_after(jiffies, last_tsf_debug + 5 * HZ)) {
-			printk(KERN_DEBUG "RX beacon SA=3D%s BSSID=3D"
-			       "%s TSF=3D0x%llx BCN=3D0x%llx diff=3D%lld "
-			       "@%lu\n",
-			       print_mac(mac, mgmt->sa), print_mac(mac2, mgmt->bssid),
-			       (unsigned long long)tsf,
-			       (unsigned long long)timestamp,
-			       (unsigned long long)(tsf - timestamp),
-			       jiffies);
-			last_tsf_debug =3D jiffies;
-		}
-#endif /* CONFIG_MAC80211_IBSS_DEBUG */
-	}
-
+	beacon_timestamp =3D le64_to_cpu(mgmt->u.beacon.timestamp);
 	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems=
);
=20
 	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && elems.supp_rates=
 &&
@@ -2329,8 +2305,10 @@ static void ieee80211_rx_bss_info(struct net_dev=
ice *dev,
=20
 	bss->band =3D rx_status->band;
=20
-	if (bss->probe_resp && beacon) {
-		/* Do not allow beacon to override data from Probe Response. */
+	if (sdata->vif.type !=3D IEEE80211_IF_TYPE_IBSS &&
+	    bss->probe_resp && beacon) {
+		/* STA mode:
+		 * Do not allow beacon to override data from Probe Response. */
 		ieee80211_rx_bss_put(dev, bss);
 		return;
 	}
@@ -2427,13 +2405,67 @@ static void ieee80211_rx_bss_info(struct net_de=
vice *dev,
 		bss->ht_ie_len =3D 0;
 	}
=20
-	bss->timestamp =3D timestamp;
+	bss->timestamp =3D beacon_timestamp;
 	bss->last_update =3D jiffies;
 	bss->rssi =3D rx_status->ssi;
 	bss->signal =3D rx_status->signal;
 	bss->noise =3D rx_status->noise;
 	if (!beacon)
 		bss->probe_resp++;
+
+	/* check if we need to merge IBSS */
+	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
+	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
+	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
+	    bss->freq =3D=3D local->oper_channel->center_freq &&
+	    elems.ssid_len =3D=3D sdata->u.sta.ssid_len &&
+	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) =3D=3D=
 0) {
+		if (rx_status->flag & RX_FLAG_TSFT) {
+			/* in order for correct IBSS merging we need mactime
+			 *
+			 * since mactime is defined as the time the first data
+			 * symbol of the frame hits the PHY, and the timestamp
+			 * of the beacon is defined as "the time that the data
+			 * symbol containing the first bit of the timestamp is
+			 * transmitted to the PHY plus the transmitting STA=E2=80=99s
+			 * delays through its local PHY from the MAC-PHY
+			 * interface to its interface with the WM"
+			 * (802.11 11.1.2) - equals the time this bit arrives at
+			 * the receiver - we have to take into account the
+			 * offset between the two.
+			 * e.g: at 1 MBit that means mactime is 192 usec earlier
+			 * (=3D24 bytes * 8 usecs/byte) than the beacon timestamp.
+			 */
+			int rate =3D local->hw.wiphy->bands[rx_status->band]->
+					bitrates[rx_status->rate_idx].bitrate;
+			rx_timestamp =3D rx_status->mactime + (24 * 8 * 10 / rate);
+		} else if (local && local->ops && local->ops->get_tsf)
+			/* second best option: get current TSF */
+			rx_timestamp =3D local->ops->get_tsf(local_to_hw(local));
+		else
+			/* can't merge without knowing the TSF */
+			rx_timestamp =3D -1LLU;
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+		printk(KERN_DEBUG "RX beacon SA=3D%s BSSID=3D"
+		       "%s TSF=3D0x%llx BCN=3D0x%llx diff=3D%lld @%lu\n",
+		       print_mac(mac, mgmt->sa),
+		       print_mac(mac2, mgmt->bssid),
+		       (unsigned long long)rx_timestamp,
+		       (unsigned long long)beacon_timestamp,
+		       (unsigned long long)(rx_timestamp - beacon_timestamp),
+		       jiffies);
+#endif /* CONFIG_MAC80211_IBSS_DEBUG */
+		if (beacon_timestamp > rx_timestamp) {
+			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
+				printk(KERN_DEBUG "%s: beacon TSF higher than "
+				       "local TSF - IBSS merge with BSSID %s\n",
+				       dev->name, print_mac(mac, mgmt->bssid));
+			ieee80211_sta_join_ibss(dev, &sdata->u.sta, bss);
+			ieee80211_ibss_add_sta(dev, NULL,
+					       mgmt->bssid, mgmt->sa);
+		}
+	}
+
 	ieee80211_rx_bss_put(dev, bss);
 }
=20
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 943ba90..5dfe6f2 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1694,7 +1694,10 @@ static int prepare_for_handlers(struct ieee80211=
_sub_if_data *sdata,
 	case IEEE80211_IF_TYPE_IBSS:
 		if (!bssid)
 			return 0;
-		if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
+		if ((rx->fc & IEEE80211_FCTL_FTYPE) =3D=3D IEEE80211_FTYPE_MGMT &&
+		    (rx->fc & IEEE80211_FCTL_STYPE) =3D=3D IEEE80211_STYPE_BEACON)
+			return 1;
+		else if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
 			if (!(rx->flags & IEEE80211_TXRXD_RXIN_SCAN))
 				return 0;
 			rx->flags &=3D ~IEEE80211_TXRXD_RXRA_MATCH;

-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


On Sat, 2008-02-16 at 11:29 +0900, Bruno Randolf wrote:
> enable IBSS cell merging. if an IBSS beacon with the same channel, same ESSID
> and a TSF higher than the local TSF (mactime) is received, we have to join its
> BSSID. while this might not be immediately apparent from reading the 802.11
> standard it is compliant and necessary to make IBSS mode functional in many
> cases. most drivers have a similar behaviour.

> [...]

Looks good, thanks for your patience. I have two minor comments still,
sorry for the lack of focus in earlier review.

> +		} else if (local && local->ops && local->ops->get_tsf)
> +			/* second best option: get current TSF */
> +			rx_timestamp = local->ops->get_tsf(local_to_hw(local));

This is assuming that we don't manage to process the frame within 192
usecs. I guess that will be true since we defer it to a tasklet, but do
we want to bet on it in the future or should we simply widen the window
where the merge *won't* happen because the driver doesn't provide enough
info and also add the 24 bytes offset here?

Also something else that just occurred to me... You wrote in the
description:

> * to enable this we have to let all beacons thru in IBSS mode, even if
> they have a different BSSID.

which I think refers to the last hunk of your patch. However, hardware
also has such filters, so I think we also should set the
FIF_BCN_PRBRESP_PROMISC filter flag when IBSS is enabled, we don't want
to force drivers to do this implicitly when IBSS interfaces are added.

Again, thanks for your patience, and if you want this patch merged now
here's my

Acked-by: Johannes Berg <johannes@sipsolutions.net>

and we can address these remaining two points in follow-up patches
instead of respinning this all the time.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Sunday 17 February 2008 18:11:43 Johannes Berg wrote:
> Looks good, thanks for your patience. 

thanks for your review :)

> I have two minor comments still, 
> sorry for the lack of focus in earlier review.
>
> > +		} else if (local && local->ops && local->ops->get_tsf)
> > +			/* second best option: get current TSF */
> > +			rx_timestamp = local->ops->get_tsf(local_to_hw(local));
>
> This is assuming that we don't manage to process the frame within 192
> usecs. I guess that will be true since we defer it to a tasklet, but do
> we want to bet on it in the future or should we simply widen the window
> where the merge *won't* happen because the driver doesn't provide enough
> info and also add the 24 bytes offset here?

true. i'll change that and resend the patch. i have to rebase it anyways for 
the current tree.

> Also something else that just occurred to me... You wrote in the
>
> description:
> > * to enable this we have to let all beacons thru in IBSS mode, even if
> > they have a different BSSID.
>
> which I think refers to the last hunk of your patch. However, hardware
> also has such filters, so I think we also should set the
> FIF_BCN_PRBRESP_PROMISC filter flag when IBSS is enabled, we don't want
> to force drivers to do this implicitly when IBSS interfaces are added.

hmm, i don't know about that and i don't seem to need that for ath5k, so can 
we do that in a follow up patch, please?

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> > which I think refers to the last hunk of your patch. However, hardware
> > also has such filters, so I think we also should set the
> > FIF_BCN_PRBRESP_PROMISC filter flag when IBSS is enabled, we don't want
> > to force drivers to do this implicitly when IBSS interfaces are added.
> 
> hmm, i don't know about that and i don't seem to need that for ath5k, so can 
> we do that in a follow up patch, please?

Yeah sure, we can do that.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Sunday 17 February 2008 18:11:43 Johannes Berg wrote:
> Looks good, thanks for your patience. I have two minor comments still,
> sorry for the lack of focus in earlier review.
>
> > +		} else if (local && local->ops && local->ops->get_tsf)
> > +			/* second best option: get current TSF */
> > +			rx_timestamp = local->ops->get_tsf(local_to_hw(local));
>
> This is assuming that we don't manage to process the frame within 192
> usecs. I guess that will be true since we defer it to a tasklet, but do
> we want to bet on it in the future or should we simply widen the window
> where the merge *won't* happen because the driver doesn't provide enough
> info and also add the 24 bytes offset here?

sorry, i was to fast in agreeing ;)
on a second thought i realized that we don't need to worry about that:

since we get to handle the frame after it was received *completely*, we can be 
sure that the current time returned by get_tsf() is later than the time when 
byte 24 was received. so no need for adding the offset.

i'll rebase the unmodified patch series and resend it.

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> > This is assuming that we don't manage to process the frame within 192
> > usecs. I guess that will be true since we defer it to a tasklet, but do
> > we want to bet on it in the future or should we simply widen the window
> > where the merge *won't* happen because the driver doesn't provide enough
> > info and also add the 24 bytes offset here?
> 
> sorry, i was to fast in agreeing ;)
> on a second thought i realized that we don't need to worry about that:
> 
> since we get to handle the frame after it was received *completely*, we can be 
> sure that the current time returned by get_tsf() is later than the time when 
> byte 24 was received. so no need for adding the offset.

Hah, good point.

johannes

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

Re: [PATCH 1/3] mac80211: better definition of mactime

[Johannes Berg]

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


On Fri, 2008-02-15 at 16:01 +0900, Bruno Randolf wrote:
> define mactime as the time when the first data symbol arrived at the HW. the
> old definition was questionable because 802.11 defines timestamp only for
> beacon and probe response frames, and there it means the timestamp field.
> 
> a stricter definition of mactime is necessary for correct merging of IBSS.
> 
> note that it is up to the driver to convert whatever its hardware returns to
> this definition. unfortunately we don't know for example when atheros hardware
> takes its rx timestamp exactly :(
> 
> Signed-off-by: Bruno Randolf <bruno@thinktube.com>

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> 
>  include/net/mac80211.h |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index 460da54..1b807f4 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -295,7 +295,8 @@ enum mac80211_rx_flags {
>   * The low-level driver should provide this information (the subset
>   * supported by hardware) to the 802.11 code with each received
>   * frame.
> - * @mactime: MAC timestamp as defined by 802.11
> + * @mactime: value in microseconds of the 64-bit Time Synchronization Function
> + * 	(TSF) timer when the first data symbol (MPDU) arrived at the hardware.
>   * @band: the active band when this frame was received
>   * @freq: frequency the radio was tuned to when receiving this frame, in MHz
>   * @ssi: signal strength when receiving this frame
> 
> 

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

Re: [PATCH] mac80211: enable IBSS merging

[Joerg Pommnitz]

Johannes Berg wrote:
> I think we should simply allow setting the BSSID (SIOCSIWAP maybe?) on
> IBSS interfaces and punt this issue to userspace.

This is exactly what we did/do with a Madwifi based IBSS network. I would
miss this feature when we switch to ath5k. So if you are taking votes on 
this, you will get mine.

Regards
  Joerg


      Lesen Sie Ihre E-Mails jetzt einfach von unterwegs.
www.yahoo.de/go

Re: [PATCH 2/2] mac80211: enable IBSS merging

[Johannes Berg]

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


> +	/* check if we need to merge IBSS */
> +	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
> +	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
> +	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> +	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0) {

This needs to test whether elems.ssid_len == sdata->u.sta.ssid_len
(before doing the memcmp to not overrun the frame).

> +		if (rx_status->flag & RX_FLAG_TSFT)
> +			/* in order for correct IBSS merging we need mactime */
> +			mactime = rx_status->mactime;

This needs to be documented (with the RX flag I guess) and b43(legacy)
should be changed to include the TSFT in beacon frames in IBSS mode...

johannes

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

[PATCH] mac80211: enable IBSS merging

[Bruno Randolf]

addressing johannes latest comments and adding a channel check...

enable IBSS cell merging. if an IBSS beacon with the same channel, same ESSID
and a TSF higher than the local TSF (mactime) is received, we have to join its
BSSID.  while this might not be immediately apparent from reading the 802.11
standard it is compliant and necessary to make IBSS mode functional in many
cases. most drivers have a similar behaviour.

* move the relevant code section (previously only containing debug code) down
to the end of the function, so we can reuse the bss structure.

* we have to compare the mactime (TSF at the time of packet receive) rather
than the current TSF. since some drivers are not able to give a reliable
mactime we fall back to use the current TSF, which will be enough to catch most
(but not all) cases where an IBSS merge is necessary.

* in IBSS mode we want to allow beacons to override probe response info so we
can correctly do merges.

* we don't only configure beacons based on scan results, so change that
message.

* to enable this we have to let all beacons thru in IBSS mode, even if they
have a different BSSID.

Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---

 include/net/mac80211.h       |    3 +-
 net/mac80211/ieee80211_sta.c |   70 ++++++++++++++++++++++++++----------------
 net/mac80211/rx.c            |    5 ++-
 3 files changed, 49 insertions(+), 29 deletions(-)


diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 460da54..4ae3a12 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -276,7 +276,8 @@ struct ieee80211_tx_control {
  * @RX_FLAG_FAILED_PLCP_CRC: Set this flag if the PCLP check failed on
  *	the frame.
  * @RX_FLAG_TSFT: The timestamp passed in the RX status (@mactime field)
- *	is valid.
+ *	is valid. This is useful in monitor mode and necessary for beacon frames
+ *	to enable IBSS merging.
  */
 enum mac80211_rx_flags {
 	RX_FLAG_MMIC_ERROR	= 1<<0,
diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index 08f89fa..4df18ae 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -2202,7 +2202,7 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 	struct ieee80211_sta_bss *bss;
 	struct sta_info *sta;
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	u64 timestamp;
+	u64 timestamp, mactime;
 	DECLARE_MAC_BUF(mac);
 	DECLARE_MAC_BUF(mac2);
 
@@ -2220,30 +2220,6 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 		return;
 
 	timestamp = le64_to_cpu(mgmt->u.beacon.timestamp);
-
-	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
-	    memcmp(mgmt->bssid, sdata->u.sta.bssid, ETH_ALEN) == 0) {
-#ifdef CONFIG_MAC80211_IBSS_DEBUG
-		static unsigned long last_tsf_debug = 0;
-		u64 tsf;
-		if (local->ops->get_tsf)
-			tsf = local->ops->get_tsf(local_to_hw(local));
-		else
-			tsf = -1LLU;
-		if (time_after(jiffies, last_tsf_debug + 5 * HZ)) {
-			printk(KERN_DEBUG "RX beacon SA=%s BSSID="
-			       "%s TSF=0x%llx BCN=0x%llx diff=%lld "
-			       "@%lu\n",
-			       print_mac(mac, mgmt->sa), print_mac(mac2, mgmt->bssid),
-			       (unsigned long long)tsf,
-			       (unsigned long long)timestamp,
-			       (unsigned long long)(tsf - timestamp),
-			       jiffies);
-			last_tsf_debug = jiffies;
-		}
-#endif /* CONFIG_MAC80211_IBSS_DEBUG */
-	}
-
 	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems);
 
 	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && elems.supp_rates &&
@@ -2329,8 +2305,10 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 
 	bss->band = rx_status->band;
 
-	if (bss->probe_resp && beacon) {
-		/* Do not allow beacon to override data from Probe Response. */
+	if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
+	    bss->probe_resp && beacon) {
+		/* STA mode:
+		 * Do not allow beacon to override data from Probe Response. */
 		ieee80211_rx_bss_put(dev, bss);
 		return;
 	}
@@ -2434,6 +2412,44 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 	bss->noise = rx_status->noise;
 	if (!beacon)
 		bss->probe_resp++;
+
+	/* check if we need to merge IBSS */
+	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
+	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
+	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
+	    bss->freq == local->oper_channel->center_freq &&
+	    elems.ssid_len == sdata->u.sta.ssid_len &&
+	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0) {
+		if (rx_status->flag & RX_FLAG_TSFT)
+			/* in order for correct IBSS merging we need mactime */
+			mactime = rx_status->mactime;
+		else if (local && local->ops && local->ops->get_tsf)
+			/* second best option: get current TSF */
+			mactime = local->ops->get_tsf(local_to_hw(local));
+		else
+			/* can't merge without knowing the TSF */
+			mactime = -1LLU;
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+		printk(KERN_DEBUG "RX beacon SA=%s BSSID="
+		       "%s TSF=0x%llx BCN=0x%llx diff=%lld @%lu\n",
+		       print_mac(mac, mgmt->sa),
+		       print_mac(mac2, mgmt->bssid),
+		       (unsigned long long)mactime,
+		       (unsigned long long)timestamp,
+		       (unsigned long long)(mactime - timestamp),
+		       jiffies);
+#endif /* CONFIG_MAC80211_IBSS_DEBUG */
+		if (mactime <= timestamp) {
+			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
+				printk(KERN_DEBUG "%s: beacon TSF higher than "
+				       "local TSF - IBSS merge with BSSID %s\n",
+				       dev->name, print_mac(mac, mgmt->bssid));
+			ieee80211_sta_join_ibss(dev, &sdata->u.sta, bss);
+			ieee80211_ibss_add_sta(dev, NULL,
+					       mgmt->bssid, mgmt->sa);
+		}
+	}
+
 	ieee80211_rx_bss_put(dev, bss);
 }
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index beda1bf..ed7cf37 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1612,7 +1612,10 @@ static int prepare_for_handlers(struct ieee80211_sub_if_data *sdata,
 	case IEEE80211_IF_TYPE_IBSS:
 		if (!bssid)
 			return 0;
-		if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
+		if ((rx->fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_MGMT &&
+		    (rx->fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BEACON)
+			return 1;
+		else if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
 			if (!(rx->flags & IEEE80211_TXRXD_RXIN_SCAN))
 				return 0;
 			rx->flags &= ~IEEE80211_TXRXD_RXRA_MATCH;

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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

Looks fine, one last question:

> +		if (mactime <= timestamp) {
> +			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
> +				printk(KERN_DEBUG "%s: beacon TSF higher than "
> +				       "local TSF - IBSS merge with BSSID %s\n",
> +				       dev->name, print_mac(mac, mgmt->bssid));

I'd rewrite that as timestamp >= mactime and rename the two variables
(maybe beacon_timestamp and phy_timestamp or so?), but that's not too
important. However, in any case "<=" seems wrong, shouldn't we only
merge when it's actually higher? If your hardware/firmware has synced
properly and the PHY delay is accounted for properly etc. then every
beacon from the own BSS will fulfil the == part of the condition, no?

Also, the beacon timestamp is defined as follows:

        A STA sending a beacon shall set the value of the beacon’s
        timestamp so that it equals the value of the STA’s TSF timer at
        the time that the data symbol containing the first bit of the
        timestamp is transmitted to the PHY plus the transmitting STA’s
        delays through its local PHY from the MAC-PHY interface to its
        interface with the WM [e.g., antenna, light-emitting diode (LED)
        emission surface].

(IEEE 802.11 11.1.2)

Since we define the RX timestamp to be when the first data symbol of the
frame hits the PHY, we here have to take into account the offset between
the two, at 1 MBit that's 192 (=24 bytes * 8 usecs/byte) usecs earlier
than the beacon timestamp.

On the other hand, you're unlikely to hit that window, but I suppose it
warrants at least a comment.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Feb 6, 2008 6:52 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> Looks fine, one last question:
>
> > +             if (mactime <= timestamp) {
> > +                     if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
> > +                             printk(KERN_DEBUG "%s: beacon TSF higher than "
> > +                                    "local TSF - IBSS merge with BSSID %s\n",
> > +                                    dev->name, print_mac(mac, mgmt->bssid));
>
> I'd rewrite that as timestamp >= mactime and rename the two variables
> (maybe beacon_timestamp and phy_timestamp or so?), but that's not too
> important. However, in any case "<=" seems wrong, shouldn't we only
> merge when it's actually higher? If your hardware/firmware has synced
> properly and the PHY delay is accounted for properly etc. then every
> beacon from the own BSS will fulfil the == part of the condition, no?
>
> Also, the beacon timestamp is defined as follows:
>
>         A STA sending a beacon shall set the value of the beacon's
>         timestamp so that it equals the value of the STA's TSF timer at
>         the time that the data symbol containing the first bit of the
>         timestamp is transmitted to the PHY plus the transmitting STA's
>         delays through its local PHY from the MAC-PHY interface to its
>         interface with the WM [e.g., antenna, light-emitting diode (LED)
>         emission surface].
>
> (IEEE 802.11 11.1.2)
>
> Since we define the RX timestamp to be when the first data symbol of the
> frame hits the PHY, we here have to take into account the offset between
> the two, at 1 MBit that's 192 (=24 bytes * 8 usecs/byte) usecs earlier
> than the beacon timestamp.
>
> On the other hand, you're unlikely to hit that window, but I suppose it
> warrants at least a comment.

Agreed, "mactime" is pretty vague, if we can add some of the above to
the documentation it would probably help other driver developers.

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Thursday 07 February 2008 08:52:35 Johannes Berg wrote:
> Looks fine, one last question:
> > +		if (mactime <=3D timestamp) {
> > +			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
> > +				printk(KERN_DEBUG "%s: beacon TSF higher than "
> > +				       "local TSF - IBSS merge with BSSID %s\n",
> > +				       dev->name, print_mac(mac, mgmt->bssid));
>
> I'd rewrite that as timestamp >=3D mactime and rename the two variabl=
es
> (maybe beacon_timestamp and phy_timestamp or so?), but that's not too
> important.

i'll do that, sure.

> However, in any case "<=3D" seems wrong, shouldn't we only=20
> merge when it's actually higher? If your hardware/firmware has synced
> properly and the PHY delay is accounted for properly etc. then every
> beacon from the own BSS will fulfil the =3D=3D part of the condition,=
 no?

true. i was not aware that the beacon timestamp is taking PHY delays in=
to=20
account and wrongly asumed that the RX timestamp should always be a bit=
=20
later.
(which is also what i saw on my atheros hardware: we usually get the RX=
=20
timestamp about 60 to 300 usec later than the beacon timestamp, but it'=
s=20
quite likely that we have not calibrated everything 100% correctly.)

> Also, the beacon timestamp is defined as follows:
>
>         A STA sending a beacon shall set the value of the beacon=E2=80=
=99s
>         timestamp so that it equals the value of the STA=E2=80=99s TS=
=46 timer at
>         the time that the data symbol containing the first bit of the
>         timestamp is transmitted to the PHY plus the transmitting STA=
=E2=80=99s
>         delays through its local PHY from the MAC-PHY interface to it=
s
>         interface with the WM [e.g., antenna, light-emitting diode (L=
ED)
>         emission surface].
>
> (IEEE 802.11 11.1.2)
>
> Since we define the RX timestamp to be when the first data symbol of =
the
> frame hits the PHY, we here have to take into account the offset betw=
een
> the two, at 1 MBit that's 192 (=3D24 bytes * 8 usecs/byte) usecs earl=
ier
> than the beacon timestamp.
>
> On the other hand, you're unlikely to hit that window, but I suppose =
it
> warrants at least a comment.

thats a very good point! if everything was calibrated properly and all =
delays=20
taken into account *exactly* that would cause us to go thru a merge all=
 the=20
time, on every received beacon. i think we have to take that into accou=
nt,=20
for different bitrates. what about something like:

timestamp >=3D ( mactime + (24 * 8 / beacon_phy_rate_in_mbps ))

i'll resend my patch adding that and the other proposed changes soon.

bruno
-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> thats a very good point! if everything was calibrated properly and all delays 
> taken into account *exactly* that would cause us to go thru a merge all the 
> time, on every received beacon. i think we have to take that into account, 
> for different bitrates. what about something like:
> 
> timestamp >= ( mactime + (24 * 8 / beacon_phy_rate_in_mbps ))

Looks reasonable, though I'm not entirely sure how we defined 'mactime'
and whether it is at the first data or the first phy symbol. I think
it's at the first data symbol which makes this correct.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Tuesday 12 February 2008 18:50:19 Johannes Berg wrote:
> > thats a very good point! if everything was calibrated properly and =
all
> > delays taken into account *exactly* that would cause us to go thru =
a
> > merge all the time, on every received beacon. i think we have to ta=
ke
> > that into account, for different bitrates. what about something lik=
e:
> >
> > timestamp >=3D ( mactime + (24 * 8 / beacon_phy_rate_in_mbps ))
>
> Looks reasonable, though I'm not entirely sure how we defined 'mactim=
e'
> and whether it is at the first data or the first phy symbol. I think
> it's at the first data symbol which makes this correct.

hmm, it seems we don't have a strict definition of mactime... the kerne=
ldoc of=20
struct ieee80211_rx_status says:
 * @mactime: MAC timestamp as defined by 802.11

but as far as i understand 802.11, it only defines "timestamp" for beac=
on and=20
probe response frames, where it actually means the timestamp field of t=
he=20
frame.

"Local Time" is defined as: "The value of the STA=E2=80=99s TSF timer a=
t the start of=20
reception of the first octet of the timestamp field of the received fra=
me=20
(probe response or beacon) from the found BSS." [802.11 p. 103]

also in "Process Validate_MPDU: it says:
"Save arrival time of first octet of {what may be a} timestamp field."
[802.11 p 391 and 460]

which could imply that this value is given as a rx timestamp (mactime).=
 in=20
that case we could directly compare timestamp and mactime.

radiotap on the other hand defines it as the first bit of the data (MPD=
U):
 * IEEE80211_RADIOTAP_TSFT              u_int64_t       microseconds
 *
 *      Value in microseconds of the MAC's 64-bit 802.11 Time
 *      Synchronization Function timer when the first bit of the
 *      MPDU arrived at the MAC. For received frames, only.

so i guess right now it depends on the hardware what we actually get as=
=20
mactime...

i could not find any definition when the rx timestamp of atheros hardwa=
re is=20
taken.

do we have that information for other hardware? is mactime
 *) the TSF at the reception of the first phy symbol?
 *) the TSF at the reception of the first data (MPDU) symbol
 *) the TSF at the reception of the byte 24 (where the timestamp field =
of
    beacon and probe response frames is)?

bruno
-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> hmm, it seems we don't have a strict definition of mactime... the kerneldoc of 
> struct ieee80211_rx_status says:
>  * @mactime: MAC timestamp as defined by 802.11

Heh.

> radiotap on the other hand defines it as the first bit of the data (MPDU):
>  * IEEE80211_RADIOTAP_TSFT              u_int64_t       microseconds
>  *
>  *      Value in microseconds of the MAC's 64-bit 802.11 Time
>  *      Synchronization Function timer when the first bit of the
>  *      MPDU arrived at the MAC. For received frames, only.

Yeah I think that makes most sense and is how it is usually implemented
anyway.

> so i guess right now it depends on the hardware what we actually get as 
> mactime...

> do we have that information for other hardware? is mactime
>  *) the TSF at the reception of the first phy symbol?
>  *) the TSF at the reception of the first data (MPDU) symbol
>  *) the TSF at the reception of the byte 24 (where the timestamp field of
>     beacon and probe response frames is)?

I very much doubt there is hardware that does the last option because
there may be frames that are shorted than 24 bytes :)

The difference between the first two options is a simple constant
(depending on the PHY mode) so the driver for hardware that supports
either one of those two options can easily translate them. Hence, I
think we should declare it like radiotap does.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> true. i was not aware that the beacon timestamp is taking PHY delays into 
> account and wrongly asumed that the RX timestamp should always be a bit 
> later.
> (which is also what i saw on my atheros hardware: we usually get the RX 
> timestamp about 60 to 300 usec later than the beacon timestamp, but it's 
> quite likely that we have not calibrated everything 100% correctly.)

Later actually surprises me, are you sure it's not earlier? If
everything was calibrated perfectly it should be 192 usecs earlier (24*8
bytes at the most common 1mbit beacon rate).

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Tuesday 12 February 2008 18:52:03 Johannes Berg wrote:
> > true. i was not aware that the beacon timestamp is taking PHY delays into
> > account and wrongly asumed that the RX timestamp should always be a bit
> > later.
> > (which is also what i saw on my atheros hardware: we usually get the RX
> > timestamp about 60 to 300 usec later than the beacon timestamp, but it's
> > quite likely that we have not calibrated everything 100% correctly.)
>
> Later actually surprises me, are you sure it's not earlier? If
> everything was calibrated perfectly it should be 192 usecs earlier (24*8
> bytes at the most common 1mbit beacon rate).

yes, i'm sure that it's later. our debug printk shows diff= (rx_tsf - 
beacon_tsf) and it's positive.

ulrich meis also confirmed that, but i think his message to the list got lost.

i'm not so surprised about that because the hardware would have to be 
calibrated 100% correctly to compensate for the TX delay as well as the RX 
delay (if both sides use ath5k) and it's really quite likely that we don't do 
that right for ath5k yet.

please see also my previous mail, i think that mactime is not well defined as 
well :(

bruno

[PATCH] mac80211: enable IBSS merging

[Bruno Randolf]

enable IBSS cell merging. if an IBSS beacon with the same ESSID and a TSF
higher than the local TSF (mactime) is received, we have to join its BSSID.

* move the relevant code section (previously only containing debug code) down
to the end of the function, so we can reuse the bss structure.

* we have to compare the mactime (TSF at the time of packet receive) rather
than the current TSF.

* in IBSS mode we want to allow beacons to override probe response info so we
can correctly do merges.

* we don't only configure beacons based on scan results, so change that
message.

* to enable all this we have to let all beacons thru in IBSS mode, even if they
have a different BSSID.

Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---

 net/mac80211/ieee80211_sta.c |   75 ++++++++++++++++++++++++++----------------
 net/mac80211/rx.c            |    5 ++-
 2 files changed, 51 insertions(+), 29 deletions(-)


diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c
index e7da1cd..f6b76a3 100644
--- a/net/mac80211/ieee80211_sta.c
+++ b/net/mac80211/ieee80211_sta.c
@@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_device *dev,
 				 struct ieee80211_sta_bss *bss);
 static int ieee80211_sta_find_ibss(struct net_device *dev,
 				   struct ieee80211_if_sta *ifsta);
+static int ieee80211_sta_join_ibss(struct net_device *dev,
+				   struct ieee80211_if_sta *ifsta,
+				   struct ieee80211_sta_bss *bss);
 static int ieee80211_sta_wep_configured(struct net_device *dev);
 static int ieee80211_sta_start_scan(struct net_device *dev,
 				    u8 *ssid, size_t ssid_len);
@@ -1825,7 +1828,7 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 	struct ieee80211_sta_bss *bss;
 	struct sta_info *sta;
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
-	u64 timestamp;
+	u64 timestamp, mactime;
 	DECLARE_MAC_BUF(mac);
 	DECLARE_MAC_BUF(mac2);
 
@@ -1843,30 +1846,6 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 		return;
 
 	timestamp = le64_to_cpu(mgmt->u.beacon.timestamp);
-
-	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
-	    memcmp(mgmt->bssid, sdata->u.sta.bssid, ETH_ALEN) == 0) {
-#ifdef CONFIG_MAC80211_IBSS_DEBUG
-		static unsigned long last_tsf_debug = 0;
-		u64 tsf;
-		if (local->ops->get_tsf)
-			tsf = local->ops->get_tsf(local_to_hw(local));
-		else
-			tsf = -1LLU;
-		if (time_after(jiffies, last_tsf_debug + 5 * HZ)) {
-			printk(KERN_DEBUG "RX beacon SA=%s BSSID="
-			       "%s TSF=0x%llx BCN=0x%llx diff=%lld "
-			       "@%lu\n",
-			       print_mac(mac, mgmt->sa), print_mac(mac2, mgmt->bssid),
-			       (unsigned long long)tsf,
-			       (unsigned long long)timestamp,
-			       (unsigned long long)(tsf - timestamp),
-			       jiffies);
-			last_tsf_debug = jiffies;
-		}
-#endif /* CONFIG_MAC80211_IBSS_DEBUG */
-	}
-
 	ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems);
 
 	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && elems.supp_rates &&
@@ -1951,7 +1930,8 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 #endif
 	}
 
-	if (bss->probe_resp && beacon) {
+	if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
+	    bss->probe_resp && beacon) {
 		/* Do not allow beacon to override data from Probe Response. */
 		ieee80211_rx_bss_put(dev, bss);
 		return;
@@ -2070,6 +2050,45 @@ static void ieee80211_rx_bss_info(struct net_device *dev,
 	bss->noise = rx_status->noise;
 	if (!beacon)
 		bss->probe_resp++;
+
+	/* check if we need to merge IBSS */
+	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
+	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
+	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
+	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0) {
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+		static unsigned long last_tsf_debug;
+#endif
+		if (rx_status->flag & RX_FLAG_TSFT)
+			mactime = rx_status->mactime;
+		else {
+			mactime = -1LLU;
+			printk(KERN_WARNING "%s: IBSS mode needs mactime for "
+				"beacons\n", dev->name);
+		}
+#ifdef CONFIG_MAC80211_IBSS_DEBUG
+		if (time_after(jiffies, last_tsf_debug + 5 * HZ)) {
+			printk(KERN_DEBUG "RX beacon SA=%s BSSID="
+			       "%s TSF=0x%llx BCN=0x%llx diff=%lld @%lu\n",
+			       print_mac(mac, mgmt->sa),
+			       print_mac(mac2, mgmt->bssid),
+			       (unsigned long long)mactime,
+			       (unsigned long long)timestamp,
+			       (unsigned long long)(mactime - timestamp),
+			       jiffies);
+			last_tsf_debug = jiffies;
+		}
+#endif /* CONFIG_MAC80211_IBSS_DEBUG */
+		if (mactime <= timestamp) {
+			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
+				" -> IBSS merge with BSSID %s\n",
+				dev->name, print_mac(mac, mgmt->bssid));
+			ieee80211_sta_join_ibss(dev, &sdata->u.sta, bss);
+			ieee80211_ibss_add_sta(dev, NULL,
+				mgmt->bssid, mgmt->sa);
+		}
+	}
+
 	ieee80211_rx_bss_put(dev, bss);
 }
 
@@ -2724,7 +2743,7 @@ static int ieee80211_sta_join_ibss(struct net_device *dev,
 		return -1;
 	}
 
-	/* Set beacon template based on scan results */
+	/* Set beacon template */
 	skb = dev_alloc_skb(local->hw.extra_tx_headroom + 400);
 	do {
 		if (!skb)
@@ -2810,7 +2829,7 @@ static int ieee80211_sta_join_ibss(struct net_device *dev,
 		    local->ops->beacon_update(local_to_hw(local),
 					     skb, &control) == 0) {
 			printk(KERN_DEBUG "%s: Configured IBSS beacon "
-			       "template based on scan results\n", dev->name);
+			       "template\n", dev->name);
 			skb = NULL;
 		}
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 6cc1e7e..5617e17 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1609,7 +1609,10 @@ static int prepare_for_handlers(struct ieee80211_sub_if_data *sdata,
 	case IEEE80211_IF_TYPE_IBSS:
 		if (!bssid)
 			return 0;
-		if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
+		if ((rx->fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_MGMT &&
+		    (rx->fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BEACON)
+			return 1;
+		else if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
 			if (!(rx->flags & IEEE80211_TXRXD_RXIN_SCAN))
 				return 0;
 			rx->flags &= ~IEEE80211_TXRXD_RXRA_MATCH;

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

Adding Johill and Michael as they are maintainers of mac80211 and they
need to review this. Please add them on further mac80211 patches.

Note to driver authors: zd1211rw, rt2x00 drivers and adm8211 will need
to set mactime if they want IBSS merge to work as this functionality
would be added with this. I believe all other drivers set it, but
didn't do a thorough grep.

On Jan 18, 2008 7:52 AM, Bruno Randolf <bruno@thinktube.com> wrote:

> +
> +       /* check if we need to merge IBSS */
> +       if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
> +           !local->sta_sw_scanning && !local->sta_hw_scanning &&
> +           mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> +           memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0) {
> +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> +               static unsigned long last_tsf_debug;
> +#endif
> +               if (rx_status->flag & RX_FLAG_TSFT)
> +                       mactime = rx_status->mactime;
> +               else {
> +                       mactime = -1LLU;
> +                       printk(KERN_WARNING "%s: IBSS mode needs mactime for "
> +                               "beacons\n", dev->name);

Does this merit a WARN_ON() ? This essentially will prevent IBSS
merges for drivers that haven't set mactime yet.

> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 6cc1e7e..5617e17 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1609,7 +1609,10 @@ static int prepare_for_handlers(struct ieee80211_sub_if_data *sdata,
>         case IEEE80211_IF_TYPE_IBSS:
>                 if (!bssid)
>                         return 0;
> -               if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
> +               if ((rx->fc & IEEE80211_FCTL_FTYPE) == IEEE80211_FTYPE_MGMT &&
> +                   (rx->fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BEACON)

Since we are letting all beacons through how about adding a check for
ssid on ieee80211_rx_bss_info()?

(memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0)

Just not sure exactly we'd check for it yet.

> +                       return 1;
> +               else if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
>                         if (!(rx->flags & IEEE80211_TXRXD_RXIN_SCAN))
>                                 return 0;
>                         rx->flags &= ~IEEE80211_TXRXD_RXRA_MATCH;
>
>

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[Ivo van Doorn]

Hi,

> Adding Johill and Michael as they are maintainers of mac80211 and they
> need to review this. Please add them on further mac80211 patches.
> 
> Note to driver authors: zd1211rw, rt2x00 drivers and adm8211 will need
> to set mactime if they want IBSS merge to work as this functionality
> would be added with this. I believe all other drivers set it, but
> didn't do a thorough grep.

Then there is a problem for rt2x00. Since the mactime isn't known.
rt2400pci is the _only_ device which has a RX_END_TIME field in the
RX descriptor.

Ivo

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Sunday 20 January 2008 19:43:09 Ivo van Doorn wrote:
> Hi,
>
> > Adding Johill and Michael as they are maintainers of mac80211 and they
> > need to review this. Please add them on further mac80211 patches.
> >
> > Note to driver authors: zd1211rw, rt2x00 drivers and adm8211 will need
> > to set mactime if they want IBSS merge to work as this functionality
> > would be added with this. I believe all other drivers set it, but
> > didn't do a thorough grep.
>
> Then there is a problem for rt2x00. Since the mactime isn't known.
> rt2400pci is the _only_ device which has a RX_END_TIME field in the
> RX descriptor.

one workaround could be to simply use the current TSF at the time in the 
tasklet or interrupt handler (to be more close to the actual rx time). this 
should be sufficient to catch most cases where an IBSS merge is necessary - 
usually the beacon's TSF will be much higher than the local TSF.

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Ivo van Doorn]

Hi,

> > Then there is a problem for rt2x00. Since the mactime isn't known.
> > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > RX descriptor.
> 
> one workaround could be to simply use the current TSF at the time in the 
> tasklet or interrupt handler (to be more close to the actual rx time). this 
> should be sufficient to catch most cases where an IBSS merge is necessary - 
> usually the beacon's TSF will be much higher than the local TSF.

Should the driver to this, or should mac80211 handle that?
Personally I think it is something for the mac80211 layer since the driver will
give what it can, and can be sure that it is what mac80211 expects instead of
drivers interpreting what mac80211 might want as replacement.
If mac80211 needs the TSF value when no mac time is given, it could just
use the get_tsf() callback function to the driver to get the substitute. When the
get_tsf() callback is not provided, then mac80211 can complain about missing
information.

Ivo

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Jan 21, 2008 11:05 AM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
> Hi,
>
> > > Then there is a problem for rt2x00. Since the mactime isn't known.
> > > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > > RX descriptor.
> >
> > one workaround could be to simply use the current TSF at the time in the
> > tasklet or interrupt handler (to be more close to the actual rx time). this
> > should be sufficient to catch most cases where an IBSS merge is necessary -
> > usually the beacon's TSF will be much higher than the local TSF.
>
> Should the driver to this, or should mac80211 handle that?

The driver should if it has access to some the mactime of the received
packet otherwise yes -- I think mac80211 can handle this using the
supplied get_tsf().

> Personally I think it is something for the mac80211 layer since the driver will
> give what it can, and can be sure that it is what mac80211 expects instead of
> drivers interpreting what mac80211 might want as replacement.
> If mac80211 needs the TSF value when no mac time is given, it could just
> use the get_tsf() callback function to the driver to get the substitute. When the
> get_tsf() callback is not provided, then mac80211 can complain about missing
> information.

Agreed, how about something like this modified to Bruno's patch:

+               if (rx_status->flag & RX_FLAG_TSFT)
+                       mactime = rx_status->mactime;
+               else {
+                       if (!local->ops->get_tsf) {
+                          WARN_ON(1);
+                          mactime = -1LLU;
+                          if (net_ratelimit())
+                             printk(KERN_WARNING "%s: cannot determine if "
+                                   "IBSS merge is required", dev->name);
+                      }
+                      else {
+                           if (net_ratelimit())
+                             printk("Could not get detailed timestamp
of beacon "
+                                          during reception, using the
driver's TSF timer for mactime\n")
+                           mactime = local->ops->get_tsf(local_to_hw(local));
+                   }
+               }

We could just not support IBSS for driver's without a get_tsf(). Thoughts?

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[Ivo van Doorn]

Hi,

> > > > Then there is a problem for rt2x00. Since the mactime isn't known.
> > > > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > > > RX descriptor.
> > >
> > > one workaround could be to simply use the current TSF at the time in the
> > > tasklet or interrupt handler (to be more close to the actual rx time). this
> > > should be sufficient to catch most cases where an IBSS merge is necessary -
> > > usually the beacon's TSF will be much higher than the local TSF.
> >
> > Should the driver to this, or should mac80211 handle that?
> 
> The driver should if it has access to some the mactime of the received
> packet otherwise yes -- I think mac80211 can handle this using the
> supplied get_tsf().
> 
> > Personally I think it is something for the mac80211 layer since the driver will
> > give what it can, and can be sure that it is what mac80211 expects instead of
> > drivers interpreting what mac80211 might want as replacement.
> > If mac80211 needs the TSF value when no mac time is given, it could just
> > use the get_tsf() callback function to the driver to get the substitute. When the
> > get_tsf() callback is not provided, then mac80211 can complain about missing
> > information.
> 
> Agreed, how about something like this modified to Bruno's patch:
> 
> +               if (rx_status->flag & RX_FLAG_TSFT)
> +                       mactime = rx_status->mactime;
> +               else {
> +                       if (!local->ops->get_tsf) {
> +                          WARN_ON(1);
> +                          mactime = -1LLU;
> +                          if (net_ratelimit())
> +                             printk(KERN_WARNING "%s: cannot determine if "
> +                                   "IBSS merge is required", dev->name);
> +                      }
> +                      else {
> +                           if (net_ratelimit())
> +                             printk("Could not get detailed timestamp
> of beacon "
> +                                          during reception, using the
> driver's TSF timer for mactime\n")
> +                           mactime = local->ops->get_tsf(local_to_hw(local));
> +                   }
> +               }
> 
> We could just not support IBSS for driver's without a get_tsf(). Thoughts?

I am not sure if we should print error warning messages when the mactime is
not provided. But other then that I agree with the above change, as it would
allow rt2x00 to use the IBSS syncing. :)

Ivo

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Jan 22, 2008 2:54 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote:
>
> Hi,
>
> > > > > Then there is a problem for rt2x00. Since the mactime isn't known.
> > > > > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > > > > RX descriptor.
> > > >
> > > > one workaround could be to simply use the current TSF at the time in the
> > > > tasklet or interrupt handler (to be more close to the actual rx time). this
> > > > should be sufficient to catch most cases where an IBSS merge is necessary -
> > > > usually the beacon's TSF will be much higher than the local TSF.
> > >
> > > Should the driver to this, or should mac80211 handle that?
> >
> > The driver should if it has access to some the mactime of the received
> > packet otherwise yes -- I think mac80211 can handle this using the
> > supplied get_tsf().
> >
> > > Personally I think it is something for the mac80211 layer since the driver will
> > > give what it can, and can be sure that it is what mac80211 expects instead of
> > > drivers interpreting what mac80211 might want as replacement.
> > > If mac80211 needs the TSF value when no mac time is given, it could just
> > > use the get_tsf() callback function to the driver to get the substitute. When the
> > > get_tsf() callback is not provided, then mac80211 can complain about missing
> > > information.
> >
> > Agreed, how about something like this modified to Bruno's patch:
> >
> > +               if (rx_status->flag & RX_FLAG_TSFT)
> > +                       mactime = rx_status->mactime;
> > +               else {
> > +                       if (!local->ops->get_tsf) {
> > +                          WARN_ON(1);
> > +                          mactime = -1LLU;
> > +                          if (net_ratelimit())
> > +                             printk(KERN_WARNING "%s: cannot determine if "
> > +                                   "IBSS merge is required", dev->name);
> > +                      }
> > +                      else {
> > +                           if (net_ratelimit())
> > +                             printk("Could not get detailed timestamp
> > of beacon "
> > +                                          during reception, using the
> > driver's TSF timer for mactime\n")
> > +                           mactime = local->ops->get_tsf(local_to_hw(local));
> > +                   }
> > +               }
> >
> > We could just not support IBSS for driver's without a get_tsf(). Thoughts?
>
> I am not sure if we should print error warning messages when the mactime is
> not provided. But other then that I agree with the above change, as it would
> allow rt2x00 to use the IBSS syncing. :)

Ah yes, ignore my printks and instead replace them with better comments :)

I still think we should inform the user if the user switches to IBSS
at some point, perhaps better during interface addition if their
driver's IBSS mode is going to have some issues. How about we add to
the enum ieee80211_hw_flags a "IEEE80211_HW_RX_MACTIME". Then we can
warn accordingly during ieee80211_if_add() if the interface type is
IBSS"

* If driver supports IEEE80211_HW_RX_MACTIME we don't warn anything
* If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is
implemented inform user IBSS merge may not behave accurately
* If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is not
implemented warn IBSS merge will not work

We could add:

static inline u64 __approx_mactime(struct ieee80211_local *local) {
   BUG_ON(!local || !local->ops);
   return (local->ops->get_tsf) ?
local->ops->get_tsf(local_to_hw(local)) : -1LLU;
}

Then in ieee80211_rx_bss_info() we can do something like:

+               if (local->hw.flags & IEEE80211_HW_RX_MACTIME) {
+                   if (rx_status->flag & RX_FLAG_TSFT)
+                       mactime = rx_status->mactime;
+                   else {
+                     WARN_ON(1);
+                     mactime = __approx_mactime(local);
+                  }
+              else {
+                     mactime = __approx_mactime(local);
+         }

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[Ivo van Doorn]

Hi,

<snip>

> > I am not sure if we should print error warning messages when the mactime is
> > not provided. But other then that I agree with the above change, as it would
> > allow rt2x00 to use the IBSS syncing. :)
> 
> Ah yes, ignore my printks and instead replace them with better comments :)
> 
> I still think we should inform the user if the user switches to IBSS
> at some point, perhaps better during interface addition if their
> driver's IBSS mode is going to have some issues. How about we add to
> the enum ieee80211_hw_flags a "IEEE80211_HW_RX_MACTIME". Then we can
> warn accordingly during ieee80211_if_add() if the interface type is
> IBSS"
> 
> * If driver supports IEEE80211_HW_RX_MACTIME we don't warn anything
> * If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is
> implemented inform user IBSS merge may not behave accurately
> * If IEEE80211_HW_RX_MACTIME is not supported and get_tsf() is not
> implemented warn IBSS merge will not work
> 
> We could add:
> 
> static inline u64 __approx_mactime(struct ieee80211_local *local) {
>    BUG_ON(!local || !local->ops);
>    return (local->ops->get_tsf) ?
> local->ops->get_tsf(local_to_hw(local)) : -1LLU;
> }
> 
> Then in ieee80211_rx_bss_info() we can do something like:
> 
> +               if (local->hw.flags & IEEE80211_HW_RX_MACTIME) {
> +                   if (rx_status->flag & RX_FLAG_TSFT)
> +                       mactime = rx_status->mactime;
> +                   else {
> +                     WARN_ON(1);
> +                     mactime = __approx_mactime(local);
> +                  }
> +              else {
> +                     mactime = __approx_mactime(local);
> +         }

Sounds good to me. :)

Ivo

Re: [PATCH] mac80211: enable IBSS merging

[Adam Baker]

Ivo van Doorn <ivdoorn@...> writes:

> 
> Hi,
> 
> > > > > Then there is a problem for rt2x00. Since the mactime isn't known.
> > > > > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > > > > RX descriptor.
> > > >
> > > > one workaround could be to simply use the current TSF at the time in
> > > > the tasklet or interrupt handler (to be more close to the actual rx 
> > > > time). this should be sufficient to catch most cases where an IBSS 
> > > > merge is necessary - usually the beacon's TSF will be much higher
> > > > than the local TSF.
> > >
> > > Should the driver to this, or should mac80211 handle that?
> > 
> > The driver should if it has access to some the mactime of the received
> > packet otherwise yes -- I think mac80211 can handle this using the
> > supplied get_tsf().
> > 
> > > Personally I think it is something for the mac80211 layer since the
> > > driver will give what it can, and can be sure that it is what mac80211 
> > > expects instead of drivers interpreting what mac80211 might want as 
> > > replacement. If mac80211 needs the TSF value when no mac time is 
> > > given, it could just use the get_tsf() callback function to the driver
> > >  to get the substitute. When the get_tsf() callback is not provided,
> > >  then mac80211 can complain about missing information.
> > 
>

I see one problem with this. At the moment rt2x00 doesn't implement get_tsf
for USB devices. In the case of RT73 this is because mac80211 requires
get_tsf to be atomic which precludes waiting for a USB device to reply
to a request. Ivo has got an implementation in the code but it is disabled
as the fact it sleeps was causing bugs. Presumably other USB devices would
have the same problem.

I'm not sure if the code path for IBSS merging would require an atomic
version, I'm guessing it wouldn't. If so do we need to consider whether 
to either have both get_tsf and get_tsf_atomic or have a flag to get_tsf
indicating if it can sleep if it needs to?

Adam Baker

Re: [PATCH] mac80211: enable IBSS merging

[Ivo van Doorn]

Hi,

> > > > > > Then there is a problem for rt2x00. Since the mactime isn't known.
> > > > > > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > > > > > RX descriptor.
> > > > >
> > > > > one workaround could be to simply use the current TSF at the time in
> > > > > the tasklet or interrupt handler (to be more close to the actual rx 
> > > > > time). this should be sufficient to catch most cases where an IBSS 
> > > > > merge is necessary - usually the beacon's TSF will be much higher
> > > > > than the local TSF.
> > > >
> > > > Should the driver to this, or should mac80211 handle that?
> > > 
> > > The driver should if it has access to some the mactime of the received
> > > packet otherwise yes -- I think mac80211 can handle this using the
> > > supplied get_tsf().
> > > 
> > > > Personally I think it is something for the mac80211 layer since the
> > > > driver will give what it can, and can be sure that it is what mac80211 
> > > > expects instead of drivers interpreting what mac80211 might want as 
> > > > replacement. If mac80211 needs the TSF value when no mac time is 
> > > > given, it could just use the get_tsf() callback function to the driver
> > > >  to get the substitute. When the get_tsf() callback is not provided,
> > > >  then mac80211 can complain about missing information.
> > > 
> >
> 
> I see one problem with this. At the moment rt2x00 doesn't implement get_tsf
> for USB devices. In the case of RT73 this is because mac80211 requires
> get_tsf to be atomic which precludes waiting for a USB device to reply
> to a request. Ivo has got an implementation in the code but it is disabled
> as the fact it sleeps was causing bugs. Presumably other USB devices would
> have the same problem.

Oops, I really should have checked it, I thought the other tsf handler was disabled
and only get_tsf was implemented. Apparently rt73usb only has reset_tsf(). :S

For rt2500usb the problem is that it contains no TSF registers at all
(at least I never found an offset which contained something looking like a TSF).

> I'm not sure if the code path for IBSS merging would require an atomic
> version, I'm guessing it wouldn't. If so do we need to consider whether 
> to either have both get_tsf and get_tsf_atomic or have a flag to get_tsf
> indicating if it can sleep if it needs to?

The new patch has get_tsf() in normal context, I think the best thing would
be moving all remaining atomic get_tsf() calls out of that context. But last time
I checked that wouldn't be too easy. :(

Ivo

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> Then there is a problem for rt2x00. Since the mactime isn't known.
> rt2400pci is the _only_ device which has a RX_END_TIME field in the
> RX descriptor.

I don't think it's required for IBSS to function, only for this from my
POV dubious "merge" functionality.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Wednesday 23 January 2008 23:49:51 Johannes Berg wrote:
> > Then there is a problem for rt2x00. Since the mactime isn't known.
> > rt2400pci is the _only_ device which has a RX_END_TIME field in the
> > RX descriptor.
>
> I don't think it's required for IBSS to function, only for this from my
> POV dubious "merge" functionality.

also you might want to check if your hard- or firmware (if you have any) 
handles IBSS merges already, especially with USB devices. i can't imagine how 
a device driver without access to that information is supposed to merge IBSS.

bruno

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Sunday 20 January 2008 19:17:04 Luis R. Rodriguez wrote:
> Adding Johill and Michael as they are maintainers of mac80211 and they
> need to review this. Please add them on further mac80211 patches.

oh, sorry. sure!

> On Jan 18, 2008 7:52 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> > +
> > +       /* check if we need to merge IBSS */
> > +       if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
> > +           !local->sta_sw_scanning && !local->sta_hw_scanning &&
> > +           mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> > +           memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len)
> > == 0) { +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> > +               static unsigned long last_tsf_debug;
> > +#endif
> > +               if (rx_status->flag & RX_FLAG_TSFT)
> > +                       mactime = rx_status->mactime;
> > +               else {
> > +                       mactime = -1LLU;
> > +                       printk(KERN_WARNING "%s: IBSS mode needs mactime
> > for " +                               "beacons\n", dev->name);
>
> Does this merit a WARN_ON() ? This essentially will prevent IBSS
> merges for drivers that haven't set mactime yet.
>
> > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> > index 6cc1e7e..5617e17 100644
> > --- a/net/mac80211/rx.c
> > +++ b/net/mac80211/rx.c
> > @@ -1609,7 +1609,10 @@ static int prepare_for_handlers(struct
> > ieee80211_sub_if_data *sdata, case IEEE80211_IF_TYPE_IBSS:
> >                 if (!bssid)
> >                         return 0;
> > -               if (!ieee80211_bssid_match(bssid, sdata->u.sta.bssid)) {
> > +               if ((rx->fc & IEEE80211_FCTL_FTYPE) ==
> > IEEE80211_FTYPE_MGMT && +                   (rx->fc &
> > IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_BEACON)
>
> Since we are letting all beacons through how about adding a check for
> ssid on ieee80211_rx_bss_info()?
>
> (memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0)
>
> Just not sure exactly we'd check for it yet.

sure, that's in the previous part of the patch:

> > +           mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> > +           memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len)
> > == 0) 

greetings,
bruno

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> enable IBSS cell merging. if an IBSS beacon with the same ESSID and a TSF
> higher than the local TSF (mactime) is received, we have to join its BSSID.

Can you back that up with standard references? I see no such requirement
in the standard text. I can see that under some circumstances this may
be desirable, but maybe not.

In fact, this seems to *break* standard behaviour, as per 11.1.3.1:
(emphasis mine)

        When a STA starts a BSS, that STA shall determine the BSSID of
        the BSS. If the BSSType indicates an infrastructure BSS, then
        the STA shall start an infrastructure BSS and the BSSID shall be
        equal to the STA’s dot11StationID. ***The value of the BSSID
        shall remain unchanged***, even if the value of dot11StationID
        is changed after the completion of the MLME-START.request. If
        the BSSType indicates an IBSS, the STA shall start an IBSS, and
        the BSSID shall be an individual locally administered IEEE MAC
        address as defined in 5.2 of IEEE Std 802-1990. The remaining 46
        bits of that MAC address shall be a number selected in a manner
        that minimizes the probability of STAs generating the same
        number, even when those STAs are subjected to the same initial
        conditions.

Remember that BSSIDs, *not* SSIDs are used to identify BSSes uniquely.

Of course you could argue that "merging" is simply tearing down the own
and then joining the other IBSS.

Comments on the code now.

> --- a/net/mac80211/ieee80211_sta.c
> +++ b/net/mac80211/ieee80211_sta.c
> @@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_device *dev,
>                                  struct ieee80211_sta_bss *bss);
>  static int ieee80211_sta_find_ibss(struct net_device *dev,
>                                    struct ieee80211_if_sta *ifsta);
> +static int ieee80211_sta_join_ibss(struct net_device *dev,
> +                                  struct ieee80211_if_sta *ifsta,
> +                                  struct ieee80211_sta_bss *bss);

No way, order the code properly, this mess needs to be cleaned up not
added to.
 
> -	if (bss->probe_resp && beacon) {
> +	if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
> +	    bss->probe_resp && beacon) {
>  		/* Do not allow beacon to override data from Probe Response. */

Ahem. Comment update required.

> +	/* check if we need to merge IBSS */

Could use a plural, but whatever.

> +	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
> +	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
> +	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> +	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0) {

I think that needs a check "&& ssid_len" or something. Someone's bound
to try making an IBSS with a hidden SSID and we really don't want that
to work.

> +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> +		static unsigned long last_tsf_debug;
> +#endif

Let's just get rid of that, it's not usable with multiple devices.

> +		if (rx_status->flag & RX_FLAG_TSFT)
> +			mactime = rx_status->mactime;
> +		else {
> +			mactime = -1LLU;
> +			printk(KERN_WARNING "%s: IBSS mode needs mactime for "
> +				"beacons\n", dev->name);

Very bad, you'll be flooded by this when others send beacons. Also, I
doubt its truthfulness.

> +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> +				" -> IBSS merge with BSSID %s\n",
> +				dev->name, print_mac(mac, mgmt->bssid));

No way. At the very least you need to ratelimit this.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[Dan Williams]

On Wed, 2008-01-23 at 15:48 +0100, Johannes Berg wrote:
> > enable IBSS cell merging. if an IBSS beacon with the same ESSID and=
 a TSF
> > higher than the local TSF (mactime) is received, we have to join it=
s BSSID.
>=20
> Can you back that up with standard references? I see no such requirem=
ent
> in the standard text. I can see that under some circumstances this ma=
y
> be desirable, but maybe not.
>=20
> In fact, this seems to *break* standard behaviour, as per 11.1.3.1:
> (emphasis mine)

The way I understood Ad-Hoc to work was that if you weren't using the
SSID + BSSID as somebody else, you weren't part of that adhoc network.
Ad-Hoc + roaming isn't supported and is even less defined, which is wha=
t
this patch would enable IIUC.  When joining an adhoc network, your
card/driver must scan around to find the SSID + (optionally) BSSID of
that adhoc network.  If it finds one that matches criteria, it joins
that adhoc network, which means using the same BSSID.  If it doesn't, i=
t
creates a new adhoc network with a generated BSSID and the specified
SSID.

If this patch were applied, then you'd pretty quickly get split adhoc
networks where some people were automatically moved to a different BSSI=
D
and the rest are left on the old BSSID (maybe missed a beacon, or
whatever), and therefore can't talk to each other.  If only because
mac80211 would jump adhoc BSSIDs, while all other drivers (including
fullmac parts) would not.

A cell is identified uniquely by the combination of BSSID + SSID,
nothing less.  You don't combine infrastructure networks of the same
SSID and a different BSSID, nor does this happen for adhoc.

Dan

>         When a STA starts a BSS, that STA shall determine the BSSID o=
f
>         the BSS. If the BSSType indicates an infrastructure BSS, then
>         the STA shall start an infrastructure BSS and the BSSID shall=
 be
>         equal to the STA=E2=80=99s dot11StationID. ***The value of th=
e BSSID
>         shall remain unchanged***, even if the value of dot11StationI=
D
>         is changed after the completion of the MLME-START.request. If
>         the BSSType indicates an IBSS, the STA shall start an IBSS, a=
nd
>         the BSSID shall be an individual locally administered IEEE MA=
C
>         address as defined in 5.2 of IEEE Std 802-1990. The remaining=
 46
>         bits of that MAC address shall be a number selected in a mann=
er
>         that minimizes the probability of STAs generating the same
>         number, even when those STAs are subjected to the same initia=
l
>         conditions.
>=20
> Remember that BSSIDs, *not* SSIDs are used to identify BSSes uniquely=
=2E
>=20
> Of course you could argue that "merging" is simply tearing down the o=
wn
> and then joining the other IBSS.
>=20
> Comments on the code now.
>=20
> > --- a/net/mac80211/ieee80211_sta.c
> > +++ b/net/mac80211/ieee80211_sta.c
> > @@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_devic=
e *dev,
> >                                  struct ieee80211_sta_bss *bss);
> >  static int ieee80211_sta_find_ibss(struct net_device *dev,
> >                                    struct ieee80211_if_sta *ifsta);
> > +static int ieee80211_sta_join_ibss(struct net_device *dev,
> > +                                  struct ieee80211_if_sta *ifsta,
> > +                                  struct ieee80211_sta_bss *bss);
>=20
> No way, order the code properly, this mess needs to be cleaned up not
> added to.
> =20
> > -	if (bss->probe_resp && beacon) {
> > +	if (sdata->vif.type !=3D IEEE80211_IF_TYPE_IBSS &&
> > +	    bss->probe_resp && beacon) {
> >  		/* Do not allow beacon to override data from Probe Response. */
>=20
> Ahem. Comment update required.
>=20
> > +	/* check if we need to merge IBSS */
>=20
> Could use a plural, but whatever.
>=20
> > +	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
> > +	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
> > +	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> > +	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) =
=3D=3D 0) {
>=20
> I think that needs a check "&& ssid_len" or something. Someone's boun=
d
> to try making an IBSS with a hidden SSID and we really don't want tha=
t
> to work.
>=20
> > +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> > +		static unsigned long last_tsf_debug;
> > +#endif
>=20
> Let's just get rid of that, it's not usable with multiple devices.
>=20
> > +		if (rx_status->flag & RX_FLAG_TSFT)
> > +			mactime =3D rx_status->mactime;
> > +		else {
> > +			mactime =3D -1LLU;
> > +			printk(KERN_WARNING "%s: IBSS mode needs mactime for "
> > +				"beacons\n", dev->name);
>=20
> Very bad, you'll be flooded by this when others send beacons. Also, I
> doubt its truthfulness.
>=20
> > +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> > +				" -> IBSS merge with BSSID %s\n",
> > +				dev->name, print_mac(mac, mgmt->bssid));
>=20
> No way. At the very least you need to ratelimit this.
>=20
> johannes

-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Thursday 24 January 2008 02:22:26 Dan Williams wrote:
> On Wed, 2008-01-23 at 15:48 +0100, Johannes Berg wrote:
> > > enable IBSS cell merging. if an IBSS beacon with the same ESSID and a
> > > TSF higher than the local TSF (mactime) is received, we have to join
> > > its BSSID.
> >
> > Can you back that up with standard references? I see no such requirement
> > in the standard text. I can see that under some circumstances this may
> > be desirable, but maybe not.
> >
> > In fact, this seems to *break* standard behaviour, as per 11.1.3.1:
> > (emphasis mine)
>
> The way I understood Ad-Hoc to work was that if you weren't using the
> SSID + BSSID as somebody else, you weren't part of that adhoc network.
> Ad-Hoc + roaming isn't supported and is even less defined, which is what
> this patch would enable IIUC. 

i agree that you could call what my patch enables is roaming. but i think in 
IBSS mode this "roaming" has to happen in order to get IBSS mode working as 
the user would expect. if you configure 2 laptops in distance, say two 
different rooms and then move one of them into the other room you would 
expect to be able to communicate with the other laptop wouldn't you?

> When joining an adhoc network, your 
> card/driver must scan around to find the SSID + (optionally) BSSID of
> that adhoc network.  If it finds one that matches criteria, it joins
> that adhoc network, which means using the same BSSID.  If it doesn't, it
> creates a new adhoc network with a generated BSSID and the specified
> SSID.
>
> If this patch were applied, then you'd pretty quickly get split adhoc
> networks where some people were automatically moved to a different BSSID
> and the rest are left on the old BSSID (maybe missed a beacon, or
> whatever), and therefore can't talk to each other. 

actually i would argue that *without* this patch you will get split adhoc 
networks all the time (which is the reason i created the patch in the first 
place). please see my previous mail for some  additional scenarios where this 
would happen. also please understand that merging in a city wide IBSS might 
seem like a crazy, irrelevant idea to you, but that is how IBSS mode is used 
in practice (too) and secondly it's just an example of similar odd cases that 
can also happen when you start an IBSS in a conference room with just a few 
participants.

> If only because 
> mac80211 would jump adhoc BSSIDs, while all other drivers (including
> fullmac parts) would not.

that's not true. most other drivers, including fullmac drivers will join 
BSSIDs when the ESSID matches and the TSF is higher.

> A cell is identified uniquely by the combination of BSSID + SSID,
> nothing less. You don't combine infrastructure networks of the same 
> SSID and a different BSSID, nor does this happen for adhoc.

sure you do that in infrastructure mode: any bigger infrastructure network 
will consist of several APs connected by ethernet and using different BSSIDs, 
with the same ESSID, allowing users to roam between them.

as far as adhoc is concerned: yes, you shouldn't do that, and this is exactly 
what my patch is trying to achieve - only one BSSID for a given ESSID and 
channel (the one with the oldest TSF value).

bruno

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Wednesday 23 January 2008 23:48:49 Johannes Berg wrote:
> > enable IBSS cell merging. if an IBSS beacon with the same ESSID and=
 a TSF
> > higher than the local TSF (mactime) is received, we have to join it=
s
> > BSSID.
>
> Can you back that up with standard references? I see no such requirem=
ent
> in the standard text. I can see that under some circumstances this ma=
y
> be desirable, but maybe not.

sure. the relevant sections of the standard are 11.1.2.3 "Beacon recept=
ion":

"STAs in an IBSS shall use other information in any received Beacon fra=
me for=20
which the IBSS subfield of the Capability field is set to 1 and the con=
tent=20
of the SSID element is equal to the SSID of the IBSS. Use of this infor=
mation=20
is specified in 11.1.4."

and in 11.1.4 "Adjusting STA timers":
(emphasis and brackets mine):

"In an IBSS, a STA shall always adopt the information in the contents o=
f a=20
Beacon or Probe Response frame when that frame contains a matching SSID=
 and=20
the value of the time stamp is later than the STA=E2=80=99s TSF timer. =
[In response=20
to an MLME-Join.request, a STA shall initialize its TSF timer to 0 and =
shall=20
not transmit a beacon or probe response until it hears a beacon or prob=
e=20
response from a member of the IBSS with a matching SSID.]"

"All Beacon and Probe Response frames carry a Timestamp field. A STA re=
ceiving=20
such a frame from another STA in an IBSS with the same SSID shall compa=
re the=20
Timestamp field with its own TSF time. If the Timestamp field of the re=
ceived=20
frame is later than its own TSF time, the STA shall adopt *all* paramet=
ers=20
contained in the Beacon frame."

i can see now how you could interpret this as only applying to timers, =
but i=20
think that does just not make sense - why would you want to sync timers=
 and=20
adapt beacon intervals if you don't share the same BSSID?

i agree that the standard is quite ambigious when it comes to IBSS, but=
=20
practically speaking, it is necessary to merge IBSS cells most of the t=
ime if=20
you want to have IBSS working as expected. therefore i tend to interpre=
t the=20
ambigious parts of the standard in a way that will improve functionalit=
y.

just think about two IBSS nodes which are configured with the same ESSI=
D and=20
which start up at nearly the same time. they would both scan, not find =
an=20
existing BSS and start their own. current mac80211 implementation would=
 after=20
a while detect that they are alone on their BSS and start another scan,=
 which=20
would finally cause one of them to reconfigure its BSSID, but this is n=
ot=20
sufficient: if you happen to have 2 nodes in each BSSID a merge would n=
ever=20
happen.
there are similar situations like this in real life, the most obvious e=
xample=20
beeing a city wide IBSS where you can easily have two separate groups o=
f=20
nodes starting up with a different BSSID. suddenly an obstacle is remov=
ed or=20
a new node comes up in the middle of these two groups (with connections=
 to=20
both) - which BSSID is it supposed to join? the one with the higher TSF=
, but=20
that would leave us with 2 distinct BSSIDs. would all involved nodes re=
start,=20
they would all share the same BSSID now. i think the only sensible answ=
er to=20
this problem is that they all should join the BSSID with the higher TSF=
=2E=20

this is also how all drivers and (also firmware based) cards i know han=
dle=20
IBSS - they will merge in a similar fashion based on the ESSID. (i know=
=2E..=20
that might not be a good argument, since most drivers are broken in one=
 way=20
or the other, especially when it comes to IBSS mode.)

> In fact, this seems to *break* standard behaviour, as per 11.1.3.1:
> (emphasis mine)
>
>         When a STA starts a BSS, that STA shall determine the BSSID o=
f
>         the BSS. If the BSSType indicates an infrastructure BSS, then
>         the STA shall start an infrastructure BSS and the BSSID shall=
 be
>         equal to the STA=E2=80=99s dot11StationID. ***The value of th=
e BSSID
>         shall remain unchanged***, even if the value of dot11StationI=
D
>         is changed after the completion of the MLME-START.request.

i understand this to apply to infrastructure STAs only.

>         If=20
>         the BSSType indicates an IBSS, the STA shall start an IBSS, a=
nd
>         the BSSID shall be an individual locally administered IEEE MA=
C
>         address as defined in 5.2 of IEEE Std 802-1990. The remaining=
 46
>         bits of that MAC address shall be a number selected in a mann=
er
>         that minimizes the probability of STAs generating the same
>         number, even when those STAs are subjected to the same initia=
l
>         conditions.
>
> Comments on the code now.
>
> > --- a/net/mac80211/ieee80211_sta.c
> > +++ b/net/mac80211/ieee80211_sta.c
> > @@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_devic=
e
> > *dev, struct ieee80211_sta_bss *bss); static int
> > ieee80211_sta_find_ibss(struct net_device *dev,
> >                                    struct ieee80211_if_sta *ifsta);
> > +static int ieee80211_sta_join_ibss(struct net_device *dev,
> > +                                  struct ieee80211_if_sta *ifsta,
> > +                                  struct ieee80211_sta_bss *bss);
>
> No way, order the code properly, this mess needs to be cleaned up not
> added to.

ok. i just didn't want to make my patch too unclear by moving stuff aro=
und in=20
addition to adding functionality. i'll make 2 separate patches, once we=
=20
agreed that this whole merge behaviour is wanted in mac80211 at all.

> > -	if (bss->probe_resp && beacon) {
> > +	if (sdata->vif.type !=3D IEEE80211_IF_TYPE_IBSS &&
> > +	    bss->probe_resp && beacon) {
> >  		/* Do not allow beacon to override data from Probe Response. */
>
> Ahem. Comment update required.

ok.

> > +	if (sdata->vif.type =3D=3D IEEE80211_IF_TYPE_IBSS && beacon &&
> > +	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
> > +	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> > +	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) =
=3D=3D 0)
> > {
>
> I think that needs a check "&& ssid_len" or something. Someone's boun=
d
> to try making an IBSS with a hidden SSID and we really don't want tha=
t
> to work.

yep.

> > +#ifdef CONFIG_MAC80211_IBSS_DEBUG
> > +		static unsigned long last_tsf_debug;
> > +#endif
>
> Let's just get rid of that, it's not usable with multiple devices.

i'm happy to do that, i just wanted to preserve as much as possible of =
the=20
original code since i wasn't sure if anybody needed it.

> > +		if (rx_status->flag & RX_FLAG_TSFT)
> > +			mactime =3D rx_status->mactime;
> > +		else {
> > +			mactime =3D -1LLU;
> > +			printk(KERN_WARNING "%s: IBSS mode needs mactime for "
> > +				"beacons\n", dev->name);
>
> Very bad, you'll be flooded by this when others send beacons. Also, I
> doubt its truthfulness.

please see my last post where i suggest to use:

+               if (rx_status->flag & RX_FLAG_TSFT)
+                       /* in order for correct IBSS merging we need ma=
ctime*/
+                       mactime =3D rx_status->mactime;
+               else if (local && local->ops && local->ops->get_tsf)
+                       /* second best option: get current TSF */
+                       mactime =3D local->ops->get_tsf(local_to_hw(loc=
al));
+               else
+                       /* can't merge without knowing the TSF */
+                       mactime =3D -1LLU;

instead.

> > +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> > +				" -> IBSS merge with BSSID %s\n",
> > +				dev->name, print_mac(mac, mgmt->bssid));
>
> No way. At the very least you need to ratelimit this.

ok, i'll add a ratelimit.

bruno
-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> i can see now how you could interpret this as only applying to timers, but i 
> think that does just not make sense - why would you want to sync timers and 
> adapt beacon intervals if you don't share the same BSSID?

Hmm. Well I thought that was pretty much just for optimising performance
but now that I think about it again...

> i agree that the standard is quite ambigious when it comes to IBSS, but 
> practically speaking, it is necessary to merge IBSS cells most of the time if 
> you want to have IBSS working as expected. therefore i tend to interpret the 
> ambigious parts of the standard in a way that will improve functionality.

I think "working as expected" is stretching it a bit ;) How do you
expect IBSS to work? I'd expect only the simplest use case from it:
start IBSS on one station and have another one in the vicinity find it
while scanning and join it. That works regardless of merging or not.

> please see my last post where i suggest to use:
> 
> +               if (rx_status->flag & RX_FLAG_TSFT)
> +                       /* in order for correct IBSS merging we need mactime*/
> +                       mactime = rx_status->mactime;
> +               else if (local && local->ops && local->ops->get_tsf)
> +                       /* second best option: get current TSF */
> +                       mactime = local->ops->get_tsf(local_to_hw(local));
> +               else
> +                       /* can't merge without knowing the TSF */
> +                       mactime = -1LLU;
> 
> instead.

Not sure. That's a bit on a fine line, if you have a slow device
get_tsf() might be well off and this could trigger a BSSID change in
both stations.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Friday 25 January 2008 01:55:04 Johannes Berg wrote:
> > i can see now how you could interpret this as only applying to timers,
> > but i think that does just not make sense - why would you want to sync
> > timers and adapt beacon intervals if you don't share the same BSSID?
>
> Hmm. Well I thought that was pretty much just for optimising performance
> but now that I think about it again...
>
> > i agree that the standard is quite ambigious when it comes to IBSS, but
> > practically speaking, it is necessary to merge IBSS cells most of the
> > time if you want to have IBSS working as expected. therefore i tend to
> > interpret the ambigious parts of the standard in a way that will improve
> > functionality.
>
> I think "working as expected" is stretching it a bit ;) How do you
> expect IBSS to work? 

i think it's a reasonable expectation from a users point of view that two or 
more IBSS nodes using the same ESSID on the same channel can communicate with 
each other, regardless of when and where they were configured.

> I'd expect only the simplest use case from it: 
> start IBSS on one station and have another one in the vicinity find it
> while scanning and join it. That works regardless of merging or not.

this is probably a valid, but a most conservative interpretation of the 
standard, which makes IBSS practically unusable in many (or most) occasions.

i guess you have never actually used IBSS much. otherwise you would know from 
experience that your simplest use case practically rarely happens. usually 
you have situations where you miss the others BSS while scanning and end up 
with 2 different BSSIDs. or what happens if there is a third station within 
station 2's reach but out of range for station 1? sta2 will adopt the BSSID 
of sta3, but sta1 will not and start a new BSSID. so not even your simplest 
use case of two newly started stations will just work (without ibss merge).
examples and scenarios are endless, do you want me to continue? ;)

also i am not inventing this "non-standard merge". it is *not* non-standard, 
it's just not directly obvious from reading the standard.

IBSS merges are done like this by various cards and drivers: prism54 does the 
same (in firmware), hostap driver did it like this (probably does but i 
haven't used it in quite a while), the broadcom driver in openwrt acts like 
this, madwifi merges like this (ups, maybe i shouldn't have said that ;)) and 
so on...

aren't there any other serious IBSS users out there who can support this case 
and confirm that we need IBSS merge functionality?

> > please see my last post where i suggest to use:
> >
> > +               if (rx_status->flag & RX_FLAG_TSFT)
> > +                       /* in order for correct IBSS merging we need
> > mactime*/ +                       mactime = rx_status->mactime;
> > +               else if (local && local->ops && local->ops->get_tsf)
> > +                       /* second best option: get current TSF */
> > +                       mactime =
> > local->ops->get_tsf(local_to_hw(local)); +               else
> > +                       /* can't merge without knowing the TSF */
> > +                       mactime = -1LLU;
> >
> > instead.
>
> Not sure. That's a bit on a fine line, if you have a slow device
> get_tsf() might be well off and this could trigger a BSSID change in
> both stations.

i don't see that problem: if get_tsf() is called very late on a slow device, 
it would be higher than the beacons TSF, and nothing would happen.
also usually the beacons TSF is quite old, maybe from an IBSS running for 
several hours or even days or weeks and therefore we would catch most 
situations where IBSS merge is necessary even if get_tsf() is a bit off the 
beacon reception time. but in order to be able to merge more correctly it 
would be better if these devices could get the TSF as early as possible.

regards,
bruno

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Jan 25, 2008 3:01 AM, bruno randolf <bruno@thinktube.com> wrote:
> On Friday 25 January 2008 01:55:04 Johannes Berg wrote:
> > > i can see now how you could interpret this as only applying to timers,
> > > but i think that does just not make sense - why would you want to sync
> > > timers and adapt beacon intervals if you don't share the same BSSID?
> >
> > Hmm. Well I thought that was pretty much just for optimising performance
> > but now that I think about it again...
> >
> > > i agree that the standard is quite ambigious when it comes to IBSS, but
> > > practically speaking, it is necessary to merge IBSS cells most of the
> > > time if you want to have IBSS working as expected. therefore i tend to
> > > interpret the ambigious parts of the standard in a way that will improve
> > > functionality.
> >
> > I think "working as expected" is stretching it a bit ;) How do you
> > expect IBSS to work?
>
> i think it's a reasonable expectation from a users point of view that two or
> more IBSS nodes using the same ESSID on the same channel can communicate with
> each other, regardless of when and where they were configured.
>
> > I'd expect only the simplest use case from it:
> > start IBSS on one station and have another one in the vicinity find it
> > while scanning and join it. That works regardless of merging or not.
>
> this is probably a valid, but a most conservative interpretation of the
> standard, which makes IBSS practically unusable in many (or most) occasions.
>
> i guess you have never actually used IBSS much. otherwise you would know from
> experience that your simplest use case practically rarely happens. usually
> you have situations where you miss the others BSS while scanning and end up
> with 2 different BSSIDs. or what happens if there is a third station within
> station 2's reach but out of range for station 1? sta2 will adopt the BSSID
> of sta3, but sta1 will not and start a new BSSID. so not even your simplest
> use case of two newly started stations will just work (without ibss merge).
> examples and scenarios are endless, do you want me to continue? ;)
>
> also i am not inventing this "non-standard merge". it is *not* non-standard,
> it's just not directly obvious from reading the standard.
>
> IBSS merges are done like this by various cards and drivers: prism54 does the
> same (in firmware), hostap driver did it like this (probably does but i
> haven't used it in quite a while), the broadcom driver in openwrt acts like
> this, madwifi merges like this (ups, maybe i shouldn't have said that ;)) and
> so on...
>
> aren't there any other serious IBSS users out there who can support this case
> and confirm that we need IBSS merge functionality?

Sure ACK. Also as was stated, even with IBSS merge functionality
working you could up with nodes splitting up due to missing the
"leader" beacon due to interference or even strange radio node
problems. A solution to this we implemented at Orbit with MadWifi was
that instead of generating "random" BSSIDs you'd create one based on
the hash of the SSID [1]. This ensures that nodes with identical SSIDs
end up with identical BSSIDs, regardless of any strange problems.
Technically from what I have gathered this doesn't break the specs but
would prevent this split-IBSS problem. I actually like to see this
technique added into mac80211 too.

[1] http://www.orbit-lab.org/wiki/HowTo/bssidFix

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Sunday 03 February 2008 08:22:12 Luis R. Rodriguez wrote:
> Sure ACK. Also as was stated, even with IBSS merge functionality
> working you could up with nodes splitting up due to missing the
> "leader" beacon due to interference or even strange radio node
> problems. A solution to this we implemented at Orbit with MadWifi was
> that instead of generating "random" BSSIDs you'd create one based on
> the hash of the SSID [1]. This ensures that nodes with identical SSIDs
> end up with identical BSSIDs, regardless of any strange problems.
> Technically from what I have gathered this doesn't break the specs but
> would prevent this split-IBSS problem. I actually like to see this
> technique added into mac80211 too.
>
> [1] http://www.orbit-lab.org/wiki/HowTo/bssidFix

this looks like a good idea to avoid many ibss merge problems, and it could be 
really useful to have that (maybe as an option?) in mac80211. but unless we 
have ibss merge functionality you won't be able to join older networks 
(higher TSF) which don't use that scheme, so we still need that to be able to 
communicate with other ("standard") drivers.

i guess the general question for mac80211 is: do we want and allow more 
liberal interpretations of the standard concerning IBSS mode or do we 
strictly stick with the "book"?

in my opinion these "extensions" are necessary in order to make IBSS mode 
useful at all and therefore we need them! and as long as they are able to 
interoperate with other drivers in IBSS mode i don't see a problem.

what's the position of the mac80211 maintainers concerning this? is there any 
chance of getting these patches merged?

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Feb 4, 2008 8:50 PM, bruno randolf <bruno@thinktube.com> wrote:
> On Sunday 03 February 2008 08:22:12 Luis R. Rodriguez wrote:
> > Sure ACK. Also as was stated, even with IBSS merge functionality
> > working you could up with nodes splitting up due to missing the
> > "leader" beacon due to interference or even strange radio node
> > problems. A solution to this we implemented at Orbit with MadWifi was
> > that instead of generating "random" BSSIDs you'd create one based on
> > the hash of the SSID [1]. This ensures that nodes with identical SSIDs
> > end up with identical BSSIDs, regardless of any strange problems.
> > Technically from what I have gathered this doesn't break the specs but
> > would prevent this split-IBSS problem. I actually like to see this
> > technique added into mac80211 too.
> >
> > [1] http://www.orbit-lab.org/wiki/HowTo/bssidFix
>
> this looks like a good idea to avoid many ibss merge problems, and it could be
> really useful to have that (maybe as an option?) in mac80211. but unless we
> have ibss merge functionality you won't be able to join older networks
> (higher TSF) which don't use that scheme, so we still need that to be able to
> communicate with other ("standard") drivers.
>
> i guess the general question for mac80211 is: do we want and allow more
> liberal interpretations of the standard concerning IBSS mode or do we
> strictly stick with the "book"?
>
> in my opinion these "extensions" are necessary in order to make IBSS mode
> useful at all and therefore we need them! and as long as they are able to
> interoperate with other drivers in IBSS mode i don't see a problem.

I agree 100%.

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> > A solution to this we implemented at Orbit with MadWifi was
> > that instead of generating "random" BSSIDs you'd create one based on
> > the hash of the SSID [1].

> this looks like a good idea to avoid many ibss merge problems, and it could be 
> really useful to have that (maybe as an option?) in mac80211. but unless we 
> have ibss merge functionality you won't be able to join older networks 
> (higher TSF) which don't use that scheme, so we still need that to be able to 
> communicate with other ("standard") drivers.

I think we should simply allow setting the BSSID (SIOCSIWAP maybe?) on
IBSS interfaces and punt this issue to userspace.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[Jouni Malinen]

On Sat, Feb 02, 2008 at 06:22:12PM -0500, Luis R. Rodriguez wrote:

> problems. A solution to this we implemented at Orbit with MadWifi was
> that instead of generating "random" BSSIDs you'd create one based on
> the hash of the SSID [1]. This ensures that nodes with identical SSIDs
> end up with identical BSSIDs, regardless of any strange problems.

How does that work with multiple channels? The patch did not seem to do
more than just use a very weak "hash" of the SSID to generate the BSSID.
In other words, I would assume it leaves IBSS STAs on whatever channel
they end up selecting, i.e., in separate groups, unless all STAs are
forced to use the same channel by not allowing them to initialize the
IBSS on other channels. This would also likely require hardcoding the
STAs not to allow merging to other BSSIDs should there be any such
(i.e., any STA that does not implement this hack).

> Technically from what I have gathered this doesn't break the specs but
> would prevent this split-IBSS problem. I actually like to see this
> technique added into mac80211 too.
> 
> [1] http://www.orbit-lab.org/wiki/HowTo/bssidFix

I don't see how this alone would either prevent the split-IBSS problem
or be compliant with the IEEE 802.11 standard. The change in this patch
makes it likely to get same BSSID for different IBSS since the first six
octets of the SSID is used as the BSSID. That would not be very
desirable.. Furthermore, it is unlikely to work well with STA
implementations that do not implement the identical mechanism for
generating the BSSID.

The BSSID generation as a hash from SSID does not sound like something
that would match the requirements of IEEE 802.11 11.1.6 ("a number
selected in a manner that minimizes the probability of STAs generating
the same number") since it is actually maximizing that probablity for
the case of same SSID ;-).

As far as joining of separate STA groups in an IBSS (same SSID) is
concerned, 11.1.4 seems to indicate that the STA in IBSS shall adopt
_all_ parameters (i.e., also BSSID and channel) contained in the Beacon
frame when the Timestamp field of the received Beacon is later than the
STA's own TSF timer. This seems to be a requirement for supporting
merging of separate IBSS groups.. Actually hearing a Beacon from another
channel may be an issue, though, but at least as far as separate groups
using different BSSIDs, but same SSID, on the same channel is concerned,
the merging behavior seems to be defined in the standard.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Feb 5, 2008 11:34 PM, Jouni Malinen <j@w1.fi> wrote:
> On Sat, Feb 02, 2008 at 06:22:12PM -0500, Luis R. Rodriguez wrote:
>
> > problems. A solution to this we implemented at Orbit with MadWifi was
> > that instead of generating "random" BSSIDs you'd create one based on
> > the hash of the SSID [1]. This ensures that nodes with identical SSIDs
> > end up with identical BSSIDs, regardless of any strange problems.
>
> How does that work with multiple channels?

Good point. It never really accounted for it. This patch to MadWifi
was just a hack to prevent IBSS split on a large grid, and should be
treated as such. I would have to check with MadWifi but my guess here
would be that we didn't see issue with channels as the channel
selected would be a default and all the cards were using the same
driver and hardware.

> The patch did not seem to do
> more than just use a very weak "hash" of the SSID to generate the BSSID.

Indeed the hash is extremely weak. If we were to use something similar
we'd have to consider a stronger hash, say maybe SHA1.

> In other words, I would assume it leaves IBSS STAs on whatever channel
> they end up selecting, i.e., in separate groups, unless all STAs are
> forced to use the same channel by not allowing them to initialize the
> IBSS on other channels.

Yes I see your point, this is definitely an issue, unless of course
the driver/stack is configured to default to use a specific channel by
default. But regardless its a problem.

> This would also likely require hardcoding the
> STAs not to allow merging to other BSSIDs should there be any such
> (i.e., any STA that does not implement this hack).

Well the idea is that if it misses the timer from the leader it'll try
to create the IBSS but will end up using the same BSSID as the leader
would have, eventually causing a merge amongst the nodes. So you would
not merge unless its with the same BSSID.

> > Technically from what I have gathered this doesn't break the specs but
> > would prevent this split-IBSS problem. I actually like to see this
> > technique added into mac80211 too.
> >
> > [1] http://www.orbit-lab.org/wiki/HowTo/bssidFix
>
> I don't see how this alone would either prevent the split-IBSS problem

I haven't run the test myself but I am informed this hack helps to try
to solve the issue with 400 nodes on IBSS mode.

> or be compliant with the IEEE 802.11 standard. The change in this patch
> makes it likely to get same BSSID for different IBSS since the first six
> octets of the SSID is used as the BSSID. That would not be very
> desirable..

Agreed, the hash used is very weak, again its a hack but perhaps it
can help find a better solution. AFAICT this is still an open problem.

> Furthermore, it is unlikely to work well with STA
> implementations that do not implement the identical mechanism for
> generating the BSSID.

Agreed, it definitely won't improve the situation in those cases, but
with those that do it would at least help.

> The BSSID generation as a hash from SSID does not sound like something
> that would match the requirements of IEEE 802.11 11.1.6 ("a number
> selected in a manner that minimizes the probability of STAs generating
> the same number") since it is actually maximizing that probablity for
> the case of same SSID ;-).

Heh, good catch :)

> As far as joining of separate STA groups in an IBSS (same SSID) is
> concerned, 11.1.4 seems to indicate that the STA in IBSS shall adopt
> _all_ parameters (i.e., also BSSID and channel) contained in the Beacon
> frame when the Timestamp field of the received Beacon is later than the
> STA's own TSF timer. This seems to be a requirement for supporting
> merging of separate IBSS groups..

ACK

> Actually hearing a Beacon from another
> channel may be an issue, though,

Broken? :)

> but at least as far as separate groups
> using different BSSIDs, but same SSID, on the same channel is concerned,
> the merging behavior seems to be defined in the standard.

ACK. Again, this seems like an open problem.

We *could* take the channel into considering for the hash along with
SSID... Anyway, good points!

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[John W. Linville]

On Wed, Feb 06, 2008 at 01:33:31PM -0500, Luis R. Rodriguez wrote:
> On Feb 5, 2008 11:34 PM, Jouni Malinen <j@w1.fi> wrote:

> > The BSSID generation as a hash from SSID does not sound like something
> > that would match the requirements of IEEE 802.11 11.1.6 ("a number
> > selected in a manner that minimizes the probability of STAs generating
> > the same number") since it is actually maximizing that probablity for
> > the case of same SSID ;-).
> 
> Heh, good catch :)

FWIW, that wouldn't be the first time we (i.e. Linux) chose to do
something that did not completely comply with a standard.  If it
interoperates with other equipment and is good for users, I don't
think picky standard compliance is worthwhile.

Please note that I am not really taking a stand in favor of this ATM,
only asserting that blind standard compliance is not a good reason
to NAK it IMHO.

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] mac80211: enable IBSS merging

[Jouni Malinen]

On Wed, Feb 06, 2008 at 03:10:35PM -0500, John W. Linville wrote:

> FWIW, that wouldn't be the first time we (i.e. Linux) chose to do
> something that did not completely comply with a standard.  If it
> interoperates with other equipment and is good for users, I don't
> think picky standard compliance is worthwhile.

Sure, there are cases where this is reasonable.

> Please note that I am not really taking a stand in favor of this ATM,
> only asserting that blind standard compliance is not a good reason
> to NAK it IMHO.

In this case, standard compliance is certainly not the only concern I
have. I'm worried about interoperability with non-mac80211
implementations. If mac80211 were to hardcoded the BSSID for IBSS and
refuse to change it no matter what (i.e., behave against the IEEE 802.11
standard), IBSS would not interoperate with any other implementation if
the other implementation happens to be the creator of the IBSS..

Same issues shows up (but in somewhat less frequent form) in an IBSS
splitting up and re-joining. In order to allow the STAs using other
implementation (no BSSID hacks) to join the IBSS, mac80211 will need to
be prepared to change the BSSID (or use some much more questionable
hacks to make sure it is always the mac80211-STA that wins in the
selection of which BSSID will survive.. ;-).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Feb 6, 2008 10:58 PM, Jouni Malinen <j@w1.fi> wrote:
> On Wed, Feb 06, 2008 at 03:10:35PM -0500, John W. Linville wrote:
>
> > FWIW, that wouldn't be the first time we (i.e. Linux) chose to do
> > something that did not completely comply with a standard.  If it
> > interoperates with other equipment and is good for users, I don't
> > think picky standard compliance is worthwhile.
>
> Sure, there are cases where this is reasonable.
>
> > Please note that I am not really taking a stand in favor of this ATM,
> > only asserting that blind standard compliance is not a good reason
> > to NAK it IMHO.
>
> In this case, standard compliance is certainly not the only concern I
> have. I'm worried about interoperability with non-mac80211
> implementations. If mac80211 were to hardcoded the BSSID for IBSS and
> refuse to change it no matter what (i.e., behave against the IEEE 802.11
> standard), IBSS would not interoperate with any other implementation if
> the other implementation happens to be the creator of the IBSS..

OK so you're saying that some people might actually expect that when
using the same SSID and same channel in close vicinity they'd get
separate BSSIDs generated? If so I don't see why people would expect
this functionality as a feature, I think this is more of a problem
than an expected result of how the standard is designed.

> Same issues shows up (but in somewhat less frequent form) in an IBSS
> splitting up and re-joining. In order to allow the STAs using other
> implementation (no BSSID hacks) to join the IBSS, mac80211 will need to
> be prepared to change the BSSID (or use some much more questionable
> hacks to make sure it is always the mac80211-STA that wins in the
> selection of which BSSID will survive.. ;-).

How would the IBSS be split up to generate a new BSSID? If you are
part of an IBSS and you don't see anyone generate a beacon between the
random backoff time you simply generate it, but the BSSID would still
be the same.

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Friday 08 February 2008 18:22:52 Luis R. Rodriguez wrote:
> On Feb 6, 2008 10:58 PM, Jouni Malinen <j@w1.fi> wrote:
> > On Wed, Feb 06, 2008 at 03:10:35PM -0500, John W. Linville wrote:
> > > FWIW, that wouldn't be the first time we (i.e. Linux) chose to do
> > > something that did not completely comply with a standard.  If it
> > > interoperates with other equipment and is good for users, I don't
> > > think picky standard compliance is worthwhile.
> >
> > Sure, there are cases where this is reasonable.
> >
> > > Please note that I am not really taking a stand in favor of this ATM,
> > > only asserting that blind standard compliance is not a good reason
> > > to NAK it IMHO.
> >
> > In this case, standard compliance is certainly not the only concern I
> > have. I'm worried about interoperability with non-mac80211
> > implementations. If mac80211 were to hardcoded the BSSID for IBSS and
> > refuse to change it no matter what (i.e., behave against the IEEE 802.11
> > standard), IBSS would not interoperate with any other implementation if
> > the other implementation happens to be the creator of the IBSS..
>
> OK so you're saying that some people might actually expect that when
> using the same SSID and same channel in close vicinity they'd get
> separate BSSIDs generated? If so I don't see why people would expect
> this functionality as a feature, I think this is more of a problem
> than an expected result of how the standard is designed.
>
> > Same issues shows up (but in somewhat less frequent form) in an IBSS
> > splitting up and re-joining. In order to allow the STAs using other
> > implementation (no BSSID hacks) to join the IBSS, mac80211 will need to
> > be prepared to change the BSSID (or use some much more questionable
> > hacks to make sure it is always the mac80211-STA that wins in the
> > selection of which BSSID will survive.. ;-).
>
> How would the IBSS be split up to generate a new BSSID? If you are
> part of an IBSS and you don't see anyone generate a beacon between the
> random backoff time you simply generate it, but the BSSID would still
> be the same.

i think the point here is that we still need to be prepared to merge IBSS 
(i.e. switch to another BSSID when we receive a beacon with an older TSF) no 
matter how we generate our own BSSID in the first place. 

using a hash of the BSSID and channel would reduce the necessity to merge IBSS 
for mac80211 drivers but we still need to be able to join older IBSS from 
other drivers.

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Mon, Feb 11, 2008 at 9:00 PM, bruno randolf <bruno@thinktube.com> wrote:
>
> On Friday 08 February 2008 18:22:52 Luis R. Rodriguez wrote:
>  > On Feb 6, 2008 10:58 PM, Jouni Malinen <j@w1.fi> wrote:
>  > > On Wed, Feb 06, 2008 at 03:10:35PM -0500, John W. Linville wrote:
>  > > > FWIW, that wouldn't be the first time we (i.e. Linux) chose to do
>  > > > something that did not completely comply with a standard.  If it
>  > > > interoperates with other equipment and is good for users, I don't
>  > > > think picky standard compliance is worthwhile.
>  > >
>  > > Sure, there are cases where this is reasonable.
>  > >
>  > > > Please note that I am not really taking a stand in favor of this ATM,
>  > > > only asserting that blind standard compliance is not a good reason
>  > > > to NAK it IMHO.
>  > >
>  > > In this case, standard compliance is certainly not the only concern I
>  > > have. I'm worried about interoperability with non-mac80211
>  > > implementations. If mac80211 were to hardcoded the BSSID for IBSS and
>  > > refuse to change it no matter what (i.e., behave against the IEEE 802.11
>  > > standard), IBSS would not interoperate with any other implementation if
>  > > the other implementation happens to be the creator of the IBSS..
>  >
>  > OK so you're saying that some people might actually expect that when
>  > using the same SSID and same channel in close vicinity they'd get
>  > separate BSSIDs generated? If so I don't see why people would expect
>  > this functionality as a feature, I think this is more of a problem
>  > than an expected result of how the standard is designed.
>  >
>  > > Same issues shows up (but in somewhat less frequent form) in an IBSS
>  > > splitting up and re-joining. In order to allow the STAs using other
>  > > implementation (no BSSID hacks) to join the IBSS, mac80211 will need to
>  > > be prepared to change the BSSID (or use some much more questionable
>  > > hacks to make sure it is always the mac80211-STA that wins in the
>  > > selection of which BSSID will survive.. ;-).
>  >
>  > How would the IBSS be split up to generate a new BSSID? If you are
>  > part of an IBSS and you don't see anyone generate a beacon between the
>  > random backoff time you simply generate it, but the BSSID would still
>  > be the same.
>
>  i think the point here is that we still need to be prepared to merge IBSS
>  (i.e. switch to another BSSID when we receive a beacon with an older TSF) no
>  matter how we generate our own BSSID in the first place.

Well that is still possible.

>  using a hash of the BSSID and channel would reduce the necessity to merge IBSS
>  for mac80211 drivers but we still need to be able to join older IBSS from
>  other drivers.

Sure, how is it that this prevents that?

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Friday 15 February 2008 10:06:37 Luis R. Rodriguez wrote:
> >  i think the point here is that we still need to be prepared to merge
> > IBSS (i.e. switch to another BSSID when we receive a beacon with an older
> > TSF) no matter how we generate our own BSSID in the first place.
>
> Well that is still possible.
>
> >  using a hash of the BSSID and channel would reduce the necessity to
> > merge IBSS for mac80211 drivers but we still need to be able to join
> > older IBSS from other drivers.
>
> Sure, how is it that this prevents that?

it wouldn't :)

i agree to johannes approach to set the BSSID from userspace, so people can 
use all sorts of hashing algorithms to set the bssid. we could implement a 
default hashing algorithm in "iw" maybe, and introduce a flag "dont merge 
ibss" to prevent ibss merge for situations where we know we don't want 
that...

"iwconfig wlan0 ap xx:xx:xx:xx:xx:xx" doesn't work for ibss right now (i'll 
try to look into that today).

bruno

Re: [PATCH] mac80211: enable IBSS merging

[Jouni Malinen]

On Wed, Feb 06, 2008 at 01:33:31PM -0500, Luis R. Rodriguez wrote:

> Yes I see your point, this is definitely an issue, unless of course
> the driver/stack is configured to default to use a specific channel by
> default. But regardless its a problem.

It doesn't really help even if the driver/stack would be configured to
default to a specific channel. This has to work with other
implementations, too. Hardcoding BSSID based on SSID may be fine for
most cases when a new IBSS is created, but still, the implementation
will need to allow this to be changed when merging with other parts of
the IBSS since those other parts may not implement this type of hack.

I'm somewhat concerned about the possibility of getting the same BSSID
on different channels, though. Using the same BSSID for different groups
is not really a very good solution.. Including the channel number in the
hash data would resolve this concern.

> Well the idea is that if it misses the timer from the leader it'll try
> to create the IBSS but will end up using the same BSSID as the leader
> would have, eventually causing a merge amongst the nodes. So you would
> not merge unless its with the same BSSID.

I'm not completely what the "misses the timer from the leader" is
referring to or even what this "leader" would be in context of IBSS.
Anyway, the implementation has to merge regardless of what BSSID is used
(i.e., SSID match is enough).

> I haven't run the test myself but I am informed this hack helps to try
> to solve the issue with 400 nodes on IBSS mode.

Sure, if you can control all the STAs in the IBSS. That just is not the
case in general and Linux wireless stack must not be designed with the
view point of all STAs following the exact same rules (unless those
rules are based on a required standard behavior)..

Taken into account that we should really implement proper IBSS merge to
allow interoperation with non-mac80211 implementations, I don't know how
much of a benefit this BSSID=hash(SSID,chan) mechanism would really
provide. If it can be shown that the IBSS merges in a large network are
not stable, this could of course be helpful and I don't think I would
object it as long as the BSSID is going to end up being same only on the
same channel and the mac80211 implementation allows the BSSID to be
changed if a Beacon frame is received with a larger timestamp value than
the local TSF timer.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] mac80211: enable IBSS merging

[Luis R. Rodriguez]

On Feb 6, 2008 10:52 PM, Jouni Malinen <j@w1.fi> wrote:
> On Wed, Feb 06, 2008 at 01:33:31PM -0500, Luis R. Rodriguez wrote:
>
> > Yes I see your point, this is definitely an issue, unless of course
> > the driver/stack is configured to default to use a specific channel by
> > default. But regardless its a problem.
>
> It doesn't really help even if the driver/stack would be configured to
> default to a specific channel. This has to work with other
> implementations, too.

Right, my point was just that if a stack was configured to try to use
a channel by default when creating an IBSS and you'd end up with the
same BSSID you'd eventually get a merge. But I see your point about
interoperability and I think its key.

> Hardcoding BSSID based on SSID may be fine for
> most cases when a new IBSS is created, but still, the implementation
> will need to allow this to be changed when merging with other parts of
> the IBSS since those other parts may not implement this type of hack.

I'm not quite sure I understand the problem here. The *key* feature of
using some sort of a hash is just to guarantee the "random" BSSID you
create matches that of the other stations using the same BSSID (and
channel if channel is part of the hash as well as you suggest). What
changes here is just the method for generation of the BSSID should no
beacon be received yet for the SSID chosen.

> I'm somewhat concerned about the possibility of getting the same BSSID
> on different channels, though. Using the same BSSID for different groups
> is not really a very good solution.. Including the channel number in the
> hash data would resolve this concern.

Sure.

> > Well the idea is that if it misses the timer from the leader it'll try
> > to create the IBSS but will end up using the same BSSID as the leader
> > would have, eventually causing a merge amongst the nodes. So you would
> > not merge unless its with the same BSSID.
>
> I'm not completely what the "misses the timer from the leader" is
> referring to or even what this "leader" would be in context of IBSS.
> Anyway, the implementation has to merge regardless of what BSSID is used
> (i.e., SSID match is enough).

Oh well I mean the TSF becaon leader when in IBSS in the BSS during
beacon generation. When in IBSS you have a node who has the greatest
time for the TSF, this is what I call the leader. If a STA misses the
"leader beacon" it'll generate its own beacon and may create a
separate BSS. If however, you change the algorithm for picking the
"random BSSID" and instead use some sort of hash based on desired SSID
(and channel) you'd end up generating beacons for the same BSSID,
eventually forcing a merge even if you missed the first few beacons
from the TSF leader for the BSS.

> > I haven't run the test myself but I am informed this hack helps to try
> > to solve the issue with 400 nodes on IBSS mode.
>
> Sure, if you can control all the STAs in the IBSS. That just is not the
> case in general and Linux wireless stack must not be designed with the
> view point of all STAs following the exact same rules (unless those
> rules are based on a required standard behavior)..

Agreed that we shouldn't assume everyone will have the same hacks, and
that we should focus on interoperability but I am wondering how this
could break things. All that would be changed here is the algorithm
for BSSID generation, instead of using a random BSSID you'd use one as
a hash of say SSID and channel. Can you think of issues with
interoperability with STAs who choose a random BSSID generator instead
of a hash other than those using a hash being able to guarantee
sticking to the same BSSID ?

> Taken into account that we should really implement proper IBSS merge to
> allow interoperation with non-mac80211 implementations, I don't know how
> much of a benefit this BSSID=hash(SSID,chan) mechanism would really
> provide.

It simply would solve the IBSS split issue, which can be caused by
several reasons, until that is solved within the standard or someone
thinks of a better solution.

> If it can be shown that the IBSS merges in a large network are
> not stable,

Oh we have proof for this :) and we also have proof of how using a
hash helps correct it, of course, as  hack. I'll try to see if we can
come up with something a bit more formal I guess.

> this could of course be helpful and I don't think I would
> object it as long as the BSSID is going to end up being same only on the
> same channel and the mac80211 implementation allows the BSSID to be
> changed if a Beacon frame is received with a larger timestamp value than
> the local TSF timer.

Respecting the higher TSF is not affected, all that would be changed
would be the algorithm used for generating the BSSID. Once we have
IBSS merge on mac80211 I'll then test a patch for this on the 40x40
grid with ath5k and let you know how it goes. I'll try to use two
BSSes, each on a separate channel. Not sure yet what topology to use.

  Luis

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

hello johannes!

i appreciate your feedback, but on a second thought, let me question this:

On Wednesday 23 January 2008 23:48:49 Johannes Berg wrote:
> Comments on the code now.
>
> > --- a/net/mac80211/ieee80211_sta.c
> > +++ b/net/mac80211/ieee80211_sta.c
> > @@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_device
> > *dev, struct ieee80211_sta_bss *bss); static int
> > ieee80211_sta_find_ibss(struct net_device *dev,
> >                                    struct ieee80211_if_sta *ifsta);
> > +static int ieee80211_sta_join_ibss(struct net_device *dev,
> > +                                  struct ieee80211_if_sta *ifsta,
> > +                                  struct ieee80211_sta_bss *bss);
>
> No way, order the code properly, this mess needs to be cleaned up not
> added to.

do you care to explain what's so bad about function declarations?

> > +	if (sdata->vif.type == IEEE80211_IF_TYPE_IBSS && beacon &&
> > +	    !local->sta_sw_scanning && !local->sta_hw_scanning &&
> > +	    mgmt->u.beacon.capab_info & WLAN_CAPABILITY_IBSS &&
> > +	    memcmp(elems.ssid, sdata->u.sta.ssid, sdata->u.sta.ssid_len) == 0)
> > {
>
> I think that needs a check "&& ssid_len" or something. Someone's bound
> to try making an IBSS with a hidden SSID and we really don't want that
> to work.

why? i know, hidden SSIDs are pretty useless but if somedody wants them... why 
would you want to make extra effort so it does not work?

> > +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> > +				" -> IBSS merge with BSSID %s\n",
> > +				dev->name, print_mac(mac, mgmt->bssid));
>
> No way. At the very least you need to ratelimit this.

well, it should not happen very often, and for debugging i would like to see 
this case. would

 			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
				printk(KERN_DEBUG "%s: beacon TSF higher than "

be ok with you?

regards,
bruno

Re: [PATCH] mac80211: enable IBSS merging

[Kalle Valo]

bruno randolf <bruno@thinktube.com> writes:

>> > --- a/net/mac80211/ieee80211_sta.c
>> > +++ b/net/mac80211/ieee80211_sta.c
>> > @@ -80,6 +80,9 @@ static void ieee80211_rx_bss_put(struct net_device
>> > *dev, struct ieee80211_sta_bss *bss); static int
>> > ieee80211_sta_find_ibss(struct net_device *dev,
>> >                                    struct ieee80211_if_sta *ifsta);
>> > +static int ieee80211_sta_join_ibss(struct net_device *dev,
>> > +                                  struct ieee80211_if_sta *ifsta,
>> > +                                  struct ieee80211_sta_bss *bss);
>>
>> No way, order the code properly, this mess needs to be cleaned up not
>> added to.
>
> Do you care to explain what's so bad about function declarations?

I don't know about Johannes, but I just see them as unnecessary
burden. If I change some parameters in a function, I'll get a compiler
error and only after that I realise to change the declaration as well.

With public (ie. non-static) functions it's a different issue, because
I know that there's a declaration in the header file. But with static
functions this is not the case.

-- 
Kalle Valo

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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



> > Do you care to explain what's so bad about function declarations?
> 
> I don't know about Johannes, but I just see them as unnecessary
> burden. If I change some parameters in a function, I'll get a compiler
> error and only after that I realise to change the declaration as well.
> 
> With public (ie. non-static) functions it's a different issue, because
> I know that there's a declaration in the header file. But with static
> functions this is not the case.

Seconded. I also see it as totally unnecessary if you structure the code
properly in the first place.

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[Johannes Berg]

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


> > I think that needs a check "&& ssid_len" or something. Someone's bound
> > to try making an IBSS with a hidden SSID and we really don't want that
> > to work.
> 
> why? i know, hidden SSIDs are pretty useless but if somedody wants them... why 
> would you want to make extra effort so it does not work?

Uh, I think you should rethink your question. Consider two different
hidden-SSID IBSSes.

> > > +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> > > +				" -> IBSS merge with BSSID %s\n",
> > > +				dev->name, print_mac(mac, mgmt->bssid));
> >
> > No way. At the very least you need to ratelimit this.
> 
> well, it should not happen very often, and for debugging i would like to see 
> this case. would

Well, "should" is pretty soft. It can also be trivially triggered as a
DOS attack by sending beacons with fake timestamps.

> 
>  			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
> 				printk(KERN_DEBUG "%s: beacon TSF higher than "
> 
> be ok with you?

Maybe && would be better?

johannes

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

Re: [PATCH] mac80211: enable IBSS merging

[bruno randolf]

On Thursday 24 January 2008 23:30:02 Johannes Berg wrote:
> > > I think that needs a check "&& ssid_len" or something. Someone's bound
> > > to try making an IBSS with a hidden SSID and we really don't want that
> > > to work.
> >
> > why? i know, hidden SSIDs are pretty useless but if somedody wants
> > them... why would you want to make extra effort so it does not work?
>
> Uh, I think you should rethink your question. Consider two different
> hidden-SSID IBSSes.

that's the same case as two different IBSS with the same non-zero ESSID.

> > > > +			printk(KERN_DEBUG "%s: beacon TSF higher than local TSF"
> > > > +				" -> IBSS merge with BSSID %s\n",
> > > > +				dev->name, print_mac(mac, mgmt->bssid));
> > >
> > > No way. At the very least you need to ratelimit this.
> >
> > well, it should not happen very often, and for debugging i would like to
> > see this case. would
>
> Well, "should" is pretty soft. It can also be trivially triggered as a
> DOS attack by sending beacons with fake timestamps.

ok, i agree that a ratelimit is necessary.

> >  			if (CONFIG_MAC80211_IBSS_DEBUG || net_ratelimit())
> > 				printk(KERN_DEBUG "%s: beacon TSF higher than "
> >
> > be ok with you?
>
> Maybe && would be better?

well, i would like to see every merge when debugging IBSS, hence the OR.
if you want i can remove that message when IBSS debugging is disabled.

bruno