1 files changed, 124 insertions, 73 deletions

diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c
index 5b3ee740d75e..0a35fb4095fb 100644
--- a/sys/net/if_bridge.c
+++ b/sys/net/if_bridge.c
@@ -76,31 +76,34 @@
  *	  heterogeneous bridges).
  */
 
-#include <sys/cdefs.h>
 #include "opt_inet.h"
 #include "opt_inet6.h"
 
+#define	EXTERR_CATEGORY	EXTERR_CAT_BRIDGE
+
 #include <sys/param.h>
+#include <sys/ctype.h>  /* string functions */
 #include <sys/eventhandler.h>
-#include <sys/mbuf.h>
+#include <sys/exterrvar.h>
+#include <sys/jail.h>
+#include <sys/kernel.h>
+#include <sys/lock.h>
 #include <sys/malloc.h>
+#include <sys/mbuf.h>
+#include <sys/module.h>
+#include <sys/mutex.h>
+#include <sys/priv.h>
+#include <sys/proc.h>
 #include <sys/protosw.h>
+#include <sys/random.h>
 #include <sys/systm.h>
-#include <sys/jail.h>
-#include <sys/time.h>
 #include <sys/socket.h> /* for net/if.h */
 #include <sys/sockio.h>
-#include <sys/ctype.h>  /* string functions */
-#include <sys/kernel.h>
-#include <sys/random.h>
 #include <sys/syslog.h>
 #include <sys/sysctl.h>
+#include <sys/time.h>
+
 #include <vm/uma.h>
-#include <sys/module.h>
-#include <sys/priv.h>
-#include <sys/proc.h>
-#include <sys/lock.h>
-#include <sys/mutex.h>
 
 #include <net/bpf.h>
 #include <net/if.h>
@@ -254,8 +257,8 @@ struct bridge_iflist {
 	uint32_t		bif_addrcnt;	/* cur. # of addresses */
 	uint32_t		bif_addrexceeded;/* # of address violations */
 	struct epoch_context	bif_epoch_ctx;
-	ether_vlanid_t		bif_untagged;	/* untagged vlan id */
-	ifbvlan_set_t		bif_vlan_set;	/* allowed tagged vlans */
+	ether_vlanid_t		bif_pvid;	/* port vlan id */
+	ifbvlan_set_t		bif_vlan_set;	/* if allowed tagged vlans */
 };
 
 /*
@@ -404,7 +407,7 @@ static int	bridge_ioctl_sma(struct bridge_softc *, void *);
 static int	bridge_ioctl_sifprio(struct bridge_softc *, void *);
 static int	bridge_ioctl_sifcost(struct bridge_softc *, void *);
 static int	bridge_ioctl_sifmaxaddr(struct bridge_softc *, void *);
-static int	bridge_ioctl_sifuntagged(struct bridge_softc *, void *);
+static int	bridge_ioctl_sifpvid(struct bridge_softc *, void *);
 static int	bridge_ioctl_sifvlanset(struct bridge_softc *, void *);
 static int	bridge_ioctl_gifvlanset(struct bridge_softc *, void *);
 static int	bridge_ioctl_addspan(struct bridge_softc *, void *);
@@ -625,7 +628,7 @@ static const struct bridge_control bridge_control_table[] = {
 	{ bridge_ioctl_sifmaxaddr,	sizeof(struct ifbreq),
 	  BC_F_COPYIN|BC_F_SUSER },
 
-	{ bridge_ioctl_sifuntagged,	sizeof(struct ifbreq),
+	{ bridge_ioctl_sifpvid,		sizeof(struct ifbreq),
 	  BC_F_COPYIN|BC_F_SUSER },
 
 	{ bridge_ioctl_sifvlanset,	sizeof(struct ifbif_vlan_req),
@@ -986,31 +989,37 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 	case SIOCGDRVSPEC:
 	case SIOCSDRVSPEC:
 		if (ifd->ifd_cmd >= bridge_control_table_size) {
-			error = EINVAL;
+			error = EXTERROR(EINVAL, "Invalid control command");
 			break;
 		}
 		bc = &bridge_control_table[ifd->ifd_cmd];
 
 		if (cmd == SIOCGDRVSPEC &&
 		    (bc->bc_flags & BC_F_COPYOUT) == 0) {
-			error = EINVAL;
+			error = EXTERROR(EINVAL,
+			    "Inappropriate ioctl for command "
+			    "(expected SIOCSDRVSPEC)");
 			break;
 		}
 		else if (cmd == SIOCSDRVSPEC &&
 		    (bc->bc_flags & BC_F_COPYOUT) != 0) {
-			error = EINVAL;
+			error = EXTERROR(EINVAL,
+			    "Inappropriate ioctl for command "
+			    "(expected SIOCGDRVSPEC)");
 			break;
 		}
 
 		if (bc->bc_flags & BC_F_SUSER) {
 			error = priv_check(td, PRIV_NET_BRIDGE);
-			if (error)
+			if (error) {
+				EXTERROR(error, "PRIV_NET_BRIDGE required");
 				break;
+			}
 		}
 
 		if (ifd->ifd_len != bc->bc_argsize ||
 		    ifd->ifd_len > sizeof(args)) {
-			error = EINVAL;
+			error = EXTERROR(EINVAL, "Invalid argument size");
 			break;
 		}
 
@@ -1062,7 +1071,8 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 		oldmtu = sc->sc_ifp->if_mtu;
 
 		if (ifr->ifr_mtu < IF_MINMTU) {
-			error = EINVAL;
+			error = EXTERROR(EINVAL,
+			    "Requested MTU is lower than IF_MINMTU");
 			break;
 		}
 		if (CK_LIST_EMPTY(&sc->sc_iflist)) {
@@ -1088,6 +1098,8 @@ bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
 				(*bif->bif_ifp->if_ioctl)(bif->bif_ifp,
 				    SIOCSIFMTU, (caddr_t)ifr);
 			}
+			EXTERROR(error,
+			    "Failed to set MTU on member interface");
 		} else {
 			sc->sc_ifp->if_mtu = ifr->ifr_mtu;
 		}
@@ -1125,14 +1137,14 @@ bridge_mutecaps(struct bridge_softc *sc)
 	mask = BRIDGE_IFCAPS_MASK;
 
 	CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
-		/* Every member must support it or its disabled */
+		/* Every member must support it or it's disabled */
 		mask &= bif->bif_savedcaps;
 	}
 
 	CK_LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
 		enabled = bif->bif_ifp->if_capenable;
 		enabled &= ~BRIDGE_IFCAPS_STRIP;
-		/* strip off mask bits and enable them again if allowed */
+		/* Strip off mask bits and enable them again if allowed */
 		enabled &= ~BRIDGE_IFCAPS_MASK;
 		enabled |= mask;
 		bridge_set_ifcap(sc, bif, enabled);
@@ -1282,7 +1294,7 @@ bridge_delete_member(struct bridge_softc *sc, struct bridge_iflist *bif,
 #endif
 			break;
 		}
-		/* reneable any interface capabilities */
+		/* Re-enable any interface capabilities */
 		bridge_set_ifcap(sc, bif, bif->bif_savedcaps);
 	}
 	bstp_destroy(&bif->bif_stp);	/* prepare to free */
@@ -1318,21 +1330,48 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 
 	ifs = ifunit(req->ifbr_ifsname);
 	if (ifs == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "No such interface",
+		    req->ifbr_ifsname));
 	if (ifs->if_ioctl == NULL)	/* must be supported */
-		return (EINVAL);
+		return (EXTERROR(EINVAL, "Interface must support ioctl(2)"));
+
+	/*
+	 * If the new interface is a vlan(4), it could be a bridge SVI.
+	 * Don't allow such things to be added to bridges.
+	 */
+	if (ifs->if_type == IFT_L2VLAN) {
+		struct ifnet *parent;
+		struct epoch_tracker et;
+		bool is_bridge;
+
+		/*
+		 * Entering NET_EPOCH with BRIDGE_LOCK held, but this is okay
+		 * since we don't sleep here.
+		 */
+		NET_EPOCH_ENTER(et);
+		parent = VLAN_TRUNKDEV(ifs);
+		is_bridge = (parent != NULL && parent->if_type == IFT_BRIDGE);
+		NET_EPOCH_EXIT(et);
+
+		if (is_bridge)
+			return (EXTERROR(EINVAL,
+			    "Bridge SVI cannot be added to a bridge"));
+	}
 
 	/* If it's in the span list, it can't be a member. */
 	CK_LIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
 		if (ifs == bif->bif_ifp)
-			return (EBUSY);
+			return (EXTERROR(EBUSY,
+			    "Span interface cannot be a member"));
 
 	if (ifs->if_bridge) {
 		struct bridge_iflist *sbif = ifs->if_bridge;
 		if (sbif->bif_sc == sc)
-			return (EEXIST);
+			return (EXTERROR(EEXIST,
+			    "Interface is already a member of this bridge"));
 
-		return (EBUSY);
+		return (EXTERROR(EBUSY,
+		    "Interface is already a member of another bridge"));
 	}
 
 	switch (ifs->if_type) {
@@ -1342,7 +1381,7 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 		/* permitted interface types */
 		break;
 	default:
-		return (EINVAL);
+		return (EXTERROR(EINVAL, "Unsupported interface type"));
 	}
 
 #ifdef INET6
@@ -1394,11 +1433,15 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 		CK_STAILQ_FOREACH(ifa, &ifs->if_addrhead, ifa_link) {
 #ifdef INET
 			if (ifa->ifa_addr->sa_family == AF_INET)
-				return (EINVAL);
+				return (EXTERROR(EINVAL,
+				    "Member interface may not have "
+				    "an IPv4 address configured"));
 #endif
 #ifdef INET6
 			if (ifa->ifa_addr->sa_family == AF_INET6)
-				return (EINVAL);
+				return (EXTERROR(EINVAL,
+				    "Member interface may not have "
+				    "an IPv6 address configured"));
 #endif
 		}
 	}
@@ -1420,7 +1463,8 @@ bridge_ioctl_add(struct bridge_softc *sc, void *arg)
 			    " new member %s\n", sc->sc_ifp->if_xname,
 			    ifr.ifr_mtu,
 			    ifs->if_xname);
-			return (EINVAL);
+			return (EXTERROR(EINVAL,
+			    "Failed to set MTU on new member"));
 		}
 	}
 
@@ -1482,7 +1526,7 @@ bridge_ioctl_del(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	bridge_delete_member(sc, bif, 0);
 
@@ -1498,7 +1542,7 @@ bridge_ioctl_gifflags(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	bp = &bif->bif_stp;
 	req->ifbr_ifsflags = bif->bif_flags;
@@ -1512,7 +1556,7 @@ bridge_ioctl_gifflags(struct bridge_softc *sc, void *arg)
 	req->ifbr_addrcnt = bif->bif_addrcnt;
 	req->ifbr_addrmax = bif->bif_addrmax;
 	req->ifbr_addrexceeded = bif->bif_addrexceeded;
-	req->ifbr_untagged = bif->bif_untagged;
+	req->ifbr_pvid = bif->bif_pvid;
 
 	/* Copy STP state options as flags */
 	if (bp->bp_operedge)
@@ -1541,12 +1585,12 @@ bridge_ioctl_sifflags(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 	bp = &bif->bif_stp;
 
 	if (req->ifbr_ifsflags & IFBIF_SPAN)
 		/* SPAN is readonly */
-		return (EINVAL);
+		return (EXTERROR(EINVAL, "Span interface cannot be modified"));
 
 	NET_EPOCH_ENTER(et);
 
@@ -1555,7 +1599,8 @@ bridge_ioctl_sifflags(struct bridge_softc *sc, void *arg)
 			error = bstp_enable(&bif->bif_stp);
 			if (error) {
 				NET_EPOCH_EXIT(et);
-				return (error);
+				return (EXTERROR(error,
+				    "Failed to enable STP"));
 			}
 		}
 	} else {
@@ -1724,7 +1769,7 @@ bridge_ioctl_saddr(struct bridge_softc *sc, void *arg)
 	bif = bridge_lookup_member(sc, req->ifba_ifsname);
 	if (bif == NULL) {
 		NET_EPOCH_EXIT(et);
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 	}
 
 	/* bridge_rtupdate() may acquire the lock. */
@@ -1858,7 +1903,7 @@ bridge_ioctl_sifprio(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	return (bstp_set_port_priority(&bif->bif_stp, req->ifbr_priority));
 }
@@ -1871,7 +1916,7 @@ bridge_ioctl_sifcost(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	return (bstp_set_path_cost(&bif->bif_stp, req->ifbr_path_cost));
 }
@@ -1884,28 +1929,28 @@ bridge_ioctl_sifmaxaddr(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	bif->bif_addrmax = req->ifbr_addrmax;
 	return (0);
 }
 
 static int
-bridge_ioctl_sifuntagged(struct bridge_softc *sc, void *arg)
+bridge_ioctl_sifpvid(struct bridge_softc *sc, void *arg)
 {
 	struct ifbreq *req = arg;
 	struct bridge_iflist *bif;
 
 	bif = bridge_lookup_member(sc, req->ifbr_ifsname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
-	if (req->ifbr_untagged > DOT1Q_VID_MAX)
-		return (EINVAL);
+	if (req->ifbr_pvid > DOT1Q_VID_MAX)
+		return (EXTERROR(EINVAL, "Invalid VLAN ID"));
 
-	if (req->ifbr_untagged != DOT1Q_VID_NULL)
+	if (req->ifbr_pvid != DOT1Q_VID_NULL)
 		bif->bif_flags |= IFBIF_VLANFILTER;
-	bif->bif_untagged = req->ifbr_untagged;
+	bif->bif_pvid = req->ifbr_pvid;
 	return (0);
 }
 
@@ -1917,12 +1962,12 @@ bridge_ioctl_sifvlanset(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->bv_ifname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	/* Reject invalid VIDs. */
 	if (BRVLAN_TEST(&req->bv_set, DOT1Q_VID_NULL) ||
 	    BRVLAN_TEST(&req->bv_set, DOT1Q_VID_RSVD_IMPL))
-		return (EINVAL);
+		return (EXTERROR(EINVAL, "Invalid VLAN ID in set"));
 
 	switch (req->bv_op) {
 		/* Replace the existing vlan set with the new set */
@@ -1942,7 +1987,8 @@ bridge_ioctl_sifvlanset(struct bridge_softc *sc, void *arg)
 
 		/* Invalid or unknown operation */
 	default:
-		return (EINVAL);
+		return (EXTERROR(EINVAL,
+		    "Unsupported BRDGSIFVLANSET operation"));
 	}
 
 	/*
@@ -1962,7 +2008,7 @@ bridge_ioctl_gifvlanset(struct bridge_softc *sc, void *arg)
 
 	bif = bridge_lookup_member(sc, req->bv_ifname);
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a bridge member"));
 
 	BIT_COPY(BRVLAN_SETSIZE, &bif->bif_vlan_set, &req->bv_set);
 	return (0);
@@ -1977,14 +2023,16 @@ bridge_ioctl_addspan(struct bridge_softc *sc, void *arg)
 
 	ifs = ifunit(req->ifbr_ifsname);
 	if (ifs == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "No such interface"));
 
 	CK_LIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
 		if (ifs == bif->bif_ifp)
-			return (EBUSY);
+			return (EXTERROR(EBUSY,
+			    "Interface is already a span port"));
 
 	if (ifs->if_bridge != NULL)
-		return (EBUSY);
+		return (EXTERROR(EEXIST,
+		    "Interface is already a bridge member"));
 
 	switch (ifs->if_type) {
 		case IFT_ETHER:
@@ -1992,7 +2040,7 @@ bridge_ioctl_addspan(struct bridge_softc *sc, void *arg)
 		case IFT_L2VLAN:
 			break;
 		default:
-			return (EINVAL);
+			return (EXTERROR(EINVAL, "Unsupported interface type"));
 	}
 
 	bif = malloc(sizeof(*bif), M_DEVBUF, M_NOWAIT|M_ZERO);
@@ -2016,14 +2064,14 @@ bridge_ioctl_delspan(struct bridge_softc *sc, void *arg)
 
 	ifs = ifunit(req->ifbr_ifsname);
 	if (ifs == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "No such interface"));
 
 	CK_LIST_FOREACH(bif, &sc->sc_spanlist, bif_next)
 		if (ifs == bif->bif_ifp)
 			break;
 
 	if (bif == NULL)
-		return (ENOENT);
+		return (EXTERROR(ENOENT, "Interface is not a span port"));
 
 	bridge_delete_span(sc, bif);
 
@@ -2278,8 +2326,8 @@ bridge_enqueue(struct bridge_softc *sc, struct ifnet *dst_ifp, struct mbuf *m,
 		 * the VLAN header.
 		 */
 		if ((bif->bif_flags & IFBIF_VLANFILTER) &&
-		    bif->bif_untagged != DOT1Q_VID_NULL &&
-		    VLANTAGOF(m) == bif->bif_untagged) {
+		    bif->bif_pvid != DOT1Q_VID_NULL &&
+		    VLANTAGOF(m) == bif->bif_pvid) {
 			m->m_flags &= ~M_VLANTAG;
 			m->m_pkthdr.ether_vtag = 0;
 		}
@@ -3145,14 +3193,14 @@ bridge_vfilter_in(const struct bridge_iflist *sbif, struct mbuf *m)
 		 * The frame doesn't have a tag.  If the interface does not
 		 * have an untagged vlan configured, drop the frame.
 		 */
-		if (sbif->bif_untagged == DOT1Q_VID_NULL)
+		if (sbif->bif_pvid == DOT1Q_VID_NULL)
 			return (false);
 
 		/*
 		 * Otherwise, insert a new tag based on the interface's
 		 * untagged vlan id.
 		 */
-		m->m_pkthdr.ether_vtag = sbif->bif_untagged;
+		m->m_pkthdr.ether_vtag = sbif->bif_pvid;
 		m->m_flags |= M_VLANTAG;
 	} else {
 		/*
@@ -3213,7 +3261,7 @@ bridge_vfilter_out(const struct bridge_iflist *dbif, const struct mbuf *m)
 	 * If the frame's vlan matches the interfaces's untagged vlan,
 	 * allow it.
 	 */
-	if (vlan == dbif->bif_untagged)
+	if (vlan == dbif->bif_pvid)
 		return (true);
 
 	/*
@@ -3244,10 +3292,11 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t *dst,
 	BRIDGE_LOCK_OR_NET_EPOCH_ASSERT(sc);
 
 	/* Check the source address is valid and not multicast. */
-	if (ETHER_IS_MULTICAST(dst) ||
-	    (dst[0] == 0 && dst[1] == 0 && dst[2] == 0 &&
-	     dst[3] == 0 && dst[4] == 0 && dst[5] == 0) != 0)
-		return (EINVAL);
+	if (ETHER_IS_MULTICAST(dst))
+		return (EXTERROR(EINVAL, "Multicast address not permitted"));
+	if (dst[0] == 0 && dst[1] == 0 && dst[2] == 0 &&
+	    dst[3] == 0 && dst[4] == 0 && dst[5] == 0)
+		return (EXTERROR(EINVAL, "Zero address not permitted"));
 
 	/*
 	 * A route for this destination might already exist.  If so,
@@ -3266,13 +3315,14 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t *dst,
 		if (sc->sc_brtcnt >= sc->sc_brtmax) {
 			sc->sc_brtexceeded++;
 			BRIDGE_RT_UNLOCK(sc);
-			return (ENOSPC);
+			return (EXTERROR(ENOSPC, "Address table is full"));
 		}
 		/* Check per interface address limits (if enabled) */
 		if (bif->bif_addrmax && bif->bif_addrcnt >= bif->bif_addrmax) {
 			bif->bif_addrexceeded++;
 			BRIDGE_RT_UNLOCK(sc);
-			return (ENOSPC);
+			return (EXTERROR(ENOSPC,
+			    "Interface address limit exceeded"));
 		}
 
 		/*
@@ -3283,7 +3333,8 @@ bridge_rtupdate(struct bridge_softc *sc, const uint8_t *dst,
 		brt = uma_zalloc(V_bridge_rtnode_zone, M_NOWAIT | M_ZERO);
 		if (brt == NULL) {
 			BRIDGE_RT_UNLOCK(sc);
-			return (ENOMEM);
+			return (EXTERROR(ENOMEM,
+			    "Cannot allocate address node"));
 		}
 		brt->brt_vnet = curvnet;
 
@@ -3631,7 +3682,7 @@ bridge_rtnode_insert(struct bridge_softc *sc, struct bridge_rtnode *brt)
 	do {
 		dir = bridge_rtnode_addr_cmp(brt->brt_addr, lbrt->brt_addr);
 		if (dir == 0 && brt->brt_vlan == lbrt->brt_vlan)
-			return (EEXIST);
+			return (EXTERROR(EEXIST, "Address already exists"));
 		if (dir > 0) {
 			CK_LIST_INSERT_BEFORE(lbrt, brt, brt_hash);
 			goto out;