On Tue, Apr 25, 2023 at 04:28:33PM -0400, Parav Pandit wrote:
>
>
> 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;
this needs a comment explaining it only exists with some
feature bits.
> > };
> 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 think I disagree.
the proposed design is consistent with regular tunneling.
however I feel we should limit this to 1-2 legacy protocols.
with that, we do not really need a new field at all,
they can fit in supported_hash_types.
> I am sorry if that was not clear enough.
>
> [1]
> https://lore.kernel.org/virtio-dev/[email protected]/
>
> > \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/
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: [email protected]
> Unsubscribe: [email protected]
> List help: [email protected]
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
