* [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
@ 2020-07-22 16:52 Kuniyuki Iwashima
2020-07-23 22:10 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Kuniyuki Iwashima @ 2020-07-22 16:52 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann
Cc: Willem de Bruijn, Jakub Sitnicki, netdev, Kuniyuki Iwashima,
Kuniyuki Iwashima, Willem de Bruijn
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>
---
net/ipv4/udp.c | 15 ++++++++-------
net/ipv6/udp.c | 15 ++++++++-------
2 files changed, 16 insertions(+), 14 deletions(-)
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;
}
}
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-22 16:52 [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2() Kuniyuki Iwashima
@ 2020-07-23 22:10 ` David Miller
2020-07-24 6:13 ` Kuniyuki Iwashima
0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2020-07-23 22:10 UTC (permalink / raw)
To: kuniyu
Cc: kuba, ast, daniel, willemb, jakub, netdev, kuni1840,
willemdebruijn.kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-23 22:10 ` David Miller
@ 2020-07-24 6:13 ` Kuniyuki Iwashima
2020-07-24 13:38 ` Willem de Bruijn
2020-07-24 23:48 ` David Miller
0 siblings, 2 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2020-07-24 6:13 UTC (permalink / raw)
To: davem
Cc: ast, daniel, jakub, kuba, kuni1840, kuniyu, netdev, willemb,
willemdebruijn.kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-24 6:13 ` Kuniyuki Iwashima
@ 2020-07-24 13:38 ` Willem de Bruijn
2020-07-24 19:12 ` Alexei Starovoitov
2020-07-24 23:48 ` David Miller
1 sibling, 1 reply; 8+ messages in thread
From: Willem de Bruijn @ 2020-07-24 13:38 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David Miller, Alexei Starovoitov, Daniel Borkmann,
Jakub Sitnicki, Jakub Kicinski, Kuniyuki Iwashima,
Network Development, Willem de Bruijn
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)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-24 13:38 ` Willem de Bruijn
@ 2020-07-24 19:12 ` Alexei Starovoitov
2020-07-25 3:04 ` Willem de Bruijn
0 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2020-07-24 19:12 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Kuniyuki Iwashima, David Miller, Alexei Starovoitov,
Daniel Borkmann, Jakub Sitnicki, Jakub Kicinski,
Kuniyuki Iwashima, Network Development
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-24 6:13 ` Kuniyuki Iwashima
2020-07-24 13:38 ` Willem de Bruijn
@ 2020-07-24 23:48 ` David Miller
2020-07-25 10:54 ` Kuniyuki Iwashima
1 sibling, 1 reply; 8+ messages in thread
From: David Miller @ 2020-07-24 23:48 UTC (permalink / raw)
To: kuniyu
Cc: ast, daniel, jakub, kuba, kuni1840, netdev, willemb,
willemdebruijn.kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-24 19:12 ` Alexei Starovoitov
@ 2020-07-25 3:04 ` Willem de Bruijn
0 siblings, 0 replies; 8+ messages in thread
From: Willem de Bruijn @ 2020-07-25 3:04 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Willem de Bruijn, Kuniyuki Iwashima, David Miller,
Alexei Starovoitov, Daniel Borkmann, Jakub Sitnicki,
Jakub Kicinski, Kuniyuki Iwashima, Network Development
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;
}
"
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2().
2020-07-24 23:48 ` David Miller
@ 2020-07-25 10:54 ` Kuniyuki Iwashima
0 siblings, 0 replies; 8+ messages in thread
From: Kuniyuki Iwashima @ 2020-07-25 10:54 UTC (permalink / raw)
To: davem
Cc: ast, daniel, jakub, kuba, kuni1840, kuniyu, netdev, willemb,
willemdebruijn.kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-07-25 10:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-22 16:52 [PATCH net] udp: Remove an unnecessary variable in udp[46]_lib_lookup2() Kuniyuki Iwashima
2020-07-23 22:10 ` David Miller
2020-07-24 6:13 ` Kuniyuki Iwashima
2020-07-24 13:38 ` Willem de Bruijn
2020-07-24 19:12 ` Alexei Starovoitov
2020-07-25 3:04 ` Willem de Bruijn
2020-07-24 23:48 ` David Miller
2020-07-25 10:54 ` Kuniyuki Iwashima
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.