netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Saeed Mahameed <saeed@kernel.org>
To: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Eric Dumazet <edumazet@google.com>
Cc: Saeed Mahameed <saeedm@nvidia.com>,
	netdev@vger.kernel.org, Tariq Toukan <tariqt@nvidia.com>,
	Maxim Mikityanskiy <maximmi@nvidia.com>
Subject: [net-next 13/16] net/mlx5e: xsk: Fix SKB headroom calculation in validation
Date: Tue, 27 Sep 2022 13:36:08 -0700	[thread overview]
Message-ID: <20220927203611.244301-14-saeed@kernel.org> (raw)
In-Reply-To: <20220927203611.244301-1-saeed@kernel.org>

From: Maxim Mikityanskiy <maximmi@nvidia.com>

In a typical scenario, if an XSK socket is opened first, then an XDP
program is attached, mlx5e_validate_xsk_param will be called twice:
first on XSK bind, second on channel restart caused by enabling XDP. The
validation includes a call to mlx5e_rx_is_linear_skb, which checks the
presence of the XDP program.

The above means that mlx5e_rx_is_linear_skb might return true the first
time, but false the second time, as mlx5e_rx_get_linear_sz_skb's return
value will increase, because of a different headroom used with XDP.

As XSK RQs never exist without XDP, it would make sense to trick
mlx5e_rx_get_linear_sz_skb into thinking XDP is enabled at the first
check as well. This way, if MTU is too big, it would be detected on XSK
bind, without giving false hope to the userspace application.

However, it turns out that this check is too restrictive in the first
place. SKBs created on XDP_PASS on XSK RQs don't have any headroom. That
means that big MTUs filtered out on the first and the second checks
might actually work.

So, address this issue in the proper way, but taking into account the
absence of the SKB headroom on XSK RQs, when calculating the buffer
size.

Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../ethernet/mellanox/mlx5/core/en/params.c   | 23 ++++++++-----------
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  2 +-
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
index c9a4a507a168..5dd3567d02d8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
@@ -24,24 +24,21 @@ u16 mlx5e_get_linear_rq_headroom(struct mlx5e_params *params,
 	return headroom;
 }
 
-static u32 mlx5e_rx_get_min_frag_sz(struct mlx5e_params *params,
-				    struct mlx5e_xsk_param *xsk)
+static u32 mlx5e_rx_get_linear_sz_xsk(struct mlx5e_params *params,
+				      struct mlx5e_xsk_param *xsk)
 {
 	u32 hw_mtu = MLX5E_SW2HW_MTU(params, params->sw_mtu);
-	u16 linear_rq_headroom = mlx5e_get_linear_rq_headroom(params, xsk);
 
-	return linear_rq_headroom + hw_mtu;
+	return xsk->headroom + hw_mtu;
 }
 
-static u32 mlx5e_rx_get_linear_sz_xsk(struct mlx5e_params *params,
-				      struct mlx5e_xsk_param *xsk)
+static u32 mlx5e_rx_get_linear_sz_skb(struct mlx5e_params *params, bool xsk)
 {
-	return mlx5e_rx_get_min_frag_sz(params, xsk);
-}
+	/* SKBs built on XDP_PASS on XSK RQs don't have headroom. */
+	u16 headroom = xsk ? 0 : mlx5e_get_linear_rq_headroom(params, NULL);
+	u32 hw_mtu = MLX5E_SW2HW_MTU(params, params->sw_mtu);
 
-static u32 mlx5e_rx_get_linear_sz_skb(struct mlx5e_params *params)
-{
-	return MLX5_SKB_FRAG_SZ(mlx5e_rx_get_min_frag_sz(params, NULL));
+	return MLX5_SKB_FRAG_SZ(headroom + hw_mtu);
 }
 
 static u32 mlx5e_rx_get_linear_stride_sz(struct mlx5e_params *params,
@@ -57,7 +54,7 @@ static u32 mlx5e_rx_get_linear_stride_sz(struct mlx5e_params *params,
 	if (params->xdp_prog)
 		return PAGE_SIZE;
 
-	return roundup_pow_of_two(mlx5e_rx_get_linear_sz_skb(params));
+	return roundup_pow_of_two(mlx5e_rx_get_linear_sz_skb(params, false));
 }
 
 u8 mlx5e_mpwqe_log_pkts_per_wqe(struct mlx5e_params *params,
@@ -77,7 +74,7 @@ bool mlx5e_rx_is_linear_skb(struct mlx5e_params *params,
 	/* Both XSK and non-XSK cases allocate an SKB on XDP_PASS. Packet data
 	 * must fit into a CPU page.
 	 */
-	if (mlx5e_rx_get_linear_sz_skb(params) > PAGE_SIZE)
+	if (mlx5e_rx_get_linear_sz_skb(params, xsk) > PAGE_SIZE)
 		return false;
 
 	/* XSK frames must be big enough to hold the packet data. */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 84cd86ff64d4..f8d45360a643 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4009,7 +4009,7 @@ static bool mlx5e_xsk_validate_mtu(struct net_device *netdev,
 			 * 2. Size of SKBs allocated on XDP_PASS <= PAGE_SIZE.
 			 */
 			max_mtu_frame = MLX5E_HW2SW_MTU(new_params, xsk.chunk_size - hr);
-			max_mtu_page = mlx5e_xdp_max_mtu(new_params, &xsk);
+			max_mtu_page = MLX5E_HW2SW_MTU(new_params, SKB_MAX_HEAD(0));
 			max_mtu = min(max_mtu_frame, max_mtu_page);
 
 			netdev_err(netdev, "MTU %d is too big for an XSK running on channel %u. Try MTU <= %d\n",
-- 
2.37.3


  parent reply	other threads:[~2022-09-27 20:37 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-27 20:35 [pull request][net-next 00/16] mlx5 updates 2022-09-27 Saeed Mahameed
2022-09-27 20:35 ` [net-next 01/16] net/mlx5: Add the log_min_mkey_entity_size capability Saeed Mahameed
2022-09-29  2:50   ` patchwork-bot+netdevbpf
2022-09-27 20:35 ` [net-next 02/16] net/mlx5e: Convert mlx5e_get_max_sq_wqebbs to u8 Saeed Mahameed
2022-09-27 20:35 ` [net-next 03/16] net/mlx5e: Remove unused fields from datapath structs Saeed Mahameed
2022-09-27 20:35 ` [net-next 04/16] net/mlx5e: Make mlx5e_verify_rx_mpwqe_strides static Saeed Mahameed
2022-09-27 20:36 ` [net-next 05/16] net/mlx5e: Validate striding RQ before enabling XDP Saeed Mahameed
2022-09-27 20:36 ` [net-next 06/16] net/mlx5e: Let mlx5e_get_sw_max_sq_mpw_wqebbs accept mdev Saeed Mahameed
2022-09-27 20:36 ` [net-next 07/16] net/mlx5e: Use mlx5e_stop_room_for_max_wqe where appropriate Saeed Mahameed
2022-09-27 20:36 ` [net-next 08/16] net/mlx5e: Fix a typo in mlx5e_xdp_mpwqe_is_full Saeed Mahameed
2022-09-27 20:36 ` [net-next 09/16] net/mlx5e: Use the aligned max TX MPWQE size Saeed Mahameed
2022-09-27 20:36 ` [net-next 10/16] net/mlx5e: kTLS, Check ICOSQ WQE size in advance Saeed Mahameed
2022-09-27 20:36 ` [net-next 11/16] net/mlx5e: Simplify stride size calculation for linear RQ Saeed Mahameed
2022-09-27 20:36 ` [net-next 12/16] net/mlx5e: xsk: Remove dead code in validation Saeed Mahameed
2022-09-27 20:36 ` Saeed Mahameed [this message]
2022-09-27 20:36 ` [net-next 14/16] net/mlx5e: Improve the MTU change shortcut Saeed Mahameed
2022-09-27 20:36 ` [net-next 15/16] net/mlx5e: Make dma_info array dynamic in struct mlx5e_mpw_info Saeed Mahameed
2022-09-27 20:36 ` [net-next 16/16] net/mlx5e: Use runtime values of striding RQ parameters in datapath Saeed Mahameed
2022-09-29  2:35 ` [pull request][net-next 00/16] mlx5 updates 2022-09-27 Jakub Kicinski
2022-09-29  7:20   ` Saeed Mahameed
2022-09-29 15:33     ` Jakub Kicinski

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220927203611.244301-14-saeed@kernel.org \
    --to=saeed@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=maximmi@nvidia.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=saeedm@nvidia.com \
    --cc=tariqt@nvidia.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;
as well as URLs for NNTP newsgroup(s).