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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200710065535.GB2531@dhcp-12-153.nay.redhat.com>
Date:   Fri, 10 Jul 2020 14:55:35 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Jiri Benc <jbenc@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Eelco Chaudron <echaudro@...hat.com>, ast@...nel.org,
        Daniel Borkmann <daniel@...earbox.net>,
        Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Subject: Re: [PATCHv6 bpf-next 1/3] xdp: add a new helper for dev map
 multicast support

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