netdev - Re: [PATCH bpf-next 7/8] xdp: remove map_to

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: <20191218140318.2c1d7140@carbon>
Date:   Wed, 18 Dec 2019 14:03:18 +0100
From:   Jesper Dangaard Brouer <jbrouer@...hat.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     netdev@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        Björn Töpel <bjorn.topel@...el.com>,
        bpf@...r.kernel.org, davem@...emloft.net,
        jakub.kicinski@...ronome.com, hawk@...nel.org,
        john.fastabend@...il.com, magnus.karlsson@...el.com,
        jonathan.lemon@...il.com
Subject: Re: [PATCH bpf-next 7/8] xdp: remove map_to_flush and map swap
 detection

On Wed, 18 Dec 2019 11:53:59 +0100
Björn Töpel <bjorn.topel@...il.com> wrote:

> From: Björn Töpel <bjorn.topel@...el.com>
> 
> Now that all XDP maps that can be used with bpf_redirect_map() tracks
> entries to be flushed in a global fashion, there is not need to track
> that the map has changed and flush from xdp_do_generic_map()
> anymore. All entries will be flushed in xdp_do_flush_map().
> 
> This means that the map_to_flush can be removed, and the corresponding
> checks. Moving the flush logic to one place, xdp_do_flush_map(), give
> a bulking behavior and performance boost.
> 
> Signed-off-by: Björn Töpel <bjorn.topel@...el.com>
> ---
>  include/linux/filter.h |  1 -
>  net/core/filter.c      | 27 +++------------------------
>  2 files changed, 3 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 37ac7025031d..69d6706fc889 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -592,7 +592,6 @@ struct bpf_redirect_info {
>  	u32 tgt_index;
>  	void *tgt_value;
>  	struct bpf_map *map;
> -	struct bpf_map *map_to_flush;
>  	u32 kern_flags;
>  };
>  
> diff --git a/net/core/filter.c b/net/core/filter.c
> index c706325b3e66..d9caa3e57ea1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3547,26 +3547,9 @@ static int __bpf_tx_xdp_map(struct net_device *dev_rx, void *fwd,
>  
>  void xdp_do_flush_map(void)
>  {
> -	struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
> -	struct bpf_map *map = ri->map_to_flush;
> -
> -	ri->map_to_flush = NULL;
> -	if (map) {
> -		switch (map->map_type) {
> -		case BPF_MAP_TYPE_DEVMAP:
> -		case BPF_MAP_TYPE_DEVMAP_HASH:
> -			__dev_map_flush();
> -			break;
> -		case BPF_MAP_TYPE_CPUMAP:
> -			__cpu_map_flush();
> -			break;
> -		case BPF_MAP_TYPE_XSKMAP:
> -			__xsk_map_flush();
> -			break;
> -		default:
> -			break;
> -		}
> -	}
> +	__dev_map_flush();
> +	__cpu_map_flush();
> +	__xsk_map_flush();
>  }
>  EXPORT_SYMBOL_GPL(xdp_do_flush_map);
>  
> @@ -3615,14 +3598,10 @@ static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
>  	ri->tgt_value = NULL;
>  	WRITE_ONCE(ri->map, NULL);
>  
> -	if (ri->map_to_flush && unlikely(ri->map_to_flush != map))
> -		xdp_do_flush_map();
> -

I guess, I need to read the other patches to understand why this is valid.

The idea here is to detect if the BPF-prog are using several different
redirect maps, and do the flush if the program uses another map.  I
assume you handle this?


>  	err = __bpf_tx_xdp_map(dev, fwd, map, xdp);
>  	if (unlikely(err))
>  		goto err;
>  
> -	ri->map_to_flush = map;
>  	_trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index);
>  	return 0;
>  err:



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer