From owner-freebsd-net@FreeBSD.ORG Mon Mar 12 13:09:16 2007 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id B049016A402; Mon, 12 Mar 2007 13:09:16 +0000 (UTC) (envelope-from rik@inse.ru) Received: from mail.inse.ru (inse.ru [144.206.128.1]) by mx1.freebsd.org (Postfix) with ESMTP id 1DC8913C44B; Mon, 12 Mar 2007 13:09:16 +0000 (UTC) (envelope-from rik@inse.ru) Received: from [127.0.0.1] (www.inse.ru [144.206.128.1]) by mail.inse.ru (Postfix) with ESMTP id 55FFA33C4F; Mon, 12 Mar 2007 16:09:14 +0300 (MSK) Message-ID: <45F5520B.80709@inse.ru> Date: Mon, 12 Mar 2007 16:13:47 +0300 From: Roman Kurakin User-Agent: Thunderbird 1.5.0.7 (X11/20061110) MIME-Version: 1.0 To: Eygene Ryabinkin References: <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> In-Reply-To: <20070312121117.GQ58523@codelabs.ru> Content-Type: multipart/mixed; boundary="------------080308030607070800070004" Cc: rik@FreeBSD.org, andre@FreeBSD.org, freebsd-net@freebsd.org, glebius@FreeBSD.org, thompsa@FreeBSD.org, "Bruce M. Simpson" Subject: Re: kern/109815: wrong interface identifier at pfil_hooks for vlans + if_bridge X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Mar 2007 13:09:16 -0000 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--