Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1734281 > unrolled thread

[PATCH 0/3] fix reuseaddr regression

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] fix reuseaddr regression josef@toxicpanda.com - 2017-09-18 18:30 +0200
    [PATCH 1/3] net: set tb->fast_sk_family josef@toxicpanda.com - 2017-09-18 18:40 +0200
    Re: [PATCH 0/3] fix reuseaddr regression Cole Robinson <crobinso@redhat.com> - 2017-09-18 19:50 +0200
    Re: [PATCH 0/3] fix reuseaddr regression David Miller <davem@davemloft.net> - 2017-09-19 23:00 +0200
      Re: [PATCH 0/3] fix reuseaddr regression Josef Bacik <josef@toxicpanda.com> - 2017-09-23 02:30 +0200

#1734281 — [PATCH 0/3] fix reuseaddr regression

I introduced a regression when reworking the fastreuse port stuff that allows
bind conflicts to occur once a reuseaddr socket successfully opens on an
existing tb.  The root cause is I reversed an if statement which caused us to
set the tb as if there were no owners on the socket if there were, which
obviously is not correct.

Dave I have follow up patches that will add a selftest for this case and I ran
the other reuseport related tests as well.  These need to go in pretty quickly
as it breaks kvm, I've marked them for stable.  Sorry for the regression,

Josef

[toc] | [next] | [standalone]

#1734290 — [PATCH 1/3] net: set tb->fast_sk_family

From: Josef Bacik <jbacik@fb.com>

We need to set the tb->fast_sk_family properly so we can use the proper
comparison function for all subsequent reuseport bind requests.

Cc: stable@vger.kernel.org
Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index b9c64b40a83a..f87f4805e244 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -328,6 +328,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 			tb->fastuid = uid;
 			tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 			tb->fast_ipv6_only = ipv6_only_sock(sk);
+			tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 			tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
@@ -354,6 +355,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 				tb->fastuid = uid;
 				tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 				tb->fast_ipv6_only = ipv6_only_sock(sk);
+				tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 				tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1734333

On 09/18/2017 12:28 PM, josef@toxicpanda.com wrote:
> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr socket successfully opens on an
> existing tb.  The root cause is I reversed an if statement which caused us to
> set the tb as if there were no owners on the socket if there were, which
> obviously is not correct.
> 
> Dave I have follow up patches that will add a selftest for this case and I ran
> the other reuseport related tests as well.  These need to go in pretty quickly
> as it breaks kvm, I've marked them for stable.  Sorry for the regression,
> 

To clarify, it doesn't really break KVM specifically, but it breaks a
port collision detection idiom that libvirt depends on to successfully
launch qemu/xen/... VMs in certain cases.

Thanks,
Cole

[toc] | [prev] | [next] | [standalone]

#1735255

From: josef@toxicpanda.com
Date: Mon, 18 Sep 2017 12:28:54 -0400

> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr socket successfully opens on an
> existing tb.  The root cause is I reversed an if statement which caused us to
> set the tb as if there were no owners on the socket if there were, which
> obviously is not correct.
> 
> Dave I have follow up patches that will add a selftest for this case and I ran
> the other reuseport related tests as well.  These need to go in pretty quickly
> as it breaks kvm, I've marked them for stable.  Sorry for the regression,

First, please fix your "From: " field so that it actually has your full
name rather than just your email address.  This matter when I apply
your patches.

Second, remove the stable CC:.  For networking changes, you simply ask
me to queue the changes up for -stable.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1737936

On Tue, Sep 19, 2017 at 01:50:56PM -0700, David Miller wrote:
> From: josef@toxicpanda.com
> Date: Mon, 18 Sep 2017 12:28:54 -0400
> 
> > I introduced a regression when reworking the fastreuse port stuff that allows
> > bind conflicts to occur once a reuseaddr socket successfully opens on an
> > existing tb.  The root cause is I reversed an if statement which caused us to
> > set the tb as if there were no owners on the socket if there were, which
> > obviously is not correct.
> > 
> > Dave I have follow up patches that will add a selftest for this case and I ran
> > the other reuseport related tests as well.  These need to go in pretty quickly
> > as it breaks kvm, I've marked them for stable.  Sorry for the regression,
> 
> First, please fix your "From: " field so that it actually has your full
> name rather than just your email address.  This matter when I apply
> your patches.
> 
> Second, remove the stable CC:.  For networking changes, you simply ask
> me to queue the changes up for -stable.
> 

Sorry Dave, I've fixed my git email settings and I droped the stable cc and sent
a new round.  Didn't see this until just now, my bad.

Josef

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web