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

> -----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.
> >>
> >>
> >>
> >> Hi,
> >>
> >>> From: [email protected] <chia-yu.chang@nokia-bell- 
> >>> labs.com>
> >>>
> >>> From: Chia-Yu Chang <[email protected]>
> >>>
> >>> Clarify that the VIRTIO_NET_HDR_GSO_ECN gso_type flag does not mean 
> >>> that TCP has IP-ECN set; instead, it identifies that the TCP CWR 
> >>> flag is set and will be cleared from the second segment of an aggregated segment.
> >>>
> >> Above text is seems to be clarified only in commit message, not in the spec changes below.
> >> Can you please add it in the actual spec wording, including requirements?
> > Hi Parav,
> >
> > This is to fix current error in virtio spec:
> > "VIRTIO_NET_HDR_GSO_ECN is a flag within the VIRTIO_NET_HDR structure that indicates the TCP header of a packet has its Explicit Congestion Notification (ECN) bit set, signifying congestion in the network"
> 
> I am unable to find the above cited text in virtio spec.
> 
> Do I miss to find above text in the spec?
> 
> 
> What I read in spec is:
> 
> The driver SHOULD NOT send to the device TCP packets requiring segmentation offload which have the
> 
> Explicit Congestion Notification bit set, 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.

Yes, you are right.

> > TCP header does not have ECN bit, and this flag shall indicate the CWR bit of TCP header is set when doing the GSO for the first packet.
> > The corresponding texts in RFC3168 are "When the TCP data sender is ready to set the CWR bit after reducing the congestion window, it SHOULD set the CWR bit only on the first new data packet that it transmits."
> >
> > So, do you think adding above texts in the commit message is ok?
> 
> Yes, but only commit message is not enough. We need equivalent text in the description and in driver + device requirement section.
> 
> Something like,
> 
> When the device is transmitting packets of gso_type of segmentation type A, B, or C, and if driver requested gso_type of HOST_ECN, the device must set the ECN bits in the IP header and CWR bit in the TCP header in the first data packet of the TCP or UDP segments.
> 

I think the text changes are already proposed below.

Or you can refer to my original email for better comparison: https://yhbt.net/lore/virtio-comment/[email protected]/T/#mc1750dcc26d74c9b19939c4e86dae8bbade6ec78

> >
> >>> This is used to offload the TCP CWR flag in a way that is compatible 
> >>> with
> >>> RFC3168 ECN but is problematic for non-RFC3168 use of the TCP CWR flag.
> >>>
[...]
> >>> -\item[VIRTIO_NET_F_GUEST_ECN (9)] Driver can receive TSO with ECN.
> >>> +\item[VIRTIO_NET_F_GUEST_ECN (9)] Driver can receive TSO with the 
> >>> +CWR
> >>> flag set
> >>> +    and follow the requirements of the CWR bit described in
> >>> +    ``Section 6.1.2. The TCP Sender'' of \hyperref[intro:rfc3168]{[RFC3168]}.
> >>>
> >>>   \item[VIRTIO_NET_F_GUEST_UFO (10)] Driver can receive UFO.
> >>>
> >>> @@ -62,7 +64,9 @@ \subsection{Feature bits}\label{sec:Device Types / 
> >>> Network Device / Feature bits
> >>>
> >>>   \item[VIRTIO_NET_F_HOST_TSO6 (12)] Device can receive TSOv6.
> >>>
> >>> -\item[VIRTIO_NET_F_HOST_ECN (13)] Device can receive TSO with ECN.
> >>> +\item[VIRTIO_NET_F_HOST_ECN (13)] Device can receive TSO with the 
> >>> +CWR
> >>> flag set
> >>> +    and follow the requirements of the CWR bit described in
> >>> +    ``Section 6.1.2. The TCP Sender'' of \hyperref[intro:rfc3168]{[RFC3168]}.
> >>>
> >>>   \item[VIRTIO_NET_F_HOST_UFO (14)] Device can receive UFO.
> >>>
> >>> @@ -729,9 +733,10 @@ \subsubsection{Packet 
> >>> Transmission}\label{sec:Device Types / Network Device / De
> >>>       header (ie. MSS).
> >>>
> >>>     \item If the driver negotiated the VIRTIO_NET_F_HOST_ECN feature,
> >>> -    the VIRTIO_NET_HDR_GSO_ECN bit in \field{gso_type}
> >>> -    indicates that the TCP packet has the ECN bit set\footnote{This case is not
> >>> handled by some older hardware, so is called out -specifically in 
> >>> the protocol.}.
> >>> +    the VIRTIO_NET_HDR_GSO_ECN bit in \field{gso_type} indicates 
> >>> + that the
> >>> TCP packet
> >>> +    has the CWR flag set and follows the requirements of the CWR 
> >>> + flag
> >>> described in
> >>> +    ``Section 6.1.2. The TCP Sender'' of \hyperref[intro:rfc3168]{[RFC3168]}.
> >>> +    \footnote{This case is not handled by some older hardware, so 
> >>> + is called
> >>> out specifically in the protocol.}.
> >>>      \end{itemize}
> >>>
> >>>   \item If the driver negotiated the 
> >>> VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO feature and the @@ -828,10 +833,9 
> >>> @@ \subsubsection{Packet Transmission}\label{sec:Device Types / 
> >>> Network Device / De \field{gso_type} to VIRTIO_NET_HDR_GSO_UDP_L4.
> >>>
> >>>   The driver SHOULD NOT send to the device TCP packets requiring 
> >>> segmentation offload -which have the Explicit Congestion 
> >>> Notification bit set, 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 -\field{gso_type}.
> >>> +which need to follow the CWR flag requirements described in ``Section 6.1.2.
> >>> The TCP Sender''
> >>> +of \hyperref[intro:rfc3168]{[RFC3168]}, 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 \field{gso_type}.
> >>>
> >>>   If VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated, the driver MAY set
> >>>   VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 bit or the
> >>> VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 bit @@ -1168,9 +1172,9 @@ 
> >>> \subsubsection{Processing of Incoming Packets}\label{sec:Device 
> >>> Types / Network  \end{enumerate}
> >>>
> >>>   Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL 
> >>> - and ECN features enable receive checksum, large receive offload 
> >>> and ECN - support which are the input equivalents of the transmit 
> >>> checksum, -transmit segmentation offloading and ECN features, as 
> >>> described
> >>> +and ECN features enable receive checksum, large receive offload and
> >>> +RFC3168 ECN support which are the input equivalents of the transmit 
> >>> +checksum, transmit segmentation offloading and RFC3168 ECN 
> >>> +features, as described
> >>>   in \ref{sec:Device Types / Network Device / Device Operation / 
> >>> Packet
> >>> Transmission}:
> >>>   \begin{enumerate}
> >>> @@ -1273,10 +1277,9 @@ \subsubsection{Processing of Incoming 
> >>> Packets}\label{sec:Device Types / Network  the 
> >>> VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM bit in \field{flags}.
> >>>
> >>>   The device SHOULD NOT send to the driver TCP packets requiring 
> >>> segmentation offload -which have the Explicit Congestion 
> >>> Notification bit set, unless the -VIRTIO_NET_F_GUEST_ECN feature is 
> >>> negotiated, in which case the -device MUST set the VIRTIO_NET_HDR_GSO_ECN bit in -\field{gso_type}.
> >>> +which need to follow the CWR flag requirements described in ``Section 6.1.2.
> >>> The TCP Sender''
> >>> +of \hyperref[intro:rfc3168]{[RFC3168]}, unless the 
> >>> +VIRTIO_NET_F_GUEST_ECN feature is negotiated, in which case the 
> >>> +device
> >>> MUST set the VIRTIO_NET_HDR_GSO_ECN bit in \field{gso_type}.
> >>>
> >>>   If VIRTIO_NET_F_OUT_NET_HEADER has been negotiated, the device MAY 
> >>> set the \field{outer_nh_offset} to nonzero value to indicate outer 
> >>> network header offset, diff --git a/introduction.tex 
> >>> b/introduction.tex index
> >>> 80aa67a..1c4cb33 100644
> >>> --- a/introduction.tex
> >>> +++ b/introduction.tex
> >>> @@ -180,6 +180,9 @@ \section{Normative 
> >>> References}\label{sec:Normative References}
> >>>       Leiba, B., "Ambiguity of Uppercase vs Lowercase in RFC 2119 
> >>> Key Words", BCP
> >>>       14, RFC 8174, DOI 10.17487/RFC8174, May 2017
> >>>
> >>> \newline\url{https://eur03.safelinks.protection.outlook.com/?url=htt
> >>> p%3A%2F%2Fhttps%2F&data=05%7C02%7Cchia-yu.chang%40nokia-bell-labs.co
> >>> m%7C748641ab9b5646b8e9a108ddffd806df%7C5d4717519675428d917b70f44f963
> >>> 0b0%7C0%7C0%7C638948025520045144%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1
> >>> hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIld
> >>> UIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=dozd2qz%2Bw5gv1Iu0XzP6wvMfgxu9CM018
> >>> 8edkRqC2kE%3D&reserved=0
> >>> %3A%2F%2Fnam11.safelinks.protection.outlook.com%2F%3Furl%3Dhttp%253A
> >>> %2
> >>> 525&data=05%7C02%7Cchia-yu.chang%40nokia-bell-labs.com%7C9bd29c383d0
> >>> e4
> >>> 96aa3c708ddff2c774a%7C5d4717519675428d917b70f44f9630b0%7C0%7C0%7C638
> >>> 94 
> >>> 7288674229109%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOi
> >>> Iw 
> >>> LjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%
> >>> 7C
> >>> %7C&sdata=vhqjeUh8%2B6VOKIe3RF5WxlMoQwNkC3CacP0C1kN60fI%3D&reserved=
> >>> 0 
> >>> 2F%2Fwww.ietf.org%2Frfc%2Frfc8174.txt&data=05%7C02%7Cparav%40nvidia.
> >>> com%7Cc6ffebfc748f4772784708dddb2a50c9%7C43083d15727340c1b7db39e
> >>> fd9ccc17a%7C0%7C0%7C638907696998232587%7CUnknown%7CTWFpbGZsb
> >>> 3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkF
> >>> OIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=jRjpor%2FkXifPfp
> >>> t7IFBo5JNF4%2FlE8pBsw7CMT4wS%2FRo%3D&reserved=0}\\
> >>> +     \phantomsection\label{intro:rfc3168}\textbf{[RFC3168]} &
> >>> +    S. Floyd., ``The Addition of Explicit Congestion Notification
> >>> + (ECN) to IP'',
> >>> September 2001.
> >>> +
> >>> +\newline\url{https://eur03.safelinks.protection.outlook.com/?url=ht
> >>> +tp%3A%2F%2Fhttp%2F&data=05%7C02%7Cchia-yu.chang%40nokia-bell-labs.c
> >>> +om%7C748641ab9b5646b8e9a108ddffd806df%7C5d4717519675428d917b70f44f9
> >>> +630b0%7C0%7C0%7C638948025520069778%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0
> >>> +eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCI
> >>> +sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=ACMLSb6oTQXGLflY1rJtEPC4f43jH6
> >>> +ppipO3cRipNIc%3D&reserved=0
> >>> +s%3A%2F%2Fnam11.safelinks.protection.outlook.com%2F%3Furl%3Dhttp%25
> >>> +25
> >>> +3&data=05%7C02%7Cchia-yu.chang%40nokia-bell-labs.com%7C9bd29c383d0e
> >>> +49
> >>> +6aa3c708ddff2c774a%7C5d4717519675428d917b70f44f9630b0%7C0%7C0%7C638
> >>> +94 
> >>> +7288674266369%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiO
> >>> +iI 
> >>> +wLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7
> >>> +C% 
> >>> +7C%7C&sdata=AjjuXnQgsRppd33zhfiyfkIaWljEuCZR8T%2Fl8cvXby8%3D&reserv
> >>> +ed
> >>> +=0 
> >>> +A%2F%2Fwww.ietf.org%2Frfc%2Frfc3168.txt&data=05%7C02%7Cparav%40nv
> >>> idia.c
> >>> +om%7Cc6ffebfc748f4772784708dddb2a50c9%7C43083d15727340c1b7db39e
> >>> fd9ccc17
> >>> +a%7C0%7C0%7C638907696998249586%7CUnknown%7CTWFpbGZsb3d8eyJ
> >>> FbXB0eU1hcGki
> >>> +OnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIj
> >>> oyfQ%
> >>> +3D%3D%7C0%7C%7C%7C&sdata=q7NaQPhNzVlOz0bGZVTw5ZYWFcq412q8J
> >>> 57YHxTpkMs%3D
> >>> +&reserved=0}\\
> >>>   \end{longtable}
> >>>
> >>>   \section{Non-Normative References}
> >>> --
> >>> 2.34.1
> >>>