[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZNoJxCgGrftwt+4x@boxer>
Date: Mon, 14 Aug 2023 13:02:28 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Stanislav Fomichev <sdf@...gle.com>
CC: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
<andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>,
<john.fastabend@...il.com>, <kpsingh@...nel.org>, <haoluo@...gle.com>,
<jolsa@...nel.org>, <kuba@...nel.org>, <toke@...nel.org>,
<willemb@...gle.com>, <dsahern@...nel.org>, <magnus.karlsson@...el.com>,
<bjorn@...nel.org>, <hawk@...nel.org>, <netdev@...r.kernel.org>,
<xdp-hints@...-project.net>, Saeed Mahameed <saeedm@...dia.com>
Subject: Re: [PATCH bpf-next 4/9] net/mlx5e: Implement AF_XDP TX timestamp
and checksum offload
On Wed, Aug 09, 2023 at 09:54:13AM -0700, Stanislav Fomichev wrote:
> TX timestamp:
> - requires passing clock, not sure I'm passing the correct one (from
> cq->mdev), but the timestamp value looks convincing
>
> TX checksum:
> - looks like device does packet parsing (and doesn't accept custom
> start/offset), so I'm ignoring user offsets
>
> Cc: Saeed Mahameed <saeedm@...dia.com>
> Signed-off-by: Stanislav Fomichev <sdf@...gle.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 4 +-
> .../net/ethernet/mellanox/mlx5/core/en/xdp.c | 72 ++++++++++++++++---
> .../net/ethernet/mellanox/mlx5/core/en/xdp.h | 10 ++-
> .../ethernet/mellanox/mlx5/core/en/xsk/tx.c | 11 ++-
> .../net/ethernet/mellanox/mlx5/core/en_main.c | 1 +
> 5 files changed, 82 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> index 0f8f70b91485..6f38627ae7f8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
> @@ -476,10 +476,12 @@ struct mlx5e_xdp_info_fifo {
>
> struct mlx5e_xdpsq;
> struct mlx5e_xmit_data;
> +struct xsk_tx_metadata;
> typedef int (*mlx5e_fp_xmit_xdp_frame_check)(struct mlx5e_xdpsq *);
> typedef bool (*mlx5e_fp_xmit_xdp_frame)(struct mlx5e_xdpsq *,
> struct mlx5e_xmit_data *,
> - int);
> + int,
> + struct xsk_tx_metadata *);
>
> struct mlx5e_xdpsq {
> /* data path */
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> index 40589cebb773..197d372048ec 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c
> @@ -102,7 +102,7 @@ mlx5e_xmit_xdp_buff(struct mlx5e_xdpsq *sq, struct mlx5e_rq *rq,
> xdptxd->dma_addr = dma_addr;
>
> if (unlikely(!INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
> - mlx5e_xmit_xdp_frame, sq, xdptxd, 0)))
> + mlx5e_xmit_xdp_frame, sq, xdptxd, 0, NULL)))
> return false;
>
> /* xmit_mode == MLX5E_XDP_XMIT_MODE_FRAME */
> @@ -144,7 +144,7 @@ mlx5e_xmit_xdp_buff(struct mlx5e_xdpsq *sq, struct mlx5e_rq *rq,
> xdptxd->dma_addr = dma_addr;
>
> if (unlikely(!INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
> - mlx5e_xmit_xdp_frame, sq, xdptxd, 0)))
> + mlx5e_xmit_xdp_frame, sq, xdptxd, 0, NULL)))
> return false;
>
> /* xmit_mode == MLX5E_XDP_XMIT_MODE_PAGE */
> @@ -260,6 +260,37 @@ const struct xdp_metadata_ops mlx5e_xdp_metadata_ops = {
> .xmo_rx_hash = mlx5e_xdp_rx_hash,
> };
>
> +struct mlx5e_xsk_tx_complete {
> + struct mlx5_cqe64 *cqe;
> + struct mlx5e_cq *cq;
> +};
> +
> +static u64 mlx5e_xsk_fill_timestamp(void *_priv)
> +{
> + struct mlx5e_xsk_tx_complete *priv = _priv;
> + u64 ts;
> +
> + ts = get_cqe_ts(priv->cqe);
> +
> + if (mlx5_is_real_time_rq(priv->cq->mdev) || mlx5_is_real_time_sq(priv->cq->mdev))
> + return mlx5_real_time_cyc2time(&priv->cq->mdev->clock, ts);
> +
> + return mlx5_timecounter_cyc2time(&priv->cq->mdev->clock, ts);
> +}
> +
> +static void mlx5e_xsk_request_checksum(u16 csum_start, u16 csum_offset, void *priv)
> +{
> + struct mlx5_wqe_eth_seg *eseg = priv;
> +
> + /* HW/FW is doing parsing, so offsets are largely ignored. */
> + eseg->cs_flags |= MLX5_ETH_WQE_L3_CSUM | MLX5_ETH_WQE_L4_CSUM;
> +}
> +
> +const struct xsk_tx_metadata_ops mlx5e_xsk_tx_metadata_ops = {
> + .tmo_fill_timestamp = mlx5e_xsk_fill_timestamp,
> + .tmo_request_checksum = mlx5e_xsk_request_checksum,
Can you explain to us why mlx5 doesn't need to implement the request
timestamp op?
> +};
> +
> /* returns true if packet was consumed by xdp */
> bool mlx5e_xdp_handle(struct mlx5e_rq *rq,
> struct bpf_prog *prog, struct mlx5e_xdp_buff *mxbuf)
> @@ -397,11 +428,11 @@ INDIRECT_CALLABLE_SCOPE int mlx5e_xmit_xdp_frame_check_mpwqe(struct mlx5e_xdpsq
>
> INDIRECT_CALLABLE_SCOPE bool
> mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
> - int check_result);
> + int check_result, struct xsk_tx_metadata *meta);
>
> INDIRECT_CALLABLE_SCOPE bool
> mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
> - int check_result)
> + int check_result, struct xsk_tx_metadata *meta)
> {
> struct mlx5e_tx_mpwqe *session = &sq->mpwqe;
> struct mlx5e_xdpsq_stats *stats = sq->stats;
> @@ -419,7 +450,7 @@ mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptx
> */
> if (unlikely(sq->mpwqe.wqe))
> mlx5e_xdp_mpwqe_complete(sq);
> - return mlx5e_xmit_xdp_frame(sq, xdptxd, 0);
> + return mlx5e_xmit_xdp_frame(sq, xdptxd, 0, meta);
> }
> if (!xdptxd->len) {
> skb_frag_t *frag = &xdptxdf->sinfo->frags[0];
> @@ -449,6 +480,7 @@ mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptx
> * and it's safe to complete it at any time.
> */
> mlx5e_xdp_mpwqe_session_start(sq);
> + xsk_tx_metadata_request(meta, &mlx5e_xsk_tx_metadata_ops, &session->wqe->eth);
> }
>
> mlx5e_xdp_mpwqe_add_dseg(sq, p, stats);
> @@ -479,7 +511,7 @@ INDIRECT_CALLABLE_SCOPE int mlx5e_xmit_xdp_frame_check(struct mlx5e_xdpsq *sq)
>
> INDIRECT_CALLABLE_SCOPE bool
> mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
> - int check_result)
> + int check_result, struct xsk_tx_metadata *meta)
> {
> struct mlx5e_xmit_data_frags *xdptxdf =
> container_of(xdptxd, struct mlx5e_xmit_data_frags, xd);
> @@ -598,6 +630,8 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
> sq->pc++;
> }
>
> + xsk_tx_metadata_request(meta, &mlx5e_xsk_tx_metadata_ops, eseg);
> +
> sq->doorbell_cseg = cseg;
>
> stats->xmit++;
> @@ -607,7 +641,9 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
> static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
> struct mlx5e_xdp_wqe_info *wi,
> u32 *xsk_frames,
> - struct xdp_frame_bulk *bq)
> + struct xdp_frame_bulk *bq,
> + struct mlx5e_cq *cq,
> + struct mlx5_cqe64 *cqe)
> {
> struct mlx5e_xdp_info_fifo *xdpi_fifo = &sq->db.xdpi_fifo;
> u16 i;
> @@ -667,10 +703,24 @@ static void mlx5e_free_xdpsq_desc(struct mlx5e_xdpsq *sq,
>
> break;
> }
> - case MLX5E_XDP_XMIT_MODE_XSK:
> + case MLX5E_XDP_XMIT_MODE_XSK: {
> /* AF_XDP send */
> + struct mlx5e_xsk_tx_complete priv = {
> + .cqe = cqe,
> + .cq = cq,
> + };
> + struct xsk_tx_metadata *meta = NULL;
> +
> + if (xp_tx_metadata_enabled(sq->xsk_pool)) {
> + xdpi = mlx5e_xdpi_fifo_pop(xdpi_fifo);
> + meta = (void *)xdpi.frame.xsk_meta;
> +
> + xsk_tx_metadata_complete(meta, &mlx5e_xsk_tx_metadata_ops, &priv);
> + }
> +
> (*xsk_frames)++;
> break;
> + }
> default:
> WARN_ON_ONCE(true);
> }
> @@ -719,7 +769,7 @@ bool mlx5e_poll_xdpsq_cq(struct mlx5e_cq *cq)
>
> sqcc += wi->num_wqebbs;
>
> - mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, &bq);
> + mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, &bq, cq, cqe);
> } while (!last_wqe);
>
> if (unlikely(get_cqe_opcode(cqe) != MLX5_CQE_REQ)) {
> @@ -766,7 +816,7 @@ void mlx5e_free_xdpsq_descs(struct mlx5e_xdpsq *sq)
>
> sq->cc += wi->num_wqebbs;
>
> - mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, &bq);
> + mlx5e_free_xdpsq_desc(sq, wi, &xsk_frames, &bq, NULL, NULL);
> }
>
> xdp_flush_frame_bulk(&bq);
> @@ -839,7 +889,7 @@ int mlx5e_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
> }
>
> ret = INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
> - mlx5e_xmit_xdp_frame, sq, xdptxd, 0);
> + mlx5e_xmit_xdp_frame, sq, xdptxd, 0, NULL);
> if (unlikely(!ret)) {
> int j;
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
> index 9e8e6184f9e4..2fcd19c16103 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xdp.h
> @@ -82,13 +82,14 @@ enum mlx5e_xdp_xmit_mode {
> * num, page_1, page_2, ... , page_num.
> *
> * MLX5E_XDP_XMIT_MODE_XSK:
> - * none.
> + * frame.xsk_meta.
> */
> union mlx5e_xdp_info {
> enum mlx5e_xdp_xmit_mode mode;
> union {
> struct xdp_frame *xdpf;
> dma_addr_t dma_addr;
> + void *xsk_meta;
> } frame;
> union {
> struct mlx5e_rq *rq;
> @@ -110,13 +111,16 @@ int mlx5e_xdp_xmit(struct net_device *dev, int n, struct xdp_frame **frames,
> u32 flags);
>
> extern const struct xdp_metadata_ops mlx5e_xdp_metadata_ops;
> +extern const struct xsk_tx_metadata_ops mlx5e_xsk_tx_metadata_ops;
>
> INDIRECT_CALLABLE_DECLARE(bool mlx5e_xmit_xdp_frame_mpwqe(struct mlx5e_xdpsq *sq,
> struct mlx5e_xmit_data *xdptxd,
> - int check_result));
> + int check_result,
> + struct xsk_tx_metadata *meta));
> INDIRECT_CALLABLE_DECLARE(bool mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq,
> struct mlx5e_xmit_data *xdptxd,
> - int check_result));
> + int check_result,
> + struct xsk_tx_metadata *meta));
> INDIRECT_CALLABLE_DECLARE(int mlx5e_xmit_xdp_frame_check_mpwqe(struct mlx5e_xdpsq *sq));
> INDIRECT_CALLABLE_DECLARE(int mlx5e_xmit_xdp_frame_check(struct mlx5e_xdpsq *sq));
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> index 597f319d4770..2f69c7912490 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> @@ -55,12 +55,16 @@ static void mlx5e_xsk_tx_post_err(struct mlx5e_xdpsq *sq,
>
> nopwqe = mlx5e_post_nop(&sq->wq, sq->sqn, &sq->pc);
> mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo, *xdpi);
> + if (xp_tx_metadata_enabled(sq->xsk_pool))
> + mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo,
> + (union mlx5e_xdp_info) { .frame.xsk_meta = NULL });
> sq->doorbell_cseg = &nopwqe->ctrl;
> }
>
> bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
> {
> struct xsk_buff_pool *pool = sq->xsk_pool;
> + struct xsk_tx_metadata *meta = NULL;
> union mlx5e_xdp_info xdpi;
> bool work_done = true;
> bool flush = false;
> @@ -93,12 +97,13 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
> xdptxd.dma_addr = xsk_buff_raw_get_dma(pool, desc.addr);
> xdptxd.data = xsk_buff_raw_get_data(pool, desc.addr);
> xdptxd.len = desc.len;
> + meta = xsk_buff_get_metadata(pool, desc.addr);
>
> xsk_buff_raw_dma_sync_for_device(pool, xdptxd.dma_addr, xdptxd.len);
>
> ret = INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
> mlx5e_xmit_xdp_frame, sq, &xdptxd,
> - check_result);
> + check_result, meta);
> if (unlikely(!ret)) {
> if (sq->mpwqe.wqe)
> mlx5e_xdp_mpwqe_complete(sq);
> @@ -106,6 +111,10 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
> mlx5e_xsk_tx_post_err(sq, &xdpi);
> } else {
> mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo, xdpi);
> + if (xp_tx_metadata_enabled(sq->xsk_pool))
> + mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo,
> + (union mlx5e_xdp_info)
> + { .frame.xsk_meta = (void *)meta });
> }
>
> flush = true;
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 1c820119e438..99c2a6babaea 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -5097,6 +5097,7 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
>
> netdev->netdev_ops = &mlx5e_netdev_ops;
> netdev->xdp_metadata_ops = &mlx5e_xdp_metadata_ops;
> + netdev->xsk_tx_metadata_ops = &mlx5e_xsk_tx_metadata_ops;
>
> mlx5e_dcbnl_build_netdev(netdev);
>
> --
> 2.41.0.640.ga95def55d0-goog
>
Powered by blists - more mailing lists