netdev - Re: [PATCH] net: bridge: suppress broadcast when multicast flood is disabled

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]

Message-ID: <a96ddd28-6d99-84d0-563a-2493a09a9e60@brocade.com>
Date:   Tue, 25 Apr 2017 14:32:44 +0100
From:   Mike Manning <mmanning@...cade.com>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        <netdev@...r.kernel.org>
CC:     "David S. Miller" <davem@...emloft.net>,
        roopa <roopa@...ulusnetworks.com>
Subject: Re: [PATCH] net: bridge: suppress broadcast when multicast flood is
 disabled

On 24/04/17 20:52, Nikolay Aleksandrov wrote:
> On 24/04/17 17:09, Mike Manning wrote:
>> Flood suppression for packets that are not unicast needs to be handled
>> consistently by also not flooding broadcast packets. As broadcast is a
>> special case of multicast, the same kernel parameter should be used to
>> suppress flooding for both of these packet types.
>>
>> Fixes: b6cb5ac8331b ("net: bridge: add per-port multicast flood flag")
>> Cc: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>> Signed-off-by: Mike Manning <mmanning@...cade.com>
>> ---
>>  net/bridge/br_forward.c | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
> 
> I do not agree that this is a bug fix, the behaviour was intentional and is close to how HW
> handles this flag. It has been like that for a few releases and changing it may impact setups
> that use the flag since up until now they've seen the broadcast but not multicast packets and
> suddenly their broadcast will stop.
> 
> I think it would be better to introduce a third flag for bcast in net-next and use that to
> filter it since that would give us the ability to program HW that can distinguish these
> and have both options available, moreover it will not break any user setups relying on
> the current flag behaviour and we have such setups.
> 
> Thanks,
>  Nik
> 
> 

Hi Nik,
What is the usecase for flooding broadcast but not multicast please? Is the lack of flood
suppression for broadcast just something that has not been explicitly tested for in those
setups? This is the case for us, the bug raised only at this stage of the release cycle.
While adding another kernel param is an option, I would only do so if absolutely necessary
so as to avoid proliferation of params. Also to justify adding such a flag for broadcast
suppression, I would need to add a comment to explain that while broadcast is a subset of
multicast, the multicast flood suppression flag excludes broadcast.

Thanks
Mike