virtio-dev.lists.oasis-open.org archive mirror
 help / color / mirror / Atom feed
* [virtio-dev] [PATCH v12] virtio-net: support inner header hash
@ 2023-04-03  4:58 Heng Qi
  2023-04-03  5:08 ` Heng Qi
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Heng Qi @ 2023-04-03  4:58 UTC (permalink / raw)
  To: virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Parav Pandit, Yuri Benditovich,
	Xuan Zhuo

1. Currently, a received encapsulated packet has an outer and an inner header, but
the virtio device is unable to calculate the hash for the inner header. The same
flow can traverse through different tunnels, resulting in the encapsulated
packets being spread across multiple receive queues (refer to the figure below).
However, in certain scenarios, we may need to direct these encapsulated packets of
the same flow to a single receive queue. This facilitates the processing
of the flow by the same CPU to improve performance (warm caches, less locking, etc.).

               client1                    client2
                  |        +-------+         |
                  +------->|tunnels|<--------+
                           +-------+
                              |  |
                              v  v
                      +-----------------+
                      | monitoring host |
                      +-----------------+

To achieve this, the device can calculate a suitable hash based on the inner headers
of this flow, for example using the Toeplitz combined with a symmetric hash key.

2. For legacy systems, they may lack entropy fields which modern protocols have in
the outer header, resulting in multiple flows with the same outer header but
different inner headers being directed to the same receive queue. This results in
poor receive performance.

To address this limitation, inner header hash can be used to enable the device to advertise
the capability to calculate the hash for the inner packet, regaining better receive performance.

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
v11->v12:
	1. Add a command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
	2. Refine the commit log. @Michael S . Tsirkin
	3. Add some tunnel types.

v10->v11:
	1. Revise commit log for clarity for readers.
	2. Some modifications to avoid undefined terms. @Parav Pandit
	3. Change VIRTIO_NET_F_HASH_TUNNEL dependency. @Parav Pandit
	4. Add the normative statements. @Parav Pandit

v9->v10:
	1. Removed hash_report_tunnel related information. @Parav Pandit
	2. Re-describe the limitations of QoS for tunneling.
	3. Some clarification.

v8->v9:
	1. Merge hash_report_tunnel_types into hash_report. @Parav Pandit
	2. Add tunnel security section. @Michael S . Tsirkin
	3. Add VIRTIO_NET_F_HASH_REPORT_TUNNEL.
	4. Fix some typos.
	5. Add more tunnel types. @Michael S . Tsirkin

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

 device-types/net/description.tex        | 143 ++++++++++++++++++++++++
 device-types/net/device-conformance.tex |   1 +
 device-types/net/driver-conformance.tex |   1 +
 introduction.tex                        |  44 ++++++++
 4 files changed, 189 insertions(+)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 0500bb6..121585e 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -83,6 +83,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.
@@ -139,6 +142,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 along with VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
 \end{description}
 
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
@@ -198,6 +202,7 @@ \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.
@@ -212,6 +217,12 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
 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.
 
+Field \field{supported_tunnel_hash_types} only exists if the device
+supports inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
+
+Filed \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 encapsulation 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,
@@ -848,6 +859,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 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.
+\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
 \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}
@@ -855,6 +867,7 @@ \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.
+\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
 \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}
@@ -870,6 +883,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
 
 \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)
@@ -980,6 +995,133 @@ \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}
 
+\paragraph{Inner Header Hash}
+\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Inner Header Hash}
+
+If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and
+the driver can configure the inner header hash calculation for encapsulated packets \ref{sec:Device Types / Network Device / Device OperatiHn / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
+by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the class VIRTIO_NET_CTRL_MQ.
+The command sets \field{hash_tunnel_types} in the structure virtio_net_hash_tunnel_config.
+
+struct virtio_net_hash_tunnel_config {
+    le32 hash_tunnel_types;
+};
+
+Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
+
+\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.
+
+If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received encapsulated packet's
+outer header matches one of the supported \field{hash_tunnel_types}, the hash
+of the inner header is calculated.
+
+Supported encapsulated packet types:
+\begin{itemize}
+\item \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]}
+\item \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]}
+\item \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]}
+\item \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]}
+\item \hyperref[intro:vxlan]{[VXLAN]}
+\item \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]}
+\item \hyperref[intro:geneve]{[GENEVE]}
+\item \hyperref[intro:ipip]{[IPIP]}
+\item \hyperref[intro:nvgre]{[NVGRE]}
+\item \hyperref[intro:sit]{[STT]}
+\end{itemize}
+
+If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type is not included in
+\field{hash_tunnel_types}, the hash of the outer header is calculated for the received encapsulated packet.
+
+The hash is calculated for the received non-encapsulated packet as if VIRTIO_NET_F_HASH_TUNNEL was not negotiated.
+
+\subparagraph{Supported/enabled encapsulation hash types}
+\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled encapsulation hash types}
+
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_NONE        (1 << 0)
+\end{lstlisting}
+
+Supported encapsulation hash types:
+Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784    (1 << 1)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890    (1 << 2)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676    (1 << 3)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP     (1 << 4)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:vxlan]{[VXLAN]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 5)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 6)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:geneve]{[GENEVE]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 7)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:ipip]{[IPIP]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP        (1 << 8)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:nvgre]{[NVGRE]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE       (1 << 9)
+\end{lstlisting}
+Hash type applicable for inner payload of the \hyperref[intro:stt]{[STT]} packet:
+\begin{lstlisting}
+#define VIRTIO_NET_HASH_TUNNEL_TYPE_STT         (1 << 10)
+\end{lstlisting}
+
+\subparagraph{Tunnel QoS limitation}
+When a specific receive queue is shared by multiple tunnels to receive encapsulated packets,
+there is no quality of service (QoS) for these packets. For example, when the packets of certain
+tunnels are spread across multiple receive queues, these receive queues may have an unbalanced
+amount of packets. This can cause a specific receive queue to become full, resulting in packet loss.
+
+Possible mitigations:
+\begin{itemize}
+\item Use a tool with good forwarding performance to keep the receive queue from filling up.
+\item If the QoS is unavailable, the driver can set \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
+      to disable inner header hash for encapsulated packets.
+\item Choose a hash key that can avoid queue collisions.
+\item Perform appropriate QoS before packets consume the receive buffers of the receive queues.
+\end{itemize}
+
+The limitations mentioned above exist with/without the inner header hash.
+
+\devicenormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
+
+The device MUST calculate the outer header hash if the received encapsulated packet has an encapsulation type not in \field{supported_tunnel_hash_types}.
+
+The device MUST respond to the VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG command with VIRTIO_NET_ERR if the device
+received an unrecognized or unsupported VIRTIO_NET_HASH_TUNNEL_TYPE_ flag.
+
+Upon reset, the device MUST initialize \field{hash_tunnel_type} to 0.
+
+\drivernormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
+
+The driver MUST have negotiated the feature VIRTIO_NET_F_HASH_TUNNEL when issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
+
+The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that are not supported by the device.
+
 \paragraph{Hash reporting for incoming packets}
 \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
 
@@ -1308,6 +1450,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
  #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic receive steering)
  #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable receive steering)
  #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable hash calculation)
+ #define VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG       3 (for configurable inner header hash)
 \end{lstlisting}
 
 If more than one multiqueue mode is negotiated, the resulting device configuration is defined by the last command sent by the driver.
diff --git a/device-types/net/device-conformance.tex b/device-types/net/device-conformance.tex
index 54f6783..f88f48b 100644
--- a/device-types/net/device-conformance.tex
+++ b/device-types/net/device-conformance.tex
@@ -14,4 +14,5 @@
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
 \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
+\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
 \end{itemize}
diff --git a/device-types/net/driver-conformance.tex b/device-types/net/driver-conformance.tex
index 97d0cc1..9d853d9 100644
--- a/device-types/net/driver-conformance.tex
+++ b/device-types/net/driver-conformance.tex
@@ -14,4 +14,5 @@
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
 \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
+\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
 \end{itemize}
diff --git a/introduction.tex b/introduction.tex
index 287c5fc..36b620f 100644
--- a/introduction.tex
+++ b/introduction.tex
@@ -99,6 +99,50 @@ \section{Normative References}\label{sec:Normative References}
     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_rfc2784}\textbf{[GRE_rfc2784]} &
+    Generic Routing Encapsulation. This protocol is only specified for IPv4 and used as either the payload or delivery protocol.
+	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
+	\phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
+    Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This protocol describes extensions by which two fields, Key and
+    Sequence Number, can be optionally carried in the GRE Header \ref{intro:gre_rfc2784}.
+	\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
+	\phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
+    IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is specified for IPv6 and used as either the payload or
+    delivery protocol. Note that this does not change the GRE header format or any behaviors specified by RFC 2784 or RFC 2890.
+	\newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
+	\phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
+    GRE-in-UDP Encapsulation. This specifies a method of encapsulating network protocol packets within GRE and UDP headers.
+    This GRE-in-UDP encapsulation allows the UDP source port field to be used as an entropy field. This protocol is specified for IPv4 and IPv6,
+    and used as either the payload or delivery protocol.
+	\newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
+	\phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
+    Virtual eXtensible Local Area Network.
+	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
+	\phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
+    Generic Protocol Extension for VXLAN. This protocol describes extending Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.
+	\newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
+	\phantomsection\label{intro:geneve}\textbf{[GENEVE]} &
+    Generic Network Virtualization Encapsulation.
+	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
+	\phantomsection\label{intro:ipip}\textbf{[IPIP]} &
+    IP Encapsulation within IP.
+	\newline\url{https://www.rfc-editor.org/rfc/rfc2003}\\
+	\phantomsection\label{intro:nvgre}\textbf{[NVGRE]} &
+    NVGRE: Network Virtualization Using Generic Routing Encapsulation
+	\newline\url{https://www.rfc-editor.org/rfc/rfc7637.html}\\
+	\phantomsection\label{intro:stt}\textbf{[STT]} &
+    Stateless Transport Tunneling. STT is particularly useful when some tunnel endpoints are in end-systems, as it utilizes the capabilities
+    of the network interface card to improve performance.
+	\newline\url{https://www.ietf.org/archive/id/draft-davie-stt-08.txt}\\
+	\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}
 
 \section{Non-Normative References}
-- 
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 related	[flat|nested] 17+ messages in thread

* Re: [virtio-dev] [PATCH v12] virtio-net: support inner header hash
  2023-04-03  4:58 [virtio-dev] [PATCH v12] virtio-net: support inner header hash Heng Qi
@ 2023-04-03  5:08 ` Heng Qi
  2023-04-11 12:08 ` [virtio-dev] Re: [virtio-comment] " Heng Qi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2023-04-03  5:08 UTC (permalink / raw)
  To: virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Parav Pandit, Yuri Benditovich,
	Xuan Zhuo

Hi Michael.

I remember that we should make virtio support XOR hashing, this is 
already on my work plan list, and I was wondering
if you have any other comments or opinions on supporting XOR hashing?

Thanks.

在 2023/4/3 下午12:58, Heng Qi 写道:
> 1. Currently, a received encapsulated packet has an outer and an inner header, but
> the virtio device is unable to calculate the hash for the inner header. The same
> flow can traverse through different tunnels, resulting in the encapsulated
> packets being spread across multiple receive queues (refer to the figure below).
> However, in certain scenarios, we may need to direct these encapsulated packets of
> the same flow to a single receive queue. This facilitates the processing
> of the flow by the same CPU to improve performance (warm caches, less locking, etc.).
>
>                 client1                    client2
>                    |        +-------+         |
>                    +------->|tunnels|<--------+
>                             +-------+
>                                |  |
>                                v  v
>                        +-----------------+
>                        | monitoring host |
>                        +-----------------+
>
> To achieve this, the device can calculate a suitable hash based on the inner headers
> of this flow, for example using the Toeplitz combined with a symmetric hash key.
>
> 2. For legacy systems, they may lack entropy fields which modern protocols have in
> the outer header, resulting in multiple flows with the same outer header but
> different inner headers being directed to the same receive queue. This results in
> poor receive performance.
>
> To address this limitation, inner header hash can be used to enable the device to advertise
> the capability to calculate the hash for the inner packet, regaining better receive performance.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> v11->v12:
> 	1. Add a command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> 	2. Refine the commit log. @Michael S . Tsirkin
> 	3. Add some tunnel types.
>
> v10->v11:
> 	1. Revise commit log for clarity for readers.
> 	2. Some modifications to avoid undefined terms. @Parav Pandit
> 	3. Change VIRTIO_NET_F_HASH_TUNNEL dependency. @Parav Pandit
> 	4. Add the normative statements. @Parav Pandit
>
> v9->v10:
> 	1. Removed hash_report_tunnel related information. @Parav Pandit
> 	2. Re-describe the limitations of QoS for tunneling.
> 	3. Some clarification.
>
> v8->v9:
> 	1. Merge hash_report_tunnel_types into hash_report. @Parav Pandit
> 	2. Add tunnel security section. @Michael S . Tsirkin
> 	3. Add VIRTIO_NET_F_HASH_REPORT_TUNNEL.
> 	4. Fix some typos.
> 	5. Add more tunnel types. @Michael S . Tsirkin
>
> 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
>
>   device-types/net/description.tex        | 143 ++++++++++++++++++++++++
>   device-types/net/device-conformance.tex |   1 +
>   device-types/net/driver-conformance.tex |   1 +
>   introduction.tex                        |  44 ++++++++
>   4 files changed, 189 insertions(+)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 0500bb6..121585e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -83,6 +83,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.
> @@ -139,6 +142,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 along with VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
>   \end{description}
>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -198,6 +202,7 @@ \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.
> @@ -212,6 +217,12 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>   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.
>   
> +Field \field{supported_tunnel_hash_types} only exists if the device
> +supports inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> +
> +Filed \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 encapsulation 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,
> @@ -848,6 +859,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -855,6 +867,7 @@ \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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -870,6 +883,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   
>   \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)
> @@ -980,6 +995,133 @@ \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}
>   
> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and
> +the driver can configure the inner header hash calculation for encapsulated packets \ref{sec:Device Types / Network Device / Device OperatiHn / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the class VIRTIO_NET_CTRL_MQ.
> +The command sets \field{hash_tunnel_types} in the structure virtio_net_hash_tunnel_config.
> +
> +struct virtio_net_hash_tunnel_config {
> +    le32 hash_tunnel_types;
> +};
> +
> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
> +
> +\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.
> +
> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received encapsulated packet's
> +outer header matches one of the supported \field{hash_tunnel_types}, the hash
> +of the inner header is calculated.
> +
> +Supported encapsulated packet types:
> +\begin{itemize}
> +\item \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]}
> +\item \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]}
> +\item \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]}
> +\item \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]}
> +\item \hyperref[intro:vxlan]{[VXLAN]}
> +\item \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]}
> +\item \hyperref[intro:geneve]{[GENEVE]}
> +\item \hyperref[intro:ipip]{[IPIP]}
> +\item \hyperref[intro:nvgre]{[NVGRE]}
> +\item \hyperref[intro:sit]{[STT]}
> +\end{itemize}
> +
> +If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type is not included in
> +\field{hash_tunnel_types}, the hash of the outer header is calculated for the received encapsulated packet.
> +
> +The hash is calculated for the received non-encapsulated packet as if VIRTIO_NET_F_HASH_TUNNEL was not negotiated.
> +
> +\subparagraph{Supported/enabled encapsulation hash types}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled encapsulation hash types}
> +
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NONE        (1 << 0)
> +\end{lstlisting}
> +
> +Supported encapsulation hash types:
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784    (1 << 1)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890    (1 << 2)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676    (1 << 3)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP     (1 << 4)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan]{[VXLAN]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 5)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 6)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:geneve]{[GENEVE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 7)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:ipip]{[IPIP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP        (1 << 8)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:nvgre]{[NVGRE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE       (1 << 9)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:stt]{[STT]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_STT         (1 << 10)
> +\end{lstlisting}
> +
> +\subparagraph{Tunnel QoS limitation}
> +When a specific receive queue is shared by multiple tunnels to receive encapsulated packets,
> +there is no quality of service (QoS) for these packets. For example, when the packets of certain
> +tunnels are spread across multiple receive queues, these receive queues may have an unbalanced
> +amount of packets. This can cause a specific receive queue to become full, resulting in packet loss.
> +
> +Possible mitigations:
> +\begin{itemize}
> +\item Use a tool with good forwarding performance to keep the receive queue from filling up.
> +\item If the QoS is unavailable, the driver can set \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> +      to disable inner header hash for encapsulated packets.
> +\item Choose a hash key that can avoid queue collisions.
> +\item Perform appropriate QoS before packets consume the receive buffers of the receive queues.
> +\end{itemize}
> +
> +The limitations mentioned above exist with/without the inner header hash.
> +
> +\devicenormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The device MUST calculate the outer header hash if the received encapsulated packet has an encapsulation type not in \field{supported_tunnel_hash_types}.
> +
> +The device MUST respond to the VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG command with VIRTIO_NET_ERR if the device
> +received an unrecognized or unsupported VIRTIO_NET_HASH_TUNNEL_TYPE_ flag.
> +
> +Upon reset, the device MUST initialize \field{hash_tunnel_type} to 0.
> +
> +\drivernormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The driver MUST have negotiated the feature VIRTIO_NET_F_HASH_TUNNEL when issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> +
> +The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that are not supported by the device.
> +
>   \paragraph{Hash reporting for incoming packets}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>   
> @@ -1308,6 +1450,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>    #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic receive steering)
>    #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable receive steering)
>    #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable hash calculation)
> + #define VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG       3 (for configurable inner header hash)
>   \end{lstlisting}
>   
>   If more than one multiqueue mode is negotiated, the resulting device configuration is defined by the last command sent by the driver.
> diff --git a/device-types/net/device-conformance.tex b/device-types/net/device-conformance.tex
> index 54f6783..f88f48b 100644
> --- a/device-types/net/device-conformance.tex
> +++ b/device-types/net/device-conformance.tex
> @@ -14,4 +14,5 @@
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
>   \end{itemize}
> diff --git a/device-types/net/driver-conformance.tex b/device-types/net/driver-conformance.tex
> index 97d0cc1..9d853d9 100644
> --- a/device-types/net/driver-conformance.tex
> +++ b/device-types/net/driver-conformance.tex
> @@ -14,4 +14,5 @@
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
>   \end{itemize}
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..36b620f 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -99,6 +99,50 @@ \section{Normative References}\label{sec:Normative References}
>       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_rfc2784}\textbf{[GRE_rfc2784]} &
> +    Generic Routing Encapsulation. This protocol is only specified for IPv4 and used as either the payload or delivery protocol.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> +	\phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
> +    Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This protocol describes extensions by which two fields, Key and
> +    Sequence Number, can be optionally carried in the GRE Header \ref{intro:gre_rfc2784}.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
> +	\phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
> +    IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is specified for IPv6 and used as either the payload or
> +    delivery protocol. Note that this does not change the GRE header format or any behaviors specified by RFC 2784 or RFC 2890.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
> +	\phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
> +    GRE-in-UDP Encapsulation. This specifies a method of encapsulating network protocol packets within GRE and UDP headers.
> +    This GRE-in-UDP encapsulation allows the UDP source port field to be used as an entropy field. This protocol is specified for IPv4 and IPv6,
> +    and used as either the payload or delivery protocol.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
> +	\phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
> +    Virtual eXtensible Local Area Network.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> +	\phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
> +    Generic Protocol Extension for VXLAN. This protocol describes extending Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.
> +	\newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
> +	\phantomsection\label{intro:geneve}\textbf{[GENEVE]} &
> +    Generic Network Virtualization Encapsulation.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> +	\phantomsection\label{intro:ipip}\textbf{[IPIP]} &
> +    IP Encapsulation within IP.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc2003}\\
> +	\phantomsection\label{intro:nvgre}\textbf{[NVGRE]} &
> +    NVGRE: Network Virtualization Using Generic Routing Encapsulation
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc7637.html}\\
> +	\phantomsection\label{intro:stt}\textbf{[STT]} &
> +    Stateless Transport Tunneling. STT is particularly useful when some tunnel endpoints are in end-systems, as it utilizes the capabilities
> +    of the network interface card to improve performance.
> +	\newline\url{https://www.ietf.org/archive/id/draft-davie-stt-08.txt}\\
> +	\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}
>   
>   \section{Non-Normative References}


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [virtio-comment] [PATCH v12] virtio-net: support inner header hash
  2023-04-03  4:58 [virtio-dev] [PATCH v12] virtio-net: support inner header hash Heng Qi
  2023-04-03  5:08 ` Heng Qi
@ 2023-04-11 12:08 ` Heng Qi
  2023-04-11 21:03 ` [virtio-dev] " Parav Pandit
  2023-04-13  3:40 ` Jason Wang
  3 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2023-04-11 12:08 UTC (permalink / raw)
  To: virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Parav Pandit, Yuri Benditovich,
	Xuan Zhuo

Hi, all.

Do you have any comments on this release?

Thanks.


在 2023/4/3 下午12:58, Heng Qi 写道:
> 1. Currently, a received encapsulated packet has an outer and an inner header, but
> the virtio device is unable to calculate the hash for the inner header. The same
> flow can traverse through different tunnels, resulting in the encapsulated
> packets being spread across multiple receive queues (refer to the figure below).
> However, in certain scenarios, we may need to direct these encapsulated packets of
> the same flow to a single receive queue. This facilitates the processing
> of the flow by the same CPU to improve performance (warm caches, less locking, etc.).
>
>                 client1                    client2
>                    |        +-------+         |
>                    +------->|tunnels|<--------+
>                             +-------+
>                                |  |
>                                v  v
>                        +-----------------+
>                        | monitoring host |
>                        +-----------------+
>
> To achieve this, the device can calculate a suitable hash based on the inner headers
> of this flow, for example using the Toeplitz combined with a symmetric hash key.
>
> 2. For legacy systems, they may lack entropy fields which modern protocols have in
> the outer header, resulting in multiple flows with the same outer header but
> different inner headers being directed to the same receive queue. This results in
> poor receive performance.
>
> To address this limitation, inner header hash can be used to enable the device to advertise
> the capability to calculate the hash for the inner packet, regaining better receive performance.
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> v11->v12:
> 	1. Add a command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> 	2. Refine the commit log. @Michael S . Tsirkin
> 	3. Add some tunnel types.
>
> v10->v11:
> 	1. Revise commit log for clarity for readers.
> 	2. Some modifications to avoid undefined terms. @Parav Pandit
> 	3. Change VIRTIO_NET_F_HASH_TUNNEL dependency. @Parav Pandit
> 	4. Add the normative statements. @Parav Pandit
>
> v9->v10:
> 	1. Removed hash_report_tunnel related information. @Parav Pandit
> 	2. Re-describe the limitations of QoS for tunneling.
> 	3. Some clarification.
>
> v8->v9:
> 	1. Merge hash_report_tunnel_types into hash_report. @Parav Pandit
> 	2. Add tunnel security section. @Michael S . Tsirkin
> 	3. Add VIRTIO_NET_F_HASH_REPORT_TUNNEL.
> 	4. Fix some typos.
> 	5. Add more tunnel types. @Michael S . Tsirkin
>
> 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
>
>   device-types/net/description.tex        | 143 ++++++++++++++++++++++++
>   device-types/net/device-conformance.tex |   1 +
>   device-types/net/driver-conformance.tex |   1 +
>   introduction.tex                        |  44 ++++++++
>   4 files changed, 189 insertions(+)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 0500bb6..121585e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -83,6 +83,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.
> @@ -139,6 +142,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 along with VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
>   \end{description}
>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -198,6 +202,7 @@ \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.
> @@ -212,6 +217,12 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>   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.
>   
> +Field \field{supported_tunnel_hash_types} only exists if the device
> +supports inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> +
> +Filed \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 encapsulation 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,
> @@ -848,6 +859,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -855,6 +867,7 @@ \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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -870,6 +883,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   
>   \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)
> @@ -980,6 +995,133 @@ \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}
>   
> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and
> +the driver can configure the inner header hash calculation for encapsulated packets \ref{sec:Device Types / Network Device / Device OperatiHn / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the class VIRTIO_NET_CTRL_MQ.
> +The command sets \field{hash_tunnel_types} in the structure virtio_net_hash_tunnel_config.
> +
> +struct virtio_net_hash_tunnel_config {
> +    le32 hash_tunnel_types;
> +};
> +
> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
> +
> +\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.
> +
> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received encapsulated packet's
> +outer header matches one of the supported \field{hash_tunnel_types}, the hash
> +of the inner header is calculated.
> +
> +Supported encapsulated packet types:
> +\begin{itemize}
> +\item \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]}
> +\item \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]}
> +\item \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]}
> +\item \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]}
> +\item \hyperref[intro:vxlan]{[VXLAN]}
> +\item \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]}
> +\item \hyperref[intro:geneve]{[GENEVE]}
> +\item \hyperref[intro:ipip]{[IPIP]}
> +\item \hyperref[intro:nvgre]{[NVGRE]}
> +\item \hyperref[intro:sit]{[STT]}
> +\end{itemize}
> +
> +If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type is not included in
> +\field{hash_tunnel_types}, the hash of the outer header is calculated for the received encapsulated packet.
> +
> +The hash is calculated for the received non-encapsulated packet as if VIRTIO_NET_F_HASH_TUNNEL was not negotiated.
> +
> +\subparagraph{Supported/enabled encapsulation hash types}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled encapsulation hash types}
> +
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NONE        (1 << 0)
> +\end{lstlisting}
> +
> +Supported encapsulation hash types:
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784    (1 << 1)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890    (1 << 2)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676    (1 << 3)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP     (1 << 4)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan]{[VXLAN]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 5)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 6)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:geneve]{[GENEVE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 7)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:ipip]{[IPIP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP        (1 << 8)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:nvgre]{[NVGRE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE       (1 << 9)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:stt]{[STT]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_STT         (1 << 10)
> +\end{lstlisting}
> +
> +\subparagraph{Tunnel QoS limitation}
> +When a specific receive queue is shared by multiple tunnels to receive encapsulated packets,
> +there is no quality of service (QoS) for these packets. For example, when the packets of certain
> +tunnels are spread across multiple receive queues, these receive queues may have an unbalanced
> +amount of packets. This can cause a specific receive queue to become full, resulting in packet loss.
> +
> +Possible mitigations:
> +\begin{itemize}
> +\item Use a tool with good forwarding performance to keep the receive queue from filling up.
> +\item If the QoS is unavailable, the driver can set \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> +      to disable inner header hash for encapsulated packets.
> +\item Choose a hash key that can avoid queue collisions.
> +\item Perform appropriate QoS before packets consume the receive buffers of the receive queues.
> +\end{itemize}
> +
> +The limitations mentioned above exist with/without the inner header hash.
> +
> +\devicenormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The device MUST calculate the outer header hash if the received encapsulated packet has an encapsulation type not in \field{supported_tunnel_hash_types}.
> +
> +The device MUST respond to the VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG command with VIRTIO_NET_ERR if the device
> +received an unrecognized or unsupported VIRTIO_NET_HASH_TUNNEL_TYPE_ flag.
> +
> +Upon reset, the device MUST initialize \field{hash_tunnel_type} to 0.
> +
> +\drivernormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The driver MUST have negotiated the feature VIRTIO_NET_F_HASH_TUNNEL when issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> +
> +The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that are not supported by the device.
> +
>   \paragraph{Hash reporting for incoming packets}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>   
> @@ -1308,6 +1450,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>    #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic receive steering)
>    #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable receive steering)
>    #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable hash calculation)
> + #define VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG       3 (for configurable inner header hash)
>   \end{lstlisting}
>   
>   If more than one multiqueue mode is negotiated, the resulting device configuration is defined by the last command sent by the driver.
> diff --git a/device-types/net/device-conformance.tex b/device-types/net/device-conformance.tex
> index 54f6783..f88f48b 100644
> --- a/device-types/net/device-conformance.tex
> +++ b/device-types/net/device-conformance.tex
> @@ -14,4 +14,5 @@
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) / RSS processing}
>   \item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> +\item \ref{devicenormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
>   \end{itemize}
> diff --git a/device-types/net/driver-conformance.tex b/device-types/net/driver-conformance.tex
> index 97d0cc1..9d853d9 100644
> --- a/device-types/net/driver-conformance.tex
> +++ b/device-types/net/driver-conformance.tex
> @@ -14,4 +14,5 @@
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Offloads State Configuration / Setting Offloads State}
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Receive-side scaling (RSS) }
>   \item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
> +\item \ref{drivernormative:Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
>   \end{itemize}
> diff --git a/introduction.tex b/introduction.tex
> index 287c5fc..36b620f 100644
> --- a/introduction.tex
> +++ b/introduction.tex
> @@ -99,6 +99,50 @@ \section{Normative References}\label{sec:Normative References}
>       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_rfc2784}\textbf{[GRE_rfc2784]} &
> +    Generic Routing Encapsulation. This protocol is only specified for IPv4 and used as either the payload or delivery protocol.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc2784/}\\
> +	\phantomsection\label{intro:gre_rfc2890}\textbf{[GRE_rfc2890]} &
> +    Key and Sequence Number Extensions to GRE \ref{intro:gre_rfc2784}. This protocol describes extensions by which two fields, Key and
> +    Sequence Number, can be optionally carried in the GRE Header \ref{intro:gre_rfc2784}.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc2890}\\
> +	\phantomsection\label{intro:gre_rfc7676}\textbf{[GRE_rfc7676]} &
> +    IPv6 Support for Generic Routing Encapsulation (GRE). This protocol is specified for IPv6 and used as either the payload or
> +    delivery protocol. Note that this does not change the GRE header format or any behaviors specified by RFC 2784 or RFC 2890.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7676/}\\
> +	\phantomsection\label{intro:gre_in_udp_rfc8086}\textbf{[GRE-in-UDP]} &
> +    GRE-in-UDP Encapsulation. This specifies a method of encapsulating network protocol packets within GRE and UDP headers.
> +    This GRE-in-UDP encapsulation allows the UDP source port field to be used as an entropy field. This protocol is specified for IPv4 and IPv6,
> +    and used as either the payload or delivery protocol.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc8086}\\
> +	\phantomsection\label{intro:vxlan}\textbf{[VXLAN]} &
> +    Virtual eXtensible Local Area Network.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc7348/}\\
> +	\phantomsection\label{intro:vxlan-gpe}\textbf{[VXLAN-GPE]} &
> +    Generic Protocol Extension for VXLAN. This protocol describes extending Virtual eXtensible Local Area Network (VXLAN) via changes to the VXLAN header.
> +	\newline\url{https://www.ietf.org/archive/id/draft-ietf-nvo3-vxlan-gpe-12.txt}\\
> +	\phantomsection\label{intro:geneve}\textbf{[GENEVE]} &
> +    Generic Network Virtualization Encapsulation.
> +	\newline\url{https://datatracker.ietf.org/doc/rfc8926/}\\
> +	\phantomsection\label{intro:ipip}\textbf{[IPIP]} &
> +    IP Encapsulation within IP.
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc2003}\\
> +	\phantomsection\label{intro:nvgre}\textbf{[NVGRE]} &
> +    NVGRE: Network Virtualization Using Generic Routing Encapsulation
> +	\newline\url{https://www.rfc-editor.org/rfc/rfc7637.html}\\
> +	\phantomsection\label{intro:stt}\textbf{[STT]} &
> +    Stateless Transport Tunneling. STT is particularly useful when some tunnel endpoints are in end-systems, as it utilizes the capabilities
> +    of the network interface card to improve performance.
> +	\newline\url{https://www.ietf.org/archive/id/draft-davie-stt-08.txt}\\
> +	\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}
>   
>   \section{Non-Normative References}


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-03  4:58 [virtio-dev] [PATCH v12] virtio-net: support inner header hash Heng Qi
  2023-04-03  5:08 ` Heng Qi
  2023-04-11 12:08 ` [virtio-dev] Re: [virtio-comment] " Heng Qi
@ 2023-04-11 21:03 ` Parav Pandit
  2023-04-12  3:05   ` Heng Qi
  2023-04-13  3:40 ` Jason Wang
  3 siblings, 1 reply; 17+ messages in thread
From: Parav Pandit @ 2023-04-11 21:03 UTC (permalink / raw)
  To: Heng Qi, virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Yuri Benditovich, Xuan Zhuo



On 4/3/2023 12:58 AM, Heng Qi wrote:

> To achieve this, the device can calculate a suitable hash based on the inner headers
> of this flow, for example using the Toeplitz combined with a symmetric hash key.
> 
I am not sure you need symmetric hash key. Toeplitz with symmetric 
hashing without the symmetric key is possible too.

So just mentioning it as a 'combined with symmetric hashing' is enough.

>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -198,6 +202,7 @@ \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;
>   };
Given that a set command is added via cvq, it make sense to also do 
symetrric work to get it via a cvq.
This is also similar to the latest work for notification coalescing for 
VQ where get and set done using single channel = cvq.

Granted that RSS and other fields are done differently, but it was bit 
in the past.

With that no need to define two fields at two different places in config 
area and also in cvq.

Just the new opcode is needed for GET and things will be fine.

> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and
> +the driver can configure the inner header hash calculation for encapsulated packets \ref{sec:Device Types / Network Device / Device OperatiHn / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the class VIRTIO_NET_CTRL_MQ.
> +The command sets \field{hash_tunnel_types} in the structure virtio_net_hash_tunnel_config.
> +
> +struct virtio_net_hash_tunnel_config {
> +    le32 hash_tunnel_types;
> +};
> +
VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG_SET
and
VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG_GET

> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
> +
> +\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.
> +
> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received encapsulated packet's
> +outer header matches one of the supported \field{hash_tunnel_types}, the hash
> +of the inner header is calculated.
s/one of the supported/one of the configured/
Because support comes from GET or config space area; out of which a 
smaller or equal subset of tunnel types are configured.

> +\devicenormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The device MUST calculate the outer header hash if the received encapsulated packet has an encapsulation type not in \field{supported_tunnel_hash_types}.
> +
Since the configured set can be smaller, a better reword is:
The device MUST calculate the hash from the outer header if the received 
encapsulated packet type is not matching from hash_tunnel_types.

> +The device MUST respond to the VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG command with VIRTIO_NET_ERR if the device
> +received an unrecognized or unsupported VIRTIO_NET_HASH_TUNNEL_TYPE_ flag.
> +
> +Upon reset, the device MUST initialize \field{hash_tunnel_type} to 0.
> +
> +\drivernormative{\subparagraph}{Inner Header Hash}{Device Types / Network Device / Device Operation / Control Virtqueue / Inner Header Hash}
> +
> +The driver MUST have negotiated the feature VIRTIO_NET_F_HASH_TUNNEL when issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> +
> +The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that are not supported by the device.
> +
>   \paragraph{Hash reporting for incoming packets}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash reporting for incoming packets}
>   
> @@ -1308,6 +1450,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>    #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic receive steering)
>    #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable receive steering)
>    #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable hash calculation)
> + #define VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG       3 (for configurable inner header hash)
MQ_TUNNEL_CONFIG is not mutually exclusive with HASH or RSS config.
Hence adding it here is incorrect.

See the text below.
"If more than one multiqueue mode is negotiated, the resulting device 
configuration is defined by the last command
sent by the driver."

So MQ_TUNNEL config without RSS or HASH will be an undefined behavior, 
as it doesn't what kind of hashing to be done.

So please tunnel GET and SET commands outside of VIRTIO_NET_CTRL_MQ.
This way it can work side by side with rss, hash and auto modes.

I am sorry for the late response.
I had a day off last week and other active discussions as you notice.

---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-11 21:03 ` [virtio-dev] " Parav Pandit
@ 2023-04-12  3:05   ` Heng Qi
  2023-04-12  3:30     ` Parav Pandit
  0 siblings, 1 reply; 17+ messages in thread
From: Heng Qi @ 2023-04-12  3:05 UTC (permalink / raw)
  To: Parav Pandit, virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Yuri Benditovich, Xuan Zhuo



在 2023/4/12 上午5:03, Parav Pandit 写道:
>
>
> On 4/3/2023 12:58 AM, Heng Qi wrote:
>
>> To achieve this, the device can calculate a suitable hash based on 
>> the inner headers
>> of this flow, for example using the Toeplitz combined with a 
>> symmetric hash key.
>>
> I am not sure you need symmetric hash key. Toeplitz with symmetric 
> hashing without the symmetric key is possible too.
>
> So just mentioning it as a 'combined with symmetric hashing' is enough.

Yes, as discussed with Michael, we will also support XOR hashing or 
Toeplitz symmetric hashing or even both, I'm thinking of starting a 
draft in a separate thread to let us have initial discussions.

The statement here I will modify.

>
>>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device 
>> Types / Network Device / Feature bits / Legacy Interface: Feature bits}
>> @@ -198,6 +202,7 @@ \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;
>>   };
> Given that a set command is added via cvq, it make sense to also do 
> symetrric work to get it via a cvq.
> This is also similar to the latest work for notification coalescing 
> for VQ where get and set done using single channel = cvq.

Only set is given to match the existing RSS/HASH configuration methods. 
But we should really look ahead as you suggest.

>
> Granted that RSS and other fields are done differently, but it was bit 
> in the past.

Yes.

>
> With that no need to define two fields at two different places in 
> config area and also in cvq.
>
> Just the new opcode is needed for GET and things will be fine.

Right.

>
>> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports 
>> inner header hash and
>> +the driver can configure the inner header hash calculation for 
>> encapsulated packets \ref{sec:Device Types / Network Device / Device 
>> OperatiHn / Processing of Incoming Packets / Hash calculation for 
>> incoming packets / Tunnel/Encapsulated packet}
>> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the 
>> class VIRTIO_NET_CTRL_MQ.
>> +The command sets \field{hash_tunnel_types} in the structure 
>> virtio_net_hash_tunnel_config.
>> +
>> +struct virtio_net_hash_tunnel_config {
>> +    le32 hash_tunnel_types;
>> +};
>> +
> VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG_SET
> and
> VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG_GET

Will do.

>
>> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
>> +
>> +\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.
>> +
>> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received 
>> encapsulated packet's
>> +outer header matches one of the supported \field{hash_tunnel_types}, 
>> the hash
>> +of the inner header is calculated.
> s/one of the supported/one of the configured/
> Because support comes from GET or config space area; out of which a 
> smaller or equal subset of tunnel types are configured.

Yes, configured is obviously more accurate than supported.

>
>> +\devicenormative{\subparagraph}{Inner Header Hash}{Device Types / 
>> Network Device / Device Operation / Control Virtqueue / Inner Header 
>> Hash}
>> +
>> +The device MUST calculate the outer header hash if the received 
>> encapsulated packet has an encapsulation type not in 
>> \field{supported_tunnel_hash_types}.
>> +
> Since the configured set can be smaller, a better reword is:
> The device MUST calculate the hash from the outer header if the 
> received encapsulated packet type is not matching from hash_tunnel_types.

Will modify.

>
>> +The device MUST respond to the VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG 
>> command with VIRTIO_NET_ERR if the device
>> +received an unrecognized or unsupported VIRTIO_NET_HASH_TUNNEL_TYPE_ 
>> flag.
>> +
>> +Upon reset, the device MUST initialize \field{hash_tunnel_type} to 0.
>> +
>> +\drivernormative{\subparagraph}{Inner Header Hash}{Device Types / 
>> Network Device / Device Operation / Control Virtqueue / Inner Header 
>> Hash}
>> +
>> +The driver MUST have negotiated the feature VIRTIO_NET_F_HASH_TUNNEL 
>> when issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
>> +
>> +The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that 
>> are not supported by the device.
>> +
>>   \paragraph{Hash reporting for incoming packets}
>>   \label{sec:Device Types / Network Device / Device Operation / 
>> Processing of Incoming Packets / Hash reporting for incoming packets}
>>   @@ -1308,6 +1450,7 @@ \subsubsection{Control 
>> Virtqueue}\label{sec:Device Types / Network Device / Devi
>>    #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET        0 (for automatic 
>> receive steering)
>>    #define VIRTIO_NET_CTRL_MQ_RSS_CONFIG          1 (for configurable 
>> receive steering)
>>    #define VIRTIO_NET_CTRL_MQ_HASH_CONFIG         2 (for configurable 
>> hash calculation)
>> + #define VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG       3 (for configurable 
>> inner header hash)
> MQ_TUNNEL_CONFIG is not mutually exclusive with HASH or RSS config.
> Hence adding it here is incorrect.
>
> See the text below.
> "If more than one multiqueue mode is negotiated, the resulting device 
> configuration is defined by the last command
> sent by the driver."
>

NOTE: We only have two modes now. The mode is not equal to the command. 
We can add VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG for commands, but there are 
still only two modes:
"
This specification defines the following modes that a device MAY 
implement for operation with multiple transmit/receive virtqueues:
\begin{itemize}
\item Automatic receive steering as defined in \ref{sec:Device Types / 
Network Device / Device Operation / Control Virtqueue / Automatic 
receive steering in multiqueue mode}.
   If a device supports this mode, it offers the VIRTIO_NET_F_MQ feature 
bit.
\item Receive-side scaling as defined in \ref{devicenormative:Device 
Types / Network Device / Device Operation / Control Virtqueue / 
Receive-side scaling (RSS) / RSS processing}.
   If a device supports this mode, it offers the VIRTIO_NET_F_RSS 
feature bit.
\end{itemize}
"


We also say:
“
\item[VIRTIO_NET_F_HASH_TUNNEL] Requires VIRTIO_NET_F_CTRL_VQ along with 
VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
”
Now,
when VIRTIO_NET_F_RSS is negotiated, we have Receive-side scaling mode.
When VIRTIO_NET_F_HASH_REPORT is negotiated, we also have the mode 
according to:
"
Note that if the device offers VIRTIO_NET_F_HASH_REPORT, 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
   \ref{sec:Device Types / Network Device / Device Operation / Control 
Virtqueue / Receive-side scaling (RSS) / Setting RSS parameters}.
\item Otherwise the device MUST support VIRTIO_NET_CTRL_MQ_HASH_CONFIG 
command per
   \ref{sec:Device Types / Network Device / Device Operation / Control 
Virtqueue / Automatic receive steering in multiqueue mode / Hash 
calculation}.
\end{itemize}”

So, it is not incorrect that we put VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG here.


> So MQ_TUNNEL config without RSS or HASH will be an undefined behavior, 
> as it doesn't what kind of hashing to be done.
>
> So please tunnel GET and SET commands outside of VIRTIO_NET_CTRL_MQ.
> This way it can work side by side with rss, hash and auto modes.

Yes, if we will also provide GET commands, it will be clearer to put 
VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG outside of VIRTIO_NET_CTRL_MQ.

Thanks!




---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-12  3:05   ` Heng Qi
@ 2023-04-12  3:30     ` Parav Pandit
  0 siblings, 0 replies; 17+ messages in thread
From: Parav Pandit @ 2023-04-12  3:30 UTC (permalink / raw)
  To: Heng Qi, virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Jason Wang, Yuri Benditovich, Xuan Zhuo


On 4/11/2023 11:05 PM, Heng Qi wrote:

> NOTE: We only have two modes now. The mode is not equal to the command. 
> We can add VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG for commands, but there are 
> still only two modes:
> "
> This specification defines the following modes that a device MAY 
> implement for operation with multiple transmit/receive virtqueues:
> \begin{itemize}
> \item Automatic receive steering as defined in \ref{sec:Device Types / 
> Network Device / Device Operation / Control Virtqueue / Automatic 
> receive steering in multiqueue mode}.
>    If a device supports this mode, it offers the VIRTIO_NET_F_MQ feature 
> bit.
> \item Receive-side scaling as defined in \ref{devicenormative:Device 
> Types / Network Device / Device Operation / Control Virtqueue / 
> Receive-side scaling (RSS) / RSS processing}.
>    If a device supports this mode, it offers the VIRTIO_NET_F_RSS 
> feature bit.
> \end{itemize}

> So, it is not incorrect that we put VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG here.

Got it. I got confused between mode and command with the text

"The command selects the mode of multiqueue operation, as follows:"

But its clear to me now.
Thanks.

---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-03  4:58 [virtio-dev] [PATCH v12] virtio-net: support inner header hash Heng Qi
                   ` (2 preceding siblings ...)
  2023-04-11 21:03 ` [virtio-dev] " Parav Pandit
@ 2023-04-13  3:40 ` Jason Wang
  2023-04-13 11:03   ` [virtio-dev] Re: [virtio-comment] " Heng Qi
  3 siblings, 1 reply; 17+ messages in thread
From: Jason Wang @ 2023-04-13  3:40 UTC (permalink / raw)
  To: Heng Qi, virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Parav Pandit, Yuri Benditovich, Xuan Zhuo


在 2023/4/3 12:58, Heng Qi 写道:
> 1. Currently, a received encapsulated packet has an outer and an inner header, but
> the virtio device is unable to calculate the hash for the inner header. The same
> flow can traverse through different tunnels, resulting in the encapsulated
> packets being spread across multiple receive queues (refer to the figure below).
> However, in certain scenarios, we may need to direct these encapsulated packets of
> the same flow to a single receive queue. This facilitates the processing
> of the flow by the same CPU to improve performance (warm caches, less locking, etc.).
>
>                 client1                    client2
>                    |        +-------+         |
>                    +------->|tunnels|<--------+
>                             +-------+
>                                |  |
>                                v  v
>                        +-----------------+
>                        | monitoring host |
>                        +-----------------+
>
> To achieve this, the device can calculate a suitable hash based on the inner headers
> of this flow, for example using the Toeplitz combined with a symmetric hash key.


Some device allows sorting n tuples before calculating hash, do we need 
to consider them?


> 2. For legacy systems, they may lack entropy fields which modern protocols have in
> the outer header, resulting in multiple flows with the same outer header but
> different inner headers being directed to the same receive queue. This results in
> poor receive performance.
>
> To address this limitation, inner header hash can be used to enable the device to advertise
> the capability to calculate the hash for the inner packet, regaining better receive performance.
>
> Signed-off-by: Heng Qi<hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo<xuanzhuo@linux.alibaba.com>
> ---
> v11->v12:
> 	1. Add a command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
> 	2. Refine the commit log. @Michael S . Tsirkin
> 	3. Add some tunnel types.
>
> v10->v11:
> 	1. Revise commit log for clarity for readers.
> 	2. Some modifications to avoid undefined terms. @Parav Pandit
> 	3. Change VIRTIO_NET_F_HASH_TUNNEL dependency. @Parav Pandit
> 	4. Add the normative statements. @Parav Pandit
>
> v9->v10:
> 	1. Removed hash_report_tunnel related information. @Parav Pandit
> 	2. Re-describe the limitations of QoS for tunneling.
> 	3. Some clarification.
>
> v8->v9:
> 	1. Merge hash_report_tunnel_types into hash_report. @Parav Pandit
> 	2. Add tunnel security section. @Michael S . Tsirkin
> 	3. Add VIRTIO_NET_F_HASH_REPORT_TUNNEL.
> 	4. Fix some typos.
> 	5. Add more tunnel types. @Michael S . Tsirkin
>
> 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
>
>   device-types/net/description.tex        | 143 ++++++++++++++++++++++++
>   device-types/net/device-conformance.tex |   1 +
>   device-types/net/driver-conformance.tex |   1 +
>   introduction.tex                        |  44 ++++++++
>   4 files changed, 189 insertions(+)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 0500bb6..121585e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -83,6 +83,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.
> @@ -139,6 +142,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 along with VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
>   \end{description}
>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -198,6 +202,7 @@ \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.
> @@ -212,6 +217,12 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>   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.
>   
> +Field \field{supported_tunnel_hash_types} only exists if the device
> +supports inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
> +
> +Filed \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 encapsulation 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,
> @@ -848,6 +859,7 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -855,6 +867,7 @@ \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.
> +\item The device uses \field{hash_tunnel_types} of the virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>   \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}
> @@ -870,6 +883,8 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   
>   \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)
> @@ -980,6 +995,133 @@ \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}
>   
> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and
> +the driver can configure the inner header hash calculation for encapsulated packets \ref{sec:Device Types / Network Device / Device OperatiHn / Processing of Incoming Packets / Hash calculation for incoming packets / Tunnel/Encapsulated packet}
> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the class VIRTIO_NET_CTRL_MQ.
> +The command sets \field{hash_tunnel_types} in the structure virtio_net_hash_tunnel_config.
> +
> +struct virtio_net_hash_tunnel_config {
> +    le32 hash_tunnel_types;
> +};
> +
> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
> +
> +\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.
> +
> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received encapsulated packet's
> +outer header matches one of the supported \field{hash_tunnel_types}, the hash
> +of the inner header is calculated.


GRE allows X over Y, we'd better document what is supported in X and Y 
(I guess they are implied in the supported_hash_types?)


> +
> +Supported encapsulated packet types:
> +\begin{itemize}
> +\item \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]}
> +\item \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]}
> +\item \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]}
> +\item \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]}
> +\item \hyperref[intro:vxlan]{[VXLAN]}
> +\item \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]}
> +\item \hyperref[intro:geneve]{[GENEVE]}
> +\item \hyperref[intro:ipip]{[IPIP]}
> +\item \hyperref[intro:nvgre]{[NVGRE]}
> +\item \hyperref[intro:sit]{[STT]}
> +\end{itemize}
> +
> +If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type is not included in
> +\field{hash_tunnel_types}, the hash of the outer header is calculated for the received encapsulated packet.
> +
> +The hash is calculated for the received non-encapsulated packet as if VIRTIO_NET_F_HASH_TUNNEL was not negotiated.
> +
> +\subparagraph{Supported/enabled encapsulation hash types}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled encapsulation hash types}
> +
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NONE        (1 << 0)
> +\end{lstlisting}
> +
> +Supported encapsulation hash types:
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784    (1 << 1)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890    (1 << 2)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676    (1 << 3)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP     (1 << 4)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan]{[VXLAN]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 5)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 6)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:geneve]{[GENEVE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 7)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:ipip]{[IPIP]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP        (1 << 8)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:nvgre]{[NVGRE]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE       (1 << 9)
> +\end{lstlisting}
> +Hash type applicable for inner payload of the \hyperref[intro:stt]{[STT]} packet:
> +\begin{lstlisting}
> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_STT         (1 << 10)
> +\end{lstlisting}
> +
> +\subparagraph{Tunnel QoS limitation}
> +When a specific receive queue is shared by multiple tunnels to receive encapsulated packets,
> +there is no quality of service (QoS) for these packets.


This is up to the implementation, so I'm not sure it's worthwhile for 
mentioning things like this and the "limitation".


>   For example, when the packets of certain
> +tunnels are spread across multiple receive queues, these receive queues may have an unbalanced
> +amount of packets. This can cause a specific receive queue to become full, resulting in packet loss.


We have many places that can lead to packet dropping. For example, the 
automatic steering is best effort. I tend to avoid mentioning things 
like this.


> +
> +Possible mitigations:
> +\begin{itemize}
> +\item Use a tool with good forwarding performance to keep the receive queue from filling up.
> +\item If the QoS is unavailable, the driver can set \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> +      to disable inner header hash for encapsulated packets.
> +\item Choose a hash key that can avoid queue collisions.
> +\item Perform appropriate QoS before packets consume the receive buffers of the receive queues.
> +\end{itemize}
> +
> +The limitations mentioned above exist with/without the inner header hash.


This conflicts with the tile "Tunnel QoS limitation" which readers may 
think it happens only for tunnel.

Thanks


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-13  3:40 ` Jason Wang
@ 2023-04-13 11:03   ` Heng Qi
  2023-04-13 21:43     ` Michael S. Tsirkin
  2023-04-13 21:46     ` Michael S. Tsirkin
  0 siblings, 2 replies; 17+ messages in thread
From: Heng Qi @ 2023-04-13 11:03 UTC (permalink / raw)
  To: Jason Wang, virtio-dev, virtio-comment
  Cc: Michael S . Tsirkin, Parav Pandit, Yuri Benditovich, Xuan Zhuo



在 2023/4/13 上午11:40, Jason Wang 写道:
>
> 在 2023/4/3 12:58, Heng Qi 写道:
>> 1. Currently, a received encapsulated packet has an outer and an 
>> inner header, but
>> the virtio device is unable to calculate the hash for the inner 
>> header. The same
>> flow can traverse through different tunnels, resulting in the 
>> encapsulated
>> packets being spread across multiple receive queues (refer to the 
>> figure below).
>> However, in certain scenarios, we may need to direct these 
>> encapsulated packets of
>> the same flow to a single receive queue. This facilitates the processing
>> of the flow by the same CPU to improve performance (warm caches, less 
>> locking, etc.).
>>
>>                 client1                    client2
>>                    |        +-------+         |
>>                    +------->|tunnels|<--------+
>>                             +-------+
>>                                |  |
>>                                v  v
>>                        +-----------------+
>>                        | monitoring host |
>>                        +-----------------+
>>
>> To achieve this, the device can calculate a suitable hash based on 
>> the inner headers
>> of this flow, for example using the Toeplitz combined with a 
>> symmetric hash key.
>
>
> Some device allows sorting n tuples before calculating hash, do we 
> need to consider them?

If a device supports inner header hash and it has the ability to sort 
tuples, the device can
choose to continue to sort tuples or continue to do a symmetric hash 
(use a special hash key, symmetric toeplitz hash or xor hash) .
It depends on them, but I think we can document these for readers.

>
>
>> 2. For legacy systems, they may lack entropy fields which modern 
>> protocols have in
>> the outer header, resulting in multiple flows with the same outer 
>> header but
>> different inner headers being directed to the same receive queue. 
>> This results in
>> poor receive performance.
>>
>> To address this limitation, inner header hash can be used to enable 
>> the device to advertise
>> the capability to calculate the hash for the inner packet, regaining 
>> better receive performance.
>>
>> Signed-off-by: Heng Qi<hengqi@linux.alibaba.com>
>> Reviewed-by: Xuan Zhuo<xuanzhuo@linux.alibaba.com>
>> ---
>> v11->v12:
>>     1. Add a command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG.
>>     2. Refine the commit log. @Michael S . Tsirkin
>>     3. Add some tunnel types.
>>
>> v10->v11:
>>     1. Revise commit log for clarity for readers.
>>     2. Some modifications to avoid undefined terms. @Parav Pandit
>>     3. Change VIRTIO_NET_F_HASH_TUNNEL dependency. @Parav Pandit
>>     4. Add the normative statements. @Parav Pandit
>>
>> v9->v10:
>>     1. Removed hash_report_tunnel related information. @Parav Pandit
>>     2. Re-describe the limitations of QoS for tunneling.
>>     3. Some clarification.
>>
>> v8->v9:
>>     1. Merge hash_report_tunnel_types into hash_report. @Parav Pandit
>>     2. Add tunnel security section. @Michael S . Tsirkin
>>     3. Add VIRTIO_NET_F_HASH_REPORT_TUNNEL.
>>     4. Fix some typos.
>>     5. Add more tunnel types. @Michael S . Tsirkin
>>
>> 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
>>
>>   device-types/net/description.tex        | 143 ++++++++++++++++++++++++
>>   device-types/net/device-conformance.tex |   1 +
>>   device-types/net/driver-conformance.tex |   1 +
>>   introduction.tex                        |  44 ++++++++
>>   4 files changed, 189 insertions(+)
>>
>> diff --git a/device-types/net/description.tex 
>> b/device-types/net/description.tex
>> index 0500bb6..121585e 100644
>> --- a/device-types/net/description.tex
>> +++ b/device-types/net/description.tex
>> @@ -83,6 +83,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.
>> @@ -139,6 +142,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 along 
>> with VIRTIO_NET_F_RSS and/or VIRTIO_NET_F_HASH_REPORT.
>>   \end{description}
>>     \subsubsection{Legacy Interface: Feature bits}\label{sec:Device 
>> Types / Network Device / Feature bits / Legacy Interface: Feature bits}
>> @@ -198,6 +202,7 @@ \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.
>> @@ -212,6 +217,12 @@ \subsection{Device configuration 
>> layout}\label{sec:Device Types / Network Device
>>   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.
>>   +Field \field{supported_tunnel_hash_types} only exists if the device
>> +supports inner header hash, i.e. if VIRTIO_NET_F_HASH_TUNNEL is set.
>> +
>> +Filed \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 encapsulation 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,
>> @@ -848,6 +859,7 @@ \subsubsection{Processing of Incoming 
>> Packets}\label{sec:Device Types / Network
>>   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.
>> +\item The device uses \field{hash_tunnel_types} of the 
>> virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel 
>> types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>>   \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}
>> @@ -855,6 +867,7 @@ \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.
>> +\item The device uses \field{hash_tunnel_types} of the 
>> virtio_net_hash_tunnel_config structure as 'Enabled hash tunnel 
>> types' bitmask if VIRTIO_NET_F_HASH_TUNNEL was negotiated.
>>   \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}
>> @@ -870,6 +883,8 @@ \subsubsection{Processing of Incoming 
>> Packets}\label{sec:Device Types / Network
>>     \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)
>> @@ -980,6 +995,133 @@ \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}
>>   +\paragraph{Inner Header Hash}
>> +\label{sec:Device Types / Network Device / Device Operation / 
>> Processing of Incoming Packets / Inner Header Hash}
>> +
>> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports 
>> inner header hash and
>> +the driver can configure the inner header hash calculation for 
>> encapsulated packets \ref{sec:Device Types / Network Device / Device 
>> OperatiHn / Processing of Incoming Packets / Hash calculation for 
>> incoming packets / Tunnel/Encapsulated packet}
>> +by issuing the command VIRTIO_NET_CTRL_MQ_TUNNEL_CONFIG from the 
>> class VIRTIO_NET_CTRL_MQ.
>> +The command sets \field{hash_tunnel_types} in the structure 
>> virtio_net_hash_tunnel_config.
>> +
>> +struct virtio_net_hash_tunnel_config {
>> +    le32 hash_tunnel_types;
>> +};
>> +
>> +Filed \field{hash_tunnel_types} contains a bitmask of supported 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}.
>> +
>> +\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.
>> +
>> +If VIRTIO_NET_F_HASH_TUNNEL is negotiated and a received 
>> encapsulated packet's
>> +outer header matches one of the supported \field{hash_tunnel_types}, 
>> the hash
>> +of the inner header is calculated.
>
>
> GRE allows X over Y, we'd better document what is supported in X and Y 

Yes, I will do.

> (I guess they are implied in the supported_hash_types?)

Yes, but as you suggest we should document these explicitly.

>
>
>> +
>> +Supported encapsulated packet types:
>> +\begin{itemize}
>> +\item \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]}
>> +\item \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]}
>> +\item \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]}
>> +\item \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]}
>> +\item \hyperref[intro:vxlan]{[VXLAN]}
>> +\item \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]}
>> +\item \hyperref[intro:geneve]{[GENEVE]}
>> +\item \hyperref[intro:ipip]{[IPIP]}
>> +\item \hyperref[intro:nvgre]{[NVGRE]}
>> +\item \hyperref[intro:sit]{[STT]}
>> +\end{itemize}
>> +
>> +If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type 
>> is not included in
>> +\field{hash_tunnel_types}, the hash of the outer header is 
>> calculated for the received encapsulated packet.
>> +
>> +The hash is calculated for the received non-encapsulated packet as 
>> if VIRTIO_NET_F_HASH_TUNNEL was not negotiated.
>> +
>> +\subparagraph{Supported/enabled encapsulation hash types}
>> +\label{sec:Device Types / Network Device / Device Operation / 
>> Processing of Incoming Packets / Hash calculation for incoming 
>> packets / Supported/enabled encapsulation hash types}
>> +
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NONE        (1 << 0)
>> +\end{lstlisting}
>> +
>> +Supported encapsulation hash types:
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:gre_rfc2784]{[GRE_rfc2784]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2784    (1 << 1)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:gre_rfc2890]{[GRE_rfc2890]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_2890    (1 << 2)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:gre_rfc7676]{[GRE_rfc7676]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_7676    (1 << 3)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:gre_in_udp_rfc8086]{[GRE-in-UDP]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GRE_UDP     (1 << 4)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:vxlan]{[VXLAN]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN       (1 << 5)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:vxlan_gpe]{[VXLAN-GPE]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_VXLAN_GPE   (1 << 6)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:geneve]{[GENEVE]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_GENEVE      (1 << 7)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:ipip]{[IPIP]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_IPIP        (1 << 8)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:nvgre]{[NVGRE]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_NVGRE       (1 << 9)
>> +\end{lstlisting}
>> +Hash type applicable for inner payload of the 
>> \hyperref[intro:stt]{[STT]} packet:
>> +\begin{lstlisting}
>> +#define VIRTIO_NET_HASH_TUNNEL_TYPE_STT         (1 << 10)
>> +\end{lstlisting}
>> +
>> +\subparagraph{Tunnel QoS limitation}
>> +When a specific receive queue is shared by multiple tunnels to 
>> receive encapsulated packets,
>> +there is no quality of service (QoS) for these packets.
>
>
> This is up to the implementation, so I'm not sure it's worthwhile for 
> mentioning things like this and the "limitation".
>

Yes, I see what you mean. As we discussed before, similar to the RFCs of 
various tunnels, they all briefly mention
security issues. Perhaps we can modify them into some simple suggestions 
here, similar to Tunnel QoS Advices:
(1)Use a tool with good forwarding performance to keep the receive queue 
from filling up;
(2)...

Anyway, I'm ok with these, it's up to you.

>
>>   For example, when the packets of certain
>> +tunnels are spread across multiple receive queues, these receive 
>> queues may have an unbalanced
>> +amount of packets. This can cause a specific receive queue to become 
>> full, resulting in packet loss.
>
>
> We have many places that can lead to packet dropping. For example, the 
> automatic steering is best effort. I tend to avoid mentioning things 
> like this.

Ok. And Michael what do you think about this?

>
>
>> +
>> +Possible mitigations:
>> +\begin{itemize}
>> +\item Use a tool with good forwarding performance to keep the 
>> receive queue from filling up.
>> +\item If the QoS is unavailable, the driver can set 
>> \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
>> +      to disable inner header hash for encapsulated packets.
>> +\item Choose a hash key that can avoid queue collisions.
>> +\item Perform appropriate QoS before packets consume the receive 
>> buffers of the receive queues.
>> +\end{itemize}
>> +
>> +The limitations mentioned above exist with/without the inner header 
>> hash.
>
>
> This conflicts with the tile "Tunnel QoS limitation" which readers may 
> think it happens only for tunnel.

Perhaps a "QoS Advices" is better?

Thanks!

>
> Thanks
>
>
> 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/


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-13 11:03   ` [virtio-dev] Re: [virtio-comment] " Heng Qi
@ 2023-04-13 21:43     ` Michael S. Tsirkin
  2023-04-14  3:56       ` Heng Qi
  2023-04-13 21:46     ` Michael S. Tsirkin
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2023-04-13 21:43 UTC (permalink / raw)
  To: Heng Qi
  Cc: Jason Wang, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo

On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > >   For example, when the packets of certain
> > > +tunnels are spread across multiple receive queues, these receive
> > > queues may have an unbalanced
> > > +amount of packets. This can cause a specific receive queue to
> > > become full, resulting in packet loss.
> > 
> > 
> > We have many places that can lead to packet dropping. For example, the
> > automatic steering is best effort. I tend to avoid mentioning things
> > like this.
> 
> Ok. And Michael what do you think about this?


I think this text did not do a great job explaining the
security aspect. Here's a better, shorter explanation:

	It is often an expectation of users that a tunnel isolates the external
	network from the internal one. By completely ignoring entropy in the
	external header and replacing it with entropy from the internal header,
	for hash calculations, this expectation might be violated to a certain
	extent, depending on how the hash is used. When the hash use is limited
	to RSS queue selection, the effect will likely be limited to ability of
	users inside the tunnel to cause packet drops in multiple queues (as
	opposed to a single queue without the feature).

> > 
> > 
> > > +
> > > +Possible mitigations:
> > > +\begin{itemize}
> > > +\item Use a tool with good forwarding performance to keep the
> > > receive queue from filling up.
> > > +\item If the QoS is unavailable, the driver can set
> > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > +      to disable inner header hash for encapsulated packets.
> > > +\item Choose a hash key that can avoid queue collisions.
> > > +\item Perform appropriate QoS before packets consume the receive
> > > buffers of the receive queues.
> > > +\end{itemize}
> > > +
> > > +The limitations mentioned above exist with/without the inner header
> > > hash.
> > 
> > 
> > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > think it happens only for tunnel.
> 
> Perhaps a "QoS Advices" is better?

Plural of "advice" is "advice" not "advices".

This advice is somewhat bogus though.

The point I keep trying to make is that this:

	Choose a hash key that can avoid queue collisions.

is impossible with the feature and possible without.
This was the whole reason I asked for a security
considerations sections.


> Thanks!
> 
> > 
> > Thanks
> > 
> > 
> > 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/


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-13 11:03   ` [virtio-dev] Re: [virtio-comment] " Heng Qi
  2023-04-13 21:43     ` Michael S. Tsirkin
@ 2023-04-13 21:46     ` Michael S. Tsirkin
  2023-04-14  3:10       ` Jason Wang
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2023-04-13 21:46 UTC (permalink / raw)
  To: Heng Qi
  Cc: Jason Wang, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo

On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > >   For example, when the packets of certain
> > > +tunnels are spread across multiple receive queues, these receive
> > > queues may have an unbalanced
> > > +amount of packets. This can cause a specific receive queue to
> > > become full, resulting in packet loss.
> > 
> > 
> > We have many places that can lead to packet dropping. For example, the
> > automatic steering is best effort. I tend to avoid mentioning things
> > like this.
> 
> Ok. And Michael what do you think about this?


I think this text did not do a great job explaining the
security aspect. Here's a better, shorter explanation:

	It is often an expectation of users that a tunnel isolates the external
	network from the internal one. By completely ignoring entropy in the
	external header and replacing it with entropy from the internal header,
	for hash calculations, this expectation might be violated to a certain
	extent, depending on how the hash is used. When the hash use is limited
	to RSS queue selection, the effect will likely be limited to ability of
	users inside the tunnel to cause packet drops in multiple queues (as
	opposed to a single queue without the feature).

> > 
> > 
> > > +
> > > +Possible mitigations:
> > > +\begin{itemize}
> > > +\item Use a tool with good forwarding performance to keep the
> > > receive queue from filling up.
> > > +\item If the QoS is unavailable, the driver can set
> > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > +      to disable inner header hash for encapsulated packets.
> > > +\item Choose a hash key that can avoid queue collisions.
> > > +\item Perform appropriate QoS before packets consume the receive
> > > buffers of the receive queues.
> > > +\end{itemize}
> > > +
> > > +The limitations mentioned above exist with/without the inner header
> > > hash.
> > 
> > 
> > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > think it happens only for tunnel.
> 
> Perhaps a "QoS Advices" is better?

Plural of "advice" is "advice" not "advices".

This advice is somewhat bogus though.

The point I keep trying to make is that this:

	Choose a hash key that can avoid queue collisions.

is impossible with the feature and possible without.
This was the whole reason I asked for a security
considerations sections.

-- 
MST


---------------------------------------------------------------------
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] 17+ messages in thread

* [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-13 21:46     ` Michael S. Tsirkin
@ 2023-04-14  3:10       ` Jason Wang
  2023-04-14  4:00         ` Heng Qi
  2023-04-14  7:18         ` Michael S. Tsirkin
  0 siblings, 2 replies; 17+ messages in thread
From: Jason Wang @ 2023-04-14  3:10 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Heng Qi, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo

On Fri, Apr 14, 2023 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > > >   For example, when the packets of certain
> > > > +tunnels are spread across multiple receive queues, these receive
> > > > queues may have an unbalanced
> > > > +amount of packets. This can cause a specific receive queue to
> > > > become full, resulting in packet loss.
> > >
> > >
> > > We have many places that can lead to packet dropping. For example, the
> > > automatic steering is best effort. I tend to avoid mentioning things
> > > like this.
> >
> > Ok. And Michael what do you think about this?
>
>
> I think this text did not do a great job explaining the
> security aspect. Here's a better, shorter explanation:
>
>         It is often an expectation of users that a tunnel isolates the external
>         network from the internal one. By completely ignoring entropy in the
>         external header and replacing it with entropy from the internal header,
>         for hash calculations, this expectation might be violated to a certain
>         extent, depending on how the hash is used. When the hash use is limited
>         to RSS queue selection, the effect will likely be limited to ability of
>         users inside the tunnel to cause packet drops in multiple queues (as
>         opposed to a single queue without the feature).

And this is only for GRE-in-UDP? This makes me think if we should add
GRE support for the outer header like:

https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tuple-GREv0

Thanks


>
> > >
> > >
> > > > +
> > > > +Possible mitigations:
> > > > +\begin{itemize}
> > > > +\item Use a tool with good forwarding performance to keep the
> > > > receive queue from filling up.
> > > > +\item If the QoS is unavailable, the driver can set
> > > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > > +      to disable inner header hash for encapsulated packets.
> > > > +\item Choose a hash key that can avoid queue collisions.
> > > > +\item Perform appropriate QoS before packets consume the receive
> > > > buffers of the receive queues.
> > > > +\end{itemize}
> > > > +
> > > > +The limitations mentioned above exist with/without the inner header
> > > > hash.
> > >
> > >
> > > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > > think it happens only for tunnel.
> >
> > Perhaps a "QoS Advices" is better?
>
> Plural of "advice" is "advice" not "advices".
>
> This advice is somewhat bogus though.
>
> The point I keep trying to make is that this:
>
>         Choose a hash key that can avoid queue collisions.
>
> is impossible with the feature and possible without.
> This was the whole reason I asked for a security
> considerations sections.
>
> --
> MST
>


---------------------------------------------------------------------
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] 17+ messages in thread

* Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-13 21:43     ` Michael S. Tsirkin
@ 2023-04-14  3:56       ` Heng Qi
  2023-04-14  7:06         ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Heng Qi @ 2023-04-14  3:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Jason Wang, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo



在 2023/4/14 上午5:43, Michael S. Tsirkin 写道:
> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
>>>>    For example, when the packets of certain
>>>> +tunnels are spread across multiple receive queues, these receive
>>>> queues may have an unbalanced
>>>> +amount of packets. This can cause a specific receive queue to
>>>> become full, resulting in packet loss.
>>>
>>> We have many places that can lead to packet dropping. For example, the
>>> automatic steering is best effort. I tend to avoid mentioning things
>>> like this.
>> Ok. And Michael what do you think about this?
>
> I think this text did not do a great job explaining the
> security aspect. Here's a better, shorter explanation:
>
> 	It is often an expectation of users that a tunnel isolates the external
> 	network from the internal one. By completely ignoring entropy in the
> 	external header and replacing it with entropy from the internal header,
> 	for hash calculations, this expectation might be violated to a certain
> 	extent, depending on how the hash is used. When the hash use is limited
> 	to RSS queue selection, the effect will likely be limited to ability of
> 	users inside the tunnel to cause packet drops in multiple queues (as
> 	opposed to a single queue without the feature).

Sure. Will do  in the v13.

>
>>>
>>>> +
>>>> +Possible mitigations:
>>>> +\begin{itemize}
>>>> +\item Use a tool with good forwarding performance to keep the
>>>> receive queue from filling up.
>>>> +\item If the QoS is unavailable, the driver can set
>>>> \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
>>>> +      to disable inner header hash for encapsulated packets.
>>>> +\item Choose a hash key that can avoid queue collisions.
>>>> +\item Perform appropriate QoS before packets consume the receive
>>>> buffers of the receive queues.
>>>> +\end{itemize}
>>>> +
>>>> +The limitations mentioned above exist with/without the inner header
>>>> hash.
>>>
>>> This conflicts with the tile "Tunnel QoS limitation" which readers may
>>> think it happens only for tunnel.
>> Perhaps a "QoS Advices" is better?
> Plural of "advice" is "advice" not "advices".

My fault.😅

>
> This advice is somewhat bogus though.
>
> The point I keep trying to make is that this:
>
> 	Choose a hash key that can avoid queue collisions.
>
> is impossible with the feature and possible without.

I don't think so, the outer headers also has corresponding entropy for 
different streams.

Thanks.

> This was the whole reason I asked for a security
> considerations sections.
>
>
>> Thanks!
>>
>>> Thanks
>>>
>>>
>>> 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/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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


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

* Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-14  3:10       ` Jason Wang
@ 2023-04-14  4:00         ` Heng Qi
  2023-04-14  4:08           ` Parav Pandit
  2023-04-14  7:18         ` Michael S. Tsirkin
  1 sibling, 1 reply; 17+ messages in thread
From: Heng Qi @ 2023-04-14  4:00 UTC (permalink / raw)
  To: Jason Wang, Michael S. Tsirkin
  Cc: virtio-dev, virtio-comment, Parav Pandit, Yuri Benditovich, Xuan Zhuo



在 2023/4/14 上午11:10, Jason Wang 写道:
> On Fri, Apr 14, 2023 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
>>>>>    For example, when the packets of certain
>>>>> +tunnels are spread across multiple receive queues, these receive
>>>>> queues may have an unbalanced
>>>>> +amount of packets. This can cause a specific receive queue to
>>>>> become full, resulting in packet loss.
>>>>
>>>> We have many places that can lead to packet dropping. For example, the
>>>> automatic steering is best effort. I tend to avoid mentioning things
>>>> like this.
>>> Ok. And Michael what do you think about this?
>>
>> I think this text did not do a great job explaining the
>> security aspect. Here's a better, shorter explanation:
>>
>>          It is often an expectation of users that a tunnel isolates the external
>>          network from the internal one. By completely ignoring entropy in the
>>          external header and replacing it with entropy from the internal header,
>>          for hash calculations, this expectation might be violated to a certain
>>          extent, depending on how the hash is used. When the hash use is limited
>>          to RSS queue selection, the effect will likely be limited to ability of
>>          users inside the tunnel to cause packet drops in multiple queues (as
>>          opposed to a single queue without the feature).
> And this is only for GRE-in-UDP? This makes me think if we should add
> GRE support for the outer header like:
>
> https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tuple-GREv0

I think this is for tunneling protocols with specific flow fields, such 
as GRE:key filed, NVGRE:FLOWID filed.

This requires us to make a requirement when calculating the hash of the 
tunnels when F_TUNNEL_HASH is not negotiated. It's a new work.

Thanks.

>
> Thanks
>
>
>>>>
>>>>> +
>>>>> +Possible mitigations:
>>>>> +\begin{itemize}
>>>>> +\item Use a tool with good forwarding performance to keep the
>>>>> receive queue from filling up.
>>>>> +\item If the QoS is unavailable, the driver can set
>>>>> \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
>>>>> +      to disable inner header hash for encapsulated packets.
>>>>> +\item Choose a hash key that can avoid queue collisions.
>>>>> +\item Perform appropriate QoS before packets consume the receive
>>>>> buffers of the receive queues.
>>>>> +\end{itemize}
>>>>> +
>>>>> +The limitations mentioned above exist with/without the inner header
>>>>> hash.
>>>>
>>>> This conflicts with the tile "Tunnel QoS limitation" which readers may
>>>> think it happens only for tunnel.
>>> Perhaps a "QoS Advices" is better?
>> Plural of "advice" is "advice" not "advices".
>>
>> This advice is somewhat bogus though.
>>
>> The point I keep trying to make is that this:
>>
>>          Choose a hash key that can avoid queue collisions.
>>
>> is impossible with the feature and possible without.
>> This was the whole reason I asked for a security
>> considerations sections.
>>
>> --
>> MST
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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


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

* RE: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-14  4:00         ` Heng Qi
@ 2023-04-14  4:08           ` Parav Pandit
  2023-04-14  4:54             ` Heng Qi
  0 siblings, 1 reply; 17+ messages in thread
From: Parav Pandit @ 2023-04-14  4:08 UTC (permalink / raw)
  To: Heng Qi, Jason Wang, Michael S. Tsirkin
  Cc: virtio-dev, virtio-comment, Yuri Benditovich, Xuan Zhuo



> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Friday, April 14, 2023 12:01 AM
> 
> 在 2023/4/14 上午11:10, Jason Wang 写道:
> > On Fri, Apr 14, 2023 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> >>>>>    For example, when the packets of certain
> >>>>> +tunnels are spread across multiple receive queues, these receive
> >>>>> queues may have an unbalanced
> >>>>> +amount of packets. This can cause a specific receive queue to
> >>>>> become full, resulting in packet loss.
> >>>>
> >>>> We have many places that can lead to packet dropping. For example,
> >>>> the automatic steering is best effort. I tend to avoid mentioning
> >>>> things like this.
> >>> Ok. And Michael what do you think about this?
> >>
> >> I think this text did not do a great job explaining the security
> >> aspect. Here's a better, shorter explanation:
> >>
> >>          It is often an expectation of users that a tunnel isolates the external
> >>          network from the internal one. By completely ignoring entropy in the
> >>          external header and replacing it with entropy from the internal header,
> >>          for hash calculations, this expectation might be violated to a certain
> >>          extent, depending on how the hash is used. When the hash use is
> limited
> >>          to RSS queue selection, the effect will likely be limited to ability of
> >>          users inside the tunnel to cause packet drops in multiple queues (as
> >>          opposed to a single queue without the feature).
> > And this is only for GRE-in-UDP? This makes me think if we should add
> > GRE support for the outer header like:
> >
> > https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tupl
> > e-GREv0
> 
> I think this is for tunneling protocols with specific flow fields, such as GRE:key
> filed, NVGRE:FLOWID filed.
> 
> This requires us to make a requirement when calculating the hash of the
> tunnels when F_TUNNEL_HASH is not negotiated. It's a new work.
> 
Yes, I think Jason is saying the same to extend the outer header hashing for newer protocols where outer header entropy is available.

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

* Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-14  4:08           ` Parav Pandit
@ 2023-04-14  4:54             ` Heng Qi
  0 siblings, 0 replies; 17+ messages in thread
From: Heng Qi @ 2023-04-14  4:54 UTC (permalink / raw)
  To: Parav Pandit, Jason Wang, Michael S. Tsirkin
  Cc: virtio-dev, virtio-comment, Yuri Benditovich, Xuan Zhuo



在 2023/4/14 下午12:08, Parav Pandit 写道:
>
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Friday, April 14, 2023 12:01 AM
>>
>> 在 2023/4/14 上午11:10, Jason Wang 写道:
>>> On Fri, Apr 14, 2023 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>>> On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
>>>>>>>     For example, when the packets of certain
>>>>>>> +tunnels are spread across multiple receive queues, these receive
>>>>>>> queues may have an unbalanced
>>>>>>> +amount of packets. This can cause a specific receive queue to
>>>>>>> become full, resulting in packet loss.
>>>>>> We have many places that can lead to packet dropping. For example,
>>>>>> the automatic steering is best effort. I tend to avoid mentioning
>>>>>> things like this.
>>>>> Ok. And Michael what do you think about this?
>>>> I think this text did not do a great job explaining the security
>>>> aspect. Here's a better, shorter explanation:
>>>>
>>>>           It is often an expectation of users that a tunnel isolates the external
>>>>           network from the internal one. By completely ignoring entropy in the
>>>>           external header and replacing it with entropy from the internal header,
>>>>           for hash calculations, this expectation might be violated to a certain
>>>>           extent, depending on how the hash is used. When the hash use is
>> limited
>>>>           to RSS queue selection, the effect will likely be limited to ability of
>>>>           users inside the tunnel to cause packet drops in multiple queues (as
>>>>           opposed to a single queue without the feature).
>>> And this is only for GRE-in-UDP? This makes me think if we should add
>>> GRE support for the outer header like:
>>>
>>> https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tupl
>>> e-GREv0
>> I think this is for tunneling protocols with specific flow fields, such as GRE:key
>> filed, NVGRE:FLOWID filed.
>>
>> This requires us to make a requirement when calculating the hash of the
>> tunnels when F_TUNNEL_HASH is not negotiated. It's a new work.
>>
> Yes, I think Jason is saying the same to extend the outer header hashing for newer protocols where outer header entropy is available.

I see:), and then we need a new feature bit to do this, which can enter 
the scheduling, but we have to consider the rest of our scheduling,
for example, I will first start with symmetric toeplitz hash/xor hash, 
and then here to things.
In addition, what I want to say is that we are also planning to do (1) 
time synchronization between drivers and devices (2) assist colleagues 
in doing hw rx timestamp, etc.

Thanks.



---------------------------------------------------------------------
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] 17+ messages in thread

* Re: [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-14  3:56       ` Heng Qi
@ 2023-04-14  7:06         ` Michael S. Tsirkin
  0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2023-04-14  7:06 UTC (permalink / raw)
  To: Heng Qi
  Cc: Jason Wang, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo

On Fri, Apr 14, 2023 at 11:56:05AM +0800, Heng Qi wrote:
> 
> 
> 在 2023/4/14 上午5:43, Michael S. Tsirkin 写道:
> > On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > > > >    For example, when the packets of certain
> > > > > +tunnels are spread across multiple receive queues, these receive
> > > > > queues may have an unbalanced
> > > > > +amount of packets. This can cause a specific receive queue to
> > > > > become full, resulting in packet loss.
> > > > 
> > > > We have many places that can lead to packet dropping. For example, the
> > > > automatic steering is best effort. I tend to avoid mentioning things
> > > > like this.
> > > Ok. And Michael what do you think about this?
> > 
> > I think this text did not do a great job explaining the
> > security aspect. Here's a better, shorter explanation:
> > 
> > 	It is often an expectation of users that a tunnel isolates the external
> > 	network from the internal one. By completely ignoring entropy in the
> > 	external header and replacing it with entropy from the internal header,
> > 	for hash calculations, this expectation might be violated to a certain
> > 	extent, depending on how the hash is used. When the hash use is limited
> > 	to RSS queue selection, the effect will likely be limited to ability of
> > 	users inside the tunnel to cause packet drops in multiple queues (as
> > 	opposed to a single queue without the feature).
> 
> Sure. Will do  in the v13.
> 
> > 
> > > > 
> > > > > +
> > > > > +Possible mitigations:
> > > > > +\begin{itemize}
> > > > > +\item Use a tool with good forwarding performance to keep the
> > > > > receive queue from filling up.
> > > > > +\item If the QoS is unavailable, the driver can set
> > > > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > > > +      to disable inner header hash for encapsulated packets.
> > > > > +\item Choose a hash key that can avoid queue collisions.
> > > > > +\item Perform appropriate QoS before packets consume the receive
> > > > > buffers of the receive queues.
> > > > > +\end{itemize}
> > > > > +
> > > > > +The limitations mentioned above exist with/without the inner header
> > > > > hash.
> > > > 
> > > > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > > > think it happens only for tunnel.
> > > Perhaps a "QoS Advices" is better?
> > Plural of "advice" is "advice" not "advices".
> 
> My fault.😅
> 
> > 
> > This advice is somewhat bogus though.
> > 
> > The point I keep trying to make is that this:
> > 
> > 	Choose a hash key that can avoid queue collisions.
> > 
> > is impossible with the feature and possible without.
> 
> I don't think so, the outer headers also has corresponding entropy for
> different streams.

But the feature when enabled ignores this entropy.

> Thanks.
> 
> > This was the whole reason I asked for a security
> > considerations sections.
> > 
> > 
> > > Thanks!
> > > 
> > > > Thanks
> > > > 
> > > > 
> > > > 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/
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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


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

* [virtio-dev] Re: [virtio-comment] Re: [PATCH v12] virtio-net: support inner header hash
  2023-04-14  3:10       ` Jason Wang
  2023-04-14  4:00         ` Heng Qi
@ 2023-04-14  7:18         ` Michael S. Tsirkin
  1 sibling, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2023-04-14  7:18 UTC (permalink / raw)
  To: Jason Wang
  Cc: Heng Qi, virtio-dev, virtio-comment, Parav Pandit,
	Yuri Benditovich, Xuan Zhuo

On Fri, Apr 14, 2023 at 11:10:36AM +0800, Jason Wang wrote:
> On Fri, Apr 14, 2023 at 5:46 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Apr 13, 2023 at 07:03:26PM +0800, Heng Qi wrote:
> > > > >   For example, when the packets of certain
> > > > > +tunnels are spread across multiple receive queues, these receive
> > > > > queues may have an unbalanced
> > > > > +amount of packets. This can cause a specific receive queue to
> > > > > become full, resulting in packet loss.
> > > >
> > > >
> > > > We have many places that can lead to packet dropping. For example, the
> > > > automatic steering is best effort. I tend to avoid mentioning things
> > > > like this.
> > >
> > > Ok. And Michael what do you think about this?
> >
> >
> > I think this text did not do a great job explaining the
> > security aspect. Here's a better, shorter explanation:
> >
> >         It is often an expectation of users that a tunnel isolates the external
> >         network from the internal one. By completely ignoring entropy in the
> >         external header and replacing it with entropy from the internal header,
> >         for hash calculations, this expectation might be violated to a certain
> >         extent, depending on how the hash is used. When the hash use is limited
> >         to RSS queue selection, the effect will likely be limited to ability of
> >         users inside the tunnel to cause packet drops in multiple queues (as
> >         opposed to a single queue without the feature).
> 
> And this is only for GRE-in-UDP?


This is whenever we discard outer header entropy.

> This makes me think if we should add
> GRE support for the outer header like:
> 
> https://docs.napatech.com/r/Feature-Set-N-ANL9/Hash-Key-Type-10-3-Tuple-GREv0
> 
> Thanks

I think this feature is only useful when entropy from inner header
is not exported to outer header, e.g. for legacy GRE
https://datatracker.ietf.org/doc/rfc2784/
When there's entropy in outer header, it's best to just use that.
This will mean that we do not need to keep adding stuff to
spec as more tunneling protocols appear.


> 
> >
> > > >
> > > >
> > > > > +
> > > > > +Possible mitigations:
> > > > > +\begin{itemize}
> > > > > +\item Use a tool with good forwarding performance to keep the
> > > > > receive queue from filling up.
> > > > > +\item If the QoS is unavailable, the driver can set
> > > > > \field{hash_tunnel_types} to VIRTIO_NET_HASH_TUNNEL_TYPE_NONE
> > > > > +      to disable inner header hash for encapsulated packets.
> > > > > +\item Choose a hash key that can avoid queue collisions.
> > > > > +\item Perform appropriate QoS before packets consume the receive
> > > > > buffers of the receive queues.
> > > > > +\end{itemize}
> > > > > +
> > > > > +The limitations mentioned above exist with/without the inner header
> > > > > hash.
> > > >
> > > >
> > > > This conflicts with the tile "Tunnel QoS limitation" which readers may
> > > > think it happens only for tunnel.
> > >
> > > Perhaps a "QoS Advices" is better?
> >
> > Plural of "advice" is "advice" not "advices".
> >
> > This advice is somewhat bogus though.
> >
> > The point I keep trying to make is that this:
> >
> >         Choose a hash key that can avoid queue collisions.
> >
> > is impossible with the feature and possible without.
> > This was the whole reason I asked for a security
> > considerations sections.
> >
> > --
> > MST
> >


---------------------------------------------------------------------
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] 17+ messages in thread

end of thread, other threads:[~2023-04-14  7:20 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-03  4:58 [virtio-dev] [PATCH v12] virtio-net: support inner header hash Heng Qi
2023-04-03  5:08 ` Heng Qi
2023-04-11 12:08 ` [virtio-dev] Re: [virtio-comment] " Heng Qi
2023-04-11 21:03 ` [virtio-dev] " Parav Pandit
2023-04-12  3:05   ` Heng Qi
2023-04-12  3:30     ` Parav Pandit
2023-04-13  3:40 ` Jason Wang
2023-04-13 11:03   ` [virtio-dev] Re: [virtio-comment] " Heng Qi
2023-04-13 21:43     ` Michael S. Tsirkin
2023-04-14  3:56       ` Heng Qi
2023-04-14  7:06         ` Michael S. Tsirkin
2023-04-13 21:46     ` Michael S. Tsirkin
2023-04-14  3:10       ` Jason Wang
2023-04-14  4:00         ` Heng Qi
2023-04-14  4:08           ` Parav Pandit
2023-04-14  4:54             ` Heng Qi
2023-04-14  7:18         ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).