Re: [PATCH v13] virtio-net: support inner header hash

[Parav Pandit <parav@nvidia.com>]

On 4/23/2023 3:35 AM, Heng Qi wrote:
>   
>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -198,6 +202,7 @@ \subsection{Device configuration layout}\label{sec:Device Types / Network Device
>           u8 rss_max_key_size;
>           le16 rss_max_indirection_table_length;
>           le32 supported_hash_types;
> +        le32 supported_tunnel_hash_types;
>   };
In v12 I was asking this to move to above field from the config area to 
the GET command in comment [1] as,

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

I am sorry if that was not clear enough.

[1] 
569cbaf9-f1fb-0e1f-a2ef-b1d7cd7dbb1f@nvidia.com/">https://lore.kernel.org/virtio-dev/569cbaf9-f1fb-0e1f-a2ef-b1d7cd7dbb1f@nvidia.com/

>   \subparagraph{Supported/enabled hash types}
>   \label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash types}
> +This paragraph relies on definitions from \hyperref[intro:IP]{[IP]}, \hyperref[intro:UDP]{[UDP]} and \hyperref[intro:TCP]{[TCP]}.
>   Hash types applicable for IPv4 packets:
>   \begin{lstlisting}
>   #define VIRTIO_NET_HASH_TYPE_IPv4              (1 << 0)
> @@ -980,6 +993,152 @@ \subsubsection{Processing of Incoming Packets}\label{sec:Device Types / Network
>   (see \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / IPv6 packets without extension header}).
>   \end{itemize}
>   
> +\paragraph{Inner Header Hash}
> +\label{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Inner Header Hash}
> +
> +If VIRTIO_NET_F_HASH_TUNNEL has been negotiated, the device supports inner header hash and the driver can send
> +commands VIRTIO_NET_CTRL_TUNNEL_HASH_SET and VIRTIO_NET_CTRL_TUNNEL_HASH_GET for the inner header hash configuration.
> +
> +struct virtio_net_hash_tunnel_config {
Please move field from the config struct to here. Both are RO fields.

le32 supported_hash_tunnel_types;
> +    le32 hash_tunnel_types;
> +};
> +
> +#define VIRTIO_NET_CTRL_TUNNEL_HASH 7
> + #define VIRTIO_NET_CTRL_TUNNEL_HASH_SET 0
> + #define VIRTIO_NET_CTRL_TUNNEL_HASH_GET 1
> +
> +Filed \field{hash_tunnel_types} contains a bitmask of configured hash tunnel types as
> +defined in \ref{sec:Device Types / Network Device / Device Operation / Processing of Incoming Packets / Hash calculation for incoming packets / Supported/enabled hash tunnel types}.
> +
> +The class VIRTIO_NET_CTRL_TUNNEL_HASH has the following commands:
> +\begin{itemize}
> +\item VIRTIO_NET_CTRL_TUNNEL_HASH_SET: set the \field{hash_tunnel_types} to configure the inner header hash calculation for the device.
> +\item VIRTIO_NET_CTRL_TUNNEL_HASH_GET: get the \field{hash_tunnel_types} from the device.
> +\end{itemize}
> +
> +For the command VIRTIO_NET_CTRL_TUNNEL_HASH_SET, the structure virtio_net_hash_tunnel_config is write-only for the driver.
> +For the command VIRTIO_NET_CTRL_TUNNEL_HASH_GET, the structure virtio_net_hash_tunnel_config is read-only for the driver.
> +
You need to split the structures to two, one for get and one for set in 
above description as get and set contains different fields.
> +
> +If VIRTIO_NET_HASH_TUNNEL_TYPE_NONE is set or the encapsulation type is not included in \field{hash_tunnel_types},
> +the hash of the outer header is calculated for the received encapsulated packet.
> +
> +
> +For scenarios with sufficient external entropy or no internal hashing requirements, inner header hash may not be needed:
> +A tunnel is often expected to isolate the external network from the internal one. By completely ignoring entropy
> +in the external header and replacing it with entropy from the internal header, for hash calculations, this expectation
You wanted to say inner here like rest of the places.

s/internal header/inner header

> +The driver MUST NOT set any VIRTIO_NET_HASH_TUNNEL_TYPE_ flags that are not supported by the device.
Multiple flags so,

s/flags that are/flags which are/