Linux Ethernet Bridging - Apr 2022 - [Bridge] [PATCH RFC net-next 08/13] net: bridge: avoid classifying unknown multicast as mrouters_only

On Tue, Apr 12, 2022 at 20:37, Nikolay Aleksandrov <razor at
blackwall.org> wrote:
> On 12/04/2022 20:27, Joachim Wiberg wrote:
>> [snip]
>> From this I'd like to argue that our current behavior in the bridge
is
>> wrong.  To me it's clear that, since we have a confiugration
option, we
>> should forward unknown IP multicast to all MCAST_FLOOD ports (as well
as
>> the router ports).
> Definitely not wrong. In fact:
> "Switches that do not forward unregistered packets to all ports must
>  include a configuration option to force the flooding of unregistered
>  packets on specified ports. [..]"
> is already implemented because the admin can mark any port as a router and
> enable flooding to it.
Hmm, I understand your point (here and below), and won't drive this
point further.  Instead I'll pick up on what you said in your first
reply ... (below, last)

Btw, thank you for taking the time to reply and explain your standpoint,
really helps my understanding of how we can develop the bridge further,
without breaking userspace! :)
>> [1]: https://www.rfc-editor.org/rfc/rfc4541.html#section-2.1.2
> RFC4541 is only recommending, it's not a mandatory behaviour. This
> default has been placed for a very long time and a lot of users and
> tests take it into consideration.
Noted.
> We cannot break such assumptions and start suddenly flooding packets,
> but we can leave it up to the admin or distribution/network software
> to configure it as default.
So, if I add a bridge flag, default off as you mentioned out earlier,
which changes the default behavior of MCAST_FLOOD, then you'd be OK with
that?  Something cheeky like this perhaps:

    if (!ipv4_is_local_multicast(ip_hdr(skb)->daddr))
       	BR_INPUT_SKB_CB(skb)->mrouters_only = !br_opt_get(br,
BROPT_MCAST_FLOOD_RFC4541);


Best regards
 /Joachim

On 13/04/2022 11:51, Joachim Wiberg wrote:
> On Tue, Apr 12, 2022 at 20:37, Nikolay Aleksandrov <razor at
blackwall.org> wrote:
>> On 12/04/2022 20:27, Joachim Wiberg wrote:
>>> [snip]
>>> From this I'd like to argue that our current behavior in the
bridge is
>>> wrong.  To me it's clear that, since we have a confiugration
option, we
>>> should forward unknown IP multicast to all MCAST_FLOOD ports (as
well as
>>> the router ports).
>> Definitely not wrong. In fact:
>> "Switches that do not forward unregistered packets to all ports
must
>>  include a configuration option to force the flooding of unregistered
>>  packets on specified ports. [..]"
>> is already implemented because the admin can mark any port as a router
and
>> enable flooding to it.
> 
> Hmm, I understand your point (here and below), and won't drive this
> point further.  Instead I'll pick up on what you said in your first
> reply ... (below, last)
> 
> Btw, thank you for taking the time to reply and explain your standpoint,
> really helps my understanding of how we can develop the bridge further,
> without breaking userspace! :)
> 
>>> [1]: https://www.rfc-editor.org/rfc/rfc4541.html#section-2.1.2
>> RFC4541 is only recommending, it's not a mandatory behaviour. This
>> default has been placed for a very long time and a lot of users and
>> tests take it into consideration.
> 
> Noted.
> 
>> We cannot break such assumptions and start suddenly flooding packets,
>> but we can leave it up to the admin or distribution/network software
>> to configure it as default.
> 
> So, if I add a bridge flag, default off as you mentioned out earlier,
> which changes the default behavior of MCAST_FLOOD, then you'd be OK
with
> that?  Something cheeky like this perhaps:
> 
>     if (!ipv4_is_local_multicast(ip_hdr(skb)->daddr))
>        	BR_INPUT_SKB_CB(skb)->mrouters_only = !br_opt_get(br,
BROPT_MCAST_FLOOD_RFC4541);
Exactly! And that is exactly what I had in mind when I wrote it. :)

Thanks,
 Nik
> 
> 
> Best regards
>  /Joachim