> On Mon, Oct 25, 2021 at 2:18 PM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
> >
> > Introduce bpf_map_get_xdp_prog to load an eBPF program on
> > CPUMAP/DEVMAP entries since both of them share the same code.
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> > ---
> > include/linux/bpf.h | 2 ++
> > kernel/bpf/core.c | 17 +++++++++++++++++
> > kernel/bpf/cpumap.c | 12 ++++--------
> > kernel/bpf/devmap.c | 16 ++++++----------
> > 4 files changed, 29 insertions(+), 18 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 26bf8c865103..891936b54b55 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1910,6 +1910,8 @@ static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
> > return bpf_prog_get_type_dev(ufd, type, false);
> > }
> >
> > +struct bpf_prog *bpf_map_get_xdp_prog(struct bpf_map *map, int fd,
> > + enum bpf_attach_type attach_type);
> > void __bpf_free_used_maps(struct bpf_prog_aux *aux,
> > struct bpf_map **used_maps, u32 len);
> >
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index dee91a2eea7b..7e72c21b6589 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2228,6 +2228,23 @@ void __bpf_free_used_maps(struct bpf_prog_aux *aux,
> > }
> > }
> >
> > +struct bpf_prog *bpf_map_get_xdp_prog(struct bpf_map *map, int fd,
> > + enum bpf_attach_type attach_type)
> > +{
> > + struct bpf_prog *prog;
> > +
> > + prog = bpf_prog_get_type(fd, BPF_PROG_TYPE_XDP);
> > + if (IS_ERR(prog))
> > + return prog;
> > +
> > + if (prog->expected_attach_type != attach_type) {
> > + bpf_prog_put(prog);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + return prog;
> > +}
>
> It is supposed to be a cleanup... but...
>
> 1. it's tweaking __cpu_map_load_bpf_program()
> to pass extra 'map' argument further into this helper,
> but the 'map' is unused.
For xdp multi-buff we will need to extend Toke's bpf_prog_map_compatible fix
running bpf_prog_map_compatible routine for cpumaps and devmaps in
order to avoid mixing xdp mb and xdp legacy programs in a cpumaps or devmaps.
For this reason I guess we will need to pass map pointer to
__cpu_map_load_bpf_program anyway.
I do not have a strong opinion on it, but the main idea here is just to have a
common code and avoid adding the same changes to cpumap and devmap.
Anyway if you prefer to do it separately for cpumap and devmap I am fine
with it.
>
> 2. bpf_map_get_xdp_prog is a confusing name. what 'map' doing in there?
maybe bpf_xdp_map_load_prog? (naming is always hard :))
Regards,
Lorenzo
>
> 3. it's placed in core.c while it's really cpumap/devmap only.
>
> I suggest leaving the code as-is.
Attachment:
signature.asc
Description: PGP signature
