mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] octeontx2-af: fix TX hang after link mode change under traffic
@ 2026-10-29  7:46 vmukunda
  2026-10-29  7:50 ` netdev-bot+sinfo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: vmukunda @ 2026-10-29  7:46 UTC (permalink / raw)
  To: linux-kernel, netdev
  Cc: Sunil Goutham, Ratheesh Kannoth, Geetha sowjanya,
	Subbaraya Sundeep, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Naveen Mamindlapalli, vmukunda

From: Naveen Mamindlapalli <naveenm@marvell.com>

During a CGX/RPM link mode change while traffic is running, the link
state is disabled and the TX link credits go negative. Once the link is
active again the credits are not restored, which leads to a TX hang.

Set TL1 SW_XOFF before the link mode change and poll until the TX link
credits return to their original value. Clear TL1 SW_XOFF after the
change, including when it fails, so that a failed request does not leave
the link stopped. A timeout in the credit poll is only reported with a
warning, and the link mode change still proceeds.

Fixes: 62945033ea22 ("octeontx2-af: poll for tx link credits before link mode change")
Signed-off-by: Naveen Mamindlapalli <naveenm@marvell.com>
Signed-off-by: vmukunda <vmukunda@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/rvu.h   |  2 +
 .../ethernet/marvell/octeontx2/af/rvu_cgx.c   |  5 ++
 .../ethernet/marvell/octeontx2/af/rvu_nix.c   | 50 +++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
index 094227404ef9..7fc10d17bde9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.h
@@ -1048,6 +1048,8 @@ int rvu_nix_mcast_get_mce_index(struct rvu *rvu, u16 pcifunc,
 				u32 mcast_grp_idx);
 int rvu_nix_mcast_update_mcam_entry(struct rvu *rvu, u16 pcifunc,
 				    u32 mcast_grp_idx, u16 mcam_index);
+int rvu_nix_tl1_xoff_wait_for_link_credits(struct rvu *rvu, u16 pcifunc);
+int rvu_nix_tl1_xoff_clear(struct rvu *rvu, u16 pcifunc);
 void rvu_nix_flr_free_bpids(struct rvu *rvu, u16 pcifunc);
 int rvu_alloc_cint_qint_mem(struct rvu *rvu, struct rvu_pfvf *pfvf,
 			    int blkaddr, int nixlf);
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
index 87d21889dc49..a66abea6e676 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
@@ -1235,8 +1235,13 @@ int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
 	else
 		linkmodes = &rvu->fwdata->cgx_fw_data[cgx_idx][lmac];
 
+	rvu_nix_tl1_xoff_wait_for_link_credits(rvu, req->hdr.pcifunc);
+
 	rsp->status = cgx_set_link_mode(cgxd, req->args, linkmodes,
 					cgx_idx, lmac);
+
+	rvu_nix_tl1_xoff_clear(rvu, req->hdr.pcifunc);
+
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
index 68cf730a8372..ec51a3911da6 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
@@ -6749,3 +6749,53 @@ void rvu_block_bcast_xon(struct rvu *rvu, int blkaddr)
 	cfg = rvu_read64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(0));
 	rvu_write64(rvu, blkaddr, NIX_AF_RX_CHANX_CFG(0), cfg);
 }
+
+int rvu_nix_tl1_xoff_wait_for_link_credits(struct rvu *rvu, u16 pcifunc)
+{
+	int link, blkaddr, count = 1000;
+	u64 tx_credits, regval;
+	struct nix_hw *nix_hw;
+
+	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
+	if (blkaddr < 0)
+		return NIX_AF_ERR_AF_LF_INVALID;
+
+	nix_hw = get_nix_hw(rvu->hw, blkaddr);
+	if (!nix_hw)
+		return NIX_AF_ERR_INVALID_NIXBLK;
+
+	/* set TL1 sw_xoff */
+	link = nix_get_tx_link(rvu, pcifunc);
+	rvu_write64(rvu, blkaddr, NIX_AF_TL1X_SW_XOFF(link), 1);
+
+	/* wait for link credits to return */
+	tx_credits = nix_hw->tx_credits[link];
+	while (1) {
+		regval = rvu_read64(rvu, blkaddr,
+				    NIX_AF_TX_LINKX_NORM_CREDIT(link));
+		if (((regval >> 12) & 0xFFFFF) == tx_credits)
+			break;
+		count--;
+		if (!count) {
+			dev_info(rvu->dev, "TX link(%d) credit poll timeout, expected=%lld credits=%lld\n",
+				 link, tx_credits, ((regval >> 12) & 0xFFFFF));
+			return -ETIMEDOUT;
+		}
+		cpu_relax();
+		udelay(1);
+	}
+	return 0;
+}
+
+int rvu_nix_tl1_xoff_clear(struct rvu *rvu, u16 pcifunc)
+{
+	int blkaddr;
+
+	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
+	if (blkaddr < 0)
+		return NIX_AF_ERR_AF_LF_INVALID;
+
+	nix_clear_tx_xoff(rvu, blkaddr, NIX_TXSCH_LVL_TL1,
+			  nix_get_tx_link(rvu, pcifunc));
+	return 0;
+}
-- 
2.25.1


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

* Re: [PATCH] octeontx2-af: fix TX hang after link mode change under traffic
  2026-10-29  7:46 [PATCH] octeontx2-af: fix TX hang after link mode change under traffic vmukunda
@ 2026-10-29  7:50 ` netdev-bot+sinfo
  2026-10-29 15:54 ` Jakub Kicinski
  2026-11-02  7:59 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-10-29  7:50 UTC (permalink / raw)
  To: vmukunda
  Cc: linux-kernel, netdev, Sunil Goutham, Ratheesh Kannoth,
	Geetha sowjanya, Subbaraya Sundeep, Andrew Lunn,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Naveen Mamindlapalli

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.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

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.

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

* Re: [PATCH] octeontx2-af: fix TX hang after link mode change under traffic
  2026-10-29  7:46 [PATCH] octeontx2-af: fix TX hang after link mode change under traffic vmukunda
  2026-10-29  7:50 ` netdev-bot+sinfo
@ 2026-10-29 15:54 ` Jakub Kicinski
  2026-11-02  7:59 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-10-29 15:54 UTC (permalink / raw)
  To: vmukunda
  Cc: linux-kernel, netdev, Sunil Goutham, Ratheesh Kannoth,
	Geetha sowjanya, Subbaraya Sundeep, Andrew Lunn,
	David S . Miller, Eric Dumazet, Paolo Abeni,
	Naveen Mamindlapalli

On Thu, 29 Oct 2026 07:46:35 +0000 vmukunda wrote:
> Signed-off-by: vmukunda <vmukunda@marvell.com>

I told you yesterday - real name needed.

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

* Re: [PATCH] octeontx2-af: fix TX hang after link mode change under traffic
  2026-10-29  7:46 [PATCH] octeontx2-af: fix TX hang after link mode change under traffic vmukunda
  2026-10-29  7:50 ` netdev-bot+sinfo
  2026-10-29 15:54 ` Jakub Kicinski
@ 2026-11-02  7:59 ` netdev-bot+sashiko
  2 siblings, 0 replies; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-11-02  7:59 UTC (permalink / raw)
  To: vmukunda
  Cc: linux-kernel, netdev, sgoutham, rkannoth, gakula, sbhatta,
	andrew+netdev, davem, edumazet, kuba, pabeni, naveenm

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

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

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] TL1 SW_XOFF is set one way and cleared another, so on some
  silicon it is never cleared.
- [Medium] The Fixes: tag points at commit 62945033ea22 ("octeontx2-af:
  poll for tx link credits before link mode change"), which is not in…
- [Medium] The credit-return poll in
  rvu_nix_tl1_xoff_wait_for_link_credits() has a fixed budget: count =…
- [Medium] Nothing serializes the new TL1 SW_XOFF set -> credit poll ->
  cgx_set_link_mode() -> clear sequence against FLR teardown.
- [Low] The commit message says a credit poll timeout 'is only reported
  with a warning', but rvu_nix_tl1_xoff_wait_for_link_credits() logs it…
- [Low] The timeout message in rvu_nix_tl1_xoff_wait_for_link_credits()
  prints two u64 values (tx_credits and ((regval >> 12) & 0xFFFFF)) with…

--

octeontx2-af: fix TX hang after link mode change under traffic

The patch sets TL1 SW_XOFF and polls for the TX link credits to return
before a CGX/RPM link mode change. It clears TL1 SW_XOFF afterwards,
including when the change fails. The aim is to stop the link credits from
going negative and hanging TX.

> Fixes: 62945033ea22 ("octeontx2-af: poll for tx link credits before link mode change")

[Severity: Medium]
Is this the right commit for the Fixes: tag? 62945033ea22 does not seem
to exist in the tree, and git show reports an unknown revision. The
quoted subject also describes what this patch itself does.

git blame on rvu_mbox_handler_cgx_set_link_mode() shows that the handler
and its cgx_set_link_mode() call come from 56b6d539861358. Later changes
came from 25d51ebf0f54f9, 5f21226b79fd8f and 4a3dba48188208.

Could the tag name the upstream commit that introduced this path, or be
dropped? Stable tooling can't resolve the current SHA.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> index 41f157ea27f4b..fbcc1038af4b2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_cgx.c
> @@ -1235,8 +1235,13 @@ int rvu_mbox_handler_cgx_set_link_mode(struct rvu *rvu,
>  	else
>  		linkmodes = &rvu->fwdata->cgx_fw_data[cgx_idx][lmac];
>
> +	rvu_nix_tl1_xoff_wait_for_link_credits(rvu, req->hdr.pcifunc);
> +
>  	rsp->status = cgx_set_link_mode(cgxd, req->args, linkmodes,
>  					cgx_idx, lmac);
> +
> +	rvu_nix_tl1_xoff_clear(rvu, req->hdr.pcifunc);
> +
>  	return 0;
>  }

[Severity: Medium]
What serializes this set, poll, mode change and clear sequence against
FLR teardown? The mbox handler holds neither rvu->rsrc_lock nor
rvu->flr_lock. Meanwhile a VF FLR runs on rvu->flr_wq:

__rvu_flr_handler()
  rvu_blklf_teardown()
    rvu_nix_lf_teardown()
      nix_txschq_free()
        nix_clear_tx_xoff(rvu, blkaddr, NIX_TXSCH_LVL_TL1,
                          nix_get_tx_link(rvu, pcifunc));

nix_get_tx_link() maps a VF to its parent PF's link. That makes it the
same TL1 SW_XOFF register that is held here across cgx_set_link_mode(),
which can take up to CGX_CMD_TIMEOUT (5000 ms).

If the VF FLR lands during the firmware mode change, TL1 XOFF is cleared
and PF traffic resumes partway through the change. Could that bring the
credit loss back?

If the XOFF set here happens after nix_txschq_free() has cleared the
XOFFs ahead of its SMQ flush, could the flush hit a stopped TL1 and time
out?

If a PF FLR detaches the NIX LF between the set and the clear,
rvu_nix_tl1_xoff_clear() repeats the lookup:

	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
	if (blkaddr < 0)
		return NIX_AF_ERR_AF_LF_INVALID;

Would that return without clearing, and leave the shared TL1 stopped for
the VFs?

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> index 29559db689290..b5122b8647ae2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c

[ ... ]

> +int rvu_nix_tl1_xoff_wait_for_link_credits(struct rvu *rvu, u16 pcifunc)
> +{
> +	int link, blkaddr, count = 1000;
> +	u64 tx_credits, regval;
> +	struct nix_hw *nix_hw;

[ ... ]

> +	/* set TL1 sw_xoff */
> +	link = nix_get_tx_link(rvu, pcifunc);
> +	rvu_write64(rvu, blkaddr, NIX_AF_TL1X_SW_XOFF(link), 1);
> +
> +	/* wait for link credits to return */
> +	tx_credits = nix_hw->tx_credits[link];
> +	while (1) {
> +		regval = rvu_read64(rvu, blkaddr,
> +				    NIX_AF_TX_LINKX_NORM_CREDIT(link));
> +		if (((regval >> 12) & 0xFFFFF) == tx_credits)
> +			break;
> +		count--;
> +		if (!count) {
> +			dev_info(rvu->dev, "TX link(%d) credit poll timeout, expected=%lld credits=%lld\n",
> +				 link, tx_credits, ((regval >> 12) & 0xFFFFF));

[Severity: Low]
The commit message says "A timeout in the credit poll is only reported
with a warning", but this uses dev_info(). Should this be dev_warn(), or
should the commit message be changed?

[Severity: Low]
This isn't a bug, but tx_credits and the masked regval are both u64.
Should these use %llu, or %#llx like the other register dumps?

> +			return -ETIMEDOUT;
> +		}
> +		cpu_relax();
> +		udelay(1);
> +	}

[Severity: Medium]
Is 1000 iterations of an MMIO read plus udelay(1), about 1-2 ms, enough
for the credits to come back? The expected value is set in
nix_link_config():

	tx_credits = (lmac_fifo_len - lmac_max_frs) / 16;

So the outstanding credits are bytes still in the LMAC TX FIFO, and those
only drain at line rate. A full FIFO of tens of KB takes about 5 ms at
100M and about 50 ms at 10M. If the peer is sending PAUSE/PFC, there is
no upper bound.

On timeout, rvu_mbox_handler_cgx_set_link_mode() ignores the -ETIMEDOUT
and starts the firmware mode change with credits still outstanding.
NIX_AF_TX_LINKX_NORM_CREDIT is only programmed at init by
nix_link_config(). Can the TX hang this patch targets still happen on
slow or backpressured links?

nix_smq_flush() handles a similar drain differently. It enables LMAC TX,
disables RX backpressure and uses a jiffies-based timeout. Would a
similar approach work here?

> +	return 0;
> +}
> +
> +int rvu_nix_tl1_xoff_clear(struct rvu *rvu, u16 pcifunc)
> +{
> +	int blkaddr;
> +
> +	blkaddr = rvu_get_blkaddr(rvu, BLKTYPE_NIX, pcifunc);
> +	if (blkaddr < 0)
> +		return NIX_AF_ERR_AF_LF_INVALID;
> +
> +	nix_clear_tx_xoff(rvu, blkaddr, NIX_TXSCH_LVL_TL1,
> +			  nix_get_tx_link(rvu, pcifunc));

[Severity: High]
Does this clear TL1 SW_XOFF on all silicon?
rvu_nix_tl1_xoff_wait_for_link_credits() always sets it:

	rvu_write64(rvu, blkaddr, NIX_AF_TL1X_SW_XOFF(link), 1);

However, nix_clear_tx_xoff() returns before writing anything when
shaping is not supported:

	/* Skip this if shaping is not supported */
	if (!hw->cap.nix_shaping)
		return;

rvu_setup_hw_capabilities() sets hw->cap.nix_shaping = false whenever
is_rvu_pre_96xx_C0() is true. That covers 96XX A0/B0 and 95XX A0/A1/B0.

On those parts, would every CGX_SET_LINK_MODE request from a CGX-mapped
PF leave the link's TL1 in XOFF? That would stop TX for the PF and all
of its VFs.

No other path seems to clear it on those parts either.
handle_txschq_shaper_update() only runs when nix_shaper_toggle_wait is
set, and that is false on pre-96xx-C0. nix_txschq_free() also goes
through nix_clear_tx_xoff().

This also seems to contradict the commit message, which says SW_XOFF is
cleared "so that a failed request does not leave the link stopped".

Could the set and the clear be gated the same way? One option is to
write 0 directly here. Another is to skip the set when
!hw->cap.nix_shaping.

> +	return 0;
> +}

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261029074635.4139111-1-vmukunda%40marvell.com

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

end of thread, other threads:[~2026-11-02  7:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29  7:46 [PATCH] octeontx2-af: fix TX hang after link mode change under traffic vmukunda
2026-10-29  7:50 ` netdev-bot+sinfo
2026-10-29 15:54 ` Jakub Kicinski
2026-11-02  7:59 ` netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®