All of lore.kernel.org
 help / color / mirror / Atom feed
* 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
@ 2017-09-12 22:35 Laura Abbott
  2017-09-12 23:12 ` Josef Bacik
  0 siblings, 1 reply; 14+ messages in thread
From: Laura Abbott @ 2017-09-12 22:35 UTC (permalink / raw)
  To: Josef Bacik, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Cole Robinson

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

Hi,

Fedora got a bug report 
https://bugzilla.redhat.com/show_bug.cgi?id=1432684 of a regression with 
automatic spice port
assignment. The libvirt team reduced this to the attached test
case run as follows:

In a separate terminal, qemu-kvm -vnc 127.0.0.1:0 to grab port 5900. 
Then do this:

$ gcc bind-collision.c && ./a.out
bind: Address already in use
AF_INET check failed.
$ gcc -D CHECK_IPV6 bind-collision.c && ./a.out
AF_INET6 success
AF_INET success
$ gcc bind-collision.c && ./a.out
AF_INET success

Bisection showed this behavior to be caused by

commit 319554f284dda9f2737d09df82ba3610bd8ddea3
Author: Josef Bacik <jbacik@fb.com>
Date:   Thu Jan 19 17:47:46 2017 -0500

     inet: don't use sk_v6_rcv_saddr directly

     When comparing two sockets we need to use inet6_rcv_saddr so we get 
a NULL
     sk_v6_rcv_saddr if the socket isn't AF_INET6, otherwise our 
comparison function
     can be wrong.

     Fixes: 637bc8b ("inet: reset tb->fastreuseport when adding a 
reuseport sk")
     Signed-off-by: Josef Bacik <jbacik@fb.com>
     Signed-off-by: David S. Miller <davem@davemloft.net>


And reverting fixed both the standalone test case and the spice issue.

Any ideas?

Thanks,
Laura

[-- Attachment #2: bind-collision.c --]
[-- Type: text/x-csrc, Size: 2024 bytes --]

#include <arpa/inet.h>
#include <netinet/in.h>
#include <stdbool.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <unistd.h>

/* Reproducer for https://bugzilla.redhat.com/show_bug.cgi?id=1432684
   Simply do something like: qemu-kvm -vnc 127.0.0.1:0
 */

#define PORT 5900

int check_port(int family) {
    int fd = -1;
    int reuseaddr = 1;
    int v6only = 1;
    int addrlen;
    int ret = -1;
    bool ipv6 = false;
    struct sockaddr *addr;

    struct sockaddr_in6 addr6 = {
        .sin6_family = AF_INET6,
        .sin6_port = htons(PORT),
        .sin6_addr = in6addr_any
    };
    struct sockaddr_in addr4 = {
        .sin_family = AF_INET,
        .sin_port = htons(PORT),
        .sin_addr.s_addr = htonl(INADDR_ANY)
    };


    if (family == AF_INET6) {
        addr = (struct sockaddr*)&addr6;
        addrlen = sizeof(addr6);
        ipv6 = true;
    } else if (family == AF_INET) {
        addr = (struct sockaddr*)&addr4;
        addrlen = sizeof(addr4);
    } else {
        printf("Unknown family\n");
        goto out;
    }

    if ((fd = socket(family, SOCK_STREAM, 0)) < 0) {
        perror("socket");
        goto out;
    }

    if (ipv6 && setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, (void*)&v6only,
                           sizeof(v6only)) < 0) {
        perror("setsockopt IPV6_V6ONLY");
        goto out;
    }

    if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,
                   &reuseaddr, sizeof(reuseaddr)) < 0) {
        perror("setsockopt SO_REUSEADDR");
        goto out;
    }

    if (bind(fd, addr, addrlen) < 0) {
        perror("bind");
        goto out;
    }

    ret = 0;
out:
    close(fd);
    return ret;
}

int main(void) {
#ifdef CHECK_IPV6
    if (check_port(AF_INET6) < 0) {
        printf("AF_INET6 check failed.\n");
        return -1;
    }
    printf("AF_INET6 success\n");
#endif

    if (check_port(AF_INET) < 0) {
        printf("AF_INET check failed.\n");
        return -1;
    }
    printf("AF_INET success\n");

    return 0;
}

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-12 22:35 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression Laura Abbott
@ 2017-09-12 23:12 ` Josef Bacik
  2017-09-13 15:44   ` Laura Abbott
  0 siblings, 1 reply; 14+ messages in thread
From: Josef Bacik @ 2017-09-12 23:12 UTC (permalink / raw)
  To: Laura Abbott, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Cole Robinson

[-- Attachment #1: Type: text/plain, Size: 1611 bytes --]

First I’m super sorry for the top post, I’m at plumbers and I forgot to upload my muttrc to my new cloud instance, so I’m screwed using outlook.

I have a completely untested, uncompiled patch that I think will fix the problem, would you mind giving it a go?  Thanks,

Josef

On 9/12/17, 3:36 PM, "Laura Abbott" <labbott@redhat.com> wrote:

Hi,

Fedora got a bug report 
https://bugzilla.redhat.com/show_bug.cgi?id=1432684 of a regression with 
automatic spice port
assignment. The libvirt team reduced this to the attached test
case run as follows:

In a separate terminal, qemu-kvm -vnc 127.0.0.1:0 to grab port 5900. 
Then do this:

$ gcc bind-collision.c && ./a.out
bind: Address already in use
AF_INET check failed.
$ gcc -D CHECK_IPV6 bind-collision.c && ./a.out
AF_INET6 success
AF_INET success
$ gcc bind-collision.c && ./a.out
AF_INET success

Bisection showed this behavior to be caused by

commit 319554f284dda9f2737d09df82ba3610bd8ddea3
Author: Josef Bacik <jbacik@fb.com>
Date:   Thu Jan 19 17:47:46 2017 -0500

     inet: don't use sk_v6_rcv_saddr directly

     When comparing two sockets we need to use inet6_rcv_saddr so we get 
a NULL
     sk_v6_rcv_saddr if the socket isn't AF_INET6, otherwise our 
comparison function
     can be wrong.

     Fixes: 637bc8b ("inet: reset tb->fastreuseport when adding a 
reuseport sk")
     Signed-off-by: Josef Bacik <jbacik@fb.com>
     Signed-off-by: David S. Miller <davem@davemloft.net>


And reverting fixed both the standalone test case and the spice issue.

Any ideas?

Thanks,
Laura



[-- Attachment #2: 0001-net-set-tb-fast_sk_family.patch --]
[-- Type: application/octet-stream, Size: 1307 bytes --]

From b3c0458435d87911f1f99d314013b268e6c850a9 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Tue, 12 Sep 2017 16:07:00 -0700
Subject: [PATCH] net: set tb->fast_sk_family

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

Signed-off-by: Josef Bacik <josef@toxicpanda.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 4089c013cb03..3cff95f10995 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.13.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-12 23:12 ` Josef Bacik
@ 2017-09-13 15:44   ` Laura Abbott
  2017-09-13 17:28     ` Josef Bacik
  0 siblings, 1 reply; 14+ messages in thread
From: Laura Abbott @ 2017-09-13 15:44 UTC (permalink / raw)
  To: Josef Bacik, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Cole Robinson

On 09/12/2017 04:12 PM, Josef Bacik wrote:
> First I’m super sorry for the top post, I’m at plumbers and I forgot to upload my muttrc to my new cloud instance, so I’m screwed using outlook.
> 
> I have a completely untested, uncompiled patch that I think will fix the problem, would you mind giving it a go?  Thanks,
> 
> Josef

Thanks for the quick turnaround. Unfortunately, the problem is still
reproducible according to the reporter.

Thanks,
Laura

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 15:44   ` Laura Abbott
@ 2017-09-13 17:28     ` Josef Bacik
  2017-09-13 17:40       ` Cole Robinson
  2017-09-13 19:47       ` Chuck Ebbert
  0 siblings, 2 replies; 14+ messages in thread
From: Josef Bacik @ 2017-09-13 17:28 UTC (permalink / raw)
  To: Laura Abbott, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel, Cole Robinson

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,

Josef

On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:

On 09/12/2017 04:12 PM, Josef Bacik wrote:
> First I’m super sorry for the top post, I’m at plumbers and I forgot to upload my muttrc to my new cloud instance, so I’m screwed using outlook.
> 
> I have a completely untested, uncompiled patch that I think will fix the problem, would you mind giving it a go?  Thanks,
> 
> Josef

Thanks for the quick turnaround. Unfortunately, the problem is still
reproducible according to the reporter.

Thanks,
Laura



[-- Attachment #2: 0001-net-use-inet6_rcv_saddr-to-compare-sockets.patch --]
[-- Type: application/octet-stream, Size: 1085 bytes --]

From 380968f58d543ac9ec0cb1aa11db3979f3aee69d Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Wed, 13 Sep 2017 10:24:22 -0700
Subject: [PATCH] net: use inet6_rcv_saddr to compare sockets

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.

Signed-off-by: Josef Bacik <josef@toxicpanda.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 3cff95f10995..ff8b15a99e42 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.13.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 17:28     ` Josef Bacik
@ 2017-09-13 17:40       ` Cole Robinson
  2017-09-13 19:13         ` Cole Robinson
  2017-09-13 19:47       ` Chuck Ebbert
  1 sibling, 1 reply; 14+ messages in thread
From: Cole Robinson @ 2017-09-13 17:40 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

On 09/13/2017 01:28 PM, Josef Bacik wrote:
> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
> 

I'm the bug reporter. I'll combine the two patches and report back

Thanks,
Cole

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 17:40       ` Cole Robinson
@ 2017-09-13 19:13         ` Cole Robinson
  2017-09-13 19:44           ` Josef Bacik
  0 siblings, 1 reply; 14+ messages in thread
From: Cole Robinson @ 2017-09-13 19:13 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

On 09/13/2017 01:40 PM, Cole Robinson wrote:
> On 09/13/2017 01:28 PM, Josef Bacik wrote:
>> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
>>
> 
> I'm the bug reporter. I'll combine the two patches and report back
> 

Nope, issue is still present with both patches applied. Tried my own build and
a package Laura provided

Thanks,
Cole

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 19:13         ` Cole Robinson
@ 2017-09-13 19:44           ` Josef Bacik
  2017-09-13 22:49             ` Cole Robinson
  0 siblings, 1 reply; 14+ messages in thread
From: Josef Bacik @ 2017-09-13 19:44 UTC (permalink / raw)
  To: Cole Robinson, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Alright thanks, this should fix it.

Josef

On 9/13/17, 12:14 PM, "Cole Robinson" <crobinso@redhat.com> wrote:

On 09/13/2017 01:40 PM, Cole Robinson wrote:
> On 09/13/2017 01:28 PM, Josef Bacik wrote:
>> Sorry I thought I had made this other fix, can you apply this on top of the other one and try that?  I have more things to try if this doesn’t work, sorry you are playing go between, but I want to make sure I know _which_ fix actually fixes the problem, and then clean up in followup patches.  Thanks,
>>
> 
> I'm the bug reporter. I'll combine the two patches and report back
> 

Nope, issue is still present with both patches applied. Tried my own build and
a package Laura provided

Thanks,
Cole




[-- Attachment #2: 0001-net-don-t-fast-patch-mismatched-sockets-in-STRICT-mo.patch --]
[-- Type: application/octet-stream, Size: 1182 bytes --]

From 8d3da6d30a7c63034d4ff7d4fc33ea9e3f23cf41 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Wed, 13 Sep 2017 12:42:13 -0700
Subject: [PATCH] net: don't fast patch mismatched sockets in STRICT mode

With FASTREUSE_STRICT we may have sockets on the tb that don't match our
fast_sk information, so if our new socket don't strictly match the
fast_sk info we need to not go down the fast path and do the
inet_csk_bind_conflict so we can look at the other sockets on the tb.

Signed-off-by: Josef Bacik <josef@toxicpanda.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 ff8b15a99e42..fe9cf4862de2 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -263,6 +263,8 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 	 */
 	if (tb->fastreuseport == FASTREUSEPORT_ANY)
 		return 1;
+	if (tb->fast_ipv6_only && tb->fast_sk_family != sk->sk_family)
+		return 0;
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 17:28     ` Josef Bacik
  2017-09-13 17:40       ` Cole Robinson
@ 2017-09-13 19:47       ` Chuck Ebbert
  2017-09-13 20:11         ` Josef Bacik
  1 sibling, 1 reply; 14+ messages in thread
From: Chuck Ebbert @ 2017-09-13 19:47 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Cole Robinson

On Wed, 13 Sep 2017 17:28:25 +0000
Josef Bacik <jbacik@fb.com> wrote:

> Sorry I thought I had made this other fix, can you apply this on top
> of the other one and try that?  I have more things to try if this
> doesn’t work, sorry you are playing go between, but I want to make
> sure I know _which_ fix actually fixes the problem, and then clean up
> in followup patches.  Thanks,
> 
> Josef
> 
> On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:
> 
> On 09/12/2017 04:12 PM, Josef Bacik wrote:
> > First I’m super sorry for the top post, I’m at plumbers and I
> > forgot to upload my muttrc to my new cloud instance, so I’m screwed
> > using outlook.
> > 
> > I have a completely untested, uncompiled patch that I think will
> > fix the problem, would you mind giving it a go?  Thanks,
> > 
> > Josef  
> 
> Thanks for the quick turnaround. Unfortunately, the problem is still
> reproducible according to the reporter.
> 
> Thanks,
> Laura

I am confused by the patch that originally caused this:

        if (sk->sk_family == AF_INET6)
                return ipv6_rcv_saddr_equal(&sk->sk_v6_rcv_saddr,
-                                           &sk2->sk_v6_rcv_saddr,
+                                           inet6_rcv_saddr(sk2),
                                            sk->sk_rcv_saddr,
                                            sk2->sk_rcv_saddr,

Shouldn't the first argument also be changed to use inet6_rcv_saddr()?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 19:47       ` Chuck Ebbert
@ 2017-09-13 20:11         ` Josef Bacik
  0 siblings, 0 replies; 14+ messages in thread
From: Josef Bacik @ 2017-09-13 20:11 UTC (permalink / raw)
  To: Chuck Ebbert
  Cc: Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel, Cole Robinson


> On Sep 13, 2017, at 12:46 PM, Chuck Ebbert <cebbert.lkml@gmail.com> wrote:
> 
> On Wed, 13 Sep 2017 17:28:25 +0000
> Josef Bacik <jbacik@fb.com> wrote:
> 
>> Sorry I thought I had made this other fix, can you apply this on top
>> of the other one and try that?  I have more things to try if this
>> doesn’t work, sorry you are playing go between, but I want to make
>> sure I know _which_ fix actually fixes the problem, and then clean up
>> in followup patches.  Thanks,
>> 
>> Josef
>> 
>> On 9/13/17, 8:45 AM, "Laura Abbott" <labbott@redhat.com> wrote:
>> 
>> On 09/12/2017 04:12 PM, Josef Bacik wrote:
>>> First I’m super sorry for the top post, I’m at plumbers and I
>>> forgot to upload my muttrc to my new cloud instance, so I’m screwed
>>> using outlook.
>>> 
>>> I have a completely untested, uncompiled patch that I think will
>>> fix the problem, would you mind giving it a go?  Thanks,
>>> 
>>> Josef  
>> 
>> Thanks for the quick turnaround. Unfortunately, the problem is still
>> reproducible according to the reporter.
>> 
>> Thanks,
>> Laura
> 
> I am confused by the patch that originally caused this:
> 
>        if (sk->sk_family == AF_INET6)
>                return ipv6_rcv_saddr_equal(&sk->sk_v6_rcv_saddr,
> -                                           &sk2->sk_v6_rcv_saddr,
> +                                           inet6_rcv_saddr(sk2),
>                                            sk->sk_rcv_saddr,
>                                            sk2->sk_rcv_saddr,
> 
> Shouldn't the first argument also be changed to use inet6_rcv_saddr()?

No we know sk is IPv6 so it's alright to use directly.  Thanks,

Josef

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 19:44           ` Josef Bacik
@ 2017-09-13 22:49             ` Cole Robinson
  2017-09-15 17:51               ` Josef Bacik
  0 siblings, 1 reply; 14+ messages in thread
From: Cole Robinson @ 2017-09-13 22:49 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

On 09/13/2017 03:44 PM, Josef Bacik wrote:
> Alright thanks, this should fix it.
> 

Still no luck with all three patches applied to fedora 4.12.8-300 RPM. Pretty
sure I didn't mess up the testing but since I rarely do kernel builds it's not
impossible...

Thanks,
Cole

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-13 22:49             ` Cole Robinson
@ 2017-09-15 17:51               ` Josef Bacik
  2017-09-17 13:17                 ` Cole Robinson
  0 siblings, 1 reply; 14+ messages in thread
From: Josef Bacik @ 2017-09-15 17:51 UTC (permalink / raw)
  To: Cole Robinson, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,

Josef

On 9/13/17, 3:49 PM, "Cole Robinson" <crobinso@redhat.com> wrote:

On 09/13/2017 03:44 PM, Josef Bacik wrote:
> Alright thanks, this should fix it.
> 

Still no luck with all three patches applied to fedora 4.12.8-300 RPM. Pretty
sure I didn't mess up the testing but since I rarely do kernel builds it's not
impossible...

Thanks,
Cole




[-- Attachment #2: 0001-net-call-sk_reuseport_match-if-we-are-a-reusesock.patch --]
[-- Type: application/octet-stream, Size: 1223 bytes --]

From b70e7a78af2c4c090ca816d9f127a2f1e5866fb8 Mon Sep 17 00:00:00 2001
From: Josef Bacik <josef@toxicpanda.com>
Date: Fri, 15 Sep 2017 10:48:17 -0700
Subject: [PATCH] net: call sk_reuseport_match if we are a reusesock

When adding the sk_reuseport_match helper I screwed up and made it so we
unconditionally succeeded if our socket was a reuseport socket and the
tb was marked reuseable.  Fix this by actually calling
sk_reuseport_match() to make sure our new socket matches the tb
reuseport settings.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/ipv4/inet_connection_sock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index fe9cf4862de2..0e30e504c7d4 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -316,8 +316,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 		if (sk->sk_reuse == SK_FORCE_REUSE)
 			goto success;
 
-		if ((tb->fastreuse > 0 && reuse) ||
-		    sk_reuseport_match(tb, sk))
+		if (tb->fastreuse > 0 && reuse && sk_reuseport_match(tb, sk))
 			goto success;
 		if (inet_csk_bind_conflict(sk, tb, true, true))
 			goto fail_unlock;
-- 
2.13.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-15 17:51               ` Josef Bacik
@ 2017-09-17 13:17                 ` Cole Robinson
  2017-09-18  8:02                   ` Marc Haber
  0 siblings, 1 reply; 14+ messages in thread
From: Cole Robinson @ 2017-09-17 13:17 UTC (permalink / raw)
  To: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI
  Cc: netdev, linux-kernel

On 09/15/2017 01:51 PM, Josef Bacik wrote:
> Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,
> 

Yup I can confirm that patch fixes things when applied on top of the
previous 3 patches. Thanks! Please tag those patches for stable releases
if appropriate, this is affecting a decent amount of libvirt users

Thanks,
Cole

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-17 13:17                 ` Cole Robinson
@ 2017-09-18  8:02                   ` Marc Haber
  2017-11-13  7:36                     ` Marc Haber
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Haber @ 2017-09-18  8:02 UTC (permalink / raw)
  To: Cole Robinson
  Cc: Josef Bacik, Laura Abbott, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, netdev, linux-kernel

On Sun, Sep 17, 2017 at 09:17:13AM -0400, Cole Robinson wrote:
> On 09/15/2017 01:51 PM, Josef Bacik wrote:
> > Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,
> > 
> 
> Yup I can confirm that patch fixes things when applied on top of the
> previous 3 patches. Thanks! Please tag those patches for stable releases
> if appropriate, this is affecting a decent amount of libvirt users

I can also confirm that these four patches fix things for me (on
Debian) as well. Thanks!

I would love to have this in one of Greg's next 4.13 releases.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany    |  lose things."    Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature |  How to make an American Quilt | Fax: *49 6224 1600421

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression
  2017-09-18  8:02                   ` Marc Haber
@ 2017-11-13  7:36                     ` Marc Haber
  0 siblings, 0 replies; 14+ messages in thread
From: Marc Haber @ 2017-11-13  7:36 UTC (permalink / raw)
  To: Cole Robinson, Josef Bacik, Laura Abbott, David S. Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev, linux-kernel

Hi,

those four patches never made it into any 4.13 release.

0001-net-call-sk_reuseport_match-if-we-are-a-reusesock.patch
0001-net-don-t-fast-patch-mismatched-sockets-in-STRICT-mo.patch
0001-net-use-inet6_rcv_saddr-to-compare-sockets.patch
0001-net-set-tb-fast_sk_family.patch

And I have just seen that the first two are not even in 4.14. What does
that mean for libvirt users on systems runnign a 4.14 kernel?

The third and fourth patch
(0001-net-use-inet6_rcv_saddr-to-compare-sockets.patch and
0001-net-set-tb-fast_sk_family.patch) seem to be in 4.14.

Greetings
Marc

On Mon, Sep 18, 2017 at 10:02:32AM +0200, Marc Haber wrote:
> On Sun, Sep 17, 2017 at 09:17:13AM -0400, Cole Robinson wrote:
> > On 09/15/2017 01:51 PM, Josef Bacik wrote:
> > > Finally got access to a box to run this down myself.  This patch on top of the other patches fixes the problem for me, could you verify it works for you?  Thanks,
> > > 
> > 
> > Yup I can confirm that patch fixes things when applied on top of the
> > previous 3 patches. Thanks! Please tag those patches for stable releases
> > if appropriate, this is affecting a decent amount of libvirt users
> 
> I can also confirm that these four patches fix things for me (on
> Debian) as well. Thanks!
> 
> I would love to have this in one of Greg's next 4.13 releases.

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany    |  lose things."    Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature |  How to make an American Quilt | Fax: *49 6224 1600421

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2017-11-13  8:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-12 22:35 319554f284dd ("inet: don't use sk_v6_rcv_saddr directly") causes bind port regression Laura Abbott
2017-09-12 23:12 ` Josef Bacik
2017-09-13 15:44   ` Laura Abbott
2017-09-13 17:28     ` Josef Bacik
2017-09-13 17:40       ` Cole Robinson
2017-09-13 19:13         ` Cole Robinson
2017-09-13 19:44           ` Josef Bacik
2017-09-13 22:49             ` Cole Robinson
2017-09-15 17:51               ` Josef Bacik
2017-09-17 13:17                 ` Cole Robinson
2017-09-18  8:02                   ` Marc Haber
2017-11-13  7:36                     ` Marc Haber
2017-09-13 19:47       ` Chuck Ebbert
2017-09-13 20:11         ` 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.