[PATCH][RFC] softmac: suggest TX rate

[PATCH][RFC] softmac: suggest TX rate

[Daniel Drake]

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

While developing the ZD1211 driver we realised how much we'd like to be 
advised from the upper layers which rate to transmit a packet at.

An example: We have a frame to transmit. What rate should we transmit it 
at? While taking any user-specified rate into account too, we want to 
transmit it at a rate supported by the access point, but that 
information is not available to us (softmac handled all scanning and 
association, so we don't know anything about the AP capabilities).

Here is a patch I cooked up, which implements some basic logic for 
suggesting TX rates based on the packet type, whether it is 
multicast/unicast, whether we are associated, and the current 
user-specified default_rate.

I'm still uncertain where this functionality should fit into the stack.

Rather than having to explicitly call ieee80211softmac_suggest_tx_rate() 
from the driver's hard_start_xmit function, it would be nicer if the 
suggested rate was passed as a parameter. But not all drivers would need 
it, so maybe the extra calculations should be controlled by a new flag.

Passing an extra parameter to hard_start_xmit would involve modifying 
ieee80211, and ieee80211 doesn't seem to have any concept of whether it 
is associated or not (plus softmac only does that loosely). So that 
might lead us on to do something silly like wrapping hard_start_xmit 
inside softmac, etc etc...

Note that even in it's current form, this patch eliminates an annoying 
(and inaccurate) chunk of code from our driver.

Ideas/comments?

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3457 bytes --]

--- linux/net/ieee80211/softmac/ieee80211softmac_module.c.orig	2006-04-17 01:04:42.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_module.c	2006-04-17 00:44:36.000000000 +0100
@@ -26,6 +26,7 @@
 
 #include "ieee80211softmac_priv.h"
 #include <linux/sort.h>
+#include <linux/etherdevice.h>
 
 struct net_device *alloc_ieee80211softmac(int sizeof_priv)
 {
@@ -315,6 +316,48 @@ static int rate_cmp(const void *a_, cons
 	return ((*a & ~IEEE80211_BASIC_RATE_MASK) - (*b & ~IEEE80211_BASIC_RATE_MASK));
 }
 
+static u8 suggest_rate_from_associnfo(struct ieee80211softmac_device *mac)
+{
+	struct ieee80211softmac_ratesinfo *net_rates = &mac->associnfo.supported_rates;
+	u8 net_max;
+
+	if (unlikely(net_rates->count == 0)) {
+		dprintkl(KERN_ERROR PFX "suggest_rate: Network has no rates?\n");
+		return IEEE80211_CCK_RATE_1MB;
+	}
+
+	/* FIXME: we need to check that the rate is supported in mac->ratesinfo */
+	net_max = net_rates->rates[net_rates->count - 1];
+	return min(mac->txrates.default_rate, net_max);
+}
+
+u8 ieee80211softmac_suggest_tx_rate(struct ieee80211softmac_device *mac,
+	struct ieee80211_hdr_1addr *hdr)
+{
+	switch (WLAN_FC_GET_TYPE(le16_to_cpu(hdr->frame_ctl))) {
+	case IEEE80211_FTYPE_MGMT:
+		/*
+		 * If we aren't associated, or we are multicasting, then
+		 * stick to 1MB for safety.
+		 */
+		if (!mac->associated || is_multicast_ether_addr(hdr->addr1))
+			return IEEE80211_CCK_RATE_1MB;
+
+		/* Otherwise, we can send at the speed of the AP. */
+		return suggest_rate_from_associnfo(mac);
+	case IEEE80211_FTYPE_DATA:
+		if (unlikely(!mac->associated)) {
+			dprintkl(KERN_ERROR PFX "suggest_tx_rate: Not associated\n");
+			return IEEE80211_CCK_RATE_1MB;
+		}
+		return suggest_rate_from_associnfo(mac);
+	default:
+		dprintkl(KERN_ERROR PFX "suggest_tx_rate: Unhandled ftype %x\n", ftype);
+		return IEEE80211_CCK_RATE_1MB;
+	}
+}
+EXPORT_SYMBOL_GPL(ieee80211softmac_suggest_tx_rate);
+
 /* Allocate a softmac network struct and fill it from a network */
 struct ieee80211softmac_network *
 ieee80211softmac_create_network(struct ieee80211softmac_device *mac,
--- linux/net/ieee80211/softmac/ieee80211softmac_assoc.c.orig	2006-04-16 23:55:23.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-04-16 23:55:39.000000000 +0100
@@ -283,6 +283,7 @@ ieee80211softmac_associated(struct ieee8
 	struct ieee80211softmac_network *net)
 {
 	mac->associnfo.associating = 0;
+	mac->associnfo.supported_rates = net->supported_rates;
 	mac->associated = 1;
 	if (mac->set_bssid_filter)
 		mac->set_bssid_filter(mac->dev, net->bssid);
--- linux/include/net/ieee80211softmac.h.orig	2006-04-16 23:37:32.000000000 +0100
+++ linux/include/net/ieee80211softmac.h	2006-04-17 00:44:47.000000000 +0100
@@ -86,6 +86,9 @@ struct ieee80211softmac_assoc_info {
 	
 	/* BSSID we're trying to associate to */
 	char bssid[ETH_ALEN];
+
+	/* Rates supported by the network */
+	struct ieee80211softmac_ratesinfo supported_rates;
 	
 	/* some flags.
 	 * static_essid is valid if the essid is constant,
@@ -246,6 +249,8 @@ extern void ieee80211softmac_fragment_lo
  * Note that the rates need to be sorted. */
 extern void ieee80211softmac_set_rates(struct net_device *dev, u8 count, u8 *rates);
 
+extern u8 ieee80211softmac_suggest_tx_rate(struct ieee80211softmac_device *mac, struct ieee80211_hdr_1addr *hdr);
+
 /* Start the SoftMAC. Call this after you initialized the device
  * and it is ready to run.
  */

Re: [PATCH][RFC] softmac: suggest TX rate

[Ulrich Kunitz]

On Mon, 17 Apr 2006, Daniel Drake wrote:

> While developing the ZD1211 driver we realised how much we'd like to be
> advised from the upper layers which rate to transmit a packet at.
> 
> An example: We have a frame to transmit. What rate should we transmit it at?
> While taking any user-specified rate into account too, we want to transmit it
> at a rate supported by the access point, but that information is not available
> to us (softmac handled all scanning and association, so we don't know anything
> about the AP capabilities).
> 
> Here is a patch I cooked up, which implements some basic logic for suggesting
> TX rates based on the packet type, whether it is multicast/unicast, whether we
> are associated, and the current user-specified default_rate.
> 
> I'm still uncertain where this functionality should fit into the stack.
> 
> Rather than having to explicitly call ieee80211softmac_suggest_tx_rate() from
> the driver's hard_start_xmit function, it would be nicer if the suggested rate
> was passed as a parameter. But not all drivers would need it, so maybe the
> extra calculations should be controlled by a new flag.
> 
> Passing an extra parameter to hard_start_xmit would involve modifying
> ieee80211, and ieee80211 doesn't seem to have any concept of whether it is
> associated or not (plus softmac only does that loosely). So that might lead us
> on to do something silly like wrapping hard_start_xmit inside softmac, etc
> etc...
> 
> Note that even in it's current form, this patch eliminates an annoying (and
> inaccurate) chunk of code from our driver.
> 
> Ideas/comments?
> 

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

Re: [PATCH][RFC] softmac: suggest TX rate

[Ulrich Kunitz]

On Mon, 17 Apr 2006, Ulrich Kunitz wrote:

Oops! Sorry, but sometime ^X and ^C are to near to each other.

I just wanted to say, I agree with the approach, but wonder how
transmission timeouts come into play here, which should lead to a
decrease of the tranmission rate. 

Daniel the patches on branch softmac-suggest-txrate look good!

Regards,

Uli

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

Re: [PATCH][RFC] softmac: suggest TX rate

[Daniel Drake]

Ulrich Kunitz wrote:
> I just wanted to say, I agree with the approach, but wonder how
> transmission timeouts come into play here, which should lead to a
> decrease of the tranmission rate. 

That's a separate issue, to do with rate management, which I'm not 
tackling yet. Note that the functions in my patch can easily be extended 
to bring rate management into the equation.

Daniel

Re: [PATCH][RFC] softmac: suggest TX rate

[Johannes Berg]

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

On Mon, 2006-04-17 at 01:28 +0100, Daniel Drake wrote:

> Rather than having to explicitly call ieee80211softmac_suggest_tx_rate() 
> from the driver's hard_start_xmit function, it would be nicer if the 
> suggested rate was passed as a parameter. But not all drivers would need 
> it, so maybe the extra calculations should be controlled by a new flag.

Couldn't we just initialise the softmac txrates substructure to
something useful when associating, and have drivers look into that
instead?

> +	case IEEE80211_FTYPE_DATA:
> +		if (unlikely(!mac->associated)) {
> +			dprintkl(KERN_ERROR PFX "suggest_tx_rate: Not associated\n");
> +			return IEEE80211_CCK_RATE_1MB;
> +		}
> +		return suggest_rate_from_associnfo(mac);

This isn't correct. Here, you have to take into account multicast frames
because those require clamping the rate to the highest rate from the
basic rateset.

I'll try to come up with an alternative.

johannes

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

Re: [PATCH][RFC] softmac: suggest TX rate

[Daniel Drake]

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

Johannes Berg wrote:
> Couldn't we just initialise the softmac txrates substructure to
> something useful when associating, and have drivers look into that
> instead?

How about this?
It also addresses the fact that multicast data must be transmitted at a 
basic rate.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 10882 bytes --]

--- linux/include/net/ieee80211softmac.h.orig	2006-04-16 23:37:32.000000000 +0100
+++ linux/include/net/ieee80211softmac.h	2006-04-17 18:40:04.000000000 +0100
@@ -86,6 +86,9 @@ struct ieee80211softmac_assoc_info {
 	
 	/* BSSID we're trying to associate to */
 	char bssid[ETH_ALEN];
+
+	/* Rates supported by the network */
+	struct ieee80211softmac_ratesinfo supported_rates;
 	
 	/* some flags.
 	 * static_essid is valid if the essid is constant,
@@ -133,12 +136,23 @@ struct ieee80211softmac_txrates {
 	 * (If the device supports fallback and hardware-retry)
 	 */
 	u8 mcast_fallback;
+
+	/* The Bit-Rate to be used for multicast management frames. */
+	u8 mgt_mcast_rate;
+	/* The Bit-Rate to be used for multicast management fallback
+	 * (If the device supports fallback and hardware-retry)
+	 */
+	u8 mgt_mcast_fallback;
+
 	/* The Bit-Rate to be used for any other (normal) data packet. */
 	u8 default_rate;
 	/* The Bit-Rate to be used for default fallback
 	 * (If the device supports fallback and hardware-retry)
 	 */
 	u8 default_fallback;
+
+	/* This is the rate that the user asked for */
+	u8 user_rate;
 };
 
 /* Bits for txrates_change callback. */
@@ -146,6 +160,8 @@ struct ieee80211softmac_txrates {
 #define IEEE80211SOFTMAC_TXRATECHG_DEFAULT_FBACK	(1 << 1) /* default_fallback */
 #define IEEE80211SOFTMAC_TXRATECHG_MCAST		(1 << 2) /* mcast_rate */
 #define IEEE80211SOFTMAC_TXRATECHG_MCAST_FBACK		(1 << 3) /* mcast_fallback */
+#define IEEE80211SOFTMAC_TXRATECHG_MGT_MCAST		(1 << 4) /* mgt_mcast_rate */
+#define IEEE80211SOFTMAC_TXRATECHG_MGT_MCAST_FBACK	(1 << 5) /* mgt_mcast_fallback */
 
 struct ieee80211softmac_device {
 	/* 802.11 structure for data stuff */
@@ -246,6 +262,28 @@ extern void ieee80211softmac_fragment_lo
  * Note that the rates need to be sorted. */
 extern void ieee80211softmac_set_rates(struct net_device *dev, u8 count, u8 *rates);
 
+/* Helper function which advises you the rate at which a frame should be
+ * transmitted at. */
+static inline u8 ieee80211softmac_suggest_txrate(struct ieee80211softmac_device *mac,
+						 int is_multicast,
+						 int is_mgt)
+{
+	struct ieee80211softmac_txrates *txrates = &mac->txrates;
+
+	if (!mac->associated)
+		return txrates->mgt_mcast_rate;
+
+	/* We are associated, sending unicast frame */
+	if (!is_multicast)
+		return txrates->default_rate;
+
+	/* We are associated, sending multicast frame */
+	if (is_mgt)
+		return txrates->mgt_mcast_rate;
+	else
+		return txrates->mcast_rate;
+}
+
 /* Start the SoftMAC. Call this after you initialized the device
  * and it is ready to run.
  */
--- linux/net/ieee80211/softmac/ieee80211softmac_module.c.orig	2006-04-17 01:04:42.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_module.c	2006-04-17 18:30:04.000000000 +0100
@@ -26,6 +26,7 @@
 
 #include "ieee80211softmac_priv.h"
 #include <linux/sort.h>
+#include <linux/etherdevice.h>
 
 struct net_device *alloc_ieee80211softmac(int sizeof_priv)
 {
@@ -166,15 +167,85 @@ static void ieee80211softmac_start_check
 	}
 }
 
-void ieee80211softmac_start(struct net_device *dev)
+int ieee80211softmac_ratesinfo_rate_supported(struct ieee80211softmac_ratesinfo *ri, u8 rate)
+{
+	int search;
+	u8 search_rate;
+
+	for (search = 0; search < ri->count; search++) {
+		search_rate = ri->rates[search];
+		search_rate &= ~IEEE80211_BASIC_RATE_MASK;
+		if (rate == search_rate)
+			return 1;
+	}
+
+	return 0;
+}
+
+/* Finds the highest rate which is:
+ *  1. Present in ri (optionally a basic rate)
+ *  2. Supported by the device
+ *  3. Less than or equal to the user-defined rate
+ */
+static u8 highest_supported_rate(struct ieee80211softmac_device *mac,
+	struct ieee80211softmac_ratesinfo *ri, int basic_only)
+{
+	u8 user_rate = mac->txrates.user_rate;
+	int i;
+	
+	if (ri->count == 0) {
+		dprintk(KERN_ERROR PFX "empty ratesinfo?\n");
+		return IEEE80211_CCK_RATE_1MB;
+	}
+
+	for (i = ri->count - 1; i >= 0; i--) {
+		u8 rate = ri->rates[i];
+		if (basic_only && !(rate & IEEE80211_BASIC_RATE_MASK))
+			continue;
+		rate &= ~IEEE80211_BASIC_RATE_MASK;
+		if (rate > user_rate)
+			continue;
+		if (ieee80211softmac_ratesinfo_rate_supported(&mac->ratesinfo, rate))
+			return rate;
+	}
+
+	/* If we haven't found a suitable rate by now, just trust the user */
+	return user_rate;
+}
+
+void ieee80211softmac_recalc_txrates(struct ieee80211softmac_device *mac)
+{
+	struct ieee80211softmac_txrates *txrates = &mac->txrates;
+	struct ieee80211softmac_txrates oldrates;
+	u32 change = 0;
+
+	if (mac->txrates_change)
+		oldrates = mac->txrates;
+
+	change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT;
+	txrates->default_rate = highest_supported_rate(mac, &mac->associnfo.supported_rates, 0);
+
+	change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT_FBACK;
+	txrates->default_fallback = lower_rate(mac, txrates->default_rate);
+
+	change |= IEEE80211SOFTMAC_TXRATECHG_MCAST;
+	txrates->mcast_rate = highest_supported_rate(mac, &mac->associnfo.supported_rates, 1);
+
+	change |= IEEE80211SOFTMAC_TXRATECHG_MGT_MCAST_FBACK;
+	txrates->mcast_fallback = lower_rate(mac, txrates->mcast_rate);
+
+	if (mac->txrates_change)
+		mac->txrates_change(mac->dev, change, &oldrates);
+
+}
+
+void ieee80211softmac_init_txrates(struct ieee80211softmac_device *mac)
 {
-	struct ieee80211softmac_device *mac = ieee80211_priv(dev);
 	struct ieee80211_device *ieee = mac->ieee;
 	u32 change = 0;
+	struct ieee80211softmac_txrates *txrates = &mac->txrates;
 	struct ieee80211softmac_txrates oldrates;
 
-	ieee80211softmac_start_check_rates(mac);
-
 	/* TODO: We need some kind of state machine to lower the default rates
 	 *       if we loose too many packets.
 	 */
@@ -189,19 +260,40 @@ void ieee80211softmac_start(struct net_d
 	   more reliable. Note similar logic in
 	   ieee80211softmac_wx_set_rate() */	 
 	if (ieee->modulation & IEEE80211_CCK_MODULATION) {
-		mac->txrates.default_rate = IEEE80211_CCK_RATE_11MB;
-		change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT;
-		mac->txrates.default_fallback = IEEE80211_CCK_RATE_5MB;
-		change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT_FBACK;
+		txrates->user_rate = IEEE80211_CCK_RATE_11MB;
 	} else if (ieee->modulation & IEEE80211_OFDM_MODULATION) {
-		mac->txrates.default_rate = IEEE80211_OFDM_RATE_54MB;
-		change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT;
-		mac->txrates.default_fallback = IEEE80211_OFDM_RATE_24MB;
-		change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT_FBACK;
+		txrates->user_rate = IEEE80211_OFDM_RATE_54MB;
 	} else
 		assert(0);
+
+	txrates->default_rate = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT;
+
+	txrates->default_fallback = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_DEFAULT_FBACK;
+
+	txrates->mcast_rate = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_MCAST;
+
+	txrates->mcast_fallback = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_MCAST_FBACK;
+
+	txrates->mgt_mcast_rate = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_MGT_MCAST;
+
+	txrates->mgt_mcast_fallback = IEEE80211_CCK_RATE_1MB;
+	change |= IEEE80211SOFTMAC_TXRATECHG_MGT_MCAST_FBACK;
+
 	if (mac->txrates_change)
-		mac->txrates_change(dev, change, &oldrates);
+		mac->txrates_change(mac->dev, change, &oldrates);
+}
+
+void ieee80211softmac_start(struct net_device *dev)
+{
+	struct ieee80211softmac_device *mac = ieee80211_priv(dev);
+
+	ieee80211softmac_start_check_rates(mac);
+	ieee80211softmac_init_txrates(mac);
 }
 EXPORT_SYMBOL_GPL(ieee80211softmac_start);
 
--- linux/net/ieee80211/softmac/ieee80211softmac_assoc.c.orig	2006-04-16 23:55:23.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-04-17 17:17:59.000000000 +0100
@@ -101,30 +101,22 @@ ieee80211softmac_disassoc(struct ieee802
 	/* Do NOT clear bssvalid as that will break ieee80211softmac_assoc_work! */
 	mac->associated = 0;
 	mac->associnfo.associating = 0;
+	ieee80211softmac_init_txrates(mac);
 	spin_unlock_irqrestore(&mac->lock, flags);
 }
 
 static inline int
 we_support_all_basic_rates(struct ieee80211softmac_device *mac, u8 *from, u8 from_len)
 {
-	int idx, search, found;
-	u8 rate, search_rate;
+	int idx;
+	u8 rate;
 
 	for (idx = 0; idx < (from_len); idx++) {
 		rate = (from)[idx];
 		if (!(rate & IEEE80211_BASIC_RATE_MASK))
 			continue;
-		found = 0;
 		rate &= ~IEEE80211_BASIC_RATE_MASK;
-		for (search = 0; search < mac->ratesinfo.count; search++) {
-			search_rate = mac->ratesinfo.rates[search];
-			search_rate &= ~IEEE80211_BASIC_RATE_MASK;
-			if (rate == search_rate) {
-				found = 1;
-				break;
-			}
-		}
-		if (!found)
+		if (!ieee80211softmac_ratesinfo_rate_supported(&mac->ratesinfo, rate))
 			return 0;
 	}
 	return 1;
@@ -283,6 +275,9 @@ ieee80211softmac_associated(struct ieee8
 	struct ieee80211softmac_network *net)
 {
 	mac->associnfo.associating = 0;
+	mac->associnfo.supported_rates = net->supported_rates;
+	ieee80211softmac_recalc_txrates(mac);
+
 	mac->associated = 1;
 	if (mac->set_bssid_filter)
 		mac->set_bssid_filter(mac->dev, net->bssid);
@@ -373,6 +368,7 @@ ieee80211softmac_handle_disassoc(struct 
 	spin_lock_irqsave(&mac->lock, flags);
 	mac->associnfo.bssvalid = 0;
 	mac->associated = 0;
+	ieee80211softmac_init_txrates(mac);
 	schedule_work(&mac->associnfo.work);
 	spin_unlock_irqrestore(&mac->lock, flags);
 	
--- linux/net/ieee80211/softmac/ieee80211softmac_wx.c.orig	2006-04-17 16:45:51.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_wx.c	2006-04-17 17:27:47.000000000 +0100
@@ -201,8 +201,8 @@ ieee80211softmac_wx_set_rate(struct net_
 	if (is_ofdm && !(ieee->modulation & IEEE80211_OFDM_MODULATION))
 		goto out_unlock;
 
-	mac->txrates.default_rate = rate;
-	mac->txrates.default_fallback = lower_rate(mac, rate);
+	mac->txrates.user_rate = rate;
+	ieee80211softmac_recalc_txrates(mac);
 	err = 0;
 
 out_unlock:	
@@ -223,7 +223,7 @@ ieee80211softmac_wx_get_rate(struct net_
 	int err = -EINVAL;
 
 	spin_lock_irqsave(&mac->lock, flags);
-	switch (mac->txrates.default_rate) {
+	switch (mac->txrates.user_rate) {
 	case IEEE80211_CCK_RATE_1MB:
 		data->bitrate.value = 1000000;
 		break;
--- linux/net/ieee80211/softmac/ieee80211softmac_priv.h.orig	2006-04-17 16:46:09.000000000 +0100
+++ linux/net/ieee80211/softmac/ieee80211softmac_priv.h	2006-04-17 17:09:08.000000000 +0100
@@ -116,7 +116,10 @@ ieee80211softmac_get_network_by_essid(st
 	struct ieee80211softmac_essid *essid);
 
 /* Rates related */
+int ieee80211softmac_ratesinfo_rate_supported(struct ieee80211softmac_ratesinfo *ri, u8 rate);
 u8 ieee80211softmac_lower_rate_delta(struct ieee80211softmac_device *mac, u8 rate, int delta);
+void ieee80211softmac_init_txrates(struct ieee80211softmac_device *mac);
+void ieee80211softmac_recalc_txrates(struct ieee80211softmac_device *mac);
 static inline u8 lower_rate(struct ieee80211softmac_device *mac, u8 rate) {
 	return ieee80211softmac_lower_rate_delta(mac, rate, 1);
 }