Re: [RFC bpf-next 3/5] libbpf: Pass prog_ifindex via bpf_object_open_opts

From: Stanislav Fomichev <sdf@google.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
	haoluo@google.com, jolsa@kernel.org,
	Jakub Kicinski <kuba@kernel.org>,
	Willem de Bruijn <willemb@google.com>,
	Jesper Dangaard Brouer <brouer@redhat.com>,
	Anatoly Burakov <anatoly.burakov@intel.com>,
	Alexander Lobakin <alexandr.lobakin@intel.com>,
	Magnus Karlsson <magnus.karlsson@gmail.com>,
	Maryam Tahhan <mtahhan@redhat.com>,
	xdp-hints@xdp-project.net, netdev@vger.kernel.org
Subject: Re: [RFC bpf-next 3/5] libbpf: Pass prog_ifindex via bpf_object_open_opts
Date: Thu, 27 Oct 2022 13:10:33 -0700	[thread overview]
Message-ID: <CAKH8qBt_GS-9jdR1fDHNBgDHTOn6Zqh79X_8CNOFzidufLZZ_g@mail.gmail.com> (raw)
In-Reply-To: <CAEf4BzbgjOaxVFAfnrRyPP0_1Rh-gC3er4tKLkfr=OPb_x-ueA@mail.gmail.com>

On Thu, Oct 27, 2022 at 1:05 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Oct 27, 2022 at 1:00 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Allow passing prog_ifindex to BPF_PROG_LOAD. This patch is
> > not XDP metadata specific but it's here because we (ab)use
> > prog_ifindex as "target metadata" device during loading.
> > We can figure out proper UAPI story if we decide to go forward
> > with the kfunc approach.
> >
> > Cc: Martin KaFai Lau <martin.lau@linux.dev>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Jesper Dangaard Brouer <brouer@redhat.com>
> > Cc: Anatoly Burakov <anatoly.burakov@intel.com>
> > Cc: Alexander Lobakin <alexandr.lobakin@intel.com>
> > Cc: Magnus Karlsson <magnus.karlsson@gmail.com>
> > Cc: Maryam Tahhan <mtahhan@redhat.com>
> > Cc: xdp-hints@xdp-project.net
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 1 +
> >  tools/lib/bpf/libbpf.h | 6 +++++-
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 5d7819edf074..61bc37006fe4 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -7190,6 +7190,7 @@ static int bpf_object_init_progs(struct bpf_object *obj, const struct bpf_object
> >
> >                 prog->type = prog->sec_def->prog_type;
> >                 prog->expected_attach_type = prog->sec_def->expected_attach_type;
> > +               prog->prog_ifindex = opts->prog_ifindex;
> >
> >                 /* sec_def can have custom callback which should be called
> >                  * after bpf_program is initialized to adjust its properties
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index eee883f007f9..4a40b7623099 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -170,9 +170,13 @@ struct bpf_object_open_opts {
> >          */
> >         __u32 kernel_log_level;
> >
> > +       /* Optional ifindex of netdev for offload purposes.
> > +        */
> > +       int prog_ifindex;
> > +
>
> nope, don't do that, open_opts are for entire object, while this is
> per-program thing
>
> So bpf_program__set_ifindex() setter would be more appropriate

Oh, doh, not sure how I missed that. Thanks!

>
> >         size_t :0;
> >  };
> > -#define bpf_object_open_opts__last_field kernel_log_level
> > +#define bpf_object_open_opts__last_field prog_ifindex
> >
> >  LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
> >
> > --
> > 2.38.1.273.g43a17bfeac-goog
> >