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

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] A Tx ring shrink on a running interface can still permanently
  stall a Tx queue, which is the failure this fix is meant to remove.
- [Medium] The new tso_max_segs comes only from the regular Tx ring size
  (ntxq_descs - 1: 1023 by default, up to 8191).
- [Low] The commit message understates what the fix addresses.

--

ionic: limit TSO segments to the Tx ring size

This patch advertises the Tx ring size minus one as tso_max_segs, so the
stack stops building GSO skbs with more segments than the ring can hold.
The limit is updated on ring resize through a new
ionic_lif_set_ntxq_descs() helper and in ionic_cmb_reconfig(). A features
change event is sent after an ethtool Tx ring resize.

> The driver never tells the stack this limit, so such skbs are built
> and handed to it instead of being segmented in software.

[Severity: Low]
Could the changelog also describe what users see when this happens?

When gso_segs is larger than num_descs - 1, ionic_start_xmit() hits:

	if (!netif_txq_maybe_stop(q_to_ndq(netdev, q),
				  ionic_q_space_avail(q),
				  ndescs, ndescs))
		return NETDEV_TX_BUSY;

ionic_q_space_avail() can never reach ndescs. The queue stops, the qdisc
requeues the skb and keeps retrying it until dev_watchdog fires, and the
tx timeout recovery then replays the same skb.

This is easy to hit with a 64-entry ring (the kdump default
IONIC_MIN_TXRX_DESC, or ethtool -G tx 64) and a 64KB TSO skb with MSS
below about 1040, or with UDP GSO.

Would it help stable backporters if the message said this fixes a Tx
queue hang, rather than only a missed software segmentation?

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 7f0da2b0defdf..ce1e51f318e01 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c

[ ... ]

> @@ -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);

[Severity: High]
Can shrinking the Tx ring on a running interface still stall a Tx queue
permanently?

The lower gso_max_segs only takes effect in validate_xmit_skb(), which
runs on skbs freshly dequeued from the qdisc. Skbs that were requeued
into q->gso_skb skip that check:

net/sched/sch_generic.c:dequeue_skb() {
    ...
		/* skb in gso_skb were already validated */
		*validate = false;
    ...
}

Here is one possible sequence:

1. The ring has 1024 entries, so gso_max_segs is 1023, and an skb with
   100 segments passes validation.

2. The ring is briefly full, so ionic_start_xmit() returns
   NETDEV_TX_BUSY and the skb is requeued into q->gso_skb. An skb can
   also end up there when sch_direct_xmit() finds the txq frozen after
   netif_device_detach().

3. The admin runs ethtool -G tx 64. Inside ionic_reconfigure_queues():
   - ionic_stop_queues_reconfig() detaches and stops the queues but
     never resets the qdisc.
   - The 64-entry rings are swapped in.
   - ionic_lif_set_ntxq_descs() lowers the limit to 63.
   - ionic_start_queues_reconfig()->netif_device_attach() wakes the
     queue.

4. The stale skb is dequeued without validation. ionic_tx_descs_needed()
   returns 100, but ionic_q_space_avail() can never be more than 63.
   ionic_start_xmit() stops the queue and returns NETDEV_TX_BUSY. The
   ring is empty, so no completion ever wakes the queue.

5. dev_watchdog fires, and ionic_tx_timeout_work() runs the stop/start
   reconfig again. That does not purge q->gso_skb either, so the same
   skb blocks the queue again. Only qdisc_reset() on ifdown clears it.

ionic_cmb_reconfig() appears to have the same window.

Is a driver-side check needed to fully close this? For example,
ionic_start_xmit() could drop or software-segment an skb whose ndescs is
more than q->num_descs - 1, instead of returning NETDEV_TX_BUSY.

>  		for (i = 0; i < qparam->nxqs; i++)
>  			ionic_swap_queues(lif->txqcqs[i], tx_qcqs[i]);
>  	}

[ ... ]

> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index e790f6b9441e2..14aaaeb360a27 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);

[Severity: Medium]
Does this limit also need to cover the hwstamp Tx ring?

With Tx hardware timestamping enabled, ionic_start_xmit() sends every
SKBTX_HW_TSTAMP skb to the hwstamp queue:

	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP))
		if (lif->hwstamp_txq && lif->phc->ts_config_tx_mode)
			return ionic_start_hwstamp_xmit(skb, netdev);

ionic_lif_create_hwstamp_txq() always creates that ring with:

	num_desc = IONIC_MIN_TXRX_DESC;

So at most 63 entries are usable there. The tso_max_segs set here is
ntxq_descs - 1, which is 1023 by default and can be up to 8191.

A timestamped TSO skb with 64 to ntxq_descs - 1 segments passes the core
gso_max_segs check. ionic_start_hwstamp_xmit() then drops it every time:

	if (unlikely(!ionic_q_has_space(q, ndescs)))
		goto err_out_drop;

The ionic_cmb_reconfig() hunk computes the limit the same way. Any
SO_TIMESTAMPING TX_HARDWARE user with MSS below about 1040 would hit
these drops, for example after setting TCP_MAXSEG or over a small path
MTU.

The commit message says the goal is to stop skbs that can never be
posted from reaching the driver. Doesn't the hwstamp ring, the smallest
Tx ring the driver posts to, still receive them?

> +}
> +

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107-b4-ionic-tso-max-segs-v1-0-67c248a5b681%40amd.com