* [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too @ 2010-10-11 15:20 Andrey Vagin 2010-10-11 15:40 ` Michael Tokarev 0 siblings, 1 reply; 13+ messages in thread From: Andrey Vagin @ 2010-10-11 15:20 UTC (permalink / raw) To: stable; +Cc: netdev, Krishna Kumar, David S. Miller, Andrey Vagin 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 Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> Acked-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Andrey Vagin <avagin@openvz.org> --- net/core/dev.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) 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; -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 15:20 [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too Andrey Vagin @ 2010-10-11 15:40 ` Michael Tokarev 2010-10-11 15:46 ` Eric Dumazet 0 siblings, 1 reply; 13+ messages in thread From: Michael Tokarev @ 2010-10-11 15:40 UTC (permalink / raw) To: Andrey Vagin; +Cc: stable, netdev, Krishna Kumar, David S. Miller 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 15:40 ` Michael Tokarev @ 2010-10-11 15:46 ` Eric Dumazet 2010-10-11 15:59 ` David Miller 0 siblings, 1 reply; 13+ messages in thread From: Eric Dumazet @ 2010-10-11 15:46 UTC (permalink / raw) To: Michael Tokarev Cc: Andrey Vagin, stable, netdev, Krishna Kumar, David S. Miller 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 ? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 15:46 ` Eric Dumazet @ 2010-10-11 15:59 ` David Miller 2010-10-11 16:19 ` Andrew Vagin 0 siblings, 1 reply; 13+ messages in thread From: David Miller @ 2010-10-11 15:59 UTC (permalink / raw) To: eric.dumazet; +Cc: mjt, avagin, stable, netdev, krkumar2 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 15:59 ` David Miller @ 2010-10-11 16:19 ` Andrew Vagin 2010-10-11 16:30 ` Eric Dumazet 0 siblings, 1 reply; 13+ messages in thread From: Andrew Vagin @ 2010-10-11 16:19 UTC (permalink / raw) To: David Miller; +Cc: eric.dumazet, mjt, avagin, stable, netdev, krkumar2 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 16:19 ` Andrew Vagin @ 2010-10-11 16:30 ` Eric Dumazet 2010-10-11 16:55 ` Michael Tokarev 2010-10-26 18:54 ` David Miller 0 siblings, 2 replies; 13+ messages in thread From: Eric Dumazet @ 2010-10-11 16:30 UTC (permalink / raw) To: avagin; +Cc: David Miller, mjt, avagin, stable, netdev, krkumar2 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 ! ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 16:30 ` Eric Dumazet @ 2010-10-11 16:55 ` Michael Tokarev 2010-10-26 18:54 ` David Miller 1 sibling, 0 replies; 13+ messages in thread From: Michael Tokarev @ 2010-10-11 16:55 UTC (permalink / raw) To: Eric Dumazet; +Cc: avagin, David Miller, avagin, stable, netdev, krkumar2 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-11 16:30 ` Eric Dumazet 2010-10-11 16:55 ` Michael Tokarev @ 2010-10-26 18:54 ` David Miller 2010-12-08 22:47 ` avagin 1 sibling, 1 reply; 13+ messages in thread From: David Miller @ 2010-10-26 18:54 UTC (permalink / raw) To: eric.dumazet; +Cc: avagin, mjt, avagin, stable, netdev, krkumar2 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> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-10-26 18:54 ` David Miller @ 2010-12-08 22:47 ` avagin 2010-12-08 23:05 ` Greg KH 0 siblings, 1 reply; 13+ messages in thread From: avagin @ 2010-12-08 22:47 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: David Miller, eric.dumazet, mjt, avagin, stable, netdev, krkumar2 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> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-12-08 22:47 ` avagin @ 2010-12-08 23:05 ` Greg KH 2010-12-09 6:43 ` [stable] " avagin 0 siblings, 1 reply; 13+ messages in thread From: Greg KH @ 2010-12-08 23:05 UTC (permalink / raw) To: avagin@gmail.com Cc: krkumar2, avagin, eric.dumazet, netdev, Greg Kroah-Hartman, mjt, stable, David Miller 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [stable] [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-12-08 23:05 ` Greg KH @ 2010-12-09 6:43 ` avagin 2010-12-09 18:19 ` Greg KH 0 siblings, 1 reply; 13+ messages in thread From: avagin @ 2010-12-09 6:43 UTC (permalink / raw) To: Greg KH Cc: Greg Kroah-Hartman, krkumar2, avagin, eric.dumazet, netdev, mjt, David Miller, stable [-- Attachment #1: Type: text/plain, Size: 288 bytes --] I add the patch in attachments On 12/09/2010 02:05 AM, Greg KH wrote: > 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 [-- Attachment #2: Attached Message --] [-- Type: message/rfc822, Size: 3214 bytes --] From: Andrey Vagin <avagin@openvz.org> To: stable@kernel.org Cc: netdev@vger.kernel.org, Krishna Kumar <krkumar2@in.ibm.com>, "David S. Miller" <davem@davemloft.net>, Andrey Vagin <avagin@openvz.org> Subject: [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too Date: Mon, 11 Oct 2010 19:20:13 +0400 Message-ID: <1286810413-30238-1-git-send-email-avagin@openvz.org> 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 Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com> Acked-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Andrey Vagin <avagin@openvz.org> --- net/core/dev.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) 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; -- 1.7.2.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [stable] [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-12-09 6:43 ` [stable] " avagin @ 2010-12-09 18:19 ` Greg KH 2010-12-21 0:07 ` Greg KH 0 siblings, 1 reply; 13+ messages in thread From: Greg KH @ 2010-12-09 18:19 UTC (permalink / raw) To: avagin@gmail.com Cc: Greg Kroah-Hartman, krkumar2, avagin, eric.dumazet, netdev, mjt, David Miller, stable On Thu, Dec 09, 2010 at 09:43:03AM +0300, avagin@gmail.com wrote: > I add the patch in attachments Ok, thanks, I'll queue it up for the next .32 release after this one. greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [stable] [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too 2010-12-09 18:19 ` Greg KH @ 2010-12-21 0:07 ` Greg KH 0 siblings, 0 replies; 13+ messages in thread From: Greg KH @ 2010-12-21 0:07 UTC (permalink / raw) To: avagin@gmail.com Cc: Greg Kroah-Hartman, krkumar2, avagin, eric.dumazet, netdev, mjt, David Miller, stable On Thu, Dec 09, 2010 at 10:19:37AM -0800, Greg KH wrote: > On Thu, Dec 09, 2010 at 09:43:03AM +0300, avagin@gmail.com wrote: > > I add the patch in attachments > > Ok, thanks, I'll queue it up for the next .32 release after this one. Now applied. greg k-h ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-12-21 0:14 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-11 15:20 [STABLE 2.6.32 PATCH] net: release dst entry while cache-hot for GSO case too Andrey Vagin 2010-10-11 15:40 ` Michael Tokarev 2010-10-11 15:46 ` Eric Dumazet 2010-10-11 15:59 ` David Miller 2010-10-11 16:19 ` Andrew Vagin 2010-10-11 16:30 ` Eric Dumazet 2010-10-11 16:55 ` Michael Tokarev 2010-10-26 18:54 ` David Miller 2010-12-08 22:47 ` avagin 2010-12-08 23:05 ` Greg KH 2010-12-09 6:43 ` [stable] " avagin 2010-12-09 18:19 ` Greg KH 2010-12-21 0:07 ` Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).