All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-dev] [PATCH v2] virtio_net: support split header
@ 2022-05-07  7:15 Xuan Zhuo
  2022-05-09  8:41 ` [virtio-dev] " Jason Wang
  2022-05-30 18:27 ` Michael S. Tsirkin
  0 siblings, 2 replies; 14+ messages in thread
From: Xuan Zhuo @ 2022-05-07  7:15 UTC (permalink / raw)
  To: virtio-dev; +Cc: jasowang, Michael S. Tsirkin

The purpose of this feature is to split the header and the payload of
the packet.

|                    receive buffer                                    |
|                       0th descriptor             | 1th descriptor    |
| virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |

We can use a buffer plus a separate page when allocating the receive
buffer. In this way, we can ensure that all payloads can be
independently in a page, which is very beneficial for the zerocopy
implemented by the upper layer.

Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
 conformance.tex |  2 ++
 content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index 663e7c3..6f561fb 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
+\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
 \end{itemize}
 
 \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
@@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
+\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
 \end{itemize}
 
 \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
diff --git a/content.tex b/content.tex
index 060bdab..3340402 100644
--- a/content.tex
+++ b/content.tex
@@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
 
+\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
+    the payload.
+
 \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
  (fragmenting the packet) the USO splits large UDP packet
  to several segments when each of these smaller packets has UDP header.
@@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
 \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
+\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
 \end{description}
 
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
@@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
 #define VIRTIO_NET_HDR_F_DATA_VALID    2
 #define VIRTIO_NET_HDR_F_RSC_INFO      4
+#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
         u8 flags;
 #define VIRTIO_NET_HDR_GSO_NONE        0
 #define VIRTIO_NET_HDR_GSO_TCPV4       1
@@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 according to the native endian of the guest rather than
 (necessarily when not using the legacy interface) little-endian.
 
+\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
+
+If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
+the device supports to split the header and the payload.
+The header and payload will be separated into different buffers.
+
+\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
+
+To configure the split header, the following layout structure and definitions
+are used:
+
+\begin{lstlisting}
+struct virtio_net_split_header_config {
+#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
+#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
+#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
+#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
+    le64 type;
+};
+
+#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
+ #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
+\end{lstlisting}
+
+The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
+VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
+
+\field{type} passed as command data is a bitmask, bits set define
+packet types to split header, bits cleared - split header to be disabled.
+
+The header contains the struct virtio_net_hdr and the header of the package.
+Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
+virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
+(include TCP options). The back part is the payload.
+
+\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
+
+Split header MUST be disabled after device initialization.
+
+A device MUST NOT perform split header in the following cases:
+\begin{itemize}
+    \item device does not recognize protocol of the packet.
+    \item \field{type} does not include the protocol of the packet.
+    \item the packet is a IP fragmentation.
+    \item the receive buffer consists of only one descriptor.
+    \item the header exceeds the size of the 0th descriptor.
+    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
+        payload is greater than the total size of the 1th\ldots Nth descriptor.
+\end{itemize}
+
+If the split header completed, then the \field{flags} of virtnet hdr MUST
+contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
+0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
+The device MUST set \field{hdr_len} of virtnet hdr.
+
+If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
+receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
+
+\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
+
+If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
+believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
+to the length of struct virtio_net_hdr plus \field{hdr_len}.
+
+If the split header function is enable, the buffers submitted by the driver
+SHOULD at least be composed of two descriptors. The buffer specified by the 0th
+descriptor SHOULD be able to accommodate the header.
 
 \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
 Types / Network Device / Legacy Interface: Framing Requirements}
-- 
2.31.0


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-07  7:15 [virtio-dev] [PATCH v2] virtio_net: support split header Xuan Zhuo
@ 2022-05-09  8:41 ` Jason Wang
  2022-05-26  6:03   ` Xuan Zhuo
  2022-05-30 18:27 ` Michael S. Tsirkin
  1 sibling, 1 reply; 14+ messages in thread
From: Jason Wang @ 2022-05-09  8:41 UTC (permalink / raw)
  To: Xuan Zhuo, virtio-dev; +Cc: Michael S. Tsirkin


在 2022/5/7 15:15, Xuan Zhuo 写道:
> The purpose of this feature is to split the header and the payload of
> the packet.
>
> |                    receive buffer                                    |
> |                       0th descriptor             | 1th descriptor    |
> | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
>
> We can use a buffer plus a separate page when allocating the receive
> buffer. In this way, we can ensure that all payloads can be
> independently in a page, which is very beneficial for the zerocopy
> implemented by the upper layer.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
>   conformance.tex |  2 ++
>   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 74 insertions(+)
>
> diff --git a/conformance.tex b/conformance.tex
> index 663e7c3..6f561fb 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
>   \end{itemize}
>   
>   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
>   \end{itemize}
>   
>   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> diff --git a/content.tex b/content.tex
> index 060bdab..3340402 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>       channel.
>   
> +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> +    the payload.
> +
>   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
>    (fragmenting the packet) the USO splits large UDP packet
>    to several segments when each of these smaller packets has UDP header.
> @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
>   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
>   \end{description}
>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
>   #define VIRTIO_NET_HDR_F_DATA_VALID    2
>   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
>           u8 flags;
>   #define VIRTIO_NET_HDR_GSO_NONE        0
>   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>   according to the native endian of the guest rather than
>   (necessarily when not using the legacy interface) little-endian.
>   
> +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> +the device supports to split the header and the payload.
> +The header and payload will be separated into different buffers.


I think you meant "descriptors" instead of "buffers".


> +
> +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> +
> +To configure the split header, the following layout structure and definitions
> +are used:
> +
> +\begin{lstlisting}
> +struct virtio_net_split_header_config {
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1


I think it's better to capitalize: TCPV4 or TCP4.


> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> +    le64 type;
> +};
> +
> +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> +\end{lstlisting}
> +
> +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> +
> +\field{type} passed as command data is a bitmask, bits set define
> +packet types to split header, bits cleared - split header to be disabled.


(Not a native speaker, try my best to help)

This sentence might need some tweaks:

The driver can enable or disable the split by setting or clearing 
corresponding bits in \field{type}.


> +
> +The header contains the struct virtio_net_hdr and the header of the package.
> +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> +(include TCP options). The back part is the payload.


I think it's better to either explain each type one by one instead of 
only describing the TCPv4. Or we can simply say the head contains all 
headers before level 4 payload?


> +
> +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +Split header MUST be disabled after device initialization.


I think you actually mean "upon device reset"?


> +
> +A device MUST NOT perform split header in the following cases:
> +\begin{itemize}
> +    \item device does not recognize protocol of the packet.
> +    \item \field{type} does not include the protocol of the packet.
> +    \item the packet is a IP fragmentation.


What's the reason for this limit?


> +    \item the receive buffer consists of only one descriptor.


descriptor actually.


> +    \item the header exceeds the size of the 0th descriptor.


It looks to me the spec count from the first.


> +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> +        payload is greater than the total size of the 1th\ldots Nth descriptor.


Maybe something like the following is better:

The size of the payload exceeds the length of the descriptor or buffer 
chain starting from the 2nd descriptor.


> +\end{itemize}
> +
> +If the split header completed,


If the header is split by the device?


>   then the \field{flags} of virtnet hdr MUST


We can drop "then" here and it's better to use structure virtio_net_hdr


> +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> +0th descriptor,


first and second should be used here.


>   and the payload MUST starts from the buffer of the 1th descriptor.
> +The device MUST set \field{hdr_len} of virtnet hdr.


To what value?


> +
> +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.


I don't see why we need this.


> +
> +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> +to the length of struct virtio_net_hdr plus \field{hdr_len}.


See above we can reuse the device normative an drop the second part 
starting from " the length of the header ..."


> +
> +If the split header function is enable, the buffers submitted by the driver
> +SHOULD at least be composed of two descriptors.


Do we need to mention it's only used for RX? (I think we don't need this 
for TX).


> The buffer specified by the 0th
> +descriptor SHOULD be able to accommodate the header.


We probably need to clarify that the header here means both the virtio 
net header and the protocol header.

Thanks


>   
>   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>   Types / Network Device / Legacy Interface: Framing Requirements}


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-09  8:41 ` [virtio-dev] " Jason Wang
@ 2022-05-26  6:03   ` Xuan Zhuo
  2022-05-30  5:56     ` Jason Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Xuan Zhuo @ 2022-05-26  6:03 UTC (permalink / raw)
  To: Jason Wang; +Cc: Michael S. Tsirkin, virtio-dev

On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
>
> 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > The purpose of this feature is to split the header and the payload of
> > the packet.
> >
> > |                    receive buffer                                    |
> > |                       0th descriptor             | 1th descriptor    |
> > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> >
> > We can use a buffer plus a separate page when allocating the receive
> > buffer. In this way, we can ensure that all payloads can be
> > independently in a page, which is very beneficial for the zerocopy
> > implemented by the upper layer.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > ---
> >   conformance.tex |  2 ++
> >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 74 insertions(+)
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index 663e7c3..6f561fb 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> >   \end{itemize}
> >
> >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> >   \end{itemize}
> >
> >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > diff --git a/content.tex b/content.tex
> > index 060bdab..3340402 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> >       channel.
> >
> > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > +    the payload.
> > +
> >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> >    (fragmenting the packet) the USO splits large UDP packet
> >    to several segments when each of these smaller packets has UDP header.
> > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> >   \end{description}
> >
> >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> >           u8 flags;
> >   #define VIRTIO_NET_HDR_GSO_NONE        0
> >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >   according to the native endian of the guest rather than
> >   (necessarily when not using the legacy interface) little-endian.
> >
> > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > +the device supports to split the header and the payload.
> > +The header and payload will be separated into different buffers.
>
>
> I think you meant "descriptors" instead of "buffers".

Will fix.

>
>
> > +
> > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > +
> > +To configure the split header, the following layout structure and definitions
> > +are used:
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_split_header_config {
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
>
>
> I think it's better to capitalize: TCPV4 or TCP4.

OK.

>
>
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > +    le64 type;
> > +};
> > +
> > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > +\end{lstlisting}
> > +
> > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > +
> > +\field{type} passed as command data is a bitmask, bits set define
> > +packet types to split header, bits cleared - split header to be disabled.
>
>
> (Not a native speaker, try my best to help)
>
> This sentence might need some tweaks:
>
> The driver can enable or disable the split by setting or clearing
> corresponding bits in \field{type}.

Thanks.

>
>
> > +
> > +The header contains the struct virtio_net_hdr and the header of the package.
> > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > +(include TCP options). The back part is the payload.
>
>
> I think it's better to either explain each type one by one instead of
> only describing the TCPv4. Or we can simply say the head contains all
> headers before level 4 payload?

OK. Thanks.


>
>
> > +
> > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +Split header MUST be disabled after device initialization.
>
>
> I think you actually mean "upon device reset"?

Yes.


>
>
> > +
> > +A device MUST NOT perform split header in the following cases:
> > +\begin{itemize}
> > +    \item device does not recognize protocol of the packet.
> > +    \item \field{type} does not include the protocol of the packet.
> > +    \item the packet is a IP fragmentation.
>
>
> What's the reason for this limit?

Rethinking this problem, some implementation difficulties that I originally
thought can be solved. So I think this limitation can be removed.

>
>
> > +    \item the receive buffer consists of only one descriptor.
>
>
> descriptor actually.

Will fix.

>
>
> > +    \item the header exceeds the size of the 0th descriptor.
>
>
> It looks to me the spec count from the first.

Will fix.

>
>
> > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
>
>
> Maybe something like the following is better:
>
> The size of the payload exceeds the length of the descriptor or buffer
> chain starting from the 2nd descriptor.

OK.

>
>
> > +\end{itemize}
> > +
> > +If the split header completed,
>
>
> If the header is split by the device?

Yes.

>
>
> >   then the \field{flags} of virtnet hdr MUST
>
>
> We can drop "then" here and it's better to use structure virtio_net_hdr

Will fix.

>
>
> > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > +0th descriptor,
>
>
> first and second should be used here.

OK

>
>
> >   and the payload MUST starts from the buffer of the 1th descriptor.
> > +The device MUST set \field{hdr_len} of virtnet hdr.
>
>
> To what value?

I'll make it clear in the next version.

>
>
> > +
> > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
>
>
> I don't see why we need this.

If used to implement zerocopy, the second desc points to a page-aligned buffer.
The first desc points to a small buffer for saving the header. The first buffer
and subsequent buffers are generally discontinuous. So giving up the first
buffer and directly guaranteeing the data to the page-aligned buffer can achieve
better performance. And the driver can reuse the first buffer.

>
>
> > +
> > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
>
>
> See above we can reuse the device normative an drop the second part
> starting from " the length of the header ..."

OK

>
>
> > +
> > +If the split header function is enable, the buffers submitted by the driver
> > +SHOULD at least be composed of two descriptors.
>
>
> Do we need to mention it's only used for RX? (I think we don't need this
> for TX).

Yes.

>
>
> > The buffer specified by the 0th
> > +descriptor SHOULD be able to accommodate the header.
>
>
> We probably need to clarify that the header here means both the virtio
> net header and the protocol header.

Will fix.

Thanks.

>
> Thanks
>
>
> >
> >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> >   Types / Network Device / Legacy Interface: Framing Requirements}
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-26  6:03   ` Xuan Zhuo
@ 2022-05-30  5:56     ` Jason Wang
  2022-05-30 18:22       ` Michael S. Tsirkin
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2022-05-30  5:56 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: Michael S. Tsirkin, Virtio-Dev

On Thu, May 26, 2022 at 2:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
>
> On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> >
> > 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > > The purpose of this feature is to split the header and the payload of
> > > the packet.
> > >
> > > |                    receive buffer                                    |
> > > |                       0th descriptor             | 1th descriptor    |
> > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > >
> > > We can use a buffer plus a separate page when allocating the receive
> > > buffer. In this way, we can ensure that all payloads can be
> > > independently in a page, which is very beneficial for the zerocopy
> > > implemented by the upper layer.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > >   conformance.tex |  2 ++
> > >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >   2 files changed, 74 insertions(+)
> > >
> > > diff --git a/conformance.tex b/conformance.tex
> > > index 663e7c3..6f561fb 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >   \end{itemize}
> > >
> > >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >   \end{itemize}
> > >
> > >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > diff --git a/content.tex b/content.tex
> > > index 060bdab..3340402 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > >       channel.
> > >
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > +    the payload.
> > > +
> > >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > >    (fragmenting the packet) the USO splits large UDP packet
> > >    to several segments when each of these smaller packets has UDP header.
> > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > >   \end{description}
> > >
> > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > >           u8 flags;
> > >   #define VIRTIO_NET_HDR_GSO_NONE        0
> > >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >   according to the native endian of the guest rather than
> > >   (necessarily when not using the legacy interface) little-endian.
> > >
> > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > +the device supports to split the header and the payload.
> > > +The header and payload will be separated into different buffers.
> >
> >
> > I think you meant "descriptors" instead of "buffers".
>
> Will fix.
>
> >
> >
> > > +
> > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > +
> > > +To configure the split header, the following layout structure and definitions
> > > +are used:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_net_split_header_config {
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> >
> >
> > I think it's better to capitalize: TCPV4 or TCP4.
>
> OK.
>
> >
> >
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > +    le64 type;
> > > +};
> > > +
> > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > +\end{lstlisting}
> > > +
> > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > +
> > > +\field{type} passed as command data is a bitmask, bits set define
> > > +packet types to split header, bits cleared - split header to be disabled.
> >
> >
> > (Not a native speaker, try my best to help)
> >
> > This sentence might need some tweaks:
> >
> > The driver can enable or disable the split by setting or clearing
> > corresponding bits in \field{type}.
>
> Thanks.
>
> >
> >
> > > +
> > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > +(include TCP options). The back part is the payload.
> >
> >
> > I think it's better to either explain each type one by one instead of
> > only describing the TCPv4. Or we can simply say the head contains all
> > headers before level 4 payload?
>
> OK. Thanks.
>
>
> >
> >
> > > +
> > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +Split header MUST be disabled after device initialization.
> >
> >
> > I think you actually mean "upon device reset"?
>
> Yes.
>
>
> >
> >
> > > +
> > > +A device MUST NOT perform split header in the following cases:
> > > +\begin{itemize}
> > > +    \item device does not recognize protocol of the packet.
> > > +    \item \field{type} does not include the protocol of the packet.
> > > +    \item the packet is a IP fragmentation.
> >
> >
> > What's the reason for this limit?
>
> Rethinking this problem, some implementation difficulties that I originally
> thought can be solved. So I think this limitation can be removed.
>
> >
> >
> > > +    \item the receive buffer consists of only one descriptor.
> >
> >
> > descriptor actually.
>
> Will fix.
>
> >
> >
> > > +    \item the header exceeds the size of the 0th descriptor.
> >
> >
> > It looks to me the spec count from the first.
>
> Will fix.
>
> >
> >
> > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> >
> >
> > Maybe something like the following is better:
> >
> > The size of the payload exceeds the length of the descriptor or buffer
> > chain starting from the 2nd descriptor.
>
> OK.
>
> >
> >
> > > +\end{itemize}
> > > +
> > > +If the split header completed,
> >
> >
> > If the header is split by the device?
>
> Yes.
>
> >
> >
> > >   then the \field{flags} of virtnet hdr MUST
> >
> >
> > We can drop "then" here and it's better to use structure virtio_net_hdr
>
> Will fix.
>
> >
> >
> > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > +0th descriptor,
> >
> >
> > first and second should be used here.
>
> OK
>
> >
> >
> > >   and the payload MUST starts from the buffer of the 1th descriptor.
> > > +The device MUST set \field{hdr_len} of virtnet hdr.
> >
> >
> > To what value?
>
> I'll make it clear in the next version.
>
> >
> >
> > > +
> > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> >
> >
> > I don't see why we need this.
>
> If used to implement zerocopy, the second desc points to a page-aligned buffer.
> The first desc points to a small buffer for saving the header. The first buffer
> and subsequent buffers are generally discontinuous. So giving up the first
> buffer and directly guaranteeing the data to the page-aligned buffer can achieve
> better performance. And the driver can reuse the first buffer.

I see, it might be better if we can explain this here.

Thanks

>
> >
> >
> > > +
> > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> >
> >
> > See above we can reuse the device normative an drop the second part
> > starting from " the length of the header ..."
>
> OK
>
> >
> >
> > > +
> > > +If the split header function is enable, the buffers submitted by the driver
> > > +SHOULD at least be composed of two descriptors.
> >
> >
> > Do we need to mention it's only used for RX? (I think we don't need this
> > for TX).
>
> Yes.
>
> >
> >
> > > The buffer specified by the 0th
> > > +descriptor SHOULD be able to accommodate the header.
> >
> >
> > We probably need to clarify that the header here means both the virtio
> > net header and the protocol header.
>
> Will fix.
>
> Thanks.
>
> >
> > Thanks
> >
> >
> > >
> > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >   Types / Network Device / Legacy Interface: Framing Requirements}
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> >
>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-30  5:56     ` Jason Wang
@ 2022-05-30 18:22       ` Michael S. Tsirkin
  2022-05-31  4:38         ` Jason Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-05-30 18:22 UTC (permalink / raw)
  To: Jason Wang; +Cc: Xuan Zhuo, Virtio-Dev

On Mon, May 30, 2022 at 01:56:06PM +0800, Jason Wang wrote:
> On Thu, May 26, 2022 at 2:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> >
> > On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > > > The purpose of this feature is to split the header and the payload of
> > > > the packet.
> > > >
> > > > |                    receive buffer                                    |
> > > > |                       0th descriptor             | 1th descriptor    |
> > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > >
> > > > We can use a buffer plus a separate page when allocating the receive
> > > > buffer. In this way, we can ensure that all payloads can be
> > > > independently in a page, which is very beneficial for the zerocopy
> > > > implemented by the upper layer.
> > > >
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > ---
> > > >   conformance.tex |  2 ++
> > > >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >   2 files changed, 74 insertions(+)
> > > >
> > > > diff --git a/conformance.tex b/conformance.tex
> > > > index 663e7c3..6f561fb 100644
> > > > --- a/conformance.tex
> > > > +++ b/conformance.tex
> > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > >   \end{itemize}
> > > >
> > > >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > >   \end{itemize}
> > > >
> > > >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > diff --git a/content.tex b/content.tex
> > > > index 060bdab..3340402 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > >       channel.
> > > >
> > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > +    the payload.
> > > > +
> > > >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > >    (fragmenting the packet) the USO splits large UDP packet
> > > >    to several segments when each of these smaller packets has UDP header.
> > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > >   \end{description}
> > > >
> > > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > >           u8 flags;
> > > >   #define VIRTIO_NET_HDR_GSO_NONE        0
> > > >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > >   according to the native endian of the guest rather than
> > > >   (necessarily when not using the legacy interface) little-endian.
> > > >
> > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > +the device supports to split the header and the payload.
> > > > +The header and payload will be separated into different buffers.
> > >
> > >
> > > I think you meant "descriptors" instead of "buffers".
> >
> > Will fix.

No, actually, I think this is exactly buffers.
device is always in terms of buffers, descriptors is
a means to implement buffers.

And the feature should
depend on mergeable buffers. Without mergeable buffers you do not
need these tricks, just add two descriptors and be done
with it.

Right?

> >
> > >
> > >
> > > > +
> > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > +
> > > > +To configure the split header, the following layout structure and definitions
> > > > +are used:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_net_split_header_config {
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > >
> > >
> > > I think it's better to capitalize: TCPV4 or TCP4.
> >
> > OK.
> >
> > >
> > >
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > +    le64 type;
> > > > +};
> > > > +
> > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > +\end{lstlisting}
> > > > +
> > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > +
> > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > +packet types to split header, bits cleared - split header to be disabled.
> > >
> > >
> > > (Not a native speaker, try my best to help)
> > >
> > > This sentence might need some tweaks:
> > >
> > > The driver can enable or disable the split by setting or clearing
> > > corresponding bits in \field{type}.
> >
> > Thanks.
> >
> > >
> > >
> > > > +
> > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > +(include TCP options). The back part is the payload.
> > >
> > >
> > > I think it's better to either explain each type one by one instead of
> > > only describing the TCPv4. Or we can simply say the head contains all
> > > headers before level 4 payload?
> >
> > OK. Thanks.
>

So this always splits out at protocol level?
What if device wants to align e.g. IP protocol header?
Ethernet?


Also, what about aligning at an offset?
E.g. NET_IP_ALIGN?


> >
> > >
> > >
> > > > +
> > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +Split header MUST be disabled after device initialization.
> > >
> > >
> > > I think you actually mean "upon device reset"?
> >
> > Yes.
> >
> >
> > >
> > >
> > > > +
> > > > +A device MUST NOT perform split header in the following cases:
> > > > +\begin{itemize}
> > > > +    \item device does not recognize protocol of the packet.
> > > > +    \item \field{type} does not include the protocol of the packet.
> > > > +    \item the packet is a IP fragmentation.
> > >
> > >
> > > What's the reason for this limit?
> >
> > Rethinking this problem, some implementation difficulties that I originally
> > thought can be solved. So I think this limitation can be removed.
> >
> > >
> > >
> > > > +    \item the receive buffer consists of only one descriptor.
> > >
> > >
> > > descriptor actually.
> >
> > Will fix.
> >
> > >
> > >
> > > > +    \item the header exceeds the size of the 0th descriptor.
> > >
> > >
> > > It looks to me the spec count from the first.
> >
> > Will fix.
> >
> > >
> > >
> > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > >
> > >
> > > Maybe something like the following is better:
> > >
> > > The size of the payload exceeds the length of the descriptor or buffer
> > > chain starting from the 2nd descriptor.
> >
> > OK.
> >
> > >
> > >
> > > > +\end{itemize}
> > > > +
> > > > +If the split header completed,
> > >
> > >
> > > If the header is split by the device?
> >
> > Yes.
> >
> > >
> > >
> > > >   then the \field{flags} of virtnet hdr MUST
> > >
> > >
> > > We can drop "then" here and it's better to use structure virtio_net_hdr
> >
> > Will fix.
> >
> > >
> > >
> > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > +0th descriptor,
> > >
> > >
> > > first and second should be used here.
> >
> > OK
> >
> > >
> > >
> > > >   and the payload MUST starts from the buffer of the 1th descriptor.
> > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > >
> > >
> > > To what value?
> >
> > I'll make it clear in the next version.
> >
> > >
> > >
> > > > +
> > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > >
> > >
> > > I don't see why we need this.
> >
> > If used to implement zerocopy, the second desc points to a page-aligned buffer.
> > The first desc points to a small buffer for saving the header. The first buffer
> > and subsequent buffers are generally discontinuous. So giving up the first
> > buffer and directly guaranteeing the data to the page-aligned buffer can achieve
> > better performance. And the driver can reuse the first buffer.
> 
> I see, it might be better if we can explain this here.
> 
> Thanks
> 
> >
> > >
> > >
> > > > +
> > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > >
> > >
> > > See above we can reuse the device normative an drop the second part
> > > starting from " the length of the header ..."
> >
> > OK
> >
> > >
> > >
> > > > +
> > > > +If the split header function is enable, the buffers submitted by the driver
> > > > +SHOULD at least be composed of two descriptors.
> > >
> > >
> > > Do we need to mention it's only used for RX? (I think we don't need this
> > > for TX).
> >
> > Yes.
> >
> > >
> > >
> > > > The buffer specified by the 0th
> > > > +descriptor SHOULD be able to accommodate the header.
> > >
> > >
> > > We probably need to clarify that the header here means both the virtio
> > > net header and the protocol header.
> >
> > Will fix.
> >
> > Thanks.
> >
> > >
> > > Thanks
> > >
> > >
> > > >
> > > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > >   Types / Network Device / Legacy Interface: Framing Requirements}
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > >
> >


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-07  7:15 [virtio-dev] [PATCH v2] virtio_net: support split header Xuan Zhuo
  2022-05-09  8:41 ` [virtio-dev] " Jason Wang
@ 2022-05-30 18:27 ` Michael S. Tsirkin
  2022-05-31  2:10   ` Xuan Zhuo
  1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-05-30 18:27 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: virtio-dev, jasowang

On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> The purpose of this feature is to split the header and the payload of
> the packet.
> 
> |                    receive buffer                                    |
> |                       0th descriptor             | 1th descriptor    |
> | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> 
> We can use a buffer plus a separate page when allocating the receive
> buffer. In this way, we can ensure that all payloads can be
> independently in a page, which is very beneficial for the zerocopy
> implemented by the upper layer.
> 
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

Okay, but I don't think this covers all possible use-cases.
If we are doing this let's try to address alignment requirements too.

> ---
>  conformance.tex |  2 ++
>  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/conformance.tex b/conformance.tex
> index 663e7c3..6f561fb 100644
> --- a/conformance.tex
> +++ b/conformance.tex
> @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
>  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
>  \end{itemize}
>  
>  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
>  \end{itemize}
>  
>  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> diff --git a/content.tex b/content.tex
> index 060bdab..3340402 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
>  
> +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> +    the payload.
> +
>  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
>   (fragmenting the packet) the USO splits large UDP packet
>   to several segments when each of these smaller packets has UDP header.
> @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
>  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
>  \end{description}
>  
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
>  #define VIRTIO_NET_HDR_F_DATA_VALID    2
>  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
>          u8 flags;
>  #define VIRTIO_NET_HDR_GSO_NONE        0
>  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  according to the native endian of the guest rather than
>  (necessarily when not using the legacy interface) little-endian.
>  
> +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> +the device supports to split the header and the payload.
> +The header and payload will be separated into different buffers.
> +
> +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> +
> +To configure the split header, the following layout structure and definitions
> +are used:
> +
> +\begin{lstlisting}
> +struct virtio_net_split_header_config {
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> +    le64 type;
> +};
> +


how do we know what types does device support?

> +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> +\end{lstlisting}
> +
> +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> +
> +\field{type} passed as command data is a bitmask, bits set define
> +packet types to split header, bits cleared - split header to be disabled.
> +
> +The header contains the struct virtio_net_hdr and the header of the package.
> +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> +(include TCP options). The back part is the payload.
> +
> +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +Split header MUST be disabled after device initialization.
> +
> +A device MUST NOT perform split header in the following cases:
> +\begin{itemize}
> +    \item device does not recognize protocol of the packet.
> +    \item \field{type} does not include the protocol of the packet.
> +    \item the packet is a IP fragmentation.
> +    \item the receive buffer consists of only one descriptor.
> +    \item the header exceeds the size of the 0th descriptor.
> +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> +\end{itemize}
> +
> +If the split header completed, then the \field{flags} of virtnet hdr MUST
> +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> +The device MUST set \field{hdr_len} of virtnet hdr.
> +
> +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> +
> +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> +
> +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> +
> +If the split header function is enable, the buffers submitted by the driver
> +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> +descriptor SHOULD be able to accommodate the header.
>

I am not sure I understand what all this is saying. Lots of this is
ungrammatical. Also - I do not understand what do you mean believe,
or 0th descriptor.  Also pls formulate in terms of buffers not descriptors. 

Thanks!
  
>  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
>  Types / Network Device / Legacy Interface: Framing Requirements}
> -- 
> 2.31.0


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-30 18:27 ` Michael S. Tsirkin
@ 2022-05-31  2:10   ` Xuan Zhuo
  2022-05-31  5:24     ` Michael S. Tsirkin
  2022-05-31  5:35     ` Michael S. Tsirkin
  0 siblings, 2 replies; 14+ messages in thread
From: Xuan Zhuo @ 2022-05-31  2:10 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, jasowang

On Mon, 30 May 2022 14:27:26 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> > The purpose of this feature is to split the header and the payload of
> > the packet.
> >
> > |                    receive buffer                                    |
> > |                       0th descriptor             | 1th descriptor    |
> > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> >
> > We can use a buffer plus a separate page when allocating the receive
> > buffer. In this way, we can ensure that all payloads can be
> > independently in a page, which is very beneficial for the zerocopy
> > implemented by the upper layer.
> >
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>
> Okay, but I don't think this covers all possible use-cases.
> If we are doing this let's try to address alignment requirements too.

Very happy to do this job.

I thinks "offset" can contain these requirements. No matter how many descriptors
are used, we treat multiple buffers as a continuous piece of memory. The device
places ip, tcp/udp at the specified location according to the specified offset.

The device does not care whether the data is finally aligned with the buffer
specified by a descriptor, the device only needs to ensure that each layer of
the protocol is placed on the specified "offset".

Many other places need to be modified, and I will publish a new spec.

Thanks.

>
> > ---
> >  conformance.tex |  2 ++
> >  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 74 insertions(+)
> >
> > diff --git a/conformance.tex b/conformance.tex
> > index 663e7c3..6f561fb 100644
> > --- a/conformance.tex
> > +++ b/conformance.tex
> > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> >  \end{itemize}
> >
> >  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > diff --git a/content.tex b/content.tex
> > index 060bdab..3340402 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> >      channel.
> >
> > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > +    the payload.
> > +
> >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> >   (fragmenting the packet) the USO splits large UDP packet
> >   to several segments when each of these smaller packets has UDP header.
> > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> >  \end{description}
> >
> >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> >          u8 flags;
> >  #define VIRTIO_NET_HDR_GSO_NONE        0
> >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >  according to the native endian of the guest rather than
> >  (necessarily when not using the legacy interface) little-endian.
> >
> > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > +the device supports to split the header and the payload.
> > +The header and payload will be separated into different buffers.
> > +
> > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > +
> > +To configure the split header, the following layout structure and definitions
> > +are used:
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_split_header_config {
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > +    le64 type;
> > +};
> > +
>
>
> how do we know what types does device support?
>
> > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > +\end{lstlisting}
> > +
> > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > +
> > +\field{type} passed as command data is a bitmask, bits set define
> > +packet types to split header, bits cleared - split header to be disabled.
> > +
> > +The header contains the struct virtio_net_hdr and the header of the package.
> > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > +(include TCP options). The back part is the payload.
> > +
> > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +Split header MUST be disabled after device initialization.
> > +
> > +A device MUST NOT perform split header in the following cases:
> > +\begin{itemize}
> > +    \item device does not recognize protocol of the packet.
> > +    \item \field{type} does not include the protocol of the packet.
> > +    \item the packet is a IP fragmentation.
> > +    \item the receive buffer consists of only one descriptor.
> > +    \item the header exceeds the size of the 0th descriptor.
> > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > +\end{itemize}
> > +
> > +If the split header completed, then the \field{flags} of virtnet hdr MUST
> > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> > +The device MUST set \field{hdr_len} of virtnet hdr.
> > +
> > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > +
> > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > +
> > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > +
> > +If the split header function is enable, the buffers submitted by the driver
> > +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> > +descriptor SHOULD be able to accommodate the header.
> >
>
> I am not sure I understand what all this is saying. Lots of this is
> ungrammatical. Also - I do not understand what do you mean believe,
> or 0th descriptor.  Also pls formulate in terms of buffers not descriptors.
>
> Thanks!
>
> >  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> >  Types / Network Device / Legacy Interface: Framing Requirements}
> > --
> > 2.31.0
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-30 18:22       ` Michael S. Tsirkin
@ 2022-05-31  4:38         ` Jason Wang
  2022-05-31  5:31           ` Michael S. Tsirkin
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Wang @ 2022-05-31  4:38 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Xuan Zhuo, Virtio-Dev

On Tue, May 31, 2022 at 2:23 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Mon, May 30, 2022 at 01:56:06PM +0800, Jason Wang wrote:
> > On Thu, May 26, 2022 at 2:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > >
> > > On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > >
> > > > 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > > > > The purpose of this feature is to split the header and the payload of
> > > > > the packet.
> > > > >
> > > > > |                    receive buffer                                    |
> > > > > |                       0th descriptor             | 1th descriptor    |
> > > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > > >
> > > > > We can use a buffer plus a separate page when allocating the receive
> > > > > buffer. In this way, we can ensure that all payloads can be
> > > > > independently in a page, which is very beneficial for the zerocopy
> > > > > implemented by the upper layer.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > ---
> > > > >   conformance.tex |  2 ++
> > > > >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >   2 files changed, 74 insertions(+)
> > > > >
> > > > > diff --git a/conformance.tex b/conformance.tex
> > > > > index 663e7c3..6f561fb 100644
> > > > > --- a/conformance.tex
> > > > > +++ b/conformance.tex
> > > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > >   \end{itemize}
> > > > >
> > > > >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > >   \end{itemize}
> > > > >
> > > > >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > > diff --git a/content.tex b/content.tex
> > > > > index 060bdab..3340402 100644
> > > > > --- a/content.tex
> > > > > +++ b/content.tex
> > > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > > >       channel.
> > > > >
> > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > > +    the payload.
> > > > > +
> > > > >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > > >    (fragmenting the packet) the USO splits large UDP packet
> > > > >    to several segments when each of these smaller packets has UDP header.
> > > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > >   \end{description}
> > > > >
> > > > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > > >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > > >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > > >           u8 flags;
> > > > >   #define VIRTIO_NET_HDR_GSO_NONE        0
> > > > >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >   according to the native endian of the guest rather than
> > > > >   (necessarily when not using the legacy interface) little-endian.
> > > > >
> > > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > > +the device supports to split the header and the payload.
> > > > > +The header and payload will be separated into different buffers.
> > > >
> > > >
> > > > I think you meant "descriptors" instead of "buffers".
> > >
> > > Will fix.
>
> No, actually, I think this is exactly buffers.
> device is always in terms of buffers, descriptors is
> a means to implement buffers.
>
> And the feature should
> depend on mergeable buffers. Without mergeable buffers you do not
> need these tricks, just add two descriptors and be done
> with it.

How do we know the length of a header in advance to allocate the exact
descriptor properly in this case?

Thanks

>
> Right?
>
> > >
> > > >
> > > >
> > > > > +
> > > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > > +
> > > > > +To configure the split header, the following layout structure and definitions
> > > > > +are used:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_net_split_header_config {
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > >
> > > >
> > > > I think it's better to capitalize: TCPV4 or TCP4.
> > >
> > > OK.
> > >
> > > >
> > > >
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > > +    le64 type;
> > > > > +};
> > > > > +
> > > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > > +\end{lstlisting}
> > > > > +
> > > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > > +
> > > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > > +packet types to split header, bits cleared - split header to be disabled.
> > > >
> > > >
> > > > (Not a native speaker, try my best to help)
> > > >
> > > > This sentence might need some tweaks:
> > > >
> > > > The driver can enable or disable the split by setting or clearing
> > > > corresponding bits in \field{type}.
> > >
> > > Thanks.
> > >
> > > >
> > > >
> > > > > +
> > > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > > +(include TCP options). The back part is the payload.
> > > >
> > > >
> > > > I think it's better to either explain each type one by one instead of
> > > > only describing the TCPv4. Or we can simply say the head contains all
> > > > headers before level 4 payload?
> > >
> > > OK. Thanks.
> >
>
> So this always splits out at protocol level?
> What if device wants to align e.g. IP protocol header?
> Ethernet?
>
>
> Also, what about aligning at an offset?
> E.g. NET_IP_ALIGN?
>
>
> > >
> > > >
> > > >
> > > > > +
> > > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +Split header MUST be disabled after device initialization.
> > > >
> > > >
> > > > I think you actually mean "upon device reset"?
> > >
> > > Yes.
> > >
> > >
> > > >
> > > >
> > > > > +
> > > > > +A device MUST NOT perform split header in the following cases:
> > > > > +\begin{itemize}
> > > > > +    \item device does not recognize protocol of the packet.
> > > > > +    \item \field{type} does not include the protocol of the packet.
> > > > > +    \item the packet is a IP fragmentation.
> > > >
> > > >
> > > > What's the reason for this limit?
> > >
> > > Rethinking this problem, some implementation difficulties that I originally
> > > thought can be solved. So I think this limitation can be removed.
> > >
> > > >
> > > >
> > > > > +    \item the receive buffer consists of only one descriptor.
> > > >
> > > >
> > > > descriptor actually.
> > >
> > > Will fix.
> > >
> > > >
> > > >
> > > > > +    \item the header exceeds the size of the 0th descriptor.
> > > >
> > > >
> > > > It looks to me the spec count from the first.
> > >
> > > Will fix.
> > >
> > > >
> > > >
> > > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > >
> > > >
> > > > Maybe something like the following is better:
> > > >
> > > > The size of the payload exceeds the length of the descriptor or buffer
> > > > chain starting from the 2nd descriptor.
> > >
> > > OK.
> > >
> > > >
> > > >
> > > > > +\end{itemize}
> > > > > +
> > > > > +If the split header completed,
> > > >
> > > >
> > > > If the header is split by the device?
> > >
> > > Yes.
> > >
> > > >
> > > >
> > > > >   then the \field{flags} of virtnet hdr MUST
> > > >
> > > >
> > > > We can drop "then" here and it's better to use structure virtio_net_hdr
> > >
> > > Will fix.
> > >
> > > >
> > > >
> > > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > > +0th descriptor,
> > > >
> > > >
> > > > first and second should be used here.
> > >
> > > OK
> > >
> > > >
> > > >
> > > > >   and the payload MUST starts from the buffer of the 1th descriptor.
> > > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > >
> > > >
> > > > To what value?
> > >
> > > I'll make it clear in the next version.
> > >
> > > >
> > > >
> > > > > +
> > > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > >
> > > >
> > > > I don't see why we need this.
> > >
> > > If used to implement zerocopy, the second desc points to a page-aligned buffer.
> > > The first desc points to a small buffer for saving the header. The first buffer
> > > and subsequent buffers are generally discontinuous. So giving up the first
> > > buffer and directly guaranteeing the data to the page-aligned buffer can achieve
> > > better performance. And the driver can reuse the first buffer.
> >
> > I see, it might be better if we can explain this here.
> >
> > Thanks
> >
> > >
> > > >
> > > >
> > > > > +
> > > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > >
> > > >
> > > > See above we can reuse the device normative an drop the second part
> > > > starting from " the length of the header ..."
> > >
> > > OK
> > >
> > > >
> > > >
> > > > > +
> > > > > +If the split header function is enable, the buffers submitted by the driver
> > > > > +SHOULD at least be composed of two descriptors.
> > > >
> > > >
> > > > Do we need to mention it's only used for RX? (I think we don't need this
> > > > for TX).
> > >
> > > Yes.
> > >
> > > >
> > > >
> > > > > The buffer specified by the 0th
> > > > > +descriptor SHOULD be able to accommodate the header.
> > > >
> > > >
> > > > We probably need to clarify that the header here means both the virtio
> > > > net header and the protocol header.
> > >
> > > Will fix.
> > >
> > > Thanks.
> > >
> > > >
> > > > Thanks
> > > >
> > > >
> > > > >
> > > > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > > >   Types / Network Device / Legacy Interface: Framing Requirements}
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > >
> > >
>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  2:10   ` Xuan Zhuo
@ 2022-05-31  5:24     ` Michael S. Tsirkin
  2022-05-31  5:35     ` Michael S. Tsirkin
  1 sibling, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-05-31  5:24 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: virtio-dev, jasowang

On Tue, May 31, 2022 at 10:10:52AM +0800, Xuan Zhuo wrote:
> On Mon, 30 May 2022 14:27:26 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> > > The purpose of this feature is to split the header and the payload of
> > > the packet.
> > >
> > > |                    receive buffer                                    |
> > > |                       0th descriptor             | 1th descriptor    |
> > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > >
> > > We can use a buffer plus a separate page when allocating the receive
> > > buffer. In this way, we can ensure that all payloads can be
> > > independently in a page, which is very beneficial for the zerocopy
> > > implemented by the upper layer.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >
> > Okay, but I don't think this covers all possible use-cases.
> > If we are doing this let's try to address alignment requirements too.
> 
> Very happy to do this job.
> 
> I thinks "offset" can contain these requirements. No matter how many descriptors
> are used, we treat multiple buffers as a continuous piece of memory. The device
> places ip, tcp/udp at the specified location according to the specified offset.
> 
> The device does not care whether the data is finally aligned with the buffer
> specified by a descriptor, the device only needs to ensure that each layer of
> the protocol is placed on the specified "offset".
> 
> Many other places need to be modified, and I will publish a new spec.
> 
> Thanks.
> 
> >
> > > ---
> > >  conformance.tex |  2 ++
> > >  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 74 insertions(+)
> > >
> > > diff --git a/conformance.tex b/conformance.tex
> > > index 663e7c3..6f561fb 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > diff --git a/content.tex b/content.tex
> > > index 060bdab..3340402 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > >      channel.
> > >
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > +    the payload.
> > > +
> > >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > >   (fragmenting the packet) the USO splits large UDP packet
> > >   to several segments when each of these smaller packets has UDP header.
> > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \end{description}
> > >
> > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > >          u8 flags;
> > >  #define VIRTIO_NET_HDR_GSO_NONE        0
> > >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >  according to the native endian of the guest rather than
> > >  (necessarily when not using the legacy interface) little-endian.
> > >
> > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > +the device supports to split the header and the payload.
> > > +The header and payload will be separated into different buffers.
> > > +
> > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > +
> > > +To configure the split header, the following layout structure and definitions
> > > +are used:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_net_split_header_config {
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > +    le64 type;
> > > +};
> > > +
> >
> >
> > how do we know what types does device support?


One idea here is to just have a feature bit per type.


> > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > +\end{lstlisting}
> > > +
> > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > +
> > > +\field{type} passed as command data is a bitmask, bits set define
> > > +packet types to split header, bits cleared - split header to be disabled.
> > > +
> > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > +(include TCP options). The back part is the payload.
> > > +
> > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +Split header MUST be disabled after device initialization.
> > > +
> > > +A device MUST NOT perform split header in the following cases:
> > > +\begin{itemize}
> > > +    \item device does not recognize protocol of the packet.
> > > +    \item \field{type} does not include the protocol of the packet.
> > > +    \item the packet is a IP fragmentation.
> > > +    \item the receive buffer consists of only one descriptor.
> > > +    \item the header exceeds the size of the 0th descriptor.
> > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > +\end{itemize}
> > > +
> > > +If the split header completed, then the \field{flags} of virtnet hdr MUST
> > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > +
> > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > +
> > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > +
> > > +If the split header function is enable, the buffers submitted by the driver
> > > +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> > > +descriptor SHOULD be able to accommodate the header.
> > >
> >
> > I am not sure I understand what all this is saying. Lots of this is
> > ungrammatical. Also - I do not understand what do you mean believe,
> > or 0th descriptor.  Also pls formulate in terms of buffers not descriptors.
> >
> > Thanks!
> >
> > >  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >  Types / Network Device / Legacy Interface: Framing Requirements}
> > > --
> > > 2.31.0
> >


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  4:38         ` Jason Wang
@ 2022-05-31  5:31           ` Michael S. Tsirkin
  2022-05-31  6:39             ` Jason Wang
  0 siblings, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-05-31  5:31 UTC (permalink / raw)
  To: Jason Wang; +Cc: Xuan Zhuo, Virtio-Dev

On Tue, May 31, 2022 at 12:38:14PM +0800, Jason Wang wrote:
> On Tue, May 31, 2022 at 2:23 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Mon, May 30, 2022 at 01:56:06PM +0800, Jason Wang wrote:
> > > On Thu, May 26, 2022 at 2:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > >
> > > > On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > > > > > The purpose of this feature is to split the header and the payload of
> > > > > > the packet.
> > > > > >
> > > > > > |                    receive buffer                                    |
> > > > > > |                       0th descriptor             | 1th descriptor    |
> > > > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > > > >
> > > > > > We can use a buffer plus a separate page when allocating the receive
> > > > > > buffer. In this way, we can ensure that all payloads can be
> > > > > > independently in a page, which is very beneficial for the zerocopy
> > > > > > implemented by the upper layer.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > ---
> > > > > >   conformance.tex |  2 ++
> > > > > >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >   2 files changed, 74 insertions(+)
> > > > > >
> > > > > > diff --git a/conformance.tex b/conformance.tex
> > > > > > index 663e7c3..6f561fb 100644
> > > > > > --- a/conformance.tex
> > > > > > +++ b/conformance.tex
> > > > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > >   \end{itemize}
> > > > > >
> > > > > >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > >   \end{itemize}
> > > > > >
> > > > > >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > > > diff --git a/content.tex b/content.tex
> > > > > > index 060bdab..3340402 100644
> > > > > > --- a/content.tex
> > > > > > +++ b/content.tex
> > > > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > > > >       channel.
> > > > > >
> > > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > > > +    the payload.
> > > > > > +
> > > > > >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > > > >    (fragmenting the packet) the USO splits large UDP packet
> > > > > >    to several segments when each of these smaller packets has UDP header.
> > > > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > > > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > >   \end{description}
> > > > > >
> > > > > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > > >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > > > >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > > > >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > > > >           u8 flags;
> > > > > >   #define VIRTIO_NET_HDR_GSO_NONE        0
> > > > > >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > >   according to the native endian of the guest rather than
> > > > > >   (necessarily when not using the legacy interface) little-endian.
> > > > > >
> > > > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > +
> > > > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > > > +the device supports to split the header and the payload.
> > > > > > +The header and payload will be separated into different buffers.
> > > > >
> > > > >
> > > > > I think you meant "descriptors" instead of "buffers".
> > > >
> > > > Will fix.
> >
> > No, actually, I think this is exactly buffers.
> > device is always in terms of buffers, descriptors is
> > a means to implement buffers.
> >
> > And the feature should
> > depend on mergeable buffers. Without mergeable buffers you do not
> > need these tricks, just add two descriptors and be done
> > with it.
> 
> How do we know the length of a header in advance to allocate the exact
> descriptor properly in this case?
> 
> Thanks

We don't, but with mergeable buffers we don't have to. So basically,
the feature says that instead of using all of 1st buffer then proceeding
to the next one, device will use the 1st one just for the headers.

- without split header
	buf1: hdr1 + data1part1
	buf2: data1part2

- with split header
	buf1: hdr1
	buf1: data1part1
	buf2: data1part2


how do buffers map to descriptors isn't that relevant from this POV.




> >
> > Right?
> >
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > > > +
> > > > > > +To configure the split header, the following layout structure and definitions
> > > > > > +are used:
> > > > > > +
> > > > > > +\begin{lstlisting}
> > > > > > +struct virtio_net_split_header_config {
> > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > > >
> > > > >
> > > > > I think it's better to capitalize: TCPV4 or TCP4.
> > > >
> > > > OK.
> > > >
> > > > >
> > > > >
> > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > > > +    le64 type;
> > > > > > +};
> > > > > > +
> > > > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > > > +\end{lstlisting}
> > > > > > +
> > > > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > > > +
> > > > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > > > +packet types to split header, bits cleared - split header to be disabled.
> > > > >
> > > > >
> > > > > (Not a native speaker, try my best to help)
> > > > >
> > > > > This sentence might need some tweaks:
> > > > >
> > > > > The driver can enable or disable the split by setting or clearing
> > > > > corresponding bits in \field{type}.
> > > >
> > > > Thanks.
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > > > +(include TCP options). The back part is the payload.
> > > > >
> > > > >
> > > > > I think it's better to either explain each type one by one instead of
> > > > > only describing the TCPv4. Or we can simply say the head contains all
> > > > > headers before level 4 payload?
> > > >
> > > > OK. Thanks.
> > >
> >
> > So this always splits out at protocol level?
> > What if device wants to align e.g. IP protocol header?
> > Ethernet?
> >
> >
> > Also, what about aligning at an offset?
> > E.g. NET_IP_ALIGN?
> >
> >
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > +
> > > > > > +Split header MUST be disabled after device initialization.
> > > > >
> > > > >
> > > > > I think you actually mean "upon device reset"?
> > > >
> > > > Yes.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +A device MUST NOT perform split header in the following cases:
> > > > > > +\begin{itemize}
> > > > > > +    \item device does not recognize protocol of the packet.
> > > > > > +    \item \field{type} does not include the protocol of the packet.
> > > > > > +    \item the packet is a IP fragmentation.
> > > > >
> > > > >
> > > > > What's the reason for this limit?
> > > >
> > > > Rethinking this problem, some implementation difficulties that I originally
> > > > thought can be solved. So I think this limitation can be removed.
> > > >
> > > > >
> > > > >
> > > > > > +    \item the receive buffer consists of only one descriptor.
> > > > >
> > > > >
> > > > > descriptor actually.
> > > >
> > > > Will fix.
> > > >
> > > > >
> > > > >
> > > > > > +    \item the header exceeds the size of the 0th descriptor.
> > > > >
> > > > >
> > > > > It looks to me the spec count from the first.
> > > >
> > > > Will fix.
> > > >
> > > > >
> > > > >
> > > > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > > >
> > > > >
> > > > > Maybe something like the following is better:
> > > > >
> > > > > The size of the payload exceeds the length of the descriptor or buffer
> > > > > chain starting from the 2nd descriptor.
> > > >
> > > > OK.
> > > >
> > > > >
> > > > >
> > > > > > +\end{itemize}
> > > > > > +
> > > > > > +If the split header completed,
> > > > >
> > > > >
> > > > > If the header is split by the device?
> > > >
> > > > Yes.
> > > >
> > > > >
> > > > >
> > > > > >   then the \field{flags} of virtnet hdr MUST
> > > > >
> > > > >
> > > > > We can drop "then" here and it's better to use structure virtio_net_hdr
> > > >
> > > > Will fix.
> > > >
> > > > >
> > > > >
> > > > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > > > +0th descriptor,
> > > > >
> > > > >
> > > > > first and second should be used here.
> > > >
> > > > OK
> > > >
> > > > >
> > > > >
> > > > > >   and the payload MUST starts from the buffer of the 1th descriptor.
> > > > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > > >
> > > > >
> > > > > To what value?
> > > >
> > > > I'll make it clear in the next version.
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > > >
> > > > >
> > > > > I don't see why we need this.
> > > >
> > > > If used to implement zerocopy, the second desc points to a page-aligned buffer.
> > > > The first desc points to a small buffer for saving the header. The first buffer
> > > > and subsequent buffers are generally discontinuous. So giving up the first
> > > > buffer and directly guaranteeing the data to the page-aligned buffer can achieve
> > > > better performance. And the driver can reuse the first buffer.
> > >
> > > I see, it might be better if we can explain this here.
> > >
> > > Thanks
> > >
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > +
> > > > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > > >
> > > > >
> > > > > See above we can reuse the device normative an drop the second part
> > > > > starting from " the length of the header ..."
> > > >
> > > > OK
> > > >
> > > > >
> > > > >
> > > > > > +
> > > > > > +If the split header function is enable, the buffers submitted by the driver
> > > > > > +SHOULD at least be composed of two descriptors.
> > > > >
> > > > >
> > > > > Do we need to mention it's only used for RX? (I think we don't need this
> > > > > for TX).
> > > >
> > > > Yes.
> > > >
> > > > >
> > > > >
> > > > > > The buffer specified by the 0th
> > > > > > +descriptor SHOULD be able to accommodate the header.
> > > > >
> > > > >
> > > > > We probably need to clarify that the header here means both the virtio
> > > > > net header and the protocol header.
> > > >
> > > > Will fix.
> > > >
> > > > Thanks.
> > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > >
> > > > > >
> > > > > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > > > >   Types / Network Device / Legacy Interface: Framing Requirements}
> > > > >
> > > > >
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > > >
> > > >
> >


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  2:10   ` Xuan Zhuo
  2022-05-31  5:24     ` Michael S. Tsirkin
@ 2022-05-31  5:35     ` Michael S. Tsirkin
  2022-05-31  5:59       ` Xuan Zhuo
  1 sibling, 1 reply; 14+ messages in thread
From: Michael S. Tsirkin @ 2022-05-31  5:35 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: virtio-dev, jasowang

On Tue, May 31, 2022 at 10:10:52AM +0800, Xuan Zhuo wrote:
> On Mon, 30 May 2022 14:27:26 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> > > The purpose of this feature is to split the header and the payload of
> > > the packet.
> > >
> > > |                    receive buffer                                    |
> > > |                       0th descriptor             | 1th descriptor    |
> > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > >
> > > We can use a buffer plus a separate page when allocating the receive
> > > buffer. In this way, we can ensure that all payloads can be
> > > independently in a page, which is very beneficial for the zerocopy
> > > implemented by the upper layer.
> > >
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> >
> > Okay, but I don't think this covers all possible use-cases.
> > If we are doing this let's try to address alignment requirements too.
> 
> Very happy to do this job.
> 
> I thinks "offset" can contain these requirements.

I am not so sure. The issue is with all of the extension headers where
the payload is is not always predictable, even for a
given type.

> No matter how many descriptors
> are used, we treat multiple buffers as a continuous piece of memory. The device
> places ip, tcp/udp at the specified location according to the specified offset.
> 
> The device does not care whether the data is finally aligned with the buffer
> specified by a descriptor, the device only needs to ensure that each layer of
> the protocol is placed on the specified "offset".
> 
> Many other places need to be modified, and I will publish a new spec.
> 
> Thanks.
> 
> >
> > > ---
> > >  conformance.tex |  2 ++
> > >  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 74 insertions(+)
> > >
> > > diff --git a/conformance.tex b/conformance.tex
> > > index 663e7c3..6f561fb 100644
> > > --- a/conformance.tex
> > > +++ b/conformance.tex
> > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > >  \end{itemize}
> > >
> > >  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > diff --git a/content.tex b/content.tex
> > > index 060bdab..3340402 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > >      channel.
> > >
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > +    the payload.
> > > +
> > >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > >   (fragmenting the packet) the USO splits large UDP packet
> > >   to several segments when each of these smaller packets has UDP header.
> > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > >  \end{description}
> > >
> > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > >          u8 flags;
> > >  #define VIRTIO_NET_HDR_GSO_NONE        0
> > >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >  according to the native endian of the guest rather than
> > >  (necessarily when not using the legacy interface) little-endian.
> > >
> > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > +the device supports to split the header and the payload.
> > > +The header and payload will be separated into different buffers.
> > > +
> > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > +
> > > +To configure the split header, the following layout structure and definitions
> > > +are used:
> > > +
> > > +\begin{lstlisting}
> > > +struct virtio_net_split_header_config {
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > +    le64 type;
> > > +};
> > > +
> >
> >
> > how do we know what types does device support?
> >
> > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > +\end{lstlisting}
> > > +
> > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > +
> > > +\field{type} passed as command data is a bitmask, bits set define
> > > +packet types to split header, bits cleared - split header to be disabled.
> > > +
> > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > +(include TCP options). The back part is the payload.
> > > +
> > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +Split header MUST be disabled after device initialization.
> > > +
> > > +A device MUST NOT perform split header in the following cases:
> > > +\begin{itemize}
> > > +    \item device does not recognize protocol of the packet.
> > > +    \item \field{type} does not include the protocol of the packet.
> > > +    \item the packet is a IP fragmentation.
> > > +    \item the receive buffer consists of only one descriptor.
> > > +    \item the header exceeds the size of the 0th descriptor.
> > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > +\end{itemize}
> > > +
> > > +If the split header completed, then the \field{flags} of virtnet hdr MUST
> > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > +
> > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > +
> > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > +
> > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > +
> > > +If the split header function is enable, the buffers submitted by the driver
> > > +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> > > +descriptor SHOULD be able to accommodate the header.
> > >
> >
> > I am not sure I understand what all this is saying. Lots of this is
> > ungrammatical. Also - I do not understand what do you mean believe,
> > or 0th descriptor.  Also pls formulate in terms of buffers not descriptors.
> >
> > Thanks!
> >
> > >  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >  Types / Network Device / Legacy Interface: Framing Requirements}
> > > --
> > > 2.31.0
> >


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  5:35     ` Michael S. Tsirkin
@ 2022-05-31  5:59       ` Xuan Zhuo
  2022-05-31  7:26         ` Xuan Zhuo
  0 siblings, 1 reply; 14+ messages in thread
From: Xuan Zhuo @ 2022-05-31  5:59 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-dev, jasowang

On Tue, 31 May 2022 01:35:17 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, May 31, 2022 at 10:10:52AM +0800, Xuan Zhuo wrote:
> > On Mon, 30 May 2022 14:27:26 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> > > > The purpose of this feature is to split the header and the payload of
> > > > the packet.
> > > >
> > > > |                    receive buffer                                    |
> > > > |                       0th descriptor             | 1th descriptor    |
> > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > >
> > > > We can use a buffer plus a separate page when allocating the receive
> > > > buffer. In this way, we can ensure that all payloads can be
> > > > independently in a page, which is very beneficial for the zerocopy
> > > > implemented by the upper layer.
> > > >
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > >
> > > Okay, but I don't think this covers all possible use-cases.
> > > If we are doing this let's try to address alignment requirements too.
> >
> > Very happy to do this job.
> >
> > I thinks "offset" can contain these requirements.
>
> I am not so sure. The issue is with all of the extension headers where
> the payload is is not always predictable, even for a
> given type.



I think we should first define clearly the problem we are trying to solve.

1. split tcp/udp payload, or more protocol payload
2. alignment for ip header

For others like eth or tcp/udp header do we need to align?

Thanks.


>
> > No matter how many descriptors
> > are used, we treat multiple buffers as a continuous piece of memory. The device
> > places ip, tcp/udp at the specified location according to the specified offset.
> >
> > The device does not care whether the data is finally aligned with the buffer
> > specified by a descriptor, the device only needs to ensure that each layer of
> > the protocol is placed on the specified "offset".
> >
> > Many other places need to be modified, and I will publish a new spec.
> >
> > Thanks.
> >
> > >
> > > > ---
> > > >  conformance.tex |  2 ++
> > > >  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 74 insertions(+)
> > > >
> > > > diff --git a/conformance.tex b/conformance.tex
> > > > index 663e7c3..6f561fb 100644
> > > > --- a/conformance.tex
> > > > +++ b/conformance.tex
> > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > >  \end{itemize}
> > > >
> > > >  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > >  \end{itemize}
> > > >
> > > >  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > diff --git a/content.tex b/content.tex
> > > > index 060bdab..3340402 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > >      channel.
> > > >
> > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > +    the payload.
> > > > +
> > > >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > >   (fragmenting the packet) the USO splits large UDP packet
> > > >   to several segments when each of these smaller packets has UDP header.
> > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > >  \end{description}
> > > >
> > > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > >          u8 flags;
> > > >  #define VIRTIO_NET_HDR_GSO_NONE        0
> > > >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > >  according to the native endian of the guest rather than
> > > >  (necessarily when not using the legacy interface) little-endian.
> > > >
> > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > +the device supports to split the header and the payload.
> > > > +The header and payload will be separated into different buffers.
> > > > +
> > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > +
> > > > +To configure the split header, the following layout structure and definitions
> > > > +are used:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_net_split_header_config {
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > +    le64 type;
> > > > +};
> > > > +
> > >
> > >
> > > how do we know what types does device support?
> > >
> > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > +\end{lstlisting}
> > > > +
> > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > +
> > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > +packet types to split header, bits cleared - split header to be disabled.
> > > > +
> > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > +(include TCP options). The back part is the payload.
> > > > +
> > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +Split header MUST be disabled after device initialization.
> > > > +
> > > > +A device MUST NOT perform split header in the following cases:
> > > > +\begin{itemize}
> > > > +    \item device does not recognize protocol of the packet.
> > > > +    \item \field{type} does not include the protocol of the packet.
> > > > +    \item the packet is a IP fragmentation.
> > > > +    \item the receive buffer consists of only one descriptor.
> > > > +    \item the header exceeds the size of the 0th descriptor.
> > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > > +\end{itemize}
> > > > +
> > > > +If the split header completed, then the \field{flags} of virtnet hdr MUST
> > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > > +
> > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > > +
> > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > +
> > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > > +
> > > > +If the split header function is enable, the buffers submitted by the driver
> > > > +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> > > > +descriptor SHOULD be able to accommodate the header.
> > > >
> > >
> > > I am not sure I understand what all this is saying. Lots of this is
> > > ungrammatical. Also - I do not understand what do you mean believe,
> > > or 0th descriptor.  Also pls formulate in terms of buffers not descriptors.
> > >
> > > Thanks!
> > >
> > > >  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > >  Types / Network Device / Legacy Interface: Framing Requirements}
> > > > --
> > > > 2.31.0
> > >
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  5:31           ` Michael S. Tsirkin
@ 2022-05-31  6:39             ` Jason Wang
  0 siblings, 0 replies; 14+ messages in thread
From: Jason Wang @ 2022-05-31  6:39 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Xuan Zhuo, Virtio-Dev

On Tue, May 31, 2022 at 1:32 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Tue, May 31, 2022 at 12:38:14PM +0800, Jason Wang wrote:
> > On Tue, May 31, 2022 at 2:23 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> > >
> > > On Mon, May 30, 2022 at 01:56:06PM +0800, Jason Wang wrote:
> > > > On Thu, May 26, 2022 at 2:31 PM Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> > > > >
> > > > > On Mon, 9 May 2022 16:41:11 +0800, Jason Wang <jasowang@redhat.com> wrote:
> > > > > >
> > > > > > 在 2022/5/7 15:15, Xuan Zhuo 写道:
> > > > > > > The purpose of this feature is to split the header and the payload of
> > > > > > > the packet.
> > > > > > >
> > > > > > > |                    receive buffer                                    |
> > > > > > > |                       0th descriptor             | 1th descriptor    |
> > > > > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > > > > >
> > > > > > > We can use a buffer plus a separate page when allocating the receive
> > > > > > > buffer. In this way, we can ensure that all payloads can be
> > > > > > > independently in a page, which is very beneficial for the zerocopy
> > > > > > > implemented by the upper layer.
> > > > > > >
> > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > > ---
> > > > > > >   conformance.tex |  2 ++
> > > > > > >   content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >   2 files changed, 74 insertions(+)
> > > > > > >
> > > > > > > diff --git a/conformance.tex b/conformance.tex
> > > > > > > index 663e7c3..6f561fb 100644
> > > > > > > --- a/conformance.tex
> > > > > > > +++ b/conformance.tex
> > > > > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > > > > >   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > >   \end{itemize}
> > > > > > >
> > > > > > >   \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > > > >   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > >   \end{itemize}
> > > > > > >
> > > > > > >   \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > > > > diff --git a/content.tex b/content.tex
> > > > > > > index 060bdab..3340402 100644
> > > > > > > --- a/content.tex
> > > > > > > +++ b/content.tex
> > > > > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > > > > >       channel.
> > > > > > >
> > > > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > > > > +    the payload.
> > > > > > > +
> > > > > > >   \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > > > > >    (fragmenting the packet) the USO splits large UDP packet
> > > > > > >    to several segments when each of these smaller packets has UDP header.
> > > > > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > > > > >   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > > >   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > > > > >   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > > >   \end{description}
> > > > > > >
> > > > > > >   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > > > >   #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > > > > >   #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > > > > >   #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > > > > >           u8 flags;
> > > > > > >   #define VIRTIO_NET_HDR_GSO_NONE        0
> > > > > > >   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > > >   according to the native endian of the guest rather than
> > > > > > >   (necessarily when not using the legacy interface) little-endian.
> > > > > > >
> > > > > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > > +
> > > > > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > > > > +the device supports to split the header and the payload.
> > > > > > > +The header and payload will be separated into different buffers.
> > > > > >
> > > > > >
> > > > > > I think you meant "descriptors" instead of "buffers".
> > > > >
> > > > > Will fix.
> > >
> > > No, actually, I think this is exactly buffers.
> > > device is always in terms of buffers, descriptors is
> > > a means to implement buffers.
> > >
> > > And the feature should
> > > depend on mergeable buffers. Without mergeable buffers you do not
> > > need these tricks, just add two descriptors and be done
> > > with it.
> >
> > How do we know the length of a header in advance to allocate the exact
> > descriptor properly in this case?
> >
> > Thanks
>
> We don't, but with mergeable buffers we don't have to. So basically,
> the feature says that instead of using all of 1st buffer then proceeding
> to the next one, device will use the 1st one just for the headers.
>
> - without split header
>         buf1: hdr1 + data1part1
>         buf2: data1part2
>
> - with split header
>         buf1: hdr1
>         buf1: data1part1
>         buf2: data1part2
>
>
> how do buffers map to descriptors isn't that relevant from this POV.

Yes, but I meant without a mergeable buffer, we still need the device
to place the payload at a dedicated descriptor, this can't be done
solely with the driver.

buff1_desc1: hdr buff1_desc2 payload

Thanks

>
>
>
>
> > >
> > > Right?
> > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > > > > +
> > > > > > > +To configure the split header, the following layout structure and definitions
> > > > > > > +are used:
> > > > > > > +
> > > > > > > +\begin{lstlisting}
> > > > > > > +struct virtio_net_split_header_config {
> > > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > > > >
> > > > > >
> > > > > > I think it's better to capitalize: TCPV4 or TCP4.
> > > > >
> > > > > OK.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > > > > +    le64 type;
> > > > > > > +};
> > > > > > > +
> > > > > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > > > > +\end{lstlisting}
> > > > > > > +
> > > > > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > > > > +
> > > > > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > > > > +packet types to split header, bits cleared - split header to be disabled.
> > > > > >
> > > > > >
> > > > > > (Not a native speaker, try my best to help)
> > > > > >
> > > > > > This sentence might need some tweaks:
> > > > > >
> > > > > > The driver can enable or disable the split by setting or clearing
> > > > > > corresponding bits in \field{type}.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > > > > +(include TCP options). The back part is the payload.
> > > > > >
> > > > > >
> > > > > > I think it's better to either explain each type one by one instead of
> > > > > > only describing the TCPv4. Or we can simply say the head contains all
> > > > > > headers before level 4 payload?
> > > > >
> > > > > OK. Thanks.
> > > >
> > >
> > > So this always splits out at protocol level?
> > > What if device wants to align e.g. IP protocol header?
> > > Ethernet?
> > >
> > >
> > > Also, what about aligning at an offset?
> > > E.g. NET_IP_ALIGN?
> > >
> > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > > +
> > > > > > > +Split header MUST be disabled after device initialization.
> > > > > >
> > > > > >
> > > > > > I think you actually mean "upon device reset"?
> > > > >
> > > > > Yes.
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +A device MUST NOT perform split header in the following cases:
> > > > > > > +\begin{itemize}
> > > > > > > +    \item device does not recognize protocol of the packet.
> > > > > > > +    \item \field{type} does not include the protocol of the packet.
> > > > > > > +    \item the packet is a IP fragmentation.
> > > > > >
> > > > > >
> > > > > > What's the reason for this limit?
> > > > >
> > > > > Rethinking this problem, some implementation difficulties that I originally
> > > > > thought can be solved. So I think this limitation can be removed.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +    \item the receive buffer consists of only one descriptor.
> > > > > >
> > > > > >
> > > > > > descriptor actually.
> > > > >
> > > > > Will fix.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +    \item the header exceeds the size of the 0th descriptor.
> > > > > >
> > > > > >
> > > > > > It looks to me the spec count from the first.
> > > > >
> > > > > Will fix.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > > > >
> > > > > >
> > > > > > Maybe something like the following is better:
> > > > > >
> > > > > > The size of the payload exceeds the length of the descriptor or buffer
> > > > > > chain starting from the 2nd descriptor.
> > > > >
> > > > > OK.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +\end{itemize}
> > > > > > > +
> > > > > > > +If the split header completed,
> > > > > >
> > > > > >
> > > > > > If the header is split by the device?
> > > > >
> > > > > Yes.
> > > > >
> > > > > >
> > > > > >
> > > > > > >   then the \field{flags} of virtnet hdr MUST
> > > > > >
> > > > > >
> > > > > > We can drop "then" here and it's better to use structure virtio_net_hdr
> > > > >
> > > > > Will fix.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > > > > +0th descriptor,
> > > > > >
> > > > > >
> > > > > > first and second should be used here.
> > > > >
> > > > > OK
> > > > >
> > > > > >
> > > > > >
> > > > > > >   and the payload MUST starts from the buffer of the 1th descriptor.
> > > > > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > > > >
> > > > > >
> > > > > > To what value?
> > > > >
> > > > > I'll make it clear in the next version.
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > > > >
> > > > > >
> > > > > > I don't see why we need this.
> > > > >
> > > > > If used to implement zerocopy, the second desc points to a page-aligned buffer.
> > > > > The first desc points to a small buffer for saving the header. The first buffer
> > > > > and subsequent buffers are generally discontinuous. So giving up the first
> > > > > buffer and directly guaranteeing the data to the page-aligned buffer can achieve
> > > > > better performance. And the driver can reuse the first buffer.
> > > >
> > > > I see, it might be better if we can explain this here.
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > > > +
> > > > > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > > > >
> > > > > >
> > > > > > See above we can reuse the device normative an drop the second part
> > > > > > starting from " the length of the header ..."
> > > > >
> > > > > OK
> > > > >
> > > > > >
> > > > > >
> > > > > > > +
> > > > > > > +If the split header function is enable, the buffers submitted by the driver
> > > > > > > +SHOULD at least be composed of two descriptors.
> > > > > >
> > > > > >
> > > > > > Do we need to mention it's only used for RX? (I think we don't need this
> > > > > > for TX).
> > > > >
> > > > > Yes.
> > > > >
> > > > > >
> > > > > >
> > > > > > > The buffer specified by the 0th
> > > > > > > +descriptor SHOULD be able to accommodate the header.
> > > > > >
> > > > > >
> > > > > > We probably need to clarify that the header here means both the virtio
> > > > > > net header and the protocol header.
> > > > >
> > > > > Will fix.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >   \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > > > > >   Types / Network Device / Legacy Interface: Framing Requirements}
> > > > > >
> > > > > >
> > > > > > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> > > > > >
> > > > >
> > >
>


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [virtio-dev] Re: [PATCH v2] virtio_net: support split header
  2022-05-31  5:59       ` Xuan Zhuo
@ 2022-05-31  7:26         ` Xuan Zhuo
  0 siblings, 0 replies; 14+ messages in thread
From: Xuan Zhuo @ 2022-05-31  7:26 UTC (permalink / raw)
  To: Xuan Zhuo; +Cc: virtio-dev, jasowang, Michael S. Tsirkin

On Tue, 31 May 2022 13:59:12 +0800, Xuan Zhuo <xuanzhuo@linux.alibaba.com> wrote:
> On Tue, 31 May 2022 01:35:17 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Tue, May 31, 2022 at 10:10:52AM +0800, Xuan Zhuo wrote:
> > > On Mon, 30 May 2022 14:27:26 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > On Sat, May 07, 2022 at 03:15:33PM +0800, Xuan Zhuo wrote:
> > > > > The purpose of this feature is to split the header and the payload of
> > > > > the packet.
> > > > >
> > > > > |                    receive buffer                                    |
> > > > > |                       0th descriptor             | 1th descriptor    |
> > > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->|   payload         |
> > > > >
> > > > > We can use a buffer plus a separate page when allocating the receive
> > > > > buffer. In this way, we can ensure that all payloads can be
> > > > > independently in a page, which is very beneficial for the zerocopy
> > > > > implemented by the upper layer.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > >
> > > > Okay, but I don't think this covers all possible use-cases.
> > > > If we are doing this let's try to address alignment requirements too.
> > >
> > > Very happy to do this job.
> > >
> > > I thinks "offset" can contain these requirements.
> >
> > I am not so sure. The issue is with all of the extension headers where
> > the payload is is not always predictable, even for a
> > given type.
>
>
>
> I think we should first define clearly the problem we are trying to solve.
>
> 1. split tcp/udp payload, or more protocol payload
> 2. alignment for ip header


If only the ip header has the requirement of alignment, I feel that a separate
feature should be added for it. Because this does not combine well with the
split header. Unless there are other more similar requirements.

Thanks.


>
> For others like eth or tcp/udp header do we need to align?
>
> Thanks.
>
>
> >
> > > No matter how many descriptors
> > > are used, we treat multiple buffers as a continuous piece of memory. The device
> > > places ip, tcp/udp at the specified location according to the specified offset.
> > >
> > > The device does not care whether the data is finally aligned with the buffer
> > > specified by a descriptor, the device only needs to ensure that each layer of
> > > the protocol is placed on the specified "offset".
> > >
> > > Many other places need to be modified, and I will publish a new spec.
> > >
> > > Thanks.
> > >
> > > >
> > > > > ---
> > > > >  conformance.tex |  2 ++
> > > > >  content.tex     | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  2 files changed, 74 insertions(+)
> > > > >
> > > > > diff --git a/conformance.tex b/conformance.tex
> > > > > index 663e7c3..6f561fb 100644
> > > > > --- a/conformance.tex
> > > > > +++ b/conformance.tex
> > > > > @@ -149,6 +149,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
> > > > >  \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
> > > > > +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > >  \end{itemize}
> > > > >
> > > > >  \conformance{\subsection}{Block Driver Conformance}\label{sec:Conformance / Driver Conformance / Block Driver Conformance}
> > > > > @@ -411,6 +412,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets}
> > > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Gratuitous Packet Sending}
> > > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
> > > > >  \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
> > > > > +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > >  \end{itemize}
> > > > >
> > > > >  \conformance{\subsection}{Block Device Conformance}\label{sec:Conformance / Device Conformance / Block Device Conformance}
> > > > > diff --git a/content.tex b/content.tex
> > > > > index 060bdab..3340402 100644
> > > > > --- a/content.tex
> > > > > +++ b/content.tex
> > > > > @@ -3092,6 +3092,9 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> > > > >      channel.
> > > > >
> > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER (55)] Device supports to split the header and
> > > > > +    the payload.
> > > > > +
> > > > >  \item[VIRTIO_NET_F_HOST_USO (56)] Device can receive USO packets. Unlike UFO
> > > > >   (fragmenting the packet) the USO splits large UDP packet
> > > > >   to several segments when each of these smaller packets has UDP header.
> > > > > @@ -3139,6 +3142,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> > > > >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > >  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> > > > >  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > > +\item[VIRTIO_NET_F_SPLIT_HEADER] Requires VIRTIO_NET_F_CTRL_VQ.
> > > > >  \end{description}
> > > > >
> > > > >  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> > > > > @@ -3370,6 +3374,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > >  #define VIRTIO_NET_HDR_F_NEEDS_CSUM    1
> > > > >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> > > > >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> > > > > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER  8
> > > > >          u8 flags;
> > > > >  #define VIRTIO_NET_HDR_GSO_NONE        0
> > > > >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > > > > @@ -4471,6 +4476,73 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >  according to the native endian of the guest rather than
> > > > >  (necessarily when not using the legacy interface) little-endian.
> > > > >
> > > > > +\paragraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +If the VIRTIO_NET_F_SPLIT_HEADER feature is negotiated,
> > > > > +the device supports to split the header and the payload.
> > > > > +The header and payload will be separated into different buffers.
> > > > > +
> > > > > +\subparagraph{Split Header}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Split Header / Setting Split Header}
> > > > > +
> > > > > +To configure the split header, the following layout structure and definitions
> > > > > +are used:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +struct virtio_net_split_header_config {
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4     1
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv6     2
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv4     4
> > > > > +#define VIRTIO_NET_SPLIT_HEADER_TYPE_UDPv6     8
> > > > > +    le64 type;
> > > > > +};
> > > > > +
> > > >
> > > >
> > > > how do we know what types does device support?
> > > >
> > > > > +#define VIRTIO_NET_CTRL_SPLIT_HEADER       6
> > > > > + #define VIRTIO_NET_CTRL_SPLIT_HEADER_SET   0
> > > > > +\end{lstlisting}
> > > > > +
> > > > > +The class VIRTIO_NET_CTRL_SPLIT_HEADER has one command:
> > > > > +VIRTIO_NET_CTRL_SPLIT_HEADER_SET applies the new split header configuration.
> > > > > +
> > > > > +\field{type} passed as command data is a bitmask, bits set define
> > > > > +packet types to split header, bits cleared - split header to be disabled.
> > > > > +
> > > > > +The header contains the struct virtio_net_hdr and the header of the package.
> > > > > +Such as \field{VIRTIO_NET_SPLIT_HEADER_TYPE_TCPv4} specified header contains
> > > > > +virtio_net_hdr, MAC header, IPv4 header (including IPv4 options), TCP header
> > > > > +(include TCP options). The back part is the payload.
> > > > > +
> > > > > +\devicenormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +Split header MUST be disabled after device initialization.
> > > > > +
> > > > > +A device MUST NOT perform split header in the following cases:
> > > > > +\begin{itemize}
> > > > > +    \item device does not recognize protocol of the packet.
> > > > > +    \item \field{type} does not include the protocol of the packet.
> > > > > +    \item the packet is a IP fragmentation.
> > > > > +    \item the receive buffer consists of only one descriptor.
> > > > > +    \item the header exceeds the size of the 0th descriptor.
> > > > > +    \item If VIRTIO_NET_F_MRG_RXBUF is not negotiated and the size of the
> > > > > +        payload is greater than the total size of the 1th\ldots Nth descriptor.
> > > > > +\end{itemize}
> > > > > +
> > > > > +If the split header completed, then the \field{flags} of virtnet hdr MUST
> > > > > +contains VIRTIO_NET_HDR_F_SPLIT_HEADER. The header MUST is on the buffer of the
> > > > > +0th descriptor, and the payload MUST starts from the buffer of the 1th descriptor.
> > > > > +The device MUST set \field{hdr_len} of virtnet hdr.
> > > > > +
> > > > > +If VIRTIO_NET_F_MRG_RXBUF is negotiated and the device is to use multiple
> > > > > +receive buffers, each subsequent receive buffer MUST skip the 0th descriptor.
> > > > > +
> > > > > +\drivernormative{\subparagraph}{Setting Split Header}{Device Types / Network Device / Device Operation / Control Virtqueue / Split Header}
> > > > > +
> > > > > +If VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MUST
> > > > > +believe \field{hdr_len}, the length of the header in the 0th descriptor is equal
> > > > > +to the length of struct virtio_net_hdr plus \field{hdr_len}.
> > > > > +
> > > > > +If the split header function is enable, the buffers submitted by the driver
> > > > > +SHOULD at least be composed of two descriptors. The buffer specified by the 0th
> > > > > +descriptor SHOULD be able to accommodate the header.
> > > > >
> > > >
> > > > I am not sure I understand what all this is saying. Lots of this is
> > > > ungrammatical. Also - I do not understand what do you mean believe,
> > > > or 0th descriptor.  Also pls formulate in terms of buffers not descriptors.
> > > >
> > > > Thanks!
> > > >
> > > > >  \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > > > >  Types / Network Device / Legacy Interface: Framing Requirements}
> > > > > --
> > > > > 2.31.0
> > > >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-05-31  7:30 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-07  7:15 [virtio-dev] [PATCH v2] virtio_net: support split header Xuan Zhuo
2022-05-09  8:41 ` [virtio-dev] " Jason Wang
2022-05-26  6:03   ` Xuan Zhuo
2022-05-30  5:56     ` Jason Wang
2022-05-30 18:22       ` Michael S. Tsirkin
2022-05-31  4:38         ` Jason Wang
2022-05-31  5:31           ` Michael S. Tsirkin
2022-05-31  6:39             ` Jason Wang
2022-05-30 18:27 ` Michael S. Tsirkin
2022-05-31  2:10   ` Xuan Zhuo
2022-05-31  5:24     ` Michael S. Tsirkin
2022-05-31  5:35     ` Michael S. Tsirkin
2022-05-31  5:59       ` Xuan Zhuo
2022-05-31  7:26         ` Xuan Zhuo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.