[virtio-dev] [RFC virtio-net 0/1] introducing RSS into virtio-net

[virtio-dev] [RFC virtio-net 0/1] introducing RSS into virtio-net

[Sameeh Jubran]

From: Sameeh Jubran <sjubran@redhat.com>

Hi all,

Most modern high end network devices today support configurable hash functions,
the following commit introduces RSS - Receive Side Scaling - [1] into virtio net
device. RSS is a technology developed by Microsoft that boosts network device
performance by efficiently distributing the traffic among the CPUs in a
multiprocessor system.

This feature is supported in most of the modern network cards as well as most
modern OSes including Linux and Windows. It is worth mentioning that both DPDK
and Hyper-v support RSS too.

Currently in NetKVM - the Windows driver for virtio-net - RSS is supported in
the driver (software implementation) this adds extra overhead to the driver
that's reflected in unneeded IPIs and TLB flushes, adding this support to the
backend should enhance the performance and help avoid these overheads.

For all the reasons I mentioned above I think it is time for virtio-net to
support this feature, please share your thoughts.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2

Thanks,
Sameeh


Sameeh Jubran (1):
  content: net: Add VIRTIO_NET_F_CTRL_RSS feature

 content.tex | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

-- 
2.13.6


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

[virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

From: Sameeh Jubran <sameeh.j@gmail.com>

Most modern high end network devices today support configurable hash functions,
this commit introduces RSS - Receive Side Scaling - [1] to virtio net device.

The RSS is a technology from Microsoft that boosts network device performance
by efficiently distributing the traffic among the CPUs in a multiprocessor
system.

This feature is supported in most of the modern network cards as well as most
modern OSes including Linux and Windows. It is worth mentioning that both DPDK
and Hyper-v support RSS too.

[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2

Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
---
 content.tex | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/content.tex b/content.tex
index c840588..5969b28 100644
--- a/content.tex
+++ b/content.tex
@@ -3115,6 +3115,9 @@ features.
 
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
+
+\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
+    functions.
 \end{description}
 
 \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
@@ -3138,6 +3141,8 @@ Some networking feature bits require other networking feature bits
 \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
+
+\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
 \end{description}
 
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
@@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
         u8 flags;
 #define VIRTIO_NET_HDR_GSO_NONE        0
 #define VIRTIO_NET_HDR_GSO_TCPV4       1
+#define VIRTIO_NET_HDR_GSO_RSS         2
 #define VIRTIO_NET_HDR_GSO_UDP         3
 #define VIRTIO_NET_HDR_GSO_TCPV6       4
 #define VIRTIO_NET_HDR_GSO_ECN      0x80
@@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
         le16 csum_start;
         le16 csum_offset;
         le16 num_buffers;
+// Only if RSS hash offload has been negotiated
+        le64 rss_hash_value;
 };
 \end{lstlisting}
 
@@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
 The device MUST NOT queue packets on receive queues greater than
 \field{virtqueue_pairs} once it has placed the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
 
+\paragraph{RSS hash offload}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
+
+\begin{lstlisting}
+#define RSS_HASH_FUNCTION_NONE      1
+#define RSS_HASH_FUNCTION_TOEPLITZ  2
+#define RSS_HASH_FUNCTION_SYMMETRIC 3
+
+// Hash function fields
+#define RSS_HASH_FIELDS_IPV4          0x00000100
+#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
+#define RSS_HASH_FIELDS_IPV6          0x00000400
+#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
+#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
+#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
+
+struct virtio_net_ctrl_rss_hash{
+le32 hash_function;
+}
+
+struct virtio_net_rss {
+le32 hash_function;
+le32 hash_function_flags;
+le32 hash_key_length;
+le32 indirection_table_length;
+	struct {
+		le32 hash_key[hash_key_length];
+		le32 indirection_table[indirection_table_length];
+	}
+};
+
+#define VIRTIO_NET_F_CTRL_RSS     24
+
+#define VIRTIO_NET_CTRL_RSS                           6
+#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
+#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
+#define VIRTIO_NET_CTRL_RSS_SET                       2
+\end{lstlisting}
+
+If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
+commands for the RSS configuration.
+
+The class VIRTIO_NET_CTRL_RSS has three commands:
+
+\begin{enumerate}
+\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash functions
+	supported by the device to the driver.
+\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing is no
+	longer required.
+\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The command is
+	used by the driver for setting RSS hash function, hash key and
+	indirection table in the device.
+\end{enumerate}
+
+If this feaure has been negotiated, the virtio header has an additional
+field{rss_hash_value} field attached to it.
+
+\devicenormative{\subparagraph}{RSS hash offload}{Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
+
+The device MUST fill the virtio_net_ctrl_rss_hash structure with the hash
+functions it supports and return the structure to the driver. Zero or more
+flags of the RSS_HASH_FUNCTION flags MUST be used to fill the \field{hash_function}
+field.
+
+Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop calculating and
+attaching hashes for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
+in the \field{gso_type} field, however the \field{hash_function} field is kept
+as a part of the header.
+
+The device MUST drop all previous RSS configuration upon receiving
+VIRTIO_NET_CTRL_RSS_SET command.
+
+The device MUST set the RSS configuration according to the settings provided as
+follows, once the configuration process is completed the device SHOULD apply
+the hash function to each of the incoming packets and distribute the packets
+through the virqueues using the calculated hash and the indirection table
+that were earlier provided by the driver.
+
+Setting RSS configuration
+\begin{enumerate}
+\item The driver fills all of the fields and passes them through the control
+	queue to the device.
+
+\item The device sets the RSS configuration as provided by the driver.
+
+\item If the device successfully applied the configuration, on each packet
+	recieved the device MUST calculate the hashing for the packet and
+	store it in the virtio-net header in rss_hash_value and the hash
+	fields used in the calculation in rss_hash_type.
+\end{enumerate}
+
+In case of any unexpected values/ unsupported hash function the driver
+MUST return VIRTIO_NET_ERR in the \field{ack} field.
+
+\drivernormative{\subparagraph}{RSS hash offload}{Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
+
+If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver SHOULD
+send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS structure
+filled. The RSS structure fields should be filled as follows:
+
+\begin{itemize}
+\item The driver SHOULD choose the hash function that SHOULD be used and fill
+	it in the \field{hash_function} field along with the approperiate flags
+	in the \field{hash_function_flags} field. These flags inidicate to the
+	device which packet fields MUST be used in the calculation process of
+	the hash.
+\item Once the hash function has been chosen a suitable hash key should be set
+	in the \field{hash_key} field, the length of the key should be stored
+	in the \field{hash_key_length} field.
+\item Lastly the driver should fill the indirection table array in the
+	\field{indirection_table} field while setting the array length in
+	\field{indirection_table_length}. This structure is used by the device
+	for determining in which RX virt queue the packet should be placed.
+\end{itemize}
+Once the configuration phase is over successfully, the packets SHOULD have the
+\field{rss_hash_value} field with the hash value that was calculated by the
+device.
+
+Whenever the driver wants to discard the current RSS settings, it can send an
+VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
+RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
+
+The driver MUST check the \field{ack} field value provided by the device, in
+case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure and
+retry another hash function or else give up.
+
 \subparagraph{Legacy Interface: Automatic receive steering in multiqueue mode}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Legacy Interface: Automatic receive steering in multiqueue mode}
 When using the legacy interface, transitional devices and drivers
 MUST format \field{virtqueue_pairs}
-- 
2.13.6


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

[virtio-dev] Re: [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

[-- Attachment #1: Type: text/plain, Size: 9300 bytes --]

Please ignore the typos, I mistakenly sent the patch without the fixes.
I'll resend the fixes along with v2.

On Thu, Jan 25, 2018 at 6:27 PM, Sameeh Jubran <sameeh@daynix.com> wrote:

> From: Sameeh Jubran <sameeh.j@gmail.com>
>
> Most modern high end network devices today support configurable hash
> functions,
> this commit introduces RSS - Receive Side Scaling - [1] to virtio net
> device.
>
> The RSS is a technology from Microsoft that boosts network device
> performance
> by efficiently distributing the traffic among the CPUs in a multiprocessor
> system.
>
> This feature is supported in most of the modern network cards as well as
> most
> modern OSes including Linux and Windows. It is worth mentioning that both
> DPDK
> and Hyper-v support RSS too.
>
> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/
> network/ndis-receive-side-scaling2
>
> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
> ---
>  content.tex | 133 ++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index c840588..5969b28 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3115,6 +3115,9 @@ features.
>
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
> +    functions.
>  \end{description}
>
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network
> Device / Feature bits / Feature bit requirements}
> @@ -3138,6 +3141,8 @@ Some networking feature bits require other
> networking feature bits
>  \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> +
> +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>  \end{description}
>
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types /
> Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>          u8 flags;
>  #define VIRTIO_NET_HDR_GSO_NONE        0
>  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> +#define VIRTIO_NET_HDR_GSO_RSS         2
>  #define VIRTIO_NET_HDR_GSO_UDP         3
>  #define VIRTIO_NET_HDR_GSO_TCPV6       4
>  #define VIRTIO_NET_HDR_GSO_ECN      0x80
> @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>          le16 csum_start;
>          le16 csum_offset;
>          le16 num_buffers;
> +// Only if RSS hash offload has been negotiated
> +        le64 rss_hash_value;
>  };
>  \end{lstlisting}
>
> @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>  The device MUST NOT queue packets on receive queues greater than
>  \field{virtqueue_pairs} once it has placed the
> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>
> +\paragraph{RSS hash offload}\label{sec:Device Types / Network Device /
> Device Operation / Control Virtqueue / RSS hash offload}
> +
> +\begin{lstlisting}
> +#define RSS_HASH_FUNCTION_NONE      1
> +#define RSS_HASH_FUNCTION_TOEPLITZ  2
> +#define RSS_HASH_FUNCTION_SYMMETRIC 3
> +
> +// Hash function fields
> +#define RSS_HASH_FIELDS_IPV4          0x00000100
> +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
> +#define RSS_HASH_FIELDS_IPV6          0x00000400
> +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
> +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
> +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
> +
> +struct virtio_net_ctrl_rss_hash{
> +le32 hash_function;
> +}
> +
> +struct virtio_net_rss {
> +le32 hash_function;
> +le32 hash_function_flags;
> +le32 hash_key_length;
> +le32 indirection_table_length;
> +       struct {
> +               le32 hash_key[hash_key_length];
> +               le32 indirection_table[indirection_table_length];
> +       }
> +};
> +
> +#define VIRTIO_NET_F_CTRL_RSS     24
> +
> +#define VIRTIO_NET_CTRL_RSS                           6
> +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
> +#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
> +#define VIRTIO_NET_CTRL_RSS_SET                       2
> +\end{lstlisting}
> +
> +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
> +commands for the RSS configuration.
> +
> +The class VIRTIO_NET_CTRL_RSS has three commands:
> +
> +\begin{enumerate}
> +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash
> functions
> +       supported by the device to the driver.
> +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing
> is no
> +       longer required.
> +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The
> command is
> +       used by the driver for setting RSS hash function, hash key and
> +       indirection table in the device.
> +\end{enumerate}
> +
> +If this feaure has been negotiated, the virtio header has an additional
> +field{rss_hash_value} field attached to it.
> +
> +\devicenormative{\subparagraph}{RSS hash offload}{Device Types / Network
> Device / Device Operation / Control Virtqueue / RSS hash offload}
> +
> +The device MUST fill the virtio_net_ctrl_rss_hash structure with the hash
> +functions it supports and return the structure to the driver. Zero or more
> +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
> \field{hash_function}
> +field.
> +
> +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop calculating
> and
> +attaching hashes for the packets as well as stop setting the
> VIRTIO_NET_HDR_GSO_RSS
> +in the \field{gso_type} field, however the \field{hash_function} field is
> kept
> +as a part of the header.
> +
> +The device MUST drop all previous RSS configuration upon receiving
> +VIRTIO_NET_CTRL_RSS_SET command.
> +
> +The device MUST set the RSS configuration according to the settings
> provided as
> +follows, once the configuration process is completed the device SHOULD
> apply
> +the hash function to each of the incoming packets and distribute the
> packets
> +through the virqueues using the calculated hash and the indirection table
> +that were earlier provided by the driver.
> +
> +Setting RSS configuration
> +\begin{enumerate}
> +\item The driver fills all of the fields and passes them through the
> control
> +       queue to the device.
> +
> +\item The device sets the RSS configuration as provided by the driver.
> +
> +\item If the device successfully applied the configuration, on each packet
> +       recieved the device MUST calculate the hashing for the packet and
> +       store it in the virtio-net header in rss_hash_value and the hash
> +       fields used in the calculation in rss_hash_type.
> +\end{enumerate}
> +
> +In case of any unexpected values/ unsupported hash function the driver
> +MUST return VIRTIO_NET_ERR in the \field{ack} field.
> +
> +\drivernormative{\subparagraph}{RSS hash offload}{Device Types / Network
> Device / Device Operation / Control Virtqueue / RSS hash offload}
> +
> +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver
> SHOULD
> +send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS
> structure
> +filled. The RSS structure fields should be filled as follows:
> +
> +\begin{itemize}
> +\item The driver SHOULD choose the hash function that SHOULD be used and
> fill
> +       it in the \field{hash_function} field along with the approperiate
> flags
> +       in the \field{hash_function_flags} field. These flags inidicate to
> the
> +       device which packet fields MUST be used in the calculation process
> of
> +       the hash.
> +\item Once the hash function has been chosen a suitable hash key should
> be set
> +       in the \field{hash_key} field, the length of the key should be
> stored
> +       in the \field{hash_key_length} field.
> +\item Lastly the driver should fill the indirection table array in the
> +       \field{indirection_table} field while setting the array length in
> +       \field{indirection_table_length}. This structure is used by the
> device
> +       for determining in which RX virt queue the packet should be placed.
> +\end{itemize}
> +Once the configuration phase is over successfully, the packets SHOULD
> have the
> +\field{rss_hash_value} field with the hash value that was calculated by
> the
> +device.
> +
> +Whenever the driver wants to discard the current RSS settings, it can
> send an
> +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
> +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
> +
> +The driver MUST check the \field{ack} field value provided by the device,
> in
> +case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure
> and
> +retry another hash function or else give up.
> +
>  \subparagraph{Legacy Interface: Automatic receive steering in multiqueue
> mode}\label{sec:Device Types / Network Device / Device Operation / Control
> Virtqueue / Automatic receive steering in multiqueue mode / Legacy
> Interface: Automatic receive steering in multiqueue mode}
>  When using the legacy interface, transitional devices and drivers
>  MUST format \field{virtqueue_pairs}
> --
> 2.13.6
>
>


-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*

[-- Attachment #2: Type: text/html, Size: 11892 bytes --]

[virtio-dev] Re: [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

[-- Attachment #1: Type: text/plain, Size: 9806 bytes --]

ping

On Thu, Jan 25, 2018 at 6:53 PM, Sameeh Jubran <sameeh@daynix.com> wrote:

> Please ignore the typos, I mistakenly sent the patch without the fixes.
> I'll resend the fixes along with v2.
>
> On Thu, Jan 25, 2018 at 6:27 PM, Sameeh Jubran <sameeh@daynix.com> wrote:
>
>> From: Sameeh Jubran <sameeh.j@gmail.com>
>>
>> Most modern high end network devices today support configurable hash
>> functions,
>> this commit introduces RSS - Receive Side Scaling - [1] to virtio net
>> device.
>>
>> The RSS is a technology from Microsoft that boosts network device
>> performance
>> by efficiently distributing the traffic among the CPUs in a multiprocessor
>> system.
>>
>> This feature is supported in most of the modern network cards as well as
>> most
>> modern OSes including Linux and Windows. It is worth mentioning that both
>> DPDK
>> and Hyper-v support RSS too.
>>
>> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ne
>> twork/ndis-receive-side-scaling2
>>
>> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
>> ---
>>  content.tex | 133 ++++++++++++++++++++++++++++++
>> ++++++++++++++++++++++++++++++
>>  1 file changed, 133 insertions(+)
>>
>> diff --git a/content.tex b/content.tex
>> index c840588..5969b28 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -3115,6 +3115,9 @@ features.
>>
>>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>      channel.
>> +
>> +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
>> +    functions.
>>  \end{description}
>>
>>  \subsubsection{Feature bit requirements}\label{sec:Device Types /
>> Network Device / Feature bits / Feature bit requirements}
>> @@ -3138,6 +3141,8 @@ Some networking feature bits require other
>> networking feature bits
>>  \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>>  \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>> +
>> +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>>  \end{description}
>>
>>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types /
>> Network Device / Feature bits / Legacy Interface: Feature bits}
>> @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>>          u8 flags;
>>  #define VIRTIO_NET_HDR_GSO_NONE        0
>>  #define VIRTIO_NET_HDR_GSO_TCPV4       1
>> +#define VIRTIO_NET_HDR_GSO_RSS         2
>>  #define VIRTIO_NET_HDR_GSO_UDP         3
>>  #define VIRTIO_NET_HDR_GSO_TCPV6       4
>>  #define VIRTIO_NET_HDR_GSO_ECN      0x80
>> @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>>          le16 csum_start;
>>          le16 csum_offset;
>>          le16 num_buffers;
>> +// Only if RSS hash offload has been negotiated
>> +        le64 rss_hash_value;
>>  };
>>  \end{lstlisting}
>>
>> @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>>  The device MUST NOT queue packets on receive queues greater than
>>  \field{virtqueue_pairs} once it has placed the
>> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>>
>> +\paragraph{RSS hash offload}\label{sec:Device Types / Network Device /
>> Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +\begin{lstlisting}
>> +#define RSS_HASH_FUNCTION_NONE      1
>> +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>> +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>> +
>> +// Hash function fields
>> +#define RSS_HASH_FIELDS_IPV4          0x00000100
>> +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>> +#define RSS_HASH_FIELDS_IPV6          0x00000400
>> +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>> +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>> +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>> +
>> +struct virtio_net_ctrl_rss_hash{
>> +le32 hash_function;
>> +}
>> +
>> +struct virtio_net_rss {
>> +le32 hash_function;
>> +le32 hash_function_flags;
>> +le32 hash_key_length;
>> +le32 indirection_table_length;
>> +       struct {
>> +               le32 hash_key[hash_key_length];
>> +               le32 indirection_table[indirection_table_length];
>> +       }
>> +};
>> +
>> +#define VIRTIO_NET_F_CTRL_RSS     24
>> +
>> +#define VIRTIO_NET_CTRL_RSS                           6
>> +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
>> +#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
>> +#define VIRTIO_NET_CTRL_RSS_SET                       2
>> +\end{lstlisting}
>> +
>> +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
>> +commands for the RSS configuration.
>> +
>> +The class VIRTIO_NET_CTRL_RSS has three commands:
>> +
>> +\begin{enumerate}
>> +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash
>> functions
>> +       supported by the device to the driver.
>> +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing
>> is no
>> +       longer required.
>> +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The
>> command is
>> +       used by the driver for setting RSS hash function, hash key and
>> +       indirection table in the device.
>> +\end{enumerate}
>> +
>> +If this feaure has been negotiated, the virtio header has an additional
>> +field{rss_hash_value} field attached to it.
>> +
>> +\devicenormative{\subparagraph}{RSS hash offload}{Device Types /
>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +The device MUST fill the virtio_net_ctrl_rss_hash structure with the hash
>> +functions it supports and return the structure to the driver. Zero or
>> more
>> +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>> \field{hash_function}
>> +field.
>> +
>> +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>> calculating and
>> +attaching hashes for the packets as well as stop setting the
>> VIRTIO_NET_HDR_GSO_RSS
>> +in the \field{gso_type} field, however the \field{hash_function} field
>> is kept
>> +as a part of the header.
>> +
>> +The device MUST drop all previous RSS configuration upon receiving
>> +VIRTIO_NET_CTRL_RSS_SET command.
>> +
>> +The device MUST set the RSS configuration according to the settings
>> provided as
>> +follows, once the configuration process is completed the device SHOULD
>> apply
>> +the hash function to each of the incoming packets and distribute the
>> packets
>> +through the virqueues using the calculated hash and the indirection table
>> +that were earlier provided by the driver.
>> +
>> +Setting RSS configuration
>> +\begin{enumerate}
>> +\item The driver fills all of the fields and passes them through the
>> control
>> +       queue to the device.
>> +
>> +\item The device sets the RSS configuration as provided by the driver.
>> +
>> +\item If the device successfully applied the configuration, on each
>> packet
>> +       recieved the device MUST calculate the hashing for the packet and
>> +       store it in the virtio-net header in rss_hash_value and the hash
>> +       fields used in the calculation in rss_hash_type.
>> +\end{enumerate}
>> +
>> +In case of any unexpected values/ unsupported hash function the driver
>> +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>> +
>> +\drivernormative{\subparagraph}{RSS hash offload}{Device Types /
>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver
>> SHOULD
>> +send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS
>> structure
>> +filled. The RSS structure fields should be filled as follows:
>> +
>> +\begin{itemize}
>> +\item The driver SHOULD choose the hash function that SHOULD be used and
>> fill
>> +       it in the \field{hash_function} field along with the approperiate
>> flags
>> +       in the \field{hash_function_flags} field. These flags inidicate
>> to the
>> +       device which packet fields MUST be used in the calculation
>> process of
>> +       the hash.
>> +\item Once the hash function has been chosen a suitable hash key should
>> be set
>> +       in the \field{hash_key} field, the length of the key should be
>> stored
>> +       in the \field{hash_key_length} field.
>> +\item Lastly the driver should fill the indirection table array in the
>> +       \field{indirection_table} field while setting the array length in
>> +       \field{indirection_table_length}. This structure is used by the
>> device
>> +       for determining in which RX virt queue the packet should be
>> placed.
>> +\end{itemize}
>> +Once the configuration phase is over successfully, the packets SHOULD
>> have the
>> +\field{rss_hash_value} field with the hash value that was calculated by
>> the
>> +device.
>> +
>> +Whenever the driver wants to discard the current RSS settings, it can
>> send an
>> +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>> +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>> +
>> +The driver MUST check the \field{ack} field value provided by the
>> device, in
>> +case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure
>> and
>> +retry another hash function or else give up.
>> +
>>  \subparagraph{Legacy Interface: Automatic receive steering in multiqueue
>> mode}\label{sec:Device Types / Network Device / Device Operation / Control
>> Virtqueue / Automatic receive steering in multiqueue mode / Legacy
>> Interface: Automatic receive steering in multiqueue mode}
>>  When using the legacy interface, transitional devices and drivers
>>  MUST format \field{virtqueue_pairs}
>> --
>> 2.13.6
>>
>>
>
>
> --
> Respectfully,
> *Sameeh Jubran*
> *Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
> *Software Engineer @ Daynix <http://www.daynix.com>.*
>



-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*

[-- Attachment #2: Type: text/html, Size: 13779 bytes --]

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Jason Wang]

On 2018年01月26日 00:27, Sameeh Jubran wrote:
> From: Sameeh Jubran <sameeh.j@gmail.com>
>
> Most modern high end network devices today support configurable hash functions,
> this commit introduces RSS - Receive Side Scaling - [1] to virtio net device.
>
> The RSS is a technology from Microsoft that boosts network device performance
> by efficiently distributing the traffic among the CPUs in a multiprocessor
> system.
>
> This feature is supported in most of the modern network cards as well as most
> modern OSes including Linux and Windows. It is worth mentioning that both DPDK
> and Hyper-v support RSS too.
>
> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2
>
> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
> ---
>   content.tex | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 133 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index c840588..5969b28 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3115,6 +3115,9 @@ features.
>   
>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>       channel.
> +
> +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
> +    functions.
>   \end{description}
>   
>   \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> @@ -3138,6 +3141,8 @@ Some networking feature bits require other networking feature bits
>   \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>   \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> +
> +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>   \end{description}

Is this a requirement of RSSv2? Looks like at least RSS can work without 
multiqueue in V1.

>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>           u8 flags;
>   #define VIRTIO_NET_HDR_GSO_NONE        0
>   #define VIRTIO_NET_HDR_GSO_TCPV4       1
> +#define VIRTIO_NET_HDR_GSO_RSS         2

What's the reason of introducing a new GSO type here? RSS should be 
independent for a specific offloading type I think.

>   #define VIRTIO_NET_HDR_GSO_UDP         3
>   #define VIRTIO_NET_HDR_GSO_TCPV6       4
>   #define VIRTIO_NET_HDR_GSO_ECN      0x80
> @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>           le16 csum_start;
>           le16 csum_offset;
>           le16 num_buffers;
> +// Only if RSS hash offload has been negotiated
> +        le64 rss_hash_value;

Not a native English speaker, but "rss_hash" should be sufficient.

>   };
>   \end{lstlisting}
>   
> @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>   The device MUST NOT queue packets on receive queues greater than
>   \field{virtqueue_pairs} once it has placed the VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>   
> +\paragraph{RSS hash offload}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
> +
> +\begin{lstlisting}
> +#define RSS_HASH_FUNCTION_NONE      1
> +#define RSS_HASH_FUNCTION_TOEPLITZ  2
> +#define RSS_HASH_FUNCTION_SYMMETRIC 3
> +
> +// Hash function fields
> +#define RSS_HASH_FIELDS_IPV4          0x00000100
> +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
> +#define RSS_HASH_FIELDS_IPV6          0x00000400
> +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
> +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
> +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
> +
> +struct virtio_net_ctrl_rss_hash{
> +le32 hash_function;
> +}
> +
> +struct virtio_net_rss {
> +le32 hash_function;
> +le32 hash_function_flags;
> +le32 hash_key_length;
> +le32 indirection_table_length;
> +	struct {
> +		le32 hash_key[hash_key_length];
> +		le32 indirection_table[indirection_table_length];
> +	}
> +};
> +
> +#define VIRTIO_NET_F_CTRL_RSS     24
> +
> +#define VIRTIO_NET_CTRL_RSS                           6
> +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
> +#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
> +#define VIRTIO_NET_CTRL_RSS_SET                       2
> +\end{lstlisting}
> +
> +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
> +commands for the RSS configuration.
> +
> +The class VIRTIO_NET_CTRL_RSS has three commands:
> +
> +\begin{enumerate}
> +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash functions
> +	supported by the device to the driver.

Can we just reuse virtio_net_rss in this case, it contains all the 
informations (e.g hash key or whatever others)?

> +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing is no
> +	longer required.

So this implies that if MQ is supported, we will go to use automatic 
steering mode? I was thinking maybe it's better to introduce a generic 
command to switch between steering mode.

> +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The command is
> +	used by the driver for setting RSS hash function, hash key and
> +	indirection table in the device.
> +\end{enumerate}
> +
> +If this feaure has been negotiated, the virtio header has an additional
> +field{rss_hash_value} field attached to it.
> +
> +\devicenormative{\subparagraph}{RSS hash offload}{Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
> +
> +The device MUST fill the virtio_net_ctrl_rss_hash structure with the hash
> +functions it supports and return the structure to the driver. Zero or more
> +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the \field{hash_function}
> +field.
> +
> +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop calculating and
> +attaching hashes

For simplicity, what if just compute the hash in this case? Consider a 
driver may want to use this hash (e.g Linux driver).

> for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
> +in the \field{gso_type} field, however the \field{hash_function} field is kept
> +as a part of the header.
> +
> +The device MUST drop all previous RSS configuration upon receiving
> +VIRTIO_NET_CTRL_RSS_SET command.
> +
> +The device MUST set the RSS configuration according to the settings provided as
> +follows, once the configuration process is completed the device SHOULD apply
> +the hash function to each of the incoming packets and distribute the packets
> +through the virqueues using the calculated hash and the indirection table
> +that were earlier provided by the driver.

We support changing #queues dynamically, so need to clarify the behavior 
when e.g the destination queues were disabled.

> +
> +Setting RSS configuration
> +\begin{enumerate}
> +\item The driver fills all of the fields and passes them through the control
> +	queue to the device.
> +
> +\item The device sets the RSS configuration as provided by the driver.
> +
> +\item If the device successfully applied the configuration, on each packet
> +	recieved the device MUST calculate the hashing for the packet and
> +	store it in the virtio-net header in rss_hash_value and the hash
> +	fields used in the calculation in rss_hash_type.
> +\end{enumerate}
> +
> +In case of any unexpected values/ unsupported hash function the driver
> +MUST return VIRTIO_NET_ERR in the \field{ack} field.
> +
> +\drivernormative{\subparagraph}{RSS hash offload}{Device Types / Network Device / Device Operation / Control Virtqueue / RSS hash offload}
> +
> +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver SHOULD
> +send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS structure
> +filled. The RSS structure fields should be filled as follows:
> +
> +\begin{itemize}
> +\item The driver SHOULD choose the hash function that SHOULD be used and fill
> +	it in the \field{hash_function} field along with the approperiate flags
> +	in the \field{hash_function_flags} field. These flags inidicate to the
> +	device which packet fields MUST be used in the calculation process of
> +	the hash.
> +\item Once the hash function has been chosen a suitable hash key should be set
> +	in the \field{hash_key} field, the length of the key should be stored
> +	in the \field{hash_key_length} field.
> +\item Lastly the driver should fill the indirection table array in the
> +	\field{indirection_table} field while setting the array length in
> +	\field{indirection_table_length}. This structure is used by the device
> +	for determining in which RX virt queue the packet should be placed.

Is the indirection table expected to be changed frequently? If yes, a 
single entry modification will cause a update of the whole table.

> +\end{itemize}
> +Once the configuration phase is over successfully, the packets SHOULD have the
> +\field{rss_hash_value} field with the hash value that was calculated by the
> +device.
> +
> +Whenever the driver wants to discard the current RSS settings, it can send an
> +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
> +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.

So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?

Thanks

> +
> +The driver MUST check the \field{ack} field value provided by the device, in
> +case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure and
> +retry another hash function or else give up.
> +
>   \subparagraph{Legacy Interface: Automatic receive steering in multiqueue mode}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Automatic receive steering in multiqueue mode / Legacy Interface: Automatic receive steering in multiqueue mode}
>   When using the legacy interface, transitional devices and drivers
>   MUST format \field{virtqueue_pairs}


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

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

[-- Attachment #1: Type: text/plain, Size: 11890 bytes --]

On Thu, Mar 15, 2018 at 4:19 AM, Jason Wang <jasowang@redhat.com> wrote:

>
>
> On 2018年01月26日 00:27, Sameeh Jubran wrote:
>
>> From: Sameeh Jubran <sameeh.j@gmail.com>
>>
>> Most modern high end network devices today support configurable hash
>> functions,
>> this commit introduces RSS - Receive Side Scaling - [1] to virtio net
>> device.
>>
>> The RSS is a technology from Microsoft that boosts network device
>> performance
>> by efficiently distributing the traffic among the CPUs in a multiprocessor
>> system.
>>
>> This feature is supported in most of the modern network cards as well as
>> most
>> modern OSes including Linux and Windows. It is worth mentioning that both
>> DPDK
>> and Hyper-v support RSS too.
>>
>> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ne
>> twork/ndis-receive-side-scaling2
>>
>> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
>> ---
>>   content.tex | 133 ++++++++++++++++++++++++++++++
>> ++++++++++++++++++++++++++++++
>>   1 file changed, 133 insertions(+)
>>
>> diff --git a/content.tex b/content.tex
>> index c840588..5969b28 100644
>> --- a/content.tex
>> +++ b/content.tex
>> @@ -3115,6 +3115,9 @@ features.
>>     \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>       channel.
>> +
>> +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
>> +    functions.
>>   \end{description}
>>     \subsubsection{Feature bit requirements}\label{sec:Device Types /
>> Network Device / Feature bits / Feature bit requirements}
>> @@ -3138,6 +3141,8 @@ Some networking feature bits require other
>> networking feature bits
>>   \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>>   \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>> +
>> +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>>   \end{description}
>>
>
> Is this a requirement of RSSv2? Looks like at least RSS can work without
> multiqueue in V1.

You are right this is not a requirement and should be dropped. More on RSS
with single hardware queue:
 https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue
<https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue>

>
>
>     \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
>> / Network Device / Feature bits / Legacy Interface: Feature bits}
>> @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>>           u8 flags;
>>   #define VIRTIO_NET_HDR_GSO_NONE        0
>>   #define VIRTIO_NET_HDR_GSO_TCPV4       1
>> +#define VIRTIO_NET_HDR_GSO_RSS         2
>>
>
> What's the reason of introducing a new GSO type here? RSS should be
> independent for a specific offloading type I think.

This was the straight forward option as there is no need to extend the
virtio header but it can be moved to a specific offload.

>
>
>   #define VIRTIO_NET_HDR_GSO_UDP         3
>>   #define VIRTIO_NET_HDR_GSO_TCPV6       4
>>   #define VIRTIO_NET_HDR_GSO_ECN      0x80
>> @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>>           le16 csum_start;
>>           le16 csum_offset;
>>           le16 num_buffers;
>> +// Only if RSS hash offload has been negotiated
>> +        le64 rss_hash_value;
>>
>
> Not a native English speaker, but "rss_hash" should be sufficient.

will do

>
>
>   };
>>   \end{lstlisting}
>>   @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>>   The device MUST NOT queue packets on receive queues greater than
>>   \field{virtqueue_pairs} once it has placed the
>> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>>   +\paragraph{RSS hash offload}\label{sec:Device Types / Network Device /
>> Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +\begin{lstlisting}
>> +#define RSS_HASH_FUNCTION_NONE      1
>> +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>> +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>> +
>> +// Hash function fields
>> +#define RSS_HASH_FIELDS_IPV4          0x00000100
>> +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>> +#define RSS_HASH_FIELDS_IPV6          0x00000400
>> +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>> +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>> +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>> +
>> +struct virtio_net_ctrl_rss_hash{
>> +le32 hash_function;
>> +}
>> +
>> +struct virtio_net_rss {
>> +le32 hash_function;
>> +le32 hash_function_flags;
>> +le32 hash_key_length;
>> +le32 indirection_table_length;
>> +       struct {
>> +               le32 hash_key[hash_key_length];
>> +               le32 indirection_table[indirection_table_length];
>> +       }
>> +};
>> +
>> +#define VIRTIO_NET_F_CTRL_RSS     24
>> +
>> +#define VIRTIO_NET_CTRL_RSS                           6
>> +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
>> +#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
>> +#define VIRTIO_NET_CTRL_RSS_SET                       2
>> +\end{lstlisting}
>> +
>> +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
>> +commands for the RSS configuration.
>> +
>> +The class VIRTIO_NET_CTRL_RSS has three commands:
>> +
>> +\begin{enumerate}
>> +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash
>> functions
>> +       supported by the device to the driver.
>>
>
> Can we just reuse virtio_net_rss in this case, it contains all the
> informations (e.g hash key or whatever others)?

Sure this can work

>
>
> +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing
>> is no
>> +       longer required.
>>
>
> So this implies that if MQ is supported, we will go to use automatic
> steering mode? I was thinking maybe it's better to introduce a generic
> command to switch between steering mode.

 So you are suggesting that we should have an additional generic command
that changes between steering modes and RSS is one of those modes?

>
>
> +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The
>> command is
>> +       used by the driver for setting RSS hash function, hash key and
>> +       indirection table in the device.
>> +\end{enumerate}
>> +
>> +If this feaure has been negotiated, the virtio header has an additional
>> +field{rss_hash_value} field attached to it.
>> +
>> +\devicenormative{\subparagraph}{RSS hash offload}{Device Types /
>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +The device MUST fill the virtio_net_ctrl_rss_hash structure with the hash
>> +functions it supports and return the structure to the driver. Zero or
>> more
>> +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>> \field{hash_function}
>> +field.
>> +
>> +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>> calculating and
>> +attaching hashes
>>
>
> For simplicity, what if just compute the hash in this case? Consider a
> driver may want to use this hash (e.g Linux driver).

I get the use case but maybe introduce a new command for computing hashes
only? I think the disable should be preserved for disabling only.

>
>
> for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
>> +in the \field{gso_type} field, however the \field{hash_function} field
>> is kept
>> +as a part of the header.
>> +
>> +The device MUST drop all previous RSS configuration upon receiving
>> +VIRTIO_NET_CTRL_RSS_SET command.
>> +
>> +The device MUST set the RSS configuration according to the settings
>> provided as
>> +follows, once the configuration process is completed the device SHOULD
>> apply
>> +the hash function to each of the incoming packets and distribute the
>> packets
>> +through the virqueues using the calculated hash and the indirection table
>> +that were earlier provided by the driver.
>>
>
> We support changing #queues dynamically, so need to clarify the behavior
> when e.g the destination queues were disabled.

I will go into these details in the next version.

>
>
> +
>> +Setting RSS configuration
>> +\begin{enumerate}
>> +\item The driver fills all of the fields and passes them through the
>> control
>> +       queue to the device.
>> +
>> +\item The device sets the RSS configuration as provided by the driver.
>> +
>> +\item If the device successfully applied the configuration, on each
>> packet
>> +       recieved the device MUST calculate the hashing for the packet and
>> +       store it in the virtio-net header in rss_hash_value and the hash
>> +       fields used in the calculation in rss_hash_type.
>> +\end{enumerate}
>> +
>> +In case of any unexpected values/ unsupported hash function the driver
>> +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>> +
>> +\drivernormative{\subparagraph}{RSS hash offload}{Device Types /
>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>> +
>> +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver
>> SHOULD
>> +send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS
>> structure
>> +filled. The RSS structure fields should be filled as follows:
>> +
>> +\begin{itemize}
>> +\item The driver SHOULD choose the hash function that SHOULD be used and
>> fill
>> +       it in the \field{hash_function} field along with the approperiate
>> flags
>> +       in the \field{hash_function_flags} field. These flags inidicate
>> to the
>> +       device which packet fields MUST be used in the calculation
>> process of
>> +       the hash.
>> +\item Once the hash function has been chosen a suitable hash key should
>> be set
>> +       in the \field{hash_key} field, the length of the key should be
>> stored
>> +       in the \field{hash_key_length} field.
>> +\item Lastly the driver should fill the indirection table array in the
>> +       \field{indirection_table} field while setting the array length in
>> +       \field{indirection_table_length}. This structure is used by the
>> device
>> +       for determining in which RX virt queue the packet should be
>> placed.
>>
>
> Is the indirection table expected to be changed frequently? If yes, a
> single entry modification will cause a update of the whole table.

No, it should not be changing frequently

>
>
> +\end{itemize}
>> +Once the configuration phase is over successfully, the packets SHOULD
>> have the
>> +\field{rss_hash_value} field with the hash value that was calculated by
>> the
>> +device.
>> +
>> +Whenever the driver wants to discard the current RSS settings, it can
>> send an
>> +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>> +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>>
>
> So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?
>
Yes, do you think we should discard one of the two?

>
> Thanks
>
>
> +
>> +The driver MUST check the \field{ack} field value provided by the
>> device, in
>> +case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure
>> and
>> +retry another hash function or else give up.
>> +
>>   \subparagraph{Legacy Interface: Automatic receive steering in
>> multiqueue mode}\label{sec:Device Types / Network Device / Device Operation
>> / Control Virtqueue / Automatic receive steering in multiqueue mode /
>> Legacy Interface: Automatic receive steering in multiqueue mode}
>>   When using the legacy interface, transitional devices and drivers
>>   MUST format \field{virtqueue_pairs}
>>
>
>


-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*

[-- Attachment #2: Type: text/html, Size: 16987 bytes --]

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Vijayabhaskar Balakrishna]

[-- Attachment #1: Type: text/plain, Size: 14579 bytes --]

On 3/19/2018 6:33 AM, Sameeh Jubran wrote:
>
>
> On Thu, Mar 15, 2018 at 4:19 AM, Jason Wang <jasowang@redhat.com 
> <mailto:jasowang@redhat.com>> wrote:
>
>
>
>     On 2018年01月26日 00:27, Sameeh Jubran wrote:
>
>         From: Sameeh Jubran <sameeh.j@gmail.com
>         <mailto:sameeh.j@gmail.com>>
>
>         Most modern high end network devices today support
>         configurable hash functions,
>         this commit introduces RSS - Receive Side Scaling - [1] to
>         virtio net device.
>
>         The RSS is a technology from Microsoft that boosts network
>         device performance
>         by efficiently distributing the traffic among the CPUs in a
>         multiprocessor
>         system.
>
>         This feature is supported in most of the modern network cards
>         as well as most
>         modern OSes including Linux and Windows. It is worth
>         mentioning that both DPDK
>         and Hyper-v support RSS too.
>
>         [1]
>         https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2
>         <https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2>
>
>         Signed-off-by: Sameeh Jubran <sjubran@redhat.com
>         <mailto:sjubran@redhat.com>>
>         ---
>           content.tex | 133
>         ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>           1 file changed, 133 insertions(+)
>
>         diff --git a/content.tex b/content.tex
>         index c840588..5969b28 100644
>         --- a/content.tex
>         +++ b/content.tex
>         @@ -3115,6 +3115,9 @@ features.
>             \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address
>         through control
>               channel.
>         +
>         +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable
>         RSS hash
>         +    functions.
>           \end{description}
>             \subsubsection{Feature bit requirements}\label{sec:Device
>         Types / Network Device / Feature bits / Feature bit requirements}
>         @@ -3138,6 +3141,8 @@ Some networking feature bits require
>         other networking feature bits
>           \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires
>         VIRTIO_NET_F_CTRL_VQ.
>           \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>           \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>         +
>         +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>           \end{description}
>
>
>     Is this a requirement of RSSv2? Looks like at least RSS can work
>     without multiqueue in V1.
>
> You are right this is not a requirement and should be dropped. More on 
> RSS with single hardware queue:
>  https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue 
> <https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue>
Do we still expect efficiency (distribution of traffic among CPUs) in 
case of single queue by having RSS (in host software)?
>
>
>
>             \subsubsection{Legacy Interface: Feature
>         bits}\label{sec:Device Types / Network Device / Feature bits /
>         Legacy Interface: Feature bits}
>         @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>                   u8 flags;
>           #define VIRTIO_NET_HDR_GSO_NONE        0
>           #define VIRTIO_NET_HDR_GSO_TCPV4       1
>         +#define VIRTIO_NET_HDR_GSO_RSS         2
>
>
>     What's the reason of introducing a new GSO type here? RSS should
>     be independent for a specific offloading type I think.
>
> This was the straight forward option as there is no need to extend the 
> virtio header but it can be moved to a specific offload.
>
>
>
>           #define VIRTIO_NET_HDR_GSO_UDP         3
>           #define VIRTIO_NET_HDR_GSO_TCPV6       4
>           #define VIRTIO_NET_HDR_GSO_ECN      0x80
>         @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>                   le16 csum_start;
>                   le16 csum_offset;
>                   le16 num_buffers;
>         +// Only if RSS hash offload has been negotiated
>         +        le64 rss_hash_value;
>
I asked Sameeh privately and asking here again.   Is compatibility 
maintained if we introduce a new field to virtio_net_hdr?
>
>
>     Not a native English speaker, but "rss_hash" should be sufficient.
>
> will do
>
>
>
>           };
>           \end{lstlisting}
>           @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>           The device MUST NOT queue packets on receive queues greater than
>           \field{virtqueue_pairs} once it has placed the
>         VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>           +\paragraph{RSS hash offload}\label{sec:Device Types /
>         Network Device / Device Operation / Control Virtqueue / RSS
>         hash offload}
>         +
>         +\begin{lstlisting}
>         +#define RSS_HASH_FUNCTION_NONE      1
>         +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>         +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>
Should above be bit field?  I mean first two are fine, should 
..SYMMETRIC should be 4 (0x4)    Otherwise description "Zero or more 
flags of the RSS_HASH_FUNCTION flags MUST be used to fill the field 
hash_function" below wouldn't be correct.

>         +
>         +// Hash function fields
>         +#define RSS_HASH_FIELDS_IPV4          0x00000100
>         +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>         +#define RSS_HASH_FIELDS_IPV6          0x00000400
>         +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>         +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>         +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>         +
>         +struct virtio_net_ctrl_rss_hash{
>         +le32 hash_function;
>         +}
>         +
>         +struct virtio_net_rss {
>         +le32 hash_function;
>         +le32 hash_function_flags;
>         +le32 hash_key_length;
>         +le32 indirection_table_length;
>         +       struct {
>         +               le32 hash_key[hash_key_length];
>         +               le32 indirection_table[indirection_table_length];
>         +       }
>         +};
>
I'm new here, hence my question: Is array size typically described like 
above in virtio spec?  And with #define constant in implementation?

Thanks,
Vijay
>
>         +
>         +#define VIRTIO_NET_F_CTRL_RSS     24
>         +
>         +#define VIRTIO_NET_CTRL_RSS    6
>         +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS  0
>         +#define VIRTIO_NET_CTRL_RSS_DISABLE    1
>         +#define VIRTIO_NET_CTRL_RSS_SET    2
>         +\end{lstlisting}
>         +
>         +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can
>         send control
>         +commands for the RSS configuration.
>         +
>         +The class VIRTIO_NET_CTRL_RSS has three commands:
>         +
>         +\begin{enumerate}
>         +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the
>         hash functions
>         +       supported by the device to the driver.
>
>
>     Can we just reuse virtio_net_rss in this case, it contains all the
>     informations (e.g hash key or whatever others)?
>
> Sure this can work
>
>
>
>         +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that
>         RSS hashing is no
>         +       longer required.
>
>
>     So this implies that if MQ is supported, we will go to use
>     automatic steering mode? I was thinking maybe it's better to
>     introduce a generic command to switch between steering mode.
>
>  So you are suggesting that we should have an additional generic 
> command that changes between steering modes and RSS is one of those modes?
>
>
>
>         +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS
>         configuration. The command is
>         +       used by the driver for setting RSS hash function, hash
>         key and
>         +       indirection table in the device.
>         +\end{enumerate}
>         +
>         +If this feaure has been negotiated, the virtio header has an
>         additional
>         +field{rss_hash_value} field attached to it.
>         +
>         +\devicenormative{\subparagraph}{RSS hash offload}{Device
>         Types / Network Device / Device Operation / Control Virtqueue
>         / RSS hash offload}
>         +
>         +The device MUST fill the virtio_net_ctrl_rss_hash structure
>         with the hash
>         +functions it supports and return the structure to the driver.
>         Zero or more
>         +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>         \field{hash_function}
>         +field.
>         +
>         +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>         calculating and
>         +attaching hashes
>
>
>     For simplicity, what if just compute the hash in this case?
>     Consider a driver may want to use this hash (e.g Linux driver).
>
> I get the use case but maybe introduce a new command for computing 
> hashes only? I think the disable should be preserved for disabling only.
>
>
>
>         for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
>         +in the \field{gso_type} field, however the
>         \field{hash_function} field is kept
>         +as a part of the header.
>         +
>         +The device MUST drop all previous RSS configuration upon
>         receiving
>         +VIRTIO_NET_CTRL_RSS_SET command.
>         +
>         +The device MUST set the RSS configuration according to the
>         settings provided as
>         +follows, once the configuration process is completed the
>         device SHOULD apply
>         +the hash function to each of the incoming packets and
>         distribute the packets
>         +through the virqueues using the calculated hash and the
>         indirection table
>         +that were earlier provided by the driver.
>
>
>     We support changing #queues dynamically, so need to clarify the
>     behavior when e.g the destination queues were disabled.
>
> I will go into these details in the next version.
>
>
>
>         +
>         +Setting RSS configuration
>         +\begin{enumerate}
>         +\item The driver fills all of the fields and passes them
>         through the control
>         +       queue to the device.
>         +
>         +\item The device sets the RSS configuration as provided by
>         the driver.
>         +
>         +\item If the device successfully applied the configuration,
>         on each packet
>         +       recieved the device MUST calculate the hashing for the
>         packet and
>         +       store it in the virtio-net header in rss_hash_value
>         and the hash
>         +       fields used in the calculation in rss_hash_type.
>         +\end{enumerate}
>         +
>         +In case of any unexpected values/ unsupported hash function
>         the driver
>         +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>         +
>         +\drivernormative{\subparagraph}{RSS hash offload}{Device
>         Types / Network Device / Device Operation / Control Virtqueue
>         / RSS hash offload}
>         +
>         +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS
>         the driver SHOULD
>         +send VIRTIO_NET_CTRL_RSS_SET command to the device along with
>         the RSS structure
>         +filled. The RSS structure fields should be filled as follows:
>         +
>         +\begin{itemize}
>         +\item The driver SHOULD choose the hash function that SHOULD
>         be used and fill
>         +       it in the \field{hash_function} field along with the
>         approperiate flags
>         +       in the \field{hash_function_flags} field. These flags
>         inidicate to the
>         +       device which packet fields MUST be used in the
>         calculation process of
>         +       the hash.
>         +\item Once the hash function has been chosen a suitable hash
>         key should be set
>         +       in the \field{hash_key} field, the length of the key
>         should be stored
>         +       in the \field{hash_key_length} field.
>         +\item Lastly the driver should fill the indirection table
>         array in the
>         +       \field{indirection_table} field while setting the
>         array length in
>         +       \field{indirection_table_length}. This structure is
>         used by the device
>         +       for determining in which RX virt queue the packet
>         should be placed.
>
>
>     Is the indirection table expected to be changed frequently? If
>     yes, a single entry modification will cause a update of the whole
>     table.
>
> No, it should not be changing frequently
>
>
>
>         +\end{itemize}
>         +Once the configuration phase is over successfully, the
>         packets SHOULD have the
>         +\field{rss_hash_value} field with the hash value that was
>         calculated by the
>         +device.
>         +
>         +Whenever the driver wants to discard the current RSS
>         settings, it can send an
>         +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>         +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>
>
>     So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?
>
> Yes, do you think we should discard one of the two?
>
>
>     Thanks
>
>
>         +
>         +The driver MUST check the \field{ack} field value provided by
>         the device, in
>         +case the value is not VIRTIO_NET_OK then the driver MUST
>         hanlde faliure and
>         +retry another hash function or else give up.
>         +
>           \subparagraph{Legacy Interface: Automatic receive steering
>         in multiqueue mode}\label{sec:Device Types / Network Device /
>         Device Operation / Control Virtqueue / Automatic receive
>         steering in multiqueue mode / Legacy Interface: Automatic
>         receive steering in multiqueue mode}
>           When using the legacy interface, transitional devices and
>         drivers
>           MUST format \field{virtqueue_pairs}
>
>
>
>
>
> -- 
> Respectfully,
> */Sameeh Jubran/*
> /Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>/
> /Software Engineer @ Daynix <http://www.daynix.com>./


[-- Attachment #2: Type: text/html, Size: 29641 bytes --]

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Jason Wang]

On 2018年03月19日 21:33, Sameeh Jubran wrote:
>
>
> On Thu, Mar 15, 2018 at 4:19 AM, Jason Wang <jasowang@redhat.com 
> <mailto:jasowang@redhat.com>> wrote:
>
>
>
>     On 2018年01月26日 00:27, Sameeh Jubran wrote:
>
>         From: Sameeh Jubran <sameeh.j@gmail.com
>         <mailto:sameeh.j@gmail.com>>
>
>         Most modern high end network devices today support
>         configurable hash functions,
>         this commit introduces RSS - Receive Side Scaling - [1] to
>         virtio net device.
>
>         The RSS is a technology from Microsoft that boosts network
>         device performance
>         by efficiently distributing the traffic among the CPUs in a
>         multiprocessor
>         system.
>
>         This feature is supported in most of the modern network cards
>         as well as most
>         modern OSes including Linux and Windows. It is worth
>         mentioning that both DPDK
>         and Hyper-v support RSS too.
>
>         [1]
>         https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2
>         <https://docs.microsoft.com/en-us/windows-hardware/drivers/network/ndis-receive-side-scaling2>
>
>         Signed-off-by: Sameeh Jubran <sjubran@redhat.com
>         <mailto:sjubran@redhat.com>>
>         ---
>           content.tex | 133
>         ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>           1 file changed, 133 insertions(+)
>
>         diff --git a/content.tex b/content.tex
>         index c840588..5969b28 100644
>         --- a/content.tex
>         +++ b/content.tex
>         @@ -3115,6 +3115,9 @@ features.
>             \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address
>         through control
>               channel.
>         +
>         +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable
>         RSS hash
>         +    functions.
>           \end{description}
>             \subsubsection{Feature bit requirements}\label{sec:Device
>         Types / Network Device / Feature bits / Feature bit requirements}
>         @@ -3138,6 +3141,8 @@ Some networking feature bits require
>         other networking feature bits
>           \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires
>         VIRTIO_NET_F_CTRL_VQ.
>           \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>           \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>         +
>         +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>           \end{description}
>
>
>     Is this a requirement of RSSv2? Looks like at least RSS can work
>     without multiqueue in V1.
>
> You are right this is not a requirement and should be dropped. More on 
> RSS with single hardware queue:
>  https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue 
> <https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue>
>
>
>
>             \subsubsection{Legacy Interface: Feature
>         bits}\label{sec:Device Types / Network Device / Feature bits /
>         Legacy Interface: Feature bits}
>         @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>                   u8 flags;
>           #define VIRTIO_NET_HDR_GSO_NONE        0
>           #define VIRTIO_NET_HDR_GSO_TCPV4       1
>         +#define VIRTIO_NET_HDR_GSO_RSS         2
>
>
>     What's the reason of introducing a new GSO type here? RSS should
>     be independent for a specific offloading type I think.
>
> This was the straight forward option as there is no need to extend the 
> virtio header but it can be moved to a specific offload.

If I understand correctly, RSS is not tied to GSO, so you mean 
VIRTIO_NET_HDR_RSS here?

>
>
>           #define VIRTIO_NET_HDR_GSO_UDP         3
>           #define VIRTIO_NET_HDR_GSO_TCPV6       4
>           #define VIRTIO_NET_HDR_GSO_ECN      0x80
>         @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>                   le16 csum_start;
>                   le16 csum_offset;
>                   le16 num_buffers;
>         +// Only if RSS hash offload has been negotiated
>         +        le64 rss_hash_value;
>
>
>     Not a native English speaker, but "rss_hash" should be sufficient.
>
> will do
>
>
>
>           };
>           \end{lstlisting}
>           @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>           The device MUST NOT queue packets on receive queues greater than
>           \field{virtqueue_pairs} once it has placed the
>         VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>           +\paragraph{RSS hash offload}\label{sec:Device Types /
>         Network Device / Device Operation / Control Virtqueue / RSS
>         hash offload}
>         +
>         +\begin{lstlisting}
>         +#define RSS_HASH_FUNCTION_NONE      1
>         +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>         +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>         +
>         +// Hash function fields
>         +#define RSS_HASH_FIELDS_IPV4          0x00000100
>         +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>         +#define RSS_HASH_FIELDS_IPV6          0x00000400
>         +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>         +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>         +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>         +
>         +struct virtio_net_ctrl_rss_hash{
>         +le32 hash_function;
>         +}
>         +
>         +struct virtio_net_rss {
>         +le32 hash_function;
>         +le32 hash_function_flags;
>         +le32 hash_key_length;
>         +le32 indirection_table_length;
>         +       struct {
>         +               le32 hash_key[hash_key_length];
>         +               le32 indirection_table[indirection_table_length];
>         +       }
>         +};
>         +
>         +#define VIRTIO_NET_F_CTRL_RSS     24
>         +
>         +#define VIRTIO_NET_CTRL_RSS    6
>         +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS  0
>         +#define VIRTIO_NET_CTRL_RSS_DISABLE    1
>         +#define VIRTIO_NET_CTRL_RSS_SET    2
>         +\end{lstlisting}
>         +
>         +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can
>         send control
>         +commands for the RSS configuration.
>         +
>         +The class VIRTIO_NET_CTRL_RSS has three commands:
>         +
>         +\begin{enumerate}
>         +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the
>         hash functions
>         +       supported by the device to the driver.
>
>
>     Can we just reuse virtio_net_rss in this case, it contains all the
>     informations (e.g hash key or whatever others)?
>
> Sure this can work
>
>
>
>         +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that
>         RSS hashing is no
>         +       longer required.
>
>
>     So this implies that if MQ is supported, we will go to use
>     automatic steering mode? I was thinking maybe it's better to
>     introduce a generic command to switch between steering mode.
>
>  So you are suggesting that we should have an additional generic 
> command that changes between steering modes and RSS is one of those modes?

Yes.

>
>
>         +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS
>         configuration. The command is
>         +       used by the driver for setting RSS hash function, hash
>         key and
>         +       indirection table in the device.
>         +\end{enumerate}
>         +
>         +If this feaure has been negotiated, the virtio header has an
>         additional
>         +field{rss_hash_value} field attached to it.
>         +
>         +\devicenormative{\subparagraph}{RSS hash offload}{Device
>         Types / Network Device / Device Operation / Control Virtqueue
>         / RSS hash offload}
>         +
>         +The device MUST fill the virtio_net_ctrl_rss_hash structure
>         with the hash
>         +functions it supports and return the structure to the driver.
>         Zero or more
>         +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>         \field{hash_function}
>         +field.
>         +
>         +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>         calculating and
>         +attaching hashes
>
>
>     For simplicity, what if just compute the hash in this case?
>     Consider a driver may want to use this hash (e.g Linux driver).
>
> I get the use case but maybe introduce a new command for computing 
> hashes only?

Then it looks unnecessary. (Btw, the above should be 
"VIRTIO_NET_CTRL_RSS_DISABLE").

> I think the disable should be preserved for disabling only.
>
>
>
>         for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
>         +in the \field{gso_type} field, however the
>         \field{hash_function} field is kept
>         +as a part of the header.
>         +
>         +The device MUST drop all previous RSS configuration upon
>         receiving
>         +VIRTIO_NET_CTRL_RSS_SET command.
>         +
>         +The device MUST set the RSS configuration according to the
>         settings provided as
>         +follows, once the configuration process is completed the
>         device SHOULD apply
>         +the hash function to each of the incoming packets and
>         distribute the packets
>         +through the virqueues using the calculated hash and the
>         indirection table
>         +that were earlier provided by the driver.
>
>
>     We support changing #queues dynamically, so need to clarify the
>     behavior when e.g the destination queues were disabled.
>
> I will go into these details in the next version.
>
>
>
>         +
>         +Setting RSS configuration
>         +\begin{enumerate}
>         +\item The driver fills all of the fields and passes them
>         through the control
>         +       queue to the device.
>         +
>         +\item The device sets the RSS configuration as provided by
>         the driver.
>         +
>         +\item If the device successfully applied the configuration,
>         on each packet
>         +       recieved the device MUST calculate the hashing for the
>         packet and
>         +       store it in the virtio-net header in rss_hash_value
>         and the hash
>         +       fields used in the calculation in rss_hash_type.
>         +\end{enumerate}
>         +
>         +In case of any unexpected values/ unsupported hash function
>         the driver
>         +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>         +
>         +\drivernormative{\subparagraph}{RSS hash offload}{Device
>         Types / Network Device / Device Operation / Control Virtqueue
>         / RSS hash offload}
>         +
>         +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS
>         the driver SHOULD
>         +send VIRTIO_NET_CTRL_RSS_SET command to the device along with
>         the RSS structure
>         +filled. The RSS structure fields should be filled as follows:
>         +
>         +\begin{itemize}
>         +\item The driver SHOULD choose the hash function that SHOULD
>         be used and fill
>         +       it in the \field{hash_function} field along with the
>         approperiate flags
>         +       in the \field{hash_function_flags} field. These flags
>         inidicate to the
>         +       device which packet fields MUST be used in the
>         calculation process of
>         +       the hash.
>         +\item Once the hash function has been chosen a suitable hash
>         key should be set
>         +       in the \field{hash_key} field, the length of the key
>         should be stored
>         +       in the \field{hash_key_length} field.
>         +\item Lastly the driver should fill the indirection table
>         array in the
>         +       \field{indirection_table} field while setting the
>         array length in
>         +       \field{indirection_table_length}. This structure is
>         used by the device
>         +       for determining in which RX virt queue the packet
>         should be placed.
>
>
>     Is the indirection table expected to be changed frequently? If
>     yes, a single entry modification will cause a update of the whole
>     table.
>
> No, it should not be changing frequently
>
>
>
>         +\end{itemize}
>         +Once the configuration phase is over successfully, the
>         packets SHOULD have the
>         +\field{rss_hash_value} field with the hash value that was
>         calculated by the
>         +device.
>         +
>         +Whenever the driver wants to discard the current RSS
>         settings, it can send an
>         +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>         +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>
>
>     So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?
>
> Yes, do you think we should discard one of the two?

Yes.

Thanks

>
>     Thanks
>
>
>         +
>         +The driver MUST check the \field{ack} field value provided by
>         the device, in
>         +case the value is not VIRTIO_NET_OK then the driver MUST
>         hanlde faliure and
>         +retry another hash function or else give up.
>         +
>           \subparagraph{Legacy Interface: Automatic receive steering
>         in multiqueue mode}\label{sec:Device Types / Network Device /
>         Device Operation / Control Virtqueue / Automatic receive
>         steering in multiqueue mode / Legacy Interface: Automatic
>         receive steering in multiqueue mode}
>           When using the legacy interface, transitional devices and
>         drivers
>           MUST format \field{virtqueue_pairs}
>
>
>
>
>
> -- 
> Respectfully,
> */Sameeh Jubran/*
> /Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>/
> /Software Engineer @ Daynix <http://www.daynix.com>./


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

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

[-- Attachment #1: Type: text/plain, Size: 13992 bytes --]

On Tue, Mar 20, 2018 at 12:29 AM, Vijayabhaskar Balakrishna <
vijay.balakrishna@oracle.com> wrote:

> On 3/19/2018 6:33 AM, Sameeh Jubran wrote:
>
>
>
> On Thu, Mar 15, 2018 at 4:19 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>>
>>
>> On 2018年01月26日 00:27, Sameeh Jubran wrote:
>>
>>> From: Sameeh Jubran <sameeh.j@gmail.com>
>>>
>>> Most modern high end network devices today support configurable hash
>>> functions,
>>> this commit introduces RSS - Receive Side Scaling - [1] to virtio net
>>> device.
>>>
>>> The RSS is a technology from Microsoft that boosts network device
>>> performance
>>> by efficiently distributing the traffic among the CPUs in a
>>> multiprocessor
>>> system.
>>>
>>> This feature is supported in most of the modern network cards as well as
>>> most
>>> modern OSes including Linux and Windows. It is worth mentioning that
>>> both DPDK
>>> and Hyper-v support RSS too.
>>>
>>> [1] https://docs.microsoft.com/en-us/windows-hardware/drivers/ne
>>> twork/ndis-receive-side-scaling2
>>>
>>> Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
>>> ---
>>>   content.tex | 133 ++++++++++++++++++++++++++++++
>>> ++++++++++++++++++++++++++++++
>>>   1 file changed, 133 insertions(+)
>>>
>>> diff --git a/content.tex b/content.tex
>>> index c840588..5969b28 100644
>>> --- a/content.tex
>>> +++ b/content.tex
>>> @@ -3115,6 +3115,9 @@ features.
>>>     \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through
>>> control
>>>       channel.
>>> +
>>> +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable RSS hash
>>> +    functions.
>>>   \end{description}
>>>     \subsubsection{Feature bit requirements}\label{sec:Device Types /
>>> Network Device / Feature bits / Feature bit requirements}
>>> @@ -3138,6 +3141,8 @@ Some networking feature bits require other
>>> networking feature bits
>>>   \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires VIRTIO_NET_F_CTRL_VQ.
>>>   \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>>>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
>>> +
>>> +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>>>   \end{description}
>>>
>>
>> Is this a requirement of RSSv2? Looks like at least RSS can work without
>> multiqueue in V1.
>
> You are right this is not a requirement and should be dropped. More on RSS
> with single hardware queue:
>  https://docs.microsoft.com/en-us/windows-hardware/drivers/
> network/rss-with-a-single-hardware-receive-queue
> <https://docs.microsoft.com/en-us/windows-hardware/drivers/network/rss-with-a-single-hardware-receive-queue>
>
> Do we still expect efficiency (distribution of traffic among CPUs) in case
> of single queue by having RSS (in host software)?
>
 This depends on the configuration settings for the hash function and
indirection table.

>
>
>>
>>     \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types
>>> / Network Device / Feature bits / Legacy Interface: Feature bits}
>>> @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>>>           u8 flags;
>>>   #define VIRTIO_NET_HDR_GSO_NONE        0
>>>   #define VIRTIO_NET_HDR_GSO_TCPV4       1
>>> +#define VIRTIO_NET_HDR_GSO_RSS         2
>>>
>>
>> What's the reason of introducing a new GSO type here? RSS should be
>> independent for a specific offloading type I think.
>
> This was the straight forward option as there is no need to extend the
> virtio header but it can be moved to a specific offload.
>
>>
>>
>>   #define VIRTIO_NET_HDR_GSO_UDP         3
>>>   #define VIRTIO_NET_HDR_GSO_TCPV6       4
>>>   #define VIRTIO_NET_HDR_GSO_ECN      0x80
>>> @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>>>           le16 csum_start;
>>>           le16 csum_offset;
>>>           le16 num_buffers;
>>> +// Only if RSS hash offload has been negotiated
>>> +        le64 rss_hash_value;
>>>
>> I asked Sameeh privately and asking here again.   Is compatibility
> maintained if we introduce a new field to virtio_net_hdr?
>
I believe so, only if the RSS feature has been acked then this header is
used which should preserve compatibility

>
>> Not a native English speaker, but "rss_hash" should be sufficient.
>
> will do
>
>>
>>
>>   };
>>>   \end{lstlisting}
>>>   @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command.
>>>   The device MUST NOT queue packets on receive queues greater than
>>>   \field{virtqueue_pairs} once it has placed the
>>> VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>>>   +\paragraph{RSS hash offload}\label{sec:Device Types / Network Device
>>> / Device Operation / Control Virtqueue / RSS hash offload}
>>> +
>>> +\begin{lstlisting}
>>> +#define RSS_HASH_FUNCTION_NONE      1
>>> +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>>> +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>>>
>> Should above be bit field?  I mean first two are fine, should ..SYMMETRIC
> should be 4 (0x4)    Otherwise description "Zero or more flags of the
> RSS_HASH_FUNCTION flags MUST be used to fill the field hash_function" below
> wouldn't be correct.
>
Each function is a distinct function, there is no sense for setting two or
more functions in the field. Indeed the description is not accurate and
I'll be addressing this in v2, thanks for catching this :)

>
>
> +
>>> +// Hash function fields
>>> +#define RSS_HASH_FIELDS_IPV4          0x00000100
>>> +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>>> +#define RSS_HASH_FIELDS_IPV6          0x00000400
>>> +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>>> +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>>> +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>>> +
>>> +struct virtio_net_ctrl_rss_hash{
>>> +le32 hash_function;
>>> +}
>>> +
>>> +struct virtio_net_rss {
>>> +le32 hash_function;
>>> +le32 hash_function_flags;
>>> +le32 hash_key_length;
>>> +le32 indirection_table_length;
>>> +       struct {
>>> +               le32 hash_key[hash_key_length];
>>> +               le32 indirection_table[indirection_table_length];
>>> +       }
>>> +};
>>>
>> I'm new here, hence my question: Is array size typically described like
> above in virtio spec?  And with #define constant in implementation?
>
Yes, take a look here for example:
https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex#L4778
<https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex#L4778>
https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex#L2716

>
> Thanks,
> Vijay
>
> +
>>> +#define VIRTIO_NET_F_CTRL_RSS     24
>>> +
>>> +#define VIRTIO_NET_CTRL_RSS                           6
>>> +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS   0
>>> +#define VIRTIO_NET_CTRL_RSS_DISABLE                   1
>>> +#define VIRTIO_NET_CTRL_RSS_SET                       2
>>> +\end{lstlisting}
>>> +
>>> +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can send control
>>> +commands for the RSS configuration.
>>> +
>>> +The class VIRTIO_NET_CTRL_RSS has three commands:
>>> +
>>> +\begin{enumerate}
>>> +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the hash
>>> functions
>>> +       supported by the device to the driver.
>>>
>>
>> Can we just reuse virtio_net_rss in this case, it contains all the
>> informations (e.g hash key or whatever others)?
>
> Sure this can work
>
>>
>>
>> +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that RSS hashing
>>> is no
>>> +       longer required.
>>>
>>
>> So this implies that if MQ is supported, we will go to use automatic
>> steering mode? I was thinking maybe it's better to introduce a generic
>> command to switch between steering mode.
>
>  So you are suggesting that we should have an additional generic command
> that changes between steering modes and RSS is one of those modes?
>
>>
>>
>> +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS configuration. The
>>> command is
>>> +       used by the driver for setting RSS hash function, hash key and
>>> +       indirection table in the device.
>>> +\end{enumerate}
>>> +
>>> +If this feaure has been negotiated, the virtio header has an additional
>>> +field{rss_hash_value} field attached to it.
>>> +
>>> +\devicenormative{\subparagraph}{RSS hash offload}{Device Types /
>>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>>> +
>>> +The device MUST fill the virtio_net_ctrl_rss_hash structure with the
>>> hash
>>> +functions it supports and return the structure to the driver. Zero or
>>> more
>>> +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>>> \field{hash_function}
>>> +field.
>>> +
>>> +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>>> calculating and
>>> +attaching hashes
>>>
>>
>> For simplicity, what if just compute the hash in this case? Consider a
>> driver may want to use this hash (e.g Linux driver).
>
> I get the use case but maybe introduce a new command for computing hashes
> only? I think the disable should be preserved for disabling only.
>
>>
>>
>> for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
>>> +in the \field{gso_type} field, however the \field{hash_function} field
>>> is kept
>>> +as a part of the header.
>>> +
>>> +The device MUST drop all previous RSS configuration upon receiving
>>> +VIRTIO_NET_CTRL_RSS_SET command.
>>> +
>>> +The device MUST set the RSS configuration according to the settings
>>> provided as
>>> +follows, once the configuration process is completed the device SHOULD
>>> apply
>>> +the hash function to each of the incoming packets and distribute the
>>> packets
>>> +through the virqueues using the calculated hash and the indirection
>>> table
>>> +that were earlier provided by the driver.
>>>
>>
>> We support changing #queues dynamically, so need to clarify the behavior
>> when e.g the destination queues were disabled.
>
> I will go into these details in the next version.
>
>>
>>
>> +
>>> +Setting RSS configuration
>>> +\begin{enumerate}
>>> +\item The driver fills all of the fields and passes them through the
>>> control
>>> +       queue to the device.
>>> +
>>> +\item The device sets the RSS configuration as provided by the driver.
>>> +
>>> +\item If the device successfully applied the configuration, on each
>>> packet
>>> +       recieved the device MUST calculate the hashing for the packet and
>>> +       store it in the virtio-net header in rss_hash_value and the hash
>>> +       fields used in the calculation in rss_hash_type.
>>> +\end{enumerate}
>>> +
>>> +In case of any unexpected values/ unsupported hash function the driver
>>> +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>>> +
>>> +\drivernormative{\subparagraph}{RSS hash offload}{Device Types /
>>> Network Device / Device Operation / Control Virtqueue / RSS hash offload}
>>> +
>>> +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS the driver
>>> SHOULD
>>> +send VIRTIO_NET_CTRL_RSS_SET command to the device along with the RSS
>>> structure
>>> +filled. The RSS structure fields should be filled as follows:
>>> +
>>> +\begin{itemize}
>>> +\item The driver SHOULD choose the hash function that SHOULD be used
>>> and fill
>>> +       it in the \field{hash_function} field along with the
>>> approperiate flags
>>> +       in the \field{hash_function_flags} field. These flags inidicate
>>> to the
>>> +       device which packet fields MUST be used in the calculation
>>> process of
>>> +       the hash.
>>> +\item Once the hash function has been chosen a suitable hash key should
>>> be set
>>> +       in the \field{hash_key} field, the length of the key should be
>>> stored
>>> +       in the \field{hash_key_length} field.
>>> +\item Lastly the driver should fill the indirection table array in the
>>> +       \field{indirection_table} field while setting the array length in
>>> +       \field{indirection_table_length}. This structure is used by the
>>> device
>>> +       for determining in which RX virt queue the packet should be
>>> placed.
>>>
>>
>> Is the indirection table expected to be changed frequently? If yes, a
>> single entry modification will cause a update of the whole table.
>
> No, it should not be changing frequently
>
>
>>
>> +\end{itemize}
>>> +Once the configuration phase is over successfully, the packets SHOULD
>>> have the
>>> +\field{rss_hash_value} field with the hash value that was calculated by
>>> the
>>> +device.
>>> +
>>> +Whenever the driver wants to discard the current RSS settings, it can
>>> send an
>>> +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>>> +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>>>
>>
>> So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?
>>
> Yes, do you think we should discard one of the two?
>
>>
>> Thanks
>>
>>
>> +
>>> +The driver MUST check the \field{ack} field value provided by the
>>> device, in
>>> +case the value is not VIRTIO_NET_OK then the driver MUST hanlde faliure
>>> and
>>> +retry another hash function or else give up.
>>> +
>>>   \subparagraph{Legacy Interface: Automatic receive steering in
>>> multiqueue mode}\label{sec:Device Types / Network Device / Device Operation
>>> / Control Virtqueue / Automatic receive steering in multiqueue mode /
>>> Legacy Interface: Automatic receive steering in multiqueue mode}
>>>   When using the legacy interface, transitional devices and drivers
>>>   MUST format \field{virtqueue_pairs}
>>>
>>
>>
>
>
> --
> Respectfully,
> *Sameeh Jubran*
> *Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
> *Software Engineer @ Daynix <http://www.daynix.com>.*
>
>
>


-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*

[-- Attachment #2: Type: text/html, Size: 32219 bytes --]

Re: [virtio-dev] [RFC virtio-net 1/1] content: net: Add VIRTIO_NET_F_CTRL_RSS feature

[Sameeh Jubran]

[-- Attachment #1: Type: text/plain, Size: 15262 bytes --]

On Tue, Mar 20, 2018 at 5:10 AM, Jason Wang <jasowang@redhat.com> wrote:

>
>
> On 2018年03月19日 21:33, Sameeh Jubran wrote:
>
>>
>>
>> On Thu, Mar 15, 2018 at 4:19 AM, Jason Wang <jasowang@redhat.com <mailto:
>> jasowang@redhat.com>> wrote:
>>
>>
>>
>>     On 2018年01月26日 00:27, Sameeh Jubran wrote:
>>
>>         From: Sameeh Jubran <sameeh.j@gmail.com
>>         <mailto:sameeh.j@gmail.com>>
>>
>>         Most modern high end network devices today support
>>         configurable hash functions,
>>         this commit introduces RSS - Receive Side Scaling - [1] to
>>         virtio net device.
>>
>>         The RSS is a technology from Microsoft that boosts network
>>         device performance
>>         by efficiently distributing the traffic among the CPUs in a
>>         multiprocessor
>>         system.
>>
>>         This feature is supported in most of the modern network cards
>>         as well as most
>>         modern OSes including Linux and Windows. It is worth
>>         mentioning that both DPDK
>>         and Hyper-v support RSS too.
>>
>>         [1]
>>         https://docs.microsoft.com/en-us/windows-hardware/drivers/ne
>> twork/ndis-receive-side-scaling2
>>         <https://docs.microsoft.com/en-us/windows-hardware/drivers/
>> network/ndis-receive-side-scaling2>
>>
>>         Signed-off-by: Sameeh Jubran <sjubran@redhat.com
>>         <mailto:sjubran@redhat.com>>
>>
>>         ---
>>           content.tex | 133
>>         ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>           1 file changed, 133 insertions(+)
>>
>>         diff --git a/content.tex b/content.tex
>>         index c840588..5969b28 100644
>>         --- a/content.tex
>>         +++ b/content.tex
>>         @@ -3115,6 +3115,9 @@ features.
>>             \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address
>>         through control
>>               channel.
>>         +
>>         +\item[VIRTIO_NET_F_CTRL_RSS(24)] Device supports configurable
>>         RSS hash
>>         +    functions.
>>           \end{description}
>>             \subsubsection{Feature bit requirements}\label{sec:Device
>>         Types / Network Device / Feature bits / Feature bit requirements}
>>         @@ -3138,6 +3141,8 @@ Some networking feature bits require
>>         other networking feature bits
>>           \item[VIRTIO_NET_F_GUEST_ANNOUNCE] Requires
>>         VIRTIO_NET_F_CTRL_VQ.
>>           \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
>>           \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires
>> VIRTIO_NET_F_CTRL_VQ.
>>         +
>>         +\item[VIRTIO_NET_F_CTRL_RSS] Requires VIRTIO_NET_F_MQ.
>>           \end{description}
>>
>>
>>     Is this a requirement of RSSv2? Looks like at least RSS can work
>>     without multiqueue in V1.
>>
>> You are right this is not a requirement and should be dropped. More on
>> RSS with single hardware queue:
>>  https://docs.microsoft.com/en-us/windows-hardware/drivers/
>> network/rss-with-a-single-hardware-receive-queue <
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/
>> network/rss-with-a-single-hardware-receive-queue>
>>
>>
>>
>>             \subsubsection{Legacy Interface: Feature
>>         bits}\label{sec:Device Types / Network Device / Feature bits /
>>         Legacy Interface: Feature bits}
>>         @@ -3308,6 +3313,7 @@ struct virtio_net_hdr {
>>                   u8 flags;
>>           #define VIRTIO_NET_HDR_GSO_NONE        0
>>           #define VIRTIO_NET_HDR_GSO_TCPV4       1
>>         +#define VIRTIO_NET_HDR_GSO_RSS         2
>>
>>
>>     What's the reason of introducing a new GSO type here? RSS should
>>     be independent for a specific offloading type I think.
>>
>> This was the straight forward option as there is no need to extend the
>> virtio header but it can be moved to a specific offload.
>>
>
> If I understand correctly, RSS is not tied to GSO, so you mean
> VIRTIO_NET_HDR_RSS here?

Yup

>
>
>
>>
>>           #define VIRTIO_NET_HDR_GSO_UDP         3
>>           #define VIRTIO_NET_HDR_GSO_TCPV6       4
>>           #define VIRTIO_NET_HDR_GSO_ECN      0x80
>>         @@ -3317,6 +3323,8 @@ struct virtio_net_hdr {
>>                   le16 csum_start;
>>                   le16 csum_offset;
>>                   le16 num_buffers;
>>         +// Only if RSS hash offload has been negotiated
>>         +        le64 rss_hash_value;
>>
>>
>>     Not a native English speaker, but "rss_hash" should be sufficient.
>>
>> will do
>>
>>
>>
>>           };
>>           \end{lstlisting}
>>           @@ -4007,6 +4015,131 @@ VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET
>> command.
>>           The device MUST NOT queue packets on receive queues greater than
>>           \field{virtqueue_pairs} once it has placed the
>>         VIRTIO_NET_CTRL_MQ_VQ_PAIRS_SET command in the used ring.
>>           +\paragraph{RSS hash offload}\label{sec:Device Types /
>>         Network Device / Device Operation / Control Virtqueue / RSS
>>         hash offload}
>>         +
>>         +\begin{lstlisting}
>>         +#define RSS_HASH_FUNCTION_NONE      1
>>         +#define RSS_HASH_FUNCTION_TOEPLITZ  2
>>         +#define RSS_HASH_FUNCTION_SYMMETRIC 3
>>         +
>>         +// Hash function fields
>>         +#define RSS_HASH_FIELDS_IPV4          0x00000100
>>         +#define RSS_HASH_FIELDS_TCP_IPV4      0x00000200
>>         +#define RSS_HASH_FIELDS_IPV6          0x00000400
>>         +#define RSS_HASH_FIELDS_IPV6_EX       0x00000800
>>         +#define RSS_HASH_FIELDS_TCP_IPV6      0x00001000
>>         +#define RSS_HASH_FIELDS_TCP_IPV6_EX   0x00002000
>>         +
>>         +struct virtio_net_ctrl_rss_hash{
>>         +le32 hash_function;
>>         +}
>>         +
>>         +struct virtio_net_rss {
>>         +le32 hash_function;
>>         +le32 hash_function_flags;
>>         +le32 hash_key_length;
>>         +le32 indirection_table_length;
>>         +       struct {
>>         +               le32 hash_key[hash_key_length];
>>         +               le32 indirection_table[indirection_table_length];
>>         +       }
>>         +};
>>         +
>>         +#define VIRTIO_NET_F_CTRL_RSS     24
>>         +
>>         +#define VIRTIO_NET_CTRL_RSS    6
>>         +#define VIRTIO_NET_CTRL_RSS_GET_SUPPORTED_FUNCTIONS  0
>>         +#define VIRTIO_NET_CTRL_RSS_DISABLE    1
>>         +#define VIRTIO_NET_CTRL_RSS_SET    2
>>         +\end{lstlisting}
>>         +
>>         +If the VIRTIO_NET_F_CTRL_RSS is negotiated the driver can
>>         send control
>>         +commands for the RSS configuration.
>>         +
>>         +The class VIRTIO_NET_CTRL_RSS has three commands:
>>         +
>>         +\begin{enumerate}
>>         +\item VIRTIO_NET_CTRL_RSS_GET_SUPPLIED_FUNCTIONS returns the
>>         hash functions
>>         +       supported by the device to the driver.
>>
>>
>>     Can we just reuse virtio_net_rss in this case, it contains all the
>>     informations (e.g hash key or whatever others)?
>>
>> Sure this can work
>>
>>
>>
>>         +\item VIRTIO_NET_CTRL_RSS_DISABLE Instructs the device that
>>         RSS hashing is no
>>         +       longer required.
>>
>>
>>     So this implies that if MQ is supported, we will go to use
>>     automatic steering mode? I was thinking maybe it's better to
>>     introduce a generic command to switch between steering mode.
>>
>>  So you are suggesting that we should have an additional generic command
>> that changes between steering modes and RSS is one of those modes?
>>
>
> Yes.
>
>
>
>>
>>         +\item VIRTIO_NET_CTRL_RSS_SET applies the new RSS
>>         configuration. The command is
>>         +       used by the driver for setting RSS hash function, hash
>>         key and
>>         +       indirection table in the device.
>>         +\end{enumerate}
>>         +
>>         +If this feaure has been negotiated, the virtio header has an
>>         additional
>>         +field{rss_hash_value} field attached to it.
>>         +
>>         +\devicenormative{\subparagraph}{RSS hash offload}{Device
>>         Types / Network Device / Device Operation / Control Virtqueue
>>         / RSS hash offload}
>>         +
>>         +The device MUST fill the virtio_net_ctrl_rss_hash structure
>>         with the hash
>>         +functions it supports and return the structure to the driver.
>>         Zero or more
>>         +flags of the RSS_HASH_FUNCTION flags MUST be used to fill the
>>         \field{hash_function}
>>         +field.
>>         +
>>         +Upon recieving VIRTIO_NET_CTRL_DISABLE the device SHOULD stop
>>         calculating and
>>         +attaching hashes
>>
>>
>>     For simplicity, what if just compute the hash in this case?
>>     Consider a driver may want to use this hash (e.g Linux driver).
>>
>> I get the use case but maybe introduce a new command for computing hashes
>> only?
>>
>
> Then it looks unnecessary. (Btw, the above should be
> "VIRTIO_NET_CTRL_RSS_DISABLE").

We can have it as a sub command of the RSS command where the command
calculates hashes only
or else make the device calculate hashes only when setting the hash
function and key but not setting the indirection table
(indirection_table_length = 0)
True, thanks.

>




>
> I think the disable should be preserved for disabling only.
>>
>>
>>
>>         for the packets as well as stop setting the VIRTIO_NET_HDR_GSO_RSS
>>         +in the \field{gso_type} field, however the
>>         \field{hash_function} field is kept
>>         +as a part of the header.
>>         +
>>         +The device MUST drop all previous RSS configuration upon
>>         receiving
>>         +VIRTIO_NET_CTRL_RSS_SET command.
>>         +
>>         +The device MUST set the RSS configuration according to the
>>         settings provided as
>>         +follows, once the configuration process is completed the
>>         device SHOULD apply
>>         +the hash function to each of the incoming packets and
>>         distribute the packets
>>         +through the virqueues using the calculated hash and the
>>         indirection table
>>         +that were earlier provided by the driver.
>>
>>
>>     We support changing #queues dynamically, so need to clarify the
>>     behavior when e.g the destination queues were disabled.
>>
>> I will go into these details in the next version.
>>
>>
>>
>>         +
>>         +Setting RSS configuration
>>         +\begin{enumerate}
>>         +\item The driver fills all of the fields and passes them
>>         through the control
>>         +       queue to the device.
>>         +
>>         +\item The device sets the RSS configuration as provided by
>>         the driver.
>>         +
>>         +\item If the device successfully applied the configuration,
>>         on each packet
>>         +       recieved the device MUST calculate the hashing for the
>>         packet and
>>         +       store it in the virtio-net header in rss_hash_value
>>         and the hash
>>         +       fields used in the calculation in rss_hash_type.
>>         +\end{enumerate}
>>         +
>>         +In case of any unexpected values/ unsupported hash function
>>         the driver
>>         +MUST return VIRTIO_NET_ERR in the \field{ack} field.
>>         +
>>         +\drivernormative{\subparagraph}{RSS hash offload}{Device
>>         Types / Network Device / Device Operation / Control Virtqueue
>>         / RSS hash offload}
>>         +
>>         +If the driver negotiates the feature VIRTIO_NET_F_CTRL_RSS
>>         the driver SHOULD
>>         +send VIRTIO_NET_CTRL_RSS_SET command to the device along with
>>         the RSS structure
>>         +filled. The RSS structure fields should be filled as follows:
>>         +
>>         +\begin{itemize}
>>         +\item The driver SHOULD choose the hash function that SHOULD
>>         be used and fill
>>         +       it in the \field{hash_function} field along with the
>>         approperiate flags
>>         +       in the \field{hash_function_flags} field. These flags
>>         inidicate to the
>>         +       device which packet fields MUST be used in the
>>         calculation process of
>>         +       the hash.
>>         +\item Once the hash function has been chosen a suitable hash
>>         key should be set
>>         +       in the \field{hash_key} field, the length of the key
>>         should be stored
>>         +       in the \field{hash_key_length} field.
>>         +\item Lastly the driver should fill the indirection table
>>         array in the
>>         +       \field{indirection_table} field while setting the
>>         array length in
>>         +       \field{indirection_table_length}. This structure is
>>         used by the device
>>         +       for determining in which RX virt queue the packet
>>         should be placed.
>>
>>
>>     Is the indirection table expected to be changed frequently? If
>>     yes, a single entry modification will cause a update of the whole
>>     table.
>>
>> No, it should not be changing frequently
>>
>>
>>
>>         +\end{itemize}
>>         +Once the configuration phase is over successfully, the
>>         packets SHOULD have the
>>         +\field{rss_hash_value} field with the hash value that was
>>         calculated by the
>>         +device.
>>         +
>>         +Whenever the driver wants to discard the current RSS
>>         settings, it can send an
>>         +VIRTIO_NET_CTRL_RSS_SET command along with rss structure that has
>>         +RSS_HASH_FUNCTION_NONE the \field{hash_function} field.
>>
>>
>>     So this function looks equivalent to VIRTIO_NET_CTRL_RSS_DISABLE?
>>
>> Yes, do you think we should discard one of the two?
>>
>
> Yes.
>
> Thanks
>
>
>>     Thanks
>>
>>
>>         +
>>         +The driver MUST check the \field{ack} field value provided by
>>         the device, in
>>         +case the value is not VIRTIO_NET_OK then the driver MUST
>>         hanlde faliure and
>>         +retry another hash function or else give up.
>>         +
>>           \subparagraph{Legacy Interface: Automatic receive steering
>>         in multiqueue mode}\label{sec:Device Types / Network Device /
>>         Device Operation / Control Virtqueue / Automatic receive
>>         steering in multiqueue mode / Legacy Interface: Automatic
>>         receive steering in multiqueue mode}
>>           When using the legacy interface, transitional devices and
>>         drivers
>>           MUST format \field{virtqueue_pairs}
>>
>>
>>
>>
>>
>> --
>> Respectfully,
>> */Sameeh Jubran/*
>> /Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>/
>> /Software Engineer @ Daynix <http://www.daynix.com>./
>>
>
>


-- 
Respectfully,
*Sameeh Jubran*
*Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>*
*Software Engineer @ Daynix <http://www.daynix.com>.*

[-- Attachment #2: Type: text/html, Size: 21193 bytes --]