Re: [virtio-comment] [PATCH] virtio-net: Add support for correct hdr_len field.

[Vitaly Mireyno <vmireyno@marvell.com>]

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