Re: [RFC net-next] net: thunderbolt: Allow per-device opt-in Tx E2E flow control

Re: [RFC net-next] net: thunderbolt: Allow per-device opt-in Tx E2E flow control

[Mika Westerberg]

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
> 

Re: [RFC net-next] net: thunderbolt: Allow per-device opt-in Tx E2E flow control

[Yusheng Zheng]

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