netdev - Re: [bpf-next V1 PATCH 0/8] bpf/xdp: add flags argument to ndo_xdp

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: <CAPhsuW4kVDb4sk8UGOFx4cac5X7bBG_MTzSDsDzJLThcwAsnpQ@mail.gmail.com>
Date:   Wed, 30 May 2018 15:18:02 -0700
From:   Song Liu <liu.song.a23@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     Networking <netdev@...r.kernel.org>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        John Fastabend <john.fastabend@...il.com>,
        makita.toshiaki@....ntt.co.jp
Subject: Re: [bpf-next V1 PATCH 0/8] bpf/xdp: add flags argument to
 ndo_xdp_xmit and flag flush operation

Overall, this set looks good to me. The only suggestion I have is to add more
documentation on the expected behavior of XDP_XMIT_FLUSH in netdevice.h
(as part of 01/08).

Thanks,
Song


On Wed, May 30, 2018 at 11:00 AM, Jesper Dangaard Brouer
<brouer@...hat.com> wrote:
> As I mentioned in merge commit 10f678683e4 ("Merge branch 'xdp_xmit-bulking'")
> I plan to change the API for ndo_xdp_xmit once more, by adding a flags
> argument, which is done in this patchset.
>
> I know it is late in the cycle (currently at rc7), but it would be
> nice to avoid changing NDOs over several kernel releases, as it is
> annoying to vendors and distro backporters, but it is not strictly
> UAPI so it is allowed (according to Alexei).
>
> The end-goal is getting rid of the ndo_xdp_flush operation, as it will
> make it possible for drivers to implement a TXQ synchronization mechanism
> that is not necessarily derived from the CPU id (smp_processor_id).
>
> This patchset removes all callers of the ndo_xdp_flush operation, but
> it doesn't take the last step of removing it from all drivers.  This
> can be done later, or I can update the patchset on request.
>
> Micro-benchmarks only show a very small performance improvement, for
> map-redirect around ~2 ns, and for non-map redirect ~7 ns.  I've not
> benchmarked this with CONFIG_RETPOLINE, but the performance benefit
> should be more visible given we end-up removing an indirect call.
>
> ---
>
> Jesper Dangaard Brouer (8):
>       xdp: add flags argument to ndo_xdp_xmit API
>       i40e: implement flush flag for ndo_xdp_xmit
>       ixgbe: implement flush flag for ndo_xdp_xmit
>       tun: implement flush flag for ndo_xdp_xmit
>       virtio_net: implement flush flag for ndo_xdp_xmit
>       xdp: done implementing ndo_xdp_xmit flush flag for all drivers
>       bpf/xdp: non-map redirect can avoid calling ndo_xdp_flush
>       bpf/xdp: devmap can avoid calling ndo_xdp_flush
>
>
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   |    9 ++++++++-
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |    3 ++-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   23 +++++++++++++++++------
>  drivers/net/tun.c                             |   25 ++++++++++++++++++-------
>  drivers/net/virtio_net.c                      |    9 ++++++++-
>  include/linux/netdevice.h                     |    7 ++++---
>  include/net/xdp.h                             |    4 ++++
>  kernel/bpf/devmap.c                           |   20 +++++++-------------
>  net/core/filter.c                             |    3 +--
>  9 files changed, 69 insertions(+), 34 deletions(-)
>
> --