* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
@ 2020-02-23 7:00 Vitaly Mireyno
2020-02-23 9:56 ` Michael S. Tsirkin
0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Mireyno @ 2020-02-23 7:00 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, jasowang
>-----Original Message-----
>From: Michael S. Tsirkin <mst@redhat.com>
>Sent: Thursday, 20 February, 2020 22:25
>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, Feb 20, 2020 at 10:18:32AM +0000, Vitaly Mireyno wrote:
>>
>> >-----Original Message-----
>> >From: Michael S. Tsirkin <mst@redhat.com>
>> >Sent: Thursday, 20 February, 2020 12:01
>> >To: Vitaly Mireyno <vmireyno@marvell.com>
>> >Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
>> >Subject: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
>> >
>> >---------------------------------------------------------------------
>> >- On Thu, Feb 20, 2020 at 09:51:17AM +0000, Vitaly Mireyno wrote:
>> >>
>> >> >-----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.
>> >
>> >So maybe we should limit this for when gso type is actually set?
>>
>> Do you mean the hdr_len field will be valid only for TSO packets, or it will be accurate only for TSO
>packets?
>> I'm fine with both options.
>
>Let's say accurate only for TSO.
>
>Can you write a spec patch like this?
Looking at the spec, I see that this is already the case - i.e. hdr_len is only required for TSO packets. Here: "5.1.6.2 Packet Transmission (3)"
>> >
>> >> To calculate the header length, I suppose a Linux driver could do something like:
>> >> skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
>
>That's only good for TCP though.
>
>
>> >
>> >That's parsing the header in software. Why not have the card do it in hardware?
>> >
>>
>> It depends on hw architecture. The architecture I'm familiar with, the hw can parse the header, but it
>happens too late for the segmentation decision.
>>
>>
>> >--
>> >MST
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
2020-02-23 7:00 [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field Vitaly Mireyno
@ 2020-02-23 9:56 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-02-23 9:56 UTC (permalink / raw)
To: Vitaly Mireyno; +Cc: virtio-comment, jasowang
On Sun, Feb 23, 2020 at 07:00:55AM +0000, Vitaly Mireyno wrote:
>
> >-----Original Message-----
> >From: Michael S. Tsirkin <mst@redhat.com>
> >Sent: Thursday, 20 February, 2020 22:25
> >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, Feb 20, 2020 at 10:18:32AM +0000, Vitaly Mireyno wrote:
> >>
> >> >-----Original Message-----
> >> >From: Michael S. Tsirkin <mst@redhat.com>
> >> >Sent: Thursday, 20 February, 2020 12:01
> >> >To: Vitaly Mireyno <vmireyno@marvell.com>
> >> >Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
> >> >Subject: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
> >> >
> >> >---------------------------------------------------------------------
> >> >- On Thu, Feb 20, 2020 at 09:51:17AM +0000, Vitaly Mireyno wrote:
> >> >>
> >> >> >-----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.
> >> >
> >> >So maybe we should limit this for when gso type is actually set?
> >>
> >> Do you mean the hdr_len field will be valid only for TSO packets, or it will be accurate only for TSO
> >packets?
> >> I'm fine with both options.
> >
> >Let's say accurate only for TSO.
> >
> >Can you write a spec patch like this?
>
> Looking at the spec, I see that this is already the case - i.e. hdr_len is only required for TSO packets. Here: "5.1.6.2 Packet Transmission (3)"
And to quote that:
\end{note}
\item If the driver negotiated
VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO, and the packet requires
TCP segmentation or UDP fragmentation, then \field{gso_type}
is set to VIRTIO_NET_HDR_GSO_TCPV4, TCPV6 or UDP.
(Otherwise, it is set to VIRTIO_NET_HDR_GSO_NONE). In this
case, packets larger than 1514 bytes can be transmitted: the
metadata indicates how to replicate the packet header to cut it
into smaller packets. The other gso fields are set:
\begin{itemize}
\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 the number of bytes from the beginning of the packet
to the 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.
}.
Fine.
However in driver requirements it is not so clear:
If \field{gso_type} differs from VIRTIO_NET_HDR_GSO_NONE, then
the driver MUST also set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in
\field{flags} and MUST set \field{gso_size} to indicate the
desired MSS.
If one of the VIRTIO_NET_F_HOST_TSO4, TSO6 or UFO options have
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}
And so on. So it's a spec defect introduced by the new feature:
it used to be a weak SHOULD so I guess the assumption was,
it's ok to ask for it in any case even for !gso packets.
But now with a MUST it's a problem. I think the TC missed this
during review (I certainly did).
Can you try to address pls?
>
> >> >
> >> >> To calculate the header length, I suppose a Linux driver could do something like:
> >> >> skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
> >
> >That's only good for TCP though.
> >
> >
> >> >
> >> >That's parsing the header in software. Why not have the card do it in hardware?
> >> >
> >>
> >> It depends on hw architecture. The architecture I'm familiar with, the hw can parse the header, but it
> >happens too late for the segmentation decision.
> >>
> >>
> >> >--
> >> >MST
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
2020-02-20 10:18 Vitaly Mireyno
@ 2020-02-20 20:24 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-02-20 20:24 UTC (permalink / raw)
To: Vitaly Mireyno; +Cc: virtio-comment, jasowang
On Thu, Feb 20, 2020 at 10:18:32AM +0000, Vitaly Mireyno wrote:
>
> >-----Original Message-----
> >From: Michael S. Tsirkin <mst@redhat.com>
> >Sent: Thursday, 20 February, 2020 12:01
> >To: Vitaly Mireyno <vmireyno@marvell.com>
> >Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
> >Subject: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
> >
> >----------------------------------------------------------------------
> >On Thu, Feb 20, 2020 at 09:51:17AM +0000, Vitaly Mireyno wrote:
> >>
> >> >-----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.
> >
> >So maybe we should limit this for when gso type is actually set?
>
> Do you mean the hdr_len field will be valid only for TSO packets, or it will be accurate only for TSO packets?
> I'm fine with both options.
Let's say accurate only for TSO.
Can you write a spec patch like this?
> >
> >> To calculate the header length, I suppose a Linux driver could do something like:
> >> skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
That's only good for TCP though.
> >
> >That's parsing the header in software. Why not have the card do it in hardware?
> >
>
> It depends on hw architecture. The architecture I'm familiar with, the hw can parse the header, but it happens too late for the segmentation decision.
>
>
> >--
> >MST
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
@ 2020-02-20 10:18 Vitaly Mireyno
2020-02-20 20:24 ` Michael S. Tsirkin
0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Mireyno @ 2020-02-20 10:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, jasowang
>-----Original Message-----
>From: Michael S. Tsirkin <mst@redhat.com>
>Sent: Thursday, 20 February, 2020 12:01
>To: Vitaly Mireyno <vmireyno@marvell.com>
>Cc: virtio-comment@lists.oasis-open.org; jasowang@redhat.com
>Subject: [EXT] Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
>
>----------------------------------------------------------------------
>On Thu, Feb 20, 2020 at 09:51:17AM +0000, Vitaly Mireyno wrote:
>>
>> >-----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.
>
>So maybe we should limit this for when gso type is actually set?
Do you mean the hdr_len field will be valid only for TSO packets, or it will be accurate only for TSO packets?
I'm fine with both options.
>
>> To calculate the header length, I suppose a Linux driver could do something like:
>> skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
>
>That's parsing the header in software. Why not have the card do it in hardware?
>
It depends on hw architecture. The architecture I'm familiar with, the hw can parse the header, but it happens too late for the segmentation decision.
>--
>MST
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
2020-02-20 9:51 Vitaly Mireyno
@ 2020-02-20 10:01 ` Michael S. Tsirkin
0 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-02-20 10:01 UTC (permalink / raw)
To: Vitaly Mireyno; +Cc: virtio-comment, jasowang
On Thu, Feb 20, 2020 at 09:51:17AM +0000, Vitaly Mireyno wrote:
>
> >-----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.
So maybe we should limit this for when gso type is actually set?
> To calculate the header length, I suppose a Linux driver could do something like:
> skb_transport_header(skb) + tcp_hdrlen(skb) - skb->data
That's parsing the header in software. Why not have the card do it in
hardware?
--
MST
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
@ 2020-02-20 9:51 Vitaly Mireyno
2020-02-20 10:01 ` Michael S. Tsirkin
0 siblings, 1 reply; 9+ messages in thread
From: Vitaly Mireyno @ 2020-02-20 9:51 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtio-comment, jasowang
>-----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=
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
2019-10-24 15:24 Vitaly Mireyno
2020-02-20 8:00 ` Michael S. Tsirkin
@ 2020-02-20 8:11 ` Michael S. Tsirkin
1 sibling, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-02-20 8:11 UTC (permalink / raw)
To: Vitaly Mireyno; +Cc: virtio-comment, jasowang
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/
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
2019-10-24 15:24 Vitaly Mireyno
@ 2020-02-20 8:00 ` Michael S. Tsirkin
2020-02-20 8:11 ` Michael S. Tsirkin
1 sibling, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2020-02-20 8:00 UTC (permalink / raw)
To: Vitaly Mireyno; +Cc: virtio-comment, jasowang
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>
Actually I was wondering about that. How important is this in practice?
Reason is I am looking to drop fields from the header.
> ---
> 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/
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/
^ permalink raw reply [flat|nested] 9+ messages in thread
* [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.
@ 2019-10-24 15:24 Vitaly Mireyno
2020-02-20 8:00 ` Michael S. Tsirkin
2020-02-20 8:11 ` Michael S. Tsirkin
0 siblings, 2 replies; 9+ messages in thread
From: Vitaly Mireyno @ 2019-10-24 15:24 UTC (permalink / raw)
To: virtio-comment; +Cc: Michael S. Tsirkin, jasowang
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>
---
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/
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-02-23 9:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-23 7:00 [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field Vitaly Mireyno
2020-02-23 9:56 ` Michael S. Tsirkin
-- strict thread matches above, loose matches on Subject: below --
2020-02-20 10:18 Vitaly Mireyno
2020-02-20 20:24 ` Michael S. Tsirkin
2020-02-20 9:51 Vitaly Mireyno
2020-02-20 10:01 ` Michael S. Tsirkin
2019-10-24 15:24 Vitaly Mireyno
2020-02-20 8:00 ` Michael S. Tsirkin
2020-02-20 8:11 ` Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.