Re: [PATCH RFC v2 17/33] mlx5: rx queue setup time determine frame_sz for XDP

Maxim Mikityanskiy
2020-04-09T09:28:15Z

On 2020-04-08 14:52, Jesper Dangaard Brouer wrote:
> The mlx5 driver have multiple memory models, which are also changed
> according to whether a XDP bpf_prog is attached.
>
> The 'rx_striding_rq' setting is adjusted via ethtool priv-flags e.g.:
> # ethtool --set-priv-flags mlx5p2 rx_striding_rq off
>
> On the general case with 4K page_size and regular MTU packet, then
> the frame_sz is 2048 and 4096 when XDP is enabled, in both modes.
>
> The info on the given frame size is stored differently depending on the
> RQ-mode and encoded in a union in struct mlx5e_rq union wqe/mpwqe.
> In rx striding mode rq->mpwqe.log_stride_sz is either 11 or 12, which
> corresponds to 2048 or 4096 (MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ).
> In non-striding mode (MLX5_WQ_TYPE_CYCLIC) the frag_stride is stored
> in rq->wqe.info.arr[0].frag_stride.
>
> To reduce effect on fast-path, this patch determine the frame_sz at
> setup time, to avoid determining the memory model runtime.
>
> Cc: Tariq Toukan <tariqt@mellanox.com>
> Cc: Saeed Mahameed <saeedm@mellanox.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 1 +
> drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c | 1 +
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 ++++
> 3 files changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 12a61bf82c14..1f280fc142ca 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -651,6 +651,7 @@ struct mlx5e_rq {
> struct {
> u16 umem_headroom;
> u16 headroom;
> + u32 frame_sz;
> u8 map_dir; /* dma map direction */
> } buff;
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> index f049e0ac308a..de4ad2c9f49a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> @@ -137,6 +137,7 @@ bool mlx5e_xdp_handle(struct mlx5e_rq *rq, struct mlx5e_dma_info *di,
> if (xsk)
> xdp.handle = di->xsk.handle;
> xdp.rxq = &rq->xdp_rxq;
> + xdp.frame_sz = rq->buff.frame_sz;
>
> act = bpf_prog_run_xdp(prog, &xdp);
> if (xsk) {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index dd7f338425eb..b9595315c45b 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -462,6 +462,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> rq->mpwqe.num_strides =
> BIT(mlx5e_mpwqe_get_log_num_strides(mdev, params, xsk));
>
> + rq->buff.frame_sz = (1 << rq->mpwqe.log_stride_sz);

I think it won't be correct from AF_XDP perspective in case unaligned
chunks are in use. As I see by the patches for Intel's drivers,
xdp.frame_sz is set to chunk_size_nohr + headroom, which is not always a
power of two.

Moreover, it won't be correct for standard (aligned to a power of two)
chunks either, because mlx5e_rx_get_linear_frag_sz always rounds up to
PAGE_SIZE in case of XDP (this usage of striding RQ is somewhat hackish
when it comes to AF_XDP), so we will end up with frame_sz == PAGE_SIZE.

So, I think we just need to use `xsk->chunk_size` here for frame_sz. The
same for non-striding RQ.

> +
> err = mlx5e_create_rq_umr_mkey(mdev, rq);
> if (err)
> goto err_rq_wq_destroy;
> @@ -485,6 +487,8 @@ static int mlx5e_alloc_rq(struct mlx5e_channel *c,
> num_xsk_frames = wq_sz << rq->wqe.info.log_num_frags;
>
> rq->wqe.info = rqp->frags_info;
> + rq->buff.frame_sz = rq->wqe.info.arr[0].frag_stride;
> +
> rq->wqe.frags =
> kvzalloc_node(array_size(sizeof(*rq->wqe.frags),
> (wq_sz << rq->wqe.info.log_num_frags)),
>
>