RE: [PATCH v5] virtio-net: Introduce a new field to indicate outer network header offset

> From: Kommula Shiva Shankar <[email protected]>
> Sent: Wednesday, April 2, 2025 1:27 AM
>
> This patch introduces a new field in the virtio_net_hdr called outer_nh_offset,
> along with a new net device feature, VIRTIO_NET_F_OUT_NET_HEADER.
>
> Currently, drivers lack a dedicated field to signal the start of the network
> header to the device when performing checksum offload and segmentation
> offload. This requires the device to read the packet in data path, which
> significantly affects performance.
> Additionally, some hardware implementations require knowledge of the outer
> L3 offset (aka L2 length) for inline IPsec hardware acceleration.
>
> To address this limitation, we propose to introduce a new field in the
> virtio_net_hdr called outer_nh_offset.
>
> The outer_nh_offset represents the start byte offset of the outer network
> header from the beginning of the packet.
>
> This issue was briefly discussed on the mailing list in a different thread, which
> can be found here.
> https://lore.k/
> ernel.org%2Fall%2FDM4PR18MB4269FAAC3CFC7E57E25DFBD2DF8B2%40D
> M4PR18MB4269.namprd18.prod.outlook.com%2F&data=05%7C02%7Cparav
> %40nvidia.com%7Cedf3f285a2444bd9556b08dd71576107%7C43083d15727
> 340c1b7db39efd9ccc17a%7C0%7C0%7C638791342319588176%7CUnknown
> %7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAi
> OiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=
> nNqdyyxNprHlSB2XWV5fAxCbwvO3uP%2BgNLXkl2hJX04%3D&reserved=0
>
> v4->v5
>  - Added padding bytes to virtio_net_hdr to ensure 64b alignment
>  - Addressed pending review comments
> v4:https://lor/
> e.kernel.org%2Fvirtio-comment%2F20250304075955.208450-1-
> kshankar%40marvell.com%2F&data=05%7C02%7Cparav%40nvidia.com%7Ce
> df3f285a2444bd9556b08dd71576107%7C43083d15727340c1b7db39efd9ccc
> 17a%7C0%7C0%7C638791342319616725%7CUnknown%7CTWFpbGZsb3d8e
> yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoi
> TWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=rrguhqiWyhYF3WqiN
> y361dZoVZhNOE6taSxTEPFJuQ0%3D&reserved=0
>
> v3 -> v4
>  - Removed the union of new flag with existing flags. Added as a separate field
>    in the virtio net header
>  - Renamed out_nh_offset to outer_nh_offset to maintain consistency with
> other fields
>  - Spellchecks in commit message description
> v3:https://lor/
> e.kernel.org%2Fall%2F20250217172509.107212-1-
> kshankar%40marvell.com&data=05%7C02%7Cparav%40nvidia.com%7Cedf3f
> 285a2444bd9556b08dd71576107%7C43083d15727340c1b7db39efd9ccc17a
> %7C0%7C0%7C638791342319633752%7CUnknown%7CTWFpbGZsb3d8eyJFb
> XB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWF
> pbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=RvjuY5N8yV3uXnETer4gRv
> NN2%2FhF23qrN9r1RFyF9%2BE%3D&reserved=0
>
> v2 -> v3:
>  - Rebase to virtio-1.4
>  - Addressed pending review comments related to wording.
> v2:https://lor/
> e.kernel.org%2Fall%2F20250128142152.3662988-1-
> kshankar%40marvell.com%2F&data=05%7C02%7Cparav%40nvidia.com%7Ce
> df3f285a2444bd9556b08dd71576107%7C43083d15727340c1b7db39efd9ccc
> 17a%7C0%7C0%7C638791342319647212%7CUnknown%7CTWFpbGZsb3d8e
> yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoi
> TWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=1IoUxJKQff8XB%2FEe
> 5Zdn%2BXX0Df19o12BsG5Bshp8V3A%3D&reserved=0
>
> v1 -> v2:
>  - explicitly state that the out_nh_offset can be set only when a valid network
> header is present.
>  - updated out_nh_offset usage in the RX direction.
>  - minor word cleanup.
> v1:
> https://lore.k/
> ernel.org%2Fvirtio-comment%2F20250114171636.3175670-1-
> kshankar%40marvell.com%2F&data=05%7C02%7Cparav%40nvidia.com%7Ce
> df3f285a2444bd9556b08dd71576107%7C43083d15727340c1b7db39efd9ccc
> 17a%7C0%7C0%7C638791342319659853%7CUnknown%7CTWFpbGZsb3d8e
> yJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoi
> TWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=DBBnVhotS3Sgdf5%2
> BgahQEfrW48c%2FisMlh7c8I9qvOdQ%3D&reserved=0
>
> Signed-off-by: Kommula Shiva Shankar <[email protected]>
> ---
>  device-types/net/description.tex | 37
> +++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex b/device-
> types/net/description.tex
> index 1b6b54d..0bb1cef 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -136,6 +136,10 @@ \subsection{Feature bits}\label{sec:Device Types /
> Network Device / Feature bits
>
>  \item[VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM (68)] Device handles
> packets
>    carried by a UDP tunnel with partial csum for the outer header.
> +
> +\item[VIRTIO_NET_F_OUT_NET_HEADER(69)] Driver can provide the start of
> +\field{outer_nh_offset} value. Device gains advantage by not reading
> +packet
> +  to calculate outer network header offset.
>  \end{description}
>
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network
> Device / Feature bits / Feature bit requirements} @@ -599,6 +603,8 @@
> \subsection{Device Operation}\label{sec:Device Types / Network Device /
> Device O
>          le16 padding_reserved;  (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
>          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 */
>  };
>  \end{lstlisting}
>
> @@ -633,6 +639,11 @@ \subsubsection{Packet
> Transmission}\label{sec:Device Types / Network Device / De  \item The driver
> can send a completely checksummed packet.  In this case,
>    \field{flags} will be zero, and \field{gso_type} will be
> VIRTIO_NET_HDR_GSO_NONE.
>
> +\item When VIRTIO_NET_F_OUT_NET_HEADER is negotiated, the driver MAY
> optionally provide
> +   the \field{outer_nh_offset} value. A nonzero value of
> \field{outer_nh_offset} indicates
> +   a valid outer network header with in the packet, and specifies the offset in
> bytes from
> +   the beginning of the packet. Otherwise \field{outer_nh_offset} MUST not
> be used.
> +
>  \item If the driver negotiated VIRTIO_NET_F_CSUM, it can skip
>    checksumming the packet:
>    \begin{itemize}
> @@ -767,6 +778,11 @@ \subsubsection{Packet
> Transmission}\label{sec:Device Types / Network Device / De  \field{flags} to
> zero and SHOULD supply a fully checksummed  packet to the device.
>
> +If the VIRTIO_NET_F_OUT_NET_HEADER feature has been negotiated, the
> +driver MAY set \field{outer_nh_offset} to nonzero value to indicate the
> +start of the outer network header offset, if the packet contains a
> +valid network header. Otherwise, \field{outer_nh_offset} is not used.
> +
>  If VIRTIO_NET_F_HOST_TSO4 is negotiated, the driver MAY set
> \field{gso_type} to VIRTIO_NET_HDR_GSO_TCPV4 to request TCPv4
> segmentation, otherwise the driver MUST NOT set @@ -948,6 +964,11 @@
> \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device
> / De  If VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} is not set, the
> device MUST NOT use the \field{csum_start} and \field{csum_offset}.
>
> +If the VIRTIO_NET_F_OUT_NET_HEADER feature has been negotiated, and
> +\field{outer_nh_offset} is nonzero, the device MAY use
> +\field{outer_nh_offset} as the outer network header offset. Otherwise,
> +device MUST NOT use the \field{outer_nh_offset}.
> +
>  If one of the VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options have
> been negotiated:
>  \begin{itemize}
> @@ -1049,7 +1070,9 @@ \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},
> +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} 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.
>
>  If VIRTIO_NET_F_MQ is negotiated, each of receiveq1\ldots receiveqN @@ -
> 1102,6 +1125,9 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network
>    both the outer UDP checksum and the inner transport checksum
>    have been validated, otherwise only one level of checksums (the outer one
>    in case of tunnels) has been validated.
> +\item If the VIRTIO_NET_F_OUT_NET_HEADER has been negotiated, and if
> +the packet
> +  contains a valid network header, \field{outer_nh_offset} MAY be set
> +to nonzero
> +  value to indicate the outer network header offset in packet.
>  \end{enumerate}
>
>  Additionally, VIRTIO_NET_F_GUEST_CSUM, TSO4, TSO6, UDP, UDP_TUNNEL
> @@ -1215,6 +1241,11 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network  device MUST set the
> VIRTIO_NET_HDR_GSO_ECN bit in  \field{gso_type}.
>
> +If VIRTIO_NET_F_OUT_NET_HEADER has been negotiated, the device MAY
> set
> +the \field{outer_nh_offset} to nonzero value to indicate outer network
> +header offset, if packet contains a valid network header. Otherwise,
> +the device MUST not use \field{outer_nh_offset}.
> +
>  If the VIRTIO_NET_F_GUEST_CSUM feature has been negotiated, the  device
> MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in  \field{flags}, if so:
> @@ -1302,6 +1333,10 @@ \subsubsection{Processing of Incoming
> Packets}\label{sec:Device Types / Network
>
>  The driver MUST ignore \field{flag} bits that it does not recognize.
>
> +If VIRTIO_NET_F_OUT_NET_HEADER has been negotiated, and if
> +\field{outer_nh_offset} is nonzero, the driver MAY use
> +\field{outer_nh_offset} as outer network header offset. Otherwise, the
> driver MUST not use the \field{outer_nh_offset}.
> +
>  If VIRTIO_NET_HDR_F_NEEDS_CSUM bit in \field{flags} is not set or  if
> VIRTIO_NET_HDR_F_RSC_INFO bit \field{flags} is set, the  driver MUST NOT
> use the \field{csum_start} and \field{csum_offset}.
> --
> 2.43.0

Reviewed-by: Parav Pandit <[email protected]>