Download raw body.
vport(4): Use VLAN HW Tagging
On Tue, Jun 24, 2025 at 10:33:43AM +0200, Jan Klemkow wrote:
> 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)?
i think the short answer to your question is: yes.
a vlan tag means the traffic belongs to a different ethernet network.
veb(4) is technically vlan-unaware, meaning it doesn't understand or
maintain vlan tags on packets going over it. by default it drops them,
but as you know you can enable the link0 flag to allow tagged packets to
traverse the bridge.
however, tagged traffic still belongs to a different network.
the other filtering options you can configure on veb(4), such as
pf or the bridge rules, are similarly vlan unaware and do not process
tagged packets.
if you do want to filter tagged packets, then create vlan(4)
interfaces on the ports and add them to their own veb(4).
link0 is an easy button to press that allows you to carry vlan
tagged packets around and switch them based on mac address. technically
the mac addresses in the fib should be scoped by the tag, but i haven't
had the energy to try and implement that yet.
> > 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?
hmm. i just spotted something in veb_offload. once that's fixed you can
put it in.
> 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)
can you call this ifp0 here to keep it consistent with the variable
names used in the rest of the driver please?
> +{
> +#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);
the previous use of vlan_inject would count this error on the veb
interface, but this is adding it to the port interface. you dont know if
a port has per-cpu counters, so this could be a segfault.
probably easiest to pass the veb ifp in and keep counting this error
against it.
> + 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);
vport(4): Use VLAN HW Tagging