From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: ixl(4): add ICAP_VLAN_MTU and NVLAN checks
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 29 Oct 2024 20:50:40 +0100

On Fri, Oct 25, 2024 at 10:30:04AM +0200, Jan Klemkow wrote:
> Hi,
> 
> ixl(4) interfaces can handle extra vlan tag bytes in hardware.
> While here also add missing NVLAN checks for vlan code.
> 
> ok?

I have tested it with and without vlan.  Works.

Please change the diff as below, IFCAP_VLAN_MTU must also be set
if NVLAN is not configured.

-     ifp->if_capabilities = IFCAP_VLAN_HWTAGGING;
+     ifp->if_capabilities = IFCAP_VLAN_MTU; 
+#if NVLAN > 0
+     ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
+#endif

I encountered the path MTU problem on a production machine where
etherip(4) was bridged to ixl(4).  No matter whether we have vlan(4)
support, bridge(4) must handle packets with vlan header correctly.

with that OK bluhm@

> Index: dev/pci/if_ixl.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_ixl.c,v
> diff -u -p -r1.101 if_ixl.c
> --- dev/pci/if_ixl.c	24 May 2024 06:02:53 -0000	1.101
> +++ dev/pci/if_ixl.c	25 Oct 2024 00:02:28 -0000
> @@ -49,6 +49,7 @@
>  
>  #include "bpfilter.h"
>  #include "kstat.h"
> +#include "vlan.h"
>  
>  #include <sys/param.h>
>  #include <sys/systm.h>
> @@ -1966,7 +1967,11 @@ ixl_attach(struct device *parent, struct
>  	strlcpy(ifp->if_xname, DEVNAME(sc), IFNAMSIZ);
>  	ifq_init_maxlen(&ifp->if_snd, sc->sc_tx_ring_ndescs);
>  
> -	ifp->if_capabilities = IFCAP_VLAN_HWTAGGING;
> +	ifp->if_capabilities = 0;
> +#if NVLAN > 0
> +	ifp->if_capabilities |= IFCAP_VLAN_MTU;
> +	ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
> +#endif
>  	ifp->if_capabilities |= IFCAP_CSUM_IPv4 |
>  	    IFCAP_CSUM_TCPv4 | IFCAP_CSUM_UDPv4 |
>  	    IFCAP_CSUM_TCPv6 | IFCAP_CSUM_UDPv6;
> @@ -2806,11 +2811,13 @@ ixl_tx_setup_offload(struct mbuf *m0, st
>  	uint64_t hlen;
>  	uint64_t offload = 0;
>  
> +#if NVLAN > 0
>  	if (ISSET(m0->m_flags, M_VLANTAG)) {
>  		uint64_t vtag = m0->m_pkthdr.ether_vtag;
>  		offload |= IXL_TX_DESC_CMD_IL2TAG1;
>  		offload |= vtag << IXL_TX_DESC_L2TAG1_SHIFT;
>  	}
> +#endif
>  
>  	if (!ISSET(m0->m_pkthdr.csum_flags,
>  	    M_IPV4_CSUM_OUT|M_TCP_CSUM_OUT|M_UDP_CSUM_OUT|M_TCP_TSO))
> @@ -3309,11 +3316,13 @@ ixl_rxeof(struct ixl_softc *sc, struct i
>  					m->m_pkthdr.csum_flags |= M_FLOWID;
>  				}
>  
> +#if NVLAN > 0
>  				if (ISSET(word, IXL_RX_DESC_L2TAG1P)) {
>  					m->m_pkthdr.ether_vtag =
>  					    lemtoh16(&rxd->l2tag1);
>  					SET(m->m_flags, M_VLANTAG);
>  				}
> +#endif
>  
>  				ixl_rx_checksum(m, word);
>  				ml_enqueue(&ml, m);