* Re: [RFC net-next] net: thunderbolt: Allow per-device opt-in Tx E2E flow control
2026-09-23 7:00 ` [RFC net-next] net: thunderbolt: Allow per-device opt-in Tx E2E flow control Mika Westerberg
@ 2026-09-23 7:50 ` Yusheng Zheng
0 siblings, 0 replies; 2+ messages in thread
From: Yusheng Zheng @ 2026-09-23 7:50 UTC (permalink / raw)
To: Mika Westerberg
Cc: westeri, netdev, YehezkelShB, andrew+netdev, davem, edumazet,
kuba, pabeni, linux-kernel
Hi Mika,
> If we advertise E2E support over XDomain we must enable it as well
You're right. I missed that the XDomain advertisement remains visible
to a peer even when the proposed flag leaves our Tx ring without E2E.
That makes this service-driver opt-in unsuitable. I won't pursue this
approach. I'll follow the NHI-side work you mentioned and can test a
public patch on the two Intel 8086:7ec2 hosts. The A/B/A results in my
RFC are observations on that pair, not a justification for this
interface.
> Also this looks pretty much like LLM generated, so you should add
> Assisted-by tag.
You're right about that too. LLM assistance was used for the code and
changelog, and I failed to disclose it. I'm sorry. For the record, the
RFC should have included Assisted-by: LLM.
My first Gmail message was rejected by netdev because it contained an
HTML part. I later sent the plain-text RFC through b4:
https://lore.kernel.org/netdev/20260923-tbnet-tx-e2e-ethtool-v11-v1-1-33a2ca1318f1@gmail.com/
Sorry for the split thread.
Thanks again,
Yusheng
On Wed, Sep 23, 2026 at 12:00 AM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> Hi,
>
> On Tue, Sep 22, 2026 at 11:43:16PM -0700, Yusheng Zheng wrote:
> > Commit a8065af3346e ("net: thunderbolt: Enable end-to-end flow control
> > also in transmit") enabled E2E flow control on Tx rings. It was reverted
> > after an ASMedia ASM4242 accepted the configuration without returning
> > credits, leaving its Tx ring wedged. A separate report described similar
> > Tx completion wedges with multiple native E2E rings on AMD Strix Halo.
> > The revert kept Rx E2E and suggested a Tx opt-in for known-good
> > controllers.
>
> AMD is also working on a patch that prevents the issue.
>
> > On two Intel Meteor Lake-P NHI (8086:7ec2) hosts, the exact patch built
> > against their running 7.3-rc3 kernel was tested in a same-boot A/B/A run.
> > Each arm used a ten-second, four-stream iperf3 transfer. For lab to g485,
> > Tx E2E on, off, then on again produced 15.43, 15.76, and 15.18 Gbit/s,
> > with 0, 29113, and 0 retransmissions. G485 Rx errors rose by 5097 during
> > the off arm and did not rise during either on arm. In the reverse direction,
> > throughput was about 19.41 Gbit/s in all three arms, with 0, 181, and 0
> > retransmissions; lab Rx errors rose by 14 only during the off arm.
> >
> > The link was brought down and up to change the flag, so link training was
> > not held fixed. The result supports an integrity improvement on this pair,
> > not a general throughput gain or proof for other controllers. Both hosts
> > were returned to their distribution module after testing.
> >
> > Add a per-netdev ethtool private flag named tx-e2e. It defaults to off and
> > can only change while the interface is down. Enabling it requires the
> > existing e2e module parameter. At open, the Tx ring uses E2E only if the
> > peer advertises TBNET_E2E. This bit proves protocol support, not working
> > credit return. Administrators must opt in only on controller pairs known
> > to return credits; a bad controller can stall Tx until the flag is
> > disabled.
>
> If we advertise E2E support over XDomain we must enable it as well
> otherwise the other side that can be something else that Linux will be
> disapointed.
>
> I think for this, we should come up a solution that deals with the NHI
> wedge problem directly in the TB driver side (and this is what AMD is
> working on). Then the Thunderbolt service drivers do not need to care.
>
> Also this looks pretty much like LLM generated, so you should add
> Assisted-by tag.
>
> > Link: https://lore.kernel.org/netdev/20260727123002.25225-1-fy15309206903@gmail.com/
> > Signed-off-by: Yusheng Zheng <yunwei356@gmail.com>
> > ---
> > RFC question: Is a per-netdev ethtool private flag an acceptable opt-in for
> > known-good controller pairs, given that the peer protocol bit cannot prove
> > that the controller returns Tx E2E credits?
> >
> > Validation: The patched module built with W=1 against the running 7.3-rc3
> > headers and ran on both Intel hosts. A same-boot A/B/A test is summarized
> > above. Enabling the flag while the interface was up returned EBUSY; ring
> > flags were observed as FRAME without the opt-in and FRAME|E2E with it.
> >
> > A net-next allmodconfig W=1 build was attempted but failed on GCC 15
> > -Werror diagnostics in unrelated files including samples/trace_events and
> > kernel/locking; it did not provide a clean whole-tree result.
> >
> > drivers/net/thunderbolt/main.c | 60 ++++++++++++++++++++++++++++++++--
> > 1 file changed, 58 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/thunderbolt/main.c b/drivers/net/thunderbolt/main.c
> > index d9fb587a62c5..b17ab8e15c1f 100644
> > --- a/drivers/net/thunderbolt/main.c
> > +++ b/drivers/net/thunderbolt/main.c
> > @@ -162,2 +162,3 @@ struct tbnet_ring {
> > * host
> > + * @tx_e2e: Enable end-to-end flow control on the Tx ring
> > * @local_transmit_path: HopID we are using to send out packets
> > @@ -192,2 +193,3 @@ struct tbnet {
> > bool login_received;
> > + bool tx_e2e;
> > int local_transmit_path;
> > @@ -950,3 +952,7 @@ static int tbnet_open(struct net_device *dev)
> > - ring = tb_ring_alloc_tx(xd->tb->nhi, -1, TBNET_RING_SIZE,
> > - RING_FLAG_FRAME);
> > + flags = RING_FLAG_FRAME;
> > + /* The peer bit advertises E2E protocol support. */
> > + if (net->tx_e2e && tbnet_e2e && net->svc->prtcstns & TBNET_E2E)
> > + flags |= RING_FLAG_E2E;
> > +
> > + ring = tb_ring_alloc_tx(xd->tb->nhi, -1, TBNET_RING_SIZE, flags);
> > if (!ring) {
> > @@ -1339,3 +1345,53 @@
> > +static const char tbnet_priv_flags[][ETH_GSTRING_LEN] = {
> > +#define TBNET_PRIV_FLAG_TX_E2E BIT(0)
> > + "tx-e2e",
> > +};
> > +
> > +static void tbnet_get_strings(struct net_device *dev, u32 stringset, u8 *data)
> > +{
> > + if (stringset == ETH_SS_PRIV_FLAGS)
> > + memcpy(data, tbnet_priv_flags, sizeof(tbnet_priv_flags));
> > +}
> > +
> > +static int tbnet_get_sset_count(struct net_device *dev, int sset)
> > +{
> > + if (sset == ETH_SS_PRIV_FLAGS)
> > + return ARRAY_SIZE(tbnet_priv_flags);
> > +
> > + return -EOPNOTSUPP;
> > +}
> > +
> > +static u32 tbnet_get_priv_flags(struct net_device *dev)
> > +{
> > + const struct tbnet *net = netdev_priv(dev);
> > +
> > + return net->tx_e2e ? TBNET_PRIV_FLAG_TX_E2E : 0;
> > +}
> > +
> > +static int tbnet_set_priv_flags(struct net_device *dev, u32 flags)
> > +{
> > + struct tbnet *net = netdev_priv(dev);
> > + bool tx_e2e;
> > +
> > + if (flags & ~TBNET_PRIV_FLAG_TX_E2E)
> > + return -EINVAL;
> > +
> > + tx_e2e = flags & TBNET_PRIV_FLAG_TX_E2E;
> > + if (net->tx_e2e == tx_e2e)
> > + return 0;
> > + if (netif_running(dev))
> > + return -EBUSY;
> > + if (tx_e2e && !tbnet_e2e)
> > + return -EOPNOTSUPP;
> > +
> > + net->tx_e2e = tx_e2e;
> > + return 0;
> > +}
> > +
> > static const struct ethtool_ops tbnet_ethtool_ops = {
> > .get_link_ksettings = tbnet_get_link_ksettings,
> > + .get_strings = tbnet_get_strings,
> > + .get_sset_count = tbnet_get_sset_count,
> > + .get_priv_flags = tbnet_get_priv_flags,
> > + .set_priv_flags = tbnet_set_priv_flags,
> > };
> > --
> > 2.55.0
> >
^ permalink raw reply [flat|nested] 2+ messages in thread