public inbox for virtio-comment@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] virtio-net: Introduce a new field to indicate outer network header offset
@ 2025-01-14 17:16 Kommula Shiva Shankar
  2025-01-15  3:44 ` Parav Pandit
  2025-01-19  9:38 ` Parav Pandit
  0 siblings, 2 replies; 4+ messages in thread
From: Kommula Shiva Shankar @ 2025-01-14 17:16 UTC (permalink / raw)
  To: pabeni, willemb, jasowang, parav, virtio-comment
  Cc: ndabilpuram, jerinj, rbhansali

Hi Everyone,
It has been some time since this patch was sent. I am resending it for your review and look forward to your feedback.

This patch introduces a new field in the virtio_net_header called out_nh_offset,
along with a new net device feature, VIRTIO_NET_F_OUT_NET_HEADER.

Currently, there is no field available to directly read the outer network header offset in case of segmentation offload.
This requires reading packet data, which significantly affects performance in datapath.
Additionally, some hardware implementations requrie knowledege of the outer L3 offset (aka L2 length) for inline IPSec hardware acceleration.

To address this limitation, we propose splitting the csum_offset field into two 8-bit fields named csum_offset and out_nh_offset.
The csum_offset indicates the offset value from the csum_start and  may not exceed 256B bits(2^8) for protocols that use a
16-bit one's complement checksum

Following table lists such protocols and their checksum offset fields within their headers

+-----+--------+
|Proto|csum_off|
+-----+--------+
| IPV4| 10B    |
| ICMP| 2B     |
| IGMP| 2B     |
|  TCP| 16B    |
|  UDP| 6B     |
+-----+--------+

The out_nh_offset represents the start offset of the outer network header from
the beginning of the packet data

This issue was briefly discussed on the mailing list in a different thread, which can be found here
https://lore.kernel.org/all/DM4PR18MB4269FAAC3CFC7E57E25DFBD2DF8B2@DM4PR18MB4269.namprd18.prod.outlook.com/

Signed-off-by: Kommula Shiva Shankar <kshankar@marvell.com>
---
 device-types/net/description.tex | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 76585b0..48c081b 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -88,6 +88,10 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
 
+\item[VIRTIO_NET_F_OUT_NET_HEADER(50)] Driver can provide the start of \field{out_nh_offset}
+    value. Device gains advantage by not reading packet data to calculate outer network
+    header offset.
+
 \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header hash for encapsulated packets.
 
 \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification coalescing.
@@ -418,7 +422,13 @@ \subsection{Device Operation}\label{sec:Device Types / Network Device / Device O
         le16 hdr_len;
         le16 gso_size;
         le16 csum_start;
-        le16 csum_offset;
+        union {
+                le16 csum_offset;
+                struct { (Only if VIRTIO_NET_F_OUT_NET_HEADER negotiated)
+                        le8 csum_offset;
+                        le8 out_nh_offset;
+                };
+        };
         le16 num_buffers;
         le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
         le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
@@ -457,6 +467,10 @@ \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 If the driver negotiated VIRTIO_NET_F_OUT_NET_HEADER,
+   \field{out_nh_offset} indicates the outer network header offset in bytes
+    from the beginning of the packet.
+
 \item If the driver negotiated VIRTIO_NET_F_CSUM, it can skip
   checksumming the packet:
   \begin{itemize}
@@ -531,6 +545,10 @@ \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 MUST set \field{out_nh_offset} to point outer
+network header start, otherwise to zero.
+
 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
@@ -596,6 +614,7 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 	the driver SHOULD set \field{hdr_len} to a value
 	not less than the length of the headers, including the transport
 	header.
+
 \end{itemize}
 
 The driver SHOULD accept the VIRTIO_NET_F_GUEST_HDRLEN feature if it has
@@ -610,6 +629,10 @@ \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,
+the device MAY use \field{out_nh_offset} as the outer network header
+offset, otherwise device MUST NOT use the \field{out_nh_offset} value.
+
 If one of the VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options have
 been negotiated:
 \begin{itemize}
@@ -631,6 +654,7 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
 	\begin{note}
 	This is due to various bugs in implementations.
 	\end{note}
+
 \end{itemize}
 
 If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* RE: [PATCH] virtio-net: Introduce a new field to indicate outer network header offset
  2025-01-14 17:16 [PATCH] virtio-net: Introduce a new field to indicate outer network header offset Kommula Shiva Shankar
@ 2025-01-15  3:44 ` Parav Pandit
  2025-01-19  9:38 ` Parav Pandit
  1 sibling, 0 replies; 4+ messages in thread
From: Parav Pandit @ 2025-01-15  3:44 UTC (permalink / raw)
  To: Kommula Shiva Shankar, pabeni@redhat.com, willemb@google.com,
	jasowang@redhat.com, virtio-comment@lists.linux.dev
  Cc: ndabilpuram@marvell.com, jerinj@marvell.com,
	rbhansali@marvell.com


> From: Kommula Shiva Shankar <kshankar@marvell.com>
> Sent: Tuesday, January 14, 2025 10:47 PM
>
> Hi Everyone,
> It has been some time since this patch was sent. I am resending it for your
> review and look forward to your feedback.
>
> This patch introduces a new field in the virtio_net_header called
> out_nh_offset, along with a new net device feature,
> VIRTIO_NET_F_OUT_NET_HEADER.
>
> Currently, there is no field available to directly read the outer network
> header offset in case of segmentation offload.
> This requires reading packet data, which significantly affects performance in
> datapath.
> Additionally, some hardware implementations requrie knowledege of the
> outer L3 offset (aka L2 length) for inline IPSec hardware acceleration.
>
> To address this limitation, we propose splitting the csum_offset field into two
> 8-bit fields named csum_offset and out_nh_offset.
> The csum_offset indicates the offset value from the csum_start and  may not
> exceed 256B bits(2^8) for protocols that use a 16-bit one's complement
> checksum
>
> Following table lists such protocols and their checksum offset fields within
> their headers
>
> +-----+--------+
> |Proto|csum_off|
> +-----+--------+
> | IPV4| 10B    |
> | ICMP| 2B     |
> | IGMP| 2B     |
> |  TCP| 16B    |
> |  UDP| 6B     |
> +-----+--------+
>
> The out_nh_offset represents the start offset of the outer network header
> from the beginning of the packet data
>
> 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%40DM
> 4PR18MB4269.namprd18.prod.outlook.com%2F&data=05%7C02%7Cparav%
> 40nvidia.com%7Ce81096db39a14378627708dd34bf3909%7C43083d1572734
> 0c1b7db39efd9ccc17a%7C0%7C0%7C638724718122375976%7CUnknown%7
> CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJ
> XaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=TH
> SOD9fclucwxtnizGLo8pIpjsTs1YoRww4VA%2FvDwgI%3D&reserved=0
>
> Signed-off-by: Kommula Shiva Shankar <kshankar@marvell.com>
> ---
>  device-types/net/description.tex | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/device-types/net/description.tex b/device-
> types/net/description.tex
> index 76585b0..48c081b 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -88,6 +88,10 @@ \subsection{Feature bits}\label{sec:Device Types /
> Network Device / Feature bits  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)]
> Set MAC address through control
>      channel.
>
> +\item[VIRTIO_NET_F_OUT_NET_HEADER(50)] Driver can provide the start of
> \field{out_nh_offset}
The driver

> +    value. Device gains advantage by not reading packet data to calculate
> outer network
> +    header offset.
> +
The device

May be just say 'packet' instead of 'packet data', because its slightly confusing between packet L2 header and packet data in this context.

>  \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header
> hash for encapsulated packets.
>
>  \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue
> notification coalescing.
> @@ -418,7 +422,13 @@ \subsection{Device Operation}\label{sec:Device
> Types / Network Device / Device O
>          le16 hdr_len;
>          le16 gso_size;
>          le16 csum_start;
> -        le16 csum_offset;
> +        union {
> +                le16 csum_offset;
> +                struct { (Only if VIRTIO_NET_F_OUT_NET_HEADER negotiated)
> +                        le8 csum_offset;
> +                        le8 out_nh_offset;
> +                };
> +        };
>          le16 num_buffers;
>          le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>          le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> @@ -457,6 +467,10 @@ \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 If the driver negotiated VIRTIO_NET_F_OUT_NET_HEADER,
> +   \field{out_nh_offset} indicates the outer network header offset in bytes
> +    from the beginning of the packet.
> +
If the driver has negotiated

>  \item If the driver negotiated VIRTIO_NET_F_CSUM, it can skip
>    checksumming the packet:
>    \begin{itemize}
> @@ -531,6 +545,10 @@ \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 MUST set \field{out_nh_offset} to point outer network header
> +start, otherwise to zero.
> +
Need to make it more verbose that, only when there is L3 header.
If it is some packet that does not need L3 header offset, the offset can be omitted.
I believe we need a flag bit in the vnet_hdr.flags to indicate that header offset is valid.

>  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 @@ -596,6 +614,7 @@
> \subsubsection{Packet Transmission}\label{sec:Device Types / Network
> Device / De
>       the driver SHOULD set \field{hdr_len} to a value
>       not less than the length of the headers, including the transport
>       header.
> +
>  \end{itemize}
>
>  The driver SHOULD accept the VIRTIO_NET_F_GUEST_HDRLEN feature if it
> has @@ -610,6 +629,10 @@ \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, the
> +device MAY use \field{out_nh_offset} as the outer network header
> +offset, otherwise device MUST NOT use the \field{out_nh_offset} value.
> +
>  If one of the VIRTIO_NET_F_HOST_TSO4, TSO6, USO or UFO options have
> been negotiated:
>  \begin{itemize}
> @@ -631,6 +654,7 @@ \subsubsection{Packet
> Transmission}\label{sec:Device Types / Network Device / De
>       \begin{note}
>       This is due to various bugs in implementations.
>       \end{note}
> +
Please remove this blank line.
>  \end{itemize}
>
>  If VIRTIO_NET_HDR_F_NEEDS_CSUM is not set, the device MUST NOT
> --
> 2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] virtio-net: Introduce a new field to indicate outer network header offset
  2025-01-14 17:16 [PATCH] virtio-net: Introduce a new field to indicate outer network header offset Kommula Shiva Shankar
  2025-01-15  3:44 ` Parav Pandit
@ 2025-01-19  9:38 ` Parav Pandit
  2025-01-22 18:39   ` Shiva Shankar Kommula
  1 sibling, 1 reply; 4+ messages in thread
From: Parav Pandit @ 2025-01-19  9:38 UTC (permalink / raw)
  To: Kommula Shiva Shankar, pabeni@redhat.com, willemb@google.com,
	jasowang@redhat.com, virtio-comment@lists.linux.dev
  Cc: ndabilpuram@marvell.com, jerinj@marvell.com,
	rbhansali@marvell.com


> From: Kommula Shiva Shankar <kshankar@marvell.com>
> Sent: Tuesday, January 14, 2025 10:47 PM
> 
> Hi Everyone,
> It has been some time since this patch was sent. I am resending it for your
> review and look forward to your feedback.
> 
> This patch introduces a new field in the virtio_net_header called
> out_nh_offset, along with a new net device feature,
> VIRTIO_NET_F_OUT_NET_HEADER.
> 

> Currently, there is no field available to directly read the outer network header
> offset in case of segmentation offload.
> This requires reading packet data, which significantly affects performance in
> datapath.
> Additionally, some hardware implementations requrie knowledege of the
> outer L3 offset (aka L2 length) for inline IPSec hardware acceleration.
> 
> To address this limitation, we propose splitting the csum_offset field into two
> 8-bit fields named csum_offset and out_nh_offset.
> The csum_offset indicates the offset value from the csum_start and  may not
> exceed 256B bits(2^8) for protocols that use a 16-bit one's complement
> checksum
> 
> Following table lists such protocols and their checksum offset fields within
> their headers
> 
> +-----+--------+
> |Proto|csum_off|
> +-----+--------+
> | IPV4| 10B    |
> | ICMP| 2B     |
> | IGMP| 2B     |
> |  TCP| 16B    |
> |  UDP| 6B     |
> +-----+--------+
> 
> The out_nh_offset represents the start offset of the outer network header
> from the beginning of the packet data
> 
> This issue was briefly discussed on the mailing list in a different thread, which
> can be found here
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fall%2FDM4PR18MB4269FAAC3CFC7E57E25DFBD2DF8B2%40
> DM4PR18MB4269.namprd18.prod.outlook.com%2F&data=05%7C02%7Cpa
> rav%40nvidia.com%7Ce81096db39a14378627708dd34bf3909%7C43083d
> 15727340c1b7db39efd9ccc17a%7C0%7C0%7C638724718122375976%7C
> Unknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMD
> AwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7
> C%7C&sdata=THSOD9fclucwxtnizGLo8pIpjsTs1YoRww4VA%2FvDwgI%3D&re
> served=0
> 
> Signed-off-by: Kommula Shiva Shankar <kshankar@marvell.com>
> ---
>  device-types/net/description.tex | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/device-types/net/description.tex b/device-
> types/net/description.tex
> index 76585b0..48c081b 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -88,6 +88,10 @@ \subsection{Feature bits}\label{sec:Device Types /
> Network Device / Feature bits  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)]
> Set MAC address through control
>      channel.
> 
> +\item[VIRTIO_NET_F_OUT_NET_HEADER(50)] Driver can provide the start of
> \field{out_nh_offset}
> +    value. Device gains advantage by not reading packet data to calculate outer
> network
> +    header offset.
> +
>  \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header hash
> for encapsulated packets.
> 
>  \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue
> notification coalescing.
> @@ -418,7 +422,13 @@ \subsection{Device Operation}\label{sec:Device
> Types / Network Device / Device O
>          le16 hdr_len;
>          le16 gso_size;
>          le16 csum_start;
> -        le16 csum_offset;
> +        union {
> +                le16 csum_offset;
> +                struct { (Only if VIRTIO_NET_F_OUT_NET_HEADER negotiated)
> +                        le8 csum_offset;
> +                        le8 out_nh_offset;
> +                };
> +        };
>          le16 num_buffers;
>          le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
>          le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT negotiated)
> @@ -457,6 +467,10 @@ \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 If the driver negotiated VIRTIO_NET_F_OUT_NET_HEADER,
> +   \field{out_nh_offset} indicates the outer network header offset in bytes
> +    from the beginning of the packet.
> +

This field is mostly useful to device when there is GSO or checksum offload.
So this should be set optionally by the driver.
When feature is negotiated, it means that csum_offset is 8 bits in width.
A nw header offset may/may not be present.
When offset = 0, its not present (because 0 is impossible value for nw header).
When its non zero, it is valid value that device should use.

Can you please update the proposal to cover these aspects?

Also, this may find its use even on the rx side too, so that sw does not need to parse it packet.
So describe the text for it similarly.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH] virtio-net: Introduce a new field to indicate outer network header offset
  2025-01-19  9:38 ` Parav Pandit
@ 2025-01-22 18:39   ` Shiva Shankar Kommula
  0 siblings, 0 replies; 4+ messages in thread
From: Shiva Shankar Kommula @ 2025-01-22 18:39 UTC (permalink / raw)
  To: Parav Pandit
  Cc: Nithin Kumar Dabilpuram, Jerin Jacob, Rahul Bhansali,
	pabeni@redhat.com, willemb@google.com, jasowang@redhat.com,
	virtio-comment@lists.linux.dev

> -----Original Message-----
> From: Parav Pandit <parav@nvidia.com>
> Sent: Sunday, January 19, 2025 3:08 PM
> To: Shiva Shankar Kommula <kshankar@marvell.com>; pabeni@redhat.com;
> willemb@google.com; jasowang@redhat.com; virtio-
> comment@lists.linux.dev
> Cc: Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; Jerin Jacob
> <jerinj@marvell.com>; Rahul Bhansali <rbhansali@marvell.com>
> Subject: [EXTERNAL] RE: [PATCH] virtio-net: Introduce a new field to indicate
> outer network header offset
> 
> > From: Kommula Shiva Shankar <kshankar@ marvell. com> > Sent: Tuesday,
> > January 14, 2025 10: 47 PM > > Hi Everyone, > It has been some time
> > since this patch was sent. I am resending it for your > review and
> > look forward
> 
> 
> > From: Kommula Shiva Shankar <kshankar@marvell.com>
> > Sent: Tuesday, January 14, 2025 10:47 PM
> >
> > Hi Everyone,
> > It has been some time since this patch was sent. I am resending it for
> > your review and look forward to your feedback.
> >
> > This patch introduces a new field in the virtio_net_header called
> > out_nh_offset, along with a new net device feature,
> > VIRTIO_NET_F_OUT_NET_HEADER.
> >
> 
> > Currently, there is no field available to directly read the outer
> > network header offset in case of segmentation offload.
> > This requires reading packet data, which significantly affects
> > performance in datapath.
> > Additionally, some hardware implementations requrie knowledege of the
> > outer L3 offset (aka L2 length) for inline IPSec hardware acceleration.
> >
> > To address this limitation, we propose splitting the csum_offset field
> > into two 8-bit fields named csum_offset and out_nh_offset.
> > The csum_offset indicates the offset value from the csum_start and
> > may not exceed 256B bits(2^8) for protocols that use a 16-bit one's
> > complement checksum
> >
> > Following table lists such protocols and their checksum offset fields
> > within their headers
> >
> > +-----+--------+
> > |Proto|csum_off|
> > +-----+--------+
> > | IPV4| 10B    |
> > | ICMP| 2B     |
> > | IGMP| 2B     |
> > |  TCP| 16B    |
> > |  UDP| 6B     |
> > +-----+--------+
> >
> > The out_nh_offset represents the start offset of the outer network
> > header from the beginning of the packet data
> >
> > This issue was briefly discussed on the mailing list in a different
> > thread, which can be found here
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__nam11.safelinks.protection.outlook.com_-3Furl-3Dhttps-253A-252F-
> 252Flore&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=XP728rR_yI3bCCyFq0
> MEWPS2176D_oSryc7cD-0TRPs&m=TIEwJZ_xRsOSm7KLONVdffWXkShROf-
> Sdyg3IKBNiKzIV8csp8rKVRdL0RlLde13&s=6cECQ7qUASslb35Xab87t1Kd3kFJyqK
> 9iT9KzbPMN0Y&e=.
> > kernel.org%2Fall%2FDM4PR18MB4269FAAC3CFC7E57E25DFBD2DF8B2%40
> > DM4PR18MB4269.namprd18.prod.outlook.com%2F&data=05%7C02%7Cpa
> > rav%40nvidia.com%7Ce81096db39a14378627708dd34bf3909%7C43083d
> > 15727340c1b7db39efd9ccc17a%7C0%7C0%7C638724718122375976%7C
> > Unknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMD
> > AwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7
> > C%7C&sdata=THSOD9fclucwxtnizGLo8pIpjsTs1YoRww4VA%2FvDwgI%3D&re
> > served=0
> >
> > Signed-off-by: Kommula Shiva Shankar <kshankar@marvell.com>
> > ---
> >  device-types/net/description.tex | 26 +++++++++++++++++++++++++-
> >  1 file changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/device-types/net/description.tex b/device-
> > types/net/description.tex index 76585b0..48c081b 100644
> > --- a/device-types/net/description.tex
> > +++ b/device-types/net/description.tex
> > @@ -88,6 +88,10 @@ \subsection{Feature bits}\label{sec:Device Types /
> > Network Device / Feature bits  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)]
> > Set MAC address through control
> >      channel.
> >
> > +\item[VIRTIO_NET_F_OUT_NET_HEADER(50)] Driver can provide the start
> > +of
> > \field{out_nh_offset}
> > +    value. Device gains advantage by not reading packet data to
> > + calculate outer
> > network
> > +    header offset.
> > +
> >  \item[VIRTIO_NET_F_HASH_TUNNEL(51)] Device supports inner header
> hash
> > for encapsulated packets.
> >
> >  \item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue
> > notification coalescing.
> > @@ -418,7 +422,13 @@ \subsection{Device Operation}\label{sec:Device
> > Types / Network Device / Device O
> >          le16 hdr_len;
> >          le16 gso_size;
> >          le16 csum_start;
> > -        le16 csum_offset;
> > +        union {
> > +                le16 csum_offset;
> > +                struct { (Only if VIRTIO_NET_F_OUT_NET_HEADER negotiated)
> > +                        le8 csum_offset;
> > +                        le8 out_nh_offset;
> > +                };
> > +        };
> >          le16 num_buffers;
> >          le32 hash_value;        (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> >          le16 hash_report;       (Only if VIRTIO_NET_F_HASH_REPORT
> negotiated)
> > @@ -457,6 +467,10 @@ \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 If the driver negotiated VIRTIO_NET_F_OUT_NET_HEADER,
> > +   \field{out_nh_offset} indicates the outer network header offset in bytes
> > +    from the beginning of the packet.
> > +
> 
> This field is mostly useful to device when there is GSO or checksum offload.
> So this should be set optionally by the driver.
> When feature is negotiated, it means that csum_offset is 8 bits in width.
> A nw header offset may/may not be present.
> When offset = 0, its not present (because 0 is impossible value for nw
> header).
> When its non zero, it is valid value that device should use.
> 
> Can you please update the proposal to cover these aspects?
> 
> Also, this may find its use even on the rx side too, so that sw does not need to
> parse it packet.
> So describe the text for it similarly.
FWIW, while processing incoming packets, when the VIRTIO_NET_HDR_F_NEEDS_CSUM flag is set 
by device, the csum_start and csum_offset fields indicate how to calculate the checksum, which still
involves reading packet data in software. 
I'm unsure how this benefit in the RX path. Could you please clarify ?

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-01-22 18:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-14 17:16 [PATCH] virtio-net: Introduce a new field to indicate outer network header offset Kommula Shiva Shankar
2025-01-15  3:44 ` Parav Pandit
2025-01-19  9:38 ` Parav Pandit
2025-01-22 18:39   ` Shiva Shankar Kommula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox