move away from using the M_PROTO1 flag to prevent loops with vports

if a vlan interface is configured on a vport interface, vlan(4)
will take the packet away from ether_input before the veb bridge
input handler gets to clear M_PROTO1. this leaves the flag on the
mbuf as it goes through the l3 stacks. if it goes back out a vport
into a veb, the presence of M_PROTO1 means the packet ends up getting
dropped, which is unexpected.

this diff specialises vport handling by veb even more to avoid the
problem the flag was handling. vports get their own bridge input
handler that skips veb processing completely because a packet being
received on a vport can only occur if a veb has decided to forward
it there and has already processed it. when the stack sends a packet
out a vport interface, then we do actual veb bridge input handling.

bug reported on misc@ and the fix tested by Simon Baker

1 files changed, 47 insertions, 24 deletions

diff --git a/sys/net/if_veb.c b/sys/net/if_veb.c
index 492edc858b1..24df521ddd0 100644
--- a/sys/net/if_veb.c
+++ b/sys/net/if_veb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_veb.c,v 1.21 2021/11/08 04:15:46 dlg Exp $ */
+/*	$OpenBSD: if_veb.c,v 1.22 2021/12/28 23:10:30 dlg Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -117,6 +117,8 @@ struct veb_port {
 	struct ifnet			*p_ifp0;
 	struct refcnt			 p_refs;
 
+	int (*p_enqueue)(struct ifnet *, struct mbuf *);
+
 	int (*p_ioctl)(struct ifnet *, u_long, caddr_t);
 	int (*p_output)(struct ifnet *, struct mbuf *, struct sockaddr *,
 	    struct rtentry *);
@@ -233,6 +235,8 @@ struct vport_softc {
 	unsigned int		 sc_dead;
 };
 
+static int	vport_if_enqueue(struct ifnet *, struct mbuf *);
+
 static int	vport_ioctl(struct ifnet *, u_long, caddr_t);
 static int	vport_enqueue(struct ifnet *, struct mbuf *);
 static void	vport_start(struct ifqueue *);
@@ -901,7 +905,7 @@ veb_broadcast(struct veb_softc *sc, struct veb_port *rp, struct mbuf *m0,
 			continue;
 		}
 
-		if_enqueue(ifp0, m); /* XXX count error? */
+		(*tp->p_enqueue)(ifp0, m); /* XXX count error */
 	}
 	smr_read_leave();
 
@@ -946,7 +950,7 @@ veb_transmit(struct veb_softc *sc, struct veb_port *rp, struct veb_port *tp,
 	counters_pkt(ifp->if_counters, ifc_opackets, ifc_obytes,
 	    m->m_pkthdr.len);
 
-	if_enqueue(ifp0, m); /* XXX count error? */
+	(*tp->p_enqueue)(ifp0, m); /* XXX count error */
 
 	return (NULL);
 drop:
@@ -955,6 +959,12 @@ drop:
 }
 
 static struct mbuf *
+veb_vport_input(struct ifnet *ifp0, struct mbuf *m, uint64_t dst, void *brport)
+{
+	return (m);
+}
+
+static struct mbuf *
 veb_port_input(struct ifnet *ifp0, struct mbuf *m, uint64_t dst, void *brport)
 {
 	struct veb_port *p = brport;
@@ -966,11 +976,6 @@ veb_port_input(struct ifnet *ifp0, struct mbuf *m, uint64_t dst, void *brport)
 	caddr_t if_bpf;
 #endif
 
-	if (ISSET(m->m_flags, M_PROTO1)) {
-		CLR(m->m_flags, M_PROTO1);
-		return (m);
-	}
-
 	if (!ISSET(ifp->if_flags, IFF_RUNNING))
 		return (m);
 
@@ -1059,7 +1064,6 @@ veb_port_input(struct ifnet *ifp0, struct mbuf *m, uint64_t dst, void *brport)
 		etherbridge_map(&sc->sc_eb, p, src);
 
 	CLR(m->m_flags, M_BCAST|M_MCAST);
-	SET(m->m_flags, M_PROTO1);
 
 	if (!ETH64_IS_MULTICAST(dst)) {
 		struct veb_port *tp = NULL;
@@ -1245,6 +1249,7 @@ veb_add_port(struct veb_softc *sc, const struct ifbreq *req, unsigned int span)
 	struct ifnet *ifp0;
 	struct veb_ports *port_list;
 	struct veb_port *p;
+	int isvport;
 	int error;
 
 	NET_ASSERT_LOCKED();
@@ -1263,6 +1268,8 @@ veb_add_port(struct veb_softc *sc, const struct ifbreq *req, unsigned int span)
 		goto put;
 	}
 
+	isvport = (ifp0->if_enqueue == vport_enqueue);
+
 	error = ether_brport_isset(ifp0);
 	if (error != 0)
 		goto put;
@@ -1283,6 +1290,7 @@ veb_add_port(struct veb_softc *sc, const struct ifbreq *req, unsigned int span)
 	SMR_TAILQ_INIT(&p->p_vr_list[0]);
 	SMR_TAILQ_INIT(&p->p_vr_list[1]);
 
+	p->p_enqueue = isvport ? vport_if_enqueue : if_enqueue;
 	p->p_ioctl = ifp0->if_ioctl;
 	p->p_output = ifp0->if_output;
 
@@ -1299,7 +1307,8 @@ veb_add_port(struct veb_softc *sc, const struct ifbreq *req, unsigned int span)
 			goto free;
 
 		p->p_bif_flags = IFBIF_LEARNING | IFBIF_DISCOVER;
-		p->p_brport.eb_input = veb_port_input;
+		p->p_brport.eb_input = isvport ?
+		    veb_vport_input : veb_port_input;
 	}
 
 	p->p_brport.eb_port_take = veb_eb_brport_take;
@@ -1323,7 +1332,7 @@ veb_add_port(struct veb_softc *sc, const struct ifbreq *req, unsigned int span)
 	port_list->l_count++;
 
 	ether_brport_set(ifp0, &p->p_brport);
-	if (ifp0->if_enqueue != vport_enqueue) { /* vport is special */
+	if (!isvport) { /* vport is special */
 		ifp0->if_ioctl = veb_p_ioctl;
 		ifp0->if_output = veb_p_output;
 	}
@@ -2176,6 +2185,20 @@ vport_down(struct vport_softc *sc)
 }
 
 static int
+vport_if_enqueue(struct ifnet *ifp, struct mbuf *m)
+{
+	/*
+	 * switching an l2 packet toward a vport means pushing it
+	 * into the network stack. this function exists to make
+	 * if_vinput compat with veb calling if_enqueue.
+	 */
+
+	if_vinput(ifp, m);
+
+	return (0);
+}
+
+static int
 vport_enqueue(struct ifnet *ifp, struct mbuf *m)
 {
 	struct arpcom *ac;
@@ -2185,24 +2208,19 @@ vport_enqueue(struct ifnet *ifp, struct mbuf *m)
 	caddr_t if_bpf;
 #endif
 
+	/*
+	 * a packet sent from the l3 stack out a vport goes into
+	 * veb for switching out another port.
+	 */
+
 #if NPF > 0
 	/*
-	 * the packet is about to leave the l3 stack and go into
-	 * the l2 switching space, or it's coming from a switch space
-	 * into the network stack. either way, there's no relationship
-	 * between pf states in those different places.
+	 * there's no relationship between pf states in the l3 stack
+	 * and the l2 bridge.
 	 */
 	pf_pkt_addr_changed(m);
 #endif
 
-	if (ISSET(m->m_flags, M_PROTO1)) {
-		/* packet is coming from a bridge */
-		if_vinput(ifp, m);
-		return (0);
-	}
-
-	/* packet is going to the bridge */
-
 	ac = (struct arpcom *)ifp;
 
 	smr_read_enter();
@@ -2211,6 +2229,8 @@ vport_enqueue(struct ifnet *ifp, struct mbuf *m)
 		eb->eb_port_take(eb->eb_port);
 	smr_read_leave();
 	if (eb != NULL) {
+		struct mbuf *(*input)(struct ifnet *, struct mbuf *,
+		    uint64_t, void *) = eb->eb_input;
 		struct ether_header *eh;
 		uint64_t dst;
 
@@ -2225,7 +2245,10 @@ vport_enqueue(struct ifnet *ifp, struct mbuf *m)
 
 		eh = mtod(m, struct ether_header *);
 		dst = ether_addr_to_e64((struct ether_addr *)eh->ether_dhost);
-		m = (*eb->eb_input)(ifp, m, dst, eb->eb_port);
+
+		if (input == veb_vport_input)
+			input = veb_port_input;
+		m = (*input)(ifp, m, dst, eb->eb_port);
 
 		error = 0;