From: sf@openbsd.org
Subject: Re: vio(4): use hardware offload with vlan(4)
To: Jan Klemkow <j.klemkow@wemelug.de>
Cc: tech@openbsd.org
Date: Thu, 6 Jun 2024 08:10:58 +0200

No detailed review, but one comment below

On Tue, 4 Jun 2024, Jan Klemkow wrote:

> Hi,
> 
> In general hardware devices handle VLAN Tags and checksum/segmentation
> offloading together with VLAN offloading.  But, some devices like
> vio(4), do no provide the capability of assisted VLAN tag injection.
> 
> The following diff introduces a new IFCAP flag.  So, vlan(4) injects an
> inline VLAN Tag, but also inherit the offloading capabilities of this
> parent device.
> 
> bye,
> Jan
> 
> Index: sbin/ifconfig/ifconfig.8
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> diff -u -p -r1.399 ifconfig.8
> --- sbin/ifconfig/ifconfig.8	11 Jan 2024 17:22:04 -0000	1.399
> +++ sbin/ifconfig/ifconfig.8	4 Jun 2024 21:22:11 -0000
> @@ -294,6 +294,9 @@ tag.
>  On transmit, the device can add the
>  .Xr vlan 4
>  tag.
> +.It Sy VLAN_HWOFFLOAD
> +On transmit, the device can handle checksum or TSO offload without
> +.Sy VLAN_HWTAGGING .
>  .It Sy WOL
>  The device supports Wake on LAN (WoL).
>  .It Sy hardmtu
> Index: sbin/ifconfig/ifconfig.c
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> diff -u -p -r1.472 ifconfig.c
> --- sbin/ifconfig/ifconfig.c	18 May 2024 02:44:22 -0000	1.472
> +++ sbin/ifconfig/ifconfig.c	4 Jun 2024 21:22:11 -0000
> @@ -125,7 +125,7 @@
>  
>  #define HWFEATURESBITS							\
>  	"\024\1CSUM_IPv4\2CSUM_TCPv4\3CSUM_UDPv4"			\
> -	"\5VLAN_MTU\6VLAN_HWTAGGING\10CSUM_TCPv6"			\
> +	"\5VLAN_MTU\6VLAN_HWTAGGING\7VLAN_HWOFFLOAD\10CSUM_TCPv6"	\
>  	"\11CSUM_UDPv6\15TSOv4\16TSOv6\17LRO\20WOL"
>  
>  struct ifencap {
> Index: sys/dev/pv/if_vio.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pv/if_vio.c,v
> diff -u -p -r1.37 if_vio.c
> --- sys/dev/pv/if_vio.c	4 Jun 2024 09:51:52 -0000	1.37
> +++ sys/dev/pv/if_vio.c	4 Jun 2024 21:22:11 -0000
> @@ -605,6 +605,7 @@ vio_attach(struct device *parent, struct
>  	ifp->if_start = vio_start;
>  	ifp->if_ioctl = vio_ioctl;
>  	ifp->if_capabilities = IFCAP_VLAN_MTU;
> +	ifp->if_capabilities = IFCAP_VLAN_HWOFFLOAD;

You meant |= here?

>  	if (virtio_has_feature(vsc, VIRTIO_NET_F_CSUM))
>  		ifp->if_capabilities |= IFCAP_CSUM_TCPv4|IFCAP_CSUM_UDPv4|
>  		    IFCAP_CSUM_TCPv6|IFCAP_CSUM_UDPv6;
> Index: sys/net/if.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if.h,v
> diff -u -p -r1.216 if.h
> --- sys/net/if.h	11 Apr 2024 15:08:18 -0000	1.216
> +++ sys/net/if.h	4 Jun 2024 21:22:11 -0000
> @@ -249,6 +249,7 @@ struct if_status_description {
>  #define	IFCAP_CSUM_UDPv4	0x00000004	/* can do IPv4/UDP csum */
>  #define	IFCAP_VLAN_MTU		0x00000010	/* VLAN-compatible MTU */
>  #define	IFCAP_VLAN_HWTAGGING	0x00000020	/* hardware VLAN tag support */
> +#define	IFCAP_VLAN_HWOFFLOAD	0x00000040	/* hw offload w/ inline tag */
>  #define	IFCAP_CSUM_TCPv6	0x00000080	/* can do IPv6/TCP checksums */
>  #define	IFCAP_CSUM_UDPv6	0x00000100	/* can do IPv6/UDP checksums */
>  #define	IFCAP_TSOv4		0x00001000	/* IPv4/TCP segment offload */
> Index: sys/net/if_vlan.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vlan.c,v
> diff -u -p -r1.218 if_vlan.c
> --- sys/net/if_vlan.c	23 Dec 2023 10:52:54 -0000	1.218
> +++ sys/net/if_vlan.c	4 Jun 2024 21:22:11 -0000
> @@ -523,7 +523,7 @@ vlan_up(struct vlan_softc *sc)
>  	/*
>  	 * Note: In cases like vio(4) and em(4) where the offsets of the
>  	 * csum can be freely defined, we could actually do csum offload
> -	 * for VLAN and QINQ packets.
> +	 * for QINQ packets.
>  	 */
>  	if (sc->sc_type != ETHERTYPE_VLAN) {
>  		/*
> @@ -531,10 +531,14 @@ vlan_up(struct vlan_softc *sc)
>  		 * ethernet type (0x8100).
>  		 */
>  		ifp->if_capabilities = 0;
> -	} else if (ISSET(ifp0->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
> +	} else if (ISSET(ifp0->if_capabilities, IFCAP_VLAN_HWTAGGING) ||
> +	    ISSET(ifp0->if_capabilities, IFCAP_VLAN_HWOFFLOAD)) {
>  		/*
>  		 * Chips that can do hardware-assisted VLAN encapsulation, can
>  		 * calculate the correct checksum for VLAN tagged packets.
> +		 *
> +		 * Hardware which does checksum offloading, but not VLAN tag
> +		 * injection, have to set IFCAP_VLAN_HWOFFLOAD.
>  		 */
>  		ifp->if_capabilities = ifp0->if_capabilities &
>  		    (IFCAP_CSUM_MASK | IFCAP_TSOv4 | IFCAP_TSOv6);
>