Kernel problem

Kernel problem

[Denis Romanenko]

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

Hi
I have high loaded router - about 1.2 Gbps

after reboot on new kernel - 2.6.27.17 i have messages in dmesg

WARNING: at kernel/softirq.c:136 local_bh_enable+0x36/0xab()
Modules linked in: netconsole configfs cls_route cls_u32 cls_fw  
sch_sfq sch_htb 8021q i2c_i801 i2c_core rng_core iTCO_wdt  
iTCO_vendor_support e1000e thermal i5000_edac
Pid: 17567, comm: ethtool Not tainted 2.6.27.17 #1
  [<c011d7d5>] warn_on_slowpath+0x41/0x65
  [<c0277fd9>] ? tc_classify_compat+0x2f/0x5e
  [<f88af738>] ? htb_add_to_id_tree+0x6c/0x74 [sch_htb]
  [<f88af7e7>] ? htb_activate_prios+0xa7/0xb2 [sch_htb]
  [<c026cc96>] ? dev_queue_xmit+0x356/0x490
  [<f88b11da>] ? htb_enqueue+0x152/0x1b1 [sch_htb]
  [<c026cd98>] ? dev_queue_xmit+0x458/0x490
  [<c01219d3>] local_bh_enable+0x36/0xab
  [<c026cd98>] dev_queue_xmit+0x458/0x490
  [<c0288334>] ip_finish_output+0x1d0/0x208
  [<c0288610>] ip_output+0x81/0x86
  [<c0285e4b>] ip_forward_finish+0x2f/0x32
  [<c0286042>] ip_forward+0x1f4/0x25a
  [<c0284e17>] ip_rcv_finish+0x2bb/0x2e9
  [<c01324dc>] ? getnstimeofday+0x4f/0xda
  [<c0285271>] ip_rcv+0x1f2/0x21c
  [<c026a3de>] netif_receive_skb+0x3d0/0x424
  [<c02b205e>] __vlan_hwaccel_rx+0x89/0xa7
  [<f886529e>] e1000_receive_skb+0x41/0x61 [e1000e]
  [<f88675c9>] e1000_clean_rx_irq+0x213/0x2a1 [e1000e]
  [<f8864af4>] e1000_clean+0x2ac/0x43a [e1000e]
  [<c027510e>] netpoll_poll+0x77/0x33b
  [<c0275487>] ? netpoll_send_skb+0xb5/0x14f
  [<c0275495>] netpoll_send_skb+0xc3/0x14f
  [<c0275987>] netpoll_send_udp+0x1e8/0x1f0
  [<f88740be>] write_msg+0x6e/0xa1 [netconsole]
  [<f8874050>] ? write_msg+0x0/0xa1 [netconsole]
  [<c011d8c3>] __call_console_drivers+0x56/0x63
  [<c011d927>] _call_console_drivers+0x57/0x5b
  [<c011dca5>] release_console_sem+0x116/0x1ab
  [<c011e20c>] vprintk+0x325/0x352
  [<c0173015>] ? __d_lookup+0xce/0x114
  [<c017303d>] ? __d_lookup+0xf6/0x114
  [<c011e24e>] printk+0x15/0x17
  [<f886226a>] e1000_set_tso+0x4c/0x5d [e1000e]
  [<c026dac7>] dev_ethtool+0x93d/0xd52
  [<c013852a>] ? __lock_acquire+0x606/0x65c
  [<c0136e13>] ? mark_held_locks+0x47/0x62
  [<c0136fba>] ? trace_hardirqs_on+0xb/0xd
  [<c0136f8e>] ? trace_hardirqs_on_caller+0xe1/0x102
  [<c02b8b16>] ? mutex_lock_nested+0x203/0x20b
  [<c0272b62>] ? rtnl_lock+0xf/0x11
  [<c026bc5b>] dev_ioctl+0x460/0x587
  [<c014fd3d>] ? __do_fault+0x2b1/0x2e9
  [<c026056f>] sock_ioctl+0x1c4/0x1d0
  [<c02603ab>] ? sock_ioctl+0x0/0x1d0
  [<c016e382>] vfs_ioctl+0x22/0x67
  [<c016e5fe>] do_vfs_ioctl+0x237/0x245
  [<c01d2cd8>] ? trace_hardirqs_on_thunk+0xc/0x10
  [<c016e638>] sys_ioctl+0x2c/0x48
  [<c01037dd>] sysenter_do_call+0x12/0x35
  =======================
---[ end trace ac68705e6d55d8fc ]---


What the problem on this ?

With best regards Denis Romanenko


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2183 bytes --]

Re: Kernel problem

[Denys Fedoryschenko]

I spoke with Denis Romanenko (i know him)

Here is some additional info:
First of all, he booted kernel with extensive debug options. He was not able 
to run it for long time, because performance of router degraded too much.

Here is part of script where all things happened

# Disk discipline
echo cfq > /sys/block/sda/queue/scheduler

# Irq for cards
echo 4 >/proc/irq/215/smp_affinity
echo 8 >/proc/irq/214/smp_affinity
echo 1 >/proc/irq/216/smp_affinity
echo 2 >/proc/irq/217/smp_affinity

# Cards setup
/usr/sbin/ethtool -G eth0 rx 1024 tx 1024
/usr/sbin/ethtool -G eth1 rx 1024 tx 1024
/usr/sbin/ethtool -G eth2 rx 1024 tx 1024
/usr/sbin/ethtool -G eth3 rx 1024 tx 1024
/usr/sbin/ethtool -C eth0 rx-usecs 3
/usr/sbin/ethtool -C eth1 rx-usecs 3
/usr/sbin/ethtool -C eth2 rx-usecs 3
/usr/sbin/ethtool -C eth3 rx-usecs 3
/usr/sbin/ethtool -K eth0 tso off
/usr/sbin/ethtool -K eth1 tso off
/usr/sbin/ethtool -K eth2 tso off
/usr/sbin/ethtool -K eth3 tso off
/usr/sbin/ethtool -K eth0 sg off
/usr/sbin/ethtool -K eth1 sg off
/usr/sbin/ethtool -K eth2 sg off
/usr/sbin/ethtool -K eth3 sg off
/usr/sbin/ethtool -K eth0 ufo off
/usr/sbin/ethtool -K eth1 ufo off
/usr/sbin/ethtool -K eth2 ufo off
/usr/sbin/ethtool -K eth3 ufo off
/usr/sbin/ethtool -K eth0 gso off
/usr/sbin/ethtool -K eth1 gso off
/usr/sbin/ethtool -K eth2 gso off
/usr/sbin/ethtool -K eth3 gso off

# interface queue
/sbin/ifconfig eth0 txqueuelen 10000
/sbin/ifconfig eth1 txqueuelen 10000
/sbin/ifconfig eth2 txqueuelen 10000
/sbin/ifconfig eth3 txqueuelen 10000


On Thursday 26 February 2009 11:39:02 Herbert Xu wrote:
> Denis Romanenko <ash@sevsky.net> wrote:
> > I have high loaded router - about 1.2 Gbps
> >
> > after reboot on new kernel - 2.6.27.17 i have messages in dmesg
> >
> > WARNING: at kernel/softirq.c:136 local_bh_enable+0x36/0xab()
>
> So somebody disabled the IRQs on us, the question is who?
>
> Cheers,

Re: Kernel problem

[Herbert Xu]

Denis Romanenko <ash@sevsky.net> wrote:
> I have high loaded router - about 1.2 Gbps
> 
> after reboot on new kernel - 2.6.27.17 i have messages in dmesg
> 
> WARNING: at kernel/softirq.c:136 local_bh_enable+0x36/0xab()

So somebody disabled the IRQs on us, the question is who?

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 problem

[Jarek Poplawski]

On 26-02-2009 10:39, Herbert Xu wrote:
> Denis Romanenko <ash@sevsky.net> wrote:
>> I have high loaded router - about 1.2 Gbps
>>
>> after reboot on new kernel - 2.6.27.17 i have messages in dmesg
>>
>> WARNING: at kernel/softirq.c:136 local_bh_enable+0x36/0xab()
> 
> So somebody disabled the IRQs on us, the question is who?

netconsole. It's a known issue, btw.

Cheers,
Jarek P.

Re: Kernel problem

[Denis Romanenko]

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




> On 26-02-2009 10:39, Herbert Xu wrote:
>> Denis Romanenko <ash@sevsky.net> wrote:
>>> I have high loaded router - about 1.2 Gbps
>>>
>>> after reboot on new kernel - 2.6.27.17 i have messages in dmesg
>>>
>>> WARNING: at kernel/softirq.c:136 local_bh_enable+0x36/0xab()
>>
>> So somebody disabled the IRQs on us, the question is who?
>
> netconsole. It's a known issue, btw.
>

hmm - for stabibility can i disable netconsole ? Or this bug is not  
give my resutlts (reboot of router after 1-2 weeks of work )?

> Cheers,
> Jarek P.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2183 bytes --]

Re: Kernel problem

[Herbert Xu]

On Thu, Feb 26, 2009 at 10:56:16AM +0000, Jarek Poplawski wrote:
> 
> netconsole. It's a known issue, btw.

I see.  So the problem in this case appears to be that we're
processing incoming packets in netpoll_poll.  So the obvious
question can we just not do that since all we need to do is
transmit packets?

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 problem

[Jarek Poplawski]

On 26-02-2009 12:24, Denis Romanenko wrote:
...
> hmm - for stabibility can i disable netconsole ? Or this bug is not  
> give my resutlts (reboot of router after 1-2 weeks of work )?

Yes, if you don't have to use this to track some more serious problem,
IMHO it's better to disable netconsole.

Jarek P.

PS: don't forget to "Reply All" next time ;-)

Re: Kernel problem

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 26 Feb 2009 19:56:04 +0800

> On Thu, Feb 26, 2009 at 10:56:16AM +0000, Jarek Poplawski wrote:
> > 
> > netconsole. It's a known issue, btw.
> 
> I see.  So the problem in this case appears to be that we're
> processing incoming packets in netpoll_poll.  So the obvious
> question can we just not do that since all we need to do is
> transmit packets?

netpoll supports receiving packets and has a trap handler to eat them
to avoid packets it is interested in going into the real stack.
(netpoll_receive_skb, netpoll_rx_skb, etc.)

I'm pretty sure this has been discussed before. :-)

If I'm not mistaken, we only see this with certain drivers, and the
issue therefore likely has to do with how such drivers implement their
poll handler.

If you do your ->poll_controller() handler without taking IRQ locks,
you're fine.  Or something like that...

Re: Kernel problem

[Jarek Poplawski]

On Thu, Feb 26, 2009 at 12:00:52PM +0000, Jarek Poplawski wrote:
> On 26-02-2009 12:24, Denis Romanenko wrote:
> ...
> > hmm - for stabibility can i disable netconsole ? Or this bug is not  
> > give my resutlts (reboot of router after 1-2 weeks of work )?

Hmm... To make sure: you have these reboots without netconsole, right?

Jarek P.

Re: Kernel problem

[Denis Romanenko]

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


> On Thu, Feb 26, 2009 at 12:00:52PM +0000, Jarek Poplawski wrote:
>> On 26-02-2009 12:24, Denis Romanenko wrote:
>> ...
>>> hmm - for stabibility can i disable netconsole ? Or this bug is not
>>> give my resutlts (reboot of router after 1-2 weeks of work )?
>
> Hmm... To make sure: you have these reboots without netconsole, right?
>

on old kernels - (without patch #4 from you) (2.6.27.3-2.6.27.7) - its  
rebooted without netconsole
Now netconsole enabled and loaded by default


> Jarek P.


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2183 bytes --]

Re: Kernel problem

[Herbert Xu]

On Thu, Feb 26, 2009 at 04:10:37AM -0800, David Miller wrote:
> 
> netpoll supports receiving packets and has a trap handler to eat them
> to avoid packets it is interested in going into the real stack.
> (netpoll_receive_skb, netpoll_rx_skb, etc.)

OK after much head scratching and staring, it turns out that
this is caused by the VLAN path not doing the netpoll check
which would normally drop the packets when a printk is active.

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 problem

[Jarek Poplawski]

On 26-02-2009 13:51, Denis Romanenko wrote:
>> On Thu, Feb 26, 2009 at 12:00:52PM +0000, Jarek Poplawski wrote:
>>> On 26-02-2009 12:24, Denis Romanenko wrote:
>>> ...
>>>> hmm - for stabibility can i disable netconsole ? Or this bug is not
>>>> give my resutlts (reboot of router after 1-2 weeks of work )?
>> Hmm... To make sure: you have these reboots without netconsole, right?
>>
> 
> on old kernels - (without patch #4 from you) (2.6.27.3-2.6.27.7) - its  
> rebooted without netconsole

OK, there was this hrtimers vs. htb lockup bug.

> Now netconsole enabled and loaded by default

So try to check without netconcole too.

Jarek P.

PS: and try to Cc me sometimes, btw...

Re: Kernel problem

[Jarek Poplawski]

On Thu, Feb 26, 2009 at 09:06:31PM +0800, Herbert Xu wrote:
> On Thu, Feb 26, 2009 at 04:10:37AM -0800, David Miller wrote:
> > 
> > netpoll supports receiving packets and has a trap handler to eat them
> > to avoid packets it is interested in going into the real stack.
> > (netpoll_receive_skb, netpoll_rx_skb, etc.)
> 
> OK after much head scratching and staring, it turns out that
> this is caused by the VLAN path not doing the netpoll check
> which would normally drop the packets when a printk is active.

Yes, nice catch/scratch!

Cheers,
Jarek P.

Re: Kernel problem

[Herbert Xu]

On Thu, Feb 26, 2009 at 09:06:31PM +0800, Herbert Xu wrote:
>
> OK after much head scratching and staring, it turns out that
> this is caused by the VLAN path not doing the netpoll check
> which would normally drop the packets when a printk is active.

Note that this patch doesn't apply to net-next-2.6 as GRO has
changed there.  But it should be easy to manually slot it in.
Let me know if there are any problems.

netpoll: Add drop checks to all entry points

The netpoll entry checks are required to ensure that we don't
receive normal packets when invoked via netpoll.  Unfortunately
it only ever worked for the netif_receive_skb/netif_rx entry
points.  The VLAN (and subsequently GRO) entry point didn't
have the check and therefore can trigger all sorts of weird
problems.

This patch adds the netpoll check to all entry points.

I'm still uneasy with receiving at all under netpoll (which
apparently is only used by the out-of-tree kdump code).  The
reason is it is perfectly legal to receive all data including
headers into highmem if netpoll is off, but if you try to do
that with netpoll on and someone gets a printk in an IRQ handler
you're going to get a nice BUG_ON.

Fortunately I don't think anyone is receiving everything into
highmem as it stands.

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 e9db889..a37782d 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -1,12 +1,16 @@
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
+#include <linux/netpoll.h>
 #include "vlan.h"
 
 /* VLAN rx hw acceleration helper.  This acts like netif_{rx,receive_skb}(). */
 int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
 		      u16 vlan_tci, int polling)
 {
+	if (netpoll_receive_skb(skb))
+		return NET_RX_DROP;
+
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -100,6 +104,9 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 {
 	int err = NET_RX_SUCCESS;
 
+	if (netpoll_receive_skb(skb))
+		return NET_RX_DROP;
+
 	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
 	case -1:
 		return netif_receive_skb(skb);
@@ -126,6 +133,9 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		goto out;
 
+	if (netpoll_receive_skb(skb))
+		goto out;
+
 	err = NET_RX_SUCCESS;
 
 	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
diff --git a/net/core/dev.c b/net/core/dev.c
index a17e006..72b0d26 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2488,6 +2488,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 
 int napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
+	if (netpoll_receive_skb(skb))
+		return NET_RX_DROP;
+
 	switch (__napi_gro_receive(napi, skb)) {
 	case -1:
 		return netif_receive_skb(skb);
@@ -2558,6 +2561,9 @@ int napi_gro_frags(struct napi_struct *napi, struct napi_gro_fraginfo *info)
 	if (!skb)
 		goto out;
 
+	if (netpoll_receive_skb(skb))
+		goto out;
+
 	err = NET_RX_SUCCESS;
 
 	switch (__napi_gro_receive(napi, skb)) {

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 problem

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 27 Feb 2009 12:11:36 +0800

>  /* VLAN rx hw acceleration helper.  This acts like netif_{rx,receive_skb}(). */
>  int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
>  		      u16 vlan_tci, int polling)
>  {
> +	if (netpoll_receive_skb(skb))
> +		return NET_RX_DROP;
> +
>  	if (skb_bond_should_drop(skb))
>  		goto drop;
>  

This case should use netpoll_rx().

netif_rx_skb() --> netpoll_rx()
netif_receive_skb() --> netpoll_receive_skb()

Re: Kernel problem

[Jarek Poplawski]

On Fri, Feb 27, 2009 at 12:11:36PM +0800, Herbert Xu wrote:
> On Thu, Feb 26, 2009 at 09:06:31PM +0800, Herbert Xu wrote:
> >
> > OK after much head scratching and staring, it turns out that
> > this is caused by the VLAN path not doing the netpoll check
> > which would normally drop the packets when a printk is active.
> 
> Note that this patch doesn't apply to net-next-2.6 as GRO has
> changed there.  But it should be easy to manually slot it in.
> Let me know if there are any problems.
> 
> netpoll: Add drop checks to all entry points
> 
> The netpoll entry checks are required to ensure that we don't
> receive normal packets when invoked via netpoll.  Unfortunately
> it only ever worked for the netif_receive_skb/netif_rx entry
> points.  The VLAN (and subsequently GRO) entry point didn't
> have the check and therefore can trigger all sorts of weird
> problems.
> 
> This patch adds the netpoll check to all entry points.

Probably I miss something, but I'm not sure it's really necessary in
all (non-VLAN) entry points. Of course it's an optimization to drop
these things early, but there is a lot off mess with replicating
various parts of netif_receive_skb() in so many places.

As a matter of fact, I wonder why it can't be done in one place, e.g.
netif_nit_deliver(), which was created partly for similar problems.

Jarek P.

PS: it would be nice to prepare a 2.6.27 version for testing yet; it
looks like needed for -stable.

> 
> I'm still uneasy with receiving at all under netpoll (which
> apparently is only used by the out-of-tree kdump code).  The
> reason is it is perfectly legal to receive all data including
> headers into highmem if netpoll is off, but if you try to do
> that with netpoll on and someone gets a printk in an IRQ handler
> you're going to get a nice BUG_ON.
> 
> Fortunately I don't think anyone is receiving everything into
> highmem as it stands.
> 
> 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 e9db889..a37782d 100644
> --- a/net/8021q/vlan_core.c
> +++ b/net/8021q/vlan_core.c
> @@ -1,12 +1,16 @@
>  #include <linux/skbuff.h>
>  #include <linux/netdevice.h>
>  #include <linux/if_vlan.h>
> +#include <linux/netpoll.h>
>  #include "vlan.h"
>  
>  /* VLAN rx hw acceleration helper.  This acts like netif_{rx,receive_skb}(). */
>  int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
>  		      u16 vlan_tci, int polling)
>  {
> +	if (netpoll_receive_skb(skb))
> +		return NET_RX_DROP;
> +
>  	if (skb_bond_should_drop(skb))
>  		goto drop;
>  
> @@ -100,6 +104,9 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
>  {
>  	int err = NET_RX_SUCCESS;
>  
> +	if (netpoll_receive_skb(skb))
> +		return NET_RX_DROP;
> +
>  	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
>  	case -1:
>  		return netif_receive_skb(skb);
> @@ -126,6 +133,9 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
>  	if (!skb)
>  		goto out;
>  
> +	if (netpoll_receive_skb(skb))
> +		goto out;
> +
>  	err = NET_RX_SUCCESS;
>  
>  	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
> diff --git a/net/core/dev.c b/net/core/dev.c
> index a17e006..72b0d26 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2488,6 +2488,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
>  
>  int napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
>  {
> +	if (netpoll_receive_skb(skb))
> +		return NET_RX_DROP;
> +
>  	switch (__napi_gro_receive(napi, skb)) {
>  	case -1:
>  		return netif_receive_skb(skb);
> @@ -2558,6 +2561,9 @@ int napi_gro_frags(struct napi_struct *napi, struct napi_gro_fraginfo *info)
>  	if (!skb)
>  		goto out;
>  
> +	if (netpoll_receive_skb(skb))
> +		goto out;
> +
>  	err = NET_RX_SUCCESS;
>  
>  	switch (__napi_gro_receive(napi, skb)) {
> 
> 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 problem

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Fri, 27 Feb 2009 08:41:10 +0000

> Probably I miss something, but I'm not sure it's really necessary in
> all (non-VLAN) entry points. Of course it's an optimization to drop
> these things early, but there is a lot off mess with replicating
> various parts of netif_receive_skb() in so many places.
> 
> As a matter of fact, I wonder why it can't be done in one place, e.g.
> netif_nit_deliver(), which was created partly for similar problems.

I think we do need to hit all possible entry points.

How would you be able to handle it in netif_nit_deliver()?
Functions like netif_receive_skb() open-code the delivery to
network taps, they don't actually call netif_receive_skb().

Re: Kernel problem

[Jarek Poplawski]

On Fri, Feb 27, 2009 at 12:59:07AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Fri, 27 Feb 2009 08:41:10 +0000
> 
> > Probably I miss something, but I'm not sure it's really necessary in
> > all (non-VLAN) entry points. Of course it's an optimization to drop
> > these things early, but there is a lot off mess with replicating
> > various parts of netif_receive_skb() in so many places.
> > 
> > As a matter of fact, I wonder why it can't be done in one place, e.g.
> > netif_nit_deliver(), which was created partly for similar problems.
> 
> I think we do need to hit all possible entry points.
> 
> How would you be able to handle it in netif_nit_deliver()?
> Functions like netif_receive_skb() open-code the delivery to
> network taps, they don't actually call netif_receive_skb().

netif_nit_deliver() is a place called by vlan with orig skb->dev, so
it could be reused to check for netpoll btw. Of course, return value
should be added etc. and maybe name changed too. It could be
something like this:

netif_receive_skb()

	if (skb->vlan_tci) {
		ret = vlan_hwaccel_do_receive(skb);
		if (ret)
			return ret;
	}
...

Jarek P.

Re: Kernel problem

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Fri, 27 Feb 2009 09:12:16 +0000

> On Fri, Feb 27, 2009 at 12:59:07AM -0800, David Miller wrote:
> > From: Jarek Poplawski <jarkao2@gmail.com>
> > Date: Fri, 27 Feb 2009 08:41:10 +0000
> > 
> > > Probably I miss something, but I'm not sure it's really necessary in
> > > all (non-VLAN) entry points. Of course it's an optimization to drop
> > > these things early, but there is a lot off mess with replicating
> > > various parts of netif_receive_skb() in so many places.
> > > 
> > > As a matter of fact, I wonder why it can't be done in one place, e.g.
> > > netif_nit_deliver(), which was created partly for similar problems.
> > 
> > I think we do need to hit all possible entry points.
> > 
> > How would you be able to handle it in netif_nit_deliver()?
> > Functions like netif_receive_skb() open-code the delivery to
> > network taps, they don't actually call netif_receive_skb().
> 
> netif_nit_deliver() is a place called by vlan with orig skb->dev, so
> it could be reused to check for netpoll btw. Of course, return value
> should be added etc. and maybe name changed too. It could be
> something like this:

Note there is already a function that could do this and which needs to
hit all the same RX entrypoints just like this check would.

And that is skb_bond_should_drop().

We could rename that to skb_rx_should_drop() and put the netpoll
checks there.

There is some weird conditinalization of skb_bond_should_drop()'s call
in netif_receive_skb() but that should be easy to change to suit our
needs.  Perhaps by putting the calculation of the netdevice bonding
pointers into that function.

Re: Kernel problem

[Jarek Poplawski]

On Fri, Feb 27, 2009 at 01:16:15AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Fri, 27 Feb 2009 09:12:16 +0000
...
> > netif_nit_deliver() is a place called by vlan with orig skb->dev, so
> > it could be reused to check for netpoll btw. Of course, return value
> > should be added etc. and maybe name changed too. It could be
> > something like this:
> 
> Note there is already a function that could do this and which needs to
> hit all the same RX entrypoints just like this check would.
> 
> And that is skb_bond_should_drop().
> 
> We could rename that to skb_rx_should_drop() and put the netpoll
> checks there.
> 
> There is some weird conditinalization of skb_bond_should_drop()'s call
> in netif_receive_skb() but that should be easy to change to suit our
> needs.  Perhaps by putting the calculation of the netdevice bonding
> pointers into that function.

Yes, it would be nice to have it in this one place, but I guess
currently for vlans we depend on vlan_hwaccel_do_receive(), and there
are probably some reasons it's so far from the bond check.

Jarek P.

Re: Kernel problem

[Herbert Xu]

On Fri, Feb 27, 2009 at 12:03:50AM -0800, David Miller wrote:
> 
> This case should use netpoll_rx().

Good point.

> netif_rx_skb() --> netpoll_rx()
> netif_receive_skb() --> netpoll_receive_skb()

Actually I think we could just use netpoll_rx as netpoll_receive_skb
is just an optimisation that skips the test if we've already done it.

I'll do another version with combining the bonding check as you
suggested.

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 problem

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 27 Feb 2009 19:45:50 +0800

> On Fri, Feb 27, 2009 at 12:03:50AM -0800, David Miller wrote:
> > 
> > This case should use netpoll_rx().
> 
> Good point.
> 
> > netif_rx_skb() --> netpoll_rx()
> > netif_receive_skb() --> netpoll_receive_skb()
> 
> Actually I think we could just use netpoll_rx as netpoll_receive_skb
> is just an optimisation that skips the test if we've already done it.
> 
> I'll do another version with combining the bonding check as you
> suggested.

Thanks a lot Herbert.

Re: Kernel problem

[Herbert Xu]

On Fri, Feb 27, 2009 at 01:16:15AM -0800, David Miller wrote:
>
> There is some weird conditinalization of skb_bond_should_drop()'s call
> in netif_receive_skb() but that should be easy to change to suit our
> needs.  Perhaps by putting the calculation of the netdevice bonding
> pointers into that function.

I had a look at this and gave up quickly :) netif_receive_skb
is trying to deliver the packet to any ptype listeners bound
to this device if skb_bond_should_drop returns true.  This is
very different from what the other callers of skb_bond_should_drop
are trying to do.

So I'm opting for the easy way out right now.

netpoll: Add drop checks to all entry points

The netpoll entry checks are required to ensure that we don't
receive normal packets when invoked via netpoll.  Unfortunately
it only ever worked for the netif_receive_skb/netif_rx entry
points.  The VLAN (and subsequently GRO) entry point didn't
have the check and therefore can trigger all sorts of weird
problems.

This patch adds the netpoll check to all entry points.

I'm still uneasy with receiving at all under netpoll (which
apparently is only used by the out-of-tree kdump code).  The
reason is it is perfectly legal to receive all data including
headers into highmem if netpoll is off, but if you try to do
that with netpoll on and someone gets a printk in an IRQ handler                                             
you're going to get a nice BUG_ON.

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 e9db889..2886d2f 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -1,12 +1,16 @@
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
 #include <linux/if_vlan.h>
+#include <linux/netpoll.h>
 #include "vlan.h"
 
 /* VLAN rx hw acceleration helper.  This acts like netif_{rx,receive_skb}(). */
 int __vlan_hwaccel_rx(struct sk_buff *skb, struct vlan_group *grp,
 		      u16 vlan_tci, int polling)
 {
+	if (netpoll_rx(skb))
+		return NET_RX_DROP;
+
 	if (skb_bond_should_drop(skb))
 		goto drop;
 
@@ -100,6 +104,9 @@ int vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
 {
 	int err = NET_RX_SUCCESS;
 
+	if (netpoll_receive_skb(skb))
+		return NET_RX_DROP;
+
 	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
 	case -1:
 		return netif_receive_skb(skb);
@@ -126,6 +133,9 @@ int vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
 	if (!skb)
 		goto out;
 
+	if (netpoll_receive_skb(skb))
+		goto out;
+
 	err = NET_RX_SUCCESS;
 
 	switch (vlan_gro_common(napi, grp, vlan_tci, skb)) {
diff --git a/net/core/dev.c b/net/core/dev.c
index a17e006..72b0d26 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2488,6 +2488,9 @@ static int __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 
 int napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
+	if (netpoll_receive_skb(skb))
+		return NET_RX_DROP;
+
 	switch (__napi_gro_receive(napi, skb)) {
 	case -1:
 		return netif_receive_skb(skb);
@@ -2558,6 +2561,9 @@ int napi_gro_frags(struct napi_struct *napi, struct napi_gro_fraginfo *info)
 	if (!skb)
 		goto out;
 
+	if (netpoll_receive_skb(skb))
+		goto out;
+
 	err = NET_RX_SUCCESS;
 
 	switch (__napi_gro_receive(napi, skb)) {

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 problem

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Sun, 1 Mar 2009 15:38:01 +0800

> netpoll: Add drop checks to all entry points
> 
> The netpoll entry checks are required to ensure that we don't
> receive normal packets when invoked via netpoll.  Unfortunately
> it only ever worked for the netif_receive_skb/netif_rx entry
> points.  The VLAN (and subsequently GRO) entry point didn't
> have the check and therefore can trigger all sorts of weird
> problems.
> 
> This patch adds the netpoll check to all entry points.
> 
> I'm still uneasy with receiving at all under netpoll (which
> apparently is only used by the out-of-tree kdump code).  The
> reason is it is perfectly legal to receive all data including
> headers into highmem if netpoll is off, but if you try to do
> that with netpoll on and someone gets a printk in an IRQ handler                                             
> you're going to get a nice BUG_ON.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

I don't blame you for being skitting about skb_bond_should_drop()
:-)

Applied, thanks Herbert!