| Message ID | 20200702092416.11961-13-jakub@cloudflare.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Run a BPF program on socket lookup | expand |
On Thu, Jul 2, 2020 at 2:25 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > Make libbpf aware of the newly added program type, and assign it a > section name. > > Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > --- > > Notes: > v3: > - Move new libbpf symbols to version 0.1.0. > - Set expected_attach_type in probe_load for new prog type. > > v2: > - Add new libbpf symbols to version 0.0.9. (Andrii) > > tools/lib/bpf/libbpf.c | 3 +++ > tools/lib/bpf/libbpf.h | 2 ++ > tools/lib/bpf/libbpf.map | 2 ++ > tools/lib/bpf/libbpf_probes.c | 3 +++ > 4 files changed, 10 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 4ea7f4f1a691..ddcbb5dd78df 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); > BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); > BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); > +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); > > enum bpf_attach_type > bpf_program__get_expected_attach_type(struct bpf_program *prog) > @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { > BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, > BPF_CGROUP_SETSOCKOPT), > BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), > + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, > + BPF_SK_LOOKUP), So it's a BPF_PROG_TYPE_SK_LOOKUP with attach type BPF_SK_LOOKUP. What other potential attach types could there be for BPF_PROG_TYPE_SK_LOOKUP? How the section name will look like in that case? > }; > > #undef BPF_PROG_SEC_IMPL > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index 2335971ed0bd..c2272132e929 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -350,6 +350,7 @@ LIBBPF_API int bpf_program__set_perf_event(struct bpf_program *prog); > LIBBPF_API int bpf_program__set_tracing(struct bpf_program *prog); > LIBBPF_API int bpf_program__set_struct_ops(struct bpf_program *prog); > LIBBPF_API int bpf_program__set_extension(struct bpf_program *prog); > +LIBBPF_API int bpf_program__set_sk_lookup(struct bpf_program *prog); > > LIBBPF_API enum bpf_prog_type bpf_program__get_type(struct bpf_program *prog); > LIBBPF_API void bpf_program__set_type(struct bpf_program *prog, > @@ -377,6 +378,7 @@ LIBBPF_API bool bpf_program__is_perf_event(const struct bpf_program *prog); > LIBBPF_API bool bpf_program__is_tracing(const struct bpf_program *prog); > LIBBPF_API bool bpf_program__is_struct_ops(const struct bpf_program *prog); > LIBBPF_API bool bpf_program__is_extension(const struct bpf_program *prog); > +LIBBPF_API bool bpf_program__is_sk_lookup(const struct bpf_program *prog); > > /* > * No need for __attribute__((packed)), all members of 'bpf_map_def' > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map > index 6544d2cd1ed6..04b99f63a45c 100644 > --- a/tools/lib/bpf/libbpf.map > +++ b/tools/lib/bpf/libbpf.map > @@ -287,5 +287,7 @@ LIBBPF_0.1.0 { > bpf_map__type; > bpf_map__value_size; > bpf_program__autoload; > + bpf_program__is_sk_lookup; > bpf_program__set_autoload; > + bpf_program__set_sk_lookup; > } LIBBPF_0.0.9; > diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c > index 10cd8d1891f5..5a3d3f078408 100644 > --- a/tools/lib/bpf/libbpf_probes.c > +++ b/tools/lib/bpf/libbpf_probes.c > @@ -78,6 +78,9 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns, > case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: > xattr.expected_attach_type = BPF_CGROUP_INET4_CONNECT; > break; > + case BPF_PROG_TYPE_SK_LOOKUP: > + xattr.expected_attach_type = BPF_SK_LOOKUP; > + break; > case BPF_PROG_TYPE_KPROBE: > xattr.kern_version = get_kernel_version(); > break; > -- > 2.25.4 >
On Thu, Jul 09, 2020 at 06:23 AM CEST, Andrii Nakryiko wrote: > On Thu, Jul 2, 2020 at 2:25 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: >> >> Make libbpf aware of the newly added program type, and assign it a >> section name. >> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> >> --- >> >> Notes: >> v3: >> - Move new libbpf symbols to version 0.1.0. >> - Set expected_attach_type in probe_load for new prog type. >> >> v2: >> - Add new libbpf symbols to version 0.0.9. (Andrii) >> >> tools/lib/bpf/libbpf.c | 3 +++ >> tools/lib/bpf/libbpf.h | 2 ++ >> tools/lib/bpf/libbpf.map | 2 ++ >> tools/lib/bpf/libbpf_probes.c | 3 +++ >> 4 files changed, 10 insertions(+) >> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >> index 4ea7f4f1a691..ddcbb5dd78df 100644 >> --- a/tools/lib/bpf/libbpf.c >> +++ b/tools/lib/bpf/libbpf.c >> @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); >> BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); >> BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); >> BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); >> +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); >> >> enum bpf_attach_type >> bpf_program__get_expected_attach_type(struct bpf_program *prog) >> @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { >> BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, >> BPF_CGROUP_SETSOCKOPT), >> BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), >> + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, >> + BPF_SK_LOOKUP), > > So it's a BPF_PROG_TYPE_SK_LOOKUP with attach type BPF_SK_LOOKUP. What > other potential attach types could there be for > BPF_PROG_TYPE_SK_LOOKUP? How the section name will look like in that > case? BPF_PROG_TYPE_SK_LOOKUP won't have any other attach types that I can forsee. There is a single attach type shared by tcp4, tcp6, udp4, and udp6 hook points. If we hook it up in the future say to sctp, I expect the same attach point will be reused. > >> }; >> >> #undef BPF_PROG_SEC_IMPL >> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h >> index 2335971ed0bd..c2272132e929 100644 >> --- a/tools/lib/bpf/libbpf.h >> +++ b/tools/lib/bpf/libbpf.h >> @@ -350,6 +350,7 @@ LIBBPF_API int bpf_program__set_perf_event(struct bpf_program *prog); >> LIBBPF_API int bpf_program__set_tracing(struct bpf_program *prog); >> LIBBPF_API int bpf_program__set_struct_ops(struct bpf_program *prog); >> LIBBPF_API int bpf_program__set_extension(struct bpf_program *prog); >> +LIBBPF_API int bpf_program__set_sk_lookup(struct bpf_program *prog); >> >> LIBBPF_API enum bpf_prog_type bpf_program__get_type(struct bpf_program *prog); >> LIBBPF_API void bpf_program__set_type(struct bpf_program *prog, >> @@ -377,6 +378,7 @@ LIBBPF_API bool bpf_program__is_perf_event(const struct bpf_program *prog); >> LIBBPF_API bool bpf_program__is_tracing(const struct bpf_program *prog); >> LIBBPF_API bool bpf_program__is_struct_ops(const struct bpf_program *prog); >> LIBBPF_API bool bpf_program__is_extension(const struct bpf_program *prog); >> +LIBBPF_API bool bpf_program__is_sk_lookup(const struct bpf_program *prog); >> >> /* >> * No need for __attribute__((packed)), all members of 'bpf_map_def' >> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map >> index 6544d2cd1ed6..04b99f63a45c 100644 >> --- a/tools/lib/bpf/libbpf.map >> +++ b/tools/lib/bpf/libbpf.map >> @@ -287,5 +287,7 @@ LIBBPF_0.1.0 { >> bpf_map__type; >> bpf_map__value_size; >> bpf_program__autoload; >> + bpf_program__is_sk_lookup; >> bpf_program__set_autoload; >> + bpf_program__set_sk_lookup; >> } LIBBPF_0.0.9; >> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c >> index 10cd8d1891f5..5a3d3f078408 100644 >> --- a/tools/lib/bpf/libbpf_probes.c >> +++ b/tools/lib/bpf/libbpf_probes.c >> @@ -78,6 +78,9 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns, >> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: >> xattr.expected_attach_type = BPF_CGROUP_INET4_CONNECT; >> break; >> + case BPF_PROG_TYPE_SK_LOOKUP: >> + xattr.expected_attach_type = BPF_SK_LOOKUP; >> + break; >> case BPF_PROG_TYPE_KPROBE: >> xattr.kern_version = get_kernel_version(); >> break; >> -- >> 2.25.4 >>
On Thu, Jul 9, 2020 at 8:51 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > On Thu, Jul 09, 2020 at 06:23 AM CEST, Andrii Nakryiko wrote: > > On Thu, Jul 2, 2020 at 2:25 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > >> > >> Make libbpf aware of the newly added program type, and assign it a > >> section name. > >> > >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > >> --- > >> > >> Notes: > >> v3: > >> - Move new libbpf symbols to version 0.1.0. > >> - Set expected_attach_type in probe_load for new prog type. > >> > >> v2: > >> - Add new libbpf symbols to version 0.0.9. (Andrii) > >> > >> tools/lib/bpf/libbpf.c | 3 +++ > >> tools/lib/bpf/libbpf.h | 2 ++ > >> tools/lib/bpf/libbpf.map | 2 ++ > >> tools/lib/bpf/libbpf_probes.c | 3 +++ > >> 4 files changed, 10 insertions(+) > >> > >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > >> index 4ea7f4f1a691..ddcbb5dd78df 100644 > >> --- a/tools/lib/bpf/libbpf.c > >> +++ b/tools/lib/bpf/libbpf.c > >> @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > >> BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); > >> BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); > >> BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); > >> +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); > >> > >> enum bpf_attach_type > >> bpf_program__get_expected_attach_type(struct bpf_program *prog) > >> @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { > >> BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, > >> BPF_CGROUP_SETSOCKOPT), > >> BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), > >> + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, > >> + BPF_SK_LOOKUP), > > > > So it's a BPF_PROG_TYPE_SK_LOOKUP with attach type BPF_SK_LOOKUP. What > > other potential attach types could there be for > > BPF_PROG_TYPE_SK_LOOKUP? How the section name will look like in that > > case? > > BPF_PROG_TYPE_SK_LOOKUP won't have any other attach types that I can > forsee. There is a single attach type shared by tcp4, tcp6, udp4, and > udp6 hook points. If we hook it up in the future say to sctp, I expect > the same attach point will be reused. So you needed to add to bpf_attach_type just to fit into link_create model of attach_type -> prog_type, right? As I mentioned extending bpf_attach_type has a real cost on each cgroup, so we either need to solve that problem (and I think that would be the best) or we can change link_create logic to not require attach_type for programs like SK_LOOKUP, where it's clear without attach type. Second order question was if we have another attach type, having SEC("sk_lookup/just_kidding_something_else") would be a bit weird :) But it seems like that's not a concern. > > > > >> }; > >> [...]
On Fri, Jul 10, 2020 at 01:13 AM CEST, Andrii Nakryiko wrote: > On Thu, Jul 9, 2020 at 8:51 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: >> >> On Thu, Jul 09, 2020 at 06:23 AM CEST, Andrii Nakryiko wrote: >> > On Thu, Jul 2, 2020 at 2:25 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: >> >> >> >> Make libbpf aware of the newly added program type, and assign it a >> >> section name. >> >> >> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> >> >> --- >> >> >> >> Notes: >> >> v3: >> >> - Move new libbpf symbols to version 0.1.0. >> >> - Set expected_attach_type in probe_load for new prog type. >> >> >> >> v2: >> >> - Add new libbpf symbols to version 0.0.9. (Andrii) >> >> >> >> tools/lib/bpf/libbpf.c | 3 +++ >> >> tools/lib/bpf/libbpf.h | 2 ++ >> >> tools/lib/bpf/libbpf.map | 2 ++ >> >> tools/lib/bpf/libbpf_probes.c | 3 +++ >> >> 4 files changed, 10 insertions(+) >> >> >> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c >> >> index 4ea7f4f1a691..ddcbb5dd78df 100644 >> >> --- a/tools/lib/bpf/libbpf.c >> >> +++ b/tools/lib/bpf/libbpf.c >> >> @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); >> >> BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); >> >> BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); >> >> BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); >> >> +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); >> >> >> >> enum bpf_attach_type >> >> bpf_program__get_expected_attach_type(struct bpf_program *prog) >> >> @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { >> >> BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, >> >> BPF_CGROUP_SETSOCKOPT), >> >> BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), >> >> + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, >> >> + BPF_SK_LOOKUP), >> > >> > So it's a BPF_PROG_TYPE_SK_LOOKUP with attach type BPF_SK_LOOKUP. What >> > other potential attach types could there be for >> > BPF_PROG_TYPE_SK_LOOKUP? How the section name will look like in that >> > case? >> >> BPF_PROG_TYPE_SK_LOOKUP won't have any other attach types that I can >> forsee. There is a single attach type shared by tcp4, tcp6, udp4, and >> udp6 hook points. If we hook it up in the future say to sctp, I expect >> the same attach point will be reused. > > So you needed to add to bpf_attach_type just to fit into link_create > model of attach_type -> prog_type, right? As I mentioned extending > bpf_attach_type has a real cost on each cgroup, so we either need to > solve that problem (and I think that would be the best) or we can > change link_create logic to not require attach_type for programs like > SK_LOOKUP, where it's clear without attach type. Right. I was thinking about that a bit. For prog types map 1:1 to an attach type, like flow_dissector or proposed sk_lookup, we don't really to know the attach type to attach the program. PROG_QUERY is more problematic though. But I imagine we could introduce a flag like BPF_QUERY_F_BY_PROG_TYPE that would make the kernel interpret attr->query.attach_type as prog type. PROG_DETACH is yet another story but sk_lookup uses only link-based attachment, so I'm ignoring it here. What also might get in the way is the fact that there is no bpf_attach_type value reserved for unspecified attach type at the moment. We would have to ensure that the first enum, BPF_CGROUP_INET_INGRESS, is not treated as an exact attach type. > > Second order question was if we have another attach type, having > SEC("sk_lookup/just_kidding_something_else") would be a bit weird :) > But it seems like that's not a concern. Yes. Sorry, I didn't mean to leave it unanswered. Just assumed that it was obvious that it's not the case. I've been happily using the part of section name following "sk_lookup" prefix to name the programs just to make section names in ELF object unique: SEC("sk_lookup/lookup_pass") SEC("sk_lookup/lookup_drop") SEC("sk_lookup/redir_port")
On Fri, Jul 10, 2020 at 1:37 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > On Fri, Jul 10, 2020 at 01:13 AM CEST, Andrii Nakryiko wrote: > > On Thu, Jul 9, 2020 at 8:51 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > >> > >> On Thu, Jul 09, 2020 at 06:23 AM CEST, Andrii Nakryiko wrote: > >> > On Thu, Jul 2, 2020 at 2:25 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: > >> >> > >> >> Make libbpf aware of the newly added program type, and assign it a > >> >> section name. > >> >> > >> >> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> > >> >> --- > >> >> > >> >> Notes: > >> >> v3: > >> >> - Move new libbpf symbols to version 0.1.0. > >> >> - Set expected_attach_type in probe_load for new prog type. > >> >> > >> >> v2: > >> >> - Add new libbpf symbols to version 0.0.9. (Andrii) > >> >> > >> >> tools/lib/bpf/libbpf.c | 3 +++ > >> >> tools/lib/bpf/libbpf.h | 2 ++ > >> >> tools/lib/bpf/libbpf.map | 2 ++ > >> >> tools/lib/bpf/libbpf_probes.c | 3 +++ > >> >> 4 files changed, 10 insertions(+) > >> >> > >> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > >> >> index 4ea7f4f1a691..ddcbb5dd78df 100644 > >> >> --- a/tools/lib/bpf/libbpf.c > >> >> +++ b/tools/lib/bpf/libbpf.c > >> >> @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); > >> >> BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); > >> >> BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); > >> >> BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); > >> >> +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); > >> >> > >> >> enum bpf_attach_type > >> >> bpf_program__get_expected_attach_type(struct bpf_program *prog) > >> >> @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { > >> >> BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, > >> >> BPF_CGROUP_SETSOCKOPT), > >> >> BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), > >> >> + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, > >> >> + BPF_SK_LOOKUP), > >> > > >> > So it's a BPF_PROG_TYPE_SK_LOOKUP with attach type BPF_SK_LOOKUP. What > >> > other potential attach types could there be for > >> > BPF_PROG_TYPE_SK_LOOKUP? How the section name will look like in that > >> > case? > >> > >> BPF_PROG_TYPE_SK_LOOKUP won't have any other attach types that I can > >> forsee. There is a single attach type shared by tcp4, tcp6, udp4, and > >> udp6 hook points. If we hook it up in the future say to sctp, I expect > >> the same attach point will be reused. > > > > So you needed to add to bpf_attach_type just to fit into link_create > > model of attach_type -> prog_type, right? As I mentioned extending > > bpf_attach_type has a real cost on each cgroup, so we either need to > > solve that problem (and I think that would be the best) or we can > > change link_create logic to not require attach_type for programs like > > SK_LOOKUP, where it's clear without attach type. > > Right. I was thinking about that a bit. For prog types map 1:1 to an > attach type, like flow_dissector or proposed sk_lookup, we don't really > to know the attach type to attach the program. > > PROG_QUERY is more problematic though. But I imagine we could introduce > a flag like BPF_QUERY_F_BY_PROG_TYPE that would make the kernel > interpret attr->query.attach_type as prog type. > > PROG_DETACH is yet another story but sk_lookup uses only link-based > attachment, so I'm ignoring it here. > > What also might get in the way is the fact that there is no > bpf_attach_type value reserved for unspecified attach type at the > moment. We would have to ensure that the first enum, > BPF_CGROUP_INET_INGRESS, is not treated as an exact attach type. > I think we should just solve this for cgroup the same way you did it for netns. We'll keep adding new attach types regardless, so better solve the problem, rather than artificially avoid it. > > > > Second order question was if we have another attach type, having > > SEC("sk_lookup/just_kidding_something_else") would be a bit weird :) > > But it seems like that's not a concern. > > Yes. Sorry, I didn't mean to leave it unanswered. Just assumed that it > was obvious that it's not the case. > > I've been happily using the part of section name following "sk_lookup" > prefix to name the programs just to make section names in ELF object > unique: > > SEC("sk_lookup/lookup_pass") > SEC("sk_lookup/lookup_drop") > SEC("sk_lookup/redir_port") oh, right, which reminds me: how about adding / to sk_lookup in that libbpf table, so that it's always sk_lookup/<something> for section name? We did similar change to xdp_devmap recently, and it seems like a good trend overall to have / separation between program type and whatever extra name user wants to give?
On Fri, Jul 10, 2020 at 08:55 PM CEST, Andrii Nakryiko wrote: > On Fri, Jul 10, 2020 at 1:37 AM Jakub Sitnicki <jakub@cloudflare.com> wrote: [...] >> I've been happily using the part of section name following "sk_lookup" >> prefix to name the programs just to make section names in ELF object >> unique: >> >> SEC("sk_lookup/lookup_pass") >> SEC("sk_lookup/lookup_drop") >> SEC("sk_lookup/redir_port") > > oh, right, which reminds me: how about adding / to sk_lookup in that > libbpf table, so that it's always sk_lookup/<something> for section > name? We did similar change to xdp_devmap recently, and it seems like > a good trend overall to have / separation between program type and > whatever extra name user wants to give? Will do. Thanks for pointing out it. I didn't pick up on it.
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 4ea7f4f1a691..ddcbb5dd78df 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -6793,6 +6793,7 @@ BPF_PROG_TYPE_FNS(perf_event, BPF_PROG_TYPE_PERF_EVENT); BPF_PROG_TYPE_FNS(tracing, BPF_PROG_TYPE_TRACING); BPF_PROG_TYPE_FNS(struct_ops, BPF_PROG_TYPE_STRUCT_OPS); BPF_PROG_TYPE_FNS(extension, BPF_PROG_TYPE_EXT); +BPF_PROG_TYPE_FNS(sk_lookup, BPF_PROG_TYPE_SK_LOOKUP); enum bpf_attach_type bpf_program__get_expected_attach_type(struct bpf_program *prog) @@ -6969,6 +6970,8 @@ static const struct bpf_sec_def section_defs[] = { BPF_EAPROG_SEC("cgroup/setsockopt", BPF_PROG_TYPE_CGROUP_SOCKOPT, BPF_CGROUP_SETSOCKOPT), BPF_PROG_SEC("struct_ops", BPF_PROG_TYPE_STRUCT_OPS), + BPF_EAPROG_SEC("sk_lookup", BPF_PROG_TYPE_SK_LOOKUP, + BPF_SK_LOOKUP), }; #undef BPF_PROG_SEC_IMPL diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index 2335971ed0bd..c2272132e929 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -350,6 +350,7 @@ LIBBPF_API int bpf_program__set_perf_event(struct bpf_program *prog); LIBBPF_API int bpf_program__set_tracing(struct bpf_program *prog); LIBBPF_API int bpf_program__set_struct_ops(struct bpf_program *prog); LIBBPF_API int bpf_program__set_extension(struct bpf_program *prog); +LIBBPF_API int bpf_program__set_sk_lookup(struct bpf_program *prog); LIBBPF_API enum bpf_prog_type bpf_program__get_type(struct bpf_program *prog); LIBBPF_API void bpf_program__set_type(struct bpf_program *prog, @@ -377,6 +378,7 @@ LIBBPF_API bool bpf_program__is_perf_event(const struct bpf_program *prog); LIBBPF_API bool bpf_program__is_tracing(const struct bpf_program *prog); LIBBPF_API bool bpf_program__is_struct_ops(const struct bpf_program *prog); LIBBPF_API bool bpf_program__is_extension(const struct bpf_program *prog); +LIBBPF_API bool bpf_program__is_sk_lookup(const struct bpf_program *prog); /* * No need for __attribute__((packed)), all members of 'bpf_map_def' diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 6544d2cd1ed6..04b99f63a45c 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -287,5 +287,7 @@ LIBBPF_0.1.0 { bpf_map__type; bpf_map__value_size; bpf_program__autoload; + bpf_program__is_sk_lookup; bpf_program__set_autoload; + bpf_program__set_sk_lookup; } LIBBPF_0.0.9; diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c index 10cd8d1891f5..5a3d3f078408 100644 --- a/tools/lib/bpf/libbpf_probes.c +++ b/tools/lib/bpf/libbpf_probes.c @@ -78,6 +78,9 @@ probe_load(enum bpf_prog_type prog_type, const struct bpf_insn *insns, case BPF_PROG_TYPE_CGROUP_SOCK_ADDR: xattr.expected_attach_type = BPF_CGROUP_INET4_CONNECT; break; + case BPF_PROG_TYPE_SK_LOOKUP: + xattr.expected_attach_type = BPF_SK_LOOKUP; + break; case BPF_PROG_TYPE_KPROBE: xattr.kern_version = get_kernel_version(); break;
Make libbpf aware of the newly added program type, and assign it a section name. Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> --- Notes: v3: - Move new libbpf symbols to version 0.1.0. - Set expected_attach_type in probe_load for new prog type. v2: - Add new libbpf symbols to version 0.0.9. (Andrii) tools/lib/bpf/libbpf.c | 3 +++ tools/lib/bpf/libbpf.h | 2 ++ tools/lib/bpf/libbpf.map | 2 ++ tools/lib/bpf/libbpf_probes.c | 3 +++ 4 files changed, 10 insertions(+)