From: David Gwynne <david@gwynne.id.au>
Subject: Re: vport(4): Use VLAN HW Tagging
To: Jan Klemkow <jan@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 24 Jun 2025 15:27:16 +1000

On Mon, Jun 23, 2025 at 05:35:48PM +0200, Jan Klemkow wrote:
> Hi,
> 
> the following diff introduces VLAN offload to vport(4).  veb(4) just
> calls vlan_inject() if the underlying send interface it not capable of
> hw tagging.  So, we don't call it in most cases and avoid an mget() for
> every packet.
> 
> ok?

i'm not against the change, but it's not complete. there's a bunch of
filtering that veb can do based on the ethertype of the frame which will
now match against tagged frames when it shouldnt.

you had to set link0 to disable filtering of vlan and svlan tagged
packets in the first place...

> 
> bye,
> Jan
> 
> Index: net/if_veb.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_veb.c,v
> diff -u -p -r1.37 if_veb.c
> --- net/if_veb.c	2 Mar 2025 21:28:32 -0000	1.37
> +++ net/if_veb.c	23 Jun 2025 15:06:54 -0000
> @@ -927,6 +927,27 @@ veb_ipsec_out(struct ifnet *ifp0, struct
>  }
>  #endif /* IPSEC */
>  
> +static struct mbuf *
> +veb_offload(struct ifnet *ifp, struct mbuf *m)
> +{
> +#if NVLAN > 0
> +	if (ISSET(m->m_flags, M_VLANTAG) &&
> +	    !ISSET(ifp->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
> +		/*
> +		 * If the underlying interface has no VLAN hardware tagging
> +		 * support, inject one in software.
> +		 */
> +		m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
> +		if (m == NULL) {
> +			counters_inc(ifp->if_counters, ifc_ierrors);
> +			return NULL;
> +		}
> +	}
> +#endif
> +
> +	return m;
> +}
> +
>  static void
>  veb_broadcast(struct veb_softc *sc, struct veb_port *rp, struct mbuf *m0,
>      uint64_t src, uint64_t dst, struct netstack *ns)
> @@ -996,6 +1017,9 @@ veb_broadcast(struct veb_softc *sc, stru
>  		if (veb_rule_filter(tp, VEB_RULE_LIST_OUT, m0, src, dst))
>  			continue;
>  
> +		if ((m0 = veb_offload(ifp0, m0)) == NULL)
> +			goto done;
> +
>  		m = m_dup_pkt(m0, max_linkhdr + ETHER_ALIGN, M_NOWAIT);
>  		if (m == NULL) {
>  			/* XXX count error? */
> @@ -1048,6 +1072,9 @@ veb_transmit(struct veb_softc *sc, struc
>  	counters_pkt(ifp->if_counters, ifc_opackets, ifc_obytes,
>  	    m->m_pkthdr.len);
>  
> +	if ((m = veb_offload(ifp0, m)) == NULL)
> +		goto drop;
> +
>  	(*tp->p_enqueue)(ifp0, m); /* XXX count error */
>  
>  	return (NULL);
> @@ -1105,20 +1132,6 @@ veb_port_input(struct ifnet *ifp0, struc
>  		}
>  	}
>  
> -#if NVLAN > 0
> -	/*
> -	 * If the underlying interface removed the VLAN header itself,
> -	 * add it back.
> -	 */
> -	if (ISSET(m->m_flags, M_VLANTAG)) {
> -		m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
> -		if (m == NULL) {
> -			counters_inc(ifp->if_counters, ifc_ierrors);
> -			goto drop;
> -		}
> -	}
> -#endif
> -
>  	counters_pkt(ifp->if_counters, ifc_ipackets, ifc_ibytes,
>  	    m->m_pkthdr.len);
>  
> @@ -2349,6 +2362,7 @@ vport_clone_create(struct if_clone *ifc,
>  	ifp->if_qstart = vport_start;
>  	ifp->if_flags = IFF_BROADCAST | IFF_SIMPLEX | IFF_MULTICAST;
>  	ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
> +	ifp->if_capabilities = IFCAP_VLAN_HWTAGGING;
>  	ether_fakeaddr(ifp);
>  
>  	if_counters_alloc(ifp);
>