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