From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Stanislav Fomichev <sdf@google.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, daniel@iogearbox.net,
andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
yhs@fb.com, john.fastabend@gmail.com, kpsingh@kernel.org,
haoluo@google.com, jolsa@kernel.org, kuba@kernel.org,
toke@kernel.org, willemb@google.com, dsahern@kernel.org,
magnus.karlsson@intel.com, bjorn@kernel.org, hawk@kernel.org,
netdev@vger.kernel.org, xdp-hints@xdp-project.net,
Saeed Mahameed <saeedm@nvidia.com>
Subject: [xdp-hints] Re: [PATCH bpf-next 4/9] net/mlx5e: Implement AF_XDP TX timestamp and checksum offload
Date: Mon, 14 Aug 2023 13:02:28 +0200 [thread overview]
Message-ID: <ZNoJxCgGrftwt+4x@boxer> (raw)
In-Reply-To: <20230809165418.2831456-5-sdf@google.com>
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@nvidia.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.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
>
next prev parent reply other threads:[~2023-08-14 11:02 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-09 16:54 [xdp-hints] [PATCH bpf-next 0/9] xsk: TX metadata Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 1/9] xsk: Support XDP_TX_METADATA_LEN Stanislav Fomichev
2023-08-14 10:56 ` [xdp-hints] " Maciej Fijalkowski
2023-08-14 18:05 ` Stanislav Fomichev
2023-08-14 22:24 ` Stanislav Fomichev
2023-08-15 12:19 ` Magnus Karlsson
2023-08-15 18:21 ` Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 2/9] xsk: add TX timestamp and TX checksum offload support Stanislav Fomichev
2023-08-09 20:18 ` [xdp-hints] " Jesper Dangaard Brouer
2023-08-10 18:25 ` Stanislav Fomichev
2023-08-10 5:26 ` kernel test robot
2023-08-10 6:12 ` kernel test robot
2023-08-14 11:01 ` Maciej Fijalkowski
2023-08-14 18:05 ` Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 3/9] tools: ynl: print xsk-features from the sample Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 4/9] net/mlx5e: Implement AF_XDP TX timestamp and checksum offload Stanislav Fomichev
2023-08-14 11:02 ` Maciej Fijalkowski [this message]
2023-08-14 18:05 ` [xdp-hints] " Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 5/9] selftests/xsk: Support XDP_TX_METADATA_LEN Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 6/9] selftests/bpf: Add csum helpers Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 7/9] selftests/bpf: Add TX side to xdp_metadata Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 8/9] selftests/bpf: Add TX side to xdp_hw_metadata Stanislav Fomichev
2023-08-09 16:54 ` [xdp-hints] [PATCH bpf-next 9/9] xsk: document XDP_TX_METADATA_LEN layout Stanislav Fomichev
2023-08-09 20:39 ` [xdp-hints] " Jesper Dangaard Brouer
2023-08-10 18:17 ` Stanislav Fomichev
2023-08-09 20:09 ` [xdp-hints] Re: [PATCH bpf-next 0/9] xsk: TX metadata Jesper Dangaard Brouer
2023-08-10 18:23 ` Stanislav Fomichev
2023-08-14 11:13 ` Maciej Fijalkowski
2023-08-14 18:04 ` Stanislav Fomichev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZNoJxCgGrftwt+4x@boxer \
--to=maciej.fijalkowski@intel.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dsahern@kernel.org \
--cc=haoluo@google.com \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=magnus.karlsson@intel.com \
--cc=martin.lau@linux.dev \
--cc=netdev@vger.kernel.org \
--cc=saeedm@nvidia.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=toke@kernel.org \
--cc=willemb@google.com \
--cc=xdp-hints@xdp-project.net \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox