Martin, good day. Sun, Jun 09, 2019 at 11:44:10AM -0300, Martin Pieuchot wrote: > On 07/06/19(Fri) 20:50, Eygene Ryabinkin wrote: > > The attached patch fixes incoming interface for pf(4) processing > > in the case of bridging of multiple VLAN interfaces which have > > the same parent iface and unicast packets destined to the bridge > > member: we can't rely solely on the destination MAC in determining > > proper incoming interface. > > > > Can it be reviewed and possibly applied? It was verified in my > > production environment. > > Are you suggesting that the order in which bridge members are checked > matters?
Yes. > In that case would it makes sense to apply the so-called unicast > check to `ifp', the incoming interface, first? Right. Had rewritten the patch to do this explicitely: +1 goto, -1 (I hope) confusion point about ifp setting. The bridge with updated patch works as expected already for some hours in production: will continue to monitor it. -- rea
>From 80808b947d3f1e980a39ae997adb91b142bcc614 Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin <[email protected]> Date: Thu, 6 Jun 2019 09:40:06 +0300 Subject: [PATCH] bridge(4): filter locally-destined packets on the proper interface When vlan(4) interfaces are used as the bridge members, they share the MAC of their parent ifs. Thus, when multiple VLANs with the same parent interface are bridged (for filtering), they will all have the same MAC and MAC-based strategy for indentification of locally-destined traffic may pick the wrong interface for pf(4) processing. Actually, it will use the most recently added interface with the packet's destination MAC. As we know the incoming interface for bridge_process(), check if it has the packet's destination MAC and use it for bridge_input() when the check succeeds. Signed-off-by: Eygene Ryabinkin <[email protected]> --- sys/net/if_bridge.c | 48 ++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/sys/net/if_bridge.c b/sys/net/if_bridge.c index 882eb9c3283..abffdc4db2f 100644 --- a/sys/net/if_bridge.c +++ b/sys/net/if_bridge.c @@ -1116,7 +1116,7 @@ bridge_input(struct ifnet *ifp, struct mbuf *m, void *cookie) void bridge_process(struct ifnet *ifp, struct mbuf *m) { - struct ifnet *brifp; + struct ifnet *brifp, *if0; struct bridge_softc *sc; struct bridge_iflist *bif, *bif0; struct ether_header *eh; @@ -1198,26 +1198,22 @@ bridge_process(struct ifnet *ifp, struct mbuf *m) } /* - * Unicast, make sure it's not for us. + * Unicast. Try to consume locally-destined packets. + * + * Bridge can have multiple interfaces sharing the same MAC + * (vlan(4) interfaces are one example of this); + * so, we first test the incoming interface: it is the most + * logical entity to process/filter the L2 packets on + * for such a case. */ bif0 = bif; + if0 = ifp; + if (bridge_ourether(ifp, eh->ether_dhost)) + goto for_bridge; SMR_SLIST_FOREACH_LOCKED(bif, &sc->sc_iflist, bif_next) { if (bridge_ourether(bif->ifp, eh->ether_dhost)) { - if (bif0->bif_flags & IFBIF_LEARNING) - bridge_rtupdate(sc, - (struct ether_addr *)&eh->ether_shost, - ifp, 0, IFBAF_DYNAMIC, m); - if (bridge_filterrule(&bif0->bif_brlin, eh, m) == - BRL_ACTION_BLOCK) { - goto bad; - } - - /* Count for the bridge */ - brifp->if_ipackets++; - brifp->if_ibytes += m->m_pkthdr.len; - ifp = bif->ifp; - goto reenqueue; + goto for_bridge; } if (bridge_ourether(bif->ifp, eh->ether_shost)) goto bad; @@ -1227,6 +1223,26 @@ bridge_process(struct ifnet *ifp, struct mbuf *m) if_put(brifp); return; +for_bridge: + /* + * The packet is for the bridge member interface on L2. + * + * Variables we expect to be set: + * - if0: incoming interface; + * - bif0: bridge_iflist that corresponds to if0; + * - ifp: interface the packet is destined to. + */ + if (bif0->bif_flags & IFBIF_LEARNING) + bridge_rtupdate(sc, (struct ether_addr *)&eh->ether_shost, + if0, 0, IFBAF_DYNAMIC, m); + if (bridge_filterrule(&bif0->bif_brlin, eh, m) == BRL_ACTION_BLOCK) + goto bad; + + /* Count for the bridge */ + brifp->if_ipackets++; + brifp->if_ibytes += m->m_pkthdr.len; + + /* Process on the selected interface */ reenqueue: bridge_ifinput(ifp, m); m = NULL; -- 2.21.0
