| Message ID | 1378580577.26319.26.camel@edumazet-glaptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 09/07/2013 12:02 PM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > commit 416186fbf8c5b4e4465 ("net: Split core bits of netdev_pick_tx > into __netdev_pick_tx") added a bug that disables caching of queue > index in the socket. > > This is the source of packet reorders for TCP flows, and > again this is happening more often when using FQ pacing. > > Old code was doing > > if (queue_index != old_index) > sk_tx_queue_set(sk, queue_index); > > Alexander renamed the variables but forgot to change sk_tx_queue_set() > 2nd parameter. > > if (queue_index != new_index) > sk_tx_queue_set(sk, queue_index); > > This means we store -1 over and over in sk->sk_tx_queue_mapping > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Alexander Duyck <alexander.h.duyck@intel.com> > --- > net/core/flow_dissector.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c > index 0ff42f0..1929af8 100644 > --- a/net/core/flow_dissector.c > +++ b/net/core/flow_dissector.c > @@ -352,7 +352,7 @@ u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb) > > if (queue_index != new_index && sk && > rcu_access_pointer(sk->sk_dst_cache)) > - sk_tx_queue_set(sk, queue_index); > + sk_tx_queue_set(sk, new_index); > > queue_index = new_index; > } > > > -- Ugh, my bad. This is a nasty one too since the behaviour appeared to be correct for most cases. It looks like this needs to go into stable for 3.9 - 3.11. Acked-by: Alexander Duyck <alexander.h.duyck@intel.com> -- 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: Alexander Duyck <alexander.duyck@gmail.com> Date: Sat, 07 Sep 2013 21:08:59 -0700 > On 09/07/2013 12:02 PM, Eric Dumazet wrote: >> From: Eric Dumazet <edumazet@google.com> >> >> commit 416186fbf8c5b4e4465 ("net: Split core bits of netdev_pick_tx >> into __netdev_pick_tx") added a bug that disables caching of queue >> index in the socket. >> >> This is the source of packet reorders for TCP flows, and >> again this is happening more often when using FQ pacing. >> >> Old code was doing >> >> if (queue_index != old_index) >> sk_tx_queue_set(sk, queue_index); >> >> Alexander renamed the variables but forgot to change sk_tx_queue_set() >> 2nd parameter. >> >> if (queue_index != new_index) >> sk_tx_queue_set(sk, queue_index); >> >> This means we store -1 over and over in sk->sk_tx_queue_mapping >> >> Signed-off-by: Eric Dumazet <edumazet@google.com> ... > Ugh, my bad. This is a nasty one too since the behaviour appeared to be > correct for most cases. > > It looks like this needs to go into stable for 3.9 - 3.11. > > Acked-by: Alexander Duyck <alexander.h.duyck@intel.com> > Applied and queued up for -stable, 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
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 0ff42f0..1929af8 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -352,7 +352,7 @@ u16 __netdev_pick_tx(struct net_device *dev, struct sk_buff *skb) if (queue_index != new_index && sk && rcu_access_pointer(sk->sk_dst_cache)) - sk_tx_queue_set(sk, queue_index); + sk_tx_queue_set(sk, new_index); queue_index = new_index; }