[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
On Thu, Oct 24, 2019 at 03:24:43PM +0000, Vitaly Mireyno wrote:
> Some devices benefit from the knowledge of the exact header length for TSO processing.
> Add a feature bit for a driver that is capable of providing the correct header length for TSO packets.
>
> Signed-off-by: Vitaly Mireyno <vmireyno@marvell.com>
So I looked at implementing this and maybe this was not such a good idea
after all.
How would we implement this in Linux?
Current code just puts skb_headlen there which is
# of bytes in the linear buffer.
Which I guess it often the header, but not at all always.
Should this have been limited to TSO packets?
I also could not figure out how this is useful for the host.
Could you enlighten me pls?
> ---
> content.tex | 49 +++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index 679391e..dac6921 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2811,6 +2811,9 @@ \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_GUEST_HDRLEN(59)] Driver can provide the exact \field{hdr_len}
> + value.
> +
> \item[VIRTIO_NET_F_RSC_EXT(61)] Device can process duplicated ACKs
> and report number of coalesced segments and duplicated ACKs
>
> @@ -2840,6 +2843,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
> \item[VIRTIO_NET_F_MQ] Requires VIRTIO_NET_F_CTRL_VQ.
> \item[VIRTIO_NET_F_CTRL_MAC_ADDR] Requires VIRTIO_NET_F_CTRL_VQ.
> \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
> +\item[VIRTIO_NET_F_GUEST_HDRLEN] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6 or VIRTIO_NET_F_HOST_UFO.
> \end{description}
>
> \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -3095,12 +3099,21 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
> into smaller packets. The other gso fields are set:
>
> \begin{itemize}
> - \item \field{hdr_len} is a hint to the device as to how much of the header
> + \item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
> + \field{hdr_len} indicates the header length that needs to be replicated
> + for each packet. It's a number of bytes from beginning of the packet
> + to beginning of the transport payload.
> + Otherwise, if the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
> + \field{hdr_len} is a hint to the device as to how much of the header
> needs to be kept to copy into each packet, usually set to the
> length of the headers, including the transport header\footnote{Due to various bugs in implementations, this field is not useful
> as a guarantee of the transport header size.
> }.
>
> + \begin{note}
> + Some devices benefit from the knowledge of the exact header length, for TSO processing.
> + \end{note}
> +
> \item \field{gso_size} is the maximum size of each packet beyond that
> header (ie. MSS).
>
> @@ -3173,9 +3186,17 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
> desired MSS.
>
> If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
> -been negotiated, the driver SHOULD set \field{hdr_len} to a value
> -not less than the length of the headers, including the transport
> -header.
> +been negotiated:
> +\begin{itemize}
> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
> + the driver MUST set \field{hdr_len} to a value equal to the length
> + of the headers, including the transport header.
> +
> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
> + 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 MUST NOT set the VIRTIO_NET_HDR_F_DATA_VALID and
> VIRTIO_NET_HDR_F_RSC_INFO bits in \field{flags}.
> @@ -3187,12 +3208,20 @@ \subsubsection{Packet Transmission}\label{sec:Device Types / Network Device / De
> device MUST NOT use the \field{csum_start} and \field{csum_offset}.
>
> If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
> -been negotiated, the device MAY use \field{hdr_len} only as a hint about the
> -transport header size.
> -The device MUST NOT rely on \field{hdr_len} to be correct.
> -\begin{note}
> -This is due to various bugs in implementations.
> -\end{note}
> +been negotiated:
> +\begin{itemize}
> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has been negotiated,
> + the device MAY use \field{hdr_len} as the transport header size.
> +
> +\item If the VIRTIO_NET_F_GUEST_HDRLEN feature has not been negotiated,
> + the device MAY use \field{hdr_len} only as a hint about the
> + transport header size.
> + The device MUST NOT rely on \field{hdr_len} to be correct.
> +
> + \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
> rely on the packet checksum being correct.
> --
>
>
> This publicly archived list offers a means to provide input to the
> OASIS Virtual I/O Device (VIRTIO) TC.
>
> In order to verify user consent to the Feedback License terms and
> to minimize spam in the list archive, subscription is required
> before posting.
>
> Subscribe: virtio-comment-subscribe@lists.oasis-open.org
> Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
> List help: virtio-comment-help@lists.oasis-open.org
> List archive: https://lists.oasis-open.org/archives/virtio-comment/
> Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
> List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
> Committee: https://www.oasis-open.org/committees/virtio/
> Join OASIS: https://www.oasis-open.org/join/
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]