| Message ID | 1271358783.16881.2949.camel@edumazet-laptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 15 Apr 2010 21:13:03 +0200 > [PATCH] ip: Fix ip_dev_loopback_xmit() Applied to net-2.6, thanks Eric. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 16, 2010 at 5:26 AM, David Miller <davem@davemloft.net> wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Thu, 15 Apr 2010 21:13:03 +0200 > >> [PATCH] ip: Fix ip_dev_loopback_xmit() > > Applied to net-2.6, thanks Eric. Now, I am doubting the correctness of the following comment: /* * The higher levels take care of making this non-reentrant (it's * called with bh's disabled). */ static netdev_tx_t loopback_xmit(struct sk_buff *skb, struct net_device *dev) { struct pcpu_lstats __percpu *pcpu_lstats; struct pcpu_lstats *lb_stats; int len; skb_orphan(skb); skb->protocol = eth_type_trans(skb, dev); And these lines: /* it's OK to use per_cpu_ptr() because BHs are off */ pcpu_lstats = (void __percpu __force *)dev->ml_priv; lb_stats = this_cpu_ptr(pcpu_lstats);
From: Changli Gao <xiaosuo@gmail.com> Date: Fri, 16 Apr 2010 08:03:59 +0800 > On Fri, Apr 16, 2010 at 5:26 AM, David Miller <davem@davemloft.net> wrote: >> From: Eric Dumazet <eric.dumazet@gmail.com> >> Date: Thu, 15 Apr 2010 21:13:03 +0200 >> >>> [PATCH] ip: Fix ip_dev_loopback_xmit() >> >> Applied to net-2.6, thanks Eric. > > > Now, I am doubting the correctness of the following comment: The ->hard_start_xmit() method always executes with software interrupts disabled. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 16, 2010 at 8:15 AM, David Miller <davem@davemloft.net> wrote: > > The ->hard_start_xmit() method always executes with software > interrupts disabled. > Oh, sorry. I traced to the wrong function.
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index c65f18e..d1bcc9f 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -120,7 +120,7 @@ static int ip_dev_loopback_xmit(struct sk_buff *newskb) newskb->pkt_type = PACKET_LOOPBACK; newskb->ip_summed = CHECKSUM_UNNECESSARY; WARN_ON(!skb_dst(newskb)); - netif_rx(newskb); + netif_rx_ni(newskb); return 0; } diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 16c4391..65f9c37 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -108,7 +108,7 @@ static int ip6_dev_loopback_xmit(struct sk_buff *newskb) newskb->ip_summed = CHECKSUM_UNNECESSARY; WARN_ON(!skb_dst(newskb)); - netif_rx(newskb); + netif_rx_ni(newskb); return 0; }
[PATCH] ip: Fix ip_dev_loopback_xmit() Eric Paris got following trace with a linux-next kernel [ 14.203970] BUG: using smp_processor_id() in preemptible [00000000] code: avahi-daemon/2093 [ 14.204025] caller is netif_rx+0xfa/0x110 [ 14.204035] Call Trace: [ 14.204064] [<ffffffff81278fe5>] debug_smp_processor_id+0x105/0x110 [ 14.204070] [<ffffffff8142163a>] netif_rx+0xfa/0x110 [ 14.204090] [<ffffffff8145b631>] ip_dev_loopback_xmit+0x71/0xa0 [ 14.204095] [<ffffffff8145b892>] ip_mc_output+0x192/0x2c0 [ 14.204099] [<ffffffff8145d610>] ip_local_out+0x20/0x30 [ 14.204105] [<ffffffff8145d8ad>] ip_push_pending_frames+0x28d/0x3d0 [ 14.204119] [<ffffffff8147f1cc>] udp_push_pending_frames+0x14c/0x400 [ 14.204125] [<ffffffff814803fc>] udp_sendmsg+0x39c/0x790 [ 14.204137] [<ffffffff814891d5>] inet_sendmsg+0x45/0x80 [ 14.204149] [<ffffffff8140af91>] sock_sendmsg+0xf1/0x110 [ 14.204189] [<ffffffff8140dc6c>] sys_sendmsg+0x20c/0x380 [ 14.204233] [<ffffffff8100ad82>] system_call_fastpath+0x16/0x1b While current linux-2.6 kernel doesnt emit this warning, bug is latent and might cause unexpected failures. ip_dev_loopback_xmit() runs in process context, preemption enabled, so must call netif_rx_ni() instead of netif_rx(), to make sure that we process pending software interrupt. Same change for ip6_dev_loopback_xmit() Reported-by: Eric Paris <eparis@redhat.com> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/ipv4/ip_output.c | 2 +- net/ipv6/ip6_output.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html