netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [pull request][net 00/10] mlx5 fixes 2023-02-23
@ 2023-02-23 22:52 Saeed Mahameed
  2023-02-23 22:52 ` [net 01/10] net/mlx5: Remove NULL check before dev_{put, hold} Saeed Mahameed
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan

From: Saeed Mahameed <saeedm@nvidia.com>

This series provides bug fixes for mlx5 driver.
Please pull and let me know if there is any problem.

Thanks,
Saeed.


The following changes since commit 68ba44639537de6f91fe32783766322d41848127:

  sctp: add a refcnt in sctp_stream_priorities to avoid a nested loop (2023-02-23 12:59:40 -0800)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2023-02-23

for you to fetch changes up to 290a9c352e2a7c422af059d585352e3a7a44c0d1:

  net/mlx5: Geneve, Fix handling of Geneve object id as error code (2023-02-23 14:50:39 -0800)

----------------------------------------------------------------
mlx5-fixes-2023-02-23

----------------------------------------------------------------
Maher Sanalla (1):
      net/mlx5: ECPF, wait for VF pages only after disabling host PFs

Maor Dickman (1):
      net/mlx5: Geneve, Fix handling of Geneve object id as error code

Maxim Mikityanskiy (2):
      net/mlx5e: XDP, Allow growing tail for XDP multi buffer
      net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ

Rahul Rameshbabu (1):
      net/mlx5e: Correct SKB room check to use all room in the fifo

Roi Dayan (1):
      net/mlx5e: Verify flow_source cap before using it

Vadim Fedorenko (2):
      mlx5: fix skb leak while fifo resync and push
      mlx5: fix possible ptp queue fifo use-after-free

Yang Li (1):
      net/mlx5: Remove NULL check before dev_{put, hold}

Yang Yingliang (1):
      net/mlx5e: TC, fix return value check in mlx5e_tc_act_stats_create()

 drivers/net/ethernet/mellanox/mlx5/core/ecpf.c     |  4 ++++
 .../net/ethernet/mellanox/mlx5/core/en/params.c    |  8 +++++--
 .../net/ethernet/mellanox/mlx5/core/en/params.h    |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c   | 25 +++++++++++++++++++---
 .../net/ethernet/mellanox/mlx5/core/en/rep/tc.c    |  3 +--
 .../ethernet/mellanox/mlx5/core/en/tc/act_stats.c  |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h  |  4 +++-
 .../net/ethernet/mellanox/mlx5/core/en/xsk/setup.c |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |  7 +++---
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.h |  1 +
 .../ethernet/mellanox/mlx5/core/eswitch_offloads.c |  3 ++-
 .../net/ethernet/mellanox/mlx5/core/lib/geneve.c   |  1 +
 drivers/net/ethernet/mellanox/mlx5/core/sriov.c    |  4 ++++
 14 files changed, 52 insertions(+), 14 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [net 01/10] net/mlx5: Remove NULL check before dev_{put, hold}
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 02/10] net/mlx5e: TC, fix return value check in mlx5e_tc_act_stats_create() Saeed Mahameed
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Yang Li, Abaci Robot

From: Yang Li <yang.lee@linux.alibaba.com>

The call netdev_{put, hold} of dev_{put, hold} will check NULL,
so there is no need to check before using dev_{put, hold},
remove it to silence the warning:

./drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c:714:2-9: WARNING: NULL check before dev_{put, hold} functions is not needed.

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4174
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
index e24b46953542..8f7452dc00ee 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/tc.c
@@ -710,8 +710,7 @@ void mlx5e_rep_tc_receive(struct mlx5_cqe64 *cqe, struct mlx5e_rq *rq,
 	else
 		napi_gro_receive(rq->cq.napi, skb);
 
-	if (tc_priv.fwd_dev)
-		dev_put(tc_priv.fwd_dev);
+	dev_put(tc_priv.fwd_dev);
 
 	return;
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 02/10] net/mlx5e: TC, fix return value check in mlx5e_tc_act_stats_create()
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
  2023-02-23 22:52 ` [net 01/10] net/mlx5: Remove NULL check before dev_{put, hold} Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer Saeed Mahameed
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Yang Yingliang, Roi Dayan

From: Yang Yingliang <yangyingliang@huawei.com>

kvzalloc() returns NULL pointer not PTR_ERR() when it fails,
so replace the IS_ERR() check with NULL pointer check.

Fixes: d13674b1d14c ("net/mlx5e: TC, map tc action cookie to a hw counter")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/tc/act_stats.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/act_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/act_stats.c
index f71766dca660..626cb7470fa5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc/act_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc/act_stats.c
@@ -37,7 +37,7 @@ mlx5e_tc_act_stats_create(void)
 	int err;
 
 	handle = kvzalloc(sizeof(*handle), GFP_KERNEL);
-	if (IS_ERR(handle))
+	if (!handle)
 		return ERR_PTR(-ENOMEM);
 
 	err = rhashtable_init(&handle->ht, &act_counters_ht_params);
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
  2023-02-23 22:52 ` [net 01/10] net/mlx5: Remove NULL check before dev_{put, hold} Saeed Mahameed
  2023-02-23 22:52 ` [net 02/10] net/mlx5e: TC, fix return value check in mlx5e_tc_act_stats_create() Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-24  0:34   ` Jakub Kicinski
  2023-02-23 22:52 ` [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ Saeed Mahameed
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Maxim Mikityanskiy

From: Maxim Mikityanskiy <maxtram95@gmail.com>

The cited commits missed passing frag_size to __xdp_rxq_info_reg, which
is required by bpf_xdp_adjust_tail to support growing the tail pointer
in fragmented packets. Pass the missing parameter when the current RQ
mode allows XDP multi buffer.

Fixes: ea5d49bdae8b ("net/mlx5e: Add XDP multi buffer support to the non-linear legacy RQ")
Fixes: 9cb9482ef10e ("net/mlx5e: Use fragments of the same size in non-linear legacy RQ with XDP")
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
Cc: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/params.c | 8 ++++++--
 drivers/net/ethernet/mellanox/mlx5/core/en/params.h | 1 +
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c   | 7 ++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
index a21bd1179477..998e31422a0b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.c
@@ -672,7 +672,8 @@ static int mlx5e_max_nonlinear_mtu(int first_frag_size, int frag_size, bool xdp)
 static int mlx5e_build_rq_frags_info(struct mlx5_core_dev *mdev,
 				     struct mlx5e_params *params,
 				     struct mlx5e_xsk_param *xsk,
-				     struct mlx5e_rq_frags_info *info)
+				     struct mlx5e_rq_frags_info *info,
+				     u32 *xdp_frag_size)
 {
 	u32 byte_count = MLX5E_SW2HW_MTU(params, params->sw_mtu);
 	int frag_size_max = DEFAULT_FRAG_SIZE;
@@ -782,6 +783,8 @@ static int mlx5e_build_rq_frags_info(struct mlx5_core_dev *mdev,
 
 	info->log_num_frags = order_base_2(info->num_frags);
 
+	*xdp_frag_size = info->num_frags > 1 && params->xdp_prog ? PAGE_SIZE : 0;
+
 	return 0;
 }
 
@@ -927,7 +930,8 @@ int mlx5e_build_rq_param(struct mlx5_core_dev *mdev,
 	}
 	default: /* MLX5_WQ_TYPE_CYCLIC */
 		MLX5_SET(wq, wq, log_wq_sz, params->log_rq_mtu_frames);
-		err = mlx5e_build_rq_frags_info(mdev, params, xsk, &param->frags_info);
+		err = mlx5e_build_rq_frags_info(mdev, params, xsk, &param->frags_info,
+						&param->xdp_frag_size);
 		if (err)
 			return err;
 		ndsegs = param->frags_info.num_frags;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
index c9be6eb88012..e5930fe752e5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/params.h
@@ -24,6 +24,7 @@ struct mlx5e_rq_param {
 	u32                        rqc[MLX5_ST_SZ_DW(rqc)];
 	struct mlx5_wq_param       wq;
 	struct mlx5e_rq_frags_info frags_info;
+	u32                        xdp_frag_size;
 };
 
 struct mlx5e_sq_param {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 53feb0529943..24d4bd44897b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -582,7 +582,7 @@ static void mlx5e_free_mpwqe_rq_drop_page(struct mlx5e_rq *rq)
 }
 
 static int mlx5e_init_rxq_rq(struct mlx5e_channel *c, struct mlx5e_params *params,
-			     struct mlx5e_rq *rq)
+			     u32 xdp_frag_size, struct mlx5e_rq *rq)
 {
 	struct mlx5_core_dev *mdev = c->mdev;
 	int err;
@@ -606,7 +606,8 @@ static int mlx5e_init_rxq_rq(struct mlx5e_channel *c, struct mlx5e_params *param
 	if (err)
 		return err;
 
-	return xdp_rxq_info_reg(&rq->xdp_rxq, rq->netdev, rq->ix, c->napi.napi_id);
+	return __xdp_rxq_info_reg(&rq->xdp_rxq, rq->netdev, rq->ix, c->napi.napi_id,
+				  xdp_frag_size);
 }
 
 static int mlx5_rq_shampo_alloc(struct mlx5_core_dev *mdev,
@@ -2193,7 +2194,7 @@ static int mlx5e_open_rxq_rq(struct mlx5e_channel *c, struct mlx5e_params *param
 {
 	int err;
 
-	err = mlx5e_init_rxq_rq(c, params, &c->rq);
+	err = mlx5e_init_rxq_rq(c, params, rq_params->xdp_frag_size, &c->rq);
 	if (err)
 		return err;
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (2 preceding siblings ...)
  2023-02-23 22:52 ` [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-24  0:35   ` Jakub Kicinski
  2023-02-23 22:52 ` [net 05/10] mlx5: fix skb leak while fifo resync and push Saeed Mahameed
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Maxim Mikityanskiy

From: Maxim Mikityanskiy <maxtram95@gmail.com>

The cited commit missed setting napi_id on XSK RQs, it only affected
regular RQs. Add the missing part to support socket busy polling on XSK
RQs.

Fixes: a2740f529da2 ("net/mlx5e: xsk: Set napi_id to support busy polling")
Signed-off-by: Maxim Mikityanskiy <maxtram95@gmail.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
index 81a567e17264..bc6706aa8afc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/setup.c
@@ -86,7 +86,7 @@ static int mlx5e_init_xsk_rq(struct mlx5e_channel *c,
 	if (err)
 		return err;
 
-	return  xdp_rxq_info_reg(&rq->xdp_rxq, rq->netdev, rq_xdp_ix, 0);
+	return xdp_rxq_info_reg(&rq->xdp_rxq, rq->netdev, rq_xdp_ix, c->napi.napi_id);
 }
 
 static int mlx5e_open_xsk_rq(struct mlx5e_channel *c, struct mlx5e_params *params,
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 05/10] mlx5: fix skb leak while fifo resync and push
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (3 preceding siblings ...)
  2023-02-23 22:52 ` [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 06/10] mlx5: fix possible ptp queue fifo use-after-free Saeed Mahameed
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Vadim Fedorenko,
	Gal Pressman

From: Vadim Fedorenko <vadfed@meta.com>

During ptp resync operation SKBs were poped from the fifo but were never
freed neither by napi_consume nor by dev_kfree_skb_any. Add call to
napi_consume_skb to properly free SKBs.

Another leak was happening because mlx5e_skb_fifo_has_room() had an error
in the check. Comparing free running counters works well unless C promotes
the types to something wider than the counter. In this case counters are
u16 but the result of the substraction is promouted to int and it causes
wrong result (negative value) of the check when producer have already
overlapped but consumer haven't yet. Explicit cast to u16 fixes the issue.

Fixes: 58a518948f60 ("net/mlx5e: Add resiliency for PTP TX port timestamp")
Reviewed-by: Gal Pressman <gal@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c  | 6 ++++--
 drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
index 9a1bc93b7dc6..392db8d5e9c7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
@@ -86,7 +86,8 @@ static bool mlx5e_ptp_ts_cqe_drop(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb
 	return (ptpsq->ts_cqe_ctr_mask && (skb_cc != skb_id));
 }
 
-static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb_id)
+static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc,
+					     u16 skb_id, int budget)
 {
 	struct skb_shared_hwtstamps hwts = {};
 	struct sk_buff *skb;
@@ -98,6 +99,7 @@ static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_
 		hwts.hwtstamp = mlx5e_skb_cb_get_hwts(skb)->cqe_hwtstamp;
 		skb_tstamp_tx(skb, &hwts);
 		ptpsq->cq_stats->resync_cqe++;
+		napi_consume_skb(skb, budget);
 		skb_cc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_cc);
 	}
 }
@@ -119,7 +121,7 @@ static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq,
 	}
 
 	if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id))
-		mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id);
+		mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id, budget);
 
 	skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo);
 	hwtstamp = mlx5e_cqe_ts_to_ns(sq->ptp_cyc2time, sq->clock, get_cqe_ts(cqe));
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
index c067d2efab51..2c10adbf1849 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
@@ -86,7 +86,7 @@ void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq);
 static inline bool
 mlx5e_skb_fifo_has_room(struct mlx5e_skb_fifo *fifo)
 {
-	return (*fifo->pc - *fifo->cc) < fifo->mask;
+	return (u16)(*fifo->pc - *fifo->cc) < fifo->mask;
 }
 
 static inline bool
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 06/10] mlx5: fix possible ptp queue fifo use-after-free
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (4 preceding siblings ...)
  2023-02-23 22:52 ` [net 05/10] mlx5: fix skb leak while fifo resync and push Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo Saeed Mahameed
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Vadim Fedorenko

From: Vadim Fedorenko <vadfed@meta.com>

Fifo indexes are not checked during pop operations and it leads to
potential use-after-free when poping from empty queue. Such case was
possible during re-sync action. WARN_ON_ONCE covers future cases.

There were out-of-order cqe spotted which lead to drain of the queue and
use-after-free because of lack of fifo pointers check. Special check and
counter are added to avoid resync operation if SKB could not exist in the
fifo because of OOO cqe (skb_id must be between consumer and producer
index).

Fixes: 58a518948f60 ("net/mlx5e: Add resiliency for PTP TX port timestamp")
Signed-off-by: Vadim Fedorenko <vadfed@meta.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 .../net/ethernet/mellanox/mlx5/core/en/ptp.c  | 19 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en/txrx.h |  2 ++
 .../ethernet/mellanox/mlx5/core/en_stats.c    |  1 +
 .../ethernet/mellanox/mlx5/core/en_stats.h    |  1 +
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
index 392db8d5e9c7..eb5aeba3addf 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
@@ -86,6 +86,17 @@ static bool mlx5e_ptp_ts_cqe_drop(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb
 	return (ptpsq->ts_cqe_ctr_mask && (skb_cc != skb_id));
 }
 
+static bool mlx5e_ptp_ts_cqe_ooo(struct mlx5e_ptpsq *ptpsq, u16 skb_id)
+{
+	u16 skb_cc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_cc);
+	u16 skb_pc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_pc);
+
+	if (PTP_WQE_CTR2IDX(skb_id - skb_cc) >= PTP_WQE_CTR2IDX(skb_pc - skb_cc))
+		return true;
+
+	return false;
+}
+
 static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc,
 					     u16 skb_id, int budget)
 {
@@ -120,8 +131,14 @@ static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq,
 		goto out;
 	}
 
-	if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id))
+	if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id)) {
+		if (mlx5e_ptp_ts_cqe_ooo(ptpsq, skb_id)) {
+			/* already handled by a previous resync */
+			ptpsq->cq_stats->ooo_cqe_drop++;
+			return;
+		}
 		mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id, budget);
+	}
 
 	skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo);
 	hwtstamp = mlx5e_cqe_ts_to_ns(sq->ptp_cyc2time, sq->clock, get_cqe_ts(cqe));
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
index 2c10adbf1849..b9c2f67d3794 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
@@ -302,6 +302,8 @@ void mlx5e_skb_fifo_push(struct mlx5e_skb_fifo *fifo, struct sk_buff *skb)
 static inline
 struct sk_buff *mlx5e_skb_fifo_pop(struct mlx5e_skb_fifo *fifo)
 {
+	WARN_ON_ONCE(*fifo->pc == *fifo->cc);
+
 	return *mlx5e_skb_fifo_get(fifo, (*fifo->cc)++);
 }
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 6687b8136e44..4478223c1720 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -2138,6 +2138,7 @@ static const struct counter_desc ptp_cq_stats_desc[] = {
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, abort_abs_diff_ns) },
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, resync_cqe) },
 	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, resync_event) },
+	{ MLX5E_DECLARE_PTP_CQ_STAT(struct mlx5e_ptp_cq_stats, ooo_cqe_drop) },
 };
 
 static const struct counter_desc ptp_rq_stats_desc[] = {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 375752d6546d..b77100b60b50 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -461,6 +461,7 @@ struct mlx5e_ptp_cq_stats {
 	u64 abort_abs_diff_ns;
 	u64 resync_cqe;
 	u64 resync_event;
+	u64 ooo_cqe_drop;
 };
 
 struct mlx5e_rep_stats {
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (5 preceding siblings ...)
  2023-02-23 22:52 ` [net 06/10] mlx5: fix possible ptp queue fifo use-after-free Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-24  0:38   ` Jakub Kicinski
  2023-02-23 22:52 ` [net 08/10] net/mlx5: ECPF, wait for VF pages only after disabling host PFs Saeed Mahameed
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Rahul Rameshbabu

From: Rahul Rameshbabu <rrameshbabu@nvidia.com>

Previous check was comparing against the fifo mask. The mask is size of the
fifo (power of two) minus one, so a less than or equal comparator should be
used for checking if the fifo has room for the SKB.

Fixes: 19b43a432e3e ("net/mlx5e: Extend SKB room check to include PTP-SQ")
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
index b9c2f67d3794..816ea83e6413 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
@@ -86,7 +86,7 @@ void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq);
 static inline bool
 mlx5e_skb_fifo_has_room(struct mlx5e_skb_fifo *fifo)
 {
-	return (u16)(*fifo->pc - *fifo->cc) < fifo->mask;
+	return (u16)(*fifo->pc - *fifo->cc) <= fifo->mask;
 }
 
 static inline bool
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 08/10] net/mlx5: ECPF, wait for VF pages only after disabling host PFs
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (6 preceding siblings ...)
  2023-02-23 22:52 ` [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 09/10] net/mlx5e: Verify flow_source cap before using it Saeed Mahameed
  2023-02-23 22:52 ` [net 10/10] net/mlx5: Geneve, Fix handling of Geneve object id as error code Saeed Mahameed
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Maher Sanalla,
	Moshe Shemesh

From: Maher Sanalla <msanalla@nvidia.com>

Currently,  during the early stages of their unloading, particularly
during SRIOV disablement, PFs/ECPFs wait on the release of all of
their VFs memory pages. Furthermore, ECPFs are considered the page
supplier for host VFs, hence the host VFs memory pages are freed only
during ECPF cleanup when host interfaces get disabled.

Thus, disabling SRIOV early in unload timeline causes the DPU ECPF
to stall on driver unload while waiting on the release of host VF pages
that won't be freed before host interfaces get disabled later on.

Therefore, for ECPFs, wait on the release of VFs pages only after the
disablement of host PFs during ECPF cleanup flow. Then, host PFs and VFs
are disabled and their memory shall be freed accordingly.

Fixes: 143a41d7623d ("net/mlx5: Disable SRIOV before PF removal")
Signed-off-by: Maher Sanalla <msanalla@nvidia.com>
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/ecpf.c  | 4 ++++
 drivers/net/ethernet/mellanox/mlx5/core/sriov.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
index 9a3878f9e582..7c9c4e40c019 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ecpf.c
@@ -98,4 +98,8 @@ void mlx5_ec_cleanup(struct mlx5_core_dev *dev)
 	err = mlx5_wait_for_pages(dev, &dev->priv.page_counters[MLX5_HOST_PF]);
 	if (err)
 		mlx5_core_warn(dev, "Timeout reclaiming external host PF pages err(%d)\n", err);
+
+	err = mlx5_wait_for_pages(dev, &dev->priv.page_counters[MLX5_VF]);
+	if (err)
+		mlx5_core_warn(dev, "Timeout reclaiming external host VFs pages err(%d)\n", err);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
index 3008e9ce2bbf..20d7662c10fb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
@@ -147,6 +147,10 @@ mlx5_device_disable_sriov(struct mlx5_core_dev *dev, int num_vfs, bool clear_vf)
 
 	mlx5_eswitch_disable_sriov(dev->priv.eswitch, clear_vf);
 
+	/* For ECPFs, skip waiting for host VF pages until ECPF is destroyed */
+	if (mlx5_core_is_ecpf(dev))
+		return;
+
 	if (mlx5_wait_for_pages(dev, &dev->priv.page_counters[MLX5_VF]))
 		mlx5_core_warn(dev, "timeout reclaiming VFs pages\n");
 }
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 09/10] net/mlx5e: Verify flow_source cap before using it
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (7 preceding siblings ...)
  2023-02-23 22:52 ` [net 08/10] net/mlx5: ECPF, wait for VF pages only after disabling host PFs Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  2023-02-23 22:52 ` [net 10/10] net/mlx5: Geneve, Fix handling of Geneve object id as error code Saeed Mahameed
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Roi Dayan, Maor Dickman

From: Roi Dayan <roid@nvidia.com>

When adding send to vport rule verify flow_source matching is
supported by checking the flow_source cap.

Fixes: d04442540372 ("net/mlx5: E-Switch, set flow source for send to uplink rule")
Signed-off-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Maor Dickman <maord@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 2a98375a0abf..d766a64b1823 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -869,7 +869,8 @@ mlx5_eswitch_add_send_to_vport_rule(struct mlx5_eswitch *on_esw,
 	dest.vport.flags |= MLX5_FLOW_DEST_VPORT_VHCA_ID;
 	flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
 
-	if (rep->vport == MLX5_VPORT_UPLINK)
+	if (MLX5_CAP_ESW_FLOWTABLE(on_esw->dev, flow_source) &&
+	    rep->vport == MLX5_VPORT_UPLINK)
 		spec->flow_context.flow_source = MLX5_FLOW_CONTEXT_FLOW_SOURCE_LOCAL_VPORT;
 
 	flow_rule = mlx5_add_flow_rules(mlx5_eswitch_get_slow_fdb(on_esw),
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [net 10/10] net/mlx5: Geneve, Fix handling of Geneve object id as error code
  2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
                   ` (8 preceding siblings ...)
  2023-02-23 22:52 ` [net 09/10] net/mlx5e: Verify flow_source cap before using it Saeed Mahameed
@ 2023-02-23 22:52 ` Saeed Mahameed
  9 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-23 22:52 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
  Cc: Saeed Mahameed, netdev, Tariq Toukan, Maor Dickman, Raed Salem

From: Maor Dickman <maord@nvidia.com>

On success, mlx5_geneve_tlv_option_create returns non negative
Geneve object id. In case the object id is positive value the
caller functions will handle it as an error (non zero) and
will fail to offload the Geneve rule.

Fix this by changing caller function ,mlx5_geneve_tlv_option_add,
to return 0 in case valid non negative object id was provided.

Fixes: 0ccc171ea6a2 ("net/mlx5: Geneve, Manage Geneve TLV options")
Signed-off-by: Maor Dickman <maord@nvidia.com>
Reviewed-by: Raed Salem <raeds@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/lib/geneve.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/geneve.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/geneve.c
index 23361a9ae4fa..6dc83e871cd7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/geneve.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/geneve.c
@@ -105,6 +105,7 @@ int mlx5_geneve_tlv_option_add(struct mlx5_geneve *geneve, struct geneve_opt *op
 		geneve->opt_type = opt->type;
 		geneve->obj_id = res;
 		geneve->refcount++;
+		res = 0;
 	}
 
 unlock:
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer
  2023-02-23 22:52 ` [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer Saeed Mahameed
@ 2023-02-24  0:34   ` Jakub Kicinski
  2023-02-24 18:07     ` Saeed Mahameed
  0 siblings, 1 reply; 19+ messages in thread
From: Jakub Kicinski @ 2023-02-24  0:34 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev, Tariq Toukan, Maxim Mikityanskiy

On Thu, 23 Feb 2023 14:52:40 -0800 Saeed Mahameed wrote:
> From: Maxim Mikityanskiy <maxtram95@gmail.com>
> 
> The cited commits missed passing frag_size to __xdp_rxq_info_reg, which
> is required by bpf_xdp_adjust_tail to support growing the tail pointer
> in fragmented packets. Pass the missing parameter when the current RQ
> mode allows XDP multi buffer.

Why is this a fix and not an (omitted) feature?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ
  2023-02-23 22:52 ` [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ Saeed Mahameed
@ 2023-02-24  0:35   ` Jakub Kicinski
  0 siblings, 0 replies; 19+ messages in thread
From: Jakub Kicinski @ 2023-02-24  0:35 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev, Tariq Toukan, Maxim Mikityanskiy

On Thu, 23 Feb 2023 14:52:41 -0800 Saeed Mahameed wrote:
> From: Maxim Mikityanskiy <maxtram95@gmail.com>
> 
> The cited commit missed setting napi_id on XSK RQs, it only affected
> regular RQs. Add the missing part to support socket busy polling on XSK
> RQs.

Again, looks like an omission, not a bug or regression.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo
  2023-02-23 22:52 ` [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo Saeed Mahameed
@ 2023-02-24  0:38   ` Jakub Kicinski
  2023-02-24 12:18     ` Vadim Fedorenko
  2023-02-24 21:58     ` David Laight
  0 siblings, 2 replies; 19+ messages in thread
From: Jakub Kicinski @ 2023-02-24  0:38 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev, Tariq Toukan, Rahul Rameshbabu

On Thu, 23 Feb 2023 14:52:44 -0800 Saeed Mahameed wrote:
> From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> 
> Previous check was comparing against the fifo mask. The mask is size of the
> fifo (power of two) minus one, so a less than or equal comparator should be
> used for checking if the fifo has room for the SKB.
> 
> Fixes: 19b43a432e3e ("net/mlx5e: Extend SKB room check to include PTP-SQ")

How big is the fifo? Not utilizing a single entry is not really worth
calling a bug if the fifo has at least 32 entries..

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo
  2023-02-24  0:38   ` Jakub Kicinski
@ 2023-02-24 12:18     ` Vadim Fedorenko
  2023-02-24 18:03       ` Saeed Mahameed
  2023-02-24 21:58     ` David Laight
  1 sibling, 1 reply; 19+ messages in thread
From: Vadim Fedorenko @ 2023-02-24 12:18 UTC (permalink / raw)
  To: Jakub Kicinski, Saeed Mahameed
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev, Tariq Toukan, Rahul Rameshbabu

On 24/02/2023 00:38, Jakub Kicinski wrote:
> On Thu, 23 Feb 2023 14:52:44 -0800 Saeed Mahameed wrote:
>> From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>>
>> Previous check was comparing against the fifo mask. The mask is size of the
>> fifo (power of two) minus one, so a less than or equal comparator should be
>> used for checking if the fifo has room for the SKB.
>>
>> Fixes: 19b43a432e3e ("net/mlx5e: Extend SKB room check to include PTP-SQ")
> 
> How big is the fifo? Not utilizing a single entry is not really worth
> calling a bug if the fifo has at least 32 entries..

AFAIK, it has the same size that any other SQ queue, up to 8192 entries 
for now.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo
  2023-02-24 12:18     ` Vadim Fedorenko
@ 2023-02-24 18:03       ` Saeed Mahameed
  0 siblings, 0 replies; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-24 18:03 UTC (permalink / raw)
  To: Vadim Fedorenko
  Cc: Jakub Kicinski, David S. Miller, Paolo Abeni, Eric Dumazet,
	Saeed Mahameed, netdev, Tariq Toukan, Rahul Rameshbabu

On 24 Feb 12:18, Vadim Fedorenko wrote:
>On 24/02/2023 00:38, Jakub Kicinski wrote:
>>On Thu, 23 Feb 2023 14:52:44 -0800 Saeed Mahameed wrote:
>>>From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>>>
>>>Previous check was comparing against the fifo mask. The mask is size of the
>>>fifo (power of two) minus one, so a less than or equal comparator should be
>>>used for checking if the fifo has room for the SKB.
>>>
>>>Fixes: 19b43a432e3e ("net/mlx5e: Extend SKB room check to include PTP-SQ")
>>
>>How big is the fifo? Not utilizing a single entry is not really worth
>>calling a bug if the fifo has at least 32 entries..
>
>AFAIK, it has the same size that any other SQ queue, up to 8192 
>entries for now.

I agree minimum size is 64, so not critical.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer
  2023-02-24  0:34   ` Jakub Kicinski
@ 2023-02-24 18:07     ` Saeed Mahameed
  2023-04-08 11:24       ` Maxim Mikityanskiy
  0 siblings, 1 reply; 19+ messages in thread
From: Saeed Mahameed @ 2023-02-24 18:07 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev, Tariq Toukan, Maxim Mikityanskiy

On 23 Feb 16:34, Jakub Kicinski wrote:
>On Thu, 23 Feb 2023 14:52:40 -0800 Saeed Mahameed wrote:
>> From: Maxim Mikityanskiy <maxtram95@gmail.com>
>>
>> The cited commits missed passing frag_size to __xdp_rxq_info_reg, which
>> is required by bpf_xdp_adjust_tail to support growing the tail pointer
>> in fragmented packets. Pass the missing parameter when the current RQ
>> mode allows XDP multi buffer.
>
>Why is this a fix and not an (omitted) feature?

I am sure not intentional, but I agree not critical.
  

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo
  2023-02-24  0:38   ` Jakub Kicinski
  2023-02-24 12:18     ` Vadim Fedorenko
@ 2023-02-24 21:58     ` David Laight
  1 sibling, 0 replies; 19+ messages in thread
From: David Laight @ 2023-02-24 21:58 UTC (permalink / raw)
  To: 'Jakub Kicinski', Saeed Mahameed
  Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
	netdev@vger.kernel.org, Tariq Toukan, Rahul Rameshbabu

From: Jakub Kicinski
> Sent: 24 February 2023 00:39
> 
> On Thu, 23 Feb 2023 14:52:44 -0800 Saeed Mahameed wrote:
> > From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> >
> > Previous check was comparing against the fifo mask. The mask is size of the
> > fifo (power of two) minus one, so a less than or equal comparator should be
> > used for checking if the fifo has room for the SKB.
> >
> > Fixes: 19b43a432e3e ("net/mlx5e: Extend SKB room check to include PTP-SQ")
> 
> How big is the fifo? Not utilizing a single entry is not really worth
> calling a bug if the fifo has at least 32 entries..

There is also the question of how 'fifo full' and 'fifo empty'
are differentiated if they both have the same index values.
I've not looked at the code in question, but not using the
last slot is less likely to be buggy.

I've taken to using array[index++ & mask] and just letting
the index wrap at 2**32 (or even (not) wrap an 2**64).
Then the full and empty conditions are trivially separated.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer
  2023-02-24 18:07     ` Saeed Mahameed
@ 2023-04-08 11:24       ` Maxim Mikityanskiy
  0 siblings, 0 replies; 19+ messages in thread
From: Maxim Mikityanskiy @ 2023-04-08 11:24 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Jakub Kicinski, David S. Miller, Paolo Abeni, Eric Dumazet,
	Saeed Mahameed, netdev, Tariq Toukan, Gal Pressman

On Fri, 24 Feb 2023 at 10:07:44 -0800, Saeed Mahameed wrote:
> On 23 Feb 16:34, Jakub Kicinski wrote:
> > On Thu, 23 Feb 2023 14:52:40 -0800 Saeed Mahameed wrote:
> > > From: Maxim Mikityanskiy <maxtram95@gmail.com>
> > > 
> > > The cited commits missed passing frag_size to __xdp_rxq_info_reg, which
> > > is required by bpf_xdp_adjust_tail to support growing the tail pointer
> > > in fragmented packets. Pass the missing parameter when the current RQ
> > > mode allows XDP multi buffer.
> > 
> > Why is this a fix and not an (omitted) feature?
> 
> I am sure not intentional, but I agree not critical.

What's the destiny of my two patches (3 and 4)? I don't see them applied
to either net or net-next.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-04-08 11:25 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-23 22:52 [pull request][net 00/10] mlx5 fixes 2023-02-23 Saeed Mahameed
2023-02-23 22:52 ` [net 01/10] net/mlx5: Remove NULL check before dev_{put, hold} Saeed Mahameed
2023-02-23 22:52 ` [net 02/10] net/mlx5e: TC, fix return value check in mlx5e_tc_act_stats_create() Saeed Mahameed
2023-02-23 22:52 ` [net 03/10] net/mlx5e: XDP, Allow growing tail for XDP multi buffer Saeed Mahameed
2023-02-24  0:34   ` Jakub Kicinski
2023-02-24 18:07     ` Saeed Mahameed
2023-04-08 11:24       ` Maxim Mikityanskiy
2023-02-23 22:52 ` [net 04/10] net/mlx5e: xsk: Set napi_id to support busy polling on XSK RQ Saeed Mahameed
2023-02-24  0:35   ` Jakub Kicinski
2023-02-23 22:52 ` [net 05/10] mlx5: fix skb leak while fifo resync and push Saeed Mahameed
2023-02-23 22:52 ` [net 06/10] mlx5: fix possible ptp queue fifo use-after-free Saeed Mahameed
2023-02-23 22:52 ` [net 07/10] net/mlx5e: Correct SKB room check to use all room in the fifo Saeed Mahameed
2023-02-24  0:38   ` Jakub Kicinski
2023-02-24 12:18     ` Vadim Fedorenko
2023-02-24 18:03       ` Saeed Mahameed
2023-02-24 21:58     ` David Laight
2023-02-23 22:52 ` [net 08/10] net/mlx5: ECPF, wait for VF pages only after disabling host PFs Saeed Mahameed
2023-02-23 22:52 ` [net 09/10] net/mlx5e: Verify flow_source cap before using it Saeed Mahameed
2023-02-23 22:52 ` [net 10/10] net/mlx5: Geneve, Fix handling of Geneve object id as error code Saeed Mahameed

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).