RE: [PATCH v6 3/4] virtio-net: extend virtio_net_hdr for IPsec support

> Subject: [EXTERNAL] RE: [PATCH v6 3/4] virtio-net: extend virtio_net_hdr for
> IPsec support
> 
> > From: Srujana Challa <schalla@ marvell. com> > Sent: Tuesday, April
> > 29, 2025 6: 10 PM > > Add IPsec resource object identifiers to the
> > virtio_net_hdr for identifying > encryption/decryption operations on
> > tx and rx side respectively,
> 
> > From: Srujana Challa <[email protected]>
> > Sent: Tuesday, April 29, 2025 6:10 PM
> >
> > Add IPsec resource object identifiers to the virtio_net_hdr for
> > identifying encryption/decryption operations on tx and rx side
> > respectively, along with flags.
> >
> > Signed-off-by: Srujana Challa <[email protected]>
> > ---
> >  device-types/net/description.tex | 51
> > ++++++++++++++++++++++++++++++--
> >  1 file changed, 48 insertions(+), 3 deletions(-)
> >
> > diff --git a/device-types/net/description.tex b/device-
> > types/net/description.tex index 105a002..2540f2b 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -597,6 +597,9 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Network Device / Device O
> >  #define VIRTIO_NET_HDR_F_DATA_VALID    2
> >  #define VIRTIO_NET_HDR_F_RSC_INFO      4
> >  #define VIRTIO_NET_HDR_F_UDP_TUNNEL_CSUM 8
> > +#define VIRTIO_NET_HDR_F_SECURITY      16
> > +#define VIRTIO_NET_HDR_F_SECURITY_ERR  32 #define
> > +VIRTIO_NET_HDR_F_SECURITY_SA_SOFT_EXPIRY_WARN 64
> >          u8 flags;
> >  #define VIRTIO_NET_HDR_GSO_NONE        0
> >  #define VIRTIO_NET_HDR_GSO_TCPV4       1
> > @@ -618,7 +621,11 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Network Device / Device O
> >          le16 outer_th_offset    (Only if
> VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO
> > or VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO negotiated)
> >          le16 inner_nh_offset;   (Only if
> VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO
> > or VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO negotiated)
> >          le16 outer_nh_offset;   /* Only if VIRTIO_NET_F_OUT_NET_HEADER
> > negotiated */
> > -        u8 padding_reserved_2[6]; /* Only if
> VIRTIO_NET_F_OUT_NET_HEADER
> > negotiated */
> > +        /* Only if VIRTIO_NET_F_IPSEC negotiated */
> 
> > +        /* Only if VIRTIO_NET_F_OUT_NET_HEADER or VIRTIO_NET_F_IPSEC
> > + negotiated */
> This should be,
> union {
> 	u8 padding_reserved_2[6];
> 	struct ipsec_resource_hdr {
>                  le32 resource_id;
>                  le16 resource_type;
>               } ipsec_resource_hdr;
>   };
> 
> >  \end{lstlisting}
> >
> > @@ -972,6 +979,15 @@ \subsubsection{Packet
> > Transmission}\label{sec:Device Types / Network Device / De  together
> > with the
> > VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV4 bit or the
> >  VIRTIO_NET_HDR_F_GSO_UDP_TUNNEL_IPV6 bit in \field{gso_type}.
> >
> > +If the device supports \hyperref[par:Device Types / Network Device /
> > +Device Operation / IPsec Operation]{IPsec Operation}, the driver may
> > +set
> > VIRTIO_NET_HDR_F_SECURITY bit in \field{flags}, if so:
> > +\begin{enumerate}
> > +\item the driver MUST create IPsec Outbound resource object
> > +\hyperref[par:Device Types / Crypto Device / Device Operation / IPsec
> > +Service Operation / Resource objects /
> > +VIRTIO-CRYPTO-RESOURCE-OBJ-IPSEC-ENC-
> > SA]{VIRTIO_NET_RESOURCE_OBJ_IPSEC_
> > +OUTB_SA} \item the driver MUST set \field{resource_id} to a valid
> > +IPsec outbound
> > +      resource object ID.
> > +\end{enumerate}
> > +
> >  \devicenormative{\paragraph}{Packet Transmission}{Device Types /
> > Network Device / Device Operation / Packet Transmission}  The device
> > MUST ignore \field{flag} bits that it does not recognize.
> >
> > @@ -1032,6 +1048,10 @@ \subsubsection{Packet
> > Transmission}\label{sec:Device Types / Network Device / De
> >
> >  If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT  rely
> > on the packet checksum being correct.
> > +
> > +If VIRTIO_NET_HDR_F_SECURITY bit in \field{flags} is not set, the
> > +device MUST NOT use the \field{resource_id} and \field{resource_type}.
> > +
> >  \paragraph{Packet Transmission Interrupt}\label{sec:Device Types /
> > Network Device / Device Operation / Packet Transmission / Packet
> > Transmission Interrupt}
> >
> >  Often a driver will suppress transmission virtqueue interrupts @@
> > -1084,8
> > +1104,8 @@ \subsubsection{Setting Up Receive Buffers}\label{sec:Device
> > Types / Network Devi  \end{note}
> >
> >  When calculating the size of \field{struct virtio_net_hdr}, the
> > driver -MUST consider all the fields inclusive up to
> > \field{padding_reserved_2}, -i.e. 32 bytes if
> > VIRTIO_NET_F_OUT_NET_HEADER is negotiated or up to
> > \field{inner_nh_offset}
> > +MUST consider all the fields inclusive up to \field{struct
> > +ipsec_resource_hdr}, i.e. 32 bytes if VIRTIO_NET_F_OUT_NET_HEADER or
> > +VIRTIO_NET_F_IPSEC is negotiated or up to \field{inner_nh_offset}
> >  i.e. 24 bytes if VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated or up
> > to \field{padding_reserved}  i.e. 20 bytes if VIRTIO_NET_F_HASH_REPORT
> > is negotiated, and 12 bytes if not.
> >
> Please change the wording to refer to padding or ipsec_resource_hdr
> accordingly.
Could you confirm if changing as below would be fine,
i.e. 32 bytes if VIRTIO_NET_F_OUT_NET_HEADER or VIRTIO_NET_F_IPSEC is negotiated or up to \field{inner_nh_offset}.
If only VIRTIO_NET_F_OUT_NET_HEADER is negotiated and VIRTIO_NET_F_IPSEC is not negotiated, the driver SHOULD
reference \field{padding_reserved_2}. Otherwise, it SHOULD reference \field{struct ipsec_resource_hdr} from
the \field{union}.
i.e. 24 bytes if VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated or up to \field{padding_reserved}.

Thanks.
> 
> > @@ -1341,6 +1361,21 @@ \subsubsection{Processing of Incoming
> > Packets}\label{sec:Device Types / Network  \field{flags}, the device
> > MUST either provide a zero outer UDP header  checksum or a fully
> > checksummed outer UDP header.
> >
> > +The device MUST set the VIRTIO_NET_HDR_F_SECURITY bit in the
> > +\field{flags} if the packet goes through the IPsec processing.
> > +Otherwise, this bit MUST be cleared. The device MUST set or clear
> > +this bit regardless of setting VIRTIO_NET_HDR_F_SECURITY_ERR or
> > VIRTIO_NET_HDR_F_SECURITY_SA_SOFT_EXPIRY_WARN bit.
> > +
> > +The device MUST set the VIRTIO_NET_HDR_F_SECURITY_ERR bit in the
> > +\field{flags} if any error is encountered during IPsec processing.
> > +Otherwise,
> > this bit MUST be cleared.
> > +The device MUST set or clear this bit regardless of setting
> > +VIRTIO_NET_HDR_F_SECURITY_SA_SOFT_EXPIRY_WARN bit.
> > +
> > +The device MUST set the
> > VIRTIO_NET_HDR_F_SECURITY_SA_SOFT_EXPIRY_WARN
> > +bit in the \field{flags} if the SA associated with
> > +\field{resource_id} reaches the SA lifetime soft limits configured in
> > +the \field{struct
> > virtio_crypto_ipsec_lifetime}.
> > +See \hyperref[par:Device Types / Crypto Device / Device Operation /
> > +IPsec
> > Service Operation / Resource objects /
> > VIRTIO-CRYPTO-RESOURCE-OBJ-IPSEC- OUTBOUND-
> SA]{VIRTIO_NET_RESOURCE_OBJ_IPSEC_OUTB_SA}.
> > +
> >  \drivernormative{\paragraph}{Processing of Incoming  Packets}{Device
> > Types / Network Device / Device Operation /  Processing of Incoming
> > Packets} @@ -
> > 1392,6 +1427,12 @@ \subsubsection{Processing of Incoming
> > Packets}\label{sec:Device Types / Network
> >  VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6 in \field{gso_type} are not set,
> > the driver MOST NOT accept the packet.
> >
> > +When VIRTIO_NET_HDR_F_SECURITY and
> > VIRTIO_NET_HDR_F_SECURITY_ERR bits
> > +are set in the \field{flags}, it indicates that the device
> > +experienced a
> > processing error on the IPsec packet.
> > +It need not be an 'error packet'. For example, a particular SA was
> > +not offloaded or SA has reached the SA lifetime hard limits
> > +configured in the
> > \field{struct virtio_crypto_ipsec_lifetime}.
> > +So vnet_hdr will have both bits set.
> > +
> >  \paragraph{Hash calculation for incoming packets}  \label{sec:Device
> > Types / Network Device / Device Operation / Processing of Incoming
> > Packets / Hash calculation for incoming packets}
> >
> > @@ -3528,6 +3569,10 @@ \subsubsection{IPsec
> > Operation}\label{sec:Device Types / Network Device / Device  See
> > \ref{sec:Device Types / Network Device / Device Operation / Flow
> > filter}  for details about flow filter.
> >
> > +Note that there is a small race condition where a SA object might be
> > +destroyed while a recieve packet is still inflight. The driver SHOULD
> > +handle this situation appropriately.
> > +
> >  \paragraph{Device and driver capabilities}  \label{par:Device Types /
> > Network Device / Device Operation / IPsec Operation / Device and
> > driver capabilities}
> >
> > --
> > 2.25.1
> 
> For the rest of the content of this patch:
> Reviewed-by: Parav Pandit <[email protected]>