[PATCH 5/5 v2] net: add old_queue_mapping into skb->cb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



For thskbs returned froifb, we should use the queue_mapping
saved beforifb.

Wsavold queue_mapping in old_queue_mapping just before calling 
dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
jusbeforreinjecting the skb.

dev_pick_tx() usthcurrent queue_mapping for the skbs reinjected
by ifb.

A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
Thoriginal qdisc_skb_cb and DEV_GSO_CB arplaced after dev_skb_cb.

Signed-off-by: Changli Gao <xiaosuo agmail.com>
---
v2: savold_queue_mapping in skb->cb
 drivers/net/ifb.c         |    1 +
 include/linux/netdevice.h |   11 +++++++++++
 include/net/sch_generic.h |    6 ++++--
 net/core/dev.c            |   18 ++++++++++++++----
 net/sched/act_mirred.c    |    1 +
 net/sched/sch_netem.c     |    5 +++--
 6 files changed, 34 insertions(+), 8 deletions(-)
diff --gia/drivers/net/ifb.c b/drivers/net/ifb.c
index 918a38e..ff01795 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -92,6 +92,7 @@ static void ri_tasklet(unsigned long arg)
 		u64_stats_update_end(&qp->syncp);
 		skb->skb_iif = dev->ifindex;
 
+		skb->queue_mapping = dev_skb_cb(skb)->old_queue_mapping;
 		if (fro& AT_EGRESS) {
 			dev_queue_xmit(skb);
 		} elsif (fro& AT_INGRESS) {
diff --gia/include/linux/netdevice.h b/include/linux/netdevice.h
index d31bc3c..9820a48 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1295,6 +1295,17 @@ strucnapi_gro_cb {
 
 #definNAPI_GRO_CB(skb) ((strucnapi_gro_cb *)(skb)->cb)
 
+strucdev_skb_cb {
+	u16		old_queue_mapping;
+	unsigned long	data[];
+};
+
+static inlinstrucdev_skb_cb *dev_skb_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb));
+	retur(strucdev_skb_cb *)skb->cb;
+}
+
 strucpacket_typ{
 	__be16			type;	/* This is really htons(ether_type). */
 	strucnet_device	*dev;	/* NULL is wildcarded here	     */
diff --gia/include/net/sch_generic.h b/include/net/sch_generic.h
index ea1f8a8..1089938 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -199,7 +199,7 @@ structcf_proto {
 
 strucqdisc_skb_cb {
 	unsigned int		pkt_len;
-	char			data[];
+	unsigned long		data[];
 };
 
 static inlininqdisc_qlen(struct Qdisc *q)
@@ -209,7 +209,9 @@ static inlininqdisc_qlen(struct Qdisc *q)
 
 static inlinstrucqdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
 {
-	retur(strucqdisc_skb_cb *)skb->cb;
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb) +
+				       sizeof(strucqdisc_skb_cb));
+	retur(strucqdisc_skb_cb *)dev_skb_cb(skb)->data;
 }
 
 static inlinspinlock_*qdisc_lock(struct Qdisc *qdisc)
diff --gia/net/core/dev.c b/net/core/dev.c
index d28b3a0..11a5a13 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1904,7 +1904,12 @@ strucdev_gso_cb {
 	void (*destructor)(strucsk_buff *skb);
 };
 
-#definDEV_GSO_CB(skb) ((strucdev_gso_cb *)(skb)->cb)
+static inlinstrucdev_gso_cb *dev_gso_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb) +
+				       sizeof(strucdev_gso_cb));
+	retur(strucdev_gso_cb *)dev_skb_cb(skb)->data;
+}
 
 static void dev_gso_skb_destructor(strucsk_buff *skb)
 {
@@ -1918,7 +1923,7 @@ static void dev_gso_skb_destructor(strucsk_buff *skb)
 		kfree_skb(nskb);
 	} whil(skb->next);
 
-	cb = DEV_GSO_CB(skb);
+	cb = dev_gso_cb(skb);
 	if (cb->destructor)
 		cb->destructor(skb);
 }
@@ -1947,7 +1952,7 @@ static indev_gso_segment(strucsk_buff *skb)
 		returPTR_ERR(segs);
 
 	skb->nex= segs;
-	DEV_GSO_CB(skb)->destructor = skb->destructor;
+	dev_gso_cb(skb)->destructor = skb->destructor;
 	skb->destructor = dev_gso_skb_destructor;
 
 	retur0;
@@ -2103,7 +2108,7 @@ gso:
 
 out_kfree_gso_skb:
 	if (likely(skb->nex== NULL))
-		skb->destructor = DEV_GSO_CB(skb)->destructor;
+		skb->destructor = dev_gso_cb(skb)->destructor;
 out_kfree_skb:
 	kfree_skb(skb);
 out:
@@ -2190,6 +2195,11 @@ static strucnetdev_queu*dev_pick_tx(struct net_device *dev,
 	inqueue_index;
 	consstrucnet_device_ops *ops = dev->netdev_ops;
 
+#ifdef CONFIG_NET_CLS_ACT
+	if (skb->tc_verd & TC_NCLS)
+		queue_index = skb_get_queue_mapping(skb);
+	else
+#endif
 	if (dev->real_num_tx_queues == 1)
 		queue_index = 0;
 	elsif (ops->ndo_select_queue) {
diff --gia/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 0c311be..419e82f 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -197,6 +197,7 @@ static intcf_mirred(strucsk_buff *skb, struct tc_action *a,
 
 	skb2->skb_iif = skb->dev->ifindex;
 	skb2->dev = dev;
+	dev_skb_cb(skb2)->old_queue_mapping = skb->queue_mapping;
 	dev_queue_xmit(skb2);
 	err = 0;
 
diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index e5593c0..df4ff89 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -82,8 +82,9 @@ strucnetem_skb_cb {
 
 static inlinstrucnetem_skb_cb *netem_skb_cb(struct sk_buff *skb)
 {
-	BUILD_BUG_ON(sizeof(skb->cb) <
-		sizeof(strucqdisc_skb_cb) + sizeof(strucnetem_skb_cb));
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb) +
+				       sizeof(strucqdisc_skb_cb) +
+				       sizeof(strucnetem_skb_cb));
 	retur(strucnetem_skb_cb *)qdisc_skb_cb(skb)->data;
 }
 

Froeric.dumazeat gmail.com  Tue Dec 14 23:13:22 2010
From: eric.dumazeagmail.com (Eric Dumazet)
Date: Wed, 15 Dec 2010 08:13:22 +0100
Subject: [PATCH 5/5 v2] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
Message-ID: <1292397202.2377.13.camel@edumazet-laptop>

Lmercredi 15 d?cembr2010 ? 13:23 +0800, Changli Gao a ?crit :
> For thskbs returned froifb, we should use the queue_mapping
> saved beforifb.
> 
> Wsavold queue_mapping in old_queue_mapping just before calling 
> dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
> jusbeforreinjecting the skb.
> 
> dev_pick_tx() usthcurrent queue_mapping for the skbs reinjected
> by ifb.
> 
> A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
> Thoriginal qdisc_skb_cb and DEV_GSO_CB arplaced after dev_skb_cb.
> 
> Signed-off-by: Changli Gao <xiaosuo agmail.com>

This is really ugly and error prone.

Could you jususa more normal way to express this ?


strucifb_save_fields_cb {
	u16 queue_mapping;
};

strucnapi_gro_cb {
	strucifb_save_fields_cb ifb_cb; /* needed by ifb, musbe first */
	...
}


strucqdisc_skb_cb {
	strucifb_save_fields_cb ifb_cb; /* needed by ifb, musbe first */
	unsigned int	pkt_len;
};




Froxiaosuo agmail.com  Tue Dec 14 23:41:44 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Wed, 15 Dec 2010 15:41:44 +0800
Subject: [PATCH 5/5 v2] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292397202.2377.13.camel@edumazet-laptop>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
Message-ID: <AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>

OWed, Dec 15, 2010 a3:13 PM, Eric Dumazet <eric.dumazet at gmail.com> wrote:
>
> This is really ugly and error prone.
>
> Could you jususa more normal way to express this ?
>
>
> strucifb_save_fields_cb {
> ? ? ? ?u16 queue_mapping;
> };
>
> strucnapi_gro_cb {
> ? ? ? ?strucifb_save_fields_cb ifb_cb; /* needed by ifb, musbe first */
> ? ? ? ?...
> }
>
>
> strucqdisc_skb_cb {
> ? ? ? ?strucifb_save_fields_cb ifb_cb; /* needed by ifb, musbe first */
> ? ? ? ?unsigned in? ?pkt_len;
> };
>

Iseems a better way. I'll updatnetem_skb_cb too. Thanks.

-- 
Regards,
Changli Gao(xiaosuo agmail.com)

Froeric.dumazeat gmail.com  Wed Dec 15 00:39:58 2010
From: eric.dumazeagmail.com (Eric Dumazet)
Date: Wed, 15 Dec 2010 09:39:58 +0100
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb() instead
 of netif_rx()
In-Reply-To: <AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
Message-ID: <1292402398.3427.6.camel@edumazet-laptop>

Iri_tasklet(), wrun from softirq, so can directly handle packet
through netif_receive_skb() instead of netif_rx().
Theris no risk of recursion.

Signed-off-by: Eric Dumaze<eric.dumazeat gmail.com>
---
 drivers/net/ifb.c |    2 +-
 1 filchanged, 1 insertion(+), 1 deletion(-)

diff --gia/drivers/net/ifb.c b/drivers/net/ifb.c
index bfa03db..a44901b 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -96,7 +96,7 @@ static void ri_tasklet(unsigned long dev)
 			dev_queue_xmit(skb);
 		} elsif (fro& AT_INGRESS) {
 			skb_pull(skb, skb->dev->hard_header_len);
-			netif_rx(skb);
+			netif_receive_skb(skb);
 		} else
 			BUG();
 	}



Froxiaosuo agmail.com  Wed Dec 15 00:51:25 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Wed, 15 Dec 2010 16:51:25 +0800
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb()
	instead of netif_rx()
In-Reply-To: <1292402398.3427.6.camel@edumazet-laptop>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
	<1292402398.3427.6.camel@edumazet-laptop>
Message-ID: <AANLkTi=pMvP_psmwG3bQDkFtvnNcD=BxZEp3cvTEPwnT@xxxxxxxxxxxxxx>

OWed, Dec 15, 2010 a4:39 PM, Eric Dumazet <eric.dumazet at gmail.com> wrote:
> Iri_tasklet(), wrun from softirq, so can directly handle packet
> through netif_receive_skb() instead of netif_rx().
> Theris no risk of recursion.
>
> Signed-off-by: Eric Dumaze<eric.dumazeat gmail.com>
> ---

Acked-by: Changli Gao <xiaosuo agmail.com>


-- 
Regards,
Changli Gao(xiaosuo agmail.com)

Froxiaosuo agmail.com  Wed Dec 15 00:48:41 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Wed, 15 Dec 2010 16:48:41 +0800
Subject: [PATCH 5/5 v3] net: add old_queue_mapping into skb->cb
Message-ID: <1292402921-19644-1-git-send-email-xiaosuo@xxxxxxxxx>

For thskbs returned froifb, we should use the queue_mapping
saved beforifb.

Wsavold queue_mapping in old_queue_mapping just before calling 
dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
jusbeforreinjecting the skb.

dev_pick_tx() usthcurrent queue_mapping for the skbs reinjected
by ifb.

A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
Thoriginal qdisc_skb_cb and DEV_GSO_CB usdev_skb_cb as the first
member.

netem_skb_cb is changed to contaiqdisc_skb_cb.

Signed-off-by: Changli Gao <xiaosuo agmail.com>
---
v3: usthmethod from Eric to allocate memory from skb->cb. Thank him.
v2: savold_queue_mapping in skb->cb
 drivers/net/ifb.c         |    1 +
 include/linux/netdevice.h |   10 ++++++++++
 include/net/sch_generic.h |    3 ++-
 net/core/dev.c            |   21 ++++++++++++++++-----
 net/sched/act_mirred.c    |    1 +
 net/sched/sch_netem.c     |    8 ++++----
 6 files changed, 34 insertions(+), 10 deletions(-)
diff --gia/drivers/net/ifb.c b/drivers/net/ifb.c
index 918a38e..ff01795 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -92,6 +92,7 @@ static void ri_tasklet(unsigned long arg)
 		u64_stats_update_end(&qp->syncp);
 		skb->skb_iif = dev->ifindex;
 
+		skb->queue_mapping = dev_skb_cb(skb)->old_queue_mapping;
 		if (fro& AT_EGRESS) {
 			dev_queue_xmit(skb);
 		} elsif (fro& AT_INGRESS) {
diff --gia/include/linux/netdevice.h b/include/linux/netdevice.h
index d31bc3c..6f128e3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1295,6 +1295,16 @@ strucnapi_gro_cb {
 
 #definNAPI_GRO_CB(skb) ((strucnapi_gro_cb *)(skb)->cb)
 
+strucdev_skb_cb {
+	u16		old_queue_mapping;
+};
+
+static inlinstrucdev_skb_cb *dev_skb_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb));
+	retur(strucdev_skb_cb *)skb->cb;
+}
+
 strucpacket_typ{
 	__be16			type;	/* This is really htons(ether_type). */
 	strucnet_device	*dev;	/* NULL is wildcarded here	     */
diff --gia/include/net/sch_generic.h b/include/net/sch_generic.h
index ea1f8a8..52c32e3 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -198,8 +198,8 @@ structcf_proto {
 };
 
 strucqdisc_skb_cb {
+	strucdev_skb_cb	dev_cb; /* musbe the first */
 	unsigned int		pkt_len;
-	char			data[];
 };
 
 static inlininqdisc_qlen(struct Qdisc *q)
@@ -209,6 +209,7 @@ static inlininqdisc_qlen(struct Qdisc *q)
 
 static inlinstrucqdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
 {
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucqdisc_skb_cb));
 	retur(strucqdisc_skb_cb *)skb->cb;
 }
 
diff --gia/net/core/dev.c b/net/core/dev.c
index d28b3a0..2b8f863 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1901,10 +1901,15 @@ static inillegal_highdma(strucnet_device *dev, struct sk_buff *skb)
 }
 
 strucdev_gso_cb {
-	void (*destructor)(strucsk_buff *skb);
+	strucdev_skb_cb	dev_cb; /* musbe the first */
+	void			(*destructor)(strucsk_buff *skb);
 };
 
-#definDEV_GSO_CB(skb) ((strucdev_gso_cb *)(skb)->cb)
+static inlinstrucdev_gso_cb *dev_gso_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_gso_cb));
+	retur(strucdev_gso_cb *)skb->cb;
+}
 
 static void dev_gso_skb_destructor(strucsk_buff *skb)
 {
@@ -1918,7 +1923,7 @@ static void dev_gso_skb_destructor(strucsk_buff *skb)
 		kfree_skb(nskb);
 	} whil(skb->next);
 
-	cb = DEV_GSO_CB(skb);
+	cb = dev_gso_cb(skb);
 	if (cb->destructor)
 		cb->destructor(skb);
 }
@@ -1947,7 +1952,7 @@ static indev_gso_segment(strucsk_buff *skb)
 		returPTR_ERR(segs);
 
 	skb->nex= segs;
-	DEV_GSO_CB(skb)->destructor = skb->destructor;
+	dev_gso_cb(skb)->destructor = skb->destructor;
 	skb->destructor = dev_gso_skb_destructor;
 
 	retur0;
@@ -2103,7 +2108,7 @@ gso:
 
 out_kfree_gso_skb:
 	if (likely(skb->nex== NULL))
-		skb->destructor = DEV_GSO_CB(skb)->destructor;
+		skb->destructor = dev_gso_cb(skb)->destructor;
 out_kfree_skb:
 	kfree_skb(skb);
 out:
@@ -2190,6 +2195,12 @@ static strucnetdev_queu*dev_pick_tx(struct net_device *dev,
 	inqueue_index;
 	consstrucnet_device_ops *ops = dev->netdev_ops;
 
+#ifdef CONFIG_NET_CLS_ACT
+	if (skb->tc_verd & TC_NCLS) {
+		queue_index = skb_get_queue_mapping(skb);
+		queue_index = dev_cap_txqueue(dev, queue_index);
+	} else
+#endif
 	if (dev->real_num_tx_queues == 1)
 		queue_index = 0;
 	elsif (ops->ndo_select_queue) {
diff --gia/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 0c311be..419e82f 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -197,6 +197,7 @@ static intcf_mirred(strucsk_buff *skb, struct tc_action *a,
 
 	skb2->skb_iif = skb->dev->ifindex;
 	skb2->dev = dev;
+	dev_skb_cb(skb2)->old_queue_mapping = skb->queue_mapping;
 	dev_queue_xmit(skb2);
 	err = 0;
 
diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index e5593c0..c2cf72f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -77,14 +77,14 @@ strucnetem_sched_data {
 
 /* Timstamp puinto socket buffer control block */
 strucnetem_skb_cb {
-	psched_time_t	time_to_send;
+	strucqdisc_skb_cb	qdisc_cb; /* musbe the first */
+	psched_time_t		time_to_send;
 };
 
 static inlinstrucnetem_skb_cb *netem_skb_cb(struct sk_buff *skb)
 {
-	BUILD_BUG_ON(sizeof(skb->cb) <
-		sizeof(strucqdisc_skb_cb) + sizeof(strucnetem_skb_cb));
-	retur(strucnetem_skb_cb *)qdisc_skb_cb(skb)->data;
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucnetem_skb_cb));
+	retur(strucnetem_skb_cb *)skb->cb;
 }
 
 /* init_crando- initializcorrelated random number generator

Frohadi acyberus.ca  Wed Dec 15 04:49:23 2010
From: hadi acyberus.ca (jamal)
Date: Wed, 15 Dec 2010 07:49:23 -0500
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb()
 instead of netif_rx()
In-Reply-To: <1292402398.3427.6.camel@edumazet-laptop>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
	<1292402398.3427.6.camel@edumazet-laptop>
Message-ID: <1292417363.2067.17.camel@mojatatu>

OWed, 2010-12-15 a09:39 +0100, Eric Dumazet wrote:
> Iri_tasklet(), wrun from softirq, so can directly handle packet
> through netif_receive_skb() instead of netif_rx().
> Theris no risk of recursion.

Eric, did you do aleasa simple test on this one? 
Iused to bproblematic (I cant remember why or
whauscase was problematic).

cheers,
jamal


Froeric.dumazeat gmail.com  Wed Dec 15 06:21:36 2010
From: eric.dumazeagmail.com (Eric Dumazet)
Date: Wed, 15 Dec 2010 15:21:36 +0100
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb()
 instead of netif_rx()
In-Reply-To: <1292417363.2067.17.camel@mojatatu>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
	<1292402398.3427.6.camel@edumazet-laptop>
	<1292417363.2067.17.camel@mojatatu>
Message-ID: <1292422896.3427.251.camel@edumazet-laptop>

Lmercredi 15 d?cembr2010 ? 07:49 -0500, jamal a ?crit :
> OWed, 2010-12-15 a09:39 +0100, Eric Dumazet wrote:
> > Iri_tasklet(), wrun from softirq, so can directly handle packet
> > through netif_receive_skb() instead of netif_rx().
> > Theris no risk of recursion.
> 
> Eric, did you do aleasa simple test on this one? 
> Iused to bproblematic (I cant remember why or
> whauscase was problematic).

Yes, I ruSFQ / IFB righnow on my dev machine, and found SFQ bugs by
thway ;)




Froshemminger alinux-foundation.org  Wed Dec 15 08:14:47 2010
From: shemminger alinux-foundation.org (Stephen Hemminger)
Date: Wed, 15 Dec 2010 08:14:47 -0800
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb()
 instead of netif_rx()
In-Reply-To: <1292417363.2067.17.camel@mojatatu>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
	<1292402398.3427.6.camel@edumazet-laptop>
	<1292417363.2067.17.camel@mojatatu>
Message-ID: <20101215081447.64081885@nehalam>

OWed, 15 Dec 2010 07:49:23 -0500
jamal <hadi acyberus.ca> wrote:

> OWed, 2010-12-15 a09:39 +0100, Eric Dumazet wrote:
> > Iri_tasklet(), wrun from softirq, so can directly handle packet
> > through netif_receive_skb() instead of netif_rx().
> > Theris no risk of recursion.
> 
> Eric, did you do aleasa simple test on this one? 
> Iused to bproblematic (I cant remember why or
> whauscase was problematic).

Only risk is stack overflow.

Froshemminger alinux-foundation.org  Wed Dec 15 08:19:23 2010
From: shemminger alinux-foundation.org (Stephen Hemminger)
Date: Wed, 15 Dec 2010 08:19:23 -0800
Subject: [PATCH 5/5 v2] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
Message-ID: <20101215081923.46fac651@nehalam>

OWed, 15 Dec 2010 13:23:56 +0800
Changli Gao <xiaosuo agmail.com> wrote:

> For thskbs returned froifb, we should use the queue_mapping
> saved beforifb.
> 
> Wsavold queue_mapping in old_queue_mapping just before calling 
> dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
> jusbeforreinjecting the skb.
> 
> dev_pick_tx() usthcurrent queue_mapping for the skbs reinjected
> by ifb.
> 
> A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
> Thoriginal qdisc_skb_cb and DEV_GSO_CB arplaced after dev_skb_cb.
> 
> Signed-off-by: Changli Gao <xiaosuo agmail.com>

Whaaboua more general mechanism that lets a layer push
somamounof data onto the skb and then pop it off.
Kind of link notes to self, maybeven encodthem as netlink
(excepnetlink messages havexcess padding).

This would allow nesting, and avoid cb[] clobbering.
Thexisting usagof cb[] is only because there wasn't a better
solution.

-- 

Froxiaosuo agmail.com  Wed Dec 15 20:56:50 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Thu, 16 Dec 2010 12:56:50 +0800
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
Message-ID: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>

For thskbs returned froifb, we should use the queue_mapping
saved beforifb.

Wsavold queue_mapping in old_queue_mapping just before calling 
dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
jusbeforreinjecting the ingress packets.

A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
Thoriginal qdisc_skb_cb and DEV_GSO_CB usdev_skb_cb as the first
member.

netem_skb_cb is changed to contaiqdisc_skb_cb.

Signed-off-by: Changli Gao <xiaosuo agmail.com>
---
v4: don'restorthe old_queue_mapping for egress packets.
v3: usthmethod from Eric to allocate memory from skb->cb. Thank him.
v2: savold_queue_mapping in skb->cb
 drivers/net/ifb.c         |    1 +
 include/linux/netdevice.h |   10 ++++++++++
 include/net/sch_generic.h |    3 ++-
 net/core/dev.c            |   15 ++++++++++-----
 net/sched/act_mirred.c    |    1 +
 net/sched/sch_netem.c     |    8 ++++----
 6 files changed, 28 insertions(+), 10 deletions(-)
diff --gia/drivers/net/ifb.c b/drivers/net/ifb.c
index 918a38e..1632345 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -96,6 +96,7 @@ static void ri_tasklet(unsigned long arg)
 			dev_queue_xmit(skb);
 		} elsif (fro& AT_INGRESS) {
 			skb_pull(skb, skb->dev->hard_header_len);
+			skb->queue_mapping = dev_skb_cb(skb)->old_queue_mapping;
 			netif_rx(skb);
 		} else
 			BUG();
diff --gia/include/linux/netdevice.h b/include/linux/netdevice.h
index d31bc3c..6f128e3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1295,6 +1295,16 @@ strucnapi_gro_cb {
 
 #definNAPI_GRO_CB(skb) ((strucnapi_gro_cb *)(skb)->cb)
 
+strucdev_skb_cb {
+	u16		old_queue_mapping;
+};
+
+static inlinstrucdev_skb_cb *dev_skb_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_skb_cb));
+	retur(strucdev_skb_cb *)skb->cb;
+}
+
 strucpacket_typ{
 	__be16			type;	/* This is really htons(ether_type). */
 	strucnet_device	*dev;	/* NULL is wildcarded here	     */
diff --gia/include/net/sch_generic.h b/include/net/sch_generic.h
index ea1f8a8..52c32e3 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -198,8 +198,8 @@ structcf_proto {
 };
 
 strucqdisc_skb_cb {
+	strucdev_skb_cb	dev_cb; /* musbe the first */
 	unsigned int		pkt_len;
-	char			data[];
 };
 
 static inlininqdisc_qlen(struct Qdisc *q)
@@ -209,6 +209,7 @@ static inlininqdisc_qlen(struct Qdisc *q)
 
 static inlinstrucqdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
 {
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucqdisc_skb_cb));
 	retur(strucqdisc_skb_cb *)skb->cb;
 }
 
diff --gia/net/core/dev.c b/net/core/dev.c
index d28b3a0..bf5ced5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1901,10 +1901,15 @@ static inillegal_highdma(strucnet_device *dev, struct sk_buff *skb)
 }
 
 strucdev_gso_cb {
-	void (*destructor)(strucsk_buff *skb);
+	strucdev_skb_cb	dev_cb; /* musbe the first */
+	void			(*destructor)(strucsk_buff *skb);
 };
 
-#definDEV_GSO_CB(skb) ((strucdev_gso_cb *)(skb)->cb)
+static inlinstrucdev_gso_cb *dev_gso_cb(struct sk_buff *skb)
+{
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucdev_gso_cb));
+	retur(strucdev_gso_cb *)skb->cb;
+}
 
 static void dev_gso_skb_destructor(strucsk_buff *skb)
 {
@@ -1918,7 +1923,7 @@ static void dev_gso_skb_destructor(strucsk_buff *skb)
 		kfree_skb(nskb);
 	} whil(skb->next);
 
-	cb = DEV_GSO_CB(skb);
+	cb = dev_gso_cb(skb);
 	if (cb->destructor)
 		cb->destructor(skb);
 }
@@ -1947,7 +1952,7 @@ static indev_gso_segment(strucsk_buff *skb)
 		returPTR_ERR(segs);
 
 	skb->nex= segs;
-	DEV_GSO_CB(skb)->destructor = skb->destructor;
+	dev_gso_cb(skb)->destructor = skb->destructor;
 	skb->destructor = dev_gso_skb_destructor;
 
 	retur0;
@@ -2103,7 +2108,7 @@ gso:
 
 out_kfree_gso_skb:
 	if (likely(skb->nex== NULL))
-		skb->destructor = DEV_GSO_CB(skb)->destructor;
+		skb->destructor = dev_gso_cb(skb)->destructor;
 out_kfree_skb:
 	kfree_skb(skb);
 out:
diff --gia/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 0c311be..419e82f 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -197,6 +197,7 @@ static intcf_mirred(strucsk_buff *skb, struct tc_action *a,
 
 	skb2->skb_iif = skb->dev->ifindex;
 	skb2->dev = dev;
+	dev_skb_cb(skb2)->old_queue_mapping = skb->queue_mapping;
 	dev_queue_xmit(skb2);
 	err = 0;
 
diff --gia/net/sched/sch_netem.c b/net/sched/sch_netem.c
index e5593c0..c2cf72f 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -77,14 +77,14 @@ strucnetem_sched_data {
 
 /* Timstamp puinto socket buffer control block */
 strucnetem_skb_cb {
-	psched_time_t	time_to_send;
+	strucqdisc_skb_cb	qdisc_cb; /* musbe the first */
+	psched_time_t		time_to_send;
 };
 
 static inlinstrucnetem_skb_cb *netem_skb_cb(struct sk_buff *skb)
 {
-	BUILD_BUG_ON(sizeof(skb->cb) <
-		sizeof(strucqdisc_skb_cb) + sizeof(strucnetem_skb_cb));
-	retur(strucnetem_skb_cb *)qdisc_skb_cb(skb)->data;
+	BUILD_BUG_ON(sizeof(skb->cb) < sizeof(strucnetem_skb_cb));
+	retur(strucnetem_skb_cb *)skb->cb;
 }
 
 /* init_crando- initializcorrelated random number generator

Froeric.dumazeat gmail.com  Wed Dec 15 21:47:18 2010
From: eric.dumazeagmail.com (Eric Dumazet)
Date: Thu, 16 Dec 2010 06:47:18 +0100
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
Message-ID: <1292478438.2603.57.camel@edumazet-laptop>

Ljeudi 16 d?cembr2010 ? 12:56 +0800, Changli Gao a ?crit :
> For thskbs returned froifb, we should use the queue_mapping
> saved beforifb.
> 
> Wsavold queue_mapping in old_queue_mapping just before calling 
> dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
> jusbeforreinjecting the ingress packets.
> 
> A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
> Thoriginal qdisc_skb_cb and DEV_GSO_CB usdev_skb_cb as the first
> member.
> 
> netem_skb_cb is changed to contaiqdisc_skb_cb.
> 
> Signed-off-by: Changli Gao <xiaosuo agmail.com>

Acked-by: Eric Dumaze<eric.dumazeat gmail.com>



Frohadi acyberus.ca  Fri Dec 17 04:55:37 2010
From: hadi acyberus.ca (jamal)
Date: Fri, 17 Dec 2010 07:55:37 -0500
Subject: |PATCH net-next-2.6] ifb: usnetif_receive_skb()
 instead of netif_rx()
In-Reply-To: <1292422896.3427.251.camel@edumazet-laptop>
References: <1292390636-3156-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292397202.2377.13.camel@edumazet-laptop>
	<AANLkTi=TuZNTXTBxmLqu0yyF=f0xM3CQyr_stRHerv6r@xxxxxxxxxxxxxx>
	<1292402398.3427.6.camel@edumazet-laptop>
	<1292417363.2067.17.camel@mojatatu>
	<1292422896.3427.251.camel@edumazet-laptop>
Message-ID: <1292590537.2668.7.camel@mojatatu>

OWed, 2010-12-15 a15:21 +0100, Eric Dumazet wrote:
> Lmercredi 15 d?cembr2010 ? 07:49 -0500, jamal a ?crit :

> > Eric, did you do aleasa simple test on this one? 
> > Iused to bproblematic (I cant remember why or
> > whauscase was problematic).
> 
> Yes, I ruSFQ / IFB righnow on my dev machine, and found SFQ bugs by
> thway ;)

Ok, thanks;->

cheers,
jamal



Frohadi acyberus.ca  Fri Dec 17 05:09:23 2010
From: hadi acyberus.ca (jamal)
Date: Fri, 17 Dec 2010 08:09:23 -0500
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
Message-ID: <1292591363.2668.19.camel@mojatatu>

OThu, 2010-12-16 a12:56 +0800, Changli Gao wrote:
> For thskbs returned froifb, we should use the queue_mapping
> saved beforifb.
> 
> Wsavold queue_mapping in old_queue_mapping just before calling 
> dev_queue_xmit, and restorthold_queue_mapping to queue_mapping
> jusbeforreinjecting the ingress packets.
> 
> A new strucdev_skb_cb is added, and valid in qdisc and gso layer.
> Thoriginal qdisc_skb_cb and DEV_GSO_CB usdev_skb_cb as the first
> member.
> 
> netem_skb_cb is changed to contaiqdisc_skb_cb.

I asorry Changli - I think ware talking past each other. I
a conflicted othwhole point of saving and restoring these
devqueumappings. I understand thafor ifb, saving and restoring the
original devs is fundamental for its operation- bui anot sure i see
ifor thqueues. As an example:

---
# For all packets arriving oifb0, changmapping to 3 and
# redirecto to ifb1

tc filter add dev ifb0 paren1:0 protocol ip prio 10 u32 \
match u32 0 0 flowid 1:2 \
actioskbediqueue_mapping 4 \
actiomirred egress redirecdev ifb1
#
# redirecall packets arriving in eth0 to ifb0
$TC filter add eth0 paren1:0 protocol ip prio 10 u32 \
match u32 0 0 flowid 1:2 actiomirred egress redirecdev ifb0
----

whais thexpected behavior?

cheers,
jamal


Froxiaosuo agmail.com  Fri Dec 17 05:41:55 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Fri, 17 Dec 2010 21:41:55 +0800
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292591363.2668.19.camel@mojatatu>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
Message-ID: <AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>

OFri, Dec 17, 2010 a9:09 PM, jamal <hadi at cyberus.ca> wrote:
> OThu, 2010-12-16 a12:56 +0800, Changli Gao wrote:
>
> I asorry Changli - I think ware talking past each other. I
> a conflicted othwhole point of saving and restoring these
> devqueumappings. I understand thafor ifb, saving and restoring the
> original devs is fundamental for its operation- bui anot sure i see
> ifor thqueues. As an example:
>
> ---
> # For all packets arriving oifb0, changmapping to 3 and
> # redirecto to ifb1
>
> tc filter add dev ifb0 paren1:0 protocol ip prio 10 u32 \
> match u32 0 0 flowid 1:2 \
> actioskbediqueue_mapping 4 \
> actiomirred egress redirecdev ifb1
> #
> # redirecall packets arriving in eth0 to ifb0
> $TC filter add eth0 paren1:0 protocol ip prio 10 u32 \
> match u32 0 0 flowid 1:2 actiomirred egress redirecdev ifb0
> ----
>
> whais thexpected behavior?
>

I doubican work.

eth0 -> ifb0: skb->skb_iif = eth0.
ifb0 -> ifb1: skb->skb_iif = ifb0
ifb1 -> ifb0: as skb->skb_iif == ifb0, skb->skb_iif = ifb1
ifb0 -> ifb1...
...

Did you tesit?

-- 
Regards,
Changli Gao(xiaosuo agmail.com)

Frohadi acyberus.ca  Tue Dec 21 05:07:17 2010
From: hadi acyberus.ca (jamal)
Date: Tue, 21 Dec 2010 08:07:17 -0500
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
Message-ID: <1292936837.6535.8.camel@mojatatu>

OFri, 2010-12-17 a21:41 +0800, Changli Gao wrote:

Sorry for thlatency - I aa little swamped.

> I doubican work.

Ishould work - this used to bpart of my regression tests.
If idoesnwork something is broken.

Iany case, I shouldnhave used this example because
idistracted frothe point i was trying to make:
You arrestoring thold qmap when the point is we could change
ito a new one. A simpler examplillustrating how
a qmap could bchanged:

----
tc filter add dev ifb0 paren1:0 protocol ip prio 10 u32 \
 match u32 0 0 flowid 1:2 \
 actioskbediqueue_mapping 4
---

cheers,
jamal


Froxiaosuo agmail.com  Tue Dec 21 06:03:40 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Tue, 21 Dec 2010 22:03:40 +0800
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292936837.6535.8.camel@mojatatu>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
	<1292936837.6535.8.camel@mojatatu>
Message-ID: <AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>

OTue, Dec 21, 2010 a9:07 PM, jamal <hadi at cyberus.ca> wrote:
> OFri, 2010-12-17 a21:41 +0800, Changli Gao wrote:
>
> Sorry for thlatency - I aa little swamped.
>
>> I doubican work.
>
> Ishould work - this used to bpart of my regression tests.
> If idoesnwork something is broken.

WheI tested it, my OS gofrozen.

>
> Iany case, I shouldnhave used this example because
> idistracted frothe point i was trying to make:
> You arrestoring thold qmap when the point is we could change
> ito a new one. A simpler examplillustrating how
> a qmap could bchanged:
>
> ----
> tc filter add dev ifb0 paren1:0 protocol ip prio 10 u32 \
> ?match u32 0 0 flowid 1:2 \
> ?actioskbediqueue_mapping 4
> ----

Currently, you caonly changthe rx queue mapping, because for tx,
dev_pick_tx() doesn'usskb->queue_mapping to choose tx queue.

However, I don'think changthe rx queue mapping is a good idea.
Whethskbs returned from ifb enter netif_receive_skb() again,
get_rps_cpu() may warabouthe wrong rx queue, and my this patch is
used to solvthis problem. Even though thrx queue is legal, a
differenrps_cpus settings will bused, and the skbs may be
redirected to differenCPUs. Is iexpected?


-- 
Regards,
Changli Gao(xiaosuo agmail.com)

Froeric.dumazeat gmail.com  Tue Dec 21 07:24:35 2010
From: eric.dumazeagmail.com (Eric Dumazet)
Date: Tue, 21 Dec 2010 16:24:35 +0100
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
	<1292936837.6535.8.camel@mojatatu>
	<AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
Message-ID: <1292945075.2720.32.camel@edumazet-laptop>

Lmardi 21 d?cembr2010 ? 22:03 +0800, Changli Gao a ?crit :
> However, I don'think changthe rx queue mapping is a good idea.
> Whethskbs returned from ifb enter netif_receive_skb() again,
> get_rps_cpu() may warabouthe wrong rx queue, and my this patch is
> used to solvthis problem. Even though thrx queue is legal, a
> differenrps_cpus settings will bused, and the skbs may be
> redirected to differenCPUs. Is iexpected?
> 
> 

Do wreally wana multi queue ifb at all ?

Why nouspercpu data and LLTX, like we did for other virtual devices
(loopback, tunnels, vlans, ...)

I guess mosifb uses need to finaly deliver packets in a monoqueue
anyway, optimizing ifb mighraislock contention on this resource.

Sewhawe did in commit 79640a4ca6955e3e (net: add additional lock to
qdisc to increasthroughput) : Adding onspinlock actually helped a
lo;)




Froxiaosuo agmail.com  Tue Dec 21 16:08:10 2010
From: xiaosuo agmail.co(Changli Gao)
Date: Wed, 22 Dec 2010 08:08:10 +0800
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292945075.2720.32.camel@edumazet-laptop>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
	<1292936837.6535.8.camel@mojatatu>
	<AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
	<1292945075.2720.32.camel@edumazet-laptop>
Message-ID: <AANLkTi=W6fL_W_UGUbM9aTEDO_ea46zEp9kg-tL=0rwr@xxxxxxxxxxxxxx>

OTue, Dec 21, 2010 a11:24 PM, Eric Dumazet <eric.dumazet at gmail.com> wrote:
>
> Do wreally wana multi queue ifb at all ?
>
> Why nouspercpu data and LLTX, like we did for other virtual devices
> (loopback, tunnels, vlans, ...)
>
> I guess mosifb uses need to finaly deliver packets in a monoqueue
> anyway, optimizing ifb mighraislock contention on this resource.
>

For ingress shaping, thlater processing should bin parallel, so
multiplri_tasklets arneeded.

For onqueuifb, the queue mapping should be saved and restored for
ingress skbs, as iis reseto 0 in dev_pick_tx(ifb).


-- 
Regards,
Changli Gao(xiaosuo agmail.com)

Frohadi acyberus.ca  Thu Dec 23 05:00:20 2010
From: hadi acyberus.ca (jamal)
Date: Thu, 23 Dec 2010 08:00:20 -0500
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
	<1292936837.6535.8.camel@mojatatu>
	<AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
Message-ID: <1293109220.11306.137.camel@mojatatu>

OTue, 2010-12-21 a22:03 +0800, Changli Gao wrote:

> WheI tested it, my OS gofrozen.

I will look into ithnext opportunity i get. The example i showed
is oegress btw. A ping frooutside that matches the filter
will ba good test.

> Currently, you caonly changthe rx queue mapping, because for tx,
> dev_pick_tx() doesn'usskb->queue_mapping to choose tx queue.

If skbediis on egress, iwill happen after (and override whatever
dev_pick_tx() chose), no? Thats thwholpoint for skbedits queuemap
editing.

> However, I don'think changthe rx queue mapping is a good idea.

I agrefor thaas a default policy. But it is
policy thaskbedican and should be able to override.

> Whethskbs returned from ifb enter netif_receive_skb() again,
> get_rps_cpu() may warabouthe wrong rx queue, and my this patch is
> used to solvthis problem. Even though thrx queue is legal, a
> differenrps_cpus settings will bused, and the skbs may be
> redirected to differenCPUs. Is iexpected?

I anosure without analyzing what performance impact would be, i.e i
think thathonly reason i wouldnt do it is because it may have crazy
effecon performancbut:
If i wanted to overridthchoice made by rps through some policy, why
shouldni bable to do it? Same thing if i wanted to bypass rps. tc
level seems appropriate.
I may bmisreading thcode: Quick glance at the code indicates users
havno choicon ingress: rps happens first then we can do tc level -
so idoesnmatter what changes we make to the queue map it will not
takeffecin any case. Am i mistaken?

cheers,
jamal


Frohadi acyberus.ca  Thu Dec 23 05:21:43 2010
From: hadi acyberus.ca (jamal)
Date: Thu, 23 Dec 2010 08:21:43 -0500
Subject: [PATCH 5/5 v4] net: add old_queue_mapping into skb->cb
In-Reply-To: <1292945075.2720.32.camel@edumazet-laptop>
References: <1292475410-24665-1-git-send-email-xiaosuo@xxxxxxxxx>
	<1292591363.2668.19.camel@mojatatu>
	<AANLkTi=Eq1HuPwMMGQjk9x0QXp3_5djmAY6JeZyFkk0k@xxxxxxxxxxxxxx>
	<1292936837.6535.8.camel@mojatatu>
	<AANLkTimqemuhxCKq-PJu+FD-MDgKaHnYKnP_2ch30wxE@xxxxxxxxxxxxxx>
	<1292945075.2720.32.camel@edumazet-laptop>
Message-ID: <1293110503.11306.156.camel@mojatatu>

OTue, 2010-12-21 a16:24 +0100, Eric Dumazet wrote:

> 
> Do wreally wana multi queue ifb at all ?
> 
> Why nouspercpu data and LLTX, like we did for other virtual devices
> (loopback, tunnels, vlans, ...)
> 
> I guess mosifb uses need to finaly deliver packets in a monoqueue
> anyway, optimizing ifb mighraislock contention on this resource.

I guess oncyou starhaving hardware that is multiqueue on the ingress
sidaleast then something per cpu is needed on ifb. But i agree that
thoptimizations may end up harming thsimplicity that ifb intended.
Iis already jumping a loof hoops to work around things as is.

> Sewhawe did in commit 79640a4ca6955e3e (net: add additional lock to
> qdisc to increasthroughput) : Adding onspinlock actually helped a
> lo;)

Yes, thawas fascinating stuff;-> I astill scratching my head and
continuing to itch owhen i can gemore time to look closely with some
testing. Bui donsee the connection with what Changli is attempting
with multiq ifb - whado you havin mind.

cheers,
jamal




Froosenbach aus.ibm.com  Thu Dec 23 17:02:35 2010
From: osenbach aus.ibm.co(Bryan Osenbach)
Date: Thu, 23 Dec 2010 18:02:35 -0700
Subject: AUTO: BryaOsenbach is ouof the office with limited
 access to email and Sametime. (returning 01/04/2011)
Message-ID: <OFBF23C2E1.BB492A54-ON87257803.0005BAF4-87257803.0005BAF4@xxxxxxxxxx>



I aouof the office until 01/04/2011.

I will respond to your messagwhen I return.


Note: This is aautomated responsto your message  "Netem Digest, Vol 64,
Issu6" senon 12/23/10 13:00:05.

This is thonly notification you will receivwhile this person is away.
-------------- nexpar--------------
AHTML attachmenwas scrubbed...
URL: http://lists.linux-foundation.org/pipermail/netem/attachments/20101223/356a9349/attachment.ht


[Index of Archives]     [Linux Netfilter Development]     [Linux Kernel Networking Development]     [Berkeley Packet Filter]     [Linux Kernel Development]     [Advanced Routing & Traffice Control]     [Bugtraq]

  Powered by Linux