Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: bridge(4): use vlan hardware tagging
To:
Timo Mirau <tmirau@genua.de>
Cc:
Jan Klemkow <jan@openbsd.org>, tech@openbsd.org
Date:
Mon, 15 Sep 2025 23:58:48 +0200

Download raw body.

Thread
On Thu, Aug 14, 2025 at 12:20:53PM +0200, Timo Mirau wrote:
> Hello Jan,
> 
> Am Mon, Aug 04, 2025 at 12:02:30PM +0200 schrieb Jan Klemkow:
> > This diff adds the use of VLAN hardware tagging in bridge(4) and
> > vether(4) similar to veb(4).
> 
> I've had a look at your diff. I simplified it by calling bridge_offload
> at the beginning of bridge_ifenqueue. I've tested this diff with vlan(4),
> vether(4) and ix(4) interfaces. I disabled the IFCAP_VLAN_HWTAGGING flag
> in the ix(4) source code to test the vlan inject call path. 
> It worked as expected in my setup.

I have tested this diff with combinations of bridge, vlan, vether.
All my tcpbench tests passed.

I wonder if there is any other use case for vether without bridge
where the vlan offloading could cause trouble.

OK bluhm@

> Index: net/if_bridge.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_bridge.c,v
> diff -u -p -r1.376 if_bridge.c
> --- net/if_bridge.c	7 Jul 2025 02:28:50 -0000	1.376
> +++ net/if_bridge.c	13 Aug 2025 12:44:19 -0000
> @@ -798,6 +798,24 @@ bridge_stop(struct bridge_softc *sc)
>  	bridge_rtflush(sc, IFBF_FLUSHDYN);
>  }
>  
> +struct mbuf *
> +bridge_offload(struct ifnet *ifp, struct mbuf *m)
> +{
> +#if NVLAN > 0
> +	/*
> +	 * If the underlying interface has no VLAN hardware tagging support,
> +	 * inject one in software.
> +	 */
> +	if (ISSET(m->m_flags, M_VLANTAG) &&
> +	    !ISSET(ifp->if_capabilities, IFCAP_VLAN_HWTAGGING)) {
> +		m = vlan_inject(m, ETHERTYPE_VLAN, m->m_pkthdr.ether_vtag);
> +		if (m == NULL)
> +			return NULL;
> +	}
> +#endif
> +	return m;
> +}
> +
>  /*
>   * Send output from the bridge.  The mbuf has the ethernet header
>   * already attached.  We must enqueue or free the mbuf before exiting.
> @@ -1170,18 +1188,6 @@ bridge_process(struct ifnet *ifp, struct
>  	if (m->m_pkthdr.len < sizeof(*eh))
>  		goto bad;
>  
> -#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)
> -			goto bad;
> -	}
> -#endif
> -
>  #if NBPFILTER > 0
>  	if_bpf = brifp->if_bpf;
>  	if (if_bpf)
> @@ -1915,21 +1921,29 @@ bridge_ifenqueue(struct ifnet *brifp, st
>  {
>  	int error, len;
>  
> +	if ((m = bridge_offload(ifp, m)) == NULL) {
> +		error = ENOBUFS;
> +		goto err;
> +	}
> +
>  	/* Loop prevention. */
>  	m->m_flags |= M_PROTO1;
>  
>  	len = m->m_pkthdr.len;
>  
>  	error = if_enqueue(ifp, m);
> -	if (error) {
> -		brifp->if_oerrors++;
> -		return (error);
> -	}
> +	if (error) 
> +		goto err; 
> +	
>  
>  	brifp->if_opackets++;
>  	brifp->if_obytes += len;
>  
>  	return (0);
> +
> + err:
> +	brifp->if_oerrors++;
> +	return (error);
>  }
>  
>  void
> Index: net/if_vether.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_vether.c,v
> diff -u -p -r1.37 if_vether.c
> --- net/if_vether.c	7 Jul 2025 02:28:50 -0000	1.37
> +++ net/if_vether.c	8 Aug 2025 14:11:52 -0000
> @@ -27,6 +27,7 @@
>  #include <netinet/in.h>
>  #include <netinet/if_ether.h>
>  
> +#include "vlan.h"
>  #include "bpfilter.h"
>  #if NBPFILTER > 0
>  #include <net/bpf.h>
> @@ -85,6 +86,9 @@ vether_clone_create(struct if_clone *ifc
>  
>  	ifp->if_hardmtu = ETHER_MAX_HARDMTU_LEN;
>  	ifp->if_capabilities = IFCAP_VLAN_MTU;
> +#if NVLAN > 0
> +	ifp->if_capabilities |= IFCAP_VLAN_HWTAGGING;
> +#endif
>  	ifp->if_xflags = IFXF_CLONED | IFXF_MPSAFE;
>  
>  	ifmedia_init(&sc->sc_media, 0, vether_media_change,