netdev - Re: [PATCH net-next 5/8] net: implement virtio helpers to handle UDP GSO tunneling.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <682fa555b2bcc_13d837294a8@willemb.c.googlers.com.notmuch>
Date: Thu, 22 May 2025 18:29:41 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Paolo Abeni <pabeni@...hat.com>, 
 netdev@...r.kernel.org
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Jason Wang <jasowang@...hat.com>, 
 Andrew Lunn <andrew+netdev@...n.ch>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 "Michael S. Tsirkin" <mst@...hat.com>, 
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, 
 Eugenio Pérez <eperezma@...hat.com>
Subject: Re: [PATCH net-next 5/8] net: implement virtio helpers to handle UDP
 GSO tunneling.

Paolo Abeni wrote:
> The virtio specification are introducing support for GSO over
> UDP tunnel.
> 
> This patch brings in the needed defines and the additional
> virtio hdr parsing/building helpers.
> 
> The UDP tunnel support uses additional fields in the virtio hdr,
> and such fields location can change depending on other negotiated
> features - specifically VIRTIO_NET_F_HASH_REPORT.
> 
> Try to be as conservative as possible with the new field validation.
> 
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>

No major concerns from me on this series. Much of the design
conversations took place earlier on the virtio list.

Maybe consider test coverage. If end-to-end testing requires qemu,
then perhaps KUnit is more suitable for testing basinc to/from skb
transformations. Just a thought.

> ---
>  include/linux/virtio_net.h      | 177 ++++++++++++++++++++++++++++++--
>  include/uapi/linux/virtio_net.h |  33 ++++++
>  2 files changed, 202 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/virtio_net.h b/include/linux/virtio_net.h
> index 02a9f4dc594d0..cf9c712a67cd4 100644
> --- a/include/linux/virtio_net.h
> +++ b/include/linux/virtio_net.h
> @@ -47,9 +47,9 @@ static inline int virtio_net_hdr_set_proto(struct sk_buff *skb,
>  	return 0;
>  }
>  
> +static inline int virtio_net_hdr_tnl_to_skb(struct sk_buff *skb,
> +					    const struct virtio_net_hdr *hdr,
> +					    unsigned int tnl_hdr_offset,
> +					    bool tnl_csum_negotiated,
> +					    bool little_endian)
> +{
> +	u8 gso_tunnel_type = hdr->gso_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL;
> +	unsigned int inner_nh, outer_th, inner_th;
> +	unsigned int inner_l3min, outer_l3min;
> +	struct virtio_net_hdr_tunnel *tnl;
> +	u8 gso_inner_type;
> +	bool outer_isv6;
> +	int ret;
> +
> +	if (!gso_tunnel_type)
> +		return virtio_net_hdr_to_skb(skb, hdr, little_endian);
> +
> +	/* Tunnel not supported/negotiated, but the hdr asks for it. */
> +	if (!tnl_hdr_offset)
> +		return -EINVAL;
> +
> +	/* Either ipv4 or ipv6. */
> +	if (gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV4 &&
> +	    gso_tunnel_type & VIRTIO_NET_HDR_GSO_UDP_TUNNEL_IPV6)
> +		return -EINVAL;
> +
> +	/* No UDP fragments over UDP tunnel. */

What are udp fragments and why is TCP with ECN not supported?

> +	gso_inner_type = hdr->gso_type & ~(VIRTIO_NET_HDR_GSO_ECN |
> +					   gso_tunnel_type);
> +	if (!gso_inner_type || gso_inner_type == VIRTIO_NET_HDR_GSO_UDP)
> +		return -EINVAL;
> +
> +	/* Relay on csum being present. */

Rely

>  #endif /* _LINUX_VIRTIO_NET_H */
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 963540deae66a..1f1ff88a5749f 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -70,6 +70,28 @@
>  					 * with the same MAC.
>  					 */
>  #define VIRTIO_NET_F_SPEED_DUPLEX 63	/* Device set linkspeed and duplex */
> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO 65 /* Driver can receive
> +					      * GSO-over-UDP-tunnel packets
> +					      */
> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM 66 /* Driver handles
> +						   * GSO-over-UDP-tunnel
> +						   * packets with partial csum
> +						   * for the outer header
> +						   */
> +#define VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO 67 /* Device can receive
> +					     * GSO-over-UDP-tunnel packets
> +					     */
> +#define VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO_CSUM 68 /* Device handles
> +						  * GSO-over-UDP-tunnel
> +						  * packets with partial csum
> +						  * for the outer header
> +						  */
> +
> +/* Offloads bits corresponding to VIRTIO_NET_F_HOST_UDP_TUNNEL_GSO{,_CSUM}
> + * features
> + */
> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_MAPPED	46
> +#define VIRTIO_NET_F_GUEST_UDP_TUNNEL_GSO_CSUM_MAPPED	47

I don't quite follow this. These are not real virtio bits?