On 19/02/2019 17:42, Linus Lüssing wrote: > On Tue, Feb 19, 2019 at 03:31:42PM +0200, Nikolay Aleksandrov wrote: >> On 19/02/2019 11:21, Linus Lüssing wrote: >>> On Tue, Feb 19, 2019 at 09:57:16AM +0100, Linus Lüssing wrote: >>>> On Mon, Feb 18, 2019 at 02:21:07PM +0200, Nikolay Aleksandrov wrote: >>>>> This is v2 of the RFC patch which aims to forward packets to known >>>>> mdsts' ports only (the no querier case). After v1 I've kept >>>>> the previous behaviour when it comes to unregistered traffic or when >>>>> a querier is present. All of this is of course only with snooping >>>>> enabled. So with this patch the following changes should occur: >>>>> - No querier: forward known mdst traffic to its registered ports, >>>>> no change about unknown mcast (flood) >>>>> - Querier present: no change >>>>> >>>>> The reason to do this is simple - we want to respect the user's mdb >>>>> configuration in both cases, that is if the user adds static mdb entries >>>>> manually then we should use that information about forwarding traffic. >>>>> >>>>> What do you think ? >>>>> >>>>> * Notes >>>>> Traffic that is currently marked as mrouters_only: >>>>> - IPv4: non-local mcast traffic, igmp reports >>>>> - IPv6: non-all-nodes-dst mcast traffic, mldv1 reports >>>>> >>>>> Simple use case: >>>>> $ echo 1 > /sys/class/net/bridge/bridge/multicast_snooping >>>>> $ bridge mdb add dev bridge port swp1 grp 239.0.0.1 >>>>> - without a querier currently traffic for 239.0.0.1 will still be flooded, >>>>> with this change it will be forwarded only to swp1 >>>> >>>> There is still the issue with unsolicited reports adding mdst >>>> entries here, too. Leading to unwanted packet loss and connectivity issues. >>> >>> Or in other words, an unsolicited report will turn a previously >>> unregistered multicast group into a registered one. However in the >>> absence of a querier the knowledge about this newly registered multicast group >>> will be incomplete. And therefore still needs to be flooded to avoid packet >>> loss. >>> >> >> Right, this is expected. If the user has enabled igmp snooping and doesn't have >> a querier present then such behaviour is to be expected. > > IGMP snooping is currently enabled by default. And IGMP/MLD > querier is disabled by default. I wouldn't want packet loss to be > the expected behaviour in a default setup. > > This default setup currently works. However with this change it > will introduce packet loss, as you acknowledged (if I understood > you correctly?). > Yeah, I'm not pushing this change anymore, the only option was to add a tunable for the behaviour which I described in my previous reply but I really want to avoid adding yet another bridge option as it has way too many already. >> What is surprising is >> the user explicitly enabling igmp snooping, adding an mdst and then still >> getting it flooded. :) > > Hm, for me that does not seem surprising. I would not expect igmp > snooping to work without a querier on the link. Why don't you just > add/enable a querier in your setups then if you want to avoid > flooding? > Yep, ack. > >> An alternative is to drop all unregistered traffic when a querier is not present. >> But that will surely break setups and at best should be a configurable option that >> is disabled by default. > > Absolutely right. Always dropping with no querier is no option. That's why I'd say > you should always flood multicast packets if there is no querier. > > >> So in effect and to try and make everybody happy we can add an option to control >> this behaviour with keeping the current as default and adding the following options: >> - no querier: flood all (default, current) > > ACK > > For the other options maybe I do not understand your scenario yet. > Wouldn't these two options result in eratic behaviour? > >> - no querier: flood unregistered, forward registered >> - no querier: drop unregistered, forward registered > To be fair I've seen all 3 options being default on different versions of network OSes by major vendors. > Let's call these two cases A) - flood unregistered, forward > registered and B) - drop unregistered, forward registered. > > > Let's say you have a bridge with two ports: > (1)<-[br]->(2). And no querier. > > Behind (2) there is a listener for group M. M is not > registered by the bridge because either that listener joined > before the bridge came up or the entry was registered once but had > timed out. Or there was packet loss and the report did not arrive > at the bridge (for instance bc. listener is behind a wireless > connection). > > For case B) we can immediately see that the listener at (2) will > not receive the traffic it signed up for. And this is a permanent > issue as the listener will not send any further reports. > > Case A) is ok, the listener behind port (2) receives its traffic. > > > Now, a listener for M joins at (1). It sends an unsolicited > report. Group M becomes registered by the bridge. Both for > cases (A) and (B) this new listener at (1) will receive its > traffic. However, not only in case B) now, but in case A), too, > the listener at (2) will rceive no more traffic for M. > > > Now 260 seconds pass (multicast_membership_interval). The entry > for M times out and is deleted on the bridge. It becomes > unregistered. > > Now for case (A) things would be ok again, both listeners at (1) > and (2) would receive traffic. For now - as long as no new listener > joins again. > > For case (B), both the listener at port (1) and (2) will fail to > receive the traffic they signed up for. > > > --- > > I hope this illustrates a bit what I'm afraid of? If you have any > measures to prevent such behavior in your setup, I'd be curious to > know. > It does and I'm aware of these scenarios, but there're static mcast trees and also such that are controlled by user-space daemons, that is the daemon keeps refreshing the mdb entries. They're non-standard for sure and obviously we suggest having a querier always. That's why I suggested having these as options, but now I'm retracting that and will table this whole thing for the time being. I really don't want to add another option to the bridge lightly. > Regards, Linus > Thanks for the discussion and all the helpful comments, Nik
