netdev - Re: [PATCH bpf-next 4/9] net/mlx5e: Implement AF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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.