| 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
| ||
|
Date: Wed, 5 Oct 2022 12:46:31 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Joe Damato <jdamato@...tly.com>
CC: <intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
<kuba@...nel.org>, <davem@...emloft.net>,
<anthony.l.nguyen@...el.com>, <jesse.brandeburg@...el.com>
Subject: Re: [next-queue 2/3] i40e: i40e_clean_tx_irq returns work done
On Wed, Oct 05, 2022 at 01:31:42AM -0700, Joe Damato wrote:
> Adjust i40e_clean_tx_irq to return the actual number of packets cleaned
> and adjust the logic in i40e_napi_poll to check this value.
>
> Signed-off-by: Joe Damato <jdamato@...tly.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 24 +++++++++++++-----------
> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 12 ++++++------
> drivers/net/ethernet/intel/i40e/i40e_xsk.h | 2 +-
> 3 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index b97c95f..ed88309 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -924,10 +924,10 @@ void i40e_detect_recover_hung(struct i40e_vsi *vsi)
> * @tx_ring: Tx ring to clean
> * @napi_budget: Used to determine if we are in netpoll
> *
> - * Returns true if there's any budget left (e.g. the clean is finished)
> + * Returns the number of packets cleaned
> **/
> -static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
> - struct i40e_ring *tx_ring, int napi_budget)
> +static int i40e_clean_tx_irq(struct i40e_vsi *vsi,
> + struct i40e_ring *tx_ring, int napi_budget)
> {
> int i = tx_ring->next_to_clean;
> struct i40e_tx_buffer *tx_buf;
> @@ -1026,7 +1026,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
> i40e_arm_wb(tx_ring, vsi, budget);
>
> if (ring_is_xdp(tx_ring))
> - return !!budget;
> + return total_packets;
>
> /* notify netdev of completed buffers */
> netdev_tx_completed_queue(txring_txq(tx_ring),
> @@ -1048,7 +1048,7 @@ static bool i40e_clean_tx_irq(struct i40e_vsi *vsi,
> }
> }
>
> - return !!budget;
> + return total_packets;
> }
>
> /**
> @@ -2689,10 +2689,12 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
> container_of(napi, struct i40e_q_vector, napi);
> struct i40e_vsi *vsi = q_vector->vsi;
> struct i40e_ring *ring;
> + bool tx_clean_complete = true;
> bool clean_complete = true;
> bool arm_wb = false;
> int budget_per_ring;
> int work_done = 0;
> + int tx_wd = 0;
>
> if (test_bit(__I40E_VSI_DOWN, vsi->state)) {
> napi_complete(napi);
> @@ -2703,12 +2705,12 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
> * budget and be more aggressive about cleaning up the Tx descriptors.
> */
> i40e_for_each_ring(ring, q_vector->tx) {
> - bool wd = ring->xsk_pool ?
> - i40e_clean_xdp_tx_irq(vsi, ring) :
> - i40e_clean_tx_irq(vsi, ring, budget);
> + tx_wd = ring->xsk_pool ?
> + i40e_clean_xdp_tx_irq(vsi, ring) :
> + i40e_clean_tx_irq(vsi, ring, budget);
>
> - if (!wd) {
> - clean_complete = false;
> + if (tx_wd >= budget) {
> + tx_clean_complete = false;
This will break for AF_XDP Tx ZC. AF_XDP Tx ZC in intel drivers ignores
budget given by NAPI. If you look at i40e_xmit_zc():
func def:
static bool i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
callsite:
return i40e_xmit_zc(tx_ring, I40E_DESC_UNUSED(tx_ring));
we give free ring space as a budget and with your change we would be
returning the amount of processed tx descriptors which you will be
comparing against NAPI budget (64, unless you have busy poll enabled with
a different batch size). Say you start with empty ring and your HW rings
are sized to 1k but there was only 512 AF_XDP descriptors ready for Tx.
You produced all of them successfully to ring and you return 512 up to
i40e_napi_poll.
> continue;
> }
> arm_wb |= ring->arm_wb;
> @@ -2742,7 +2744,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
> }
>
> /* If work not completed, return budget and polling will return */
> - if (!clean_complete) {
> + if (!clean_complete || !tx_clean_complete) {
> int cpu_id = smp_processor_id();
>
> /* It is possible that the interrupt affinity has changed but,
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 790aaeff..925682c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -531,9 +531,9 @@ static void i40e_set_rs_bit(struct i40e_ring *xdp_ring)
> * @xdp_ring: XDP Tx ring
> * @budget: NAPI budget
> *
> - * Returns true if the work is finished.
> + * Returns number of packets cleaned
> **/
> -static bool i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
> +static int i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
> {
> struct xdp_desc *descs = xdp_ring->xsk_pool->tx_descs;
> u32 nb_pkts, nb_processed = 0;
> @@ -541,7 +541,7 @@ static bool i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
>
> nb_pkts = xsk_tx_peek_release_desc_batch(xdp_ring->xsk_pool, budget);
> if (!nb_pkts)
> - return true;
> + return 0;
>
> if (xdp_ring->next_to_use + nb_pkts >= xdp_ring->count) {
> nb_processed = xdp_ring->count - xdp_ring->next_to_use;
> @@ -558,7 +558,7 @@ static bool i40e_xmit_zc(struct i40e_ring *xdp_ring, unsigned int budget)
>
> i40e_update_tx_stats(xdp_ring, nb_pkts, total_bytes);
>
> - return nb_pkts < budget;
> + return nb_pkts;
> }
>
> /**
> @@ -582,9 +582,9 @@ static void i40e_clean_xdp_tx_buffer(struct i40e_ring *tx_ring,
> * @vsi: Current VSI
> * @tx_ring: XDP Tx ring
> *
> - * Returns true if cleanup/tranmission is done.
> + * Returns number of packets cleaned
> **/
> -bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi, struct i40e_ring *tx_ring)
> +int i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi, struct i40e_ring *tx_ring)
> {
> struct xsk_buff_pool *bp = tx_ring->xsk_pool;
> u32 i, completed_frames, xsk_frames = 0;
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.h b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
> index 821df24..4e810c2 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
> @@ -30,7 +30,7 @@ int i40e_xsk_pool_setup(struct i40e_vsi *vsi, struct xsk_buff_pool *pool,
> bool i40e_alloc_rx_buffers_zc(struct i40e_ring *rx_ring, u16 cleaned_count);
> int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);
>
> -bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi, struct i40e_ring *tx_ring);
> +int i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi, struct i40e_ring *tx_ring);
> int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
> int i40e_realloc_rx_bi_zc(struct i40e_vsi *vsi, bool zc);
> void i40e_clear_rx_bi_zc(struct i40e_ring *rx_ring);
> --
> 2.7.4
>
Powered by blists - more mailing lists