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

> From: Srujana Challa <[email protected]>
> Sent: Tuesday, May 20, 2025 2:39 PM
> 
> > 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}.

Small nit changes to adhere to existing wording,

When calculating the size of struct virtio_net_hdr, the driver MUST consider all the fields inclusive up to
padding_reserved_2, i.e. 32 bytes if VIRTIO_NET_F_OUT_NET_HEADER or VIRTIO_NET_F_OUT_NET_HEADER is negotiated or up to inner_-
nh_offset i.e. 24 bytes if VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO is negotiated or up to padding_-
reserved i.e. 20 bytes if VIRTIO_NET_F_HASH_REPORT is negotiated, and 12 bytes if not.

> 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}.
> 
When describing the vnet hdr size allocation, do not mix it on what to access or not.
You can move this in ipsec description if its not already there.

> 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]>