On 30/07/2019 20:21, Nikolay Aleksandrov wrote: > On 30/07/2019 20:18, David Miller wrote: >> From: Nikolay Aleksandrov <[email protected]> >> Date: Tue, 30 Jul 2019 14:21:00 +0300 >> >>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c >>> index 3d8deac2353d..f8cac3702712 100644 >>> --- a/net/bridge/br_multicast.c >>> +++ b/net/bridge/br_multicast.c >>> @@ -1388,6 +1388,9 @@ br_multicast_leave_group(struct net_bridge *br, >>> if (!br_port_group_equal(p, port, src)) >>> continue; >>> >>> + if (p->flags & MDB_PG_FLAGS_PERMANENT) >>> + break; >>> + >> >> Like David, I also don't understand why this can be a break. Is it because >> permanent entries are always the last on the list? Why will there be no >> other entries that might need to be processed on the list? >> > > The reason is that only one port can match. See the first clause of > br_port_group_equal, > that port can participate only once. We could easily add a break statement in > the end > when a match is found and it will be correct. Even in the presence of > MULTICAST_TO_UNICAST > flag, the port must match and can be added only once. >
Like I wrote in the patch I plan to re-work that code in net-next to remove the duplication and make it more understandable to avoid such confusions. This code will be functionally equivalent if I put continue there, we'll just walk over all of them even after a match or permanent are found. There can only be one match though as I said, so walking the rest of the ports is a waste.
