netdev - Re: [PATCH net 1/2] i40e: Fix failure message when XDP is configured in TX only mode

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]

Date:   Tue, 15 Nov 2022 13:13:45 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To:     Tony Nguyen <anthony.l.nguyen@...el.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
        <edumazet@...gle.com>,
        Sylwester Dziedziuch <sylwesterx.dziedziuch@...el.com>,
        <netdev@...r.kernel.org>, <magnus.karlsson@...el.com>,
        <bjorn@...nel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
        <hawk@...nel.org>, <john.fastabend@...il.com>,
        <bpf@...r.kernel.org>,
        Mateusz Palczewski <mateusz.palczewski@...el.com>,
        Shwetha Nagaraju <Shwetha.nagaraju@...el.com>
Subject: Re: [PATCH net 1/2] i40e: Fix failure message when XDP is configured
 in TX only mode

On Mon, Nov 14, 2022 at 04:03:23PM -0800, Tony Nguyen wrote:
> From: Sylwester Dziedziuch <sylwesterx.dziedziuch@...el.com>
> 
> When starting xdpsock program in TX only mode:
> 
> samples/bpf/xdpsock -i <interface> -t
> 
> there was an error on i40e driver:
> 
> Failed to allocate some buffers on AF_XDP ZC enabled Rx ring 0 (pf_q 81)
> 
> It was caused by trying to allocate RX buffers even though
> no RX buffers are available because we run in TX only mode.
> 
> Fix this by checking for number of available buffers
> for RX queue when allocating buffers during XDP setup.

I was not sure if we want to proceed with this or not. For sure it's not a
fix to me, behavior was not broken, txonly mode was working correctly.
We're only getting rid of the bogus message that caused confusion within
people.

I feel that if we want that in then we should route this via -next and
address other drivers as well. Not sure what are Magnus' thoughts on this.

> 
> Fixes: 0a714186d3c0 ("i40e: add AF_XDP zero-copy Rx support")
> Signed-off-by: Sylwester Dziedziuch <sylwesterx.dziedziuch@...el.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@...el.com>
> Tested-by: Shwetha Nagaraju <Shwetha.nagaraju@...el.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index b5dcd15ced36..41112f92f9ef 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3555,7 +3555,7 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
>  	struct i40e_hw *hw = &vsi->back->hw;
>  	struct i40e_hmc_obj_rxq rx_ctx;
>  	i40e_status err = 0;
> -	bool ok;
> +	bool ok = true;
>  	int ret;
>  
>  	bitmap_zero(ring->state, __I40E_RING_STATE_NBITS);
> @@ -3653,7 +3653,9 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
>  
>  	if (ring->xsk_pool) {
>  		xsk_pool_set_rxq_info(ring->xsk_pool, &ring->xdp_rxq);
> -		ok = i40e_alloc_rx_buffers_zc(ring, I40E_DESC_UNUSED(ring));
> +		if (ring->xsk_pool->free_list_cnt)
> +			ok = i40e_alloc_rx_buffers_zc(ring,
> +						      I40E_DESC_UNUSED(ring));
>  	} else {
>  		ok = !i40e_alloc_rx_buffers(ring, I40E_DESC_UNUSED(ring));
>  	}
> -- 
> 2.35.1
>