Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 12 Mar 2007 16:13:47 +0300
From:      Roman Kurakin <rik@inse.ru>
To:        Eygene Ryabinkin <rea-fbsd@codelabs.ru>
Cc:        rik@FreeBSD.org, andre@FreeBSD.org, freebsd-net@freebsd.org, glebius@FreeBSD.org, thompsa@FreeBSD.org, "Bruce M. Simpson" <bms@FreeBSD.org>
Subject:   Re: kern/109815: wrong interface identifier at pfil_hooks for vlans +	if_bridge
Message-ID:  <45F5520B.80709@inse.ru>
In-Reply-To: <20070312121117.GQ58523@codelabs.ru>
References:  <E1HNbWw-000LoF-Bo@pobox.codelabs.ru> <45E9F1E8.2000802@inse.ru>	<20070304160613.GN80319@codelabs.ru> <45EB4915.1090703@FreeBSD.org>	<20070305145647.GT80319@codelabs.ru> <45EC3EFD.3000301@FreeBSD.org>	<20070306073945.GR57456@codelabs.ru> <45ED900A.7050208@FreeBSD.org>	<20070312092406.GJ58523@codelabs.ru> <45F51F2B.5020906@FreeBSD.org> <20070312121117.GQ58523@codelabs.ru>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------080308030607070800070004
Content-Type: text/plain; charset=koi8-r; format=flowed
Content-Transfer-Encoding: 7bit

Eygene Ryabinkin wrote:
> [...]
> We're not checking if the interface member is a VLAN interface. We just
> do the generic checks for the incoming interface. rik@ will send the
> patch today, at least he just promised me ;))
>   
Here it is. I'll check it for compilation this evening and I hope Eygene 
will be
able to put it in a production for testings.

I've found one more strange place in the code. Probably it is not a 
problem but I
do not understand smth. How common the situation that we get our own packets
on the bridge? Probably we could do some optimization around this case ...
 From the original commit (in OpenBSD IIRC) that adds this check it is 
not clear
why it was added.

rik


--------------080308030607070800070004
Content-Type: text/plain;
 name="fbsd070312-3.pch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="fbsd070312-3.pch"

Index: if_bridge.c
===================================================================
RCS file: /home/ncvs/src/sys/net/if_bridge.c,v
retrieving revision 1.94
diff -u -r1.94 if_bridge.c
--- if_bridge.c	9 Mar 2007 19:34:55 -0000	1.94
+++ if_bridge.c	12 Mar 2007 12:50:45 -0000
@@ -2121,42 +2121,62 @@
 		return (m);
 	}
 
+#ifdef DEV_CARP
+#   define OR_CARP_CHECK_WE_ARE_DST(iface) \
+	|| ((iface)->if_carp \
+	    && carp_forus((iface)->if_carp, eh->ether_dhost))
+#   define OR_CARP_CHECK_WE_ARE_SRC(iface) \
+	|| ((iface)->if_carp \
+	    && carp_forus((iface)->if_carp, eh->ether_shost))
+#else
+#   define OR_CARP_CHECK_WE_ARE_DST(iface)
+#   define OR_CARP_CHECK_WE_ARE_SRC(iface)
+#endif
+
+#define GRAB_OUR_PACKETS(iface) \
+	if ((iface)->if_type == IFT_GIF) \
+		continue; \
+	/* It is destined for us. */ \
+	if (memcmp(IF_LLADDR((iface)), eh->ether_dhost,  ETHER_ADDR_LEN) == 0 \
+	    OR_CARP_CHECK_WE_ARE_DST((iface))				\
+	    ) {								\
+		if (bif->bif_flags & IFBIF_LEARNING)			\
+			(void) bridge_rtupdate(sc,			\
+			    eh->ether_shost, bif, 0, IFBAF_DYNAMIC);	\
+		m->m_pkthdr.rcvif = iface;				\
+		BRIDGE_UNLOCK(sc);					\
+		return (m);						\
+	}								\
+									\
+	/* We just received a packet that we sent out. */		\
+	if (memcmp(IF_LLADDR((iface)), eh->ether_shost, ETHER_ADDR_LEN) == 0 \
+	    OR_CARP_CHECK_WE_ARE_SRC((iface))			\
+	    ) {								\
+		BRIDGE_UNLOCK(sc);					\
+		m_freem(m);						\
+		return (NULL);						\
+	}
+
 	/*
 	 * Unicast.  Make sure it's not for us.
+	 *
+	 * Give a chance for ifp at first priority. This will help when	the
+	 * packet comes through the interface like VLAN's with the same MACs
+	 * on several interfaces from the same bridge. This also will save
+	 * some CPU cycles in case the destination interface and the input
+	 * interface (eq ifp) are the same.
 	 */
-	LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) {
-		if (bif2->bif_ifp->if_type == IFT_GIF)
-			continue;
-		/* It is destined for us. */
-		if (memcmp(IF_LLADDR(bif2->bif_ifp), eh->ether_dhost,
-		    ETHER_ADDR_LEN) == 0
-#ifdef DEV_CARP
-		    || (bif2->bif_ifp->if_carp 
-			&& carp_forus(bif2->bif_ifp->if_carp, eh->ether_dhost))
-#endif
-		    ) {
-			if (bif->bif_flags & IFBIF_LEARNING)
-				(void) bridge_rtupdate(sc,
-				    eh->ether_shost, bif, 0, IFBAF_DYNAMIC);
-			m->m_pkthdr.rcvif = bif2->bif_ifp;
-			BRIDGE_UNLOCK(sc);
-			return (m);
-		}
+	do { GRAB_OUR_PACKETS(ifp) } while (0);
 
-		/* We just received a packet that we sent out. */
-		if (memcmp(IF_LLADDR(bif2->bif_ifp), eh->ether_shost,
-		    ETHER_ADDR_LEN) == 0
-#ifdef DEV_CARP
-		    || (bif2->bif_ifp->if_carp 
-			&& carp_forus(bif2->bif_ifp->if_carp, eh->ether_shost))
-#endif
-		    ) {
-			BRIDGE_UNLOCK(sc);
-			m_freem(m);
-			return (NULL);
-		}
+	/* Now check the all bridge members. */
+	LIST_FOREACH(bif2, &sc->sc_iflist, bif_next) {
+		GRAB_OUR_PACKETS(bif2->bif_ifp)
 	}
 
+#undef OR_CARP_CHECK_WE_ARE_DST
+#undef OR_CARP_CHECK_WE_ARE_SRC
+#undef GRAB_OUR_PACKETS
+
 	/* Perform the bridge forwarding function. */
 	bridge_forward(sc, m);
 

--------------080308030607070800070004--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45F5520B.80709>