On Thu, Jan 27, 2022 at 9:24 AM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
>
> > On Thu, Jan 27, 2022 at 7:37 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
> > >
> > > Lorenzo Bianconi <lorenzo@xxxxxxxxxx> writes:
> > >
> > > > Deprecate xdp_cpumap xdp_devmap sec definitions.
> > > > Introduce xdp/devmap and xdp/cpumap definitions according to the standard
> > > > for SEC("") in libbpf:
> > > > - prog_type.prog_flags/attach_place
> > > > Update cpumap/devmap samples and kselftests
> > > >
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > > > ---
> > > > samples/bpf/xdp_redirect_cpu.bpf.c | 8 ++++----
> > > > samples/bpf/xdp_redirect_map.bpf.c | 2 +-
> > > > samples/bpf/xdp_redirect_map_multi.bpf.c | 2 +-
> > > > tools/lib/bpf/libbpf.c | 12 ++++++++++--
> > > > .../bpf/progs/test_xdp_with_cpumap_frags_helpers.c | 2 +-
> > > > .../bpf/progs/test_xdp_with_cpumap_helpers.c | 2 +-
> > > > .../bpf/progs/test_xdp_with_devmap_frags_helpers.c | 2 +-
> > > > .../bpf/progs/test_xdp_with_devmap_helpers.c | 2 +-
> > > > .../selftests/bpf/progs/xdp_redirect_multi_kern.c | 2 +-
> > > > 9 files changed, 21 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/samples/bpf/xdp_redirect_cpu.bpf.c b/samples/bpf/xdp_redirect_cpu.bpf.c
> > > > index 25e3a405375f..87c54bfdbb70 100644
> > > > --- a/samples/bpf/xdp_redirect_cpu.bpf.c
> > > > +++ b/samples/bpf/xdp_redirect_cpu.bpf.c
> > > > @@ -491,7 +491,7 @@ int xdp_prognum5_lb_hash_ip_pairs(struct xdp_md *ctx)
> > > > return bpf_redirect_map(&cpu_map, cpu_dest, 0);
> > > > }
> > > >
> > > > -SEC("xdp_cpumap/redirect")
> > > > +SEC("xdp/cpumap")
> > > > int xdp_redirect_cpu_devmap(struct xdp_md *ctx)
> > > > {
> > > > void *data_end = (void *)(long)ctx->data_end;
> > > > @@ -507,19 +507,19 @@ int xdp_redirect_cpu_devmap(struct xdp_md *ctx)
> > > > return bpf_redirect_map(&tx_port, 0, 0);
> > > > }
> > > >
> > > > -SEC("xdp_cpumap/pass")
> > > > +SEC("xdp/cpumap")
> > > > int xdp_redirect_cpu_pass(struct xdp_md *ctx)
> > > > {
> > > > return XDP_PASS;
> > > > }
> > > >
> > > > -SEC("xdp_cpumap/drop")
> > > > +SEC("xdp/cpumap")
> > > > int xdp_redirect_cpu_drop(struct xdp_md *ctx)
> > > > {
> > > > return XDP_DROP;
> > > > }
> > > >
> > > > -SEC("xdp_devmap/egress")
> > > > +SEC("xdp/devmap")
> > > > int xdp_redirect_egress_prog(struct xdp_md *ctx)
> > > > {
> > > > void *data_end = (void *)(long)ctx->data_end;
> > > > diff --git a/samples/bpf/xdp_redirect_map.bpf.c b/samples/bpf/xdp_redirect_map.bpf.c
> > > > index 59efd656e1b2..415bac1758e3 100644
> > > > --- a/samples/bpf/xdp_redirect_map.bpf.c
> > > > +++ b/samples/bpf/xdp_redirect_map.bpf.c
> > > > @@ -68,7 +68,7 @@ int xdp_redirect_map_native(struct xdp_md *ctx)
> > > > return xdp_redirect_map(ctx, &tx_port_native);
> > > > }
> > > >
> > > > -SEC("xdp_devmap/egress")
> > > > +SEC("xdp/devmap")
> > > > int xdp_redirect_map_egress(struct xdp_md *ctx)
> > > > {
> > > > void *data_end = (void *)(long)ctx->data_end;
> > > > diff --git a/samples/bpf/xdp_redirect_map_multi.bpf.c b/samples/bpf/xdp_redirect_map_multi.bpf.c
> > > > index bb0a5a3bfcf0..8b2fd4ec2c76 100644
> > > > --- a/samples/bpf/xdp_redirect_map_multi.bpf.c
> > > > +++ b/samples/bpf/xdp_redirect_map_multi.bpf.c
> > > > @@ -53,7 +53,7 @@ int xdp_redirect_map_native(struct xdp_md *ctx)
> > > > return xdp_redirect_map(ctx, &forward_map_native);
> > > > }
> > > >
> > > > -SEC("xdp_devmap/egress")
> > > > +SEC("xdp/devmap")
> > > > int xdp_devmap_prog(struct xdp_md *ctx)
> > > > {
> > > > void *data_end = (void *)(long)ctx->data_end;
> > > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > > index 4ce94f4ed34a..1d97bc346be6 100644
> > > > --- a/tools/lib/bpf/libbpf.c
> > > > +++ b/tools/lib/bpf/libbpf.c
> > > > @@ -237,6 +237,8 @@ enum sec_def_flags {
> > > > SEC_SLOPPY_PFX = 16,
> > > > /* BPF program support non-linear XDP buffer */
> > > > SEC_XDP_FRAGS = 32,
> > > > + /* deprecated sec definitions not supposed to be used */
> > > > + SEC_DEPRECATED = 64,
> > > > };
> > > >
> > > > struct bpf_sec_def {
> > > > @@ -6575,6 +6577,10 @@ static int libbpf_preload_prog(struct bpf_program *prog,
> > > > if (prog->type == BPF_PROG_TYPE_XDP && (def & SEC_XDP_FRAGS))
> > > > opts->prog_flags |= BPF_F_XDP_HAS_FRAGS;
> > > >
> > > > + if (def & SEC_DEPRECATED)
> > > > + pr_warn("sec '%s' is deprecated, please use new version instead\n",
> > > > + prog->sec_name);
> > > > +
> > >
> > > How is the user supposed to figure out what "the new version" is?
> >
> >
> > Let's add the section to
> > https://github.com/libbpf/libbpf/wiki/Libbpf-1.0-migration-guide and
> > link to it from the deprecation warning.
>
> so, is it better to add an utility routine to map the deprecated sec_name to
> the new one, or is it enough to add a section in Libbpf-1.0-migration-guide?
> I am fine both ways.
Let's add a section about deprecated sections (we were also talking
about deprecating "classifier" in favor of "tc", so we can mention
that as well). It's better to maintain that mapping in a wiki than
through libbpf code, IMO. Wiki can provide more context than what we
can reasonable put into deprecation messages as well.
>
> Regards,
> Lorenzo
>
> >
> > >
> > > -Toke
> > >
