[virtio-dev] Re: [PATCH] virtio_net: support split header
On Wed, Feb 16, 2022 at 11:01 AM Xuan Zhuo wrote: > > The purpose of this feature is to write the payload of the packet to a > specified location in the receive buffer after the device receives the > packet. > > |receive buffer | > | offset || > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->| payload | This looks more like header padding instead of header split? I had something similar thought in the past, I wonder why not simply add the padding/hold via virtnet hdr? | virtnet hdr| <- hold -> | mac | ip | tcp | payload | > > Based on this feature, we can obtain two benefits. > > 1. We can use a buffer of size "offset" plus a separate page when >allocating the receive buffer. In this way, we can ensure that all >payloads can be independently in a page, Can we achieve this without this proposal today? It looks to me it's possible if we allocated a dedicated sg for vnet header + all the other headers. > which is very beneficial for >the zerocopy implemented by the upper layer. Right, it allows us to use TCP RX zerocopy. So TCP zero copy requires header split which is kind of different from what is being proposed here. AFAIK, it requires the hardware to place the l4 payload in a descriptor. Is this something what you really want: VIRTIO_NET_F_HEADER_SPLIT: The device MUST place the l4 payload at the start of the next descriptor (or drop it if there's no descriptor in the chain). Then we don't even need the stuff like, cvq/commands/protocol types. > > 2. We can include SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) when >setting the offset, so that in the linux implementation, we can >construct skb very efficiently based on build_skb(). No need to copy >the network header. For spec patch, I think we need to avoid talking about any specific driver implementation but describe the logic behind that. > >For MRG_RX, we require that all payloads are placed in the position >specified by offset in each receive buffer. So we can also use >build_skb() without wasting memory. Because we can reuse the >"offset" parts that are not used. > > Signed-off-by: Xuan Zhuo > --- > conformance.tex | 2 ++ > content.tex | 59 + > 2 files changed, 61 insertions(+) > > diff --git a/conformance.tex b/conformance.tex > index 42f8537..e5d2ca8 100644 > --- a/conformance.tex > +++ b/conformance.tex > @@ -142,6 +142,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} > @@ -401,6 +402,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 c6f116c..02cde55 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 can separate the header and the > +payload. The payload will be placed at the specified offset. > + > \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[VI
[virtio-dev] Re: [PATCH] virtio_net: support split header
On Wed, 16 Feb 2022 12:32:03 +0800, Jason Wang wrote: > On Wed, Feb 16, 2022 at 11:01 AM Xuan Zhuo wrote: > > > > The purpose of this feature is to write the payload of the packet to a > > specified location in the receive buffer after the device receives the > > packet. > > > > |receive buffer | > > | offset || > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->| payload | > > This looks more like header padding instead of header split? > > I had something similar thought in the past, I wonder why not simply > add the padding/hold via virtnet hdr? > > | virtnet hdr| <- hold -> | mac | ip | tcp | payload | > > > > > Based on this feature, we can obtain two benefits. > > > > 1. We can use a buffer of size "offset" plus a separate page when > >allocating the receive buffer. In this way, we can ensure that all > >payloads can be independently in a page, > > Can we achieve this without this proposal today? > > It looks to me it's possible if we allocated a dedicated sg for vnet > header + all the other headers. > > > which is very beneficial for > >the zerocopy implemented by the upper layer. > > Right, it allows us to use TCP RX zerocopy. > > So TCP zero copy requires header split which is kind of different from > what is being proposed here. AFAIK, it requires the hardware to place > the l4 payload in a descriptor. Is this something what you really > want: > > VIRTIO_NET_F_HEADER_SPLIT: The device MUST place the l4 payload at the > start of the next descriptor (or drop it if there's no descriptor in > the chain). Yes, in fact I am referring to such a function. In order to make it more general, I introduced a mechanism such as offset. Because in some cases, we can obtain continuous memory while meeting the requirements, using offset can save a desc. | page|page| | |offset|| ^ pointer At the same time I want to specify which types of packages to use this function. And I hope this may be dynamically switchable. So introduce cvq/command. > > Then we don't even need the stuff like, cvq/commands/protocol types. > > > > > > 2. We can include SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) when > >setting the offset, so that in the linux implementation, we can > >construct skb very efficiently based on build_skb(). No need to copy > >the network header. > > For spec patch, I think we need to avoid talking about any specific > driver implementation but describe the logic behind that. Yes. > > > > >For MRG_RX, we require that all payloads are placed in the position > >specified by offset in each receive buffer. So we can also use > >build_skb() without wasting memory. Because we can reuse the > >"offset" parts that are not used. > > > > Signed-off-by: Xuan Zhuo > > --- > > conformance.tex | 2 ++ > > content.tex | 59 + > > 2 files changed, 61 insertions(+) > > > > diff --git a/conformance.tex b/conformance.tex > > index 42f8537..e5d2ca8 100644 > > --- a/conformance.tex > > +++ b/conformance.tex > > @@ -142,6 +142,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} > > @@ -401,6 +402,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 c6f116c..02cde55 100644 > > --- a/content.tex > > +++ b/content.tex > > @@
[virtio-dev] Re: [PATCH] virtio_net: support split header
On Wed, Feb 16, 2022 at 2:31 PM Xuan Zhuo wrote: > > On Wed, 16 Feb 2022 12:32:03 +0800, Jason Wang wrote: > > On Wed, Feb 16, 2022 at 11:01 AM Xuan Zhuo > > wrote: > > > > > > The purpose of this feature is to write the payload of the packet to a > > > specified location in the receive buffer after the device receives the > > > packet. > > > > > > |receive buffer | > > > | offset || > > > | virtnet hdr | mac | ip hdr | tcp hdr|<-- hold -->| payload | > > > > This looks more like header padding instead of header split? > > > > I had something similar thought in the past, I wonder why not simply > > add the padding/hold via virtnet hdr? > > > > | virtnet hdr| <- hold -> | mac | ip | tcp | payload | > > > > > > > > Based on this feature, we can obtain two benefits. > > > > > > 1. We can use a buffer of size "offset" plus a separate page when > > >allocating the receive buffer. In this way, we can ensure that all > > >payloads can be independently in a page, > > > > Can we achieve this without this proposal today? > > > > It looks to me it's possible if we allocated a dedicated sg for vnet > > header + all the other headers. > > > > > which is very beneficial for > > >the zerocopy implemented by the upper layer. > > > > Right, it allows us to use TCP RX zerocopy. > > > > So TCP zero copy requires header split which is kind of different from > > what is being proposed here. AFAIK, it requires the hardware to place > > the l4 payload in a descriptor. Is this something what you really > > want: > > > > VIRTIO_NET_F_HEADER_SPLIT: The device MUST place the l4 payload at the > > start of the next descriptor (or drop it if there's no descriptor in > > the chain). > > Yes, in fact I am referring to such a function. > > In order to make it more general, I introduced a mechanism such as offset. > Because in some cases, we can obtain continuous memory while meeting the > requirements, using offset can save a desc. > > | page|page| > | |offset|| >^ > pointer > This looks rare, e.g it requires the header start near the end of a page which looks odd (at least not the behaviour of the current Linux). Having dedicated buffer for header and data seems much more simpler. Btw, using dedicated buffer descriptors seems more general and it is the way that is used in real hardware vendors (e.g xl710). > At the same time I want to specify which types of packages to use this > function. > And I hope this may be dynamically switchable. So introduce cvq/command. That's fine just as control_offload but it needs to be designed that not having too much of configurations (e.g we may want to support tunneling anyhow). > > > > > > Then we don't even need the stuff like, cvq/commands/protocol types. > > > > > > > > > > 2. We can include SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) when > > >setting the offset, so that in the linux implementation, we can > > >construct skb very efficiently based on build_skb(). No need to copy > > >the network header. > > > > For spec patch, I think we need to avoid talking about any specific > > driver implementation but describe the logic behind that. > > Yes. > > > > > > > > >For MRG_RX, we require that all payloads are placed in the position > > >specified by offset in each receive buffer. So we can also use > > >build_skb() without wasting memory. Because we can reuse the > > >"offset" parts that are not used. > > > > > > Signed-off-by: Xuan Zhuo > > > --- > > > conformance.tex | 2 ++ > > > content.tex | 59 + > > > 2 files changed, 61 insertions(+) > > > > > > diff --git a/conformance.tex b/conformance.tex > > > index 42f8537..e5d2ca8 100644 > > > --- a/conformance.tex > > > +++ b/conformance.tex > > > @@ -142,6 +142,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} > > > @@ -401,6 +402,7 @@ \section{Conformance Targets}\label{sec:Conformance / > > > Conformance Targets} > > > \item \ref{devicenormative:Device Types / Network Device / Device > > > O
[virtio-dev] Re: [PATCH] virtio_net: support split header
The content of the mail just sent is about "[PATCH v6] virtio_net: support split header". 在 2022/8/1 下午2:59, Heng Qi 写道: From: 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 Signed-off-by: Heng Qi Reviewed-by: Kangjie Xu --- v6: 1. Fix some syntax issues. @Cornelia Huck 2. Clarify some paragraphs. @Cornelia Huck 3. Determine the device what to do if it does not perform header split on a packet. v5: 1. Determine when hdr_len is credible in the process of rx 2. Clean up the use of buffers and descriptors 3. Clarify the meaning of used lenght if the first descriptor is skipped in the case of merge v4: 1. fix typo @Cornelia Huck @Jason Wang 2. do not split header for IP fragmentation packet. @Jason Wang v3: 1. Fix some syntax issues 2. Fix some terminology issues 3. It is not unified with ip alignment, so ip alignment is not included 4. Make it clear that the device must support four types, in the case of successful negotiation. conformance.tex | 2 + content.tex | 111 ++-- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/conformance.tex b/conformance.tex index 2b86fc6..bd0f463 100644 --- a/conformance.tex +++ b/conformance.tex @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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} @@ -415,6 +416,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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 e863709..74c36fe 100644 --- a/content.tex +++ b/content.tex @@ -3084,6 +3084,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 (52)] Device supports splitting the protocol +header and the payload. + \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing. \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets. @@ -3140,6 +3143,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device \item[VIRTIO_NET_F_NOTF_COAL] 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} @@ -3371,6 +3375,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O #define VIRTIO_NET_HDR_F_NEEDS_CSUM1 #define VIRTIO_NET_HDR_F_DATA_VALID2 #define VIRTIO_NET_HDR_F_RSC_INFO 4 +#define VIRTIO_NET_HDR_F_SPLIT_HEADER 8 u8 flags; #define VIRTIO_NET_HDR_GSO_NONE0 #define VIRTIO_NET_HDR_GSO_TCPV4 1 @@ -3799,9 +3804,10 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network not set VIRTIO_NET_HDR_F_RSC_INFO bit in \
[virtio-dev] Re: [PATCH] virtio_net: support split header
On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: > > From: 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 > Signed-off-by: Heng Qi > Reviewed-by: Kangjie Xu > --- > > v6: > 1. Fix some syntax issues. @Cornelia Huck > 2. Clarify some paragraphs. @Cornelia Huck > 3. Determine the device what to do if it does not perform header split on > a packet. > > v5: > 1. Determine when hdr_len is credible in the process of rx > 2. Clean up the use of buffers and descriptors > 3. Clarify the meaning of used lenght if the first descriptor is skipped > in the case of merge > > v4: > 1. fix typo @Cornelia Huck @Jason Wang > 2. do not split header for IP fragmentation packet. @Jason Wang > > v3: > 1. Fix some syntax issues > 2. Fix some terminology issues > 3. It is not unified with ip alignment, so ip alignment is not included > 4. Make it clear that the device must support four types, in the case of > successful negotiation. > > conformance.tex | 2 + > content.tex | 111 > ++-- > 2 files changed, 109 insertions(+), 4 deletions(-) > > diff --git a/conformance.tex b/conformance.tex > index 2b86fc6..bd0f463 100644 > --- a/conformance.tex > +++ b/conformance.tex > @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / > Conformance Targets} > \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 / Notifications Coalescing} > +\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} > @@ -415,6 +416,7 @@ \section{Conformance Targets}\label{sec:Conformance / > Conformance Targets} > \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 / Notifications Coalescing} > +\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 e863709..74c36fe 100644 > --- a/content.tex > +++ b/content.tex > @@ -3084,6 +3084,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 (52)] Device supports splitting the protocol > +header and the payload. > + > \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing. > > \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets. > @@ -3140,6 +3143,7 @@ \subsubsection{Feature bit > requirements}\label{sec:Device Types / Network Device > \item[VIRTIO_NET_F_NOTF_COAL] 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} > @@ -3371,6 +3375,7 @@ \subsection{Device Operation}\label{sec:Device Types / > Network Device / Device O > #define VIRTIO_NET_HDR_F_NEEDS_CSUM1 > #define VIRTIO_NET_HDR_F_DATA_VALID2 > #define VIRTIO_NET_HDR_F_RSC_INFO 4 > +#define VIRTIO_NET_HDR_F_SPLIT_HEADER 8 > u8 flags; > #define VIRTIO_NET_HDR_GSO_NONE0 > #define VIRTIO_NET_HDR_GSO_TCPV4 1 > @@ -3799,9 +3804,10 @@ \subsubsection{Processing of Incoming
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
在 2022/8/4 下午2:27, Jason Wang 写道: On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: From: 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 Signed-off-by: Heng Qi Reviewed-by: Kangjie Xu --- v6: 1. Fix some syntax issues. @Cornelia Huck 2. Clarify some paragraphs. @Cornelia Huck 3. Determine the device what to do if it does not perform header split on a packet. v5: 1. Determine when hdr_len is credible in the process of rx 2. Clean up the use of buffers and descriptors 3. Clarify the meaning of used lenght if the first descriptor is skipped in the case of merge v4: 1. fix typo @Cornelia Huck @Jason Wang 2. do not split header for IP fragmentation packet. @Jason Wang v3: 1. Fix some syntax issues 2. Fix some terminology issues 3. It is not unified with ip alignment, so ip alignment is not included 4. Make it clear that the device must support four types, in the case of successful negotiation. conformance.tex | 2 + content.tex | 111 ++-- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/conformance.tex b/conformance.tex index 2b86fc6..bd0f463 100644 --- a/conformance.tex +++ b/conformance.tex @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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} @@ -415,6 +416,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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 e863709..74c36fe 100644 --- a/content.tex +++ b/content.tex @@ -3084,6 +3084,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 (52)] Device supports splitting the protocol +header and the payload. + \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing. \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets. @@ -3140,6 +3143,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device \item[VIRTIO_NET_F_NOTF_COAL] 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} @@ -3371,6 +3375,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O #define VIRTIO_NET_HDR_F_NEEDS_CSUM1 #define VIRTIO_NET_HDR_F_DATA_VALID2 #define VIRTIO_NET_HDR_F_RSC_INFO 4 +#define VIRTIO_NET_HDR_F_SPLIT_HEADER 8 u8 flags; #define VIRTIO_NET_HDR_GSO_NONE0 #define VIRTIO_NET_HDR_GSO_TCPV4 1 @@ -3799,9 +3804,10 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network not set VIRTIO_NET_HDR_F_RSC_INFO bit in \field{flags}. If one of the VIRTIO_NET_F_GUEST_TSO4,
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
On Thu, Aug 04 2022, Heng Qi wrote: > 在 2022/8/4 下午2:27, Jason Wang 写道: >> On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: >>> @@ -3820,9 +3826,13 @@ \subsubsection{Processing of Incoming >>> Packets}\label{sec:Device Types / Network >>> driver MUST NOT use the \field{csum_start} and \field{csum_offset}. >>> >>> If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options >>> have >>> -been negotiated, the driver MAY use \field{hdr_len} only as a hint about >>> the >>> +been negotiated and the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} >>> +is not set, the driver MAY use \field{hdr_len} only as a hint about the >>> transport header size. >>> -The driver MUST NOT rely on \field{hdr_len} to be correct. >>> + >>> +If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is not set, the >>> driver >>> +MUST NOT rely on \field{hdr_len} to be correct. >> I think we should keep the above description as-is. For whatever case, >> the driver must not trust the metadata set by the device and must >> perform necessary sanity tests on them. > > > My idea is to keep the current description as it is, > but to emphasize in the next version: > "If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, > the driver MAY treat the \field{hdr_len} as the length of the > protocol header inside the first descriptor." Just to be clear, you suggest using "If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have been negotiated, the driver MAY use \field{hdr_len} only as a hint about the transport header size. The driver MUST NOT rely on \field{hdr_len} to be correct. If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MAY treat the \field{hdr_len} as the length of the protocol header inside the first descriptor." (Maybe "...the driver MAY use \field{hdr_len} as a hint about the length of the protocol header..."? It's still not reliable, right?) > > >> >>> + >>> \begin{note} >>> This is due to various bugs in implementations. >>> \end{note} - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
在 2022/8/4 下午9:50, Cornelia Huck 写道: On Thu, Aug 04 2022, Heng Qi wrote: 在 2022/8/4 下午2:27, Jason Wang 写道: On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: @@ -3820,9 +3826,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network driver MUST NOT use the \field{csum_start} and \field{csum_offset}. If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have -been negotiated, the driver MAY use \field{hdr_len} only as a hint about the +been negotiated and the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} +is not set, the driver MAY use \field{hdr_len} only as a hint about the transport header size. -The driver MUST NOT rely on \field{hdr_len} to be correct. + +If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is not set, the driver +MUST NOT rely on \field{hdr_len} to be correct. I think we should keep the above description as-is. For whatever case, the driver must not trust the metadata set by the device and must perform necessary sanity tests on them. My idea is to keep the current description as it is, but to emphasize in the next version: "If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MAY treat the \field{hdr_len} as the length of the protocol header inside the first descriptor." Just to be clear, you suggest using "If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have been negotiated, the driver MAY use \field{hdr_len} only as a hint about the transport header size. The driver MUST NOT rely on \field{hdr_len} to be correct. If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, the driver MAY treat the \field{hdr_len} as the length of the protocol header inside the first descriptor." Yes. I will use the above description to make it clearer in the next version. (Maybe "...the driver MAY use \field{hdr_len} as a hint about the length of the protocol header..."? It's still not reliable, right?) \field{hdr_len} is unreliable when VIRTIO_NET_F_SPLIT_HEADER is not negotiated. If VIRTIO_NET_F_SPLIT_HEADER is negotiated, "split header" MAY perform the split from the IP layer, so the protocol header and the transport header are different. so I think the "...the driver MAY use \field{hdr_len} only as a hint about the transport header size..." paragraph can be left as-is.
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
On Fri, Aug 05 2022, Heng Qi wrote: > 在 2022/8/4 下午9:50, Cornelia Huck 写道: >> On Thu, Aug 04 2022, Heng Qi wrote: >> >>> 在 2022/8/4 下午2:27, Jason Wang 写道: On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: > @@ -3820,9 +3826,13 @@ \subsubsection{Processing of Incoming > Packets}\label{sec:Device Types / Network >driver MUST NOT use the \field{csum_start} and \field{csum_offset}. > >If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options > have > -been negotiated, the driver MAY use \field{hdr_len} only as a hint about > the > +been negotiated and the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in > \field{flags} > +is not set, the driver MAY use \field{hdr_len} only as a hint about the >transport header size. > -The driver MUST NOT rely on \field{hdr_len} to be correct. > + > +If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is not set, > the driver > +MUST NOT rely on \field{hdr_len} to be correct. I think we should keep the above description as-is. For whatever case, the driver must not trust the metadata set by the device and must perform necessary sanity tests on them. >>> >>> My idea is to keep the current description as it is, >>> but to emphasize in the next version: >>> "If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, >>> the driver MAY treat the \field{hdr_len} as the length of the >>> protocol header inside the first descriptor." >> >> Just to be clear, you suggest using >> >> "If one of the VIRTIO_NET_F_GUEST_TSO4, TSO6, UFO, USO4 or USO6 options have >> been negotiated, the driver MAY use \field{hdr_len} only as a hint about the >> transport header size. >> >> The driver MUST NOT rely on \field{hdr_len} to be correct. >> >> If the VIRTIO_NET_HDR_F_SPLIT_HEADER bit in \field{flags} is set, >> the driver MAY treat the \field{hdr_len} as the length of the >> protocol header inside the first descriptor." > > Yes. I will use the above description to make it clearer in the next version. > > >> >> (Maybe "...the driver MAY use \field{hdr_len} as a hint about the length >> of the protocol header..."? It's still not reliable, right?) >> > \field{hdr_len} is unreliable when VIRTIO_NET_F_SPLIT_HEADER is not > negotiated. > > > If VIRTIO_NET_F_SPLIT_HEADER is negotiated, "split header" MAY perform the > split > from the IP layer, so the protocol header and the transport header are > different. > > so I think the "...the driver MAY use \field{hdr_len} only as a hint about the > transport header size..." paragraph can be left as-is. Ok, then let's keep it like that. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
在 2022/8/9 下午5:18, Jason Wang 写道: On Thu, Aug 4, 2022 at 8:48 PM Heng Qi wrote: 在 2022/8/4 下午2:27, Jason Wang 写道: On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: From: 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 Signed-off-by: Heng Qi Reviewed-by: Kangjie Xu --- v6: 1. Fix some syntax issues. @Cornelia Huck 2. Clarify some paragraphs. @Cornelia Huck 3. Determine the device what to do if it does not perform header split on a packet. v5: 1. Determine when hdr_len is credible in the process of rx 2. Clean up the use of buffers and descriptors 3. Clarify the meaning of used lenght if the first descriptor is skipped in the case of merge v4: 1. fix typo @Cornelia Huck @Jason Wang 2. do not split header for IP fragmentation packet. @Jason Wang v3: 1. Fix some syntax issues 2. Fix some terminology issues 3. It is not unified with ip alignment, so ip alignment is not included 4. Make it clear that the device must support four types, in the case of successful negotiation. conformance.tex | 2 + content.tex | 111 ++-- 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/conformance.tex b/conformance.tex index 2b86fc6..bd0f463 100644 --- a/conformance.tex +++ b/conformance.tex @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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} @@ -415,6 +416,7 @@ \section{Conformance Targets}\label{sec:Conformance / Conformance Targets} \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 / Notifications Coalescing} +\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 e863709..74c36fe 100644 --- a/content.tex +++ b/content.tex @@ -3084,6 +3084,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 (52)] Device supports splitting the protocol +header and the payload. + \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing. \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets. @@ -3140,6 +3143,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device \item[VIRTIO_NET_F_NOTF_COAL] 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} @@ -3371,6 +3375,7 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O #define VIRTIO_NET_HDR_F_NEEDS_CSUM1 #define VIRTIO_NET_HDR_F_DATA_VALID2 #define VIRTIO_NET_HDR_F_RSC_INFO 4 +#define VIRTIO_NET_HDR_F_SPLIT_HEADER 8 u8 flags; #define VIRTIO_NET_HDR_GSO_NONE0 #define VIRTIO_NET_HDR_GSO_TCPV4 1 @@ -3799,9 +3804,10 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network not set VIRTIO
Re: [virtio-dev] Re: [PATCH] virtio_net: support split header
On Thu, 4 Aug 2022 21:01:30 +0800, Heng Qi wrote: > > 在 2022/8/4 下午2:27, Jason Wang 写道: > > On Mon, Aug 1, 2022 at 2:59 PM Heng Qi wrote: > >> From: 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 > >> Signed-off-by: Heng Qi > >> Reviewed-by: Kangjie Xu > >> --- > >> > >> v6: > >> 1. Fix some syntax issues. @Cornelia Huck > >> 2. Clarify some paragraphs. @Cornelia Huck > >> 3. Determine the device what to do if it does not perform header > >> split on a packet. > >> > >> v5: > >> 1. Determine when hdr_len is credible in the process of rx > >> 2. Clean up the use of buffers and descriptors > >> 3. Clarify the meaning of used lenght if the first descriptor is > >> skipped in the case of merge > >> > >> v4: > >> 1. fix typo @Cornelia Huck @Jason Wang > >> 2. do not split header for IP fragmentation packet. @Jason Wang > >> > >> v3: > >> 1. Fix some syntax issues > >> 2. Fix some terminology issues > >> 3. It is not unified with ip alignment, so ip alignment is not > >> included > >> 4. Make it clear that the device must support four types, in the case > >> of > >> successful negotiation. > >> > >> conformance.tex | 2 + > >> content.tex | 111 > >> ++-- > >> 2 files changed, 109 insertions(+), 4 deletions(-) > >> > >> diff --git a/conformance.tex b/conformance.tex > >> index 2b86fc6..bd0f463 100644 > >> --- a/conformance.tex > >> +++ b/conformance.tex > >> @@ -150,6 +150,7 @@ \section{Conformance Targets}\label{sec:Conformance / > >> Conformance Targets} > >> \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 / Notifications Coalescing} > >> +\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} > >> @@ -415,6 +416,7 @@ \section{Conformance Targets}\label{sec:Conformance / > >> Conformance Targets} > >> \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 / Notifications Coalescing} > >> +\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 e863709..74c36fe 100644 > >> --- a/content.tex > >> +++ b/content.tex > >> @@ -3084,6 +3084,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 (52)] Device supports splitting the > >> protocol > >> +header and the payload. > >> + > >> \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications > >> coalescing. > >> > >> \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets. > >> @@ -3140,6 +3143,7 @@ \subsubsection{Feature bit > >> requirements}\label{sec:Device Types / Network Device > >> \item[VIRTIO_NET_F_NOTF_COAL] 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