On 5/23/25 10:16 AM, Akihiko Odaki wrote:
> On 2025/05/21 20:34, Paolo Abeni wrote:
>> @@ -890,6 +915,12 @@ static void virtio_net_apply_guest_offloads(VirtIONet
>> *n)
>> .ufo = !!(n->curr_guest_offloads & (1ULL <<
>> VIRTIO_NET_F_GUEST_UFO)),
>> .uso4 = !!(n->curr_guest_offloads & (1ULL <<
>> VIRTIO_NET_F_GUEST_USO4)),
>> .uso6 = !!(n->curr_guest_offloads & (1ULL <<
>> VIRTIO_NET_F_GUEST_USO6)),
>> +#ifdef CONFIG_INT128
>> + .tnl = !!(n->curr_guest_offloads &
>> + (1ULL << VIRTIO_NET_O_GUEST_UDP_TUNNEL_GSO)),
>> + .tnl_csum = !!(n->curr_guest_offloads &
>> + (1ULL << VIRTIO_NET_O_GUEST_UDP_TUNNEL_GSO_CSUM)),
>
> "[PATCH RFC 14/16] net: bundle all offloads in a single struct" added a
> struct for offloading, but how about passing n->curr_guest_offloads as
> is instead?
>
> It loses some type safety and makes it prone to have unknown bits, but
> omitting duplicate these bit operations may outweigh the downside.
I *think* that one of the relevant point about the current interface is
that qemu_set_offload() abstracts from the virtio specifics, as it's
also used by other drivers. Forcing them to covert the to-be-configured
offloads to a virtio specific bitmask sound incorrect to me. Possibly I
misread your suggestion?
[...]
>> diff --git a/net/tap-linux.c b/net/tap-linux.c
>> index aa5f3a6e22..b7662ece63 100644
>> --- a/net/tap-linux.c
>> +++ b/net/tap-linux.c
>> @@ -287,6 +287,12 @@ void tap_fd_set_offload(int fd, const NetOffloads *ol)
>> if (ol->uso6) {
>> offload |= TUN_F_USO6;
>> }
>> + if ((ol->tso4 || ol->tso6 || ol->uso4 || ol->uso6) && ol->tnl) {
>
> Is it possible to have ol->tnl without TSO or USO? If so, is ignoring
> ol->tnl really what you want?
The virtio specifications actually prevent setting UDP-tunnel offload
without any other "inner" offload (TSO or USO), as it makes little to no
sense (the stack can't GSO/GRO the outer header without doing the same
for the inner).
Does the above makes sense/answer your questions?
/P
