Message ID | 20200722165227.51046-1-kuniyu@amazon.co.jp |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2(). | expand |
From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Date: Thu, 23 Jul 2020 01:52:27 +0900 > This patch removes an unnecessary variable in udp[46]_lib_lookup2() and > makes it easier to resolve a merge conflict with bpf-next reported in > the link below. > > Link: https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ > Fixes: efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> This doesn't apply to net-next.
From: David Miller <davem@davemloft.net> Date: Thu, 23 Jul 2020 15:10:51 -0700 (PDT) > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > Date: Thu, 23 Jul 2020 01:52:27 +0900 > > > This patch removes an unnecessary variable in udp[46]_lib_lookup2() and > > makes it easier to resolve a merge conflict with bpf-next reported in > > the link below. > > > > Link: https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ > > Fixes: efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > > This doesn't apply to net-next. Yes. I think this kind of patch should be submitted to net-next, but this is for the net tree. Please let me add more description. Currently, the net and net-next trees conflict in udp[46]_lib_lookup2() between efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") and 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport group") 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport group") . The conflict is reported in the link[0] and Jakub suggested how to resolve it[1]. To ease the merge conflict, Jakub and I have to send follow up patches to the bpf-next and net trees. Now, his patchset (7629c73a1466 and 2a08748cd384) to bpf-next is merged into net-next, and his follow up patch is applied in bpf-next[2]. I fixed a bug in efc6b6f6c311, but it introduced an unnecessary variable and made the conflict worse. So I sent this follow up patch to net tree. However, I do not know the best way to resolve the conflict, so any comments are welcome. [0] https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ [1] https://lore.kernel.org/linux-next/87wo2vwxq6.fsf@cloudflare.com/ [2] https://lore.kernel.org/netdev/20200722165902.51857-1-kuniyu@amazon.co.jp/T/#t Best Regards, Kuniyuki
On Fri, Jul 24, 2020 at 2:13 AM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote: > > From: David Miller <davem@davemloft.net> > Date: Thu, 23 Jul 2020 15:10:51 -0700 (PDT) > > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > Date: Thu, 23 Jul 2020 01:52:27 +0900 > > > > > This patch removes an unnecessary variable in udp[46]_lib_lookup2() and > > > makes it easier to resolve a merge conflict with bpf-next reported in > > > the link below. > > > > > > Link: https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ > > > Fixes: efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > > > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > > > > This doesn't apply to net-next. > > Yes. I think this kind of patch should be submitted to net-next, but this > is for the net tree. Please let me add more description. > > Currently, the net and net-next trees conflict in udp[46]_lib_lookup2() > between > > efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > and > > 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport group") > 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport group") > . > > The conflict is reported in the link[0] and Jakub suggested how to resolve > it[1]. To ease the merge conflict, Jakub and I have to send follow up patches to > the bpf-next and net trees. > > Now, his patchset (7629c73a1466 and 2a08748cd384) to bpf-next is merged > into net-next, and his follow up patch is applied in bpf-next[2]. > > I fixed a bug in efc6b6f6c311, but it introduced an unnecessary variable > and made the conflict worse. So I sent this follow up patch to net tree. > > However, I do not know the best way to resolve the conflict, so any comments > are welcome. Perhaps simpler is to apply this change to bpf-next: " badness = score; - result = sk; + if (!result) + result = sk; " After which the remaining conflict between bpf-next and net is " ++<<<<<<< HEAD + result = lookup_reuseport(net, sk, skb, + saddr, sport, daddr, hnum); + if (result && !reuseport_has_conns(sk, false)) + return result; + + badness = score; + if (!result) + result = sk; ++======= + reuseport_result = NULL; + + if (sk->sk_reuseport && + sk->sk_state != TCP_ESTABLISHED) { + hash = udp_ehashfn(net, daddr, hnum, + saddr, sport); + 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; ++>>>>>>> netdev-net/master " And we can just take bpf-next HEAD. Either that or handle the corresponding change in the merge fix-up itself. (note that bpf-next is one patch ahead of netdev-nn)
On Fri, Jul 24, 2020 at 6:38 AM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > On Fri, Jul 24, 2020 at 2:13 AM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote: > > > > From: David Miller <davem@davemloft.net> > > Date: Thu, 23 Jul 2020 15:10:51 -0700 (PDT) > > > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > > Date: Thu, 23 Jul 2020 01:52:27 +0900 > > > > > > > This patch removes an unnecessary variable in udp[46]_lib_lookup2() and > > > > makes it easier to resolve a merge conflict with bpf-next reported in > > > > the link below. > > > > > > > > Link: https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ > > > > Fixes: efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > > > > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > > > > > > This doesn't apply to net-next. > > > > Yes. I think this kind of patch should be submitted to net-next, but this > > is for the net tree. Please let me add more description. > > > > Currently, the net and net-next trees conflict in udp[46]_lib_lookup2() > > between > > > > efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > > > and > > > > 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport group") > > 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport group") > > . > > > > The conflict is reported in the link[0] and Jakub suggested how to resolve > > it[1]. To ease the merge conflict, Jakub and I have to send follow up patches to > > the bpf-next and net trees. > > > > Now, his patchset (7629c73a1466 and 2a08748cd384) to bpf-next is merged > > into net-next, and his follow up patch is applied in bpf-next[2]. > > > > I fixed a bug in efc6b6f6c311, but it introduced an unnecessary variable > > and made the conflict worse. So I sent this follow up patch to net tree. > > > > However, I do not know the best way to resolve the conflict, so any comments > > are welcome. > > Perhaps simpler is to apply this change to bpf-next: I'm fine whichever way. Could you please submit an official patch?
From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> Date: Fri, 24 Jul 2020 15:13:04 +0900 > Yes. I think this kind of patch should be submitted to net-next, but > this is for the net tree. Please let me add more description. This does not fix a bug, therefore 'net' is not appropriate. The merge conflicts should be handled by the appropriate maintainer when the merges happen.
On Fri, Jul 24, 2020 at 3:13 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Jul 24, 2020 at 6:38 AM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > On Fri, Jul 24, 2020 at 2:13 AM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote: > > > > > > From: David Miller <davem@davemloft.net> > > > Date: Thu, 23 Jul 2020 15:10:51 -0700 (PDT) > > > > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > > > Date: Thu, 23 Jul 2020 01:52:27 +0900 > > > > > > > > > This patch removes an unnecessary variable in udp[46]_lib_lookup2() and > > > > > makes it easier to resolve a merge conflict with bpf-next reported in > > > > > the link below. > > > > > > > > > > Link: https://lore.kernel.org/linux-next/20200722132143.700a5ccc@canb.auug.org.au/ > > > > > Fixes: efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > > > > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > > > > > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > > > > > > > > This doesn't apply to net-next. > > > > > > Yes. I think this kind of patch should be submitted to net-next, but this > > > is for the net tree. Please let me add more description. > > > > > > Currently, the net and net-next trees conflict in udp[46]_lib_lookup2() > > > between > > > > > > efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") > > > > > > and > > > > > > 7629c73a1466 ("udp: Extract helper for selecting socket from reuseport group") > > > 2a08748cd384 ("udp6: Extract helper for selecting socket from reuseport group") > > > . > > > > > > The conflict is reported in the link[0] and Jakub suggested how to resolve > > > it[1]. To ease the merge conflict, Jakub and I have to send follow up patches to > > > the bpf-next and net trees. > > > > > > Now, his patchset (7629c73a1466 and 2a08748cd384) to bpf-next is merged > > > into net-next, and his follow up patch is applied in bpf-next[2]. > > > > > > I fixed a bug in efc6b6f6c311, but it introduced an unnecessary variable > > > and made the conflict worse. So I sent this follow up patch to net tree. > > > > > > However, I do not know the best way to resolve the conflict, so any comments > > > are welcome. > > > > Perhaps simpler is to apply this change to bpf-next: > > I'm fine whichever way. > Could you please submit an official patch? http://patchwork.ozlabs.org/project/netdev/patch/20200725025457.1004164-1-willemdebruijn.kernel@gmail.com/ Not sure whether it helps vs doing this as part of the merge conflict (which remains). Either way after conflict resolution should be " static struct sock *udp4_lib_lookup2(struct net *net, __be32 saddr, __be16 sport, __be32 daddr, unsigned int hnum, int dif, int sdif, struct udp_hslot *hslot2, struct sk_buff *skb) { struct sock *sk, *result; int score, badness; result = NULL; badness = 0; udp_portaddr_for_each_entry_rcu(sk, &hslot2->head) { score = compute_score(sk, net, saddr, sport, daddr, hnum, dif, sdif); if (score > badness) { result = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum); if (result && !reuseport_has_conns(sk, false)) return result; badness = score; if (!result) result = sk; } } return result; } "
From: David Miller <davem@davemloft.net> Date: Fri, 24 Jul 2020 16:48:47 -0700 (PDT) > From: Kuniyuki Iwashima <kuniyu@amazon.co.jp> > Date: Fri, 24 Jul 2020 15:13:04 +0900 > > > Yes. I think this kind of patch should be submitted to net-next, but > > this is for the net tree. Please let me add more description. > > This does not fix a bug, therefore 'net' is not appropriate. Exactly, I am sorry for the confusion. > The merge conflicts should be handled by the appropriate maintainer > when the merges happen. I will keep this in mind, thank you. Best Regards, Kuniyuki
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 4077d589b72e..22fb231e27c3 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, *reuseport_result; + struct sock *sk, *result; int score, badness; u32 hash = 0; @@ -426,19 +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; + result = NULL; if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp_ehashfn(net, daddr, hnum, saddr, sport); - reuseport_result = reuseport_select_sock(sk, hash, skb, - sizeof(struct udphdr)); - if (reuseport_result && !reuseport_has_conns(sk, false)) - return reuseport_result; + result = reuseport_select_sock(sk, hash, skb, + sizeof(struct udphdr)); + if (result && !reuseport_has_conns(sk, false)) + return result; } - result = reuseport_result ? : sk; + if (!result) + result = sk; badness = score; } } diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index a8d74f44056a..29c7bb2609c4 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, *reuseport_result; + struct sock *sk, *result; int score, badness; u32 hash = 0; @@ -158,20 +158,21 @@ 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; + result = NULL; if (sk->sk_reuseport && sk->sk_state != TCP_ESTABLISHED) { hash = udp6_ehashfn(net, daddr, hnum, saddr, sport); - reuseport_result = reuseport_select_sock(sk, hash, skb, - sizeof(struct udphdr)); - if (reuseport_result && !reuseport_has_conns(sk, false)) - return reuseport_result; + result = reuseport_select_sock(sk, hash, skb, + sizeof(struct udphdr)); + if (result && !reuseport_has_conns(sk, false)) + return result; } - result = reuseport_result ? : sk; + if (!result) + result = sk; badness = score; } }