minstrel_ht: new rate control module for 802.11n

RFC Superseded Felix Fietkau 2 participants 4 messages Mar 7, 2010 – Mar 8, 2010 mac80211

lore

Share & tools

lore

Felix Fietkau

author

2010-03-07 16:40 UTC #0 lore raw

RFC v2 1/1 minstrel_ht: new rate control module for 802.11n

Hi again,

here is an updated version of my minstrel_ht code. Since the last version, I've added the following improvements:

- Use accurate A-MPDU statistics from tx feedback
- Rewrite the sampling algorithm to optimize for good A-MPDU lengths and faster throughput recovery
- Implement .rate_update to handle changes in HT capabilities or channel bandwidth
- Remove the private tx flags hack

With the new version, performance has improved visibly for HT40 in all of my tests. I think it's getting closer to being ready for inclusion.
---

net/mac80211/Makefile +2 -2

@@ -47,8 +47,8 @@ CFLAGS_driver-trace.o := -I$(src)
 rc80211_pid-y := rc80211_pid_algo.o
 rc80211_pid-$(CONFIG_MAC80211_DEBUGFS) += rc80211_pid_debugfs.o
 
-rc80211_minstrel-y := rc80211_minstrel.o
-rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += rc80211_minstrel_debugfs.o
+rc80211_minstrel-y := rc80211_minstrel.o rc80211_minstrel_ht.o
+rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += rc80211_minstrel_debugfs.o rc80211_minstrel_ht_debugfs.o
 
 mac80211-$(CONFIG_MAC80211_RC_PID) += $(rc80211_pid-y)
 mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y)

net/mac80211/main.c +7 -0

@@ -714,6 +714,10 @@ static int __init ieee80211_init(void)
 	if (ret)
 		return ret;
 
+	ret = rc80211_minstrel_ht_init();
+	if (ret)
+		goto err_minstrel;
+
 	ret = rc80211_pid_init();
 	if (ret)
 		goto err_pid;

@@ -726,6 +730,8 @@ static int __init ieee80211_init(void)
  err_netdev:
 	rc80211_pid_exit();
  err_pid:
+	rc80211_minstrel_ht_exit();
+ err_minstrel:
 	rc80211_minstrel_exit();
 
 	return ret;

@@ -734,6 +740,7 @@ static int __init ieee80211_init(void)
 static void __exit ieee80211_exit(void)
 {
 	rc80211_pid_exit();
+	rc80211_minstrel_ht_exit();
 	rc80211_minstrel_exit();
 
 	/*

net/mac80211/rate.h +9 -0

@@ -137,6 +137,8 @@ static inline void rc80211_pid_exit(void
 #ifdef CONFIG_MAC80211_RC_MINSTREL
 extern int rc80211_minstrel_init(void);
 extern void rc80211_minstrel_exit(void);
+extern int rc80211_minstrel_ht_init(void);
+extern void rc80211_minstrel_ht_exit(void);
 #else
 static inline int rc80211_minstrel_init(void)
 {

@@ -145,6 +147,13 @@ static inline int rc80211_minstrel_init(
 static inline void rc80211_minstrel_exit(void)
 {
 }
+static inline int rc80211_minstrel_ht_init(void)
+{
+	return 0;
+}
+static inline void rc80211_minstrel_ht_exit(void)
+{
+}
 #endif

net/mac80211/rc80211_minstrel_ht.c new file +800 -0

800 lines not on this page · raw message

net/mac80211/rc80211_minstrel_ht.h new file +120 -0

120 lines not on this page · raw message

net/mac80211/rc80211_minstrel_ht_debugfs.c new file +120 -0

120 lines not on this page · raw message

Christian Lamparter

2010-03-07 17:59 UTC #1 lore raw

Quoted 10 lines · On Sunday 07 March 2010 17:39:47 Felix Fietkau wrote:

here is an updated version of my minstrel_ht code. Since the last version, I've added the following improvements:
- Use accurate A-MPDU statistics from tx feedback
- Rewrite the sampling algorithm to optimize for good A-MPDU lengths and faster throughput recovery
- Implement .rate_update to handle changes in HT capabilities or channel bandwidth
- Remove the private tx flags hack

Quoted 3 lines

With the new version, performance has improved visibly for HT40 in all of my tests. I think it's getting closer to being ready for inclusion.
---

Just released carl9170 1.0.3.1 which now uses minstrel_ht.

I found a few things, not sure if they are bugs in the driver or minstrel_ht code. (Patch attached, but added the comments as a response to the code)

Quoted 25 lines

net/mac80211/rc80211_minstrel_ht.c new file +20 -0

@@ -0,0 +1,800 @@
[...]
+
+static void
+minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
+                      struct ieee80211_sta *sta, void *priv_sta,
+                      struct sk_buff *skb)
+{
[...]
+
+	for (i = 0; !last; i++) {
+		last = (i == IEEE80211_TX_MAX_RATES - 1) ||
+		       !minstrel_ht_txstat_valid(&ar[i + 1]);
+
+		if (!minstrel_ht_txstat_valid(&ar[i]))
+			break;
+
+		group = minstrel_ht_get_group_idx(&ar[i]);
+		rate = &mi->groups[group].rates[ar[i].idx % 8];
+
+		if (last && (info->flags & IEEE80211_TX_STAT_ACK))
+			rate->success += info->status.ampdu_ack_len;
+

net/mac80211/rc80211_minstrel_ht.c

Quoted 10 lines

net/mac80211/rc80211_minstrel_ht.c new file +10 -0

+		rate->attempts += ar[i].count * info->status.ampdu_len;
+	}
+
+	/*
+	 * check for sudden death of spatial multiplexing,
+	 * downgrade to a lower number of streams if necessary.
+	 */
+	rate = minstrel_get_ratestats(mi, mi->max_tp_rate);
+	if (MINSTREL_FRAC(rate->success, rate->attempts) <
+	    MINSTREL_FRAC(20, 100) && rate->attempts > 30)

BUGGED right here. Due to rate->attempts being 0. so, what about this instead and move the attemps > 30:

	if (rate->attempts > 30 &&
	    MINSTREL_FRAC(rate->success, rate->attempts) <
	    MINSTREL_FRAC(20, 100))

net/mac80211/rc80211_minstrel_ht.c

Quoted 5 lines

net/mac80211/rc80211_minstrel_ht.c new file +5 -0

+		minstrel_downgrade_rate(mi, &mi->max_tp_rate, true);
+
+	rate2 = minstrel_get_ratestats(mi, mi->max_tp_rate2);
+	if (MINSTREL_FRAC(rate->success, rate->attempts) <
+	    MINSTREL_FRAC(20, 100) && rate->attempts > 30)

(same as above, put "rate->attempts > 30" in front of MINSTREL_FRAC

Quoted 9 lines

[...]

net/mac80211/rc80211_minstrel_ht.c new file +8 -0

+static void
+minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
+                     struct ieee80211_tx_rate_control *txrc)
+{
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(txrc->skb);
+	struct ieee80211_tx_rate *ar = info->status.rates;
+	struct minstrel_ht_sta_priv *msp = priv_sta;
+	struct minstrel_ht_sta *mi = &msp->ht;

net/mac80211/rc80211_minstrel_ht.c

Quoted 10 lines

net/mac80211/rc80211_minstrel_ht.c new file +10 -0

+	struct minstrel_priv *mp = priv;
+	int sample_idx;
+
+	if (rate_control_send_low(sta, priv_sta, txrc))
+		return;
+
+	if (!msp->is_ht)
+		return mac80211_minstrel.get_rate(priv, sta, &msp->legacy, txrc);
+
+	minstrel_aggr_check(mp, sta, txrc->skb);

on startup, carl9170 complains about non-aggregated (CTL_AMPDU not set) MCS frames. I had to add an alternative path for these _early_ frames. They are now send with the "lowest" legacy rate and aren't rejected by the driver/HW.

Quoted 35 lines

net/mac80211/rc80211_minstrel_ht.c new file +28 -0

+
+	sample_idx = minstrel_get_sample_rate(mp, mi);
+	if (sample_idx >= 0) {
+		minstrel_ht_set_rate(mp, mi, &ar[0], sample_idx,
+			txrc, true, false);
+		minstrel_ht_set_rate(mp, mi, &ar[1], mi->max_tp_rate,
+			txrc, false, true);
+		info->flags |= IEEE80211_TX_CTL_RATE_CTRL_PROBE;
+	} else {
+		minstrel_ht_set_rate(mp, mi, &ar[0], mi->max_tp_rate,
+			txrc, false, false);
+		minstrel_ht_set_rate(mp, mi, &ar[1], mi->max_tp_rate2,
+			txrc, false, true);
+	}
+	minstrel_ht_set_rate(mp, mi, &ar[2], mi->max_prob_rate, txrc, false, true);
+
+	ar[3].count = 0;
+	ar[3].idx = -1;
+
+	mi->total_packets++;
+
+	/* wraparound */
+	if (mi->total_packets == ~0) {
+		mi->total_packets = 0;
+		mi->sample_packets = 0;
+	}
+}
+

net/mac80211/rc80211_minstrel_ht_debugfs.c new file +2 -0

@@ -0,0 +1,120 @@
[...]
+static int
+minstrel_ht_stats_open(struct inode *inode, struct file *file)

net/mac80211/rc80211_minstrel_ht_debugfs.c

Quoted 10 lines

net/mac80211/rc80211_minstrel_ht_debugfs.c new file +9 -0

+{
[...]
+
+		for (j = 0; j < MCS_GROUP_RATES; j++) {
+			struct minstrel_rate_stats *mr = &mi->groups[i].rates[j];
+			int idx = i * MCS_GROUP_RATES + j;
+
+			if (!mi->groups[i].supported & BIT(j))
+				continue;
+

gcc complains about !a & b.

net/mac80211/rc80211_minstrel_ht_debugfs.c

Quoted 2 lines

net/mac80211/rc80211_minstrel_ht_debugfs.c new file +2 -0
+			p += sprintf(p, "HT%c0/%cGI ", htmode, gimode);
+

[...]
---
From: Christian Lamparter <[email protected]>
Subject: mac80211: carl9170 specific minstrel_ht fixes

net/mac80211/rc80211_minstrel_ht.c +13 -4

index 8b60bd8..ef98ccf 100644
@@ -407,13 +407,15 @@ minstrel_ht_tx_status(void *priv, struct ieee80211_supported_band *sband,
 	 * downgrade to a lower number of streams if necessary.
 	 */
 	rate = minstrel_get_ratestats(mi, mi->max_tp_rate);
-	if (MINSTREL_FRAC(rate->success, rate->attempts) <
-	    MINSTREL_FRAC(20, 100) && rate->attempts > 30)
+	if (rate->attempts > 30 &&
+	    MINSTREL_FRAC(rate->success, rate->attempts) <
+	    MINSTREL_FRAC(20, 100))
 		minstrel_downgrade_rate(mi, &mi->max_tp_rate, true);
 
 	rate2 = minstrel_get_ratestats(mi, mi->max_tp_rate2);
-	if (MINSTREL_FRAC(rate->success, rate->attempts) <
-	    MINSTREL_FRAC(20, 100) && rate->attempts > 30)
+	if (rate->attempts > 30 &&
+	    MINSTREL_FRAC(rate->success, rate->attempts) <
+	    MINSTREL_FRAC(20, 100))
 		minstrel_downgrade_rate(mi, &mi->max_tp_rate2, false);
 
 	if (time_after(jiffies, mi->stats_update + (mp->update_interval / 2 * HZ) / 1000))

@@ -574,6 +576,12 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 
 	minstrel_aggr_check(mp, sta, txrc->skb);
 
+	if (!(info->flags & IEEE80211_TX_CTL_AMPDU)) {
+		ar[0].count = mp->hw->max_rate_tries;
+		ar[0].idx = rate_lowest_index(txrc->sband, sta);
+		goto rate_out;
+	}
+
 	sample_idx = minstrel_get_sample_rate(mp, mi);
 	if (sample_idx >= 0) {
 		minstrel_ht_set_rate(mp, mi, &ar[0], sample_idx,

@@ -589,6 +597,7 @@ minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	}
 	minstrel_ht_set_rate(mp, mi, &ar[2], mi->max_prob_rate, txrc, false, true);
 
+rate_out:
 	ar[3].count = 0;
 	ar[3].idx = -1;

net/mac80211/rc80211_minstrel_ht_debugfs.c +1 -1

index 4689aac..4fb3ccb 100644
@@ -57,7 +57,7 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
 			struct minstrel_rate_stats *mr = &mi->groups[i].rates[j];
 			int idx = i * MCS_GROUP_RATES + j;
 
-			if (!mi->groups[i].supported & BIT(j))
+			if (!(mi->groups[i].supported & BIT(j)))
 				continue;
 
 			p += sprintf(p, "HT%c0/%cGI ", htmode, gimode);

---

Regards,
Chr

Felix Fietkau

author

2010-03-07 18:31 UTC #2 lore raw

Quoted 11 lines · Christian Lamparter

Quoted 10 lines · On Sunday 07 March 2010 17:39:47 Felix Fietkau wrote:
here is an updated version of my minstrel_ht code. Since the last version, I've added the following improvements:
- Use accurate A-MPDU statistics from tx feedback
- Rewrite the sampling algorithm to optimize for good A-MPDU lengths and faster throughput recovery
- Implement .rate_update to handle changes in HT capabilities or channel bandwidth
- Remove the private tx flags hack

Quoted 8 lines

Quoted 3 lines
With the new version, performance has improved visibly for HT40 in all of my tests. I think it's getting closer to being ready for inclusion.
---
Just released carl9170 1.0.3.1 which now uses minstrel_ht.
I found a few things, not sure if they are bugs in the driver or minstrel_ht code. (Patch attached, but added the comments as a response to the code)

Thanks. How well does it perform in your tests compared to the old rc?

Quoted 5 lines

on startup, carl9170 complains about non-aggregated (CTL_AMPDU not set) MCS frames. I had to add an alternative path for these _early_ frames. They are now send with the "lowest" legacy rate and aren't rejected by the driver/HW.

Why does it complain - is that a driver thing or does the hardware really refuse it? I didn't see anything in the standard that mandates the use of A-MPDU with MCS rates.

- Felix

Christian Lamparter

2010-03-08 14:31 UTC #3 lore raw

Quoted 15 lines · On Sunday 07 March 2010 19:31:37 Felix Fietkau wrote:

Quoted 14 lines · Christian Lamparter
Quoted 13 lines · On Sunday 07 March 2010 17:39:47 Felix Fietkau wrote:
here is an updated version of my minstrel_ht code. Since the last version, I've added the following improvements:
- Use accurate A-MPDU statistics from tx feedback
- Rewrite the sampling algorithm to optimize for good A-MPDU lengths and faster throughput recovery
- Implement .rate_update to handle changes in HT capabilities or channel bandwidth
- Remove the private tx flags hack
With the new version, performance has improved visibly for HT40 in all of my tests. I think it's getting closer to being ready for inclusion.
---

Quoted 6 lines

Quoted 5 lines
Just released carl9170 1.0.3.1 which now uses minstrel_ht.
I found a few things, not sure if they are bugs in the driver or minstrel_ht code. (Patch attached, but added the comments as a response to the code)
Thanks. How well does it perform in your tests compared to the old rc?

Hard to see any big difference under real life conditions. But there are some "gains" with _synthetic_ benchmarks.

most notably: uni-directional UDP traffic now suffers much less from package loss (old rc: ~ 3 - 7%, compared to minstrel: 0.0001% - 0.01%). The throughput is not affected, It levels out at around ~130Mbits/s in both tests.

Quoted 8 lines

Quoted 5 lines
on startup, carl9170 complains about non-aggregated (CTL_AMPDU not set) MCS frames. I had to add an alternative path for these _early_ frames. They are now send with the "lowest" legacy rate and aren't rejected by the driver/HW.
Why does it complain - is that a driver thing or does the hardware really refuse it? I didn't see anything in the standard that mandates the use of A-MPDU with MCS rates.

WARN_ON in the driver. But yeah, on a second look:
It's just some old code from my own minstrel_ht (back in October 2009) On a third look: The original vendor driver logic almost _forces_ the A-MPDU-bit upon HT frames. I can't tell for sure, if the "other" mode actually works or just served as a testbed for development?! So let's try: carl9170 1.0.3.2

Regards,
Chr