* [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
@ 2020-07-26 12:02 Jakub Sitnicki
2020-07-28 1:20 ` Martin KaFai Lau
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Sitnicki @ 2020-07-26 12:02 UTC (permalink / raw)
To: bpf
Cc: netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Alexei Starovoitov
When BPF sk lookup invokes reuseport handling for the selected socket, it
should ignore the fact that reuseport group can contain connected UDP
sockets. With BPF sk lookup this is not relevant as we are not scoring
sockets to find the best match, which might be a connected UDP socket.
Fix it by unconditionally accepting the socket selected by reuseport.
This fixes the following two failures reported by test_progs.
# ./test_progs -t sk_lookup
...
#73/14 UDP IPv4 redir and reuseport with conns:FAIL
...
#73/20 UDP IPv6 redir and reuseport with conns:FAIL
...
Fixes: a57066b1a019 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
Cc: David S. Miller <davem@davemloft.net>
Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
---
net/ipv4/udp.c | 2 +-
net/ipv6/udp.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 7ce31beccfc2..e88efba07551 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -473,7 +473,7 @@ static struct sock *udp4_lookup_run_bpf(struct net *net,
return sk;
reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
- if (reuse_sk && !reuseport_has_conns(sk, false))
+ if (reuse_sk)
sk = reuse_sk;
return sk;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index c394e674f486..29d9691359b9 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
return sk;
reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
- if (reuse_sk && !reuseport_has_conns(sk, false))
+ if (reuse_sk)
sk = reuse_sk;
return sk;
}
--
2.25.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
2020-07-26 12:02 [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup Jakub Sitnicki
@ 2020-07-28 1:20 ` Martin KaFai Lau
2020-07-28 15:46 ` Jakub Sitnicki
0 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2020-07-28 1:20 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: bpf, netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Alexei Starovoitov
On Sun, Jul 26, 2020 at 02:02:28PM +0200, Jakub Sitnicki wrote:
> When BPF sk lookup invokes reuseport handling for the selected socket, it
> should ignore the fact that reuseport group can contain connected UDP
> sockets. With BPF sk lookup this is not relevant as we are not scoring
> sockets to find the best match, which might be a connected UDP socket.
>
> Fix it by unconditionally accepting the socket selected by reuseport.
>
> This fixes the following two failures reported by test_progs.
>
> # ./test_progs -t sk_lookup
> ...
> #73/14 UDP IPv4 redir and reuseport with conns:FAIL
> ...
> #73/20 UDP IPv6 redir and reuseport with conns:FAIL
> ...
>
> Fixes: a57066b1a019 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> Cc: David S. Miller <davem@davemloft.net>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> ---
> net/ipv4/udp.c | 2 +-
> net/ipv6/udp.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 7ce31beccfc2..e88efba07551 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -473,7 +473,7 @@ static struct sock *udp4_lookup_run_bpf(struct net *net,
> return sk;
>
> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
> - if (reuse_sk && !reuseport_has_conns(sk, false))
> + if (reuse_sk)
> sk = reuse_sk;
> return sk;
> }
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index c394e674f486..29d9691359b9 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
> return sk;
>
> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
> - if (reuse_sk && !reuseport_has_conns(sk, false))
> + if (reuse_sk)
From __udp[46]_lib_lookup,
1. The connected udp is picked by the kernel first.
If a 4-tuple-matched connected udp is found. It should have already
been returned there.
2. If kernel cannot find a connected udp, the sk-lookup bpf prog can
get a chance to pick another socket (likely bound to a different
IP/PORT that the packet is destinated to) by bpf_sk_lookup_assign().
However, bpf_sk_lookup_assign() does not allow TCP_ESTABLISHED.
With the change in this patch, it then allows the reuseport-bpf-prog
to pick a connected udp which cannot be found in step (1). Can you
explain a use case for this?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
2020-07-28 1:20 ` Martin KaFai Lau
@ 2020-07-28 15:46 ` Jakub Sitnicki
2020-07-28 16:37 ` Martin KaFai Lau
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Sitnicki @ 2020-07-28 15:46 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Alexei Starovoitov
On Tue, Jul 28, 2020 at 03:20 AM CEST, Martin KaFai Lau wrote:
> On Sun, Jul 26, 2020 at 02:02:28PM +0200, Jakub Sitnicki wrote:
>> When BPF sk lookup invokes reuseport handling for the selected socket, it
>> should ignore the fact that reuseport group can contain connected UDP
>> sockets. With BPF sk lookup this is not relevant as we are not scoring
>> sockets to find the best match, which might be a connected UDP socket.
>>
>> Fix it by unconditionally accepting the socket selected by reuseport.
>>
>> This fixes the following two failures reported by test_progs.
>>
>> # ./test_progs -t sk_lookup
>> ...
>> #73/14 UDP IPv4 redir and reuseport with conns:FAIL
>> ...
>> #73/20 UDP IPv6 redir and reuseport with conns:FAIL
>> ...
>>
>> Fixes: a57066b1a019 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
>> Cc: David S. Miller <davem@davemloft.net>
>> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> ---
>> net/ipv4/udp.c | 2 +-
>> net/ipv6/udp.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>> index 7ce31beccfc2..e88efba07551 100644
>> --- a/net/ipv4/udp.c
>> +++ b/net/ipv4/udp.c
>> @@ -473,7 +473,7 @@ static struct sock *udp4_lookup_run_bpf(struct net *net,
>> return sk;
>>
>> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
>> - if (reuse_sk && !reuseport_has_conns(sk, false))
>> + if (reuse_sk)
>> sk = reuse_sk;
>> return sk;
>> }
>> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>> index c394e674f486..29d9691359b9 100644
>> --- a/net/ipv6/udp.c
>> +++ b/net/ipv6/udp.c
>> @@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
>> return sk;
>>
>> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
>> - if (reuse_sk && !reuseport_has_conns(sk, false))
>> + if (reuse_sk)
> From __udp[46]_lib_lookup,
> 1. The connected udp is picked by the kernel first.
> If a 4-tuple-matched connected udp is found. It should have already
> been returned there.
>
> 2. If kernel cannot find a connected udp, the sk-lookup bpf prog can
> get a chance to pick another socket (likely bound to a different
> IP/PORT that the packet is destinated to) by bpf_sk_lookup_assign().
> However, bpf_sk_lookup_assign() does not allow TCP_ESTABLISHED.
>
> With the change in this patch, it then allows the reuseport-bpf-prog
> to pick a connected udp which cannot be found in step (1). Can you
> explain a use case for this?
It is not intentional. It should not allow reuseport to pick a connected
udp socket to be consistent with what sk-lookup prog can select. Thanks
for pointing it out.
I've incorrectly assumed that after acdcecc61285 ("udp: correct
reuseport selection with connected sockets") reuseport returns only
unconnected udp sockets, but thats not true for bpf reuseport.
So this patch fixes one corner base, but breaks another one.
I'll change the check to the below and respin:
- if (reuse_sk && !reuseport_has_conns(sk, false))
+ if (reuse_sk && reuse_sk->sk_state != TCP_ESTABLISHED)
Thanks,
-jkbs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
2020-07-28 15:46 ` Jakub Sitnicki
@ 2020-07-28 16:37 ` Martin KaFai Lau
2020-07-28 17:29 ` Jakub Sitnicki
0 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2020-07-28 16:37 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: bpf, netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Alexei Starovoitov
On Tue, Jul 28, 2020 at 05:46:29PM +0200, Jakub Sitnicki wrote:
> On Tue, Jul 28, 2020 at 03:20 AM CEST, Martin KaFai Lau wrote:
> > On Sun, Jul 26, 2020 at 02:02:28PM +0200, Jakub Sitnicki wrote:
> >> When BPF sk lookup invokes reuseport handling for the selected socket, it
> >> should ignore the fact that reuseport group can contain connected UDP
> >> sockets. With BPF sk lookup this is not relevant as we are not scoring
> >> sockets to find the best match, which might be a connected UDP socket.
> >>
> >> Fix it by unconditionally accepting the socket selected by reuseport.
> >>
> >> This fixes the following two failures reported by test_progs.
> >>
> >> # ./test_progs -t sk_lookup
> >> ...
> >> #73/14 UDP IPv4 redir and reuseport with conns:FAIL
> >> ...
> >> #73/20 UDP IPv6 redir and reuseport with conns:FAIL
> >> ...
> >>
> >> Fixes: a57066b1a019 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> >> Cc: David S. Miller <davem@davemloft.net>
> >> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
> >> ---
> >> net/ipv4/udp.c | 2 +-
> >> net/ipv6/udp.c | 2 +-
> >> 2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> >> index 7ce31beccfc2..e88efba07551 100644
> >> --- a/net/ipv4/udp.c
> >> +++ b/net/ipv4/udp.c
> >> @@ -473,7 +473,7 @@ static struct sock *udp4_lookup_run_bpf(struct net *net,
> >> return sk;
> >>
> >> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
> >> + if (reuse_sk)
> >> sk = reuse_sk;
> >> return sk;
> >> }
> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> >> index c394e674f486..29d9691359b9 100644
> >> --- a/net/ipv6/udp.c
> >> +++ b/net/ipv6/udp.c
> >> @@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
> >> return sk;
> >>
> >> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
> >> + if (reuse_sk)
> > From __udp[46]_lib_lookup,
> > 1. The connected udp is picked by the kernel first.
> > If a 4-tuple-matched connected udp is found. It should have already
> > been returned there.
> >
> > 2. If kernel cannot find a connected udp, the sk-lookup bpf prog can
> > get a chance to pick another socket (likely bound to a different
> > IP/PORT that the packet is destinated to) by bpf_sk_lookup_assign().
> > However, bpf_sk_lookup_assign() does not allow TCP_ESTABLISHED.
> >
> > With the change in this patch, it then allows the reuseport-bpf-prog
> > to pick a connected udp which cannot be found in step (1). Can you
> > explain a use case for this?
>
> It is not intentional. It should not allow reuseport to pick a connected
> udp socket to be consistent with what sk-lookup prog can select. Thanks
> for pointing it out.
>
> I've incorrectly assumed that after acdcecc61285 ("udp: correct
> reuseport selection with connected sockets") reuseport returns only
> unconnected udp sockets, but thats not true for bpf reuseport.
>
> So this patch fixes one corner base, but breaks another one.
>
> I'll change the check to the below and respin:
>
> - if (reuse_sk && !reuseport_has_conns(sk, false))
> + if (reuse_sk && reuse_sk->sk_state != TCP_ESTABLISHED)
May be disallow TCP_ESTABLISHED in bpf_sk_select_reuseport() instead
so that the bpf reuseport prog can have a more consistent
behavior among sk-lookup and the regular sk-reuseport-select case.
Thought?
From reuseport_select_sock(), it seems the kernel's select_by_hash
also avoids returning established sk.
In the mid term, we may consider to remove the connected udp
from the sockmap and reuseport_array.
I am a bit confused in the current situation on bpf@reuseport returning
connected sk and I also can't think of a use case in the
sk-reuseport-prog-type side. It was why I was curious on
the sk-lookup use case.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
2020-07-28 16:37 ` Martin KaFai Lau
@ 2020-07-28 17:29 ` Jakub Sitnicki
2020-07-28 19:19 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Sitnicki @ 2020-07-28 17:29 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, netdev, kernel-team, Alexei Starovoitov, Daniel Borkmann,
David S. Miller, Jakub Kicinski, Alexei Starovoitov,
Willem de Bruijn, Marek Majkowski
On Tue, Jul 28, 2020 at 06:37 PM CEST, Martin KaFai Lau wrote:
> On Tue, Jul 28, 2020 at 05:46:29PM +0200, Jakub Sitnicki wrote:
>> On Tue, Jul 28, 2020 at 03:20 AM CEST, Martin KaFai Lau wrote:
>> > On Sun, Jul 26, 2020 at 02:02:28PM +0200, Jakub Sitnicki wrote:
>> >> When BPF sk lookup invokes reuseport handling for the selected socket, it
>> >> should ignore the fact that reuseport group can contain connected UDP
>> >> sockets. With BPF sk lookup this is not relevant as we are not scoring
>> >> sockets to find the best match, which might be a connected UDP socket.
>> >>
>> >> Fix it by unconditionally accepting the socket selected by reuseport.
>> >>
>> >> This fixes the following two failures reported by test_progs.
>> >>
>> >> # ./test_progs -t sk_lookup
>> >> ...
>> >> #73/14 UDP IPv4 redir and reuseport with conns:FAIL
>> >> ...
>> >> #73/20 UDP IPv6 redir and reuseport with conns:FAIL
>> >> ...
>> >>
>> >> Fixes: a57066b1a019 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
>> >> Cc: David S. Miller <davem@davemloft.net>
>> >> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
>> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com>
>> >> ---
>> >> net/ipv4/udp.c | 2 +-
>> >> net/ipv6/udp.c | 2 +-
>> >> 2 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>> >> index 7ce31beccfc2..e88efba07551 100644
>> >> --- a/net/ipv4/udp.c
>> >> +++ b/net/ipv4/udp.c
>> >> @@ -473,7 +473,7 @@ static struct sock *udp4_lookup_run_bpf(struct net *net,
>> >> return sk;
>> >>
>> >> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
>> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
>> >> + if (reuse_sk)
>> >> sk = reuse_sk;
>> >> return sk;
>> >> }
>> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
>> >> index c394e674f486..29d9691359b9 100644
>> >> --- a/net/ipv6/udp.c
>> >> +++ b/net/ipv6/udp.c
>> >> @@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
>> >> return sk;
>> >>
>> >> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
>> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
>> >> + if (reuse_sk)
>> > From __udp[46]_lib_lookup,
>> > 1. The connected udp is picked by the kernel first.
>> > If a 4-tuple-matched connected udp is found. It should have already
>> > been returned there.
>> >
>> > 2. If kernel cannot find a connected udp, the sk-lookup bpf prog can
>> > get a chance to pick another socket (likely bound to a different
>> > IP/PORT that the packet is destinated to) by bpf_sk_lookup_assign().
>> > However, bpf_sk_lookup_assign() does not allow TCP_ESTABLISHED.
>> >
>> > With the change in this patch, it then allows the reuseport-bpf-prog
>> > to pick a connected udp which cannot be found in step (1). Can you
>> > explain a use case for this?
>>
>> It is not intentional. It should not allow reuseport to pick a connected
>> udp socket to be consistent with what sk-lookup prog can select. Thanks
>> for pointing it out.
>>
>> I've incorrectly assumed that after acdcecc61285 ("udp: correct
>> reuseport selection with connected sockets") reuseport returns only
>> unconnected udp sockets, but thats not true for bpf reuseport.
>>
>> So this patch fixes one corner base, but breaks another one.
>>
>> I'll change the check to the below and respin:
>>
>> - if (reuse_sk && !reuseport_has_conns(sk, false))
>> + if (reuse_sk && reuse_sk->sk_state != TCP_ESTABLISHED)
> May be disallow TCP_ESTABLISHED in bpf_sk_select_reuseport() instead
> so that the bpf reuseport prog can have a more consistent
> behavior among sk-lookup and the regular sk-reuseport-select case.
> Thought?
Ah, I see now what you had in mind. If that option is on the table, I'm
all for it. Being consistent makes it easier to explain and use.
In that case, let me make that change in a separate submission. I want
to get test coverage in for the three reuseport flavors.
> From reuseport_select_sock(), it seems the kernel's select_by_hash
> also avoids returning established sk.
Right. CC'ing Willem to check if bpf was left out on purpose or not.
> In the mid term, we may consider to remove the connected udp
> from the sockmap and reuseport_array.
SGTM.
> I am a bit confused in the current situation on bpf@reuseport returning
> connected sk and I also can't think of a use case in the
> sk-reuseport-prog-type side. It was why I was curious on
> the sk-lookup use case.
I don't know about any use cases for selecting a connected udp socket
from bpf reuseport either. It certainly sounds like unexpected behavior
for the receiving process, which expects traffic from just one remote
peer.
Marek, does anything come to mind?
If not, I guess we'll see if anyone screams when the change is proposed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup
2020-07-28 17:29 ` Jakub Sitnicki
@ 2020-07-28 19:19 ` Willem de Bruijn
0 siblings, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2020-07-28 19:19 UTC (permalink / raw)
To: Jakub Sitnicki
Cc: Martin KaFai Lau, bpf, Network Development, kernel-team,
Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Alexei Starovoitov, Marek Majkowski
> >> >> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> >> >> index c394e674f486..29d9691359b9 100644
> >> >> --- a/net/ipv6/udp.c
> >> >> +++ b/net/ipv6/udp.c
> >> >> @@ -208,7 +208,7 @@ static inline struct sock *udp6_lookup_run_bpf(struct net *net,
> >> >> return sk;
> >> >>
> >> >> reuse_sk = lookup_reuseport(net, sk, skb, saddr, sport, daddr, hnum);
> >> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
> >> >> + if (reuse_sk)
> >> > From __udp[46]_lib_lookup,
> >> > 1. The connected udp is picked by the kernel first.
> >> > If a 4-tuple-matched connected udp is found. It should have already
> >> > been returned there.
> >> >
> >> > 2. If kernel cannot find a connected udp, the sk-lookup bpf prog can
> >> > get a chance to pick another socket (likely bound to a different
> >> > IP/PORT that the packet is destinated to) by bpf_sk_lookup_assign().
> >> > However, bpf_sk_lookup_assign() does not allow TCP_ESTABLISHED.
> >> >
> >> > With the change in this patch, it then allows the reuseport-bpf-prog
> >> > to pick a connected udp which cannot be found in step (1). Can you
> >> > explain a use case for this?
> >>
> >> It is not intentional. It should not allow reuseport to pick a connected
> >> udp socket to be consistent with what sk-lookup prog can select. Thanks
> >> for pointing it out.
> >>
> >> I've incorrectly assumed that after acdcecc61285 ("udp: correct
> >> reuseport selection with connected sockets") reuseport returns only
> >> unconnected udp sockets, but thats not true for bpf reuseport.
> >>
> >> So this patch fixes one corner base, but breaks another one.
> >>
> >> I'll change the check to the below and respin:
> >>
> >> - if (reuse_sk && !reuseport_has_conns(sk, false))
> >> + if (reuse_sk && reuse_sk->sk_state != TCP_ESTABLISHED)
> > May be disallow TCP_ESTABLISHED in bpf_sk_select_reuseport() instead
> > so that the bpf reuseport prog can have a more consistent
> > behavior among sk-lookup and the regular sk-reuseport-select case.
> > Thought?
>
> Ah, I see now what you had in mind. If that option is on the table, I'm
> all for it. Being consistent makes it easier to explain and use.
>
> In that case, let me make that change in a separate submission. I want
> to get test coverage in for the three reuseport flavors.
>
> > From reuseport_select_sock(), it seems the kernel's select_by_hash
> > also avoids returning established sk.
>
> Right. CC'ing Willem to check if bpf was left out on purpose or not.
Not on purpose. I considered that this is up to the BPF program.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-28 19:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-26 12:02 [PATCH bpf-next] udp, bpf: Ignore connections in reuseport group after BPF sk lookup Jakub Sitnicki
2020-07-28 1:20 ` Martin KaFai Lau
2020-07-28 15:46 ` Jakub Sitnicki
2020-07-28 16:37 ` Martin KaFai Lau
2020-07-28 17:29 ` Jakub Sitnicki
2020-07-28 19:19 ` Willem de Bruijn
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).