From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
netdev@vger.kernel.org, magnus.karlsson@intel.com
Subject: Re: [PATCH bpf-next v2 3/4] ice: xsk: improve AF_XDP ZC Tx and use batching API
Date: Thu, 30 Dec 2021 14:13:10 +0100 [thread overview]
Message-ID: <Yc2wZvfA8qr/XB8P@boxer> (raw)
In-Reply-To: <20211229131131.1460702-1-alexandr.lobakin@intel.com>
On Wed, Dec 29, 2021 at 02:11:31PM +0100, Alexander Lobakin wrote:
> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Date: Thu, 16 Dec 2021 14:59:57 +0100
>
> > Follow mostly the logic from commit 9610bd988df9 ("ice: optimize XDP_TX
> > workloads") that has been done in order to address the massive tx_busy
> > statistic bump and improve the performance as well.
> >
> > Increase the ICE_TX_THRESH to 64 as it seems to work out better for both
> > XDP and AF_XDP. Also, separating the stats structs onto separate cache
> > lines seemed to improve the performance. Batching approach is inspired
> > by i40e's implementation with adjustments to the cleaning logic.
> >
> > One difference from 'xdpdrv' XDP_TX is when ring has less than
> > ICE_TX_THRESH free entries, the cleaning routine will not stop after
> > cleaning a single ICE_TX_THRESH amount of descs but rather will forward
> > the next_dd pointer and check the DD bit and for this bit being set the
> > cleaning will be repeated. IOW clean until there are descs that can be
> > cleaned.
> >
> > It takes three separate xdpsock instances in txonly mode to achieve the
> > line rate and this was not previously possible.
> >
> > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> > ---
> > drivers/net/ethernet/intel/ice/ice_txrx.c | 2 +-
> > drivers/net/ethernet/intel/ice/ice_txrx.h | 4 +-
> > drivers/net/ethernet/intel/ice/ice_xsk.c | 249 ++++++++++++++--------
> > drivers/net/ethernet/intel/ice/ice_xsk.h | 26 ++-
> > 4 files changed, 182 insertions(+), 99 deletions(-)
> >
>
> -- 8< --
>
> > diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.h b/drivers/net/ethernet/intel/ice/ice_xsk.h
> > index 4c7bd8e9dfc4..f2eb99063c1f 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_xsk.h
> > +++ b/drivers/net/ethernet/intel/ice/ice_xsk.h
> > @@ -6,19 +6,36 @@
> > #include "ice_txrx.h"
> > #include "ice.h"
> >
> > +#define PKTS_PER_BATCH 8
> > +
> > +#ifdef __clang__
> > +#define loop_unrolled_for _Pragma("clang loop unroll_count(8)") for
> > +#elif __GNUC__ >= 4
> > +#define loop_unrolled_for _Pragma("GCC unroll 8") for
> > +#else
> > +#define loop_unrolled_for for
> > +#endif
>
> It's used in a bunch more places across the tree, what about
> defining that in linux/compiler{,_clang,_gcc}.h?
> Is it possible to pass '8' as an argument? Like
Like where besides i40e? I might currently suck at grepping, let's blame
christmas break for that.
If there are actually other callsites besides i40e then this is a good
idea to me, maybe as a follow-up?
>
> loop_unrolled_for(PKTS_PER_BATCH) ( ; ; ) { }
>
> Could be quite handy.
> If it is not, I'd maybe try to define a couple of precoded macros
> for 8, 16 and 32, like
>
> #define loop_unrolled_for_8 ...
> #define loop_unrolled_for_16 ...
> ...
>
> So they could be used as generic. I don't think I've seen them with
> values other than 8-32.
>
> > +
> > struct ice_vsi;
> >
> > #ifdef CONFIG_XDP_SOCKETS
> > int ice_xsk_pool_setup(struct ice_vsi *vsi, struct xsk_buff_pool *pool,
> > u16 qid);
> > int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget);
> > -bool ice_clean_tx_irq_zc(struct ice_tx_ring *xdp_ring, int budget);
> > int ice_xsk_wakeup(struct net_device *netdev, u32 queue_id, u32 flags);
> > bool ice_alloc_rx_bufs_zc(struct ice_rx_ring *rx_ring, u16 count);
> > bool ice_xsk_any_rx_ring_ena(struct ice_vsi *vsi);
> > void ice_xsk_clean_rx_ring(struct ice_rx_ring *rx_ring);
> > void ice_xsk_clean_xdp_ring(struct ice_tx_ring *xdp_ring);
> > +bool ice_xmit_zc(struct ice_tx_ring *xdp_ring, u32 budget);
> > #else
> > +static inline bool
> > +ice_xmit_zc(struct ice_tx_ring __always_unused *xdp_ring,
> > + u32 __always_unused budget)
> > +{
> > + return false;
> > +}
> > +
> > static inline int
> > ice_xsk_pool_setup(struct ice_vsi __always_unused *vsi,
> > struct xsk_buff_pool __always_unused *pool,
> > @@ -34,13 +51,6 @@ ice_clean_rx_irq_zc(struct ice_rx_ring __always_unused *rx_ring,
> > return 0;
> > }
> >
> > -static inline bool
> > -ice_clean_tx_irq_zc(struct ice_tx_ring __always_unused *xdp_ring,
> > - int __always_unused budget)
> > -{
> > - return false;
> > -}
> > -
> > static inline bool
> > ice_alloc_rx_bufs_zc(struct ice_rx_ring __always_unused *rx_ring,
> > u16 __always_unused count)
> > --
> > 2.33.1
>
> Thanks,
> Al
next prev parent reply other threads:[~2021-12-30 13:13 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-16 13:59 [PATCH bpf-next v2 0/4] xsk: Tx improvements Maciej Fijalkowski
2021-12-16 13:59 ` [PATCH bpf-next v2 1/4] i40e: xsk: move tmp desc array from driver to pool Maciej Fijalkowski
2021-12-16 13:59 ` [PATCH bpf-next v2 2/4] ice: xsk: avoid potential dead AF_XDP Tx processing Maciej Fijalkowski
2021-12-21 7:38 ` Magnus Karlsson
2021-12-16 13:59 ` [PATCH bpf-next v2 3/4] ice: xsk: improve AF_XDP ZC Tx and use batching API Maciej Fijalkowski
2021-12-29 3:02 ` Alexei Starovoitov
2021-12-29 13:11 ` Alexander Lobakin
2021-12-30 13:13 ` Maciej Fijalkowski [this message]
2021-12-30 16:07 ` Alexander Lobakin
2022-01-05 20:55 ` Alexei Starovoitov
2021-12-16 13:59 ` [PATCH bpf-next v2 4/4] ice: xsk: borrow xdp_tx_active logic from i40e Maciej Fijalkowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yc2wZvfA8qr/XB8P@boxer \
--to=maciej.fijalkowski@intel.com \
--cc=alexandr.lobakin@intel.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).