All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nikolay Aleksandrov <razor@blackwall.org>
To: Joachim Wiberg <troglobit@gmail.com>, Roopa Prabhu <roopa@nvidia.com>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Tobias Waldekranz <tobias@waldekranz.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>
Subject: Re: [PATCH RFC net-next 08/13] net: bridge: avoid classifying unknown multicast as mrouters_only
Date: Wed, 13 Apr 2022 11:55:09 +0300	[thread overview]
Message-ID: <96bb8ff0-26d8-e9d3-e7c8-78f2abd28126@blackwall.org> (raw)
In-Reply-To: <87pmll9xj1.fsf@gmail.com>

On 13/04/2022 11:51, Joachim Wiberg wrote:
> On Tue, Apr 12, 2022 at 20:37, Nikolay Aleksandrov <razor@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


WARNING: multiple messages have this Message-ID (diff)
From: Nikolay Aleksandrov <razor@blackwall.org>
To: Joachim Wiberg <troglobit@gmail.com>, Roopa Prabhu <roopa@nvidia.com>
Cc: netdev@vger.kernel.org, bridge@lists.linux-foundation.org,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Jakub Kicinski <kuba@kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Tobias Waldekranz <tobias@waldekranz.com>
Subject: Re: [Bridge] [PATCH RFC net-next 08/13] net: bridge: avoid classifying unknown multicast as mrouters_only
Date: Wed, 13 Apr 2022 11:55:09 +0300	[thread overview]
Message-ID: <96bb8ff0-26d8-e9d3-e7c8-78f2abd28126@blackwall.org> (raw)
In-Reply-To: <87pmll9xj1.fsf@gmail.com>

On 13/04/2022 11:51, Joachim Wiberg wrote:
> On Tue, Apr 12, 2022 at 20:37, Nikolay Aleksandrov <razor@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


  reply	other threads:[~2022-04-13  8:55 UTC|newest]

Thread overview: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-11 13:38 [PATCH RFC net-next 00/13] net: bridge: forwarding of unknown IPv4/IPv6/MAC BUM traffic Joachim Wiberg
2022-04-11 13:38 ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 01/13] net: bridge: add control of bum flooding to bridge itself Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-12 18:27   ` Nikolay Aleksandrov
2022-04-12 18:27     ` [Bridge] " Nikolay Aleksandrov
2022-04-12 20:29     ` Nikolay Aleksandrov
2022-04-12 20:29       ` [Bridge] " Nikolay Aleksandrov
2022-04-13  9:51     ` Joachim Wiberg
2022-04-13  9:51       ` [Bridge] " Joachim Wiberg
2022-04-13  9:58       ` Nikolay Aleksandrov
2022-04-13  9:58         ` [Bridge] " Nikolay Aleksandrov
2022-04-13 10:09         ` Joachim Wiberg
2022-04-13 10:09           ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 02/13] net: bridge: rename br_switchdev_set_port_flag() to .._dev_flag() Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 03/13] net: bridge: minor refactor of br_setlink() for readability Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-12 18:36   ` Nikolay Aleksandrov
2022-04-12 18:36     ` [Bridge] " Nikolay Aleksandrov
2022-04-13  9:22     ` Joachim Wiberg
2022-04-13  9:22       ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 04/13] net: bridge: netlink support for controlling BUM flooding to bridge Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-12 18:24   ` Nikolay Aleksandrov
2022-04-12 18:24     ` [Bridge] " Nikolay Aleksandrov
2022-04-13 10:04     ` Joachim Wiberg
2022-04-13 10:04       ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 05/13] selftests: forwarding: add TCPDUMP_EXTRA_FLAGS to lib.sh Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 17:20   ` Vladimir Oltean
2022-04-11 17:20     ` [Bridge] " Vladimir Oltean
2022-04-12  7:39     ` Joachim Wiberg
2022-04-12  7:39       ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 06/13] selftests: forwarding: multiple instances in tcpdump helper Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 17:26   ` Vladimir Oltean
2022-04-11 17:26     ` [Bridge] " Vladimir Oltean
2022-04-11 13:38 ` [PATCH RFC net-next 07/13] selftests: forwarding: new test, verify bridge flood flags Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 20:21   ` Vladimir Oltean
2022-04-11 20:21     ` [Bridge] " Vladimir Oltean
2022-04-12  7:55     ` Joachim Wiberg
2022-04-12  7:55       ` [Bridge] " Joachim Wiberg
2022-04-12 13:40       ` Vladimir Oltean
2022-04-12 13:40         ` [Bridge] " Vladimir Oltean
2022-04-11 13:38 ` [PATCH RFC net-next 08/13] net: bridge: avoid classifying unknown multicast as mrouters_only Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-12 13:59   ` Nikolay Aleksandrov
2022-04-12 13:59     ` [Bridge] " Nikolay Aleksandrov
2022-04-12 17:27     ` Joachim Wiberg
2022-04-12 17:27       ` [Bridge] " Joachim Wiberg
2022-04-12 17:37       ` Nikolay Aleksandrov
2022-04-12 17:37         ` [Bridge] " Nikolay Aleksandrov
2022-04-13  8:51         ` Joachim Wiberg
2022-04-13  8:51           ` [Bridge] " Joachim Wiberg
2022-04-13  8:55           ` Nikolay Aleksandrov [this message]
2022-04-13  8:55             ` Nikolay Aleksandrov
2022-04-13  9:00             ` Nikolay Aleksandrov
2022-04-13  9:00               ` [Bridge] " Nikolay Aleksandrov
2022-04-13 10:12               ` Joachim Wiberg
2022-04-13 10:12                 ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 09/13] selftests: forwarding: rename test groups for next bridge mdb tests Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 20:23   ` Vladimir Oltean
2022-04-11 20:23     ` [Bridge] " Vladimir Oltean
2022-04-12  7:57     ` Joachim Wiberg
2022-04-12  7:57       ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 10/13] selftests: forwarding: verify flooding of unknown multicast Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 11/13] selftests: forwarding: verify strict mdb fwd of known multicast Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 12/13] selftests: forwarding: verify strict filtering doesn't leak Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg
2022-04-11 13:38 ` [PATCH RFC net-next 13/13] selftests: forwarding: verify flood of known mc on mcast_router port Joachim Wiberg
2022-04-11 13:38   ` [Bridge] " Joachim Wiberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=96bb8ff0-26d8-e9d3-e7c8-78f2abd28126@blackwall.org \
    --to=razor@blackwall.org \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@nvidia.com \
    --cc=tobias@waldekranz.com \
    --cc=troglobit@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.