On Thu, Apr 29, 2021 at 3:31 PM Maxime Coquelin
<maxime.coque...@redhat.com> wrote:
> On 4/29/21 3:30 PM, Maxime Coquelin wrote:
> >> The vhost library current configures Tx offloading (PKT_TX_*) on any
> >> packet received from a guest virtio device which asks for some offloading.
> >>
> >> This is problematic, as Tx offloading is something that the application
> >> must ask for: the application needs to configure devices
> >> to support every used offloads (ip, tcp checksumming, tso..), and the
> >> various l2/l3/l4 lengths must be set following any processing that
> >> happened in the application itself.
> >>
> >> On the other hand, the received packets are not marked wrt current
> >> packet l3/l4 checksumming info.
> >>
> >> Copy virtio rx processing to fix those offload flags but accepting
> >> VIRTIO_NET_HDR_GSO_ECN and VIRTIO_NET_HDR_GSO_UDP too.
> >>
> >> The vhost example has been updated accordingly: TSO is applied to any
> >> packet marked LRO.
> >>
> >> Fixes: 859b480d5afd ("vhost: add guest offload setting")
>
> As I understand it, this change kind of break the ABI, but it is
> actually fixing a misuse of the mbuf API, so I think we should
> take this patch.Indeed, this breaks the v21 ABI. But the only usecase I can think of is an application using TSO / checksum offloads *only* for traffic coming from vhost. I say *only* for traffic coming from vhost, because to have this application do TSO / checksum offloaing for traffic coming from a physical port, it would comply with the mbuf API and set the PKT_TX_* flags. Apart from the example/vhost, I am not sure there is such an application that only does v2v or v2p but _not_ p2v TSO / checksum offloading. (Note: I am unable to use this example... it seems unhappy with the mlx5 port I use => FPE because this driver does not support vmdq o_O) I see three options: - fix the vhost library and break the ABI that only works in an example (this current patch), - maintain the v21 ABI * using symbol versioning, this adds no branch, recompiled application use the new ABI, this can't be backported to 20.11, * keeping the current behavior by default, but introducing a new flag that an application would pass to rte_vhost_driver_register(). This new flag triggers this current patch behavior but it would add an additional branch per packets bulk in vhost dequeue path. This *could* be backported to 20.11. -- David Marchand
