[PATCH net 0/2] ionic: fix Tx stalls on TSO skbs larger than the Tx ring

[PATCH net 0/2] ionic: fix Tx stalls on TSO skbs larger than the Tx ring

[Nikhil P. Rao]

ionic uses one Tx descriptor per TSO segment but never tells the stack
how many it can hold, so tso_max_segs stays at its 65535 default. A TSO
skb with more segments than the Tx ring holds can never be posted: the
queue stops with the ring empty, nothing wakes it, and the netdev
watchdog fires again after every reset. A 64 KB TSO skb needs more
than 1023 segments, the limit of the default 1024-entry ring, once the
MSS is below 64 bytes.

Patch 1 rejects Tx ring sizes below 64. The completion path wakes a
stopped Tx queue only once 44 descriptors are free, so a smaller ring
can stall on its own, and patch 2 would otherwise let such a ring lower
gso_max_segs to a uselessly small value.

Patch 2 advertises tso_max_segs from the Tx ring size and keeps it in
step with ethtool -G, so the stack sizes TSO skbs to fit the ring and
segments larger ones in software.

Tested with the in-tree driver in a VM with the PF passed through:
GSO frames with a virtio-net header and about 1500 segments, injected
through a tap redirected to the ionic netdev with tc mirred, stalled the
Tx queue before the series and are sent without a stall after it;
tso_max_segs follows ethtool -G with the interface up and down, and a
Tx ring of 32 is rejected.

This series was developed with the help of an LLM-based coding
assistant (Claude).

---
Nikhil P. Rao (2):
      ionic: reject Tx ring sizes below the minimum
      ionic: limit TSO segments to the Tx ring size

 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 19 ++++++++++++++++---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 10 +++++++---
 drivers/net/ethernet/pensando/ionic/ionic_lif.h     | 10 ++++++++++
 3 files changed, 33 insertions(+), 6 deletions(-)
---
base-commit: a36b9e205cb3c452ab96cc783e63465d466fe615
change-id: 20261107-b4-ionic-tso-max-segs-057bde9cfe49

Best regards,
--  
Nikhil P. Rao <nikhil.rao@amd.com>

[PATCH net 1/2] ionic: reject Tx ring sizes below the minimum

[Nikhil P. Rao]

ionic_tx_cq_service() wakes a stopped Tx queue only once
IONIC_TSO_DESCS_NEEDED (44) descriptors are free. The threshold is
sized for a 64 KB TSO skb at a 1500 byte MSS and assumes the ring can
have that many free; a Tx ring of 32 or fewer descriptors cannot, so a
stopped queue is never woken and the netdev watchdog fires.
ionic_set_ringparam() only checks that the size is a power of 2, and
the ethtool core only checks the maximum, so such sizes are accepted.

Reject Tx ring sizes below IONIC_MIN_TXRX_DESC.

Fixes: 061b9bedbef1 ("ionic: Rework Tx start/stop flow")
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 119ac720c078..3518fbf5882d 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -708,6 +708,13 @@ static int ionic_set_ringparam(struct net_device *netdev,
 		return -EINVAL;
 	}
 
+	if (ring->tx_pending < IONIC_MIN_TXRX_DESC) {
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Tx descriptor count must be at least %d",
+				       IONIC_MIN_TXRX_DESC);
+		return -EINVAL;
+	}
+
 	/* if nothing to do return success */
 	if (ring->tx_pending == lif->ntxq_descs &&
 	    ring->rx_pending == lif->nrxq_descs &&

-- 
2.43.0

[PATCH net 2/2] ionic: limit TSO segments to the Tx ring size

[Nikhil P. Rao]

ionic uses one Tx descriptor per TSO segment, so a GSO skb with more
segments than the ring can hold (num_descs - 1) can never be posted.
The driver never tells the stack this limit, so such skbs are built
and handed to it instead of being segmented in software.

Advertise the ring-derived limit as tso_max_segs, so locally
generated TCP stays within it and the core's gso_max_segs check
catches larger skbs.

The core only ever lowers gso_max_segs when tso_max_segs changes, so after
the ring grows gso_max_segs stays at the old limit until the admin raises
it. The driver leaves it there: it cannot tell a value the core lowered
from one the admin set, so raising it could override the admin's choice,
and the helper that sets gso_max_segs is internal to the core.

ionic_cmb_reconfig() applies the new ring size before it allocates the
queues and restores the old one if that fails, so it updates
tso_max_segs only once the allocation has succeeded, rather than
leaving gso_max_segs lowered after a rollback.

Send a features change event after an ethtool Tx ring resize, so
stacked devices (VLAN, macvlan, bond) pick up the new tso_max_segs
instead of keeping the one copied at their creation.

The firmware-recovery replay runs without RTNL, but always with the
current ring size, so it leaves tso_max_segs and gso_max_segs untouched;
rtnetlink changes them only under RTNL.

Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_ethtool.c | 12 +++++++++---
 drivers/net/ethernet/pensando/ionic/ionic_lif.c     | 10 +++++++---
 drivers/net/ethernet/pensando/ionic/ionic_lif.h     | 10 ++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
index 3518fbf5882d..556e17023320 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_ethtool.c
@@ -689,6 +689,7 @@ static int ionic_set_ringparam(struct net_device *netdev,
 			       struct netlink_ext_ack *extack)
 {
 	struct ionic_lif *lif = netdev_priv(netdev);
+	unsigned int ntxq_descs = lif->ntxq_descs;
 	struct ionic_queue_params qparam;
 	int err;
 
@@ -747,11 +748,12 @@ static int ionic_set_ringparam(struct net_device *netdev,
 		netdev_info(netdev, "Changing Rx ring size from %d to %d\n",
 			    lif->nrxq_descs, ring->rx_pending);
 
-	/* if we're not running, just set the values and return */
+	/* if we're not running, just set the values */
 	if (!netif_running(lif->netdev)) {
-		lif->ntxq_descs = ring->tx_pending;
+		ionic_lif_set_ntxq_descs(lif, ring->tx_pending);
 		lif->nrxq_descs = ring->rx_pending;
-		return 0;
+		err = 0;
+		goto out;
 	}
 
 	mutex_lock(&lif->queue_lock);
@@ -760,6 +762,10 @@ static int ionic_set_ringparam(struct net_device *netdev,
 	if (err)
 		netdev_info(netdev, "Ring reconfiguration failed, changes canceled: %d\n", err);
 
+out:
+	if (lif->ntxq_descs != ntxq_descs)
+		netdev_features_change(netdev);
+
 	return err;
 }
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 7f0da2b0defd..ce1e51f318e0 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -2884,6 +2884,10 @@ static int ionic_cmb_reconfig(struct ionic_lif *lif,
 			}
 		}
 
+		if (lif->ntxq_descs != start_qparams.ntxq_descs)
+			netif_set_tso_max_segs(lif->netdev,
+					       lif->ntxq_descs - 1);
+
 		err = ionic_start_queues_reconfig(lif);
 		if (err) {
 			dev_err(lif->ionic->dev,
@@ -3065,7 +3069,7 @@ int ionic_reconfigure_queues(struct ionic_lif *lif,
 
 	/* swap new desc_info and rings, keeping existing interrupt config */
 	if (tx_qcqs) {
-		lif->ntxq_descs = qparam->ntxq_descs;
+		ionic_lif_set_ntxq_descs(lif, qparam->ntxq_descs);
 		for (i = 0; i < qparam->nxqs; i++)
 			ionic_swap_queues(lif->txqcqs[i], tx_qcqs[i]);
 	}
@@ -3284,10 +3288,10 @@ int ionic_lif_alloc(struct ionic *ionic)
 	lif->index = 0;
 
 	if (is_kdump_kernel()) {
-		lif->ntxq_descs = IONIC_MIN_TXRX_DESC;
+		ionic_lif_set_ntxq_descs(lif, IONIC_MIN_TXRX_DESC);
 		lif->nrxq_descs = IONIC_MIN_TXRX_DESC;
 	} else {
-		lif->ntxq_descs = IONIC_DEF_TXRX_DESC;
+		ionic_lif_set_ntxq_descs(lif, IONIC_DEF_TXRX_DESC);
 		lif->nrxq_descs = IONIC_DEF_TXRX_DESC;
 	}
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index e790f6b9441e..14aaaeb360a2 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -290,6 +290,16 @@ static inline void ionic_init_queue_params(struct ionic_lif *lif,
 	qparam->cmb_rx = test_bit(IONIC_LIF_F_CMB_RX_RINGS, lif->state);
 }
 
+static inline void ionic_lif_set_ntxq_descs(struct ionic_lif *lif,
+					    unsigned int descs)
+{
+	if (descs == lif->ntxq_descs)
+		return;
+
+	lif->ntxq_descs = descs;
+	netif_set_tso_max_segs(lif->netdev, descs - 1);
+}
+
 static inline void ionic_set_queue_params(struct ionic_lif *lif,
 					  struct ionic_queue_params *qparam)
 {

-- 
2.43.0

Re: [PATCH net 0/2] ionic: fix Tx stalls on TSO skbs larger than the Tx ring

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.