public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Jason Wang <jasowang@redhat.com>
Cc: virtio-comment@lists.linux.dev, lulu@redhat.com,
	nguyenlienviet@google.com
Subject: Re: [PATCH] virtio-net: introduce TSO limit feature
Date: Wed, 15 Oct 2025 03:27:18 -0400	[thread overview]
Message-ID: <20251015030738-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <CACGkMEs7Y9DLuRp_3ex8ac3Hv5y7X5Q-PnS61DMjOVjAkvazjA@mail.gmail.com>

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.

> >
> > > of a TSO packet
> >
> > what is a TSO packet?
> 
> Packet for device to perform TCP segmentation offload.

pls define terms before use.

> > > that the
> > > +device can process.
> >
> > process in which direction? you mean device can receive?
> 
> It works only for TX (as TSO works only for TX).


rest of spec says "device receives from driver"  for this.
process is ambiguous




> >
> > > When VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is set,
> > > +it specifies the maximum inner TCP length of a UDP tunnel TSO packet
> > > +that the device can process.
> >
> > Rest of spec talks of " GSO over UDP tunnels packets" is this the same?
> 
> Not exactly the same, this is only for TSO not genreal GSO.

rest of spec mostly talks of GSO. in fact virtio tso is a kind of
accelerated gso.  either do the same or add a lot of text
explaining tso as opposed to gso.

> >
> > even if it's actually unused?
> >
> > this, on the assumption that the length for tunnel is smaller?
> 
> It means the device should have the same limitation for plain TSO and
> tunnel TSO.

Hmm. I have doubts how it can work given the overhead.

> >
> > I think this kind of things should be explicit.
> >
> >
> > > +
> > > +The following field, \field{tso_max_segs} only exists if
> > > +VIRTIO_NET_F_HOST_TSO_LIMIT is set.
> > > +It specifies the maximum number of segments that can be produced by
> > > +the device after performing segmentation on TSO packet or a UDP tunnel
> > > +TSO packet (when VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is set).
> >
> > I don't get this field at all. the assumption is that all segments
> > are the same size, right? Then it is just based on length?
> 
> It's the device side limitation, for example a device can produce 100
> segments at most, even if the tso_max_size is 256K, when MTU is 1500,
> the driver can't send a TSO packet whose TCP length is greater than
> (1500 - 20 - 20) * 100 = 146K.

then "can be produced" is again confusing. device
transmits packets but it does not "produce" them as such.
maybe you just mean "supported".

> >
> >
> >
> > > +
> > >  \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> > >
> > >  The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> > > @@ -326,6 +345,17 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > >  The device SHOULD NOT offer VIRTIO_NET_F_CTRL_RX_EXTRA if it
> > >  does not offer VIRTIO_NET_F_CTRL_VQ.
> > >
> > > +If VIRTIO_NET_F_HOST_TSO_LIMIT and VIRTIO_NET_F_MTU have been
> > > +negotiated, the device SHOULD set \field{tso_max_size} so that a TCP
> > > +segment that fully utilizes the configured MTU can be processed by TSO
> > > +(e.g., for IPv4 without options: at least \field{mtu} - 20; for IPv6
> > > +without extension headers: at least \field{mtu} - 40). This
> > > +recommendation does not account for IPv4 options or IPv6 extension
> > > +headers, which reduce the effective segment size.
> > > +
> > > +If VIRTIO_NET_F_HOST_TSO_LIMIT has been negotiated, the device MUST
> > > +set \field{tso_max_segs} to at least 64.
> >
> > where does this 64 come from? pls document.
> 
> A simple backward compatibility which makes sure the value can make
> sure 64K TSO can be segmented with 1500 MTU.

2^16/64 == 1024  

not ~1500

And we don't know MTU is 1500, either.



-- 
MST


  parent reply	other threads:[~2025-10-15  7:27 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-14  4:22 [PATCH] virtio-net: introduce TSO limit feature Jason Wang
2025-10-14  8:59 ` Michael S. Tsirkin
2025-10-15  4:29   ` Jason Wang
2025-10-15  7:01     ` Michael S. Tsirkin
2025-10-16  5:46       ` Jason Wang
2025-10-16  6:17         ` Michael S. Tsirkin
2025-10-16  6:19         ` Michael S. Tsirkin
2025-10-16  6:22           ` Jason Wang
2025-10-16 10:16             ` Michael S. Tsirkin
2025-10-15  7:27     ` Michael S. Tsirkin [this message]
2025-10-16  5:57       ` Jason Wang
2025-10-16  6:17         ` Michael S. Tsirkin
2025-10-16  6:31           ` Jason Wang
2025-10-16 10:02             ` Michael S. Tsirkin
2025-10-20  6:25               ` Jason Wang
2025-10-20  8:19                 ` Michael S. Tsirkin
2025-10-21  3:01                   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251015030738-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=lulu@redhat.com \
    --cc=nguyenlienviet@google.com \
    --cc=virtio-comment@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox