[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKH8qBsoi1PbDMwDEQ+-=r_D+=JCesV4JbZwbH7+rkjxZ3pWtg@mail.gmail.com>
Date: Mon, 14 Aug 2023 11:05:02 -0700
From: Stanislav Fomichev <sdf@...gle.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.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 Mon, Aug 14, 2023 at 4:02 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> 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?
There is always a timestamp in the tx completion descriptor, so no
need to explicitly request it.
Powered by blists - more mailing lists