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

On Tue, Jun 24, 2025 at 03:27:16PM GMT, David Gwynne wrote:
> On Mon, Jun 23, 2025 at 05:35:48PM +0200, Jan Klemkow wrote:
> > 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.

Yes. That confused me, while reading this code.  So, I get it right,
the use of VLANs disables all filters of a veb(4)?

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

The following diff adds additional checks for mbuf VLAN flags.

ok?

Thanks,
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	24 Jun 2025 07:45:30 -0000
@@ -458,6 +458,9 @@ veb_ip_filter(const struct mbuf *m)
 {
 	const struct ether_header *eh;
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (1);
+
 	eh = mtod(m, struct ether_header *);
 	switch (ntohs(eh->ether_type)) {
 	case ETHERTYPE_IP:
@@ -477,6 +480,9 @@ veb_vlan_filter(const struct mbuf *m)
 {
 	const struct ether_header *eh;
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (1);
+
 	eh = mtod(m, struct ether_header *);
 	switch (ntohs(eh->ether_type)) {
 	case ETHERTYPE_VLAN:
@@ -495,6 +501,9 @@ veb_rule_arp_match(const struct veb_rule
 	struct ether_header *eh;
 	struct ether_arp ea;
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (0);
+
 	eh = mtod(m, struct ether_header *);
 
 	if (eh->ether_type != htons(ETHERTYPE_ARP))
@@ -625,6 +634,9 @@ veb_pf(struct ifnet *ifp0, int dir, stru
 	if (ifp0->if_enqueue == vport_enqueue)
 		return (m);
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (m);
+
 	eh = mtod(m, struct ether_header *);
 	switch (ntohs(eh->ether_type)) {
 	case ETHERTYPE_IP:
@@ -793,6 +805,9 @@ veb_ipsec_in(struct ifnet *ifp0, struct 
 	if (ifp0->if_enqueue == vport_enqueue)
 		return (m);
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (m);
+
 	eh = mtod(m, struct ether_header *);
 	switch (ntohs(eh->ether_type)) {
 	case ETHERTYPE_IP:
@@ -895,6 +910,9 @@ veb_ipsec_out(struct ifnet *ifp0, struct
 	if (ifp0->if_enqueue == vport_enqueue)
 		return (m);
 
+	if (ISSET(m->m_flags, M_VLANTAG))
+		return (m);
+
 	eh = mtod(m, struct ether_header *);
 	switch (ntohs(eh->ether_type)) {
 	case ETHERTYPE_IP:
@@ -927,6 +945,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 +1035,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 +1090,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 +1150,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 +2380,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);