On 2025/05/23 23:46, Paolo Abeni wrote:
On 5/23/25 1:35 PM, Akihiko Odaki wrote:
On 2025/05/23 19:40, Paolo Abeni wrote:
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?
virtio is also an interface, and we can reuse it for QEMU-internal
interfaces too if it is appropriate.
That said, the feature bitmask defined by virtio is inappropriate for
for qemu_set_offload() because it also contains other features not
related to guest offloading. We need an alternative interface, and the
current qemu_set_offload() just passes each flag separately.
Now, "[PATCH RFC 12/16] virtio-net: implement extended features
support." is adding another format that derives from virtio for guest
offloading. This format only contains bits related to guest offloading
by definition and suits well with qemu_set_offload().
Bit names like VIRTIO_NET_O_GUEST_UDP_TUNNEL_GSO will imply that it
derives from the virtio spec I think this is actually an improvement;
the virtio spec have been the definitive document of the offloading
features of tuntap, and some features even used the virtio header (so
e1000e and igb parse and build virtio headers). These bit names make
this relationship between tuntap and the virtio spec explicit.
Let me check we are on the same page. You are suggesting the following:
- change set_offload() signature to:
typedef void (SetOffload)(NetClientState *, uint64_t);
- define VIRTIO_NET_O_GUEST_<offload> masks for known/supported offload
in include/net/net.h (including TSO, USO, etc...)
- adapt the drivers to the above interface.
- move this patch as series pre-req.
Am I correct?
Yes, that's what I meant.
Regards,
Akihiko Odaki
