* [PATCH 0/3] fix reuseaddr regression
@ 2017-09-23 0:20 Josef Bacik
2017-09-23 0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel
I introduced a regression when reworking the fastreuse port stuff that allows
bind conflicts to occur once a reuseaddr 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 could you please queue these changes up for -stable, I've run them through
the net tests and added another test to check for this problem specifically.
Thanks,
Josef
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] net: set tb->fast_sk_family
2017-09-23 0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
@ 2017-09-23 0:20 ` Josef Bacik
2017-09-23 0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
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.
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets
2017-09-23 0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
2017-09-23 0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
@ 2017-09-23 0:20 ` Josef Bacik
2017-09-23 0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
2017-09-23 3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
From: Josef Bacik <jbacik@fb.com>
In ipv6_rcv_saddr_equal() we need to use inet6_rcv_saddr(sk) for the
ipv6 compare with the fast socket information to make sure we're doing
the proper comparisons.
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, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f87f4805e244..a1bf30438bc5 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -266,7 +266,7 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
#if IS_ENABLED(CONFIG_IPV6)
if (tb->fast_sk_family == AF_INET6)
return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
- &sk->sk_v6_rcv_saddr,
+ inet6_rcv_saddr(sk),
tb->fast_rcv_saddr,
sk->sk_rcv_saddr,
tb->fast_ipv6_only,
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/3] inet: fix improper empty comparison
2017-09-23 0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
2017-09-23 0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
2017-09-23 0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
@ 2017-09-23 0:20 ` Josef Bacik
2017-09-23 3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
3 siblings, 0 replies; 9+ messages in thread
From: Josef Bacik @ 2017-09-23 0:20 UTC (permalink / raw)
To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik
From: Josef Bacik <jbacik@fb.com>
When doing my reuseport rework I screwed up and changed a
if (hlist_empty(&tb->owners))
to
if (!hlist_empty(&tb->owners))
This is obviously bad as all of the reuseport/reuse logic was reversed,
which caused weird problems like allowing an ipv4 bind conflict if we
opened an ipv4 only socket on a port followed by an ipv6 only socket on
the same port.
Fixes: b9470c27607b ("inet: kill smallest_size and smallest_port")
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
net/ipv4/inet_connection_sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index a1bf30438bc5..c039c937ba90 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -321,7 +321,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
goto fail_unlock;
}
success:
- if (!hlist_empty(&tb->owners)) {
+ if (hlist_empty(&tb->owners)) {
tb->fastreuse = reuse;
if (sk->sk_reuseport) {
tb->fastreuseport = FASTREUSEPORT_ANY;
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/3] fix reuseaddr regression
2017-09-23 0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
` (2 preceding siblings ...)
2017-09-23 0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
@ 2017-09-23 3:40 ` David Miller
3 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-09-23 3:40 UTC (permalink / raw)
To: josef; +Cc: netdev, kernel-team, linux-kernel
From: Josef Bacik <josef@toxicpanda.com>
Date: Fri, 22 Sep 2017 20:20:05 -0400
> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr 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 could you please queue these changes up for -stable, I've run them through
> the net tests and added another test to check for this problem specifically.
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/3] fix reuseaddr regression
@ 2017-09-18 16:28 josef
2017-09-18 17:44 ` Cole Robinson
2017-09-19 20:50 ` David Miller
0 siblings, 2 replies; 9+ messages in thread
From: josef @ 2017-09-18 16:28 UTC (permalink / raw)
To: davem, netdev, linux-kernel, crobinso, labbott, kernel-team
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] fix reuseaddr regression
2017-09-18 16:28 josef
@ 2017-09-18 17:44 ` Cole Robinson
2017-09-19 20:50 ` David Miller
1 sibling, 0 replies; 9+ messages in thread
From: Cole Robinson @ 2017-09-18 17:44 UTC (permalink / raw)
To: josef, davem, netdev, linux-kernel, labbott, kernel-team
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] fix reuseaddr regression
2017-09-18 16:28 josef
2017-09-18 17:44 ` Cole Robinson
@ 2017-09-19 20:50 ` David Miller
2017-09-23 0:28 ` Josef Bacik
1 sibling, 1 reply; 9+ messages in thread
From: David Miller @ 2017-09-19 20:50 UTC (permalink / raw)
To: josef; +Cc: netdev, linux-kernel, crobinso, labbott, kernel-team
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] fix reuseaddr regression
2017-09-19 20:50 ` David Miller
@ 2017-09-23 0:28 ` Josef Bacik
0 siblings, 0 replies; 9+ messages in thread
From: Josef Bacik @ 2017-09-23 0:28 UTC (permalink / raw)
To: David Miller; +Cc: josef, netdev, linux-kernel, crobinso, labbott, kernel-team
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-09-23 3:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-23 0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
2017-09-23 0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
2017-09-23 0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
2017-09-23 0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
2017-09-23 3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
-- strict thread matches above, loose matches on Subject: below --
2017-09-18 16:28 josef
2017-09-18 17:44 ` Cole Robinson
2017-09-19 20:50 ` David Miller
2017-09-23 0:28 ` Josef Bacik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.