Re: [PATCH] virtio-net: introduce TSO limit feature

On Mon, Oct 20, 2025 at 4:19 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Mon, Oct 20, 2025 at 02:25:56PM +0800, Jason Wang wrote:
> > On Thu, Oct 16, 2025 at 6:03 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Thu, Oct 16, 2025 at 02:31:46PM +0800, Jason Wang wrote:
> > > > On Thu, Oct 16, 2025 at 2:17 PM Michael S. Tsirkin <[email protected]> wrote:
> > > > >
> > > > > On Thu, Oct 16, 2025 at 01:57:41PM +0800, Jason Wang wrote:
> > > > > > On Wed, Oct 15, 2025 at 3:27 PM Michael S. Tsirkin <[email protected]> wrote:
> > > > > > >
> > > > > > > Thanks for the answers. Some more comments:
> > > > > > >
> > > > > > > On Wed, Oct 15, 2025 at 12:29:13PM +0800, Jason Wang wrote:
> > > > > > > > > >  device-types/net/description.tex | 46 ++++++++++++++++++++++++++++++++
> > > > > > > > > >  1 file changed, 46 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> > > > > > > > > > index 415c7fd..e56df75 100644
> > > > > > > > > > --- a/device-types/net/description.tex
> > > > > > > > > > +++ b/device-types/net/description.tex
> > > > > > > > > > @@ -146,6 +146,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > > > > > > >   when VIRTIO_NET_F_IPSEC is negotiated. When a device offers IPsec feature, it SHOULD
> > > > > > > > > >   also offer the VIRTIO_NET_F_OUT_NET_HEADER feature.
> > > > > > > > > >
> > > > > > > > > > +\item[VIRTIO_NET_F_HOST_TSO_LIMIT(71)] Device limits the maximum TCP
> > > > > > > > > > +  length and the number of segments when performing TCP segmentation.
> > > > > > > > > > +
> > > > > > > > > >  \end{description}
> > > > > > > > > >
> > > > > > > > > >  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> > > > > > > > > > @@ -184,6 +187,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > > > > > > > >  \item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > > > > > >  \item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ along with VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > > > > > > > > >  \item[VIRTIO_NET_F_RSS_CONTEXT] Requires VIRTIO_NET_F_CTRL_VQ and VIRTIO_NET_F_RSS.
> > > > > > > > > > +\item[VIRTIO_NET_F_HOST_TSO_LIMIT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6
> > > > > > > > > >  \end{description}
> > > > > > > > > >
> > > > > > > > > >  \begin{note}
> > > > > > > > > > @@ -220,6 +224,8 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > > > > > > >          le16 rss_max_indirection_table_length;
> > > > > > > > > >          le32 supported_hash_types;
> > > > > > > > > >          le32 supported_tunnel_types;
> > > > > > > > > > +        le32 tso_max_size;
> > > > > > > > > > +        le32 tso_max_segs;
> > > > > > > > > >  };
> > > > > > > > > >  \end{lstlisting}
> > > > > > > > > >
> > > > > > > > > > @@ -276,6 +282,19 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > > > > > > >  Encapsulation types are defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets /
> > > > > > > > > >  Hash calculation for incoming packets / Encapsulation types supported/enabled for inner header hash}.
> > > > > > > > > >
> > > > > > > > > > +The following field, \field{tso_max_size} only exists if
> > > > > > > > > > +VIRTIO_NET_F_HOST_TSO_LIMIT is set.
> > > > > > > > > > +It specifies the maximum TCP length
> > > > > > > > >
> > > > > > > > > what is TCP length?
> > > > > > > >
> > > > > > > > It's defined in the rfc793:
> > > > > > > >
> > > > > > > > """
> > > > > > > >       The TCP Length is the TCP header length plus the data length in
> > > > > > > >       octets (this is not an explicitly transmitted quantity, but is
> > > > > > > >       computed), and it does not count the 12 octets of the pseudo
> > > > > > > >       header.
> > > > > > > > """
> > > > > > >
> > > > > > >
> > > > > > > But that one is 16 bit so can not exceed 65535.
> > > > > >
> > > > > > I just reuse the terminology instead of defining something new.
> > > > >
> > > > > Let's use a generic term that will work with big tcp.
> > > >
> > > > After glancing at RFC TCP length seems to be the best as TCP header
> > > > doesn't have a length.
> > > >
> > > > Maybe you have something better in mind?
> > >
> > >
> > > Depends on what do you mean by it, exactly.
> >
> > TCP header length plus the data length. Maybe TCP packet length?
>
> Hmm. It's not even a packet length is it?
> Linux has this:
>         Device (as in HW) limit on the max TSO request size
>
> but switching to "request" is more work.
>
> for now, we have:
>
>
>
>
> \item If the driver negotiated
>   VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO, and the packet requires
>   TCP segmentation, UDP segmentation or fragmentation, then \field{gso_type}
>   is set to VIRTIO_NET_HDR_GSO_TCPV4, TCPV6, UDP_L4 or UDP.
>   (Otherwise, it is set to VIRTIO_NET_HDR_GSO_NONE). In this
>   case, packets larger than 1514 bytes can be transmitted: the
>   metadata indicates how to replicate the packet header to cut it
>   into smaller packets. The other gso fields are set:
>
>
> So if we just go with "length of a packet that requires TCP
> segmentation" then I think it's at least not making things
> any worse.

Ok, I will do it.

>
>
> > >
> > > > >
> > > > > > Note
> > > > > > that it is only used in pseudo header for csum after device has
> > > > > > performed TSO. The value in the pseudo header is capped by MTU/MSS.
> > > > > >
> > > > > > As replied in another thread, BIG TCP requires more work or features.
> > > > > > Driver needs to set ip->tot_len 0 with a new gso type to let the
> > > > > > device know about BIG TCP packet.
> > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > > of a TSO packet
> > > > > > > > >
> > > > > > > > > what is a TSO packet?
> > > > > > > >
> > > > > > > > Packet for device to perform TCP segmentation offload.
> > > > > > >
> > > > > > > pls define terms before use.
> > > > > >
> > > > > > I may miss something but TSO has been widely used in the spec before
> > > > > > this feature:
> > > > > >
> > > > > > """
> > > > > > \item[VIRTIO_NET_F_GUEST_ECN (9)] Driver can receive TSO with ECN.
> > > > > > ...
> > > > > > \item[VIRTIO_NET_F_HOST_TSO4 (11)] Device can receive TSOv4.
> > > > > > ...
> > > > > > """
> > > > >
> > > > >
> > > > > Yes but that does not define a "TSO packet".
> > > >
> > > > How about a patch beforehand to explain thing like:
> > > >
> > > > \item[VIRTIO_NET_F_GUEST_ECN (9)] Driver can receive TSO (TCP
> > > > Segmentation Offload) with ECN.
> > >
> > > That does not define is either?
> > >
> > > virtio$ git grep 'TSO packet'
> > > virtio$
> >
> > Something like this?
> >
> > Maybe "TSO packet is the packet that device may segment it into
> > multiple frames"?
>
>
> The current text is a bit of a mess where "packet" interchangeably means
> the result of segmentation or the request to segment.
> We should fix that but for your patch I think it is reasonable
> to just spell it out at each instance: "packet that requires TCP
> segmentation".

Fine.

Thanks