[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJ8uoz0ATqhepbBQLv0u-NVrEJeL1EZ-w+xTkW5cUeZGxCT6NA@mail.gmail.com>
Date: Thu, 25 Jan 2024 09:03:54 +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, kuba@...nel.org
Subject: Re: [PATCH v6 bpf 10/11] i40e: set xdp_rxq_info::frag_size
On Wed, 24 Jan 2024 at 20:29, Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> i40e support XDP multi-buffer so it is supposed to use
> __xdp_rxq_info_reg() instead of xdp_rxq_info_reg() and set the
> frag_size. It can not be simply converted at existing callsite because
> rx_buf_len could be un-initialized, so let us register xdp_rxq_info
> within i40e_configure_rx_ring(), which happen to be called with already
> initialized rx_buf_len value.
>
> Commit 5180ff1364bc ("i40e: use int for i40e_status") converted 'err' to
> int, so two variables to deal with return codes are not needed within
> i40e_configure_rx_ring(). Remove 'ret' and use 'err' to handle status
> from xdp_rxq_info registration.
Acked-by: Magnus Karlsson <magnus.karlsson@...el.com>
> Fixes: e213ced19bef ("i40e: add support for XDP multi-buffer Rx")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> ---
> drivers/net/ethernet/intel/i40e/i40e_main.c | 40 ++++++++++++---------
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 -----
> 2 files changed, 24 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index ae8f9f135725..d3b00d8ed39a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3588,40 +3588,48 @@ static int i40e_configure_rx_ring(struct i40e_ring *ring)
> struct i40e_hmc_obj_rxq rx_ctx;
> int err = 0;
> bool ok;
> - int ret;
>
> bitmap_zero(ring->state, __I40E_RING_STATE_NBITS);
>
> /* clear the context structure first */
> memset(&rx_ctx, 0, sizeof(rx_ctx));
>
> - if (ring->vsi->type == I40E_VSI_MAIN)
> - xdp_rxq_info_unreg_mem_model(&ring->xdp_rxq);
> + ring->rx_buf_len = vsi->rx_buf_len;
> +
> + /* XDP RX-queue info only needed for RX rings exposed to XDP */
> + if (ring->vsi->type != I40E_VSI_MAIN)
> + goto skip;
> +
> + if (!xdp_rxq_info_is_reg(&ring->xdp_rxq)) {
> + err = __xdp_rxq_info_reg(&ring->xdp_rxq, ring->netdev,
> + ring->queue_index,
> + ring->q_vector->napi.napi_id,
> + ring->rx_buf_len);
> + if (err)
> + return err;
> + }
>
> ring->xsk_pool = i40e_xsk_pool(ring);
> if (ring->xsk_pool) {
> - ring->rx_buf_len =
> - xsk_pool_get_rx_frame_size(ring->xsk_pool);
> - ret = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> + ring->rx_buf_len = xsk_pool_get_rx_frame_size(ring->xsk_pool);
> + err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> MEM_TYPE_XSK_BUFF_POOL,
> NULL);
> - if (ret)
> - return ret;
> + if (err)
> + return err;
> dev_info(&vsi->back->pdev->dev,
> "Registered XDP mem model MEM_TYPE_XSK_BUFF_POOL on Rx ring %d\n",
> ring->queue_index);
>
> } else {
> - ring->rx_buf_len = vsi->rx_buf_len;
> - if (ring->vsi->type == I40E_VSI_MAIN) {
> - ret = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> - MEM_TYPE_PAGE_SHARED,
> - NULL);
> - if (ret)
> - return ret;
> - }
> + err = xdp_rxq_info_reg_mem_model(&ring->xdp_rxq,
> + MEM_TYPE_PAGE_SHARED,
> + NULL);
> + if (err)
> + return err;
> }
>
> +skip:
> xdp_init_buff(&ring->xdp, i40e_rx_pg_size(ring) / 2, &ring->xdp_rxq);
>
> rx_ctx.dbuff = DIV_ROUND_UP(ring->rx_buf_len,
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> index 1f0a0f13a334..0d7177083708 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
> @@ -1548,7 +1548,6 @@ void i40e_free_rx_resources(struct i40e_ring *rx_ring)
> int i40e_setup_rx_descriptors(struct i40e_ring *rx_ring)
> {
> struct device *dev = rx_ring->dev;
> - int err;
>
> u64_stats_init(&rx_ring->syncp);
>
> @@ -1569,14 +1568,6 @@ int i40e_setup_rx_descriptors(struct i40e_ring *rx_ring)
> rx_ring->next_to_process = 0;
> rx_ring->next_to_use = 0;
>
> - /* XDP RX-queue info only needed for RX rings exposed to XDP */
> - if (rx_ring->vsi->type == I40E_VSI_MAIN) {
> - err = xdp_rxq_info_reg(&rx_ring->xdp_rxq, rx_ring->netdev,
> - rx_ring->queue_index, rx_ring->q_vector->napi.napi_id);
> - if (err < 0)
> - return err;
> - }
> -
> rx_ring->xdp_prog = rx_ring->vsi->xdp_prog;
>
> rx_ring->rx_bi =
> --
> 2.34.1
>
>
Powered by blists - more mailing lists