* [PATCH net 0/2] udp: Fix reuseport selection with connected sockets. @ 2020-07-21 6:15 Kuniyuki Iwashima 2020-07-21 6:15 ` [PATCH net 1/2] udp: Copy has_conns in reuseport_grow() Kuniyuki Iwashima ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Kuniyuki Iwashima @ 2020-07-21 6:15 UTC (permalink / raw) To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: Willem de Bruijn, Eric Dumazet, Craig Gallek, Paolo Abeni, netdev, Kuniyuki Iwashima, Kuniyuki Iwashima, Benjamin Herrenschmidt, osa-contribution-log From: kuniyu <kuniyu@amazon.co.jp> This patch set addresses two issues which happen when both connected and unconnected sockets are in the same UDP reuseport group. Kuniyuki Iwashima (2): udp: Copy has_conns in reuseport_grow(). udp: Improve load balancing for SO_REUSEPORT. net/core/sock_reuseport.c | 1 + net/ipv4/udp.c | 15 +++++++++------ net/ipv6/udp.c | 15 +++++++++------ 3 files changed, 19 insertions(+), 12 deletions(-) -- 2.17.2 (Apple Git-113) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 1/2] udp: Copy has_conns in reuseport_grow(). 2020-07-21 6:15 [PATCH net 0/2] udp: Fix reuseport selection with connected sockets Kuniyuki Iwashima @ 2020-07-21 6:15 ` Kuniyuki Iwashima 2020-07-21 12:25 ` Willem de Bruijn 2020-07-21 6:15 ` [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT Kuniyuki Iwashima 2020-07-21 22:32 ` [PATCH net 0/2] udp: Fix reuseport selection with connected sockets David Miller 2 siblings, 1 reply; 7+ messages in thread From: Kuniyuki Iwashima @ 2020-07-21 6:15 UTC (permalink / raw) To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: Willem de Bruijn, Eric Dumazet, Craig Gallek, Paolo Abeni, netdev, Kuniyuki Iwashima, Kuniyuki Iwashima, Benjamin Herrenschmidt, osa-contribution-log If an unconnected socket in a UDP reuseport group connect()s, has_conns is set to 1. Then, when a packet is received, udp[46]_lib_lookup2() scans all sockets in udp_hslot looking for the connected socket with the highest score. However, when the number of sockets bound to the port exceeds max_socks, reuseport_grow() resets has_conns to 0. It can cause udp[46]_lib_lookup2() to return without scanning all sockets, resulting in that packets sent to connected sockets may be distributed to unconnected sockets. Therefore, reuseport_grow() should copy has_conns. Fixes: acdcecc61285 ("udp: correct reuseport selection with connected sockets") CC: Willem de Bruijn <willemb@google.com> Reviewed-by: Benjamin Herrenschmidt <benh@amazon.com> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> --- net/core/sock_reuseport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c index adcb3aea576d..bbdd3c7b6cb5 100644 --- a/net/core/sock_reuseport.c +++ b/net/core/sock_reuseport.c @@ -101,6 +101,7 @@ static struct sock_reuseport *reuseport_grow(struct sock_reuseport *reuse) more_reuse->prog = reuse->prog; more_reuse->reuseport_id = reuse->reuseport_id; more_reuse->bind_inany = reuse->bind_inany; + more_reuse->has_conns = reuse->has_conns; memcpy(more_reuse->socks, reuse->socks, reuse->num_socks * sizeof(struct sock *)); -- 2.17.2 (Apple Git-113) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 1/2] udp: Copy has_conns in reuseport_grow(). 2020-07-21 6:15 ` [PATCH net 1/2] udp: Copy has_conns in reuseport_grow() Kuniyuki Iwashima @ 2020-07-21 12:25 ` Willem de Bruijn 0 siblings, 0 replies; 7+ messages in thread From: Willem de Bruijn @ 2020-07-21 12:25 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, Eric Dumazet, Craig Gallek, Paolo Abeni, Network Development, Kuniyuki Iwashima, Benjamin Herrenschmidt, osa-contribution-log On Tue, Jul 21, 2020 at 2:16 AM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote: > > If an unconnected socket in a UDP reuseport group connect()s, has_conns is > set to 1. Then, when a packet is received, udp[46]_lib_lookup2() scans all > sockets in udp_hslot looking for the connected socket with the highest > score. > > However, when the number of sockets bound to the port exceeds max_socks, > reuseport_grow() resets has_conns to 0. It can cause udp[46]_lib_lookup2() > to return without scanning all sockets, resulting in that packets sent to > connected sockets may be distributed to unconnected sockets. > > Therefore, reuseport_grow() should copy has_conns. > > Fixes: acdcecc61285 ("udp: correct reuseport selection with connected sockets") > CC: Willem de Bruijn <willemb@google.com> > Reviewed-by: Benjamin Herrenschmidt <benh@amazon.com> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Acked-by: Willem de Bruijn <willemb@google.com> Thanks. Yes, I missed this resize operation when adding the field. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT. 2020-07-21 6:15 [PATCH net 0/2] udp: Fix reuseport selection with connected sockets Kuniyuki Iwashima 2020-07-21 6:15 ` [PATCH net 1/2] udp: Copy has_conns in reuseport_grow() Kuniyuki Iwashima @ 2020-07-21 6:15 ` Kuniyuki Iwashima 2020-07-21 12:33 ` Willem de Bruijn 2020-07-21 22:32 ` [PATCH net 0/2] udp: Fix reuseport selection with connected sockets David Miller 2 siblings, 1 reply; 7+ messages in thread From: Kuniyuki Iwashima @ 2020-07-21 6:15 UTC (permalink / raw) To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski Cc: Willem de Bruijn, Eric Dumazet, Craig Gallek, Paolo Abeni, netdev, Kuniyuki Iwashima, Kuniyuki Iwashima, Benjamin Herrenschmidt, osa-contribution-log Currently, SO_REUSEPORT does not work well if connected sockets are in a UDP reuseport group. Then reuseport_has_conns() returns true and the result of reuseport_select_sock() is discarded. Also, unconnected sockets have the same score, hence only does the first unconnected socket in udp_hslot always receive all packets sent to unconnected sockets. So, the result of reuseport_select_sock() should be used for load balancing. The noteworthy point is that the unconnected sockets placed after connected sockets in sock_reuseport.socks will receive more packets than others because of the algorithm in reuseport_select_sock(). index | connected | reciprocal_scale | result --------------------------------------------- 0 | no | 20% | 40% 1 | no | 20% | 20% 2 | yes | 20% | 0% 3 | no | 20% | 40% 4 | yes | 20% | 0% If most of the sockets are connected, this can be a problem, but it still works better than now. Fixes: acdcecc61285 ("udp: correct reuseport selection with connected sockets") CC: Willem de Bruijn <willemb@google.com> Reviewed-by: Benjamin Herrenschmidt <benh@amazon.com> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> --- net/ipv4/udp.c | 15 +++++++++------ net/ipv6/udp.c | 15 +++++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 1b7ebbcae497..99251d3c70d0 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -416,7 +416,7 @@ static struct sock *udp4_lib_lookup2(struct net *net, struct udp_hslot *hslot2, struct sk_buff *skb) { - struct sock *sk, *result; + struct sock *sk, *result, *reuseport_result; int score, badness; u32 hash = 0; @@ -426,17 +426,20 @@ static struct sock *udp4_lib_lookup2(struct net *net, score = compute_score(sk, net, saddr, sport, daddr, hnum, dif, sdif); if (score > badness) { + reuseport_result = NULL; + if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp_ehashfn(net, daddr, hnum, saddr, sport); - result = reuseport_select_sock(sk, hash, skb, - sizeof(struct udphdr)); - if (result && !reuseport_has_conns(sk, false)) - return result; + reuseport_result = reuseport_select_sock(sk, hash, skb, + sizeof(struct udphdr)); + if (reuseport_result && !reuseport_has_conns(sk, false)) + return reuseport_result; } + + result = reuseport_result ? : sk; badness = score; - result = sk; } } return result; diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 7d4151747340..9503c87ac0b3 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -148,7 +148,7 @@ static struct sock *udp6_lib_lookup2(struct net *net, int dif, int sdif, struct udp_hslot *hslot2, struct sk_buff *skb) { - struct sock *sk, *result; + struct sock *sk, *result, *reuseport_result; int score, badness; u32 hash = 0; @@ -158,17 +158,20 @@ static struct sock *udp6_lib_lookup2(struct net *net, score = compute_score(sk, net, saddr, sport, daddr, hnum, dif, sdif); if (score > badness) { + reuseport_result = NULL; + if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp6_ehashfn(net, daddr, hnum, saddr, sport); - result = reuseport_select_sock(sk, hash, skb, - sizeof(struct udphdr)); - if (result && !reuseport_has_conns(sk, false)) - return result; + reuseport_result = reuseport_select_sock(sk, hash, skb, + sizeof(struct udphdr)); + if (reuseport_result && !reuseport_has_conns(sk, false)) + return reuseport_result; } - result = sk; + + result = reuseport_result ? : sk; badness = score; } } -- 2.17.2 (Apple Git-113) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT. 2020-07-21 6:15 ` [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT Kuniyuki Iwashima @ 2020-07-21 12:33 ` Willem de Bruijn 0 siblings, 0 replies; 7+ messages in thread From: Willem de Bruijn @ 2020-07-21 12:33 UTC (permalink / raw) To: Kuniyuki Iwashima Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jakub Kicinski, Eric Dumazet, Craig Gallek, Paolo Abeni, Network Development, Kuniyuki Iwashima, Benjamin Herrenschmidt, osa-contribution-log On Tue, Jul 21, 2020 at 2:17 AM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote: > > Currently, SO_REUSEPORT does not work well if connected sockets are in a > UDP reuseport group. > > Then reuseport_has_conns() returns true and the result of > reuseport_select_sock() is discarded. Also, unconnected sockets have the > same score, hence only does the first unconnected socket in udp_hslot > always receive all packets sent to unconnected sockets. > > So, the result of reuseport_select_sock() should be used for load > balancing. > > The noteworthy point is that the unconnected sockets placed after > connected sockets in sock_reuseport.socks will receive more packets than > others because of the algorithm in reuseport_select_sock(). > > index | connected | reciprocal_scale | result > --------------------------------------------- > 0 | no | 20% | 40% > 1 | no | 20% | 20% > 2 | yes | 20% | 0% > 3 | no | 20% | 40% > 4 | yes | 20% | 0% > > If most of the sockets are connected, this can be a problem, but it still > works better than now. > > Fixes: acdcecc61285 ("udp: correct reuseport selection with connected sockets") > CC: Willem de Bruijn <willemb@google.com> > Reviewed-by: Benjamin Herrenschmidt <benh@amazon.com> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Acked-by: Willem de Bruijn <willemb@google.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/2] udp: Fix reuseport selection with connected sockets. 2020-07-21 6:15 [PATCH net 0/2] udp: Fix reuseport selection with connected sockets Kuniyuki Iwashima 2020-07-21 6:15 ` [PATCH net 1/2] udp: Copy has_conns in reuseport_grow() Kuniyuki Iwashima 2020-07-21 6:15 ` [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT Kuniyuki Iwashima @ 2020-07-21 22:32 ` David Miller 2020-07-22 4:28 ` Kuniyuki Iwashima 2 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2020-07-21 22:32 UTC (permalink / raw) To: kuniyu Cc: kuznet, yoshfuji, kuba, willemb, edumazet, kraig, pabeni, netdev, kuni1840, benh, osa-contribution-log From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Date: Tue, 21 Jul 2020 15:15:29 +0900 > From: kuniyu <kuniyu@amazon.co.jp> Please fix your configuration to show your full name in this "From: " field, I had to edit it out and use the one from your email headers. > This patch set addresses two issues which happen when both connected and > unconnected sockets are in the same UDP reuseport group. Series applied and queued up for -stable, th anks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net 0/2] udp: Fix reuseport selection with connected sockets. 2020-07-21 22:32 ` [PATCH net 0/2] udp: Fix reuseport selection with connected sockets David Miller @ 2020-07-22 4:28 ` Kuniyuki Iwashima 0 siblings, 0 replies; 7+ messages in thread From: Kuniyuki Iwashima @ 2020-07-22 4:28 UTC (permalink / raw) To: davem Cc: benh, edumazet, kraig, kuba, kuni1840, kuniyu, kuznet, netdev, osa-contribution-log, pabeni, willemb, yoshfuji From: David Miller <davem@davemloft.net> Date: Tue, 21 Jul 2020 15:32:39 -0700 (PDT) > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > Date: Tue, 21 Jul 2020 15:15:29 +0900 > > > From: kuniyu <kuniyu@amazon.co.jp> > > Please fix your configuration to show your full name in this > "From: " field, I had to edit it out and use the one from your > email headers. > > > This patch set addresses two issues which happen when both connected and > > unconnected sockets are in the same UDP reuseport group. > > Series applied and queued up for -stable, th anks. I am sorry for bothering you... and grateful for your kind advice. I have fixed my configuration for net repository as with net-next. Thank you. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-22 4:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-21 6:15 [PATCH net 0/2] udp: Fix reuseport selection with connected sockets Kuniyuki Iwashima 2020-07-21 6:15 ` [PATCH net 1/2] udp: Copy has_conns in reuseport_grow() Kuniyuki Iwashima 2020-07-21 12:25 ` Willem de Bruijn 2020-07-21 6:15 ` [PATCH net 2/2] udp: Improve load balancing for SO_REUSEPORT Kuniyuki Iwashima 2020-07-21 12:33 ` Willem de Bruijn 2020-07-21 22:32 ` [PATCH net 0/2] udp: Fix reuseport selection with connected sockets David Miller 2020-07-22 4:28 ` Kuniyuki Iwashima
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).