On Tue, Jul 14, 2020 at 11:52:14PM +0200, Toke Høiland-Jørgensen wrote:
> > +bool dev_in_exclude_map(struct bpf_dtab_netdev *obj, struct bpf_map *map,
> > + int exclude_ifindex)
> > +{
> > + struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
> > + struct bpf_dtab_netdev *dev;
> > + struct hlist_head *head;
> > + int i = 0;
> > +
> > + if (obj->dev->ifindex == exclude_ifindex)
> > + return true;
> > +
> > + if (!map || map->map_type != BPF_MAP_TYPE_DEVMAP_HASH)
> > + return false;
>
> The map type should probably be checked earlier and the whole operation
> aborted if it is wrong...
Yes, I have already checked it in the helper, there should no need to double
check. I will remove this check.
>
> > +
> > + for (; i < dtab->n_buckets; i++) {
> > + head = dev_map_index_hash(dtab, i);
> > +
> > + dev = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),
> > + struct bpf_dtab_netdev,
> > + index_hlist);
> > +
> > + if (dev && dev->idx == exclude_ifindex)
> > + return true;
> > + }
>
> This looks broken; why are you iterating through the buckets? Shouldn't
> this just be something like:
>
> return __dev_map_hash_lookup_elem(map, obj->dev->ifindex) != NULL;
Ah, yes, I forgot this. I will update the code.
Thanks
Hangbin
