RE: [PATCH v10 1/2] virtio-net: Fix ECN feature descriptions

> From: Chia-Yu Chang (Nokia) <[email protected]>
> Sent: 01 October 2025 05:56 PM
> 
> > -----Original Message-----
> > From: Parav Pandit <[email protected]>
> > Sent: Wednesday, October 1, 2025 12:51 PM
> > To: Chia-Yu Chang (Nokia) <[email protected]>;
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> [email protected];
> > [email protected]; [email protected]; [email protected]; Koen De
> > Schepper (Nokia) <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; [email protected]
> > Subject: RE: [PATCH v10 1/2] virtio-net: Fix ECN feature descriptions
> >
> >
> > CAUTION: This is an external email. Please be very careful when clicking
> links or opening attachments. See the URL nok.it/ext for additional
> information.
> >
> >
> >
> > > From: Chia-Yu Chang (Nokia) <[email protected]>
> > > Sent: 01 October 2025 01:04 PM
> > >
> > > > -----Original Message-----
> > > > From: Parav Pandit <[email protected]>
> > > > Sent: Wednesday, October 1, 2025 6:24 AM
> > > > To: Chia-Yu Chang (Nokia) <[email protected]>;
> > > > [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected];
> > > [email protected];
> > > > [email protected]; [email protected]; [email protected]; Koen De
> > > > Schepper (Nokia) <[email protected]>;
> > > > [email protected]; [email protected];
> > > > [email protected]; [email protected]
> > > > Subject: RE: [PATCH v10 1/2] virtio-net: Fix ECN feature
> > > > descriptions
> > > >
> > > >
> > > > CAUTION: This is an external email. Please be very careful when
> > > > clicking
> > > links or opening attachments. See the URL nok.it/ext for additional
> > > information.
> > > >
> > > >
> > > >
> > > > > From: Chia-Yu Chang (Nokia) <[email protected]>
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Parav Pandit <[email protected]>
> > > > > > Sent: Tuesday, September 30, 2025 6:15 AM
> > > > > > To: Chia-Yu Chang (Nokia) <[email protected]>;
> > > > > > [email protected]; [email protected];
> > > > > > [email protected]; [email protected];
> [email protected];
> > > > > [email protected];
> > > > > > [email protected]; [email protected]; [email protected];
> Koen
> > > > > > De Schepper (Nokia) <[email protected]>;
> > > > > > [email protected]; [email protected];
> > > > > > [email protected];
> > > > > > [email protected]
> > > > > > Subject: Re: [PATCH v10 1/2] virtio-net: Fix ECN feature
> > > > > > descriptions
> > > > > >
> > > > > >
> > > > > > CAUTION: This is an external email. Please be very careful
> > > > > > when clicking
> > > > > links or opening attachments. See the URL nok.it/ext for
> > > > > additional information.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On 29-09-2025 11:14 pm, Chia-Yu Chang (Nokia) wrote:
> > > > > > >> -----Original Message-----
> > > > > > >> From: Parav Pandit <[email protected]>
> > > > > > >> Sent: Monday, September 29, 2025 9:48 AM
> > > > > > >> To: Chia-Yu Chang (Nokia)
> > > > > > >> <[email protected]>;
> > > > > > >> [email protected]; [email protected];
> > > > > > >> [email protected]; [email protected];
> > > > > > >> [email protected]; [email protected];
> > > > > > >> [email protected];
> > > [email protected];
> > > > > > >> [email protected]; Koen De Schepper (Nokia)
> > > > > > >> <[email protected]>;
> > > > > > >> [email protected]; [email protected];
> > > > > > >> [email protected];
> > > [email protected]
> > > > > > >> Subject: RE: [PATCH v10 1/2] virtio-net: Fix ECN feature
> > > > > > >> descriptions
> > > > > > >>
> > > > > > >>
> > > > > > >> CAUTION: This is an external email. Please be very careful
> > > > > > >> when clicking
> > > > > links or opening attachments. See the URL nok.it/ext for
> > > > > additional information.
> > > > > > >>
> > > > > > >>
> > > > > > >>
> [...]
> > Virtio spec requirements are clearly written.
> > So just by referring to RFC 3168 is as blurry as today in context of GSO.
> >
> > At best possibly it can be written as "device SHOULD do ECN and CWR in
> first pkt.. and subsequent packets SHOULD have cleared".
> >
> > If that is not aligning, than better to not make it more blurry.
> > ACCECN feature bit can be more crisply defined in that case.
> 
> OK, then could you let us know if we make below changes in the next version,
> are they clear enough for virtio spec?
> 
> 
> 1. VIRTIO_NET_F_GUEST_ECN (9): Driver can receive TSO with the CWR flag
> set. This CWR flag is only applied to the first packet, and the CWR flag of
> subsequent packets should be cleared.
> 
I am not confident of #1.
The Linux document clearly documents usage of VIRTIO_NET_F_HOST_ECN for the TSO (and not for the GRO).
The two hw drivers who have set SKB_GSO_TCP_ECN seems like a bug to me.
Looking at the internal specification of mlx5 hw device and code [1] and zero usage of this flag on the rx skb, it appears a bug to me.

I am going check about bug [1] in few days with more experts internally and likely provide the driver fix to remove it.

[1] https://elixir.bootlin.com/linux/v6.17/source/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c#L1299


> 2. VIRTIO_NET_F_HOST_ECN (13): Device can receive TSO with the CWR flag
> set. This CWR flag is only applied to the first packet, and the CWR flag of
> subsequent packets should be cleared.
>
I believe it applies to the two bits of the IP header too (in addition to CWR).

In the non-requirement section avoid using the key word 'should'.
In other words above line to be reworded something like:

Device can receive TSO with the CWR flag set. This TCP CWR flag and IP header ECN codepoints applies to only the first packet; these flags are cleared in subsequent packets in the offloaded segment..

 
> 3. If the driver negotiated the VIRTIO_NET_F_HOST_ECN feature, the
> VIRTIO_NET_HDR_GSO_ECN bit in gso_type indicates that the CWR flag of the
> TCP packet applied only to the first packet and is cleared in subsequent
> packets.
>
Similar wording to include IP header as well.
 
> 4. The driver SHOULD NOT send to the device TCP packets requiring
> segmentation offload with the CWR flag set ONLY for to the first packet and
> cleared in subsequent packets, unless the VIRTIO_NET_F_HOST_ECN feature is
> negotiated, in which case the driver MUST set the
> VIRTIO_NET_HDR_GSO_ECN bit in gso_type.
> 

> 5. The device SHOULD NOT send to the driver TCP packets requiring
> segmentation offload with the CWR flag set ONLY for to the first packet and
> cleared in subsequent packets, unless the VIRTIO_NET_F_GUEST_ECN feature
> is negotiated, in which case the driver MUST set the
> VIRTIO_NET_HDR_GSO_ECN bit in gso_type.
> 
s/driver MUST set the/device MUST set/

the wording around "segmentation offload' for receive is confusing in the current form.

Better wording would be,

The device SHOULD coalesce TCP packets under single GSO stream if following conditions are met.
1. first TCP packet has ECN bits set
2. second onwards packets do not have ECN bits set etc