On Thu, Jul 9, 2020 at 8:51 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> wrote:
> >>
> >> Make libbpf aware of the newly added program type, and assign it a
> >> section name.
> >>
> >> Signed-off-by: Jakub Sitnicki <jakub@xxxxxxxxxxxxxx>
> >> ---
> >>
> >> 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.
>
> >
> >> };
> >>
[...]
