Re: [PATCH RFC 1/4] virtio-crypto: Add IPsec service operation and Capabilities

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

Hello, 

I left some minor comments:

On Fri, Nov 15, 2024 at 05:15:20PM +0530, Srujana Challa wrote:
> This commit introduces the IPsec service operation to the Crypto
> device, enabling offloading of IPsec processing.
> 
> Capabilities:
> 
> 1. IPsec Resource Capability (VIRTIO_CRYPTO_IPSEC_RESOURCE_CAP):
>    Indicates the device's IPsec resource limits, such as the number of
>    outbound and inbound Security Associations (SAs).
> 2. IPsec SA Capability (VIRTIO_CRYPTO_IPSEC_SA_CAP): Specifies the
>    supported IPsec modes, along with the supported cryptographic
>    algorithms, authentication algorithms, IPsec options and
>    anti-replay window size.
> 
> Signed-off-by: Srujana Challa <schalla@marvell.com>
> ---
>  device-types/crypto/description.tex | 164 +++++++++++++++++++++++++++-
>  1 file changed, 159 insertions(+), 5 deletions(-)
> 
> diff --git a/device-types/crypto/description.tex b/device-types/crypto/description.tex
> index 5705e26..ce4b1fb 100644
> --- a/device-types/crypto/description.tex
> +++ b/device-types/crypto/description.tex
> @@ -2,11 +2,11 @@ \section{Crypto Device}\label{sec:Device Types / Crypto Device}
>  
>  The virtio crypto device is a virtual cryptography device as well as a
>  virtual cryptographic accelerator. The virtio crypto device provides the
> -following crypto services: CIPHER, MAC, HASH, AEAD and AKCIPHER. Virtio crypto
> -devices have a single control queue and at least one data queue. Crypto
> -operation requests are placed into a data queue, and serviced by the
> -device. Some crypto operation requests are only valid in the context of a
> -session. The role of the control queue is facilitating control operation
> +following crypto services: CIPHER, MAC, HASH, AEAD, AKCIPHER and IPSEC.
> +Virtio crypto devices have a single control queue and at least one data
> +queue. Crypto operation requests are placed into a data queue, and serviced
> +by the device. Some crypto operation requests are only valid in the context
> +of a session. The role of the control queue is facilitating control operation
>  requests. Sessions management is realized with control operation
>  requests.
>  
> @@ -72,6 +72,8 @@ \subsection{Supported crypto services}\label{sec:Device Types / Crypto Device /
>  #define VIRTIO_CRYPTO_SERVICE_AEAD   3
>  /* AKCIPHER (Asymmetric Key Cipher) service */
>  #define VIRTIO_CRYPTO_SERVICE_AKCIPHER 4
> +/* IPSEC service */
> +#define VIRTIO_CRYPTO_SERVICE_IPSEC  5
>  \end{lstlisting}
>  
>  The above constants designate bits used to indicate the which of crypto services are
> @@ -318,6 +320,20 @@ \subsection{Device Initialization}\label{sec:Device Types / Crypto Device / Devi
>  \item The driver MUST read the supported algorithms based on \field{crypto_services} field.
>  \end{itemize*}
>  
> +\subsection{Device and driver capabilities}\label{sec:Device Types / Crypto Device / Device and driver capabilities}
> +
> +The crypto device has the following capabilities.
> +
> +\begin{tabularx}{\textwidth}{ |l||l|X| }
> +\hline
> +Identifier & Name & Description \\
> +\hline \hline
> +0x0800 & \hyperref[par:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-RESOURCE-CAP]{VIRTIO_CRYPTO_IPSEC_RESOURCE_CAP} & IPsec resource capability \\
> +\hline
> +0x0801 & \hyperref[par:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP]{VIRTIO_CRYPTO_IPSEC_SA_CAP} & IPsec Security Association(SA) capability \\
> +\hline
> +\end{tabularx}
> +
>  \subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
>  
>  The operation of a virtio crypto device is driven by requests placed on the virtqueues.
> @@ -1872,3 +1888,141 @@ \subsubsection{AKCIPHER Service Operation}\label{sec:Device Types / Crypto Devic
>  \item VIRTIO_CRYPTO_ERR if any failure not mentioned above occurs.
>  \end{itemize*}
>  \end{itemize*}
> +
> +\subsubsection{IPSEC Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / IPSEC Service Operation}
> +
> +A crypto device can support the programming of IPsec Security Associations(SAs).
> +In addition to standard crypto processing, the IPsec protocol processing is
> +also offloaded to the Crypto Device as lookaside operation.
> +
> +IPsec Inbound processing: The device will perform decryption, authentication,
> +integrity checking and remove additional headers, including tunnel headers if
> +in tunnel mode, as well as the ESP/AH header from the packet. The resulting
> +packet contains only the plain data.
> +
> +IPsec Outbound processing: The device will perform encryption, attach ICV,
> +update/add IP headers, add ESP/AH headers/trailers.
> +
> +A crypto device can support number of IPsec SAs, allowing it to manage multiple secure
> +connections simultaneously.
> +
> +The device and the driver indicates IPsec SA resource limits using the capability

s/indicates/indicate

> +VIRTIO_CRYPTO_IPSEC_RESOURCE_CAP specifying the limits on the number of IPsec outbound and
> +inbound SA resource objects.
> +The capability VIRTIO_CRYPTO_IPSEC_SA_CAP specifies which IPsec protocol capabilities
> +the device supports. The driver indicates the IPsec parameters(Crypto algorithm,
> +IPsec mode, anti-replay window size, etc.) it is using by setting the IPsec SA capability,
> +prior to adding any resource objects.
> +

I think you mean:
`The driver indicates the IPsec parameters by setting the IPsec SA
capability prior to adding any resource objects`.

> +The driver controls the IPsec SA resource objects using administration commands described in
> +\ref{sec:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Device resource objects}.
> +
> +\paragraph{Device and driver capabilities}
> +\label{par:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities}
> +
> +\subparagraph{VIRTIO_CRYPTO_IPSEC_RESOURCE_CAP}
> +\label{par:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-RESOURCE-CAP}
> +
> +The capability VIRTIO_CRYPTO_IPSEC_RESOURCE_CAP indicates the IPsec SA resource limits.
> +\field{cap_specific_data} is in the format
> +\field{struct virtio_crypto_ipsec_resource_cap}.
> +
> +\begin{lstlisting}
> +struct virtio_crypto_ipsec_resource_cap {
> +        le32 inb_sa_limit;
> +        le32 outb_sa_limit;
> +};
> +\end{lstlisting}
> +
> +\field{inb_sa_limit}, and \field{outb_sa_limit} denote the maximum number of IPsec
> +security Associations (SAs) that can be utilized for IPsec inbound and outbound processing,
> +respectively, which the device is capable of creating.
> +
> +\subparagraph{VIRTIO_CRYPTO_IPSEC_SA_CAP}
> +\label{par:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP}
> +
> +The capability VIRTIO_CRYPTO_IPSEC_SA_CAP lists the supported IPsec modes along with

I think here `capability` goes after `VIRTIO_CRYPTO_IPSEC_SA_CAP`. I
think this also applies to cases above.

> +the supported cryptographic, authentication algorithms and anti-replay window size for
> +each IPsec mode.
> +\field{cap_specific_data} is in the format \field{struct virtio_crypto_ipsec_sa_cap_data}.
> +
> +\begin{lstlisting}[label={lst:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP / virtio-crypto-ipsec-mode-cap}]
> +struct virtio_crypto_ipsec_mode_cap {
> +        le32 replay_win_sz_max;
> +        le32 options;
> +        le64 cipher_algo;
> +        le64 hmac_algo;
> +        le32 aead_algo;
> +        le32 max_cipher_key_len;
> +        le32 max_auth_key_len;
> +        u8 mode;
> +        u8 padding[3];
> +};
> +
> +struct virtio_crypto_ipsec_sa_cap_data {
> +        u8 count;
> +        u8 reserved[7];
> +        struct virtio_crypto_ipsec_mode_cap mode[];
> +};
> +\end{lstlisting}
> +
> +\field{count} indicates number of valid entries in the \field{mode} array.
> +\field{mode[]} is an array of supported IPsec modes. Within each array entry:
> +
> +\field{mode} specifies the IPsec mode, as defined in table
> +\ref{table:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP / IPsec Modes}.
> +\field{replay_win_sz_max} specifies the maximum anti-replay window size the device supports.
> +
> +\field{options} represents the IPsec protocol options the device supports, as defined in table
> +\ref{table:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP / IPsec Options}.
> +
> +\field{cipher_algo} CIPHER algorithms mask, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
> +
> +\field{hmac_algo} HMAC algorithms mask, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
> +
> +\field{aead_algo} AEAD algorithms mask, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / AEAD services}.
> +
> +\field{max_cipher_key_len} is the maximum length of cipher key supported by the device.
> +
> +\field{max_auth_key_len} is the maximum length of authenticated key supported by the device.
> +
> +\begin{table}[H]
> +\caption{IPsec Modes}
> +\label{table:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP / IPsec Modes}
> +\begin{tabularx}{\textwidth}{ |l|X|X| }
> +\hline
> +Type & Name & Description \\
> +\hline \hline
> +0x0 & - & Reserved \\
> +\hline
> +0x1 & VIRTIO_CRYPTO_IPSEC_MODE_ESP_TUNNEL & IPsec ESP protocol in tunnel mode \\
> +\hline
> +0x2 & VIRTIO_CRYPTO_IPSEC_MODE_ESP_TRANSPORT & IPsec ESP protocol in transport mode \\
> +\hline
> +0x3 & VIRTIO_CRYPTO_IPSEC_MODE_AH_TUNNEL & IPsec AH protocol in tunnel mode \\
> +\hline
> +0x4 & VIRTIO_CRYPTO_IPSEC_MODE_AH_TRANSPORT & IPsec AH protocol in transport mode \\
> +\hline
> +\end{tabularx}
> +\end{table}
> +
> +\begin{table}[H]
> +\caption{IPsec Options}
> +\label{table:Device Types / Crypto Device / Device Operation / IPsec Service Operation / Device and driver capabilities / VIRTIO-CRYPTO-IPSEC-SA-CAP / IPsec Options}
> +\begin{tabularx}{\textwidth}{ |l|X|X| }
> +\hline
> +Type & Name & Description \\
> +\hline \hline
> +0x0 & - & Reserved \\
> +\hline
> +0x1 & VIRTIO_CRYPTO_IPSEC_ESN & Specifies whether extended sequence number is supported \\
> +\hline
> +0x2 & VIRTIO_CRYPTO_IPSEC_UDP_ENCAP & Specifies whether udp encapsulation is supported \\
> +\hline
> +0x3 & VIRTIO_CRYPTO_IPSEC_COPY_DSCP & Specifies whether copy dscp is supported \\
> +\hline
> +0x4 & VIRTIO_CRYPTO_IPSEC_DEC_TTL & Specifies whether decrementing the time to live is supported \\
> +\hline
> +\end{tabularx}
> +\end{table}
> +
> -- 
> 2.25.1
> 
>