On 7/10/20 12:55 AM, Hangbin Liu wrote:
> 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.
Right.
The point of DEVMAP_HASH is to allow map management where key == device
index (vs DEVMAP which for any non-trivial use case is going to require
key != device index). You could require the exclude map to be
DEVMAP_HASH and the key to be the index allowing you to do a direct
lookup. Having to roam the entire map looking for a match does not scale
and is going to have poor performance with increasing number of entries.
XDP is targeted at performance with expert level of control, so
constraints like this have to be part of the deal.
