On Thu, Jun 12, 2025 at 1:03 PM Paolo Abeni [off-list ref] wrote:

On 6/12/25 6:55 AM, Jason Wang wrote:

quoted

On Fri, Jun 6, 2025 at 7:46 PM Paolo Abeni [off-list ref] wrote:

quoted

@@ -1720,8 +1732,16 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,

        if (tun->flags & IFF_VNET_HDR) {
                int vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz);
+               int parsed_size;

-               hdr_len = tun_vnet_hdr_get(vnet_hdr_sz, tun->flags, from, &gso);
+               if (vnet_hdr_sz < TUN_VNET_TNL_SIZE) {

I still don't understand why we need to duplicate netdev features in
flags, and it seems to introduce unnecessary complexities. Can we
simply check dev->features instead?

I think I've asked before, for example, we don't duplicate gso and
csum for non tunnel packets.

[...]

Still the additional complexity is ~5 lines and makes all the needed
information available on a single int, which is quite nice performance
wise. Do you have strong feeling against it?

I forgot to mention a couple of relevant points: the tun_vnet_*
helpers are used also by tap devices, so we can't pass the tun struct
as an argument, and we will need to add a new argument to pass the
dev->features or dev pointer, which is IMHO not nice. Also we should
provide backward compatible variants for all the helpers to avoid
touching the tap driver. Overall using the 'dev->features' will
require a comparable code churn, likely even greater.

For plain GSO offload, currently the code validation is quite liberal
and doesn't check the actual offloaded features. We  can't change the
existing behaviour for backward compatibility, but we want to be more
conservative with the new code, when possible - so we want the
information available to the helpers.

/P