Re: [dpdk-dev] [PATCH 5/5] vhost: fix offload flags in Rx path

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 4/8/21 8:38 PM, Flavio Leitner wrote:
> On Thu, Apr 01, 2021 at 11:52:43AM +0200, David Marchand 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.
>>
>> The vhost example needs a reworking as it was built with the assumption
>> that mbuf TSO configuration is set up by the vhost library.
>> This is not done in this patch for now so TSO activation is forcibly
>> refused.
>>
>> Fixes: 859b480d5afd ("vhost: add guest offload setting")
> 
> There is change that before ECN was ignored and now it is invalid.
> I think that's the right way to go, but not sure if virtio blocks
> the negotiation of that feature.

No, I just tested and the feature gets negotiated.

Disabling it in Vhost lib should be avoided to avoid breaking
live-migration.

It might be safer to revert back to older behavior for it, i.e. just
ignore the bit. I don't think it is ever set, because otherwise we would
have had lots of reports since the Vhost log would be flooded with:

VHOST_LOG_DATA(WARNING,
	"unsupported gso type %u.\n", hdr->gso_type);

David, what do you think?

> Reviewed-by: Flavio Leitner <fbl@sysclose.org>
> 
> fbl
>