[virtio-comment] [PATCH v3] virtio-net: Add support for correct hdr_len field.

* [virtio-comment] [PATCH v3] virtio-net: Add support for correct hdr_len field.
@ 2019-11-05 15:26 Vitaly Mireyno
  2019-11-17  6:24 ` Vitaly Mireyno
  0 siblings, 1 reply; 3+ messages in thread
From: Vitaly Mireyno @ 2019-11-05 15:26 UTC (permalink / raw)
  To: virtio-comment; +Cc: Michael S. Tsirkin, Jason Wang

Incorporated more v1 comments

Changes from v2:
 * Added device implementation note for using hdr_len

Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
---
 content.tex | 56 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/content.tex b/content.tex
index b1ea9b9..5d3f865 100644
--- a/content.tex
+++ b/content.tex
@@ -2811,6 +2811,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_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
+    value. Device benefits from knowing the exact header length.
+
 \item[VIRTIO_NET_F_RSC_EXT(61)] Device can process duplicated ACKs
     and report number of coalesced segments and duplicated ACKs
 
@@ -3095,12 +3098,21 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
   into smaller packets. The other gso fields are set:
 
   \begin{itemize}
-  \item \field{hdr_len} is a hint to the device as to how much of the header
+  \item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
+    \field{hdr_len} indicates the header length that needs to be replicated
+    for each packet. It's a number of bytes from beginning of the packet
+    to beginning of the transport payload.
+    Otherwise, if the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
+    \field{hdr_len} is a hint to the device as to how much of the header
     needs to be kept to copy into each packet, usually set to the
     length of the headers, including the transport header\footnote{Due to various bugs in implementations, this field is not useful
 as a guarantee of the transport header size.
 }.
 
+  \begin{note}
+  Some devices benefit from knowledge of the exact header length.
+  \end{note}
+
   \item \field{gso_size} is the maximum size of each packet beyond that
     header (ie. MSS).
 
@@ -3173,9 +3185,20 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 desired MSS.
 
 If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
-been negotiated, the driver SHOULD set \field{hdr_len} to a value
-not less than the length of the headers, including the transport
-header.
+been negotiated:
+\begin{itemize}
+\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
+	the driver MUST set \field{hdr_len} to a value equal to the length
+	of the headers, including the transport header.
+
+\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
+	the driver SHOULD set \field{hdr_len} to a value
+	not less than the length of the headers, including the transport
+	header.
+\end{itemize}
+
+The driver SHOULD accept the VIRTIO_NET_F_GUEST_HDRLEN feature if it has
+been offered, and if it's able to provide the exact header length.
 
 The driver MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID and
 VIRTIO_NET_HDR_F_RSC_INFO bits in \field{flags}.
@@ -3187,12 +3210,25 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 device MUST NOT use the \field{csum_start} and \field{csum_offset}.
 
 If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
-been negotiated, the device MAY use \field{hdr_len} only as a hint about the
-transport header size.
-The device MUST NOT rely on \field{hdr_len} to be correct.
-\begin{note}
-This is due to various bugs in implementations.
-\end{note}
+been negotiated:
+\begin{itemize}
+\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
+	the device MAY use \field{hdr_len} as the transport header size.
+
+	\begin{note}
+	Caution should be taken by the implementation so as to prevent
+	malicious driver from attacking the device by setting incorrect hdr_len.
+	\end{note}
+
+\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
+	the device MAY use \field{hdr_len} only as a hint about the
+	transport header size.
+	The device MUST NOT rely on \field{hdr_len} to be correct.
+
+	\begin{note}
+	This is due to various bugs in implementations.
+	\end{note}
+\end{itemize}
 
 If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT
 rely on the packet checksum being correct.
--

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


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