--fdj2RfSjLxBAspz7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Mar 14, 2003 at 06:57:48PM +0100, Manuel Bouyer wrote:
> Hi,
> there is currently a problem with promiscous mode and vlan(4) interfaces.
> When a vlan interface is put in promiscous ode, the underlying inerface
> is as well. But the packets for others hosts never get delivered to the
> vlan interface, because they are filtered too early in ether_input().
> The main problem is that this breaks bridges on top of vlans.
> 
> I see two ways of fixing this:
> a) deliver the packet to vlans devices before the (ifp->if_flags & IFF_PROMISC)
>    check. The bad effect is that we'll have to do this check again in
>    vlan_input, and we'll test etype one more time.
> b) add a new M_PROMISC mbuf flag, and set it in the (ifp->if_flags &
>    IFF_PROMISC) check in ether_input instead of dropping the packet
>    if it's not for us. We'll drop it later if approriate, either in
>    vlan_input() or in the others switch (etype) cases.
>    This may also allow to fix the XXX comment before
>    (ifp->if_flags & IFF_PROMISC) in ether_input() (I didn't look at this
>    yet).

I've gone with solution b), but using one of the M_LINK flag instead of
burning a new one (there's only one byte left in m_flags).
See attached patch
comments ?

--
Manuel Bouyer, LIP6, Universite Paris VI.           Manuel.Bouyer@lip6.fr
     NetBSD: 24 ans d'experience feront toujours la difference
--

--fdj2RfSjLxBAspz7
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: if_ether.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_ether.h,v
retrieving revision 1.29
diff -u -r1.29 if_ether.h
--- if_ether.h	2003/02/26 06:31:12	1.29
+++ if_ether.h	2003/03/19 18:23:02
@@ -103,7 +103,8 @@
 /*
  * Ethernet-specific mbuf flags.
  */
-#define	M_HASFCS	M_LINK0		/* FCS included at end of frame */
+#define	M_HASFCS	M_LINK0	/* FCS included at end of frame */
+#define	M_PROMISC	M_LINK1	/* this packet is not for us */
 
 #ifdef _KERNEL
 /*
Index: if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.105
diff -u -r1.105 if_ethersubr.c
--- if_ethersubr.c	2003/03/02 10:50:14	1.105
+++ if_ethersubr.c	2003/03/19 18:23:02
@@ -738,29 +738,27 @@
 		 * to "bridge" the packet locally.
 		 */
 		ifp = m->m_pkthdr.rcvif;
-	}
+	} else 
 #endif /* NBRIDGE > 0 */
-
-	/*
-	 * XXX This comparison is redundant if we are a bridge
-	 * XXX and processing the packet locally.
-	 */
-	if ((m->m_flags & (M_BCAST|M_MCAST)) == 0 &&
-	    (ifp->if_flags & IFF_PROMISC) != 0 &&
-	    memcmp(LLADDR(ifp->if_sadl), eh->ether_dhost,
-		   ETHER_ADDR_LEN) != 0) {
-		m_freem(m);
-		return;
+	{
+		if ((m->m_flags & (M_BCAST|M_MCAST)) == 0 &&
+		    (ifp->if_flags & IFF_PROMISC) != 0 &&
+		    memcmp(LLADDR(ifp->if_sadl), eh->ether_dhost,
+			   ETHER_ADDR_LEN) != 0) {
+			m->m_flags |= M_PROMISC;
+		}
 	}
 
 #ifdef PFIL_HOOKS
-	if (pfil_run_hooks(&ifp->if_pfil, &m, ifp, PFIL_IN) != 0)
-		return;
-	if (m == NULL)
-		return;
+	if ((m->m_flags & M_PROMISC) == 0) {
+		if (pfil_run_hooks(&ifp->if_pfil, &m, ifp, PFIL_IN) != 0)
+			return;
+		if (m == NULL)
+			return;
 
-	eh = mtod(m, struct ether_header *);
-	etype = ntohs(eh->ether_type);
+		eh = mtod(m, struct ether_header *);
+		etype = ntohs(eh->ether_type);
+	}
 #endif
 
 	/*
@@ -775,6 +773,7 @@
 		 * vlan_input() will either recursively call ether_input()
 		 * or drop the packet.
 		 */
+		m->m_flags &= ~M_PROMISC;
 		vlan_input(ifp, m);
 #else
 		m_freem(m);
@@ -793,15 +792,21 @@
 		 * vlan_input() will either recursively call ether_input()
 		 * or drop the packet.
 		 */
-		if (((struct ethercom *)ifp)->ec_nvlans != 0)
+		if (((struct ethercom *)ifp)->ec_nvlans != 0) {
+			m->m_flags &= ~M_PROMISC;
 			vlan_input(ifp, m);
-		else
+		} else {
 			m_freem(m);
+		}
 		return;
 #endif /* NVLAN > 0 */
 #if NPPPOE > 0
 	case ETHERTYPE_PPPOEDISC:
 	case ETHERTYPE_PPPOE:
+		if (m->m_flags & M_PROMISC) {
+			m_freem(m);
+			return;
+		}
 #ifndef PPPOE_SERVER
 		if (m->m_flags & (M_MCAST | M_BCAST)) {
 			m_freem(m);
@@ -829,7 +834,10 @@
 		return;
 #endif /* NPPPOE > 0 */
 	default:
-		; /* Nothing. */
+		if (m->m_flags & M_PROMISC) {
+			m_freem(m);
+			return;
+		}
 	}
 
 	/* Strip off the Ethernet header. */

--fdj2RfSjLxBAspz7--