netdev - Re: [PATCH v5 bpf 06/11] ice: remove redundant xdp_rxq

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: <CAJ8uoz3EQfvxqQNTmq2txz9SFv97b=8drbEB=TqmbMFqSe6y8g@mail.gmail.com>
Date: Wed, 24 Jan 2024 09:39:10 +0100
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: bpf@...r.kernel.org, ast@...nel.org, daniel@...earbox.net, 
	andrii@...nel.org, netdev@...r.kernel.org, magnus.karlsson@...el.com, 
	bjorn@...nel.org, echaudro@...hat.com, lorenzo@...nel.org, 
	martin.lau@...ux.dev, tirthendu.sarkar@...el.com, john.fastabend@...il.com, 
	horms@...nel.org
Subject: Re: [PATCH v5 bpf 06/11] ice: remove redundant xdp_rxq_info registration

On Mon, 22 Jan 2024 at 23:17, Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> xdp_rxq_info struct can be registered by drivers via two functions -
> xdp_rxq_info_reg() and __xdp_rxq_info_reg(). The latter one allows
> drivers that support XDP multi-buffer to set up xdp_rxq_info::frag_size
> which in turn will make it possible to grow the packet via
> bpf_xdp_adjust_tail() BPF helper.
>
> Currently, ice registers xdp_rxq_info in two spots:
> 1) ice_setup_rx_ring() // via xdp_rxq_info_reg(), BUG
> 2) ice_vsi_cfg_rxq()   // via __xdp_rxq_info_reg(), OK
>
> Cited commit under fixes tag took care of setting up frag_size and
> updated registration scheme in 2) but it did not help as
> 1) is called before 2) and as shown above it uses old registration
> function. This means that 2) sees that xdp_rxq_info is already
> registered and never calls __xdp_rxq_info_reg() which leaves us with
> xdp_rxq_info::frag_size being set to 0.
>
> To fix this misbehavior, simply remove xdp_rxq_info_reg() call from
> ice_setup_rx_ring().

Acked-by: Magnus Karlsson <magnus.karlsson@...el.com>

> Fixes: 2fba7dc5157b ("ice: Add support for XDP multi-buffer on Rx side")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_txrx.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> index 1760e81379cc..765aea630a1f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> @@ -513,11 +513,6 @@ int ice_setup_rx_ring(struct ice_rx_ring *rx_ring)
>         if (ice_is_xdp_ena_vsi(rx_ring->vsi))
>                 WRITE_ONCE(rx_ring->xdp_prog, rx_ring->vsi->xdp_prog);
>
> -       if (rx_ring->vsi->type == ICE_VSI_PF &&
> -           !xdp_rxq_info_is_reg(&rx_ring->xdp_rxq))
> -               if (xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
> -                                    rx_ring->q_index, rx_ring->q_vector->napi.napi_id))
> -                       goto err;
>         return 0;
>
>  err:
> --
> 2.34.1
>
>