* Design for sk_lookup helper function in context of sk_lookup hook
@ 2021-03-17 9:04 Shanti Lombard née Bouchez-Mongardé
2021-03-19 16:55 ` Martin KaFai Lau
0 siblings, 1 reply; 4+ messages in thread
From: Shanti Lombard née Bouchez-Mongardé @ 2021-03-17 9:04 UTC (permalink / raw)
To: bpf; +Cc: netdev, alexei.starovoitov, kafai
Hello everyone,
Background discussion:
https://lore.kernel.org/bpf/CAADnVQJnX-+9u--px_VnhrMTPB=O9Y0LH9T7RJbqzfLchbUFvg@mail.gmail.com/
In a previous e-mail I was explaining that sk_lookup BPF programs had no
way to lookup existing sockets in kernel space. The sockets have to be
provided by userspace, and the userspace has to find a way to get them
and update them, which in some circumstances can be difficult. I'm
working on a patch to make socket lookup available to BPF programs in
the context of the sk_lookup hook.
There is also two helper function bpf_sk_lokup_tcp and bpf_sk_lookup_udp
which exists but are not available in the context of sk_lookup hooks.
Making them available in this context is not very difficult (just
configure it in net/core/filter.c) but those helpers will call back BPF
code as part of the socket lookup potentially causing an infinite loop.
We need to find a way to perform socket lookup but disable the BPF hook
while doing so.
Around all this, I have a few design questions :
Q1: How do we prevent socket lookup from triggering BPF sk_lookup
causing a loop?
- Solution A: We add a flag to the existing inet_lookup exported
function (and similarly for inet6, udp4 and udp6). The
INET_LOOKUP_SKIP_BPF_SK_LOOKUP flag, when set, would prevent BPF
sk_lookup from happening. It also requires modifying every location
making use of those functions.
- Solution B: We export a new symbol in inet_hashtables, a wrapper
around static function inet_lhash2_lookup for inet4 and similar
functions for inet6 and udp4/6. Looking up specific IP/port and if not
found looking up for INADDR_ANY could be done in the helper function in
net/core/filters.c or in the BPF program.
Q2: Should we reuse the bpf_sk_lokup_tcp and bpf_sk_lookup_udp helper
functions or create new ones?
For solution A above, the helper functions can be reused almose
identically, just adding a flag or boolean argument to tell if we are in
a sk_lookup program or not. In solution B is preferred, them perhaps it
would make sense to expose the new raw lookup function created, and the
BPF program would be responsible for falling back to INADDR_ANY if the
specific socket is not found. It adds more power to the BPF program in
this case but requires to create a new helper function.
I was going with Solution A abd identical function names, but as I am
touching the code it seems that maybe solution B with a new helper
function could be better. I'm open to ideas.
Thank you.
PS: please include me in replies if you are responding only to the
netdev mailing list as I'm not part of it. I'm subscribed to bpf.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Design for sk_lookup helper function in context of sk_lookup hook
2021-03-17 9:04 Design for sk_lookup helper function in context of sk_lookup hook Shanti Lombard née Bouchez-Mongardé
@ 2021-03-19 16:55 ` Martin KaFai Lau
2021-03-19 17:05 ` Shanti Lombard née Bouchez-Mongardé
0 siblings, 1 reply; 4+ messages in thread
From: Martin KaFai Lau @ 2021-03-19 16:55 UTC (permalink / raw)
To: Shanti Lombard née Bouchez-Mongardé
Cc: bpf, netdev, alexei.starovoitov
On Wed, Mar 17, 2021 at 10:04:18AM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
> Hello everyone,
>
> Background discussion: https://lore.kernel.org/bpf/CAADnVQJnX-+9u--px_VnhrMTPB=O9Y0LH9T7RJbqzfLchbUFvg@mail.gmail.com/
>
> In a previous e-mail I was explaining that sk_lookup BPF programs had no way
> to lookup existing sockets in kernel space. The sockets have to be provided
> by userspace, and the userspace has to find a way to get them and update
> them, which in some circumstances can be difficult. I'm working on a patch
> to make socket lookup available to BPF programs in the context of the
> sk_lookup hook.
>
> There is also two helper function bpf_sk_lokup_tcp and bpf_sk_lookup_udp
> which exists but are not available in the context of sk_lookup hooks. Making
> them available in this context is not very difficult (just configure it in
> net/core/filter.c) but those helpers will call back BPF code as part of the
> socket lookup potentially causing an infinite loop. We need to find a way to
> perform socket lookup but disable the BPF hook while doing so.
>
> Around all this, I have a few design questions :
>
> Q1: How do we prevent socket lookup from triggering BPF sk_lookup causing a
> loop?
The bpf_sk_lookup_(tcp|udp) will be called from the BPF_PROG_TYPE_SK_LOOKUP program?
>
> - Solution A: We add a flag to the existing inet_lookup exported function
> (and similarly for inet6, udp4 and udp6). The INET_LOOKUP_SKIP_BPF_SK_LOOKUP
> flag, when set, would prevent BPF sk_lookup from happening. It also requires
> modifying every location making use of those functions.
>
> - Solution B: We export a new symbol in inet_hashtables, a wrapper around
> static function inet_lhash2_lookup for inet4 and similar functions for inet6
> and udp4/6. Looking up specific IP/port and if not found looking up for
> INADDR_ANY could be done in the helper function in net/core/filters.c or in
> the BPF program.
>
> Q2: Should we reuse the bpf_sk_lokup_tcp and bpf_sk_lookup_udp helper
> functions or create new ones?
If the args passing to the bpf_sk_lookup_(tcp|udp) is the same,
it makes sense to reuse the same BPF_FUNC_sk_lookup_*.
The actual helper implementation could be different though.
Look at bpf_xdp_sk_lookup_tcp_proto and bpf_sk_lookup_tcp_proto.
>
> For solution A above, the helper functions can be reused almose identically,
> just adding a flag or boolean argument to tell if we are in a sk_lookup
> program or not. In solution B is preferred, them perhaps it would make sense
> to expose the new raw lookup function created, and the BPF program would be
> responsible for falling back to INADDR_ANY if the specific socket is not
> found. It adds more power to the BPF program in this case but requires to
> create a new helper function.
>
> I was going with Solution A abd identical function names, but as I am
> touching the code it seems that maybe solution B with a new helper function
> could be better. I'm open to ideas.
>
> Thank you.
>
> PS: please include me in replies if you are responding only to the netdev
> mailing list as I'm not part of it. I'm subscribed to bpf.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Design for sk_lookup helper function in context of sk_lookup hook
2021-03-19 16:55 ` Martin KaFai Lau
@ 2021-03-19 17:05 ` Shanti Lombard née Bouchez-Mongardé
2021-03-22 17:59 ` Martin KaFai Lau
0 siblings, 1 reply; 4+ messages in thread
From: Shanti Lombard née Bouchez-Mongardé @ 2021-03-19 17:05 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: bpf, netdev, alexei.starovoitov
Le 19/03/2021 à 17:55, Martin KaFai Lau a écrit :
> On Wed, Mar 17, 2021 at 10:04:18AM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
>> Q1: How do we prevent socket lookup from triggering BPF sk_lookup causing a
>> loop?
> The bpf_sk_lookup_(tcp|udp) will be called from the BPF_PROG_TYPE_SK_LOOKUP program?
Yes, the idea is to allow the BPF program to redirect incoming
connections for 0.0.0.0:1234 to a specific IP address such as
127.0.12.34:1234 or any other combinaison with a binding not done based
on a predefined socket file descriptor but based on a listening IP
address for a socket.
See linked discussion in the original message
>> - Solution A: We add a flag to the existing inet_lookup exported function
>> (and similarly for inet6, udp4 and udp6). The INET_LOOKUP_SKIP_BPF_SK_LOOKUP
>> flag, when set, would prevent BPF sk_lookup from happening. It also requires
>> modifying every location making use of those functions.
>>
>> - Solution B: We export a new symbol in inet_hashtables, a wrapper around
>> static function inet_lhash2_lookup for inet4 and similar functions for inet6
>> and udp4/6. Looking up specific IP/port and if not found looking up for
>> INADDR_ANY could be done in the helper function in net/core/filters.c or in
>> the BPF program.
>>
>> Q2: Should we reuse the bpf_sk_lokup_tcp and bpf_sk_lookup_udp helper
>> functions or create new ones?
> If the args passing to the bpf_sk_lookup_(tcp|udp) is the same,
> it makes sense to reuse the same BPF_FUNC_sk_lookup_*.
> The actual helper implementation could be different though.
> Look at bpf_xdp_sk_lookup_tcp_proto and bpf_sk_lookup_tcp_proto.
I was thinking that perhaps a different helper method which would take
IPPROTO_TCP or IPPROTO_UDP parameter would be better suited. it would
avoid BPF code such as :
switch(ctx->protocol){
case IPPROTO_TCP:
sk = bpf_sk_lookup_tcp(ctx, &tuple, tuple_size, -1, 0);
break;
case IPPROTO_UDP:
sk = bpf_sk_lookup_udp(ctx, &tuple, tuple_size, -1, 0);
break;
default:
return SK_PASS;
}
But then there is the limit of 5 arguments, isn't it, so perhaps the
_tcp/_udp functions are not such a bad idea after all.
I saw already that the same helper functions could be given different
implementations. And if there is no way to have more than 5 arguments
then this is probably better to reuse the same helper function name and
signature.
Thank you
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Design for sk_lookup helper function in context of sk_lookup hook
2021-03-19 17:05 ` Shanti Lombard née Bouchez-Mongardé
@ 2021-03-22 17:59 ` Martin KaFai Lau
0 siblings, 0 replies; 4+ messages in thread
From: Martin KaFai Lau @ 2021-03-22 17:59 UTC (permalink / raw)
To: Shanti Lombard née Bouchez-Mongardé
Cc: bpf, netdev, alexei.starovoitov
On Fri, Mar 19, 2021 at 06:05:20PM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
> Le 19/03/2021 à 17:55, Martin KaFai Lau a écrit :
> > On Wed, Mar 17, 2021 at 10:04:18AM +0100, Shanti Lombard née Bouchez-Mongardé wrote:
> > > Q1: How do we prevent socket lookup from triggering BPF sk_lookup causing a
> > > loop?
> > The bpf_sk_lookup_(tcp|udp) will be called from the BPF_PROG_TYPE_SK_LOOKUP program?
>
> Yes, the idea is to allow the BPF program to redirect incoming connections
> for 0.0.0.0:1234 to a specific IP address such as 127.0.12.34:1234 or any
> other combinaison with a binding not done based on a predefined socket file
> descriptor but based on a listening IP address for a socket.
>
> See linked discussion in the original message
>
> > > - Solution A: We add a flag to the existing inet_lookup exported function
> > > (and similarly for inet6, udp4 and udp6). The INET_LOOKUP_SKIP_BPF_SK_LOOKUP
> > > flag, when set, would prevent BPF sk_lookup from happening. It also requires
> > > modifying every location making use of those functions.
> > >
> > > - Solution B: We export a new symbol in inet_hashtables, a wrapper around
> > > static function inet_lhash2_lookup for inet4 and similar functions for inet6
> > > and udp4/6. Looking up specific IP/port and if not found looking up for
> > > INADDR_ANY could be done in the helper function in net/core/filters.c or in
> > > the BPF program.
For TCP, it is only for lhash lookup, right?
> > >
> > > Q2: Should we reuse the bpf_sk_lokup_tcp and bpf_sk_lookup_udp helper
> > > functions or create new ones?
> > If the args passing to the bpf_sk_lookup_(tcp|udp) is the same,
> > it makes sense to reuse the same BPF_FUNC_sk_lookup_*.
> > The actual helper implementation could be different though.
> > Look at bpf_xdp_sk_lookup_tcp_proto and bpf_sk_lookup_tcp_proto.
>
> I was thinking that perhaps a different helper method which would take
> IPPROTO_TCP or IPPROTO_UDP parameter would be better suited. it would avoid
> BPF code such as :
>
> switch(ctx->protocol){
> case IPPROTO_TCP:
> sk = bpf_sk_lookup_tcp(ctx, &tuple, tuple_size, -1, 0);
> break;
> case IPPROTO_UDP:
> sk = bpf_sk_lookup_udp(ctx, &tuple, tuple_size, -1, 0);
> break;
> default:
> return SK_PASS;
> }
>
> But then there is the limit of 5 arguments, isn't it, so perhaps the
> _tcp/_udp functions are not such a bad idea after all.
>
> I saw already that the same helper functions could be given different
> implementations. And if there is no way to have more than 5 arguments then
> this is probably better to reuse the same helper function name and
> signature.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-22 18:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-17 9:04 Design for sk_lookup helper function in context of sk_lookup hook Shanti Lombard née Bouchez-Mongardé
2021-03-19 16:55 ` Martin KaFai Lau
2021-03-19 17:05 ` Shanti Lombard née Bouchez-Mongardé
2021-03-22 17:59 ` Martin KaFai Lau
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).