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

David Ahern
2020-07-09T16:33:44Z

On 7/8/20 7:30 PM, Hangbin Liu wrote:
> This patch is for xdp multicast support. In this implementation we
> add a new helper to accept two maps: forward map and exclude map.
> We will redirect the packet to all the interfaces in *forward map*, but
> exclude the interfaces that in *exclude map*.
>

good feature. I bet we could use this to create a simpler xdp dumper -
redirect to an xdpmon device which converts to an skb and passes to any
attached sockets.

> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 10abb06065bb..617a51391971 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -512,6 +512,160 @@ int dev_map_enqueue(struct bpf_dtab_netdev *dst, struct xdp_buff *xdp,
> return __xdp_enqueue(dev, xdp, dev_rx);
> }
>
> +/* Use direct call in fast path instead of map->ops->map_get_next_key() */
> +static int devmap_get_next_key(struct bpf_map *map, void *key, void *next_key)
> +{
> +
> + switch (map->map_type) {
> + case BPF_MAP_TYPE_DEVMAP:
> + return dev_map_get_next_key(map, key, next_key);
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> + return dev_map_hash_get_next_key(map, key, next_key);
> + default:
> + break;
> + }
> +
> + return -ENOENT;
> +}
> +
> +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?

> + return true;
> +
> + err = devmap_get_next_key(map, &key, &next_key);
> + if (err)
> + break;
> +
> + key = next_key;
> + }
> +
> + return false;
> +}
> +

> @@ -3741,6 +3810,34 @@ static const struct bpf_func_proto bpf_xdp_redirect_map_proto = {
> .arg3_type = ARG_ANYTHING,
> };
>
> +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