RE: [PATCH v1 1/1] virtio-net: Fix ECN feature descritpion

> -----Original Message-----
> From: Michael S. Tsirkin <[email protected]> 
> Sent: Wednesday, April 16, 2025 8:45 PM
> To: Chia-Yu Chang (Nokia) <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Koen De Schepper (Nokia) <[email protected]>; g.white <[email protected]>; vidhi_goel <[email protected]>; [email protected]; [email protected]
> Subject: Re: [PATCH v1 1/1] virtio-net: Fix ECN feature descritpion
> 
> 
> 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 Wed, Apr 16, 2025 at 05:52:23PM +0200, [email protected] wrote:
> > 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 TCP CWR flag is set and 
> > will be cleared from the second segment. This is used to offload TCP 
> > CWR flag in a way that is compatible with RFC3168 ECN but is 
> > problematic for non-RFC3168 use of CWR flag.
> >
> > Signed-off-by: Chia-Yu Chang <[email protected]>
> 
> Can you pls send a patch sans typos? Thanks!

Hello,

I've just sent v7 of RFC3168 ECN and AccECN patch to v1.4 branch
And this single patch is to backport the RFC3168 ECN to the master branch (v1.3).
Is this being the right way to do the backporting for v1.3 standard? Thanks.

Chia-Yu

> 
> > ---
> >  device-types/net/description.tex | 33 +++++++++++++++++---------------
> >  introduction.tex                 |  3 +++
> >  2 files changed, 21 insertions(+), 15 deletions(-)
> >
> > diff --git a/device-types/net/description.tex 
> > b/device-types/net/description.tex
> > index 4494633..f15b74a 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -54,7 +54,9 @@ \subsection{Feature bits}\label{sec:Device Types / 
> > Network Device / Feature bits
> >
> >  \item[VIRTIO_NET_F_GUEST_TSO6 (8)] Driver can receive TSOv6.
> >
> > -\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 TCP CWR flag set
> > +    and follow the ACE bits handling approach mentioned in
> > +    \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 TCP CWR flag set
> > +    and follow the ACE bits handling approach mentioend in
> > +    \hyperref[intro:rfc3168]{[RFC3168]}.
> >
> >  \item[VIRTIO_NET_F_HOST_UFO (14)] Device can receive UFO.
> >
> > @@ -512,8 +516,9 @@ \subsubsection{Packet 
> > Transmission}\label{sec:Device Types / Network Device / De
> >
> >    \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.}.
> > +    indicates that the TCP packet has TCP CWR flag set and the flag will be handled differently to all segements of
> > +    an aggregated segment, as mentioned in \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 \field{num_buffers} is set to zero.  This field is unused on transmitted packets.
> > @@ -555,10 +560,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 have the TCP CWR flag set and require the flag be handled as 
> > +mentioned in \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 the VIRTIO_NET_F_CSUM feature has been negotiated, the  driver MAY 
> > set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in @@ -748,9 +752,9 @@ 
> > \subsubsection{Processing of Incoming Packets}\label{sec:Device Types 
> > / Network  \end{enumerate}
> >
> >  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP 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
> > +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}
> > @@ -821,10 +825,9 @@ \subsubsection{Processing of Incoming 
> > Packets}\label{sec:Device Types / Network  the device MUST NOT set \field{gso_type} to VIRTIO_NET_HDR_GSO_UDP_L4.
> >
> >  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 have the TCP CWR flag set and require the flag be handled as 
> > +mentioned in \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 the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the  
> > device MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in diff --git 
> > a/introduction.tex b/introduction.tex index 44c4ca4..8b79cba 100644
> > --- a/introduction.tex
> > +++ b/introduction.tex
> > @@ -145,6 +145,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{http://www.ietf.org/rfc/rfc8174.txt}\\
> > +        \phantomsection\label{intro:rfc3168}\textbf{[RFC3168]} &
> > +    S. Floyd., ``The Addition of Explicit Congestion Notification (ECN) to IP'', September 2001.
> > +        \newline\url{http://www.ietf.org/rfc/rfc3168.txt}\\
> >  \end{longtable}
> >
> >  \section{Non-Normative References}
> > --
> > 2.34.1