| Message ID | 1286810413-30238-1-git-send-email-avagin@openvz.org |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
Andrey Vagin wrote: > From: Krishna Kumar <krkumar2@in.ibm.com> > > commit 068a2de57ddf4f472e32e7af868613c574ad1d88 upstream. > > Non-GSO code drops dst entry for performance reasons, but > the same is missing for GSO code. Drop dst while cache-hot > for GSO case too. > > Note: Without this patch the kernel may oops if used bridged veth > devices. A bridge set skb->dst = fake_dst_ops, veth transfers this skb > to netif_receive_skb...ip_rcv_finish and it calls dst_input(skb), but > fake_dst_ops->input = NULL -> Oops Hmm. Isn't this the reason for my mysterious OOPSes (jump to NULL) which I concluded are due to stack overflow? See f.e. http://www.spinics.net/lists/netdev/msg142104.html This started happening when I updated virtio drivers in a windows virtual machne to the ones which supports GSO, and my config involves bridging veth devices, and this is where the prob actually occurs - when doing guest => virtio => tap => bridge => veth route.... Thanks! /mjt -- 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
Le lundi 11 octobre 2010 à 19:40 +0400, Michael Tokarev a écrit : > Andrey Vagin wrote: > > From: Krishna Kumar <krkumar2@in.ibm.com> > > > > commit 068a2de57ddf4f472e32e7af868613c574ad1d88 upstream. > > > > Non-GSO code drops dst entry for performance reasons, but > > the same is missing for GSO code. Drop dst while cache-hot > > for GSO case too. > > > > Note: Without this patch the kernel may oops if used bridged veth > > devices. A bridge set skb->dst = fake_dst_ops, veth transfers this skb > > to netif_receive_skb...ip_rcv_finish and it calls dst_input(skb), but > > fake_dst_ops->input = NULL -> Oops > > Hmm. Isn't this the reason for my mysterious OOPSes (jump to > NULL) which I concluded are due to stack overflow? See f.e. > http://www.spinics.net/lists/netdev/msg142104.html > > This started happening when I updated virtio drivers in a windows > virtual machne to the ones which supports GSO, and my config > involves bridging veth devices, and this is where the prob > actually occurs - when doing guest => virtio => tap => bridge => veth > route.... > > Thanks! This patch was an optimization, not a bug fix. If something gets better, then a bug is somewhere else ? -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 11 Oct 2010 17:46:49 +0200 > This patch was an optimization, not a bug fix. Right, this has no business going into 2.6.32-stable at all. -- 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 10/11/2010 07:59 PM, David Miller wrote: > From: Eric Dumazet<eric.dumazet@gmail.com> > Date: Mon, 11 Oct 2010 17:46:49 +0200 > >> This patch was an optimization, not a bug fix. > Right, this has no business going into 2.6.32-stable at all. This is bug fix. Now nobody drops dst in case gso and veth, because the commit 60df914e295a21a223e43a7ee01e0c73c64dd111 deletes skb_dst_drop from the veth.c. We should commit my patch or revert commit 60df914e. We have two bug reports: http://www.spinics.net/lists/netdev/msg142104.html http://bugzilla.openvz.org/show_bug.cgi?id=1634 Taylan verified that the patch really fix his bug. -- 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
Le lundi 11 octobre 2010 à 20:19 +0400, Andrew Vagin a écrit : > On 10/11/2010 07:59 PM, David Miller wrote: > > From: Eric Dumazet<eric.dumazet@gmail.com> > > Date: Mon, 11 Oct 2010 17:46:49 +0200 > > > >> This patch was an optimization, not a bug fix. > > Right, this has no business going into 2.6.32-stable at all. > This is bug fix. Now nobody drops dst in case gso and veth, because the > commit 60df914e295a21a223e43a7ee01e0c73c64dd111 deletes skb_dst_drop > from the veth.c. We should commit my patch or revert commit 60df914e. > > We have two bug reports: > > http://www.spinics.net/lists/netdev/msg142104.html > > http://bugzilla.openvz.org/show_bug.cgi?id=1634 > > Taylan verified that the patch really fix his bug. Now that makes sense ;) This is always good to mention commit id of a bug origin. Because reading your patch (changelog and content), it was not obvious it fixed a bug. Thanks ! -- 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
11.10.2010 20:30, Eric Dumazet wrote: > Le lundi 11 octobre 2010 à 20:19 +0400, Andrew Vagin a écrit : >> On 10/11/2010 07:59 PM, David Miller wrote: >>> From: Eric Dumazet<eric.dumazet@gmail.com> >>> Date: Mon, 11 Oct 2010 17:46:49 +0200 >>> >>>> This patch was an optimization, not a bug fix. >>> Right, this has no business going into 2.6.32-stable at all. >> This is bug fix. Now nobody drops dst in case gso and veth, because the >> commit 60df914e295a21a223e43a7ee01e0c73c64dd111 deletes skb_dst_drop >> from the veth.c. We should commit my patch or revert commit 60df914e. >> >> We have two bug reports: >> >> http://www.spinics.net/lists/netdev/msg142104.html This is korg#17251: https://bugzilla.kernel.org/show_bug.cgi?id=17251 , from me. But I really wonder how that thing does not happen anymore when I disabled netfilter hooks... I can't experiment till weekend, but I'll try to get back to it and re-verify again, with and without this fix, with and without netfilter hooks. What I know for sure is 2 facts: I can't trigger the problem now (with the hooks disabled), and I can't trigger it on a subsequent kernel releases - e.g. 2.6.35 does not have the issue, but 2.6.32 has. >> http://bugzilla.openvz.org/show_bug.cgi?id=1634 Thanks! /mjt -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 11 Oct 2010 18:30:52 +0200 > Le lundi 11 octobre 2010 à 20:19 +0400, Andrew Vagin a écrit : >> On 10/11/2010 07:59 PM, David Miller wrote: >> > From: Eric Dumazet<eric.dumazet@gmail.com> >> > Date: Mon, 11 Oct 2010 17:46:49 +0200 >> > >> >> This patch was an optimization, not a bug fix. >> > Right, this has no business going into 2.6.32-stable at all. >> This is bug fix. Now nobody drops dst in case gso and veth, because the >> commit 60df914e295a21a223e43a7ee01e0c73c64dd111 deletes skb_dst_drop >> from the veth.c. We should commit my patch or revert commit 60df914e. >> >> We have two bug reports: >> >> http://www.spinics.net/lists/netdev/msg142104.html >> >> http://bugzilla.openvz.org/show_bug.cgi?id=1634 >> >> Taylan verified that the patch really fix his bug. > > Now that makes sense ;) In case there is any doubt about this -stable patch submission: Acked-by: David S. Miller <davem@davemloft.net> -- 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
Hi Greg, This patch is acked by David S. Miller. Greg, maybe you can commit it? On 10/26/2010 10:54 PM, David Miller wrote: > From: Eric Dumazet<eric.dumazet@gmail.com> > Date: Mon, 11 Oct 2010 18:30:52 +0200 > >> Le lundi 11 octobre 2010 à 20:19 +0400, Andrew Vagin a écrit : >>> On 10/11/2010 07:59 PM, David Miller wrote: >>>> From: Eric Dumazet<eric.dumazet@gmail.com> >>>> Date: Mon, 11 Oct 2010 17:46:49 +0200 >>>> >>>>> This patch was an optimization, not a bug fix. >>>> Right, this has no business going into 2.6.32-stable at all. >>> This is bug fix. Now nobody drops dst in case gso and veth, because the >>> commit 60df914e295a21a223e43a7ee01e0c73c64dd111 deletes skb_dst_drop >>> from the veth.c. We should commit my patch or revert commit 60df914e. >>> >>> We have two bug reports: >>> >>> http://www.spinics.net/lists/netdev/msg142104.html >>> >>> http://bugzilla.openvz.org/show_bug.cgi?id=1634 >>> >>> Taylan verified that the patch really fix his bug. >> >> Now that makes sense ;) > > In case there is any doubt about this -stable patch submission: > > Acked-by: David S. Miller<davem@davemloft.net> -- 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 Thu, Dec 09, 2010 at 01:47:41AM +0300, avagin@gmail.com wrote: > Hi Greg, > > This patch is acked by David S. Miller. Greg, maybe you can commit it? What specific patch please? confused, greg k-h -- 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
diff --git a/net/core/dev.c b/net/core/dev.c index 915d0ae..c325ab6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1747,6 +1747,14 @@ gso: skb->next = nskb->next; nskb->next = NULL; + + /* + * If device doesnt need nskb->dst, release it right now while + * its hot in this cpu cache + */ + if (dev->priv_flags & IFF_XMIT_DST_RELEASE) + skb_dst_drop(nskb); + rc = ops->ndo_start_xmit(nskb, dev); if (unlikely(rc != NETDEV_TX_OK)) { nskb->next = skb->next;