On Tue, Sep 27, 2022 at 5:40 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 9/27/22 11:23 AM, Pramukh Naduthota wrote:
> > Ignore BPF_F_RDONLY_PROG when checking for compatibility for devmaps. The
> > kernel adds the flag to all devmap creates, and this breaks pinning
> > behavior, as libbpf will then check the actual vs user supplied flags and
> > see this difference. Work around this by adding RDONLY_PROG to the
> > users's flags when testing against the pinned map
> >
> > Fixes: 57a00f41644f ("libbpf: Add auto-pinning of maps when loading BPF objects")
> > Signed-off-by: Pramukh Naduthota <pnaduthota@xxxxxxxxxx>
> > ---
> > tools/lib/bpf/libbpf.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 50d41815f4..a3dae26d82 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -4818,6 +4818,7 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
> > char msg[STRERR_BUFSIZE];
> > __u32 map_info_len;
> > int err;
> > + unsigned int effective_flags = map->def.map_flags;
> >
> > map_info_len = sizeof(map_info);
> >
> > @@ -4830,11 +4831,16 @@ static bool map_is_reuse_compat(const struct bpf_map *map, int map_fd)
> > return false;
> > }
> >
> > + /* The kernel adds RDONLY_PROG to devmaps */
> > + if (map->def.type == BPF_MAP_TYPE_DEVMAP ||
> > + map->def.type == BPF_MAP_TYPE_DEVMAP_HASH)
> > + effective_flags |= BPF_F_RDONLY_PROG;
>
> May be set BPF_F_RDONLY_PROG in effective_flags only when map->def.map_flags
> does not have both BPF_F_RDONLY_PROG and BPF_F_WRONLY_PROG set? Just in case
> the devmap may support setting them during map creation in the future.
>
Would it be sane to just ignore
BPF_F_RDONLY|BPF_F_WRONLY|BPF_F_RDONLY_PROG|BPF_F_WRONLY_PROG flags
during this comparison? Those flags don't really change compatibility
of two maps in terms of their definition, right? Just that for some
combination of those flags BPF program load might fail (e.g., if it is
trying to modify BPF_F_RDONLY_PROG map). But that will be pretty
obvious from BPF the verifier log?
> > +
> > return (map_info.type == map->def.type &&
> > map_info.key_size == map->def.key_size &&
> > map_info.value_size == map->def.value_size &&
> > map_info.max_entries == map->def.max_entries &&
> > - map_info.map_flags == map->def.map_flags &&
> > + map_info.map_flags == effective_flags &&
> > map_info.map_extra == map->map_extra);
> > }
> >
>
