Date: Tue, 13 Mar 2007 11:36:54 +0000 From: "Bruce M. Simpson" <bms@FreeBSD.org> To: Eygene Ryabinkin <rea-fbsd@codelabs.ru> Cc: rik@FreeBSD.org, Roman Kurakin <rik@inse.ru>, andre@FreeBSD.org, Yar Tikhiy <yar@comp.chem.msu.su>, glebius@FreeBSD.org, thompsa@FreeBSD.org, freebsd-net@FreeBSD.org Subject: Re: kern/109815: wrong interface identifier at pfil_hooks for vlans + if_bridge Message-ID: <45F68CD6.2030300@FreeBSD.org> In-Reply-To: <20070313055029.GK58523@codelabs.ru> References: <45ED900A.7050208@FreeBSD.org> <20070312092406.GJ58523@codelabs.ru> <45F51F2B.5020906@FreeBSD.org> <20070312112056.GC44732@comp.chem.msu.su> <45F554F5.8020505@FreeBSD.org> <20070312140219.GE44732@comp.chem.msu.su> <20070312143811.GA58523@codelabs.ru> <20070312165145.GF44732@comp.chem.msu.su> <45F5BD36.1070205@inse.ru> <20070312214926.GK44732@comp.chem.msu.su> <20070313055029.GK58523@codelabs.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Eygene Ryabinkin wrote: > I tried to understand this, because Bruce already gave me a patch, > but I am a bit stupid: I do not see how M_PROMISC that is cleared > unconditionally before BRIDGE_INPUT will help us to identify the > right interface. As I see now, the BRIDGE_INPUT is called once from > if_ethersubr.c, once from if_gif.c and once from ng_ether.c: > http://fxr.watson.org/fxr/ident?i=BRIDGE_INPUT > So there is no distinct code paths that can allow BRIDGE_INPUT to > modify its behaviour based on the M_PROMISC flag. > > But I feel that I am wrong in some place and missing some discuission > on the M_PROMISC. Can anyone point me to the right place? > In short: M_PROMISC exists to easily identify frames which were received promiscuously, to prevent infinite recursion, and to simplify code which needs to re-enter ether_input(). M_PROMISC is a flag introduced by NetBSD into their ethernet input path to deal with the case where an entity in the network stack needs to receive frames promiscuously, without necessarily passing those frames to the upper layers e.g. IPv4. It is not documented; the code is the documentation in this instance. It is cleared when an mbuf chain is passed to another entity which may consume the frame in that mbuf chain, in case the entity re-enters ether_input() with the same mbuf chain for local delivery (e.g. bridge, netgraph, vlan). I do not think M_PROMISC alone is sufficient to solve our architectural problems at Layer 2. > >> So all the tangled if()s inside LIST_FOREACH() will be gone completely >> from bridge_input(). >> > > But we still need to see if we want to consume the packet by the > bridge or it members or to do forwarding. Am I missing something? > Correct. Just because a frame was received promiscuously, does not imply that the bridge will be the only consumer of that frame. > >>>> I'm afraid there is a serious flaw in the very notion of such a >>>> "logical" interface. If it's true, we should start by admitting >>>> that the support for "logical" interfaces should be a side hack for >>>> compatibility, and not something that can live forever on the main >>>> code path. >>>> > > I agree with you. That is why I patched if_bridge once again to enable > the pfil hooks for the "physical" incoming interface. And there are > two ways to solve the problem: > - to give each VLAN interface the distinct MAC, as Bruce suggested, > I didn't suggest this. :-) I pointed out that the code matches on destination MAC only at the moment. vlan(4) is an abstraction of something which exists as part of the Ethernet framing, and is not a physical interface in its own right, as was correctly identified above. > - to refuse the "logical" interfaces completely and to support only > "physical" ones. It is what my very first (and very short) patch > did. But this can break some existing firewall rulesets. And that > should be discuissed -- we do not need the total breakage due to > out changes. And you're right: the best way for this alternative is to > leave the current behaviour as the compatibility sysctl that is turned > off by default and move to the filtering on the "physical" interfaces > by default. No problem, but skilled network people that are using > FreeBSD as the bridge for VLANs should say if they are happy with it. > I think it is acceptable for if_bridge(4) to know about the existence of VLAN interfaces and to deal with them accordingly as a special case, because Spanning Tree is specified differently in the case where VLANs are present. Therefore it is not unreasonable for if_bridge(4) to be looking at VLAN headers in the mbuf chain. As such I think the behaviour Andrew Thompson and I were discussing off list should be made the default: that is, the first 802.1q VLAN header is stripped off and turned into an M_VLANTAG before being passed to other consumers in the stack. The presence of M_VLANTAG makes it very easy to see that a frame was received with a VLAN header without involving vlan(4) and reduces the amount of 802.1q specific code across Layer 2 subsystems. Regards, BMS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?45F68CD6.2030300>