[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.
>-----Original Message-----
>From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-open.org> On Behalf Of
>Michael S. Tsirkin
>Sent: Thursday, 20 February, 2020 10:12
>To: Vitaly Mireyno <vmireyno@marvell.com>
>Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
>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?
As see it, header length is essential for TSO, and maybe not so useful for non-TSO.
To calculate the header length, I suppose a Linux driver could do something like:
skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
>
>> ---
>> 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://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-2Dope
>> n.org_archives_virtio-2Dcomment_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=l
>> DHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-
>> mREeI_KYFcLA9Idzwo&s=nlr22vvxamnoP6GTBunPiIQXatfR1jEpMVWo3QWjgkU&e=
>> Feedback License:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_who_ipr_feedback-5Flicense.pdf&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r
>> =lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915En
>> u-mREeI_KYFcLA9Idzwo&s=fLR52kyo8ob2sRKD-YqfBkcFpc4N8CjVyoGB8l9jiDY&e=
>> List Guidelines:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_policies-2Dguidelines_mailing-2Dlists&d=DwIFAg&c=nKjWec2b6R0mOyPaz
>> 7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kL
>> fj915Enu-mREeI_KYFcLA9Idzwo&s=T4qEUr9UYy6-wRGkVEQCwR6bmlDDnJHJMSR3cDx2
>> jow&e=
>> Committee:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_committees_virtio_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ
>> 3lqqsArgFRdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFc
>> LA9Idzwo&s=FvN-HcjlfOBLxKBa3aCs7Q8oLFjlwi9b0VEfabOQ-yc&e=
>> Join OASIS:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-2Dopen.
>> org_join_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdce
>> vq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=x-
>> xmnzrTYOlr2UQIHWAbcuM9xWt0IXCPnEFjNNdC2sA&e=
>
>
>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://urldefense.proofpoint.com/v2/url?u=https-3A__lists.oasis-
>2Dopen.org_archives_virtio-
>2Dcomment_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-
>mREeI_KYFcLA9Idzwo&s=nlr22vvxamnoP6GTBunPiIQXatfR1jEpMVWo3QWjgkU&e=
>Feedback License: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_who_ipr_feedback-
>5Flicense.pdf&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4
>A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=fLR52kyo8ob2sRKD-
>YqfBkcFpc4N8CjVyoGB8l9jiDY&e=
>List Guidelines: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_policies-2Dguidelines_mailing-
>2Dlists&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQAw4A_NO7
>xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=T4qEUr9UYy6-
>wRGkVEQCwR6bmlDDnJHJMSR3cDx2jow&e=
>Committee: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_committees_virtio_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgF
>Rdcevq01tbLQAw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=FvN-
>HcjlfOBLxKBa3aCs7Q8oLFjlwi9b0VEfabOQ-yc&e=
>Join OASIS: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.oasis-
>2Dopen.org_join_&d=DwIFAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=lDHJ2FW52oJ3lqqsArgFRdcevq01tbLQ
>Aw4A_NO7xgI&m=BJCwxFbolHCRz9kLfj915Enu-mREeI_KYFcLA9Idzwo&s=x-
>xmnzrTYOlr2UQIHWAbcuM9xWt0IXCPnEFjNNdC2sA&e=
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]