Re: FW: Kernel panic during stress with igb in the upstream kernel

Re: FW: Kernel panic during stress with igb in the upstream kernel

[Herbert Xu]

On Thu, Mar 12, 2009 at 09:31:18AM -0600, Tantilov, Emil S wrote:
> 
> Yeah - that's probably the case. I will restart the test with LRO off to make sure.
> 
> I was also able to reproduce the panic with the master branch, so I think we should notify netdev. I can't figure out how I missed this before ... Something must be different about this configuration. I am running all kinds of stress tests in parallel with igb/igbvf drivers and I don't see this.
> 
> Here's the trace:
> 
> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> last sysfs file: /sys/devices/pci0000:00/0000:00:04.0/0000:0b:00.0/local_cpus
> CPU 3 
> Modules linked in: igb nfsd lockd exportfs sunrpc pci_slot [last unloaded: igb]
> Pid: 28747, comm: netperf Not tainted 2.6.29-rc7-net-next-igb-master #3 X7DA8
> RIP: 0010:[<ffffffff804d4316>]  [<ffffffff804d4316>] netpoll_receive_skb+0xe/0x1e

Sorry, it seems that I've managed to get the net/net-next patches
mixed up.  Does this help?

GRO: Move netpoll checks to correct location

As my netpoll fix for net doesn't really work for net-next, we
need this update to move the checks into the right place.  As it
stands we may pass freed skbs to netpoll_receive_skb.

This patch also introduces a netpoll_rx_on function to avoid GRO
completely if we're invoked through netpoll.  This might seem
paranoid but as netpoll may have an external receive hook it's
better to be safe than sorry.  I don't think we need this for
2.6.29 though since there's nothing immediately broken by it.

This patch also moves the GRO_* return values to netdevice.h since
VLAN needs them too (I tried to avoid this originally but alas
this seems to be the easiest way out).  This fixes a bug in VLAN
where it continued to use the old return value 2 instead of the
correct GRO_DROP.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 493b065..be3ebd7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -330,6 +330,14 @@ enum
 	NAPI_STATE_NPSVC,	/* Netpoll - don't dequeue from poll_list */
 };
 
+enum {
+	GRO_MERGED,
+	GRO_MERGED_FREE,
+	GRO_HELD,
+	GRO_NORMAL,
+	GRO_DROP,
+};
+
 extern void __napi_schedule(struct napi_struct *n);
 
 static inline int napi_disable_pending(struct napi_struct *n)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index e38d3c9..5f239dc 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,6 +63,13 @@ static inline int netpoll_rx(struct sk_buff *skb)
 	return ret;
 }
 
+static inline int netpoll_rx_on(struct sk_buff *skb)
+{
+	struct netpoll_info *npinfo = skb->dev->npinfo;
+
+	return npinfo && (npinfo->rx_np || npinfo->rx_flags);
+}
+
 static inline int netpoll_receive_skb(struct sk_buff *skb)
 {
 	if (!list_empty(&skb->dev->napi_list))
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 2d6e405..6227248 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -79,6 +79,9 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -98,7 +101,7 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 	return dev_gro_receive(napi, skb);
 
 drop:
-	return 2;
+	return GRO_DROP;
 }
 
 int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
@@ -106,9 +109,6 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 {
 	skb_gro_reset_offset(skb);
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_skb_finish(vlan_gro_common(napi, grp, vlan_tci, skb), skb);
 }
 EXPORT_SYMBOL(vlan_gro_receive);
@@ -121,9 +121,6 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		return NET_RX_DROP;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_frags_finish(napi, skb,
 				 vlan_gro_common(napi, grp, vlan_tci, skb));
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 033d7ca..7bd3c29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,14 +135,6 @@
 /* This should be increased if a protocol with a bigger head is added. */
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
 
-enum {
-	GRO_MERGED,
-	GRO_MERGED_FREE,
-	GRO_HELD,
-	GRO_NORMAL,
-	GRO_DROP,
-};
-
 /*
  *	The list of packet types we will receive (as opposed to discard)
  *	and the routines to invoke.
@@ -2474,6 +2466,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	for (p = napi->gro_list; p; p = p->next) {
 		NAPI_GRO_CB(p)->same_flow = !compare_ether_header(
 			skb_mac_header(p), skb_gro_mac_header(skb));
@@ -2487,9 +2482,6 @@ int napi_skb_finish(int ret, struct sk_buff *skb)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 		return netif_receive_skb(skb);
@@ -2587,9 +2579,6 @@ int napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb, int ret)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 	case GRO_HELD:

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: FW: Kernel panic during stress with igb in the upstream kernel

[Tantilov, Emil S]

Thanks Herbert,

With this patch the stress test has been going for more than 4 hours now (compared to minutes until panic before). I will leave the test running over the weekend, but at least for now looks like it's fixed.

Thanks,
Emil 

-----Original Message-----
From: Herbert Xu [mailto:herbert@gondor.apana.org.au] 
Sent: Friday, March 13, 2009 4:53 AM
To: Tantilov, Emil S
Cc: Duyck, Alexander H; Brandeburg, Jesse; Kirsher, Jeffrey T; David S. Miller; netdev@vger.kernel.org
Subject: Re: FW: Kernel panic during stress with igb in the upstream kernel

On Thu, Mar 12, 2009 at 09:31:18AM -0600, Tantilov, Emil S wrote:
> 
> Yeah - that's probably the case. I will restart the test with LRO off to make sure.
> 
> I was also able to reproduce the panic with the master branch, so I think we should notify netdev. I can't figure out how I missed this before ... Something must be different about this configuration. I am running all kinds of stress tests in parallel with igb/igbvf drivers and I don't see this.
> 
> Here's the trace:
> 
> general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> last sysfs file: /sys/devices/pci0000:00/0000:00:04.0/0000:0b:00.0/local_cpus
> CPU 3 
> Modules linked in: igb nfsd lockd exportfs sunrpc pci_slot [last unloaded: igb]
> Pid: 28747, comm: netperf Not tainted 2.6.29-rc7-net-next-igb-master #3 X7DA8
> RIP: 0010:[<ffffffff804d4316>]  [<ffffffff804d4316>] netpoll_receive_skb+0xe/0x1e

Sorry, it seems that I've managed to get the net/net-next patches
mixed up.  Does this help?

GRO: Move netpoll checks to correct location

As my netpoll fix for net doesn't really work for net-next, we
need this update to move the checks into the right place.  As it
stands we may pass freed skbs to netpoll_receive_skb.

This patch also introduces a netpoll_rx_on function to avoid GRO
completely if we're invoked through netpoll.  This might seem
paranoid but as netpoll may have an external receive hook it's
better to be safe than sorry.  I don't think we need this for
2.6.29 though since there's nothing immediately broken by it.

This patch also moves the GRO_* return values to netdevice.h since
VLAN needs them too (I tried to avoid this originally but alas
this seems to be the easiest way out).  This fixes a bug in VLAN
where it continued to use the old return value 2 instead of the
correct GRO_DROP.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 493b065..be3ebd7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -330,6 +330,14 @@ enum
 	NAPI_STATE_NPSVC,	/* Netpoll - don't dequeue from poll_list */
 };
 
+enum {
+	GRO_MERGED,
+	GRO_MERGED_FREE,
+	GRO_HELD,
+	GRO_NORMAL,
+	GRO_DROP,
+};
+
 extern void __napi_schedule(struct napi_struct *n);
 
 static inline int napi_disable_pending(struct napi_struct *n)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index e38d3c9..5f239dc 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,6 +63,13 @@ static inline int netpoll_rx(struct sk_buff *skb)
 	return ret;
 }
 
+static inline int netpoll_rx_on(struct sk_buff *skb)
+{
+	struct netpoll_info *npinfo = skb->dev->npinfo;
+
+	return npinfo && (npinfo->rx_np || npinfo->rx_flags);
+}
+
 static inline int netpoll_receive_skb(struct sk_buff *skb)
 {
 	if (!list_empty(&skb->dev->napi_list))
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 2d6e405..6227248 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -79,6 +79,9 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -98,7 +101,7 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 	return dev_gro_receive(napi, skb);
 
 drop:
-	return 2;
+	return GRO_DROP;
 }
 
 int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
@@ -106,9 +109,6 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 {
 	skb_gro_reset_offset(skb);
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_skb_finish(vlan_gro_common(napi, grp, vlan_tci, skb), skb);
 }
 EXPORT_SYMBOL(vlan_gro_receive);
@@ -121,9 +121,6 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		return NET_RX_DROP;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_frags_finish(napi, skb,
 				 vlan_gro_common(napi, grp, vlan_tci, skb));
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 033d7ca..7bd3c29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,14 +135,6 @@
 /* This should be increased if a protocol with a bigger head is added. */
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
 
-enum {
-	GRO_MERGED,
-	GRO_MERGED_FREE,
-	GRO_HELD,
-	GRO_NORMAL,
-	GRO_DROP,
-};
-
 /*
  *	The list of packet types we will receive (as opposed to discard)
  *	and the routines to invoke.
@@ -2474,6 +2466,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	for (p = napi->gro_list; p; p = p->next) {
 		NAPI_GRO_CB(p)->same_flow = !compare_ether_header(
 			skb_mac_header(p), skb_gro_mac_header(skb));
@@ -2487,9 +2482,6 @@ int napi_skb_finish(int ret, struct sk_buff *skb)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 		return netif_receive_skb(skb);
@@ -2587,9 +2579,6 @@ int napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb, int ret)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 	case GRO_HELD:

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Kernel panic during stress with igb in the upstream kernel

[David Miller]

From: "Tantilov, Emil S" <emil.s.tantilov@intel.com>
Date: Fri, 13 Mar 2009 15:53:47 -0600

> With this patch the stress test has been going for more than 4 hours
> now (compared to minutes until panic before). I will leave the test
> running over the weekend, but at least for now looks like it's
> fixed.

Thanks for testing.

Herbert, want me to toss this into net-2.6 now?

Re: Kernel panic during stress with igb in the upstream kernel

[Herbert Xu]

On Fri, Mar 13, 2009 at 02:54:48PM -0700, David Miller wrote:
> 
> Herbert, want me to toss this into net-2.6 now?

No, the bug only exists in net-next-2.6 :)

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Kernel panic during stress with igb in the upstream kernel

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 13 Mar 2009 19:52:45 +0800

> Sorry, it seems that I've managed to get the net/net-next patches
> mixed up.  Does this help?

Actually I introduced this problem when I did the net-2.6
--> net-next-2.6 merge when your netpoll fix went it. :-/

> GRO: Move netpoll checks to correct location
> 
> As my netpoll fix for net doesn't really work for net-next, we
> need this update to move the checks into the right place.  As it
> stands we may pass freed skbs to netpoll_receive_skb.
> 
> This patch also introduces a netpoll_rx_on function to avoid GRO
> completely if we're invoked through netpoll.  This might seem
> paranoid but as netpoll may have an external receive hook it's
> better to be safe than sorry.  I don't think we need this for
> 2.6.29 though since there's nothing immediately broken by it.
> 
> This patch also moves the GRO_* return values to netdevice.h since
> VLAN needs them too (I tried to avoid this originally but alas
> this seems to be the easiest way out).  This fixes a bug in VLAN
> where it continued to use the old return value 2 instead of the
> correct GRO_DROP.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks!

Re: Kernel panic during stress with igb in the upstream kernel

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Sun, 15 Mar 2009 20:13:53 -0700 (PDT)

> From: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Fri, 13 Mar 2009 19:52:45 +0800
> 
> > GRO: Move netpoll checks to correct location
 ...
> 
> Applied, thanks!

Actually, I had to revert, it doesn't build with netpoll
disabled:

net/8021q/vlan_core.c: In function ‘vlan_gro_common’:
net/8021q/vlan_core.c:82: error: implicit declaration of function ‘netpoll_rx_on’
make[2]: *** [net/8021q/vlan_core.o] Error 1
make[1]: *** [net/8021q] Error 2
make: *** [net] Error 2
make: *** Waiting for unfinished jobs....

Please fix this up and resubmit, thanks!

Re: Kernel panic during stress with igb in the upstream kernel

[Herbert Xu]

On Sun, Mar 15, 2009 at 08:18:02PM -0700, David Miller wrote:
> 
> Actually, I had to revert, it doesn't build with netpoll
> disabled:
> 
> net/8021q/vlan_core.c: In function ‘vlan_gro_common’:
> net/8021q/vlan_core.c:82: error: implicit declaration of function ‘netpoll_rx_on’
> make[2]: *** [net/8021q/vlan_core.o] Error 1
> make[1]: *** [net/8021q] Error 2
> make: *** [net] Error 2
> make: *** Waiting for unfinished jobs....

Oops, forgot the #else case.

GRO: Move netpoll checks to correct location

As my netpoll fix for net doesn't really work for net-next, we
need this update to move the checks into the right place.  As it
stands we may pass freed skbs to netpoll_receive_skb.

This patch also introduces a netpoll_rx_on function to avoid GRO
completely if we're invoked through netpoll.  This might seem
paranoid but as netpoll may have an external receive hook it's
better to be safe than sorry.  I don't think we need this for
2.6.29 though since there's nothing immediately broken by it.

This patch also moves the GRO_* return values to netdevice.h since
VLAN needs them too (I tried to avoid this originally but alas
this seems to be the easiest way out).  This fixes a bug in VLAN
where it continued to use the old return value 2 instead of the
correct GRO_DROP.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 493b065..be3ebd7 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -330,6 +330,14 @@ enum
 	NAPI_STATE_NPSVC,	/* Netpoll - don't dequeue from poll_list */
 };
 
+enum {
+	GRO_MERGED,
+	GRO_MERGED_FREE,
+	GRO_HELD,
+	GRO_NORMAL,
+	GRO_DROP,
+};
+
 extern void __napi_schedule(struct napi_struct *n);
 
 static inline int napi_disable_pending(struct napi_struct *n)
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index e38d3c9..de99025 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,6 +63,13 @@ static inline int netpoll_rx(struct sk_buff *skb)
 	return ret;
 }
 
+static inline int netpoll_rx_on(struct sk_buff *skb)
+{
+	struct netpoll_info *npinfo = skb->dev->npinfo;
+
+	return npinfo && (npinfo->rx_np || npinfo->rx_flags);
+}
+
 static inline int netpoll_receive_skb(struct sk_buff *skb)
 {
 	if (!list_empty(&skb->dev->napi_list))
@@ -99,6 +106,10 @@ static inline int netpoll_rx(struct sk_buff *skb)
 {
 	return 0;
 }
+static inline int netpoll_rx_on(struct sk_buff *skb)
+{
+	return 0;
+}
 static inline int netpoll_receive_skb(struct sk_buff *skb)
 {
 	return 0;
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 2d6e405..6227248 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -79,6 +79,9 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -98,7 +101,7 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 	return dev_gro_receive(napi, skb);
 
 drop:
-	return 2;
+	return GRO_DROP;
 }
 
 int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
@@ -106,9 +109,6 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 {
 	skb_gro_reset_offset(skb);
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_skb_finish(vlan_gro_common(napi, grp, vlan_tci, skb), skb);
 }
 EXPORT_SYMBOL(vlan_gro_receive);
@@ -121,9 +121,6 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		return NET_RX_DROP;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	return napi_frags_finish(napi, skb,
 				 vlan_gro_common(napi, grp, vlan_tci, skb));
 }
diff --git a/net/core/dev.c b/net/core/dev.c
index 033d7ca..7bd3c29 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,14 +135,6 @@
 /* This should be increased if a protocol with a bigger head is added. */
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
 
-enum {
-	GRO_MERGED,
-	GRO_MERGED_FREE,
-	GRO_HELD,
-	GRO_NORMAL,
-	GRO_DROP,
-};
-
 /*
  *	The list of packet types we will receive (as opposed to discard)
  *	and the routines to invoke.
@@ -2474,6 +2466,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
 	struct sk_buff *p;
 
+	if (netpoll_rx_on(skb))
+		return GRO_NORMAL;
+
 	for (p = napi->gro_list; p; p = p->next) {
 		NAPI_GRO_CB(p)->same_flow = !compare_ether_header(
 			skb_mac_header(p), skb_gro_mac_header(skb));
@@ -2487,9 +2482,6 @@ int napi_skb_finish(int ret, struct sk_buff *skb)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 		return netif_receive_skb(skb);
@@ -2587,9 +2579,6 @@ int napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb, int ret)
 {
 	int err = NET_RX_SUCCESS;
 
-	if (netpoll_receive_skb(skb))
-		return NET_RX_DROP;
-
 	switch (ret) {
 	case GRO_NORMAL:
 	case GRO_HELD:

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Kernel panic during stress with igb in the upstream kernel

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Mon, 16 Mar 2009 21:22:40 +0800

> On Sun, Mar 15, 2009 at 08:18:02PM -0700, David Miller wrote:
> > 
> > Actually, I had to revert, it doesn't build with netpoll
> > disabled:
> > 
> > net/8021q/vlan_core.c: In function ‘vlan_gro_common’:
> > net/8021q/vlan_core.c:82: error: implicit declaration of function ‘netpoll_rx_on’
> > make[2]: *** [net/8021q/vlan_core.o] Error 1
> > make[1]: *** [net/8021q] Error 2
> > make: *** [net] Error 2
> > make: *** Waiting for unfinished jobs....
> 
> Oops, forgot the #else case.
> 
> GRO: Move netpoll checks to correct location

Applied, thanks!

Re: Kernel panic during stress with igb in the upstream kernel

[Jarek Poplawski]

On 16-03-2009 14:22, Herbert Xu wrote:
...
> GRO: Move netpoll checks to correct location
...
> +static inline int netpoll_rx_on(struct sk_buff *skb)
> +{
> +	struct netpoll_info *npinfo = skb->dev->npinfo;
> +
> +	return npinfo && (npinfo->rx_np || npinfo->rx_flags);
> +}
> +
>  static inline int netpoll_receive_skb(struct sk_buff *skb)
>  {
>  	if (!list_empty(&skb->dev->napi_list))
> @@ -99,6 +106,10 @@ static inline int netpoll_rx(struct sk_buff *skb)
>  {
>  	return 0;
>  }
> +static inline int netpoll_rx_on(struct sk_buff *skb)
> +{
> +	return 0;
> +}
>  static inline int netpoll_receive_skb(struct sk_buff *skb)
>  {
>  	return 0;
> diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
> index 2d6e405..6227248 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -79,6 +79,9 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
>  {
>  	struct sk_buff *p;
>  
> +	if (netpoll_rx_on(skb))
> +		return GRO_NORMAL;
> +

Probably I miss something, but you seem to assume here this skb will
be taken by netpoll later. But if it's not active (trapped) vlan
packet will be passed as "normal"?

Thanks,
Jarek P.

Re: Kernel panic during stress with igb in the upstream kernel

[Herbert Xu]

On Tue, Mar 17, 2009 at 09:37:24AM +0000, Jarek Poplawski wrote:
>
> Probably I miss something, but you seem to assume here this skb will
> be taken by netpoll later. But if it's not active (trapped) vlan
> packet will be passed as "normal"?

Good point.  I'll see if this stuff can be simplified further,
but for now, let's go for the obvious fix.

gro: Fix vlan/netpoll check again

Jarek Poplawski pointed out that my previous fix is broken for
VLAN+netpoll as if netpoll is enabled we'd end up in the normal
receive path instead of the VLAN receive path.

This patch fixes it by calling the VLAN receive hook.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 6227248..654e45f 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -79,9 +79,6 @@ static int vlan_gro_common(struct napi_struct *napi, struct vlan_group *grp,
 {
 	struct sk_buff *p;
 
-	if (netpoll_rx_on(skb))
-		return GRO_NORMAL;
-
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -107,6 +104,9 @@ drop:
 int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 		     unsigned int vlan_tci, struct sk_buff *skb)
 {
+	if (netpoll_rx_on(skb))
+		return vlan_hwaccel_receive_skb(skb, grp, vlan_tci);
+
 	skb_gro_reset_offset(skb);
 
 	return napi_skb_finish(vlan_gro_common(napi, grp, vlan_tci, skb), skb);
@@ -121,6 +121,9 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		return NET_RX_DROP;
 
+	if (netpoll_rx_on(skb))
+		return vlan_hwaccel_receive_skb(skb, grp, vlan_tci);
+
 	return napi_frags_finish(napi, skb,
 				 vlan_gro_common(napi, grp, vlan_tci, skb));
 }

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: Kernel panic during stress with igb in the upstream kernel

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Tue, 17 Mar 2009 18:02:03 +0800

> On Tue, Mar 17, 2009 at 09:37:24AM +0000, Jarek Poplawski wrote:
> >
> > Probably I miss something, but you seem to assume here this skb will
> > be taken by netpoll later. But if it's not active (trapped) vlan
> > packet will be passed as "normal"?
> 
> Good point.  I'll see if this stuff can be simplified further,
> but for now, let's go for the obvious fix.
> 
> gro: Fix vlan/netpoll check again
> 
> Jarek Poplawski pointed out that my previous fix is broken for
> VLAN+netpoll as if netpoll is enabled we'd end up in the normal
> receive path instead of the VLAN receive path.
> 
> This patch fixes it by calling the VLAN receive hook.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Applied, thanks Herbert.