Re: [RFC PATCH 2/5] driver/net/tun: Added features for USO.

[Jason Wang <jasowang@redhat.com>]

在 2022/1/25 下午4:46, Andrew Melnychenko 写道:
> Added support for USO4 and USO6, also added code for new ioctl TUNGETSUPPORTEDOFFLOADS.
> For now, to "enable" USO, it's required to set both USO4 and USO6 simultaneously.
> USO enables NETIF_F_GSO_UDP_L4.
>
> Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> ---
>   drivers/net/tap.c | 18 ++++++++++++++++--
>   drivers/net/tun.c | 15 ++++++++++++++-
>   2 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 8e3a28ba6b28..82d742ba78b1 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -940,6 +940,10 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>   			if (arg & TUN_F_TSO6)
>   				feature_mask |= NETIF_F_TSO6;
>   		}
> +
> +		/* TODO: for now USO4 and USO6 should work simultaneously */
> +		if (arg & (TUN_F_USO4 | TUN_F_USO6) == (TUN_F_USO4 | TUN_F_USO6))
> +			features |= NETIF_F_GSO_UDP_L4;


If kernel doesn't want to split the GSO_UDP features, I wonder how much 
value to keep separated features for TUN and virtio.

Thanks


>   	}
>   
>   	/* tun/tap driver inverts the usage for TSO offloads, where
> @@ -950,7 +954,8 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>   	 * When user space turns off TSO, we turn off GSO/LRO so that
>   	 * user-space will not receive TSO frames.
>   	 */
> -	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
> +	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
> +	    feature_mask & (TUN_F_USO4 | TUN_F_USO6) == (TUN_F_USO4 | TUN_F_USO6))
>   		features |= RX_OFFLOADS;
>   	else
>   		features &= ~RX_OFFLOADS;
> @@ -979,6 +984,7 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>   	unsigned short u;
>   	int __user *sp = argp;
>   	struct sockaddr sa;
> +	unsigned int supported_offloads;
>   	int s;
>   	int ret;
>   
> @@ -1074,7 +1080,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>   	case TUNSETOFFLOAD:
>   		/* let the user check for future flags */
>   		if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> -			    TUN_F_TSO_ECN | TUN_F_UFO))
> +			    TUN_F_TSO_ECN | TUN_F_UFO |
> +			    TUN_F_USO4 | TUN_F_USO6))
>   			return -EINVAL;
>   
>   		rtnl_lock();
> @@ -1082,6 +1089,13 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
>   		rtnl_unlock();
>   		return ret;
>   
> +	case TUNGETSUPPORTEDOFFLOADS:
> +		supported_offloads = TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> +						TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_USO4 | TUN_F_USO6;
> +		if (copy_to_user(&arg, &supported_offloads, sizeof(supported_offloads)))
> +			return -EFAULT;
> +		return 0;
> +
>   	case SIOCGIFHWADDR:
>   		rtnl_lock();
>   		tap = tap_get_tap_dev(q);
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index fed85447701a..4f2105d1e6f1 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -185,7 +185,7 @@ struct tun_struct {
>   	struct net_device	*dev;
>   	netdev_features_t	set_features;
>   #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
> -			  NETIF_F_TSO6)
> +			  NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4)
>   
>   	int			align;
>   	int			vnet_hdr_sz;
> @@ -2821,6 +2821,12 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)
>   		}
>   
>   		arg &= ~TUN_F_UFO;
> +
> +		/* TODO: for now USO4 and USO6 should work simultaneously */
> +		if (arg & TUN_F_USO4 && arg & TUN_F_USO6) {
> +			features |= NETIF_F_GSO_UDP_L4;
> +			arg &= ~(TUN_F_USO4 | TUN_F_USO6);
> +		}
>   	}
>   
>   	/* This gives the user a way to test for new features in future by
> @@ -2991,6 +2997,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
>   	int sndbuf;
>   	int vnet_hdr_sz;
>   	int le;
> +	unsigned int supported_offloads;
>   	int ret;
>   	bool do_notify = false;
>   
> @@ -3154,6 +3161,12 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
>   	case TUNSETOFFLOAD:
>   		ret = set_offload(tun, arg);
>   		break;
> +	case TUNGETSUPPORTEDOFFLOADS:
> +		supported_offloads = TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> +				TUN_F_TSO_ECN | TUN_F_UFO | TUN_F_USO4 | TUN_F_USO6;
> +		if (copy_to_user(&arg, &supported_offloads, sizeof(supported_offloads)))
> +			ret = -EFAULT;
> +		break;
>   
>   	case TUNSETTXFILTER:
>   		/* Can be set only for TAPs */