Patchwork ECN + pfifo_fast borked? (Was Re: [Bloat] shaper team forming up)

login
register
mail settings
Submitter Eric Dumazet
Date March 15, 2011, 6:28 p.m.
Message ID <1300213714.2927.37.camel@edumazet-laptop>
Download mbox | patch
Permalink /patch/87026/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric Dumazet - March 15, 2011, 6:28 p.m.
Le mardi 15 mars 2011 à 19:09 +0200, Jonathan Morton a écrit :
> On 15 Mar, 2011, at 8:15 am, Eric Dumazet wrote:
> 
> > band 0 : high priority packets (like now)
> > band 1 : (old band 1, ECN capable flows)
> > band 2 : (old band 1, no ECN flows)
> > band 3 : low priority packets (old band 2)
> 
> This seems good to me.  It would provide a concrete (if minor) enticement to turn ECN on.
> 
>  

Here is a patch to implement that, on top of net-next-2.6 git tree

qdisc pfifo_fast 0: dev eth1 root refcnt 2 bands 4 priomap  2 1 3 3 2 3 0 0 2 2 2 2 2 2 2 2
 Sent 168 bytes 2 pkt (dropped 0, overlimits 0 requeues 0) 
 backlog 0b 0p requeues 0 




--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jonathan Morton - March 15, 2011, 6:37 p.m.
On 15 Mar, 2011, at 8:28 pm, Eric Dumazet wrote:

>>> band 0 : high priority packets (like now)
>>> band 1 : (old band 1, ECN capable flows)
>>> band 2 : (old band 1, no ECN flows)
>>> band 3 : low priority packets (old band 2)
>> 
>> This seems good to me.  It would provide a concrete (if minor) enticement to turn ECN on.
> 
> Here is a patch to implement that, on top of net-next-2.6 git tree

Does this take both ECN bits into account?  The ECT(0), ECT(1) and ECE codepoints all need to be recognised equally.

 - Jonathan

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Dumazet - March 15, 2011, 7:56 p.m.
Le mardi 15 mars 2011 à 20:37 +0200, Jonathan Morton a écrit :
> On 15 Mar, 2011, at 8:28 pm, Eric Dumazet wrote:
> 
> >>> band 0 : high priority packets (like now)
> >>> band 1 : (old band 1, ECN capable flows)
> >>> band 2 : (old band 1, no ECN flows)
> >>> band 3 : low priority packets (old band 2)
> >> 
> >> This seems good to me.  It would provide a concrete (if minor) enticement to turn ECN on.
> > 
> > Here is a patch to implement that, on top of net-next-2.6 git tree
> 
> Does this take both ECN bits into account?  The ECT(0), ECT(1) and ECE codepoints all need to be recognised equally.

This is done in a different layer, as already explained.

Current linux code ignores low order bit when doing TOS -> skb->priority
mapping.



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

Patch

diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index c84b659..95ddf54 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -412,19 +412,39 @@  static struct Qdisc noqueue_qdisc = {
 };
 
 
-static const u8 prio2band[TC_PRIO_MAX + 1] = {
-	1, 2, 2, 2, 1, 2, 0, 0 , 1, 1, 1, 1, 1, 1, 1, 1
+/* 4-band FIFO queue: old style, but should be a bit faster than
+   generic prio+fifo combination.
+ */
+
+enum {
+	BAND_HIGH_PRIO,
+	BAND_MEDIUM_ECN_PRIO,
+	BAND_MEDIUM_PRIO,
+	BAND_LOW_PRIO,
+
+	PFIFO_FAST_BANDS
 };
 
-/* 3-band FIFO queue: old style, but should be a bit faster than
-   generic prio+fifo combination.
+/*
+ * We give a litle incent to ECN flows
  */
+static const u8 prio2band[TC_PRIO_MAX + 1] = {
+	[TC_PRIO_BESTEFFORT]	= BAND_MEDIUM_PRIO,
+	[TC_PRIO_FILLER]	= BAND_MEDIUM_ECN_PRIO,
+	[TC_PRIO_BULK]		= BAND_LOW_PRIO,
+	[TC_PRIO_BULK + 1]	= BAND_LOW_PRIO,
+	[TC_PRIO_INTERACTIVE_BULK] = BAND_MEDIUM_PRIO,
+	[TC_PRIO_INTERACTIVE_BULK + 1] = BAND_LOW_PRIO,
+	[TC_PRIO_INTERACTIVE]	= BAND_HIGH_PRIO,
+	[TC_PRIO_CONTROL]	= BAND_HIGH_PRIO,
+
+	[TC_PRIO_CONTROL+1 ... TC_PRIO_MAX] = BAND_MEDIUM_PRIO,
+};
 
-#define PFIFO_FAST_BANDS 3
 
 /*
  * Private data for a pfifo_fast scheduler containing:
- * 	- queues for the three band
+ * 	- queues for the four bands
  * 	- bitmap indicating which of the bands contain skbs
  */
 struct pfifo_fast_priv {
@@ -436,9 +456,13 @@  struct pfifo_fast_priv {
  * Convert a bitmap to the first band number where an skb is queued, where:
  * 	bitmap=0 means there are no skbs on any band.
  * 	bitmap=1 means there is an skb on band 0.
- *	bitmap=7 means there are skbs on all 3 bands, etc.
+ * 	bitmap=2 means there is an skb on band 1.
+ *	bitmap=15 means there are skbs on all 4 bands.
  */
-static const int bitmap2band[] = {-1, 0, 1, 0, 2, 0, 1, 0};
+static const int bitmap2band[1 << PFIFO_FAST_BANDS] = {
+	-1, 0, 1, 0, 2, 0, 1, 0,
+	 3, 0, 1, 0, 2, 0, 1, 0
+};
 
 static inline struct sk_buff_head *band2list(struct pfifo_fast_priv *priv,
 					     int band)