From: Jakub Sitnicki <jakub@cloudflare.com>
To: Lorenz Bauer <lmb@cloudflare.com>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
dccp@vger.kernel.org, kernel-team <kernel-team@cloudflare.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Gerrit Renker <gerrit@erg.abdn.ac.uk>,
Jakub Kicinski <kuba@kernel.org>,
Andrii Nakryiko <andrii.nakryiko@gmail.com>,
Martin KaFai Lau <kafai@fb.com>,
Marek Majkowski <marek@cloudflare.com>
Subject: Re: [PATCH bpf-next v2 05/17] inet: Run SK_LOOKUP BPF program on socket lookup
Date: Wed, 13 May 2020 16:50:30 +0200 [thread overview]
Message-ID: <87v9kzubwp.fsf@cloudflare.com> (raw)
In-Reply-To: <CACAyw98ngR+nQdg-MYhGMqQkdhyOGOcWQB+fgy8eTGwKj9-Rzg@mail.gmail.com>
On Wed, May 13, 2020 at 04:21 PM CEST, Lorenz Bauer wrote:
> On Tue, 12 May 2020 at 14:52, Jakub Sitnicki <jakub@cloudflare.com> wrote:
[...]
>> So if IIUC the rough idea here would be like below?
>>
>> - 1st program calls
>>
>> bpf_sk_assign(ctx, sk1, 0 /*flags*/) -> 0 (OK)
>>
>> - 2nd program calls
>>
>> bpf_sk_assign(ctx, sk2, 0) -> -EBUSY (already selected)
>> bpf_sk_assign(ctx, sk2, BPF_EXIST) -> 0 (OK, replace existing)
>>
>> In this case the last program to run has the final say, as opposed to
>> the semantics where DROP/REDIRECT terminates.
>
> Does sk_assign from TC also gain BPF_EXIST semantics? As you know,
> I'm a bit concerned that TC and sk_lookup sk_assign are actually to completely
> separate helpers. This is a good way to figure out if its a good idea to
> overload the name, imo.
I don't have a strong opinion here. We could have a dedicated helper.
Personally I'm not finding it confusing. As a BPF user you know what
program type you're working with (TC vs SK_LOOKUP), and both helper
variants will be documented separately in the bpf-helpers man-page, like
so:
int bpf_sk_assign(struct sk_buff *skb, struct bpf_sock *sk, u64
flags)
Description
Helper is overloaded depending on BPF program
type. This description applies to
BPF_PROG_TYPE_SCHED_CLS and
BPF_PROG_TYPE_SCHED_ACT programs.
Assign the sk to the skb. When combined with
appropriate routing configuration to receive the
packet towards the socket, will cause skb to be
delivered to the specified socket. Subsequent
redirection of skb via bpf_redirect(),
bpf_clone_redirect() or other methods outside of
BPF may interfere with successful delivery to the
socket.
This operation is only valid from TC ingress
path.
The flags argument must be zero.
Return 0 on success, or a negative errno in case of
failure.
· -EINVAL Unsupported flags specified.
· -ENOENT Socket is unavailable for
assignment.
· -ENETUNREACH Socket is unreachable (wrong
netns).
·
-EOPNOTSUPP Unsupported operation, for example
a
call from outside of TC ingress.
· -ESOCKTNOSUPPORT Socket type not supported
(reuseport).
int bpf_sk_assign(struct bpf_sk_lookup *ctx, struct bpf_sock
*sk, u64 flags)
Description
Helper is overloaded depending on BPF program
type. This description applies to
BPF_PROG_TYPE_SK_LOOKUP programs.
Select the sk as a result of a socket lookup.
For the operation to succeed passed socket must
be compatible with the packet description pro‐
vided by the ctx object.
L4 protocol (IPPROTO_TCP or IPPROTO_UDP) must be
an exact match. While IP family (AF_INET or
AF_INET6) must be compatible, that is IPv6 sock‐
ets that are not v6-only can be selected for IPv4
packets.
Only TCP listeners and UDP sockets, that is sock‐
ets which have SOCK_RCU_FREE flag set, can be
selected.
The flags argument must be zero.
Return 0 on success, or a negative errno in case of
failure.
-EAFNOSUPPORT is socket family (sk->family) is
not compatible with packet family (ctx->family).
-EINVAL if unsupported flags were specified.
-EPROTOTYPE if socket L4 protocol (sk->protocol)
doesn't match packet protocol (ctx->protocol).
-ESOCKTNOSUPPORT if socket does not use RCU free‐
ing.
But it would be helpful to hear what others think about it.
next prev parent reply other threads:[~2020-05-13 15:00 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-11 18:52 [PATCH bpf-next v2 00/17] Run a BPF program on socket lookup Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 01/17] flow_dissector: Extract attach/detach/query helpers Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 02/17] bpf: Introduce SK_LOOKUP program type with a dedicated attach point Jakub Sitnicki
2020-05-11 19:06 ` Jakub Sitnicki
2020-05-13 5:41 ` Martin KaFai Lau
2020-05-13 14:34 ` Jakub Sitnicki
2020-05-13 18:10 ` Martin KaFai Lau
2020-05-11 18:52 ` [PATCH bpf-next v2 03/17] inet: Store layer 4 protocol in inet_hashinfo Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 04/17] inet: Extract helper for selecting socket from reuseport group Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 05/17] inet: Run SK_LOOKUP BPF program on socket lookup Jakub Sitnicki
2020-05-11 20:44 ` Alexei Starovoitov
2020-05-12 13:52 ` Jakub Sitnicki
2020-05-12 23:58 ` Alexei Starovoitov
2020-05-13 13:55 ` Jakub Sitnicki
2020-05-13 14:21 ` Lorenz Bauer
2020-05-13 14:50 ` Jakub Sitnicki [this message]
2020-05-15 12:28 ` Jakub Sitnicki
2020-05-15 15:07 ` Alexei Starovoitov
2020-05-11 18:52 ` [PATCH bpf-next v2 06/17] inet6: Extract helper for selecting socket from reuseport group Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 07/17] inet6: Run SK_LOOKUP BPF program on socket lookup Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 08/17] udp: Store layer 4 protocol in udp_table Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 09/17] udp: Extract helper for selecting socket from reuseport group Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 10/17] udp: Run SK_LOOKUP BPF program on socket lookup Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 11/17] udp6: Extract helper for selecting socket from reuseport group Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 12/17] udp6: Run SK_LOOKUP BPF program on socket lookup Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 13/17] bpf: Sync linux/bpf.h to tools/ Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 14/17] libbpf: Add support for SK_LOOKUP program type Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 15/17] selftests/bpf: Add verifier tests for bpf_sk_lookup context access Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 16/17] selftests/bpf: Rename test_sk_lookup_kern.c to test_ref_track_kern.c Jakub Sitnicki
2020-05-11 18:52 ` [PATCH bpf-next v2 17/17] selftests/bpf: Tests for BPF_SK_LOOKUP attach point Jakub Sitnicki
2020-05-11 19:45 ` [PATCH bpf-next v2 00/17] Run a BPF program on socket lookup Martin KaFai Lau
2020-05-12 11:57 ` Jakub Sitnicki
2020-05-12 16:34 ` Martin KaFai Lau
2020-05-13 17:54 ` Jakub Sitnicki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87v9kzubwp.fsf@cloudflare.com \
--to=jakub@cloudflare.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii.nakryiko@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dccp@vger.kernel.org \
--cc=edumazet@google.com \
--cc=gerrit@erg.abdn.ac.uk \
--cc=kafai@fb.com \
--cc=kernel-team@cloudflare.com \
--cc=kuba@kernel.org \
--cc=lmb@cloudflare.com \
--cc=marek@cloudflare.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).