mbox series

[bpf-next,V2,0/8] bpf/xdp: add flags argument to ndo_xdp_xmit and flag flush operation

Message ID 152775714013.24817.5067576840614810786.stgit@firesoul
Headers show
Series bpf/xdp: add flags argument to ndo_xdp_xmit and flag flush operation | expand

Message

Jesper Dangaard Brouer May 31, 2018, 8:59 a.m. UTC
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.

---
V2: Updated based on feedback from Song Liu <songliubraving@fb.com>


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   |   14 ++++++++++++--
 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                           |   19 ++++++-------------
 net/core/filter.c                             |    3 +--
 9 files changed, 72 insertions(+), 35 deletions(-)

--

Comments

Alexei Starovoitov June 3, 2018, 3:17 p.m. UTC | #1
On Thu, May 31, 2018 at 10:59:42AM +0200, Jesper Dangaard Brouer 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.
> 
> ---
> V2: Updated based on feedback from Song Liu <songliubraving@fb.com>

Applied, but please send a follow up patch to remove ndo_xdp_flush().
Otherwise this patch set is just a code churn that doing the opposite
of what you're trying to achieve and creating more backport pains.