Re: [PATCHv6 bpf-next 1/3] xdp: add a new helper for dev map multicast support

Hangbin Liu
2020-07-10T06:55:49Z

Hi David,
On Thu, Jul 09, 2020 at 10:33:38AM -0600, David Ahern wrote:
> > +bool dev_in_exclude_map(struct bpf_dtab_netdev *obj, struct bpf_map *map,
> > + int exclude_ifindex)
> > +{
> > + struct bpf_dtab_netdev *ex_obj = NULL;
> > + u32 key, next_key;
> > + int err;
> > +
> > + if (obj->dev->ifindex == exclude_ifindex)
> > + return true;
> > +
> > + if (!map)
> > + return false;
> > +
> > + err = devmap_get_next_key(map, NULL, &key);
> > + if (err)
> > + return false;
> > +
> > + for (;;) {
> > + switch (map->map_type) {
> > + case BPF_MAP_TYPE_DEVMAP:
> > + ex_obj = __dev_map_lookup_elem(map, key);
> > + break;
> > + case BPF_MAP_TYPE_DEVMAP_HASH:
> > + ex_obj = __dev_map_hash_lookup_elem(map, key);
> > + break;
> > + default:
> > + break;
> > + }
> > +
> > + if (ex_obj && ex_obj->dev->ifindex == obj->dev->ifindex)
>
> I'm probably missing something fundamental, but why do you need to walk
> the keys? Why not just do a lookup on the device index?

This functions is to check if the device index is in exclude map.

The device indexes are stored as values in the map. The user could store
the values by any key number. There is no way to lookup the device index
directly unless loop the map and check each values we stored.

Is there a map feature which could get an exact value directly?

> > +BPF_CALL_3(bpf_xdp_redirect_map_multi, struct bpf_map *, map,
> > + struct bpf_map *, ex_map, u64, flags)
> > +{
> > + struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
> > +
> > + if (unlikely(!map || flags > BPF_F_EXCLUDE_INGRESS))
>
> If flags is a bitfield, the check should be:
> flags & ~BPF_F_EXCLUDE_INGRESS

Thanks for the tips, I will fix it.

Cheers
Hangbin