Re: [xdp-hints] Re: [PATCH bpf-next v7 15/17] net/mlx5e: Introduce wrapper for xdp_buff

Stanislav Fomichev [off-list ref] writes:

On Thu, Jan 12, 2023 at 1:55 PM Toke Høiland-Jørgensen [off-list ref] wrote:

quoted

Toke Høiland-Jørgensen [off-list ref] writes:

quoted

Stanislav Fomichev [off-list ref] writes:

quoted

On Thu, Jan 12, 2023 at 12:07 AM Tariq Toukan [off-list ref] wrote:

quoted

On 12/01/2023 2:32, Stanislav Fomichev wrote:

quoted

From: Toke Høiland-Jørgensen <redacted>

Preparation for implementing HW metadata kfuncs. No functional change.

Cc: Tariq Toukan <tariqt@nvidia.com>
Cc: Saeed Mahameed <saeedm@nvidia.com>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: David Ahern <redacted>
Cc: Martin KaFai Lau <martin.lau@linux.dev>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Jesper Dangaard Brouer <redacted>
Cc: Anatoly Burakov <redacted>
Cc: Alexander Lobakin <redacted>
Cc: Magnus Karlsson <redacted>
Cc: Maryam Tahhan <redacted>
Cc: xdp-hints@xdp-project.net
Cc: netdev@vger.kernel.org
Signed-off-by: Toke Høiland-Jørgensen <redacted>
Signed-off-by: Stanislav Fomichev <redacted>
---
  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  1 +
  .../net/ethernet/mellanox/mlx5/core/en/xdp.c  |  3 +-
  .../net/ethernet/mellanox/mlx5/core/en/xdp.h  |  6 +-
  .../ethernet/mellanox/mlx5/core/en/xsk/rx.c   | 25 ++++----
  .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 58 +++++++++----------
  5 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 2d77fb8a8a01..af663978d1b4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h

@@ -469,6 +469,7 @@ struct mlx5e_txqsq {
  union mlx5e_alloc_unit {
      struct page *page;
      struct xdp_buff *xsk;
+     struct mlx5e_xdp_buff *mxbuf;

In XSK files below you mix usage of both alloc_units[page_idx].mxbuf and
alloc_units[page_idx].xsk, while both fields share the memory of a union.

As struct mlx5e_xdp_buff wraps struct xdp_buff, I think that you just
need to change the existing xsk field type from struct xdp_buff *xsk
into struct mlx5e_xdp_buff *xsk and align the usage.

Hmmm, good point. I'm actually not sure how it works currently.
mlx5e_alloc_unit.mxbuf doesn't seem to be initialized anywhere? Toke,
am I missing something?

It's initialised piecemeal in different places; but yeah, we're mixing
things a bit...

quoted

I'm thinking about something like this:

Seems more invasive? I don't care much tbf, but what's wrong with
keeping 'xdp_buff xsk' member and use it consistently?

Yeah, it's more invasive, but it's also more consistent with the non-xsk
path where every usage of struct xdp_buff is replaced with the wrapping
struct?

Both will work, I suppose (in fact I think the resulting code will be
more or less identical), so it's more a matter of which one is easier to
read and where we put the type-safety-breaking casts.

I can live with either one (just note you'll have to move the
'mxbuf->rq' initialisation next to the one for mxbuf->cqe for yours, but
that's probably fine too). Let's see which way Tariq prefers...

-Toke