netdev - Re: [PATCH RFC] net: bridge: don't flood known multicast traffic when snooping is enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 16 Feb 2019 21:27:26 +0200
From:   nikolay@...ulusnetworks.com
To:     Ido Schimmel <idosch@...sch.org>
CC:     Linus Lüssing <linus.luessing@...3.blue>,
        netdev@...r.kernel.org, roopa@...ulusnetworks.com,
        wkok@...ulusnetworks.com, anuradhak@...ulusnetworks.com,
        bridge@...ts.linux-foundation.org, davem@...emloft.net,
        stephen@...workplumber.org
Subject: Re: [PATCH RFC] net: bridge: don't flood known multicast traffic when snooping is enabled

On 16 February 2019 21:15:21 EET, nikolay@...ulusnetworks.com wrote:
>On 16 February 2019 20:43:53 EET, Ido Schimmel <idosch@...sch.org>
>wrote:
>>On Sat, Feb 16, 2019 at 10:05:40AM +0200, Nikolay Aleksandrov wrote:
>>> On 15/02/2019 19:13, Linus Lüssing wrote:
>>> > On Fri, Feb 15, 2019 at 03:04:27PM +0200, Nikolay Aleksandrov
>>wrote:
>>> >> Every user would expect to have traffic forwarded only to the
>>configured
>>> >> mdb destination when snooping is enabled, instead now to get that
>>one
>>> >> needs to enable both snooping and querier. Enabling querier on
>all
>>> >> switches could be problematic and is not a good solution,
>>> > 
>>> > There is no need to set the querier on all snooping switches.
>>> > br_multicast_querier_exists() checks if a querier exists on the
>>> > link in general, not if this particular host/bridge is a querier.
>>> > 
>>> 
>>> We need a generic solution for the case of existing mdst and no
>>querier.
>>> More below.
>>> 
>>> > 
>>> >> for example as summarized by our multicast experts:
>>> >> "every switch would send an IGMP query
>>> > 
>>> > What? RFC3810, section 7.1 says:
>>> > 
>>> > "If it is the case, a querier election mechanism (described in
>>> >  section 7.6.2) is used to elect a single multicast router to be
>>> >  in Querier state. [...] Nevertheless, it is only the [elected]
>>Querier
>>> >  that sends periodical or triggered query messages on the subnet."
>>> > >> for any random multicast traffic it
>>> >> received across the entire domain and it would send it forever as
>>long as a
>>> >> host exists wanting that stream even if it has no
>>downstream/directly
>>> >> connected receivers"
>>> > 
>>> 
>>> This was taken out of context and it's my bad, I think everyone is
>>aware
>>> of the election process, please nevermind the above statement.
>>> 
>>> [snip]> 
>>> > 
>>> > Have you done some tests with this change yet, Nikolay?
>>> > 
>>> 
>>> You've raised good questions, IPv6 indeed needs more work - we'll
>>have to flood
>>> link-local packets etc. but I wanted to have a discussion about no
>>querier/existing mdst.
>>> To simplify we can modify the patch and have traffic forwarded to
>the
>>proper ports when an
>>> mdst exists and there is no querier for both unsolicited report and
>>user-added entry.
>>> We can keep the current behaviour for unknown traffic with and
>>without querier.
>>> This would align it closer to what other vendors currently do as
>well
>>IIRC.
>>> What do you think ?
>>
>>The no querier condition is not currently reflected via switchdev, so
>>the behavior you're proposing in your patch is what actually happens
>in
>>the data plane.
>>
>>We already hit the problem Linus mentioned in commit b00589af3b04
>>("bridge: disable snooping if there is no querier"). Namely, IPv6 ND
>>broke because a port joined before the bridge was created.
>>
>>I introduced a workaround in commit 9d45deb04c59 ("mlxsw: spectrum:
>>Treat IPv6 unregistered multicast as broadcast"). I'm interested to
>>know
>>what other vendors are doing. Can you elaborate?
>>
>
>Exactly like your fix. :) Flood it, this patch unfortunately breaks it 
>because of mrouters flag, but we can retain the behaviour
>by forwarding only known mdsts to their ports and flooding
>unregistered mcast when there is no querier. I think that is 
>what others do by default too, actually I think they flood with querier
>as well. Maybe unknown mcast flooding should be controlled by a flag
>when a querier is present
>because we've had this behaviour for a long time, personally I'd have
>it on
>by default. 

Ugh, mispoke please read the above statement to be only about no querier. 
I meant flooding v6 link-local always. 

>I am currently away and will be able to prepare a rfc patch after the
>weekend. 
>
>
>>We can trap IPv6 ND packets at L2 (we'll eventually need to do for ND
>>suppression) and let the bridge take care of flooding them correctly.
>>I'm not sure it's good enough.


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.