All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8] virtio-net: support inner header hash
@ 2023-02-08  9:08 Heng Qi
  2023-02-13 13:05 ` [virtio-comment] " Heng Qi
  2023-02-13 22:20 ` Michael S. Tsirkin
  0 siblings, 2 replies; 20+ messages in thread
From: Heng Qi @ 2023-02-08  9:08 UTC (permalink / raw)
  To: virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Parav Pandit, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo

If the tunnel is used to encapsulate the packets, the hash calculated
using the outer header of the receive packets is always fixed for the
same flow packets, i.e. they will be steered to the same receive queue.

We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
in \field{hash_tunnel_types}, which instructs the device to calculate the
hash using the inner headers of tunnel-encapsulated packets. Besides,
values in \field{hash_report_tunnel_types} are added to report tunnel types.

Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
header hash, and does not give the device the ability to use the hash value
to select a receiving queue to place the packet.

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151

Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
v7->v8:
	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
	3. Removed re-definition for inner packet hashing. @Parav Pandit
	4. Fix some typos. @Michael S . Tsirkin
	5. Clarify some sentences. @Michael S . Tsirkin

v6->v7:
	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
	2. Fix some syntax issues. @Michael S. Tsirkin

v5->v6:
	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
	3. Move the links to introduction section. @Michael S. Tsirkin
	4. Clarify some sentences. @Michael S. Tsirkin

v4->v5:
	1. Clarify some paragraphs. @Cornelia Huck
	2. Fix the u8 type. @Cornelia Huck

v3->v4:
	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin

v2->v3:
	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin

v1->v2:
	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
	2. Clarify some paragraphs. @Jason Wang
	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich

 content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
 introduction.tex |  19 +++++++
 2 files changed, 140 insertions(+), 14 deletions(-)

diff --git a/content.tex b/content.tex
index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
+	for tunnel-encapsulated packets.
+
 \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
 
 \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
@@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
  to several segments when each of these smaller packets has UDP header.
 
 \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
-    value and a type of calculated hash.
+    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
+    is negotiated, an encapsulation packet type.
 
 \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
     value. Device benefits from knowing the exact header length.
@@ -3140,6 +3144,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_HASH_TUNNEL] 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}
@@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
         u8 rss_max_key_size;
         le16 rss_max_indirection_table_length;
         le32 supported_hash_types;
+        le32 supported_tunnel_hash_types;
 };
 \end{lstlisting}
-The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
+The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
 It specifies the maximum supported length of RSS key in bytes.
 
 The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
 It specifies the maximum number of 16-bit entries in RSS indirection table.
 
 The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
-i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
+i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
 
 Field \field{supported_hash_types} contains the bitmask of supported hash types.
 See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
 
+The next field, \field{supported_tunnel_hash_types} only exists if the device
+supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
+
+Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
+See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
+
 \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
 
 The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
@@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
 negotiated.
 
 The device MUST set \field{rss_max_key_size} to at least 40, if it offers
-VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
+VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
 
 The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
 VIRTIO_NET_F_RSS.
@@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
         le16 csum_start;
         le16 csum_offset;
         le16 num_buffers;
-        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
-        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
-        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
+        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
+        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
+        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
+        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
 };
 \end{lstlisting}
 
@@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 \begin{itemize}
 \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
 \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
+	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
+\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
 \end{itemize}
 
 If the feature VIRTIO_NET_F_RSS was negotiated:
 \begin{itemize}
 \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
+	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
+	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
 \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
 \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
 \end{itemize}
@@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 If the feature VIRTIO_NET_F_RSS was not negotiated:
 \begin{itemize}
 \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
+	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
+	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
 \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
 \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
 \end{itemize}
 
-Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
+Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
 at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
 \begin{itemize}
 \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
@@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
  \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
 \end{itemize}
 
+\subparagraph{Tunnel/Encapsulated packet}
+\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
+A tunnel packet is encapsulated from the original packet based on the tunneling
+protocol (only a single level of encapsulation is currently supported). The
+encapsulated packet contains an outer header and an inner header, and the device
+calculates the hash over either the inner header or the outer header.
+
+When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
+encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
+type of encapsulated packet is calculated over the inner as opposed to outer header.
+Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
+Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
+Supported/enabled hash tunnel types}.
+
+If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
+and the corresponding encapsulation type is set in \field{hash_tunnel_types},
+the device supports inner hash calculation for the encapsulated packet,
+For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
+\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
+headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
+respectively, and \field{hash_report_tunnel_types} should be respectively set to
+VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
+
+If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
+negotiated, the device calculates the hash over the outer header, and \field{hash_report}
+reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
+
 \subparagraph{Supported/enabled hash types}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
+This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
+\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
 Hash types applicable for IPv4 packets:
 \begin{lstlisting}
 #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
@@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
 \end{lstlisting}
 
+\subparagraph{Supported/enabled tunnel hash types}
+\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
+If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
+hash type indicates that the hash is calculated over the inner header of
+the encapsulated packet:
+Hash type applicable for inner payload of the gre-encapsulated packet
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
+\end{lstlisting}
+Hash type applicable for inner payload of the vxlan-encapsulated packet
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
+\end{lstlisting}
+Hash type applicable for inner payload of the geneve-encapsulated packet
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
+\end{lstlisting}
+
 \subparagraph{IPv4 packets}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
 The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
@@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
 \end{itemize}
 
+\subparagraph{Inner hash calculation of an encapsulated packet}
+If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
+encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
+hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
+/ Network Device / Device Operation / Processing of Incoming Packets /
+Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
+\field{hash_report_tunnel_types} contains the valid outer tunnel type.
+
 \paragraph{Hash reporting for incoming packets}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
-
-If VIRTIO_NET_F_HASH_REPORT was negotiated and
- the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
-and \field{hash_value} with the value of calculated hash.
+If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
+hash for the packet, the device fills \field{hash_report} with the report type
+of calculated hash, and \field{hash_value} with the value of calculated hash.
+Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
+\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
+and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
 
 If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
-hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
+hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
+and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
 
 Possible values that the device can report in \field{hash_report} are defined below.
 They correspond to supported hash types defined in
@@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
 \end{lstlisting}
 
+\field{hash_report_tunnel} can report the type of the encapsulated
+packet to the driver when the inner header hash is calculated.
+Possible values that the device can report in \field{hash_report_tunnel_types}
+are defined below.
+They correspond to supported hash tunnel types defined in
+\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
+as follows:
+
+VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
+
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
+#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
+#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
+#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
+\end{lstlisting}
+
+They correspond to supported hash types defined in
+\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
+
 \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
 
 The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
@@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \begin{lstlisting}
 struct virtio_net_hash_config {
     le32 hash_types;
+    le32 hash_tunnel_types;
     le16 reserved[4];
     u8 hash_key_length;
     u8 hash_key_data[hash_key_length];
@@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 Field \field{hash_types} contains a bitmask of allowed hash types as
 defined in
 \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
-Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
+
+Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
+defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
+
+Initially the device has all hash types and hash tunnel types disabled and reports only
+VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
 
 Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
 defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
@@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 \begin{lstlisting}
 struct virtio_net_rss_config {
     le32 hash_types;
+    le32 hash_tunnel_types;
     le16 indirection_table_mask;
     le16 unclassified_queue;
     le16 indirection_table[indirection_table_length];
@@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 defined in
 \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
 
+Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
+defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
+
 Field \field{indirection_table_mask} is a mask to be applied to
 the calculated hash to produce an index in the
 \field{indirection_table} array.
diff --git a/introduction.tex b/introduction.tex
index 287c5fc..ff01a9b 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
 	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
     Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
 	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
+	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
+	Generic Routing Encapsulation
+	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
+	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
+	Virtual eXtensible Local Area Network
+	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
+	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
+	Generic Network Virtualization Encapsulation
+	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
+	\phantomsection\label{intro:IP}\textbf{[IP]} &
+	INTERNET PROTOCOL
+	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
+	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
+	User Datagram Protocol
+	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
+	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
+	TRANSMISSION CONTROL PROTOCOL
+	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
+
 
 \end{longtable}
 
-- 
2.19.1.6.gb485710b


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

* Re: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-08  9:08 [PATCH v8] virtio-net: support inner header hash Heng Qi
@ 2023-02-13 13:05 ` Heng Qi
  2023-02-13 20:43   ` Michael S. Tsirkin
  2023-02-13 21:42   ` Parav Pandit
  2023-02-13 22:20 ` Michael S. Tsirkin
  1 sibling, 2 replies; 20+ messages in thread
From: Heng Qi @ 2023-02-13 13:05 UTC (permalink / raw)
  To: virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Parav Pandit, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo

Hi, all.

Do you have any comments on this version?

Thanks.

在 2023/2/8 下午5:08, Heng Qi 写道:
> If the tunnel is used to encapsulate the packets, the hash calculated
> using the outer header of the receive packets is always fixed for the
> same flow packets, i.e. they will be steered to the same receive queue.
>
> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> in \field{hash_tunnel_types}, which instructs the device to calculate the
> hash using the inner headers of tunnel-encapsulated packets. Besides,
> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>
> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> header hash, and does not give the device the ability to use the hash value
> to select a receiving queue to place the packet.
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
>
> Reviewed-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> v7->v8:
> 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> 	4. Fix some typos. @Michael S . Tsirkin
> 	5. Clarify some sentences. @Michael S . Tsirkin
>
> v6->v7:
> 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> 	2. Fix some syntax issues. @Michael S. Tsirkin
>
> v5->v6:
> 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> 	3. Move the links to introduction section. @Michael S. Tsirkin
> 	4. Clarify some sentences. @Michael S. Tsirkin
>
> v4->v5:
> 	1. Clarify some paragraphs. @Cornelia Huck
> 	2. Fix the u8 type. @Cornelia Huck
>
> v3->v4:
> 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
>
> v2->v3:
> 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
>
> v1->v2:
> 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> 	2. Clarify some paragraphs. @Jason Wang
> 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
>
>   content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
>   introduction.tex |  19 +++++++
>   2 files changed, 140 insertions(+), 14 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> +	for tunnel-encapsulated packets.
> +
>   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>   
>   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>    to several segments when each of these smaller packets has UDP header.
>   
>   \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> -    value and a type of calculated hash.
> +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> +    is negotiated, an encapsulation packet type.
>   
>   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>       value. Device benefits from knowing the exact header length.
> @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>           u8 rss_max_key_size;
>           le16 rss_max_indirection_table_length;
>           le32 supported_hash_types;
> +        le32 supported_tunnel_hash_types;
>   };
>   \end{lstlisting}
> -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>   It specifies the maximum supported length of RSS key in bytes.
>   
>   The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
>   It specifies the maximum number of 16-bit entries in RSS indirection table.
>   
>   The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>   
>   Field \field{supported_hash_types} contains the bitmask of supported hash types.
>   See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
>   
> +The next field, \field{supported_tunnel_hash_types} only exists if the device
> +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> +
> +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> +
>   \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>   
>   The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>   negotiated.
>   
>   The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
>   
>   The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
>   VIRTIO_NET_F_RSS.
> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>           le16 csum_start;
>           le16 csum_offset;
>           le16 num_buffers;
> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>   };
>   \end{lstlisting}
>   
> @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   \begin{itemize}
>   \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
>   \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
>   \end{itemize}
>   
>   If the feature VIRTIO_NET_F_RSS was negotiated:
>   \begin{itemize}
>   \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
>   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
>   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
>   \end{itemize}
> @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   If the feature VIRTIO_NET_F_RSS was not negotiated:
>   \begin{itemize}
>   \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
>   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
>   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
>   \end{itemize}
>   
> -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
>   at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
>   \begin{itemize}
>   \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
>   \end{itemize}
>   
> +\subparagraph{Tunnel/Encapsulated packet}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +A tunnel packet is encapsulated from the original packet based on the tunneling
> +protocol (only a single level of encapsulation is currently supported). The
> +encapsulated packet contains an outer header and an inner header, and the device
> +calculates the hash over either the inner header or the outer header.
> +
> +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> +type of encapsulated packet is calculated over the inner as opposed to outer header.
> +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> +Supported/enabled hash tunnel types}.
> +
> +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> +the device supports inner hash calculation for the encapsulated packet,
> +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> +
> +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> +
>   \subparagraph{Supported/enabled hash types}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>   Hash types applicable for IPv4 packets:
>   \begin{lstlisting}
>   #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
>   \end{lstlisting}
>   
> +\subparagraph{Supported/enabled tunnel hash types}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> +hash type indicates that the hash is calculated over the inner header of
> +the encapsulated packet:
> +Hash type applicable for inner payload of the gre-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the vxlan-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the geneve-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> +\end{lstlisting}
> +
>   \subparagraph{IPv4 packets}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
>   The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>   \end{itemize}
>   
> +\subparagraph{Inner hash calculation of an encapsulated packet}
> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> +/ Network Device / Device Operation / Processing of Incoming Packets /
> +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> +
>   \paragraph{Hash reporting for incoming packets}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> -
> -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> -and \field{hash_value} with the value of calculated hash.
> +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> +hash for the packet, the device fills \field{hash_report} with the report type
> +of calculated hash, and \field{hash_value} with the value of calculated hash.
> +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
>   
>   If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
>   
>   Possible values that the device can report in \field{hash_report} are defined below.
>   They correspond to supported hash types defined in
> @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
>   \end{lstlisting}
>   
> +\field{hash_report_tunnel} can report the type of the encapsulated
> +packet to the driver when the inner header hash is calculated.
> +Possible values that the device can report in \field{hash_report_tunnel_types}
> +are defined below.
> +They correspond to supported hash tunnel types defined in
> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> +as follows:
> +
> +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> +
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> +\end{lstlisting}
> +
> +They correspond to supported hash types defined in
> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> +
>   \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
>   
>   The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>   \begin{lstlisting}
>   struct virtio_net_hash_config {
>       le32 hash_types;
> +    le32 hash_tunnel_types;
>       le16 reserved[4];
>       u8 hash_key_length;
>       u8 hash_key_data[hash_key_length];
> @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>   Field \field{hash_types} contains a bitmask of allowed hash types as
>   defined in
>   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> +
> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> +
> +Initially the device has all hash types and hash tunnel types disabled and reports only
> +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
>   
>   Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
>   defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>   \begin{lstlisting}
>   struct virtio_net_rss_config {
>       le32 hash_types;
> +    le32 hash_tunnel_types;
>       le16 indirection_table_mask;
>       le16 unclassified_queue;
>       le16 indirection_table[indirection_table_length];
> @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>   defined in
>   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>   
> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> +
>   Field \field{indirection_table_mask} is a mask to be applied to
>   the calculated hash to produce an index in the
>   \field{indirection_table} array.
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..ff01a9b 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
>   	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>       Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
>   	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> +	Generic Routing Encapsulation
> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> +	Virtual eXtensible Local Area Network
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> +	Generic Network Virtualization Encapsulation
> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> +	INTERNET PROTOCOL
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> +	User Datagram Protocol
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> +	TRANSMISSION CONTROL PROTOCOL
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> +
>   
>   \end{longtable}
>   


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

* Re: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 13:05 ` [virtio-comment] " Heng Qi
@ 2023-02-13 20:43   ` Michael S. Tsirkin
  2023-02-13 22:29     ` Parav Pandit
  2023-02-16  6:48     ` [virtio-comment] Re: [virtio-dev] " Heng Qi
  2023-02-13 21:42   ` Parav Pandit
  1 sibling, 2 replies; 20+ messages in thread
From: Michael S. Tsirkin @ 2023-02-13 20:43 UTC (permalink / raw)
  To: Heng Qi
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo

On Mon, Feb 13, 2023 at 09:05:04PM +0800, Heng Qi wrote:
> Hi, all.
> 
> Do you have any comments on this version?
> 
> Thanks.
> 
> 在 2023/2/8 下午5:08, Heng Qi 写道:
> > If the tunnel is used to encapsulate the packets, the hash calculated
> > using the outer header of the receive packets is always fixed for the
> > same flow packets, i.e. they will be steered to the same receive queue.
> > 
> > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> > in \field{hash_tunnel_types}, which instructs the device to calculate the
> > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> > 
> > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> > header hash, and does not give the device the ability to use the hash value
> > to select a receiving queue to place the packet.
> > 
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
> > 
> > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>

I didn't yes come up with anything convincing for improving
security (dos/usage info leak due to queue collisions between
tunnels).

I'd like to at least have a security considerations section
documenting the issue with using RSS with tunneling.
In fact it's high time we started adding these sections all over
the place.

I also found out at least mlx5 has these tunnel offloads,
so I'd like to ask nvidia guys here on list
- does this cover e.g. mlx5 functionality? e.g. is the tunnel list
  sufficient?
- does mlx5 cover queue collisions between tunnels and if yes how?

Thanks!


> > ---
> > v7->v8:
> > 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> > 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> > 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> > 	4. Fix some typos. @Michael S . Tsirkin
> > 	5. Clarify some sentences. @Michael S . Tsirkin
> > 
> > v6->v7:
> > 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> > 	2. Fix some syntax issues. @Michael S. Tsirkin
> > 
> > v5->v6:
> > 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> > 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> > 	3. Move the links to introduction section. @Michael S. Tsirkin
> > 	4. Clarify some sentences. @Michael S. Tsirkin
> > 
> > v4->v5:
> > 	1. Clarify some paragraphs. @Cornelia Huck
> > 	2. Fix the u8 type. @Cornelia Huck
> > 
> > v3->v4:
> > 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> > 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> > 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> > 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
> > 
> > v2->v3:
> > 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> > 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
> > 
> > v1->v2:
> > 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> > 	2. Clarify some paragraphs. @Jason Wang
> > 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
> > 
> >   content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
> >   introduction.tex |  19 +++++++
> >   2 files changed, 140 insertions(+), 14 deletions(-)
> > 
> > diff --git a/content.tex b/content.tex
> > index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> > +	for tunnel-encapsulated packets.
> > +
> >   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
> >   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> >    to several segments when each of these smaller packets has UDP header.
> >   \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> > -    value and a type of calculated hash.
> > +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> > +    is negotiated, an encapsulation packet type.
> >   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
> >       value. Device benefits from knowing the exact header length.
> > @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> > @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> >           u8 rss_max_key_size;
> >           le16 rss_max_indirection_table_length;
> >           le32 supported_hash_types;
> > +        le32 supported_tunnel_hash_types;
> >   };
> >   \end{lstlisting}
> > -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> >   It specifies the maximum supported length of RSS key in bytes.
> >   The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
> >   It specifies the maximum number of 16-bit entries in RSS indirection table.
> >   The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> > -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> >   Field \field{supported_hash_types} contains the bitmask of supported hash types.
> >   See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
> > +The next field, \field{supported_tunnel_hash_types} only exists if the device
> > +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> > +
> > +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> > +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> > +
> >   \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> >   The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> > @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> >   negotiated.
> >   The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> > -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
> >   The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
> >   VIRTIO_NET_F_RSS.
> > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> >           le16 csum_start;
> >           le16 csum_offset;
> >           le16 num_buffers;
> > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> > +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> >   };
> >   \end{lstlisting}
> > @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >   \begin{itemize}
> >   \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
> >   \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> > +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
> >   \end{itemize}
> >   If the feature VIRTIO_NET_F_RSS was negotiated:
> >   \begin{itemize}
> >   \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
> >   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
> >   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
> >   \end{itemize}
> > @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >   If the feature VIRTIO_NET_F_RSS was not negotiated:
> >   \begin{itemize}
> >   \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
> >   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
> >   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
> >   \end{itemize}
> > -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> > +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
> >   at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
> >   \begin{itemize}
> >   \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> > @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
> >   \end{itemize}
> > +\subparagraph{Tunnel/Encapsulated packet}
> > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> > +A tunnel packet is encapsulated from the original packet based on the tunneling
> > +protocol (only a single level of encapsulation is currently supported). The
> > +encapsulated packet contains an outer header and an inner header, and the device
> > +calculates the hash over either the inner header or the outer header.
> > +
> > +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> > +type of encapsulated packet is calculated over the inner as opposed to outer header.
> > +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> > +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> > +Supported/enabled hash tunnel types}.
> > +
> > +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> > +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> > +the device supports inner hash calculation for the encapsulated packet,
> > +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> > +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> > +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> > +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> > +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> > +
> > +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> > +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> > +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> > +
> >   \subparagraph{Supported/enabled hash types}
> >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> > +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> > +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
> >   Hash types applicable for IPv4 packets:
> >   \begin{lstlisting}
> >   #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> > @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >   #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
> >   \end{lstlisting}
> > +\subparagraph{Supported/enabled tunnel hash types}
> > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> > +hash type indicates that the hash is calculated over the inner header of
> > +the encapsulated packet:
> > +Hash type applicable for inner payload of the gre-encapsulated packet
> > +\begin{lstlisting}
> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> > +\end{lstlisting}
> > +Hash type applicable for inner payload of the vxlan-encapsulated packet
> > +\begin{lstlisting}
> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> > +\end{lstlisting}
> > +Hash type applicable for inner payload of the geneve-encapsulated packet
> > +\begin{lstlisting}
> > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> > +\end{lstlisting}
> > +
> >   \subparagraph{IPv4 packets}
> >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
> >   The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> > @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >   (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
> >   \end{itemize}
> > +\subparagraph{Inner hash calculation of an encapsulated packet}
> > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> > +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> > +/ Network Device / Device Operation / Processing of Incoming Packets /
> > +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> > +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> > +
> >   \paragraph{Hash reporting for incoming packets}
> >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> > -
> > -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> > - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> > -and \field{hash_value} with the value of calculated hash.
> > +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> > +hash for the packet, the device fills \field{hash_report} with the report type
> > +of calculated hash, and \field{hash_value} with the value of calculated hash.
> > +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> > +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> > +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
> >   If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> > -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> > +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> > +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
> >   Possible values that the device can report in \field{hash_report} are defined below.
> >   They correspond to supported hash types defined in
> > @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> >   #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
> >   \end{lstlisting}
> > +\field{hash_report_tunnel} can report the type of the encapsulated
> > +packet to the driver when the inner header hash is calculated.
> > +Possible values that the device can report in \field{hash_report_tunnel_types}
> > +are defined below.
> > +They correspond to supported hash tunnel types defined in
> > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> > +as follows:
> > +
> > +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> > +\end{lstlisting}
> > +
> > +They correspond to supported hash types defined in
> > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > +
> >   \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
> >   The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> > @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >   \begin{lstlisting}
> >   struct virtio_net_hash_config {
> >       le32 hash_types;
> > +    le32 hash_tunnel_types;
> >       le16 reserved[4];
> >       u8 hash_key_length;
> >       u8 hash_key_data[hash_key_length];
> > @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >   Field \field{hash_types} contains a bitmask of allowed hash types as
> >   defined in
> >   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> > +
> > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > +
> > +Initially the device has all hash types and hash tunnel types disabled and reports only
> > +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
> >   Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
> >   defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> > @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >   \begin{lstlisting}
> >   struct virtio_net_rss_config {
> >       le32 hash_types;
> > +    le32 hash_tunnel_types;
> >       le16 indirection_table_mask;
> >       le16 unclassified_queue;
> >       le16 indirection_table[indirection_table_length];
> > @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> >   defined in
> >   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > +
> >   Field \field{indirection_table_mask} is a mask to be applied to
> >   the calculated hash to produce an index in the
> >   \field{indirection_table} array.
> > diff --git a/introduction.tex b/introduction.tex
> > index 287c5fc..ff01a9b 100644
> > --- a/introduction.tex
> > +++ b/introduction.tex
> > @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
> >   	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> >       Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
> >   	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> > +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> > +	Generic Routing Encapsulation
> > +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> > +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> > +	Virtual eXtensible Local Area Network
> > +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> > +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> > +	Generic Network Virtualization Encapsulation
> > +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> > +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> > +	INTERNET PROTOCOL
> > +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> > +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> > +	User Datagram Protocol
> > +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> > +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> > +	TRANSMISSION CONTROL PROTOCOL
> > +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> > +
> >   \end{longtable}


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

* RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 13:05 ` [virtio-comment] " Heng Qi
  2023-02-13 20:43   ` Michael S. Tsirkin
@ 2023-02-13 21:42   ` Parav Pandit
  2023-02-13 22:23     ` Michael S. Tsirkin
  2023-02-16  6:57     ` [virtio-dev] " Heng Qi
  1 sibling, 2 replies; 20+ messages in thread
From: Parav Pandit @ 2023-02-13 21:42 UTC (permalink / raw)
  To: Heng Qi, virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Cornelia Huck, Yuri Benditovich,
	Xuan Zhuo



> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Monday, February 13, 2023 8:05 AM
> Hi, all.
> 
> Do you have any comments on this version?
> 
> Thanks.
> 
> 在 2023/2/8 下午5:08, Heng Qi 写道:
> > If the tunnel is used to encapsulate the packets, the hash calculated
> > using the outer header of the receive packets is always fixed for the
> > same flow packets, i.e. they will be steered to the same receive queue.
> >
> > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks in
> > \field{hash_tunnel_types}, which instructs the device to calculate the
> > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> >
> > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the
> > inner header hash, and does not give the device the ability to use the
> > hash value to select a receiving queue to place the packet.
> >
[..]
> > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device
> Types / Network Device / Device O
> >           le16 csum_start;
> >           le16 csum_offset;
> >           le16 num_buffers;
> > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT

I am yet to review the changes of v8.
But the quick response is, I do not see a use case of above field by sw driver.
And this addition requires the core hw data path to supply this value.
Without good motivation, it is hard to have it here.

What is valuable is to have a VNI already identified and coming to the driver, like a hash value.
This can cut down the cpu processing power, in outer header packet processing.
However, that is relatively a different feature than inner hash.

So, my input is to omit hash_report_tunnel_types.
Will respond to Michael question in other thread.


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

* Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-08  9:08 [PATCH v8] virtio-net: support inner header hash Heng Qi
  2023-02-13 13:05 ` [virtio-comment] " Heng Qi
@ 2023-02-13 22:20 ` Michael S. Tsirkin
  2023-02-16  7:20   ` [virtio-dev] " Heng Qi
  1 sibling, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2023-02-13 22:20 UTC (permalink / raw)
  To: Heng Qi
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo

On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
> If the tunnel is used to encapsulate the packets, the hash calculated
> using the outer header of the receive packets is always fixed for the
> same flow packets, i.e. they will be steered to the same receive queue.
> 
> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> in \field{hash_tunnel_types}, which instructs the device to calculate the
> hash using the inner headers of tunnel-encapsulated packets. Besides,
> values in \field{hash_report_tunnel_types} are added to report tunnel types.
> 
> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> header hash, and does not give the device the ability to use the hash value
> to select a receiving queue to place the packet.

This is the part I am missing. Where is this in the proposal?
We currently have:

The device MUST determine the destination queue for a network packet as follows:
\begin{itemize}
\item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
\item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
\item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).





> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
> 
> Reviewed-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> v7->v8:
> 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> 	4. Fix some typos. @Michael S . Tsirkin
> 	5. Clarify some sentences. @Michael S . Tsirkin
> 
> v6->v7:
> 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> 	2. Fix some syntax issues. @Michael S. Tsirkin
> 
> v5->v6:
> 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> 	3. Move the links to introduction section. @Michael S. Tsirkin
> 	4. Clarify some sentences. @Michael S. Tsirkin
> 
> v4->v5:
> 	1. Clarify some paragraphs. @Cornelia Huck
> 	2. Fix the u8 type. @Cornelia Huck
> 
> v3->v4:
> 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
> 
> v2->v3:
> 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
> 
> v1->v2:
> 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> 	2. Clarify some paragraphs. @Jason Wang
> 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
> 
>  content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
>  introduction.tex |  19 +++++++
>  2 files changed, 140 insertions(+), 14 deletions(-)
> 
> diff --git a/content.tex b/content.tex
> index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> +	for tunnel-encapsulated packets.
> +
>  \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>  
>  \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>   to several segments when each of these smaller packets has UDP header.
>  
>  \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> -    value and a type of calculated hash.
> +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> +    is negotiated, an encapsulation packet type.
>  
>  \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>      value. Device benefits from knowing the exact header length.
> @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>          u8 rss_max_key_size;
>          le16 rss_max_indirection_table_length;
>          le32 supported_hash_types;
> +        le32 supported_tunnel_hash_types;
>  };
>  \end{lstlisting}
> -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>  It specifies the maximum supported length of RSS key in bytes.
>  
>  The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
>  It specifies the maximum number of 16-bit entries in RSS indirection table.
>  
>  The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>  
>  Field \field{supported_hash_types} contains the bitmask of supported hash types.
>  See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
>  
> +The next field, \field{supported_tunnel_hash_types} only exists if the device
> +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> +
> +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> +
>  \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>  
>  The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>  negotiated.
>  
>  The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
>  
>  The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
>  VIRTIO_NET_F_RSS.
> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>          le16 csum_start;
>          le16 csum_offset;
>          le16 num_buffers;
> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>  };
>  \end{lstlisting}
>  
> @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>  \begin{itemize}
>  \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
>  \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
>  \end{itemize}
>  
>  If the feature VIRTIO_NET_F_RSS was negotiated:
>  \begin{itemize}
>  \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
>  \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
>  \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
>  \end{itemize}
> @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>  If the feature VIRTIO_NET_F_RSS was not negotiated:
>  \begin{itemize}
>  \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
>  \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
>  \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
>  \end{itemize}
>  
> -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
>  at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
>  \begin{itemize}
>  \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
>  \end{itemize}
>  
> +\subparagraph{Tunnel/Encapsulated packet}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +A tunnel packet is encapsulated from the original packet based on the tunneling
> +protocol (only a single level of encapsulation is currently supported). The
> +encapsulated packet contains an outer header and an inner header, and the device
> +calculates the hash over either the inner header or the outer header.
> +
> +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> +type of encapsulated packet is calculated over the inner as opposed to outer header.
> +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> +Supported/enabled hash tunnel types}.
> +
> +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> +the device supports inner hash calculation for the encapsulated packet,
> +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> +
> +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> +
>  \subparagraph{Supported/enabled hash types}
>  \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>  Hash types applicable for IPv4 packets:
>  \begin{lstlisting}
>  #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>  #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
>  \end{lstlisting}
>  
> +\subparagraph{Supported/enabled tunnel hash types}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> +hash type indicates that the hash is calculated over the inner header of
> +the encapsulated packet:
> +Hash type applicable for inner payload of the gre-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the vxlan-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the geneve-encapsulated packet
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> +\end{lstlisting}
> +
>  \subparagraph{IPv4 packets}
>  \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
>  The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>  (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>  \end{itemize}
>  
> +\subparagraph{Inner hash calculation of an encapsulated packet}
> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> +/ Network Device / Device Operation / Processing of Incoming Packets /
> +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> +
>  \paragraph{Hash reporting for incoming packets}
>  \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> -
> -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> -and \field{hash_value} with the value of calculated hash.
> +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> +hash for the packet, the device fills \field{hash_report} with the report type
> +of calculated hash, and \field{hash_value} with the value of calculated hash.
> +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
>  
>  If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
>  
>  Possible values that the device can report in \field{hash_report} are defined below.
>  They correspond to supported hash types defined in
> @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>  #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
>  \end{lstlisting}
>  
> +\field{hash_report_tunnel} can report the type of the encapsulated
> +packet to the driver when the inner header hash is calculated.
> +Possible values that the device can report in \field{hash_report_tunnel_types}
> +are defined below.
> +They correspond to supported hash tunnel types defined in
> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> +as follows:
> +
> +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> +
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> +\end{lstlisting}
> +
> +They correspond to supported hash types defined in
> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> +
>  \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
>  
>  The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \begin{lstlisting}
>  struct virtio_net_hash_config {
>      le32 hash_types;
> +    le32 hash_tunnel_types;
>      le16 reserved[4];
>      u8 hash_key_length;
>      u8 hash_key_data[hash_key_length];
> @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  Field \field{hash_types} contains a bitmask of allowed hash types as
>  defined in
>  \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> +
> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> +
> +Initially the device has all hash types and hash tunnel types disabled and reports only
> +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
>  
>  Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
>  defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  \begin{lstlisting}
>  struct virtio_net_rss_config {
>      le32 hash_types;
> +    le32 hash_tunnel_types;
>      le16 indirection_table_mask;
>      le16 unclassified_queue;
>      le16 indirection_table[indirection_table_length];
> @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  defined in
>  \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>  
> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> +
>  Field \field{indirection_table_mask} is a mask to be applied to
>  the calculated hash to produce an index in the
>  \field{indirection_table} array.
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..ff01a9b 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
>  	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>      Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
>  	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> +	Generic Routing Encapsulation
> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> +	Virtual eXtensible Local Area Network
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> +	Generic Network Virtualization Encapsulation
> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> +	INTERNET PROTOCOL
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> +	User Datagram Protocol
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> +	TRANSMISSION CONTROL PROTOCOL
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> +
>  
>  \end{longtable}
>  
> -- 
> 2.19.1.6.gb485710b


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

* Re: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 21:42   ` Parav Pandit
@ 2023-02-13 22:23     ` Michael S. Tsirkin
  2023-02-16  7:23       ` [virtio-comment] Re: [virtio-dev] " Heng Qi
  2023-02-16  6:57     ` [virtio-dev] " Heng Qi
  1 sibling, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2023-02-13 22:23 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Heng Qi, virtio-comment, virtio-dev, Jason Wang, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo

On Mon, Feb 13, 2023 at 09:42:25PM +0000, Parav Pandit wrote:
> 
> 
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Monday, February 13, 2023 8:05 AM
> > Hi, all.
> > 
> > Do you have any comments on this version?
> > 
> > Thanks.
> > 
> > 在 2023/2/8 下午5:08, Heng Qi 写道:
> > > If the tunnel is used to encapsulate the packets, the hash calculated
> > > using the outer header of the receive packets is always fixed for the
> > > same flow packets, i.e. they will be steered to the same receive queue.
> > >
> > > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks in
> > > \field{hash_tunnel_types}, which instructs the device to calculate the
> > > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> > >
> > > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the
> > > inner header hash, and does not give the device the ability to use the
> > > hash value to select a receiving queue to place the packet.
> > >
> [..]
> > > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Network Device / Device O
> > >           le16 csum_start;
> > >           le16 csum_offset;
> > >           le16 num_buffers;
> > > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT
> > negotiated)
> > > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT
> > negotiated)
> > > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT
> > negotiated)
> > > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT
> > negotiated)
> > > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT
> > negotiated)
> > > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT
> 
> I am yet to review the changes of v8.
> But the quick response is, I do not see a use case of above field by sw driver.
> And this addition requires the core hw data path to supply this value.
> Without good motivation, it is hard to have it here.

I think I agree that we should be careful about adding stuff in packet
header. Yes it's using the padding but we only have 2 bytes of that.

> What is valuable is to have a VNI already identified and coming to the driver, like a hash value.
> This can cut down the cpu processing power, in outer header packet processing.
> However, that is relatively a different feature than inner hash.
> 
> So, my input is to omit hash_report_tunnel_types.
> Will respond to Michael question in other thread.


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

* RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 20:43   ` Michael S. Tsirkin
@ 2023-02-13 22:29     ` Parav Pandit
  2023-02-16  6:48     ` [virtio-comment] Re: [virtio-dev] " Heng Qi
  1 sibling, 0 replies; 20+ messages in thread
From: Parav Pandit @ 2023-02-13 22:29 UTC (permalink / raw)
  To: Michael S. Tsirkin, Heng Qi
  Cc: virtio-comment, virtio-dev, Jason Wang, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo


> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, February 13, 2023 3:44 PM

[..]
> 
> I didn't yes come up with anything convincing for improving security (dos/usage
> info leak due to queue collisions between tunnels).
> 
> I'd like to at least have a security considerations section documenting the issue
> with using RSS with tunneling.
This is good to add.

> In fact it's high time we started adding these sections all over the place.
> 
> I also found out at least mlx5 has these tunnel offloads, so I'd like to ask nvidia
> guys here on list
> - does this cover e.g. mlx5 functionality? e.g. is the tunnel list
>   sufficient?
Yes, it covers the functionality.
In most cases the sender is rate limited.
But for sure, RX can be under DOS.
Tunnel offloads are mostly used on the switching device than the guest VM NIC.

In such scenario of switching device, there are dedicated RQs per each port of the switch (multi-port NIC) with dedicated RQs.
So, there is no DOS attack per say on the RQ.
Tunnel offload is just to overcome the limitations of hashing done on the outer.

At virtio we are bit far away from such multi-port NIC. Tunnel offloads is step 1 in getting there.

> - does mlx5 cover queue collisions between tunnels and if yes how?
In scenario where there is collision, a receive policer is deployed to avoid the DOS.

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

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 20:43   ` Michael S. Tsirkin
  2023-02-13 22:29     ` Parav Pandit
@ 2023-02-16  6:48     ` Heng Qi
  1 sibling, 0 replies; 20+ messages in thread
From: Heng Qi @ 2023-02-16  6:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo



在 2023/2/14 上午4:43, Michael S. Tsirkin 写道:
> On Mon, Feb 13, 2023 at 09:05:04PM +0800, Heng Qi wrote:
>> Hi, all.
>>
>> Do you have any comments on this version?
>>
>> Thanks.
>>
>> 在 2023/2/8 下午5:08, Heng Qi 写道:
>>> If the tunnel is used to encapsulate the packets, the hash calculated
>>> using the outer header of the receive packets is always fixed for the
>>> same flow packets, i.e. they will be steered to the same receive queue.
>>>
>>> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
>>> in \field{hash_tunnel_types}, which instructs the device to calculate the
>>> hash using the inner headers of tunnel-encapsulated packets. Besides,
>>> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>>>
>>> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
>>> header hash, and does not give the device the ability to use the hash value
>>> to select a receiving queue to place the packet.
>>>
>>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
>>>
>>> Reviewed-by: Jason Wang <jasowang@redhat.com>
>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> I didn't yes come up with anything convincing for improving
> security (dos/usage info leak due to queue collisions between
> tunnels).
>
> I'd like to at least have a security considerations section
> documenting the issue with using RSS with tunneling.

I think it's a good idea, I agree.

> In fact it's high time we started adding these sections all over
> the place.
>
> I also found out at least mlx5 has these tunnel offloads,
> so I'd like to ask nvidia guys here on list
> - does this cover e.g. mlx5 functionality? e.g. is the tunnel list
>    sufficient?
> - does mlx5 cover queue collisions between tunnels and if yes how?
>
> Thanks!
>
>
>>> ---
>>> v7->v8:
>>> 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
>>> 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
>>> 	3. Removed re-definition for inner packet hashing. @Parav Pandit
>>> 	4. Fix some typos. @Michael S . Tsirkin
>>> 	5. Clarify some sentences. @Michael S . Tsirkin
>>>
>>> v6->v7:
>>> 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
>>> 	2. Fix some syntax issues. @Michael S. Tsirkin
>>>
>>> v5->v6:
>>> 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
>>> 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
>>> 	3. Move the links to introduction section. @Michael S. Tsirkin
>>> 	4. Clarify some sentences. @Michael S. Tsirkin
>>>
>>> v4->v5:
>>> 	1. Clarify some paragraphs. @Cornelia Huck
>>> 	2. Fix the u8 type. @Cornelia Huck
>>>
>>> v3->v4:
>>> 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
>>> 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
>>> 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
>>> 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
>>>
>>> v2->v3:
>>> 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
>>> 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
>>>
>>> v1->v2:
>>> 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
>>> 	2. Clarify some paragraphs. @Jason Wang
>>> 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
>>>
>>>    content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
>>>    introduction.tex |  19 +++++++
>>>    2 files changed, 140 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/content.tex b/content.tex
>>> index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
>>> +	for tunnel-encapsulated packets.
>>> +
>>>    \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>>>    \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
>>> @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>>>     to several segments when each of these smaller packets has UDP header.
>>>    \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
>>> -    value and a type of calculated hash.
>>> +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
>>> +    is negotiated, an encapsulation packet type.
>>>    \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>>        value. Device benefits from knowing the exact header length.
>>> @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
>>> @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>>            u8 rss_max_key_size;
>>>            le16 rss_max_indirection_table_length;
>>>            le32 supported_hash_types;
>>> +        le32 supported_tunnel_hash_types;
>>>    };
>>>    \end{lstlisting}
>>> -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>>> +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>>    It specifies the maximum supported length of RSS key in bytes.
>>>    The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
>>>    It specifies the maximum number of 16-bit entries in RSS indirection table.
>>>    The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
>>> -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>>> +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>>    Field \field{supported_hash_types} contains the bitmask of supported hash types.
>>>    See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
>>> +The next field, \field{supported_tunnel_hash_types} only exists if the device
>>> +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
>>> +
>>> +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
>>> +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
>>> +
>>>    \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>>>    The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
>>> @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>>    negotiated.
>>>    The device MUST set \field{rss_max_key_size} to at least 40, if it offers
>>> -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
>>> +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
>>>    The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
>>>    VIRTIO_NET_F_RSS.
>>> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>>>            le16 csum_start;
>>>            le16 csum_offset;
>>>            le16 num_buffers;
>>> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
>>> +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>    };
>>>    \end{lstlisting}
>>> @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>    \begin{itemize}
>>>    \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
>>>    \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
>>> +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
>>>    \end{itemize}
>>>    If the feature VIRTIO_NET_F_RSS was negotiated:
>>>    \begin{itemize}
>>>    \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>>> +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
>>>    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
>>>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
>>>    \end{itemize}
>>> @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>    If the feature VIRTIO_NET_F_RSS was not negotiated:
>>>    \begin{itemize}
>>>    \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>>> +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
>>>    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
>>>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
>>>    \end{itemize}
>>> -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
>>> +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
>>>    at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
>>>    \begin{itemize}
>>>    \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
>>> @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>     \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
>>>    \end{itemize}
>>> +\subparagraph{Tunnel/Encapsulated packet}
>>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
>>> +A tunnel packet is encapsulated from the original packet based on the tunneling
>>> +protocol (only a single level of encapsulation is currently supported). The
>>> +encapsulated packet contains an outer header and an inner header, and the device
>>> +calculates the hash over either the inner header or the outer header.
>>> +
>>> +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>>> +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
>>> +type of encapsulated packet is calculated over the inner as opposed to outer header.
>>> +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
>>> +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
>>> +Supported/enabled hash tunnel types}.
>>> +
>>> +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
>>> +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
>>> +the device supports inner hash calculation for the encapsulated packet,
>>> +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
>>> +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
>>> +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
>>> +respectively, and \field{hash_report_tunnel_types} should be respectively set to
>>> +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
>>> +
>>> +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
>>> +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
>>> +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
>>> +
>>>    \subparagraph{Supported/enabled hash types}
>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
>>> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
>>> +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>>>    Hash types applicable for IPv4 packets:
>>>    \begin{lstlisting}
>>>    #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
>>> @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>    #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
>>>    \end{lstlisting}
>>> +\subparagraph{Supported/enabled tunnel hash types}
>>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
>>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
>>> +hash type indicates that the hash is calculated over the inner header of
>>> +the encapsulated packet:
>>> +Hash type applicable for inner payload of the gre-encapsulated packet
>>> +\begin{lstlisting}
>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
>>> +\end{lstlisting}
>>> +Hash type applicable for inner payload of the vxlan-encapsulated packet
>>> +\begin{lstlisting}
>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
>>> +\end{lstlisting}
>>> +Hash type applicable for inner payload of the geneve-encapsulated packet
>>> +\begin{lstlisting}
>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
>>> +\end{lstlisting}
>>> +
>>>    \subparagraph{IPv4 packets}
>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
>>>    The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
>>> @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>    (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>>>    \end{itemize}
>>> +\subparagraph{Inner hash calculation of an encapsulated packet}
>>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>>> +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
>>> +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
>>> +/ Network Device / Device Operation / Processing of Incoming Packets /
>>> +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
>>> +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
>>> +
>>>    \paragraph{Hash reporting for incoming packets}
>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>>> -
>>> -If VIRTIO_NET_F_HASH_REPORT was negotiated and
>>> - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
>>> -and \field{hash_value} with the value of calculated hash.
>>> +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
>>> +hash for the packet, the device fills \field{hash_report} with the report type
>>> +of calculated hash, and \field{hash_value} with the value of calculated hash.
>>> +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
>>> +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
>>> +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
>>>    If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
>>> -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
>>> +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
>>> +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
>>>    Possible values that the device can report in \field{hash_report} are defined below.
>>>    They correspond to supported hash types defined in
>>> @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>    #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
>>>    \end{lstlisting}
>>> +\field{hash_report_tunnel} can report the type of the encapsulated
>>> +packet to the driver when the inner header hash is calculated.
>>> +Possible values that the device can report in \field{hash_report_tunnel_types}
>>> +are defined below.
>>> +They correspond to supported hash tunnel types defined in
>>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
>>> +as follows:
>>> +
>>> +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
>>> +
>>> +\begin{lstlisting}
>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
>>> +\end{lstlisting}
>>> +
>>> +They correspond to supported hash types defined in
>>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>> +
>>>    \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
>>>    The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
>>> @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>    \begin{lstlisting}
>>>    struct virtio_net_hash_config {
>>>        le32 hash_types;
>>> +    le32 hash_tunnel_types;
>>>        le16 reserved[4];
>>>        u8 hash_key_length;
>>>        u8 hash_key_data[hash_key_length];
>>> @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>    Field \field{hash_types} contains a bitmask of allowed hash types as
>>>    defined in
>>>    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>> -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
>>> +
>>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>>> +
>>> +Initially the device has all hash types and hash tunnel types disabled and reports only
>>> +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
>>>    Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
>>>    defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
>>> @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>    \begin{lstlisting}
>>>    struct virtio_net_rss_config {
>>>        le32 hash_types;
>>> +    le32 hash_tunnel_types;
>>>        le16 indirection_table_mask;
>>>        le16 unclassified_queue;
>>>        le16 indirection_table[indirection_table_length];
>>> @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>    defined in
>>>    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>>> +
>>>    Field \field{indirection_table_mask} is a mask to be applied to
>>>    the calculated hash to produce an index in the
>>>    \field{indirection_table} array.
>>> diff --git a/introduction.tex b/introduction.tex
>>> index 287c5fc..ff01a9b 100644
>>> --- a/introduction.tex
>>> +++ b/introduction.tex
>>> @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
>>>    	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>>>        Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
>>>    	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
>>> +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
>>> +	Generic Routing Encapsulation
>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
>>> +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
>>> +	Virtual eXtensible Local Area Network
>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
>>> +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
>>> +	Generic Network Virtualization Encapsulation
>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
>>> +	\phantomsection\label{intro:IP}\textbf{[IP]} &
>>> +	INTERNET PROTOCOL
>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
>>> +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
>>> +	User Datagram Protocol
>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
>>> +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
>>> +	TRANSMISSION CONTROL PROTOCOL
>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
>>> +
>>>    \end{longtable}
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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	[flat|nested] 20+ messages in thread

* Re: [virtio-dev] RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 21:42   ` Parav Pandit
  2023-02-13 22:23     ` Michael S. Tsirkin
@ 2023-02-16  6:57     ` Heng Qi
  2023-02-16 13:25       ` [virtio-comment] " Parav Pandit
  1 sibling, 1 reply; 20+ messages in thread
From: Heng Qi @ 2023-02-16  6:57 UTC (permalink / raw)
  To: Parav Pandit, virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Cornelia Huck, Yuri Benditovich,
	Xuan Zhuo



在 2023/2/14 上午5:42, Parav Pandit 写道:
>
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Monday, February 13, 2023 8:05 AM
>> Hi, all.
>>
>> Do you have any comments on this version?
>>
>> Thanks.
>>
>> 在 2023/2/8 下午5:08, Heng Qi 写道:
>>> If the tunnel is used to encapsulate the packets, the hash calculated
>>> using the outer header of the receive packets is always fixed for the
>>> same flow packets, i.e. they will be steered to the same receive queue.
>>>
>>> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks in
>>> \field{hash_tunnel_types}, which instructs the device to calculate the
>>> hash using the inner headers of tunnel-encapsulated packets. Besides,
>>> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>>>
>>> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the
>>> inner header hash, and does not give the device the ability to use the
>>> hash value to select a receiving queue to place the packet.
>>>
> [..]
>>> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device
>> Types / Network Device / Device O
>>>            le16 csum_start;
>>>            le16 csum_offset;
>>>            le16 num_buffers;
>>> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT
>> negotiated)
>>> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT
>> negotiated)
>>> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT
>> negotiated)
>>> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT
>> negotiated)
>>> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT
>> negotiated)
>>> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT
> I am yet to review the changes of v8.
> But the quick response is, I do not see a use case of above field by sw driver.
> And this addition requires the core hw data path to supply this value.
> Without good motivation, it is hard to have it here.

I'm really okay with this, the tunnel type seems to help the software 
driver do more things in the future,
for example, the driver may not want to use the outer hash value when 
forwarding packets. But if there is
really no practical scenario for the driver at present, then let us hide 
the tunnel types in the \field{hash_report}?

>
> What is valuable is to have a VNI already identified and coming to the driver, like a hash value.

This is too specific to a certain tunnel protocol, not every protocol 
has corresponding VNI information.

Thanks.

> This can cut down the cpu processing power, in outer header packet processing.
> However, that is relatively a different feature than inner hash.
>
> So, my input is to omit hash_report_tunnel_types.
> Will respond to Michael question in other thread.
>
> ---------------------------------------------------------------------
> 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] 20+ messages in thread

* Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-13 22:20 ` Michael S. Tsirkin
@ 2023-02-16  7:20   ` Heng Qi
  2023-02-16 11:59     ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Heng Qi @ 2023-02-16  7:20 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo



在 2023/2/14 上午6:20, Michael S. Tsirkin 写道:
> On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
>> If the tunnel is used to encapsulate the packets, the hash calculated
>> using the outer header of the receive packets is always fixed for the
>> same flow packets, i.e. they will be steered to the same receive queue.
>>
>> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
>> in \field{hash_tunnel_types}, which instructs the device to calculate the
>> hash using the inner headers of tunnel-encapsulated packets. Besides,
>> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>>
>> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
>> header hash, and does not give the device the ability to use the hash value
>> to select a receiving queue to place the packet.
> This is the part I am missing. Where is this in the proposal?

The core function of the inner header hash feature is to provide a hash 
value calculated using the inner header.
This is its semantics. We just tell the device that if this feature is 
negotiated, the value of \field{hash_value}
comes from the inner header. VIRTIO_NET_F_HASH_REPORT is also such a 
function, it tells the device that you
need to provide the hash value and hash type. If the device needs to use 
the calculated hash value to select the
queue to place packets, then use VIRTIO_NET_F_RSS at the same time.

Thanks.

> We currently have:
>
> The device MUST determine the destination queue for a network packet as follows:
> \begin{itemize}
> \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
> \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
> \item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
>
>
>
>
>
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
>>
>> Reviewed-by: Jason Wang <jasowang@redhat.com>
>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>> v7->v8:
>> 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
>> 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
>> 	3. Removed re-definition for inner packet hashing. @Parav Pandit
>> 	4. Fix some typos. @Michael S . Tsirkin
>> 	5. Clarify some sentences. @Michael S . Tsirkin
>>
>> v6->v7:
>> 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
>> 	2. Fix some syntax issues. @Michael S. Tsirkin
>>
>> v5->v6:
>> 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
>> 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
>> 	3. Move the links to introduction section. @Michael S. Tsirkin
>> 	4. Clarify some sentences. @Michael S. Tsirkin
>>
>> v4->v5:
>> 	1. Clarify some paragraphs. @Cornelia Huck
>> 	2. Fix the u8 type. @Cornelia Huck
>>
>> v3->v4:
>> 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
>> 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
>> 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
>> 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
>>
>> v2->v3:
>> 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
>> 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
>>
>> v1->v2:
>> 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
>> 	2. Clarify some paragraphs. @Jason Wang
>> 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
>>
>>   content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
>>   introduction.tex |  19 +++++++
>>   2 files changed, 140 insertions(+), 14 deletions(-)
>>
>> diff --git a/content.tex b/content.tex
>> index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
>> +	for tunnel-encapsulated packets.
>> +
>>   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>>   
>>   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
>> @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>>    to several segments when each of these smaller packets has UDP header.
>>   
>>   \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
>> -    value and a type of calculated hash.
>> +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
>> +    is negotiated, an encapsulation packet type.
>>   
>>   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>       value. Device benefits from knowing the exact header length.
>> @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
>> @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>           u8 rss_max_key_size;
>>           le16 rss_max_indirection_table_length;
>>           le32 supported_hash_types;
>> +        le32 supported_tunnel_hash_types;
>>   };
>>   \end{lstlisting}
>> -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>> +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>   It specifies the maximum supported length of RSS key in bytes.
>>   
>>   The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
>>   It specifies the maximum number of 16-bit entries in RSS indirection table.
>>   
>>   The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
>> -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>> +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>   
>>   Field \field{supported_hash_types} contains the bitmask of supported hash types.
>>   See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
>>   
>> +The next field, \field{supported_tunnel_hash_types} only exists if the device
>> +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
>> +
>> +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
>> +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
>> +
>>   \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>>   
>>   The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
>> @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>   negotiated.
>>   
>>   The device MUST set \field{rss_max_key_size} to at least 40, if it offers
>> -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
>> +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
>>   
>>   The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
>>   VIRTIO_NET_F_RSS.
>> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>>           le16 csum_start;
>>           le16 csum_offset;
>>           le16 num_buffers;
>> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
>> +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>   };
>>   \end{lstlisting}
>>   
>> @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>   \begin{itemize}
>>   \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
>>   \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
>> +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
>>   \end{itemize}
>>   
>>   If the feature VIRTIO_NET_F_RSS was negotiated:
>>   \begin{itemize}
>>   \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>> +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
>>   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
>>   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
>>   \end{itemize}
>> @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>   If the feature VIRTIO_NET_F_RSS was not negotiated:
>>   \begin{itemize}
>>   \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>> +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
>>   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
>>   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
>>   \end{itemize}
>>   
>> -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
>> +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
>>   at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
>>   \begin{itemize}
>>   \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
>> @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
>>   \end{itemize}
>>   
>> +\subparagraph{Tunnel/Encapsulated packet}
>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
>> +A tunnel packet is encapsulated from the original packet based on the tunneling
>> +protocol (only a single level of encapsulation is currently supported). The
>> +encapsulated packet contains an outer header and an inner header, and the device
>> +calculates the hash over either the inner header or the outer header.
>> +
>> +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>> +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
>> +type of encapsulated packet is calculated over the inner as opposed to outer header.
>> +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
>> +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
>> +Supported/enabled hash tunnel types}.
>> +
>> +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
>> +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
>> +the device supports inner hash calculation for the encapsulated packet,
>> +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
>> +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
>> +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
>> +respectively, and \field{hash_report_tunnel_types} should be respectively set to
>> +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
>> +
>> +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
>> +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
>> +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
>> +
>>   \subparagraph{Supported/enabled hash types}
>>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
>> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
>> +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>>   Hash types applicable for IPv4 packets:
>>   \begin{lstlisting}
>>   #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
>> @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>   #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
>>   \end{lstlisting}
>>   
>> +\subparagraph{Supported/enabled tunnel hash types}
>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
>> +hash type indicates that the hash is calculated over the inner header of
>> +the encapsulated packet:
>> +Hash type applicable for inner payload of the gre-encapsulated packet
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the vxlan-encapsulated packet
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the geneve-encapsulated packet
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
>> +\end{lstlisting}
>> +
>>   \subparagraph{IPv4 packets}
>>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
>>   The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
>> @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>   (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>>   \end{itemize}
>>   
>> +\subparagraph{Inner hash calculation of an encapsulated packet}
>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>> +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
>> +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
>> +/ Network Device / Device Operation / Processing of Incoming Packets /
>> +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
>> +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
>> +
>>   \paragraph{Hash reporting for incoming packets}
>>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>> -
>> -If VIRTIO_NET_F_HASH_REPORT was negotiated and
>> - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
>> -and \field{hash_value} with the value of calculated hash.
>> +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
>> +hash for the packet, the device fills \field{hash_report} with the report type
>> +of calculated hash, and \field{hash_value} with the value of calculated hash.
>> +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
>> +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
>> +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
>>   
>>   If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
>> -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
>> +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
>> +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
>>   
>>   Possible values that the device can report in \field{hash_report} are defined below.
>>   They correspond to supported hash types defined in
>> @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>   #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
>>   \end{lstlisting}
>>   
>> +\field{hash_report_tunnel} can report the type of the encapsulated
>> +packet to the driver when the inner header hash is calculated.
>> +Possible values that the device can report in \field{hash_report_tunnel_types}
>> +are defined below.
>> +They correspond to supported hash tunnel types defined in
>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
>> +as follows:
>> +
>> +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
>> +
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
>> +\end{lstlisting}
>> +
>> +They correspond to supported hash types defined in
>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>> +
>>   \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
>>   
>>   The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
>> @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   \begin{lstlisting}
>>   struct virtio_net_hash_config {
>>       le32 hash_types;
>> +    le32 hash_tunnel_types;
>>       le16 reserved[4];
>>       u8 hash_key_length;
>>       u8 hash_key_data[hash_key_length];
>> @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   Field \field{hash_types} contains a bitmask of allowed hash types as
>>   defined in
>>   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>> -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
>> +
>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>> +
>> +Initially the device has all hash types and hash tunnel types disabled and reports only
>> +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
>>   
>>   Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
>>   defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
>> @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   \begin{lstlisting}
>>   struct virtio_net_rss_config {
>>       le32 hash_types;
>> +    le32 hash_tunnel_types;
>>       le16 indirection_table_mask;
>>       le16 unclassified_queue;
>>       le16 indirection_table[indirection_table_length];
>> @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   defined in
>>   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>   
>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>> +
>>   Field \field{indirection_table_mask} is a mask to be applied to
>>   the calculated hash to produce an index in the
>>   \field{indirection_table} array.
>> diff --git a/introduction.tex b/introduction.tex
>> index 287c5fc..ff01a9b 100644
>> --- a/introduction.tex
>> +++ b/introduction.tex
>> @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
>>   	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>>       Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
>>   	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
>> +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
>> +	Generic Routing Encapsulation
>> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
>> +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
>> +	Virtual eXtensible Local Area Network
>> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
>> +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
>> +	Generic Network Virtualization Encapsulation
>> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
>> +	\phantomsection\label{intro:IP}\textbf{[IP]} &
>> +	INTERNET PROTOCOL
>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
>> +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
>> +	User Datagram Protocol
>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
>> +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
>> +	TRANSMISSION CONTROL PROTOCOL
>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
>> +
>>   
>>   \end{longtable}
>>   
>> -- 
>> 2.19.1.6.gb485710b
>
> ---------------------------------------------------------------------
> 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] 20+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-13 22:23     ` Michael S. Tsirkin
@ 2023-02-16  7:23       ` Heng Qi
  0 siblings, 0 replies; 20+ messages in thread
From: Heng Qi @ 2023-02-16  7:23 UTC (permalink / raw)
  To: Michael S. Tsirkin, Parav Pandit, Jason Wang
  Cc: virtio-comment, virtio-dev, Cornelia Huck, Yuri Benditovich, Xuan Zhuo



在 2023/2/14 上午6:23, Michael S. Tsirkin 写道:
> On Mon, Feb 13, 2023 at 09:42:25PM +0000, Parav Pandit wrote:
>>
>>> From: Heng Qi <hengqi@linux.alibaba.com>
>>> Sent: Monday, February 13, 2023 8:05 AM
>>> Hi, all.
>>>
>>> Do you have any comments on this version?
>>>
>>> Thanks.
>>>
>>> 在 2023/2/8 下午5:08, Heng Qi 写道:
>>>> If the tunnel is used to encapsulate the packets, the hash calculated
>>>> using the outer header of the receive packets is always fixed for the
>>>> same flow packets, i.e. they will be steered to the same receive queue.
>>>>
>>>> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks in
>>>> \field{hash_tunnel_types}, which instructs the device to calculate the
>>>> hash using the inner headers of tunnel-encapsulated packets. Besides,
>>>> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>>>>
>>>> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the
>>>> inner header hash, and does not give the device the ability to use the
>>>> hash value to select a receiving queue to place the packet.
>>>>
>> [..]
>>>> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device
>>> Types / Network Device / Device O
>>>>            le16 csum_start;
>>>>            le16 csum_offset;
>>>>            le16 num_buffers;
>>>> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT
>>> negotiated)
>>>> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT
>>> negotiated)
>>>> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT
>>> negotiated)
>>>> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT
>>> negotiated)
>>>> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT
>>> negotiated)
>>>> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT
>> I am yet to review the changes of v8.
>> But the quick response is, I do not see a use case of above field by sw driver.
>> And this addition requires the core hw data path to supply this value.
>> Without good motivation, it is hard to have it here.
> I think I agree that we should be careful about adding stuff in packet
> header. Yes it's using the padding but we only have 2 bytes of that.

I agree.

Cc Jason, do you think \field{hash_report_tunnel_type} has more 
important capabilities?

Thanks.

>
>> What is valuable is to have a VNI already identified and coming to the driver, like a hash value.
>> This can cut down the cpu processing power, in outer header packet processing.
>> However, that is relatively a different feature than inner hash.
>>
>> So, my input is to omit hash_report_tunnel_types.
>> Will respond to Michael question in other thread.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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	[flat|nested] 20+ messages in thread

* Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-16  7:20   ` [virtio-dev] " Heng Qi
@ 2023-02-16 11:59     ` Michael S. Tsirkin
  2023-02-16 14:11       ` Heng Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2023-02-16 11:59 UTC (permalink / raw)
  To: Heng Qi
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo

On Thu, Feb 16, 2023 at 03:20:17PM +0800, Heng Qi wrote:
> 
> 
> 在 2023/2/14 上午6:20, Michael S. Tsirkin 写道:
> > On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
> > > If the tunnel is used to encapsulate the packets, the hash calculated
> > > using the outer header of the receive packets is always fixed for the
> > > same flow packets, i.e. they will be steered to the same receive queue.
> > > 
> > > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> > > in \field{hash_tunnel_types}, which instructs the device to calculate the
> > > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> > > 
> > > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> > > header hash, and does not give the device the ability to use the hash value
> > > to select a receiving queue to place the packet.
> > This is the part I am missing. Where is this in the proposal?
> 
> The core function of the inner header hash feature is to provide a hash
> value calculated using the inner header.
> This is its semantics. We just tell the device that if this feature is
> negotiated, the value of \field{hash_value}
> comes from the inner header. VIRTIO_NET_F_HASH_REPORT is also such a
> function, it tells the device that you
> need to provide the hash value and hash type. If the device needs to use the
> calculated hash value to select the
> queue to place packets, then use VIRTIO_NET_F_RSS at the same time.
> 
> Thanks.

So VIRTIO_NET_F_HASH_TUNNEL indeed does not give the device the ability to use the hash value
to select a receiving queue to place the packet.
However, the new hash_tunnel_types introduced here do give the device
this ability if enabled by VIRTIO_NET_F_RSS.

Asymmetrical.

To me, VIRTIO_NET_F_HASH_TUNNEL makes sense if we keep
hash_report_tunnel. And Parav insists we should drop hash_report_tunnel.


So I have a suggestion:

Disconnect VIRTIO_NET_F_HASH_TUNNEL from hash calculation.
Just make it imply that hash_report_tunnel is valid.

Whether hash is calculated over the inner header is controlled
by the hash_report_tunnel_types.




> > We currently have:
> > 
> > The device MUST determine the destination queue for a network packet as follows:
> > \begin{itemize}
> > \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
> > \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
> > \item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
> > 
> > 
> > 
> > 
> > 
> > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
> > > 
> > > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > ---
> > > v7->v8:
> > > 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> > > 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> > > 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> > > 	4. Fix some typos. @Michael S . Tsirkin
> > > 	5. Clarify some sentences. @Michael S . Tsirkin
> > > 
> > > v6->v7:
> > > 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> > > 	2. Fix some syntax issues. @Michael S. Tsirkin
> > > 
> > > v5->v6:
> > > 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> > > 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> > > 	3. Move the links to introduction section. @Michael S. Tsirkin
> > > 	4. Clarify some sentences. @Michael S. Tsirkin
> > > 
> > > v4->v5:
> > > 	1. Clarify some paragraphs. @Cornelia Huck
> > > 	2. Fix the u8 type. @Cornelia Huck
> > > 
> > > v3->v4:
> > > 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> > > 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> > > 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> > > 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
> > > 
> > > v2->v3:
> > > 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> > > 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
> > > 
> > > v1->v2:
> > > 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> > > 	2. Clarify some paragraphs. @Jason Wang
> > > 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
> > > 
> > >   content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
> > >   introduction.tex |  19 +++++++
> > >   2 files changed, 140 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/content.tex b/content.tex
> > > index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> > > +	for tunnel-encapsulated packets.
> > > +
> > >   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
> > >   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > > @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > >    to several segments when each of these smaller packets has UDP header.
> > >   \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> > > -    value and a type of calculated hash.
> > > +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> > > +    is negotiated, an encapsulation packet type.
> > >   \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
> > >       value. Device benefits from knowing the exact header length.
> > > @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> > > @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > >           u8 rss_max_key_size;
> > >           le16 rss_max_indirection_table_length;
> > >           le32 supported_hash_types;
> > > +        le32 supported_tunnel_hash_types;
> > >   };
> > >   \end{lstlisting}
> > > -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > >   It specifies the maximum supported length of RSS key in bytes.
> > >   The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
> > >   It specifies the maximum number of 16-bit entries in RSS indirection table.
> > >   The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> > > -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > >   Field \field{supported_hash_types} contains the bitmask of supported hash types.
> > >   See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
> > > +The next field, \field{supported_tunnel_hash_types} only exists if the device
> > > +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> > > +
> > > +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> > > +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> > > +
> > >   \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> > >   The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> > > @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > >   negotiated.
> > >   The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> > > -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > > +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
> > >   The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
> > >   VIRTIO_NET_F_RSS.
> > > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > >           le16 csum_start;
> > >           le16 csum_offset;
> > >           le16 num_buffers;
> > > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> > > +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > >   };
> > >   \end{lstlisting}
> > > @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >   \begin{itemize}
> > >   \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
> > >   \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> > > +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
> > >   \end{itemize}
> > >   If the feature VIRTIO_NET_F_RSS was negotiated:
> > >   \begin{itemize}
> > >   \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
> > >   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
> > >   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
> > >   \end{itemize}
> > > @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >   If the feature VIRTIO_NET_F_RSS was not negotiated:
> > >   \begin{itemize}
> > >   \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
> > >   \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
> > >   \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
> > >   \end{itemize}
> > > -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> > > +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
> > >   at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
> > >   \begin{itemize}
> > >   \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> > > @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
> > >   \end{itemize}
> > > +\subparagraph{Tunnel/Encapsulated packet}
> > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> > > +A tunnel packet is encapsulated from the original packet based on the tunneling
> > > +protocol (only a single level of encapsulation is currently supported). The
> > > +encapsulated packet contains an outer header and an inner header, and the device
> > > +calculates the hash over either the inner header or the outer header.
> > > +
> > > +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> > > +type of encapsulated packet is calculated over the inner as opposed to outer header.
> > > +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> > > +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> > > +Supported/enabled hash tunnel types}.
> > > +
> > > +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> > > +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> > > +the device supports inner hash calculation for the encapsulated packet,
> > > +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> > > +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> > > +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> > > +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> > > +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> > > +
> > > +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> > > +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> > > +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> > > +
> > >   \subparagraph{Supported/enabled hash types}
> > >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> > > +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> > > +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
> > >   Hash types applicable for IPv4 packets:
> > >   \begin{lstlisting}
> > >   #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> > > @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >   #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
> > >   \end{lstlisting}
> > > +\subparagraph{Supported/enabled tunnel hash types}
> > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> > > +hash type indicates that the hash is calculated over the inner header of
> > > +the encapsulated packet:
> > > +Hash type applicable for inner payload of the gre-encapsulated packet
> > > +\begin{lstlisting}
> > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> > > +\end{lstlisting}
> > > +Hash type applicable for inner payload of the vxlan-encapsulated packet
> > > +\begin{lstlisting}
> > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> > > +\end{lstlisting}
> > > +Hash type applicable for inner payload of the geneve-encapsulated packet
> > > +\begin{lstlisting}
> > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> > > +\end{lstlisting}

are these the only tunnel types we can thinkably support?

> > >   \subparagraph{IPv4 packets}
> > >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
> > >   The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> > > @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >   (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
> > >   \end{itemize}
> > > +\subparagraph{Inner hash calculation of an encapsulated packet}
> > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> > > +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> > > +/ Network Device / Device Operation / Processing of Incoming Packets /
> > > +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> > > +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> > > +
> > >   \paragraph{Hash reporting for incoming packets}
> > >   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> > > -
> > > -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> > > - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> > > -and \field{hash_value} with the value of calculated hash.
> > > +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> > > +hash for the packet, the device fills \field{hash_report} with the report type
> > > +of calculated hash, and \field{hash_value} with the value of calculated hash.
> > > +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> > > +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> > > +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
> > >   If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> > > -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> > > +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> > > +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
> > >   Possible values that the device can report in \field{hash_report} are defined below.
> > >   They correspond to supported hash types defined in
> > > @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > >   #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
> > >   \end{lstlisting}
> > > +\field{hash_report_tunnel} can report the type of the encapsulated
> > > +packet to the driver when the inner header hash is calculated.
> > > +Possible values that the device can report in \field{hash_report_tunnel_types}
> > > +are defined below.
> > > +They correspond to supported hash tunnel types defined in
> > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> > > +as follows:
> > > +
> > > +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> > > +
> > > +\begin{lstlisting}
> > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> > > +\end{lstlisting}
> > > +
> > > +They correspond to supported hash types defined in
> > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > +
> > >   \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
> > >   The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> > > @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >   \begin{lstlisting}
> > >   struct virtio_net_hash_config {
> > >       le32 hash_types;
> > > +    le32 hash_tunnel_types;
> > >       le16 reserved[4];

wait a second. this must be reserved[2] now.




> > >       u8 hash_key_length;
> > >       u8 hash_key_data[hash_key_length];
> > > @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >   Field \field{hash_types} contains a bitmask of allowed hash types as
> > >   defined in
> > >   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> > > +
> > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > +
> > > +Initially the device has all hash types and hash tunnel types disabled and reports only
> > > +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
> > >   Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
> > >   defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> > > @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >   \begin{lstlisting}
> > >   struct virtio_net_rss_config {
> > >       le32 hash_types;
> > > +    le32 hash_tunnel_types;
> > >       le16 indirection_table_mask;
> > >       le16 unclassified_queue;
> > >       le16 indirection_table[indirection_table_length];
> > > @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > >   defined in
> > >   \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > +
> > >   Field \field{indirection_table_mask} is a mask to be applied to
> > >   the calculated hash to produce an index in the
> > >   \field{indirection_table} array.
> > > diff --git a/introduction.tex b/introduction.tex
> > > index 287c5fc..ff01a9b 100644
> > > --- a/introduction.tex
> > > +++ b/introduction.tex
> > > @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
> > >   	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> > >       Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
> > >   	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> > > +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> > > +	Generic Routing Encapsulation
> > > +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> > > +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> > > +	Virtual eXtensible Local Area Network
> > > +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> > > +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> > > +	Generic Network Virtualization Encapsulation
> > > +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> > > +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> > > +	INTERNET PROTOCOL
> > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> > > +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> > > +	User Datagram Protocol
> > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> > > +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> > > +	TRANSMISSION CONTROL PROTOCOL
> > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> > > +
> > >   \end{longtable}
> > > -- 
> > > 2.19.1.6.gb485710b
> > 
> > ---------------------------------------------------------------------
> > 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] 20+ messages in thread

* [virtio-comment] RE: [virtio-dev] RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-16  6:57     ` [virtio-dev] " Heng Qi
@ 2023-02-16 13:25       ` Parav Pandit
  2023-02-16 14:19         ` Heng Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Parav Pandit @ 2023-02-16 13:25 UTC (permalink / raw)
  To: Heng Qi, virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Cornelia Huck, Yuri Benditovich,
	Xuan Zhuo



> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Thursday, February 16, 2023 1:58 AM
> 
> I'm really okay with this, the tunnel type seems to help the software driver do
> more things in the future, for example, the driver may not want to use the
> outer hash value when forwarding packets. But if there is really no practical
> scenario for the driver at present, then let us hide the tunnel types in the
> \field{hash_report}?
>
Right. Lets omit hash_report in virtio_net_hdr.
 
> >
> > What is valuable is to have a VNI already identified and coming to the driver,
> like a hash value.
> 
> This is too specific to a certain tunnel protocol, not every protocol has
> corresponding VNI information.
> 
True. I was considering a hash of the generic outer header through which it can reach to generic tunnel.
But it is a very different feature of its own, so yes, better to split and proceed with this to improve inner hash.

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

* Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-16 11:59     ` Michael S. Tsirkin
@ 2023-02-16 14:11       ` Heng Qi
  2023-02-17 14:25         ` Michael S. Tsirkin
  0 siblings, 1 reply; 20+ messages in thread
From: Heng Qi @ 2023-02-16 14:11 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo



在 2023/2/16 下午7:59, Michael S. Tsirkin 写道:
> On Thu, Feb 16, 2023 at 03:20:17PM +0800, Heng Qi wrote:
>>
>> 在 2023/2/14 上午6:20, Michael S. Tsirkin 写道:
>>> On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
>>>> If the tunnel is used to encapsulate the packets, the hash calculated
>>>> using the outer header of the receive packets is always fixed for the
>>>> same flow packets, i.e. they will be steered to the same receive queue.
>>>>
>>>> We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
>>>> in \field{hash_tunnel_types}, which instructs the device to calculate the
>>>> hash using the inner headers of tunnel-encapsulated packets. Besides,
>>>> values in \field{hash_report_tunnel_types} are added to report tunnel types.
>>>>
>>>> Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
>>>> header hash, and does not give the device the ability to use the hash value
>>>> to select a receiving queue to place the packet.
>>> This is the part I am missing. Where is this in the proposal?
>> The core function of the inner header hash feature is to provide a hash
>> value calculated using the inner header.
>> This is its semantics. We just tell the device that if this feature is
>> negotiated, the value of \field{hash_value}
>> comes from the inner header. VIRTIO_NET_F_HASH_REPORT is also such a
>> function, it tells the device that you
>> need to provide the hash value and hash type. If the device needs to use the
>> calculated hash value to select the
>> queue to place packets, then use VIRTIO_NET_F_RSS at the same time.
>>
>> Thanks.
> So VIRTIO_NET_F_HASH_TUNNEL indeed does not give the device the ability to use the hash value
> to select a receiving queue to place the packet.

Yes.

> However, the new hash_tunnel_types introduced here do give the device
> this ability if enabled by VIRTIO_NET_F_RSS.
>
> Asymmetrical.

supported_hash_types only means that the device has the ability to 
calculate hash, and does not force the device
to use hash to select a queue and place the packets. Even if only 
VIRTIO_NET_F_HASH_REPORT is negotiated and
VIRTIO_NET_F_RSS is not negotiated, the device can also have the ability 
to calculate hash instead of selecting a
queue based on supported_hash_types.

VIRTIO_NET_F_HASH_TUNNEL is similar to VIRTIO_NET_F_HASH_REPORT, and 
supported_tunnel_hash_types is also similar to supported_hash_types,
which has nothing to do with selecting a queue to place packets.

Take an example in the spec:
"The next field, \field{supported_hash_types} only exists if the device 
supports hash calculation,
i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set."

>
> To me, VIRTIO_NET_F_HASH_TUNNEL makes sense if we keep
> hash_report_tunnel. And Parav insists we should drop hash_report_tunnel.

If we really don't have a practical use case for 
hash_report_tunnel_type, we can merge its type into hash_report.

>
> So I have a suggestion:
>
> Disconnect VIRTIO_NET_F_HASH_TUNNEL from hash calculation.
> Just make it imply that hash_report_tunnel is valid.
>
> Whether hash is calculated over the inner header is controlled
> by the hash_report_tunnel_types.

I don't really follow this, hash_report_tunnel_type is better off 
keeping it "report" literally.

>
>
>
>>> We currently have:
>>>
>>> The device MUST determine the destination queue for a network packet as follows:
>>> \begin{itemize}
>>> \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
>>> \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
>>> \item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
>>>
>>>
>>>
>>>
>>>
>>>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
>>>>
>>>> Reviewed-by: Jason Wang <jasowang@redhat.com>
>>>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>>>> Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>>>> ---
>>>> v7->v8:
>>>> 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
>>>> 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
>>>> 	3. Removed re-definition for inner packet hashing. @Parav Pandit
>>>> 	4. Fix some typos. @Michael S . Tsirkin
>>>> 	5. Clarify some sentences. @Michael S . Tsirkin
>>>>
>>>> v6->v7:
>>>> 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
>>>> 	2. Fix some syntax issues. @Michael S. Tsirkin
>>>>
>>>> v5->v6:
>>>> 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
>>>> 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
>>>> 	3. Move the links to introduction section. @Michael S. Tsirkin
>>>> 	4. Clarify some sentences. @Michael S. Tsirkin
>>>>
>>>> v4->v5:
>>>> 	1. Clarify some paragraphs. @Cornelia Huck
>>>> 	2. Fix the u8 type. @Cornelia Huck
>>>>
>>>> v3->v4:
>>>> 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
>>>> 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
>>>> 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
>>>> 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
>>>>
>>>> v2->v3:
>>>> 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
>>>> 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
>>>>
>>>> v1->v2:
>>>> 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
>>>> 	2. Clarify some paragraphs. @Jason Wang
>>>> 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
>>>>
>>>>    content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
>>>>    introduction.tex |  19 +++++++
>>>>    2 files changed, 140 insertions(+), 14 deletions(-)
>>>>
>>>> diff --git a/content.tex b/content.tex
>>>> index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
>>>> +	for tunnel-encapsulated packets.
>>>> +
>>>>    \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>>>>    \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
>>>> @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>>>>     to several segments when each of these smaller packets has UDP header.
>>>>    \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
>>>> -    value and a type of calculated hash.
>>>> +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
>>>> +    is negotiated, an encapsulation packet type.
>>>>    \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
>>>>        value. Device benefits from knowing the exact header length.
>>>> @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
>>>> @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>>>            u8 rss_max_key_size;
>>>>            le16 rss_max_indirection_table_length;
>>>>            le32 supported_hash_types;
>>>> +        le32 supported_tunnel_hash_types;
>>>>    };
>>>>    \end{lstlisting}
>>>> -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>>>> +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>>>    It specifies the maximum supported length of RSS key in bytes.
>>>>    The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
>>>>    It specifies the maximum number of 16-bit entries in RSS indirection table.
>>>>    The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
>>>> -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
>>>> +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
>>>>    Field \field{supported_hash_types} contains the bitmask of supported hash types.
>>>>    See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
>>>> +The next field, \field{supported_tunnel_hash_types} only exists if the device
>>>> +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
>>>> +
>>>> +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
>>>> +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
>>>> +
>>>>    \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>>>>    The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
>>>> @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>>>>    negotiated.
>>>>    The device MUST set \field{rss_max_key_size} to at least 40, if it offers
>>>> -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
>>>> +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
>>>>    The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
>>>>    VIRTIO_NET_F_RSS.
>>>> @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
>>>>            le16 csum_start;
>>>>            le16 csum_offset;
>>>>            le16 num_buffers;
>>>> -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>> -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>> -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>> +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>> +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>> +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
>>>> +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>>>>    };
>>>>    \end{lstlisting}
>>>> @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>    \begin{itemize}
>>>>    \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
>>>>    \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
>>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
>>>> +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
>>>>    \end{itemize}
>>>>    If the feature VIRTIO_NET_F_RSS was negotiated:
>>>>    \begin{itemize}
>>>>    \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
>>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>>>> +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
>>>>    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
>>>>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
>>>>    \end{itemize}
>>>> @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>    If the feature VIRTIO_NET_F_RSS was not negotiated:
>>>>    \begin{itemize}
>>>>    \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
>>>> +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
>>>> +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
>>>>    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
>>>>    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
>>>>    \end{itemize}
>>>> -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
>>>> +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
>>>>    at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
>>>>    \begin{itemize}
>>>>    \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
>>>> @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>     \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
>>>>    \end{itemize}
>>>> +\subparagraph{Tunnel/Encapsulated packet}
>>>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
>>>> +A tunnel packet is encapsulated from the original packet based on the tunneling
>>>> +protocol (only a single level of encapsulation is currently supported). The
>>>> +encapsulated packet contains an outer header and an inner header, and the device
>>>> +calculates the hash over either the inner header or the outer header.
>>>> +
>>>> +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>>>> +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
>>>> +type of encapsulated packet is calculated over the inner as opposed to outer header.
>>>> +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
>>>> +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
>>>> +Supported/enabled hash tunnel types}.
>>>> +
>>>> +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
>>>> +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
>>>> +the device supports inner hash calculation for the encapsulated packet,
>>>> +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
>>>> +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
>>>> +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
>>>> +respectively, and \field{hash_report_tunnel_types} should be respectively set to
>>>> +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
>>>> +
>>>> +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
>>>> +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
>>>> +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
>>>> +
>>>>    \subparagraph{Supported/enabled hash types}
>>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
>>>> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
>>>> +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>>>>    Hash types applicable for IPv4 packets:
>>>>    \begin{lstlisting}
>>>>    #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
>>>> @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>    #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
>>>>    \end{lstlisting}
>>>> +\subparagraph{Supported/enabled tunnel hash types}
>>>> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
>>>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
>>>> +hash type indicates that the hash is calculated over the inner header of
>>>> +the encapsulated packet:
>>>> +Hash type applicable for inner payload of the gre-encapsulated packet
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
>>>> +\end{lstlisting}
>>>> +Hash type applicable for inner payload of the vxlan-encapsulated packet
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
>>>> +\end{lstlisting}
>>>> +Hash type applicable for inner payload of the geneve-encapsulated packet
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
>>>> +\end{lstlisting}
> are these the only tunnel types we can thinkably support?

This is the tunnel types our group currently uses, do you think we 
should add more tunnel types?

>
>>>>    \subparagraph{IPv4 packets}
>>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
>>>>    The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
>>>> @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>    (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>>>>    \end{itemize}
>>>> +\subparagraph{Inner hash calculation of an encapsulated packet}
>>>> +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
>>>> +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
>>>> +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
>>>> +/ Network Device / Device Operation / Processing of Incoming Packets /
>>>> +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
>>>> +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
>>>> +
>>>>    \paragraph{Hash reporting for incoming packets}
>>>>    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>>>> -
>>>> -If VIRTIO_NET_F_HASH_REPORT was negotiated and
>>>> - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
>>>> -and \field{hash_value} with the value of calculated hash.
>>>> +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
>>>> +hash for the packet, the device fills \field{hash_report} with the report type
>>>> +of calculated hash, and \field{hash_value} with the value of calculated hash.
>>>> +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
>>>> +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
>>>> +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
>>>>    If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
>>>> -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
>>>> +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
>>>> +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
>>>>    Possible values that the device can report in \field{hash_report} are defined below.
>>>>    They correspond to supported hash types defined in
>>>> @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>>>>    #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
>>>>    \end{lstlisting}
>>>> +\field{hash_report_tunnel} can report the type of the encapsulated
>>>> +packet to the driver when the inner header hash is calculated.
>>>> +Possible values that the device can report in \field{hash_report_tunnel_types}
>>>> +are defined below.
>>>> +They correspond to supported hash tunnel types defined in
>>>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
>>>> +as follows:
>>>> +
>>>> +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
>>>> +
>>>> +\begin{lstlisting}
>>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
>>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
>>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
>>>> +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
>>>> +\end{lstlisting}
>>>> +
>>>> +They correspond to supported hash types defined in
>>>> +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>>> +
>>>>    \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
>>>>    The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
>>>> @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>>    \begin{lstlisting}
>>>>    struct virtio_net_hash_config {
>>>>        le32 hash_types;
>>>> +    le32 hash_tunnel_types;
>>>>        le16 reserved[4];
> wait a second. this must be reserved[2] now.
>

Thanks for pointing it out. I'll fix it.

>
>
>>>>        u8 hash_key_length;
>>>>        u8 hash_key_data[hash_key_length];
>>>> @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>>    Field \field{hash_types} contains a bitmask of allowed hash types as
>>>>    defined in
>>>>    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>>> -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
>>>> +
>>>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>>>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>>>> +
>>>> +Initially the device has all hash types and hash tunnel types disabled and reports only
>>>> +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
>>>>    Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
>>>>    defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
>>>> @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>>    \begin{lstlisting}
>>>>    struct virtio_net_rss_config {
>>>>        le32 hash_types;
>>>> +    le32 hash_tunnel_types;
>>>>        le16 indirection_table_mask;
>>>>        le16 unclassified_queue;
>>>>        le16 indirection_table[indirection_table_length];
>>>> @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>>>    defined in
>>>>    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
>>>> +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
>>>> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
>>>> +
>>>>    Field \field{indirection_table_mask} is a mask to be applied to
>>>>    the calculated hash to produce an index in the
>>>>    \field{indirection_table} array.
>>>> diff --git a/introduction.tex b/introduction.tex
>>>> index 287c5fc..ff01a9b 100644
>>>> --- a/introduction.tex
>>>> +++ b/introduction.tex
>>>> @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
>>>>    	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
>>>>        Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
>>>>    	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
>>>> +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
>>>> +	Generic Routing Encapsulation
>>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
>>>> +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
>>>> +	Virtual eXtensible Local Area Network
>>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
>>>> +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
>>>> +	Generic Network Virtualization Encapsulation
>>>> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
>>>> +	\phantomsection\label{intro:IP}\textbf{[IP]} &
>>>> +	INTERNET PROTOCOL
>>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
>>>> +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
>>>> +	User Datagram Protocol
>>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
>>>> +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
>>>> +	TRANSMISSION CONTROL PROTOCOL
>>>> +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
>>>> +
>>>>    \end{longtable}
>>>> -- 
>>>> 2.19.1.6.gb485710b
>>> ---------------------------------------------------------------------
>>> 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] 20+ messages in thread

* Re: [virtio-comment] RE: [virtio-dev] RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-16 13:25       ` [virtio-comment] " Parav Pandit
@ 2023-02-16 14:19         ` Heng Qi
  2023-02-16 14:23           ` Parav Pandit
  0 siblings, 1 reply; 20+ messages in thread
From: Heng Qi @ 2023-02-16 14:19 UTC (permalink / raw)
  To: Parav Pandit, virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Cornelia Huck, Yuri Benditovich,
	Xuan Zhuo



在 2023/2/16 下午9:25, Parav Pandit 写道:
>
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Thursday, February 16, 2023 1:58 AM
>>
>> I'm really okay with this, the tunnel type seems to help the software driver do
>> more things in the future, for example, the driver may not want to use the
>> outer hash value when forwarding packets. But if there is really no practical
>> scenario for the driver at present, then let us hide the tunnel types in the
>> \field{hash_report}?
>>
> Right. Lets omit hash_report in virtio_net_hdr.

Ok. Will do.

>   
>>> What is valuable is to have a VNI already identified and coming to the driver,
>> like a hash value.
>>
>> This is too specific to a certain tunnel protocol, not every protocol has
>> corresponding VNI information.
>>
> True. I was considering a hash of the generic outer header through which it can reach to generic tunnel.
> But it is a very different feature of its own, so yes, better to split and proceed with this to improve inner hash.

It's a good idea, but I think the current work needs to focus on the 
inner header hash, and the more general features we can continue in 
other work.

Thanks.




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

* RE: [virtio-comment] RE: [virtio-dev] RE: [virtio-comment] [PATCH v8] virtio-net: support inner header hash
  2023-02-16 14:19         ` Heng Qi
@ 2023-02-16 14:23           ` Parav Pandit
  0 siblings, 0 replies; 20+ messages in thread
From: Parav Pandit @ 2023-02-16 14:23 UTC (permalink / raw)
  To: Heng Qi, virtio-comment, virtio-dev
  Cc: Jason Wang, Michael S . Tsirkin, Cornelia Huck, Yuri Benditovich,
	Xuan Zhuo



> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Thursday, February 16, 2023 9:19 AM
> >> I'm really okay with this, the tunnel type seems to help the software
> >> driver do more things in the future, for example, the driver may not
> >> want to use the outer hash value when forwarding packets. But if
> >> there is really no practical scenario for the driver at present, then
> >> let us hide the tunnel types in the \field{hash_report}?
> >>
> > Right. Lets omit hash_report in virtio_net_hdr.
> 
> Ok. Will do.
Ok. thanks.

> 
> >
> >>> What is valuable is to have a VNI already identified and coming to
> >>> the driver,
> >> like a hash value.
> >>
> >> This is too specific to a certain tunnel protocol, not every protocol
> >> has corresponding VNI information.
> >>
> > True. I was considering a hash of the generic outer header through which it
> can reach to generic tunnel.
> > But it is a very different feature of its own, so yes, better to split and proceed
> with this to improve inner hash.
> 
> It's a good idea, but I think the current work needs to focus on the inner header
> hash, and the more general features we can continue in other work.
> 
Right. May be my previous comment didn’t clarify that enough.
We both are saying same point to not focus on outer header optimization in these series. :)


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

* Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-16 14:11       ` Heng Qi
@ 2023-02-17 14:25         ` Michael S. Tsirkin
  2023-02-17 15:36           ` [virtio-comment] " Heng Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Michael S. Tsirkin @ 2023-02-17 14:25 UTC (permalink / raw)
  To: Heng Qi
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo

On Thu, Feb 16, 2023 at 10:11:58PM +0800, Heng Qi wrote:
> 
> 
> 在 2023/2/16 下午7:59, Michael S. Tsirkin 写道:
> > On Thu, Feb 16, 2023 at 03:20:17PM +0800, Heng Qi wrote:
> > > 
> > > 在 2023/2/14 上午6:20, Michael S. Tsirkin 写道:
> > > > On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
> > > > > If the tunnel is used to encapsulate the packets, the hash calculated
> > > > > using the outer header of the receive packets is always fixed for the
> > > > > same flow packets, i.e. they will be steered to the same receive queue.
> > > > > 
> > > > > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> > > > > in \field{hash_tunnel_types}, which instructs the device to calculate the
> > > > > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > > > > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> > > > > 
> > > > > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> > > > > header hash, and does not give the device the ability to use the hash value
> > > > > to select a receiving queue to place the packet.
> > > > This is the part I am missing. Where is this in the proposal?
> > > The core function of the inner header hash feature is to provide a hash
> > > value calculated using the inner header.
> > > This is its semantics. We just tell the device that if this feature is
> > > negotiated, the value of \field{hash_value}
> > > comes from the inner header. VIRTIO_NET_F_HASH_REPORT is also such a
> > > function, it tells the device that you
> > > need to provide the hash value and hash type. If the device needs to use the
> > > calculated hash value to select the
> > > queue to place packets, then use VIRTIO_NET_F_RSS at the same time.
> > > 
> > > Thanks.
> > So VIRTIO_NET_F_HASH_TUNNEL indeed does not give the device the ability to use the hash value
> > to select a receiving queue to place the packet.
> 
> Yes.
> 
> > However, the new hash_tunnel_types introduced here do give the device
> > this ability if enabled by VIRTIO_NET_F_RSS.
> > 
> > Asymmetrical.
> 
> supported_hash_types only means that the device has the ability to calculate
> hash, and does not force the device
> to use hash to select a queue and place the packets. Even if only
> VIRTIO_NET_F_HASH_REPORT is negotiated and
> VIRTIO_NET_F_RSS is not negotiated, the device can also have the ability to
> calculate hash instead of selecting a
> queue based on supported_hash_types.
> 
> VIRTIO_NET_F_HASH_TUNNEL is similar to VIRTIO_NET_F_HASH_REPORT, and
> supported_tunnel_hash_types is also similar to supported_hash_types,

yes

> which has nothing to do with selecting a queue to place packets.

no

If you enable VIRTIO_NET_F_RSS then hash whatever it is
will affect the queue selected. So "nothing to do" is wrong
is it not?


> Take an example in the spec:
> "The next field, \field{supported_hash_types} only exists if the device
> supports hash calculation,
> i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set."
> 
> > 
> > To me, VIRTIO_NET_F_HASH_TUNNEL makes sense if we keep
> > hash_report_tunnel. And Parav insists we should drop hash_report_tunnel.
> 
> If we really don't have a practical use case for hash_report_tunnel_type, we
> can merge its type into hash_report.
> 
> > 
> > So I have a suggestion:
> > 
> > Disconnect VIRTIO_NET_F_HASH_TUNNEL from hash calculation.
> > Just make it imply that hash_report_tunnel is valid.
> > 
> > Whether hash is calculated over the inner header is controlled
> > by the hash_report_tunnel_types.
> 
> I don't really follow this, hash_report_tunnel_type is better off keeping it
> "report" literally.


Talking about VIRTIO_NET_F_HASH_TUNNEL here. Not
hash_report_tunnel_type.

> > 
> > 
> > 
> > > > We currently have:
> > > > 
> > > > The device MUST determine the destination queue for a network packet as follows:
> > > > \begin{itemize}
> > > > \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
> > > > \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
> > > > \item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
> > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
> > > > > 
> > > > > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > > > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > ---
> > > > > v7->v8:
> > > > > 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> > > > > 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> > > > > 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> > > > > 	4. Fix some typos. @Michael S . Tsirkin
> > > > > 	5. Clarify some sentences. @Michael S . Tsirkin
> > > > > 
> > > > > v6->v7:
> > > > > 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> > > > > 	2. Fix some syntax issues. @Michael S. Tsirkin
> > > > > 
> > > > > v5->v6:
> > > > > 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> > > > > 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> > > > > 	3. Move the links to introduction section. @Michael S. Tsirkin
> > > > > 	4. Clarify some sentences. @Michael S. Tsirkin
> > > > > 
> > > > > v4->v5:
> > > > > 	1. Clarify some paragraphs. @Cornelia Huck
> > > > > 	2. Fix the u8 type. @Cornelia Huck
> > > > > 
> > > > > v3->v4:
> > > > > 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> > > > > 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> > > > > 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> > > > > 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
> > > > > 
> > > > > v2->v3:
> > > > > 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> > > > > 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
> > > > > 
> > > > > v1->v2:
> > > > > 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> > > > > 	2. Clarify some paragraphs. @Jason Wang
> > > > > 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
> > > > > 
> > > > >    content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
> > > > >    introduction.tex |  19 +++++++
> > > > >    2 files changed, 140 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/content.tex b/content.tex
> > > > > index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> > > > > +	for tunnel-encapsulated packets.
> > > > > +
> > > > >    \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
> > > > >    \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > > > > @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > >     to several segments when each of these smaller packets has UDP header.
> > > > >    \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> > > > > -    value and a type of calculated hash.
> > > > > +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> > > > > +    is negotiated, an encapsulation packet type.
> > > > >    \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
> > > > >        value. Device benefits from knowing the exact header length.
> > > > > @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> > > > > @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > >            u8 rss_max_key_size;
> > > > >            le16 rss_max_indirection_table_length;
> > > > >            le32 supported_hash_types;
> > > > > +        le32 supported_tunnel_hash_types;
> > > > >    };
> > > > >    \end{lstlisting}
> > > > > -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > > > +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > >    It specifies the maximum supported length of RSS key in bytes.
> > > > >    The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
> > > > >    It specifies the maximum number of 16-bit entries in RSS indirection table.
> > > > >    The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> > > > > -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > > > +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > >    Field \field{supported_hash_types} contains the bitmask of supported hash types.
> > > > >    See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
> > > > > +The next field, \field{supported_tunnel_hash_types} only exists if the device
> > > > > +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > > +
> > > > > +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> > > > > +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> > > > > +
> > > > >    \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> > > > >    The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> > > > > @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > >    negotiated.
> > > > >    The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> > > > > -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > > > > +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
> > > > >    The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
> > > > >    VIRTIO_NET_F_RSS.
> > > > > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > >            le16 csum_start;
> > > > >            le16 csum_offset;
> > > > >            le16 num_buffers;
> > > > > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> > > > > +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > >    };
> > > > >    \end{lstlisting}
> > > > > @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >    \begin{itemize}
> > > > >    \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
> > > > >    \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> > > > > +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
> > > > >    \end{itemize}
> > > > >    If the feature VIRTIO_NET_F_RSS was negotiated:
> > > > >    \begin{itemize}
> > > > >    \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > > > +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
> > > > >    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
> > > > >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
> > > > >    \end{itemize}
> > > > > @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >    If the feature VIRTIO_NET_F_RSS was not negotiated:
> > > > >    \begin{itemize}
> > > > >    \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > > > +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
> > > > >    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
> > > > >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
> > > > >    \end{itemize}
> > > > > -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> > > > > +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
> > > > >    at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
> > > > >    \begin{itemize}
> > > > >    \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> > > > > @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >     \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
> > > > >    \end{itemize}
> > > > > +\subparagraph{Tunnel/Encapsulated packet}
> > > > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> > > > > +A tunnel packet is encapsulated from the original packet based on the tunneling
> > > > > +protocol (only a single level of encapsulation is currently supported). The
> > > > > +encapsulated packet contains an outer header and an inner header, and the device
> > > > > +calculates the hash over either the inner header or the outer header.
> > > > > +
> > > > > +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > > > +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> > > > > +type of encapsulated packet is calculated over the inner as opposed to outer header.
> > > > > +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> > > > > +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> > > > > +Supported/enabled hash tunnel types}.
> > > > > +
> > > > > +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> > > > > +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> > > > > +the device supports inner hash calculation for the encapsulated packet,
> > > > > +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> > > > > +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> > > > > +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> > > > > +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> > > > > +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> > > > > +
> > > > > +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> > > > > +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> > > > > +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> > > > > +
> > > > >    \subparagraph{Supported/enabled hash types}
> > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> > > > > +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> > > > > +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
> > > > >    Hash types applicable for IPv4 packets:
> > > > >    \begin{lstlisting}
> > > > >    #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> > > > > @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >    #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
> > > > >    \end{lstlisting}
> > > > > +\subparagraph{Supported/enabled tunnel hash types}
> > > > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> > > > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> > > > > +hash type indicates that the hash is calculated over the inner header of
> > > > > +the encapsulated packet:
> > > > > +Hash type applicable for inner payload of the gre-encapsulated packet
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> > > > > +\end{lstlisting}
> > > > > +Hash type applicable for inner payload of the vxlan-encapsulated packet
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> > > > > +\end{lstlisting}
> > > > > +Hash type applicable for inner payload of the geneve-encapsulated packet
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> > > > > +\end{lstlisting}
> > are these the only tunnel types we can thinkably support?
> 
> This is the tunnel types our group currently uses, do you think we should
> add more tunnel types?

I would try to be inclusive, yes.

> > 
> > > > >    \subparagraph{IPv4 packets}
> > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
> > > > >    The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> > > > > @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >    (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
> > > > >    \end{itemize}
> > > > > +\subparagraph{Inner hash calculation of an encapsulated packet}
> > > > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > > > +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> > > > > +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> > > > > +/ Network Device / Device Operation / Processing of Incoming Packets /
> > > > > +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> > > > > +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> > > > > +
> > > > >    \paragraph{Hash reporting for incoming packets}
> > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> > > > > -
> > > > > -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> > > > > - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> > > > > -and \field{hash_value} with the value of calculated hash.
> > > > > +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> > > > > +hash for the packet, the device fills \field{hash_report} with the report type
> > > > > +of calculated hash, and \field{hash_value} with the value of calculated hash.
> > > > > +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> > > > > +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> > > > > +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
> > > > >    If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> > > > > -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> > > > > +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> > > > > +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
> > > > >    Possible values that the device can report in \field{hash_report} are defined below.
> > > > >    They correspond to supported hash types defined in
> > > > > @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > >    #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
> > > > >    \end{lstlisting}
> > > > > +\field{hash_report_tunnel} can report the type of the encapsulated
> > > > > +packet to the driver when the inner header hash is calculated.
> > > > > +Possible values that the device can report in \field{hash_report_tunnel_types}
> > > > > +are defined below.
> > > > > +They correspond to supported hash tunnel types defined in
> > > > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> > > > > +as follows:
> > > > > +
> > > > > +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> > > > > +\end{lstlisting}
> > > > > +
> > > > > +They correspond to supported hash types defined in
> > > > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > +
> > > > >    \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
> > > > >    The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> > > > > @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >    \begin{lstlisting}
> > > > >    struct virtio_net_hash_config {
> > > > >        le32 hash_types;
> > > > > +    le32 hash_tunnel_types;
> > > > >        le16 reserved[4];
> > wait a second. this must be reserved[2] now.
> > 
> 
> Thanks for pointing it out. I'll fix it.
> 
> > 
> > 
> > > > >        u8 hash_key_length;
> > > > >        u8 hash_key_data[hash_key_length];
> > > > > @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >    Field \field{hash_types} contains a bitmask of allowed hash types as
> > > > >    defined in
> > > > >    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> > > > > +
> > > > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > > > +
> > > > > +Initially the device has all hash types and hash tunnel types disabled and reports only
> > > > > +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
> > > > >    Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
> > > > >    defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> > > > > @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >    \begin{lstlisting}
> > > > >    struct virtio_net_rss_config {
> > > > >        le32 hash_types;
> > > > > +    le32 hash_tunnel_types;
> > > > >        le16 indirection_table_mask;
> > > > >        le16 unclassified_queue;
> > > > >        le16 indirection_table[indirection_table_length];
> > > > > @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > >    defined in
> > > > >    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > > > +
> > > > >    Field \field{indirection_table_mask} is a mask to be applied to
> > > > >    the calculated hash to produce an index in the
> > > > >    \field{indirection_table} array.
> > > > > diff --git a/introduction.tex b/introduction.tex
> > > > > index 287c5fc..ff01a9b 100644
> > > > > --- a/introduction.tex
> > > > > +++ b/introduction.tex
> > > > > @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
> > > > >    	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> > > > >        Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
> > > > >    	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> > > > > +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> > > > > +	Generic Routing Encapsulation
> > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> > > > > +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> > > > > +	Virtual eXtensible Local Area Network
> > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> > > > > +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> > > > > +	Generic Network Virtualization Encapsulation
> > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> > > > > +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> > > > > +	INTERNET PROTOCOL
> > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> > > > > +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> > > > > +	User Datagram Protocol
> > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> > > > > +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> > > > > +	TRANSMISSION CONTROL PROTOCOL
> > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> > > > > +
> > > > >    \end{longtable}
> > > > > -- 
> > > > > 2.19.1.6.gb485710b
> > > > ---------------------------------------------------------------------
> > > > 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] 20+ messages in thread

* Re: [virtio-comment] Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-17 14:25         ` Michael S. Tsirkin
@ 2023-02-17 15:36           ` Heng Qi
  2023-02-17 16:24             ` [virtio-comment] RE: [virtio-dev] " Parav Pandit
  0 siblings, 1 reply; 20+ messages in thread
From: Heng Qi @ 2023-02-17 15:36 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Parav Pandit,
	Cornelia Huck, Yuri Benditovich, Xuan Zhuo

On Fri, Feb 17, 2023 at 09:25:01AM -0500, Michael S. Tsirkin wrote:
> On Thu, Feb 16, 2023 at 10:11:58PM +0800, Heng Qi wrote:
> > 
> > 
> > 在 2023/2/16 下午7:59, Michael S. Tsirkin 写道:
> > > On Thu, Feb 16, 2023 at 03:20:17PM +0800, Heng Qi wrote:
> > > > 
> > > > 在 2023/2/14 上午6:20, Michael S. Tsirkin 写道:
> > > > > On Wed, Feb 08, 2023 at 05:08:36PM +0800, Heng Qi wrote:
> > > > > > If the tunnel is used to encapsulate the packets, the hash calculated
> > > > > > using the outer header of the receive packets is always fixed for the
> > > > > > same flow packets, i.e. they will be steered to the same receive queue.
> > > > > > 
> > > > > > We add a feature bit VIRTIO_NET_F_HASH_TUNNEL and related bitmasks
> > > > > > in \field{hash_tunnel_types}, which instructs the device to calculate the
> > > > > > hash using the inner headers of tunnel-encapsulated packets. Besides,
> > > > > > values in \field{hash_report_tunnel_types} are added to report tunnel types.
> > > > > > 
> > > > > > Note that VIRTIO_NET_F_HASH_TUNNEL only indicates the ability of the inner
> > > > > > header hash, and does not give the device the ability to use the hash value
> > > > > > to select a receiving queue to place the packet.
> > > > > This is the part I am missing. Where is this in the proposal?
> > > > The core function of the inner header hash feature is to provide a hash
> > > > value calculated using the inner header.
> > > > This is its semantics. We just tell the device that if this feature is
> > > > negotiated, the value of \field{hash_value}
> > > > comes from the inner header. VIRTIO_NET_F_HASH_REPORT is also such a
> > > > function, it tells the device that you
> > > > need to provide the hash value and hash type. If the device needs to use the
> > > > calculated hash value to select the
> > > > queue to place packets, then use VIRTIO_NET_F_RSS at the same time.
> > > > 
> > > > Thanks.
> > > So VIRTIO_NET_F_HASH_TUNNEL indeed does not give the device the ability to use the hash value
> > > to select a receiving queue to place the packet.
> > 
> > Yes.
> > 
> > > However, the new hash_tunnel_types introduced here do give the device
> > > this ability if enabled by VIRTIO_NET_F_RSS.
> > > 
> > > Asymmetrical.
> > 
> > supported_hash_types only means that the device has the ability to calculate
> > hash, and does not force the device
> > to use hash to select a queue and place the packets. Even if only
> > VIRTIO_NET_F_HASH_REPORT is negotiated and
> > VIRTIO_NET_F_RSS is not negotiated, the device can also have the ability to
> > calculate hash instead of selecting a
> > queue based on supported_hash_types.
> > 
> > VIRTIO_NET_F_HASH_TUNNEL is similar to VIRTIO_NET_F_HASH_REPORT, and
> > supported_tunnel_hash_types is also similar to supported_hash_types,
> 
> yes
> 
> > which has nothing to do with selecting a queue to place packets.
> 
> no
> 
> If you enable VIRTIO_NET_F_RSS then hash whatever it is
> will affect the queue selected. So "nothing to do" is wrong
> is it not?

Yes. And sorry I didn't express clearly, what I mean is that
VIRTIO_NET_F_HASH_TUNNEL provides the capability of inner header hash
calculation, and VIRTIO_NET_F_RSS may use this hash value to select a queue.

> 
> 
> > Take an example in the spec:
> > "The next field, \field{supported_hash_types} only exists if the device
> > supports hash calculation,
> > i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set."
> > 
> > > 
> > > To me, VIRTIO_NET_F_HASH_TUNNEL makes sense if we keep
> > > hash_report_tunnel. And Parav insists we should drop hash_report_tunnel.
> > 
> > If we really don't have a practical use case for hash_report_tunnel_type, we
> > can merge its type into hash_report.
> > 
> > > 
> > > So I have a suggestion:
> > > 
> > > Disconnect VIRTIO_NET_F_HASH_TUNNEL from hash calculation.
> > > Just make it imply that hash_report_tunnel is valid.
> > > 

Is there any better advantage of disconnecting VIRTIO_NET_F_HASH_TUNNEL from hash calculation?
Can you explain more clearly?
I think both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_RSS indicate by
default that the device can compute hash, controlling the presence of
related structures and hash_types. VIRTIO_NET_F_HASH_TUNNEL also makes
sense to do so.

> > > Whether hash is calculated over the inner header is controlled
> > > by the hash_report_tunnel_types.
> > 

We assume that hash_report_tunnel_types is still present in the next
version, but it only exists in virtio net hdr and should be populated by
the device after the hash calculation. hash_tunnel_types already
controls whether the device computes internal header hashes.

> > I don't really follow this, hash_report_tunnel_type is better off keeping it
> > "report" literally.
> 
> 
> Talking about VIRTIO_NET_F_HASH_TUNNEL here. Not
> hash_report_tunnel_type.

Ok.

Thanks.

> 
> > > 
> > > 
> > > 
> > > > > We currently have:
> > > > > 
> > > > > The device MUST determine the destination queue for a network packet as follows:
> > > > > \begin{itemize}
> > > > > \item Calculate the hash of the packet as defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets}.
> > > > > \item If the device did not calculate the hash for the specific packet, the device directs the packet to the receiveq specified by \field{unclassified_queue} of virtio_net_rss_config structure (value of 0 corresponds to receiveq1).
> > > > > \item Apply \field{indirection_table_mask} to the calculated hash and use the result as the index in the indirection table to get 0-based number of destination receiveq (value of 0 corresponds to receiveq1).
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/151
> > > > > > 
> > > > > > Reviewed-by: Jason Wang <jasowang@redhat.com>
> > > > > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > > ---
> > > > > > v7->v8:
> > > > > > 	1. Add supported_hash_tunnel_types. @Jason Wang, @Parav Pandit
> > > > > > 	2. Change hash_report_tunnel to hash_report_tunnel_types. @Parav Pandit
> > > > > > 	3. Removed re-definition for inner packet hashing. @Parav Pandit
> > > > > > 	4. Fix some typos. @Michael S . Tsirkin
> > > > > > 	5. Clarify some sentences. @Michael S . Tsirkin
> > > > > > 
> > > > > > v6->v7:
> > > > > > 	1. Modify the wording of some sentences for clarity. @Michael S. Tsirkin
> > > > > > 	2. Fix some syntax issues. @Michael S. Tsirkin
> > > > > > 
> > > > > > v5->v6:
> > > > > > 	1. Fix some syntax and capitalization issues. @Michael S. Tsirkin
> > > > > > 	2. Use encapsulated/encaptulation uniformly. @Michael S. Tsirkin
> > > > > > 	3. Move the links to introduction section. @Michael S. Tsirkin
> > > > > > 	4. Clarify some sentences. @Michael S. Tsirkin
> > > > > > 
> > > > > > v4->v5:
> > > > > > 	1. Clarify some paragraphs. @Cornelia Huck
> > > > > > 	2. Fix the u8 type. @Cornelia Huck
> > > > > > 
> > > > > > v3->v4:
> > > > > > 	1. Rename VIRTIO_NET_F_HASH_GRE_VXLAN_GENEVE_INNER to VIRTIO_NET_F_HASH_TUNNEL. @Jason Wang
> > > > > > 	2. Make things clearer. @Jason Wang @Michael S. Tsirkin
> > > > > > 	3. Keep the possibility to use inner hash for automatic receive steering. @Jason Wang
> > > > > > 	4. Add the "Tunnel packet" paragraph to avoid repeating the GRE etc. many times. @Michael S. Tsirkin
> > > > > > 
> > > > > > v2->v3:
> > > > > > 	1. Add a feature bit for GRE/VXLAN/GENEVE inner hash. @Jason Wang
> > > > > > 	2. Chang \field{hash_tunnel} to \field{hash_report_tunnel}. @Jason Wang, @Michael S. Tsirkin
> > > > > > 
> > > > > > v1->v2:
> > > > > > 	1. Remove the patch for the bitmask fix. @Michael S. Tsirkin
> > > > > > 	2. Clarify some paragraphs. @Jason Wang
> > > > > > 	3. Add \field{hash_tunnel} and VIRTIO_NET_HASH_REPORT_GRE. @Yuri Benditovich
> > > > > > 
> > > > > >    content.tex      | 135 ++++++++++++++++++++++++++++++++++++++++++-----
> > > > > >    introduction.tex |  19 +++++++
> > > > > >    2 files changed, 140 insertions(+), 14 deletions(-)
> > > > > > 
> > > > > > diff --git a/content.tex b/content.tex
> > > > > > index e863709..2598d96 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_HASH_TUNNEL(52)] Device supports inner header hash
> > > > > > +	for tunnel-encapsulated packets.
> > > > > > +
> > > > > >    \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
> > > > > >    \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > > > > > @@ -3095,7 +3098,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
> > > > > >     to several segments when each of these smaller packets has UDP header.
> > > > > >    \item[VIRTIO_NET_F_HASH_REPORT(57)] Device can report per-packet hash
> > > > > > -    value and a type of calculated hash.
> > > > > > +    value, a type of calculated hash, and, if VIRTIO_NET_F_HASH_TUNNEL
> > > > > > +    is negotiated, an encapsulation packet type.
> > > > > >    \item[VIRTIO_NET_F_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
> > > > > >        value. Device benefits from knowing the exact header length.
> > > > > > @@ -3140,6 +3144,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_HASH_TUNNEL] 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}
> > > > > > @@ -3199,20 +3204,27 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > > >            u8 rss_max_key_size;
> > > > > >            le16 rss_max_indirection_table_length;
> > > > > >            le32 supported_hash_types;
> > > > > > +        le32 supported_tunnel_hash_types;
> > > > > >    };
> > > > > >    \end{lstlisting}
> > > > > > -The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > > > > +The following field, \field{rss_max_key_size} only exists if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > > >    It specifies the maximum supported length of RSS key in bytes.
> > > > > >    The following field, \field{rss_max_indirection_table_length} only exists if VIRTIO_NET_F_RSS is set.
> > > > > >    It specifies the maximum number of 16-bit entries in RSS indirection table.
> > > > > >    The next field, \field{supported_hash_types} only exists if the device supports hash calculation,
> > > > > > -i.e. if VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT is set.
> > > > > > +i.e. if VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > > >    Field \field{supported_hash_types} contains the bitmask of supported hash types.
> > > > > >    See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types} for details of supported hash types.
> > > > > > +The next field, \field{supported_tunnel_hash_types} only exists if the device
> > > > > > +supports inner hash calculation, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> > > > > > +
> > > > > > +Field \field{supported_tunnel_hash_types} contains the bitmask of supported tunnel hash types.
> > > > > > +See \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types} for details of supported tunnel hash types.
> > > > > > +
> > > > > >    \devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> > > > > >    The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
> > > > > > @@ -3236,7 +3248,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
> > > > > >    negotiated.
> > > > > >    The device MUST set \field{rss_max_key_size} to at least 40, if it offers
> > > > > > -VIRTIO_NET_F_RSS or VIRTIO_NET_F_HASH_REPORT.
> > > > > > +VIRTIO_NET_F_RSS, VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL.
> > > > > >    The device MUST set \field{rss_max_indirection_table_length} to at least 128, if it offers
> > > > > >    VIRTIO_NET_F_RSS.
> > > > > > @@ -3384,9 +3396,10 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
> > > > > >            le16 csum_start;
> > > > > >            le16 csum_offset;
> > > > > >            le16 num_buffers;
> > > > > > -        le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > > -        le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > > -        le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > > +        le32 hash_value;              (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > > +        le16 hash_report;             (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > > +        u8 hash_report_tunnel_types;  (Only if VIRTIO_NET_F_HASH_REPORT negotiated, only valid if VIRTIO_NET_F_HASH_TUNNEL negotiated, otherwise reserved)
> > > > > > +        u8 padding_reserved;          (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> > > > > >    };
> > > > > >    \end{lstlisting}
> > > > > > @@ -3838,11 +3851,15 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >    \begin{itemize}
> > > > > >    \item The feature VIRTIO_NET_F_RSS was negotiated. The device uses the hash to determine the receive virtqueue to place incoming packets.
> > > > > >    \item The feature VIRTIO_NET_F_HASH_REPORT was negotiated. The device reports the hash value and the hash type with the packet.
> > > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device reports the encapsulation type as well.
> > > > > > +\item The feature VIRTIO_NET_F_HASH_TUNNEL was negotiated. The device supports inner hash calculation.
> > > > > >    \end{itemize}
> > > > > >    If the feature VIRTIO_NET_F_RSS was negotiated:
> > > > > >    \begin{itemize}
> > > > > >    \item The device uses \field{hash_types} of the virtio_net_rss_config structure as 'Enabled hash types' bitmask.
> > > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > > > > +	virtio_net_rss_config structure as 'Enabled hash tunnel types' bitmask.
> > > > > >    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_rss_config structure (see
> > > > > >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}).
> > > > > >    \end{itemize}
> > > > > > @@ -3850,11 +3867,13 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >    If the feature VIRTIO_NET_F_RSS was not negotiated:
> > > > > >    \begin{itemize}
> > > > > >    \item The device uses \field{hash_types} of the virtio_net_hash_config structure as 'Enabled hash types' bitmask.
> > > > > > +	If additionally VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device uses \field{hash_tunnel_types} of the
> > > > > > +	virtio_net_hash_config structure as 'Enabled hash tunnel types' bitmask.
> > > > > >    \item The device uses a key as defined in \field{hash_key_data} and \field{hash_key_length} of the virtio_net_hash_config structure (see
> > > > > >    \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}).
> > > > > >    \end{itemize}
> > > > > > -Note that if the device offers VIRTIO_NET_F_HASH_REPORT, even if it supports only one pair of virtqueues, it MUST support
> > > > > > +Note that if the device offers VIRTIO_NET_F_HASH_REPORT or VIRTIO_NET_F_HASH_TUNNEL, even if it supports only one pair of virtqueues, it MUST support
> > > > > >    at least one of commands of VIRTIO_NET_CTRL_MQ class to configure reported hash parameters:
> > > > > >    \begin{itemize}
> > > > > >    \item If the device offers VIRTIO_NET_F_RSS, it MUST support VIRTIO_NET_CTRL_MQ_RSS_CONFIG command per
> > > > > > @@ -3863,8 +3882,37 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >     \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Hash calculation}.
> > > > > >    \end{itemize}
> > > > > > +\subparagraph{Tunnel/Encapsulated packet}
> > > > > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> > > > > > +A tunnel packet is encapsulated from the original packet based on the tunneling
> > > > > > +protocol (only a single level of encapsulation is currently supported). The
> > > > > > +encapsulated packet contains an outer header and an inner header, and the device
> > > > > > +calculates the hash over either the inner header or the outer header.
> > > > > > +
> > > > > > +When the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > > > > +encapsulation type is set in \field{hash_tunnel_types}, the hash for a specific
> > > > > > +type of encapsulated packet is calculated over the inner as opposed to outer header.
> > > > > > +Supported encapsulation types are listed in \ref{sec:Device Types / Network Device /
> > > > > > +Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets /
> > > > > > +Supported/enabled hash tunnel types}.
> > > > > > +
> > > > > > +If both VIRTIO_NET_F_HASH_REPORT and VIRTIO_NET_F_HASH_TUNNEL are negotiated,
> > > > > > +and the corresponding encapsulation type is set in \field{hash_tunnel_types},
> > > > > > +the device supports inner hash calculation for the encapsulated packet,
> > > > > > +For example, if the encapsulated packets \hyperref[intro:GRE]{[GRE]},
> > > > > > +\hyperref[intro:VXLAN]{[VXLAN]} and \hyperref[intro:GENEVE]{[GENEVE]} are hashed in inner
> > > > > > +headers, then \field{hash_tunnel_types} should be set to VIRTIO_NET_HASH_TUNNEL_TYPE_{GRE, VXLAN, GENEVE}
> > > > > > +respectively, and \field{hash_report_tunnel_types} should be respectively set to
> > > > > > +VIRTIO_NET_HASH_TUNNEL_REPORT_{GRE, VXLAN, GENEVE}.
> > > > > > +
> > > > > > +If VIRTIO_NET_F_HASH_REPORT is negotiated but VIRTIO_NET_F_HASH_TUNNEL is not
> > > > > > +negotiated, the device calculates the hash over the outer header, and \field{hash_report}
> > > > > > +reports the hash type. \field{hash_report_tunnel_types} is no longer valid.
> > > > > > +
> > > > > >    \subparagraph{Supported/enabled hash types}
> > > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> > > > > > +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]},
> > > > > > +\hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
> > > > > >    Hash types applicable for IPv4 packets:
> > > > > >    \begin{lstlisting}
> > > > > >    #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> > > > > > @@ -3884,6 +3932,24 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >    #define VIRTIO_NET_HASH_TYPE_UDP_EX            (1 << 8)
> > > > > >    \end{lstlisting}
> > > > > > +\subparagraph{Supported/enabled tunnel hash types}
> > > > > > +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled tunnel hash types}
> > > > > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated, the encapsulation
> > > > > > +hash type indicates that the hash is calculated over the inner header of
> > > > > > +the encapsulated packet:
> > > > > > +Hash type applicable for inner payload of the gre-encapsulated packet
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE         (1 << 0)
> > > > > > +\end{lstlisting}
> > > > > > +Hash type applicable for inner payload of the vxlan-encapsulated packet
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 1)
> > > > > > +\end{lstlisting}
> > > > > > +Hash type applicable for inner payload of the geneve-encapsulated packet
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 2)
> > > > > > +\end{lstlisting}
> > > are these the only tunnel types we can thinkably support?
> > 
> > This is the tunnel types our group currently uses, do you think we should
> > add more tunnel types?
> 
> I would try to be inclusive, yes.
> 
> > > 
> > > > > >    \subparagraph{IPv4 packets}
> > > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv4 packets}
> > > > > >    The device calculates the hash on IPv4 packets according to 'Enabled hash types' bitmask as follows:
> > > > > > @@ -3975,15 +4041,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >    (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
> > > > > >    \end{itemize}
> > > > > > +\subparagraph{Inner hash calculation of an encapsulated packet}
> > > > > > +If the feature VIRTIO_NET_F_HASH_TUNNEL is negotiated and the corresponding
> > > > > > +encapsulation hash type is set in \field{hash_tunnel_types}, the device calculates the
> > > > > > +hash on the inner header of an encapsulated packet (See \ref{sec:Device Types
> > > > > > +/ Network Device / Device Operation / Processing of Incoming Packets /
> > > > > > +Hash calculation for incoming packets / Tunnel/Encapsulated packet}), and
> > > > > > +\field{hash_report_tunnel_types} contains the valid outer tunnel type.
> > > > > > +
> > > > > >    \paragraph{Hash reporting for incoming packets}
> > > > > >    \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
> > > > > > -
> > > > > > -If VIRTIO_NET_F_HASH_REPORT was negotiated and
> > > > > > - the device has calculated the hash for the packet, the device fills \field{hash_report} with the report type of calculated hash
> > > > > > -and \field{hash_value} with the value of calculated hash.
> > > > > > +If VIRTIO_NET_F_HASH_REPORT was negotiated and the device has calculated the
> > > > > > +hash for the packet, the device fills \field{hash_report} with the report type
> > > > > > +of calculated hash, and \field{hash_value} with the value of calculated hash.
> > > > > > +Also, if VIRTIO_NET_F_HASH_TUNNEL was negotiated, the device needs to fill
> > > > > > +\field{hash_report_tunnel_types} with the report type of the encapsulated packet,
> > > > > > +and it is set to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE for the unencapsulated packet.
> > > > > >    If VIRTIO_NET_F_HASH_REPORT was negotiated but due to any reason the
> > > > > > -hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE.
> > > > > > +hash was not calculated, the device sets \field{hash_report} to VIRTIO_NET_HASH_REPORT_NONE,
> > > > > > +and sets \field{hash_report_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_REPORT_NONE.
> > > > > >    Possible values that the device can report in \field{hash_report} are defined below.
> > > > > >    They correspond to supported hash types defined in
> > > > > > @@ -4005,6 +4082,26 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
> > > > > >    #define VIRTIO_NET_HASH_REPORT_UDPv6_EX        9
> > > > > >    \end{lstlisting}
> > > > > > +\field{hash_report_tunnel} can report the type of the encapsulated
> > > > > > +packet to the driver when the inner header hash is calculated.
> > > > > > +Possible values that the device can report in \field{hash_report_tunnel_types}
> > > > > > +are defined below.
> > > > > > +They correspond to supported hash tunnel types defined in
> > > > > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}
> > > > > > +as follows:
> > > > > > +
> > > > > > +VIRTIO_NET_HASH_TUNNEL_TYPE_XXX = 1 << (VIRTIO_NET_HASH_TUNNEL_REPORT_XXX -1)
> > > > > > +
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_NONE     0
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GRE      1
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_VXLAN    2
> > > > > > +#define VIRTIO_NET_HASH_TUNNEL_REPORT_GENEVE   3
> > > > > > +\end{lstlisting}
> > > > > > +
> > > > > > +They correspond to supported hash types defined in
> > > > > > +\ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > > +
> > > > > >    \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue}
> > > > > >    The driver uses the control virtqueue (if VIRTIO_NET_F_CTRL_VQ is
> > > > > > @@ -4364,6 +4461,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > >    \begin{lstlisting}
> > > > > >    struct virtio_net_hash_config {
> > > > > >        le32 hash_types;
> > > > > > +    le32 hash_tunnel_types;
> > > > > >        le16 reserved[4];
> > > wait a second. this must be reserved[2] now.
> > > 
> > 
> > Thanks for pointing it out. I'll fix it.
> > 
> > > 
> > > 
> > > > > >        u8 hash_key_length;
> > > > > >        u8 hash_key_data[hash_key_length];
> > > > > > @@ -4372,7 +4470,12 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > >    Field \field{hash_types} contains a bitmask of allowed hash types as
> > > > > >    defined in
> > > > > >    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > > -Initially the device has all hash types disabled and reports only VIRTIO_NET_HASH_REPORT_NONE.
> > > > > > +
> > > > > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > > > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > > > > +
> > > > > > +Initially the device has all hash types and hash tunnel types disabled and reports only
> > > > > > +VIRTIO_NET_HASH_REPORT_NONE and VIRTIO_NET_HASH_TUNNEL_REPORT_NONE respectively.
> > > > > >    Field \field{reserved} MUST contain zeroes. It is defined to make the structure to match the layout of virtio_net_rss_config structure,
> > > > > >    defined in \ref{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS)}.
> > > > > > @@ -4390,6 +4493,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > >    \begin{lstlisting}
> > > > > >    struct virtio_net_rss_config {
> > > > > >        le32 hash_types;
> > > > > > +    le32 hash_tunnel_types;
> > > > > >        le16 indirection_table_mask;
> > > > > >        le16 unclassified_queue;
> > > > > >        le16 indirection_table[indirection_table_length];
> > > > > > @@ -4402,6 +4506,9 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > > > >    defined in
> > > > > >    \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}.
> > > > > > +Field \field{hash_tunnel_types} contains a bitmask of allowed hash tunnel types as
> > > > > > +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> > > > > > +
> > > > > >    Field \field{indirection_table_mask} is a mask to be applied to
> > > > > >    the calculated hash to produce an index in the
> > > > > >    \field{indirection_table} array.
> > > > > > diff --git a/introduction.tex b/introduction.tex
> > > > > > index 287c5fc..ff01a9b 100644
> > > > > > --- a/introduction.tex
> > > > > > +++ b/introduction.tex
> > > > > > @@ -98,6 +98,25 @@ \section{Normative References}\label{sec:Normative References}
> > > > > >    	\phantomsection\label{intro:SEC1}\textbf{[SEC1]} &
> > > > > >        Standards for Efficient Cryptography Group(SECG), ``SEC1: Elliptic Cureve Cryptography'', Version 1.0, September 2000.
> > > > > >    	\newline\url{https://www.secg.org/sec1-v2.pdf}\\
> > > > > > +	\phantomsection\label{intro:GRE}\textbf{[GRE]} &
> > > > > > +	Generic Routing Encapsulation
> > > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> > > > > > +	\phantomsection\label{intro:VXLAN}\textbf{[VXLAN]} &
> > > > > > +	Virtual eXtensible Local Area Network
> > > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> > > > > > +	\phantomsection\label{intro:GENEVE}\textbf{[GENEVE]} &
> > > > > > +	Generic Network Virtualization Encapsulation
> > > > > > +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> > > > > > +	\phantomsection\label{intro:IP}\textbf{[IP]} &
> > > > > > +	INTERNET PROTOCOL
> > > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc791}\\
> > > > > > +	\phantomsection\label{intro:UDP}\textbf{[UDP]} &
> > > > > > +	User Datagram Protocol
> > > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc768}\\
> > > > > > +	\phantomsection\label{intro:TCP}\textbf{[TCP]} &
> > > > > > +	TRANSMISSION CONTROL PROTOCOL
> > > > > > +	\newline\url{https://www.rfc-editor.org/rfc/rfc793}\\
> > > > > > +
> > > > > >    \end{longtable}
> > > > > > -- 
> > > > > > 2.19.1.6.gb485710b
> > > > > ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 
> 
> 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	[flat|nested] 20+ messages in thread

* [virtio-comment] RE: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-17 15:36           ` [virtio-comment] " Heng Qi
@ 2023-02-17 16:24             ` Parav Pandit
  2023-02-17 16:32               ` [virtio-comment] " Heng Qi
  0 siblings, 1 reply; 20+ messages in thread
From: Parav Pandit @ 2023-02-17 16:24 UTC (permalink / raw)
  To: Heng Qi, Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo


> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
> Behalf Of Heng Qi

[..]

> We assume that hash_report_tunnel_types is still present in the next version,

I am little lost.
I thought we all agreed that reporting just the tunnel type in data path path virtio_net_hdr is not useful to sw.
And hence we should omit in the virtio_net_hdr.

Did I miss the motivation to add it back?
If not, probably it is better to review and discuss v9 without it, which will be easier to discuss.

> but it only exists in virtio net hdr and should be populated by the device after
> the hash calculation. hash_tunnel_types already controls whether the device
> computes internal header hashes.
> 
> > > I don't really follow this, hash_report_tunnel_type is better off
> > > keeping it "report" literally.
> >
> >
> > Talking about VIRTIO_NET_F_HASH_TUNNEL here. Not
> > hash_report_tunnel_type.

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	[flat|nested] 20+ messages in thread

* [virtio-comment] Re: [virtio-dev] Re: [virtio-comment] Re: [virtio-dev] Re: [PATCH v8] virtio-net: support inner header hash
  2023-02-17 16:24             ` [virtio-comment] RE: [virtio-dev] " Parav Pandit
@ 2023-02-17 16:32               ` Heng Qi
  0 siblings, 0 replies; 20+ messages in thread
From: Heng Qi @ 2023-02-17 16:32 UTC (permalink / raw)
  To: Parav Pandit, Michael S. Tsirkin
  Cc: virtio-comment, virtio-dev, Jason Wang, Cornelia Huck,
	Yuri Benditovich, Xuan Zhuo


在 2023/2/18 上午12:24, Parav Pandit 写道:
>> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
>> Behalf Of Heng Qi
> [..]
>
>> We assume that hash_report_tunnel_types is still present in the next version,
> I am little lost.

Hi, Parav.

You are not lost. I'm just answering some of Michael's questions and 
making assumptions. :)

> I thought we all agreed that reporting just the tunnel type in data path path virtio_net_hdr is not useful to sw.
> And hence we should omit in the virtio_net_hdr.
>
> Did I miss the motivation to add it back?
> If not, probably it is better to review and discuss v9 without it, which will be easier to discuss.
>
>> but it only exists in virtio net hdr and should be populated by the device after
>> the hash calculation. hash_tunnel_types already controls whether the device
>> computes internal header hashes.
>>
>>>> I don't really follow this, hash_report_tunnel_type is better off
>>>> keeping it "report" literally.
>>>
>>> Talking about VIRTIO_NET_F_HASH_TUNNEL here. Not
>>> hash_report_tunnel_type.

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	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-02-17 16:32 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-08  9:08 [PATCH v8] virtio-net: support inner header hash Heng Qi
2023-02-13 13:05 ` [virtio-comment] " Heng Qi
2023-02-13 20:43   ` Michael S. Tsirkin
2023-02-13 22:29     ` Parav Pandit
2023-02-16  6:48     ` [virtio-comment] Re: [virtio-dev] " Heng Qi
2023-02-13 21:42   ` Parav Pandit
2023-02-13 22:23     ` Michael S. Tsirkin
2023-02-16  7:23       ` [virtio-comment] Re: [virtio-dev] " Heng Qi
2023-02-16  6:57     ` [virtio-dev] " Heng Qi
2023-02-16 13:25       ` [virtio-comment] " Parav Pandit
2023-02-16 14:19         ` Heng Qi
2023-02-16 14:23           ` Parav Pandit
2023-02-13 22:20 ` Michael S. Tsirkin
2023-02-16  7:20   ` [virtio-dev] " Heng Qi
2023-02-16 11:59     ` Michael S. Tsirkin
2023-02-16 14:11       ` Heng Qi
2023-02-17 14:25         ` Michael S. Tsirkin
2023-02-17 15:36           ` [virtio-comment] " Heng Qi
2023-02-17 16:24             ` [virtio-comment] RE: [virtio-dev] " Parav Pandit
2023-02-17 16:32               ` [virtio-comment] " Heng Qi

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.