On Wed, 18 Dec 2019 at 14:03, Jesper Dangaard Brouer <jbrouer@xxxxxxxxxx> wrote:
>
> 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.
>
Please do; Review would be very much appreciated!
> 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?
>
Yes, the highlevel idea is that since the maps are dealing partly with
this already (but swaps map internal), let the map code deal with map
swaps as well.
