On 13/12/2020 15:22, Nikolay Aleksandrov wrote: > On 13/12/2020 04:40, Vladimir Oltean wrote: >> Currently the bridge emits atomic switchdev notifications for >> dynamically learnt FDB entries. Monitoring these notifications works >> wonders for switchdev drivers that want to keep their hardware FDB in >> sync with the bridge's FDB. >> >> For example station A wants to talk to station B in the diagram below, >> and we are concerned with the behavior of the bridge on the DUT device: >> >> DUT >> +-------------------------------------+ >> | br0 | >> | +------+ +------+ +------+ +------+ | >> | | | | | | | | | | >> | | swp0 | | swp1 | | swp2 | | eth0 | | >> +-------------------------------------+ >> | | | >> Station A | | >> | | >> +--+------+--+ +--+------+--+ >> | | | | | | | | >> | | swp0 | | | | swp0 | | >> Another | +------+ | | +------+ | Another >> switch | br0 | | br0 | switch >> | +------+ | | +------+ | >> | | | | | | | | >> | | swp1 | | | | swp1 | | >> +--+------+--+ +--+------+--+ >> | >> Station B >> >> Interfaces swp0, swp1, swp2 are handled by a switchdev driver that has >> the following property: frames injected from its control interface bypass >> the internal address analyzer logic, and therefore, this hardware does >> not learn from the source address of packets transmitted by the network >> stack through it. So, since bridging between eth0 (where Station B is >> attached) and swp0 (where Station A is attached) is done in software, >> the switchdev hardware will never learn the source address of Station B. >> So the traffic towards that destination will be treated as unknown, i.e. >> flooded. >> >> This is where the bridge notifications come in handy. When br0 on the >> DUT sees frames with Station B's MAC address on eth0, the switchdev >> driver gets these notifications and can install a rule to send frames >> towards Station B's address that are incoming from swp0, swp1, swp2, >> only towards the control interface. This is all switchdev driver private >> business, which the notification makes possible. >> >> All is fine until someone unplugs Station B's cable and moves it to the >> other switch: >> >> DUT >> +-------------------------------------+ >> | br0 | >> | +------+ +------+ +------+ +------+ | >> | | | | | | | | | | >> | | swp0 | | swp1 | | swp2 | | eth0 | | >> +-------------------------------------+ >> | | | >> Station A | | >> | | >> +--+------+--+ +--+------+--+ >> | | | | | | | | >> | | swp0 | | | | swp0 | | >> Another | +------+ | | +------+ | Another >> switch | br0 | | br0 | switch >> | +------+ | | +------+ | >> | | | | | | | | >> | | swp1 | | | | swp1 | | >> +--+------+--+ +--+------+--+ >> | >> Station B >> >> Luckily for the use cases we care about, Station B is noisy enough that >> the DUT hears it (on swp1 this time). swp1 receives the frames and >> delivers them to the bridge, who enters the unlikely path in br_fdb_update >> of updating an existing entry. It moves the entry in the software bridge >> to swp1 and emits an addition notification towards that. >> >> As far as the switchdev driver is concerned, all that it needs to ensure >> is that traffic between Station A and Station B is not forever broken. >> If it does nothing, then the stale rule to send frames for Station B >> towards the control interface remains in place. But Station B is no >> longer reachable via the control interface, but via a port that can >> offload the bridge port learning attribute. It's just that the port is >> prevented from learning this address, since the rule overrides FDB >> updates. So the rule needs to go. The question is via what mechanism. >> >> It sure would be possible for this switchdev driver to keep track of all >> addresses which are sent to the control interface, and then also listen >> for bridge notifier events on its own ports, searching for the ones that >> have a MAC address which was previously sent to the control interface. >> But this is cumbersome and inefficient. Instead, with one small change, >> the bridge could notify of the address deletion from the old port, in a >> symmetrical manner with how it did for the insertion. Then the switchdev >> driver would not be required to monitor learn/forget events for its own >> ports. It could just delete the rule towards the control interface upon >> bridge entry migration. This would make hardware address learning be >> possible again. Then it would take a few more packets until the hardware >> and software FDB would be in sync again. >> >> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx> >> --- >> Changes in v2: >> Patch is new. >> >> net/bridge/br_fdb.c | 1 + >> 1 file changed, 1 insertion(+) >> > > Hi Vladimir, > Thank you for the good explanation, it really helps a lot to understand the issue. > Even though it's deceptively simple, that call adds another lock/unlock for everyone > when moving or learning (due to notifier lock), but I do like how simple the solution > becomes with this change, so I'm not strictly against it. I think I'll add a "refcnt"-like > check in the switchdev fn which would process the chain only when there are registered users > to avoid any locks when moving fdbs on pure software bridges (like it was before swdev). > > I get that the alternative is to track these within DSA, I'm tempted to say that's not such > a bad alternative as this change would make moving fdbs slower in general. Have you thought > about another way to find out, e.g. if more fdb information is passed to the notifications ? > > Thanks, > Nik > Nevermind the whole comment. :) I was looking at the wrong code and got confused. All is well (thanks to Ido). Acked-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxx>
