Re: [PATCH v9 0/2] virtio-net: define UDP tunnel offload

[Paolo Abeni <pabeni@redhat.com>]

On 10/9/24 09:24, Jason Wang wrote:
> On Sat, Oct 5, 2024 at 12:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
>>
>> On 10/4/24 10:13, Paolo Abeni wrote:
>>> Note that such while the mentioned PoC is based on the previous
>>> iteration of this change, it already deals with all the delta introduced
>>> here except the refined DATA_VALID handling. I'll try to update the PoC
>>> WRT that ASAP.
>>
>> The updated PoC dealing with DATA_VALID according to the spec change is
>> avail here:
>>
>> https://github.com/pabeni/linux-devel/commits/virtio_tunnel_gso_v9/
>> https://github.com/pabeni/qemu/commits/virtio_tunnel_gso_v9/
>>
>> It's slighly less untested than the previous iteration, as the
>> CHECKSUM_COMPLETE/DATA_VALID code path is explicitly covered allowing
>> the user-space to configure the tun device to set DATA_VALID on the
>> xmitted virtio hdr.
>>
>> @Jason, please LMK is something additional is needed to reach the board
>> voting.
> 
> It should work at a first glance. But the tnl_offset and new ioctl
> seems a little odd. Any reason we can't deduce it from the features?

The tun device does not know the complete layout of the virtio_net_hdr. 
It knows only the total size but is not aware i.e. if the 
VIRTIO_NET_F_MRG_RXBUF or the VIRTIO_NET_F_HASH_REPORT are negotiated.

To support UDP tunnel GSO offload, The tun device must be able to parse 
correctly the UDP tunnel fields, so it must know at least the tun fields 
offset inside the virtio_net_hdr structure.

The new ioctl is added to pass such additional data. A possible 
alternative would be piggybacking the mentioned 'tunnel offset' inside 
the offload 'long'. I avoid such option because it felt quite hackish, 
but it you feel that as preferrable it's still doable.

Another option would let the user-space know to tun all the relevant 
negotiated features, but that look quite more invasive (I would avoid 
such option at least for a PoC).

> I'm asking since the design needs to work with hardware implementation
> as well. So any channel outside virtio (e.g the TUNSETTNLOFFLOAD) may
> not work.

I've no idea about the above. My uneducated guess is that H/W 
implementation will still need a S/W interface, and such interface could 
implement the new ioctl(). In any case I guess a new SW and/or firmware 
revision will be needed to support UDP tunnel GSO offload on virtio H/W 
impl.

/P