[RFC net-next 4/7] net/mlx5e: Drain old TX SQs before activating new channels

[Tariq Toukan]

On devices with tis_tir_td_order, commit 3b88a535a8e1 ("net/mlx5e:
Defer channels closure to reduce interface down time") closes the old
channels only after the new ones are activated. Their SQs are
deactivated before that, which rings a final doorbell, but nothing
waits for the HW to execute their pending WQEs: the completions of a
deactivated SQ are not polled, and the SQ is destroyed only when its
channel is closed. Meanwhile, the new SQ of the same TX queue starts
transmitting. A flow's WQEs still pending on the old SQ can then
execute after its newer WQEs on the new SQ, as WQE fences order only
within an SQ.

Besides reordering packets, this breaks kTLS TX offload. The WQEs of a
connection update its TIS crypto and record tracking state as they
execute. When old WQEs execute after new ones, e.g. because a PFC
pause held the old SQ back, the TIS state gets out of sync with the
TCP stream, and the connection's records go out corrupted.

Before activating the new channels, drain the TX SQs of the old ones:
reap their completions until all their WQEs have completed. If they
do not drain in time, e.g. under a long pause, warn, and close the old
channels before activating the new ones, as before that commit. Move
the deferred closure into mlx5e_switch_priv_channels(), next to that
decision.

Pointed out by Rishikesh Jethwani while discussing kTLS TX TIS reuse
for TLS 1.3 rekey on netdev.

Reproduced on ConnectX-6 Dx, with 248 channels, by changing the
channel count between 247 and 248 every 2 seconds while 32 kTLS TX
offloaded connections stream pattern data to a software kTLS receiver
that checks every byte. On this setup, the new channels are activated
about 230 ms after the old ones are deactivated, so the old SQs must
still hold WQEs by then: with every TX queue rate limited to 10 Mb/s,
9 of 41 connections failed decryption in 60 seconds, and 11 of 43 in
120 seconds. With this patch, no record failed in five such runs, 7
minutes in total; the drain timed out and fell back to closing the old
channels first 7 times. At 1 Gb/s per queue, the old SQs always
drained in time.

Fixes: 3b88a535a8e1 ("net/mlx5e: Defer channels closure to reduce interface down time")
Reported-by: Rishikesh Jethwani <rjethwani@xxxxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/netdev/CAKaoeS1hj9XuM7ojLBCjGA=EXc2M-b+eRjC2Sk=8L_8XYeJ18w@xxxxxxxxxxxxxx/
Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
---
 .../net/ethernet/mellanox/mlx5/core/en/txrx.h |  1 +
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 88 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   | 41 +++++++--
 3 files changed, 118 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
index 24e91279243e..b5bd1b6e8601 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h
@@ -112,6 +112,7 @@ struct mlx5e_xmit_data_frags {
 
 netdev_tx_t mlx5e_xmit(struct sk_buff *skb, struct net_device *dev);
 bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget);
+bool mlx5e_txqsq_drain(struct mlx5e_txqsq *sq);
 void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq);
 
 static inline bool
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f..a721e30bcaa8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -3421,6 +3421,82 @@ static int mlx5e_switch_priv_params(struct mlx5e_priv *priv,
 	return 0;
 }
 
+/* Time for the TX SQs of deactivated channels to complete their WQEs. */
+#define MLX5E_SQ_DRAIN_TIMEOUT 100 /* msecs */
+
+/* Returns 1 if the SQ still has pending WQEs, and records the first such SQ. */
+static int mlx5e_drain_txqsq(struct mlx5e_txqsq *sq,
+			     struct mlx5e_txqsq **pending)
+{
+	if (mlx5e_txqsq_drain(sq))
+		return 0;
+
+	if (!*pending)
+		*pending = sq;
+
+	return 1;
+}
+
+/* Returns the number of the channel's TX SQs that still have pending WQEs. */
+static int mlx5e_drain_channel_txqsqs(struct mlx5e_channel *c,
+				      struct mlx5e_txqsq **pending)
+{
+	struct mlx5e_txqsq __rcu **qos_sqs;
+	int num_pending = 0;
+	int i;
+
+	for (i = 0; i < c->num_tc; i++)
+		num_pending += mlx5e_drain_txqsq(&c->sq[i], pending);
+
+	qos_sqs = mlx5e_state_dereference(c->priv, c->qos_sqs);
+	if (!qos_sqs)
+		return num_pending;
+
+	for (i = 0; i < c->qos_sqs_size; i++) {
+		struct mlx5e_txqsq *sq;
+
+		sq = mlx5e_state_dereference(c->priv, qos_sqs[i]);
+		if (sq)
+			num_pending += mlx5e_drain_txqsq(sq, pending);
+	}
+
+	return num_pending;
+}
+
+/* Wait for the TX SQs of deactivated channels to complete all their WQEs,
+ * so that none of them executes concurrently with the new SQs.
+ * The PTP SQs are not waited for, they carry PTP event packets only.
+ */
+static bool mlx5e_drain_channels_txqsqs(struct mlx5e_channels *chs)
+{
+	unsigned long exp_time;
+
+	exp_time = jiffies + msecs_to_jiffies(MLX5E_SQ_DRAIN_TIMEOUT);
+	for (;;) {
+		struct mlx5e_txqsq *pending = NULL;
+		int num_pending = 0;
+		int i;
+
+		for (i = 0; i < chs->num; i++)
+			num_pending += mlx5e_drain_channel_txqsqs(chs->c[i],
+								  &pending);
+
+		if (!num_pending)
+			return true;
+
+		if (time_after(jiffies, exp_time)) {
+			netdev_warn(pending->netdev,
+				    "Old TX SQs not drained in %d msecs (%d pending, e.g. SQ 0x%x txq %d cc 0x%x pc 0x%x), closing the old channels first\n",
+				    MLX5E_SQ_DRAIN_TIMEOUT, num_pending,
+				    pending->sqn, pending->txq_ix,
+				    pending->cc, pending->pc);
+			return false;
+		}
+
+		usleep_range(10, 20);
+	}
+}
+
 static int mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
 				      struct mlx5e_channels *old_chs,
 				      struct mlx5e_channels *new_chs,
@@ -3428,6 +3504,7 @@ static int mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
 				      void *context)
 {
 	struct net_device *netdev = priv->netdev;
+	bool defer_close = false;
 	int carrier_ok;
 	int err = 0;
 
@@ -3450,7 +3527,10 @@ static int mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
 	}
 
 	mlx5e_set_xdp_feature(priv);
-	if (!MLX5_CAP_GEN(priv->mdev, tis_tir_td_order))
+
+	defer_close = MLX5_CAP_GEN(priv->mdev, tis_tir_td_order) &&
+		      mlx5e_drain_channels_txqsqs(old_chs);
+	if (!defer_close)
 		mlx5e_close_channels(old_chs);
 	priv->profile->update_rx(priv);
 
@@ -3462,6 +3542,9 @@ static int mlx5e_switch_priv_channels(struct mlx5e_priv *priv,
 	if (carrier_ok)
 		netif_carrier_on(netdev);
 
+	if (defer_close)
+		mlx5e_close_channels(old_chs);
+
 	return err;
 }
 
@@ -3499,9 +3582,6 @@ int mlx5e_safe_switch_params(struct mlx5e_priv *priv,
 	if (err)
 		goto err_close;
 
-	if (MLX5_CAP_GEN(priv->mdev, tis_tir_td_order))
-		mlx5e_close_channels(old_chs);
-
 	kfree(new_chs);
 	kfree(old_chs);
 	return 0;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index bee6f0bf1eff..eb20e3d14d46 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -780,10 +780,11 @@ void mlx5e_txqsq_wake(struct mlx5e_txqsq *sq)
 	}
 }
 
-bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
+/* Returns the number of polled CQEs. */
+static int mlx5e_poll_tx_cqes(struct mlx5e_txqsq *sq, int napi_budget)
 {
+	struct mlx5e_cq *cq = &sq->cq;
 	struct mlx5e_sq_stats *stats;
-	struct mlx5e_txqsq *sq;
 	struct mlx5_cqe64 *cqe;
 	u32 dma_fifo_cc;
 	u32 nbytes;
@@ -791,14 +792,9 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 	u16 sqcc;
 	int i;
 
-	sq = container_of(cq, struct mlx5e_txqsq, cq);
-
-	if (unlikely(!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state)))
-		return false;
-
 	cqe = mlx5_cqwq_get_cqe(&cq->wq);
 	if (!cqe)
-		return false;
+		return 0;
 
 	stats = sq->stats;
 
@@ -891,11 +887,40 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 
 	netdev_tx_completed_queue(sq->txq, npkts, nbytes);
 
+	return i;
+}
+
+bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
+{
+	struct mlx5e_txqsq *sq = container_of(cq, struct mlx5e_txqsq, cq);
+	int i;
+
+	if (unlikely(!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state)))
+		return false;
+
+	i = mlx5e_poll_tx_cqes(sq, napi_budget);
+	if (!i)
+		return false;
+
 	mlx5e_txqsq_wake(sq);
 
 	return (i == MLX5E_TX_CQ_POLL_BUDGET);
 }
 
+/* Reap the completions of a deactivated SQ, whose NAPI is disabled, without
+ * waking its TX queue. Returns true once all its WQEs have completed.
+ */
+bool mlx5e_txqsq_drain(struct mlx5e_txqsq *sq)
+{
+	int polled;
+
+	do {
+		polled = mlx5e_poll_tx_cqes(sq, 0);
+	} while (polled == MLX5E_TX_CQ_POLL_BUDGET);
+
+	return sq->cc == sq->pc;
+}
+
 static void mlx5e_tx_wi_kfree_fifo_skbs(struct mlx5e_txqsq *sq, struct mlx5e_tx_wqe_info *wi)
 {
 	int i;
-- 
2.44.0