Date: Sun, 4 Mar 2007 19:06:14 +0300 From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: freebsd-net@freebsd.org Cc: rik@FreeBSD.org, glebius@FreeBSD.org, andre@FreeBSD.org, thompsa@FreeBSD.org Subject: kern/109815: wrong interface identifier at pfil_hooks for vlans + if_bridge Message-ID: <20070304160613.GN80319@codelabs.ru> In-Reply-To: <20070304062203.GL80319@codelabs.ru> <45E9F1E8.2000802@inse.ru> References: <E1HNbWw-000LoF-Bo@pobox.codelabs.ru> <45E9F1E8.2000802@inse.ru> <20070304062203.GL80319@codelabs.ru> <E1HNbWw-000LoF-Bo@pobox.codelabs.ru> <45E9F1E8.2000802@inse.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Good day, the freebsd-net readers. Perhaps someone can look at the kern/109815 and provide some feedback. Citing my PR: > >When if_bridge is used to gather multiple vlan interfaces that have > >the same physical parent (I will call such vlan interfaces the 'vlan > >group') the interface identifier that will be passed to the > >pfil_hooks will be sometimes wrong. For all packets coming from the > >'vlan group' and destined to some local interface the incoming interface > >passed to the pfil_hooks will be the last interface in that group > >regardless of the actual incoming interface. The 'last interface' is > >the interface that was added to the if_bridge at the very last 'addm' > >command. > > > >The problem is lying in the fact that MAC addresses of the 'vlan group' > >are just the same and they are equal to the MAC address of the parent > >interface. And the check for the unicat packet that is destined for > >'us' in the if_bridge.c:bridge_input() is walking by the list of the > >if_bridge-attached interfaces and compares the MAC addresses to the > >packet's one. Once match is found the interface in the packet header > >will be rewritten to the found list entry's one. Apparently such > >code flow will select the last added interface from the 'vlan group' > >because FreeBSD list macros are adding list members to the beginning > >of the linked list. > > > >BPF will receive the right interface, because it is tapped before > >bridge_input (in if_ethersubr.c). Roman Kurakin suggested that the patch can break some things: Sun, Mar 04, 2007 at 01:08:40AM +0300, Roman Kurakin wrote: > The idea behind current code could be that in case of bridge the > interface for which this packet in?nded is much more important > than the physical interface it is arrived from. > If this is the case, than it would be much better do the same logic > for ifp and in case it is not that interface to check the list. > This will fix the problem in case of vlans and will leave the old > behavior for the other cases. > > Any comments? I've slightly elaborated his idea and commented on it: Sun, Mar 04, 2007 at 09:22:03AM +0300, Eygene Ryabinkin wrote: > > So, you're saying the following: let us have two interfaces in the > bridge ifA and ifB with the MAC-A and MAC-B. In the current situation > packet coming from the physical interface ifA but destined to the > MAC-B, then the interface seen by the packet filter will be ifB and > not the ifA. Right? > > In principle, this situation can be used for something. But then we > should at least teah BPF to behave this way, because as you remember > from spending three hours before the console with me and trying to > diagnose the problem with tcpdump we were amazed to see the discrepance > between bpf and pfil. So it should be at least documented. > > But as I understand, my patch will let the described situation to > work as usual -- packets will still be delivered to the ifB, but > packet filter will see the physical incoming interface. The patch > should not break the delivery of the packet since all I do is the > change of the rcvif. Or I am wrong? I traced the current if_bridge.c behaviour to the NetBSD's if_bridge.c 1.9. This was the first version in that the firewall hooks were introduced. And the assumtion that the MAC identifies the physical interfaces was used in this first version. And a question: can anyone say if my patch will break some known good behaviour and if the current behaviour of if_bridge is based on some logic I am currently failing to understand. Thank you! -- Eygene
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070304160613.GN80319>