On Wed, 18 Dec 2019 11:53:59 +0100
Björn Töpel <bjorn.topel@xxxxxxxxx> wrote:
> From: Björn Töpel <bjorn.topel@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> 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
