Received-SPF: pass (google.com: domain of linux-kernel+bounces-164484-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <b90caf5f-fa1e-41e6-a7c2-5af042b0828e@gmail.com>
Date: Tue, 30 Apr 2024 13:01:08 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH RFC net-next 00/10] MC Flood disable and snooping
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>,
 Joseph Huang <Joseph.Huang@garmin.com>, netdev@vger.kernel.org,
 Andrew Lunn <andrew@lunn.ch>, Florian Fainelli <f.fainelli@gmail.com>,
 "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>,
 Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
 Roopa Prabhu <roopa@nvidia.com>, =?UTF-8?Q?Linus_L=C3=BCssing?=
 <linus.luessing@c0d3.blue>, linux-kernel@vger.kernel.org,
 bridge@lists.linux.dev
References: <20240402174348.wosc37adyub5o7xu@skbuf>
 <a8968719-a63b-4969-a971-173c010d708f@blackwall.org>
 <20240402204600.5ep4xlzrhleqzw7k@skbuf>
 <065b803f-14a9-4013-8f11-712bb8d54848@blackwall.org>
 <804b7bf3-1b29-42c4-be42-4c23f1355aaf@gmail.com>
 <20240405102033.vjkkoc3wy2i3vdvg@skbuf>
 <935c18c1-7736-416c-b5c5-13ca42035b1f@blackwall.org>
 <651c87fc-1f21-4153-bade-2dad048eecbd@gmail.com>
 <20240405211502.q5gfwcwyhkm6w7xy@skbuf>
 <1f385946-84d0-499c-9bf6-90ef65918356@gmail.com>
 <20240430012159.rmllu5s5gcdepjnc@skbuf>
Content-Language: en-US
From: Joseph Huang <joseph.huang.2024@gmail.com>
In-Reply-To: <20240430012159.rmllu5s5gcdepjnc@skbuf>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/29/2024 9:21 PM, Vladimir Oltean wrote:
> On Mon, Apr 29, 2024 at 04:14:03PM -0400, Joseph Huang wrote:
>> How about the following syntax? I think it satisfies all the "not breaking
>> existing behavior" requirements (new option defaults to off, and missing
>> user space netlink attributes does not change the existing behavior):
>>
>> mcast_flood off
>>    all off
>> mcast_flood off mcast_flood_rfc4541 off
>>    all off
>> mcast_flood off mcast_flood_rfc4541 on
>>    224.0.0.X and ff02::1 on, the rest off
>> mcast_flood on
>>    all on
>> mcast_flood on mcast_flood_rfc4541 off
>>    all on (mcast_flood on overrides mcast_flood_rfc4541)
>> mcast_flood on mcast_flood_rfc4541 on
>>    all on
>> mcast_flood_rfc4541 off
>>    invalid (mcast_flood_rfc4541 is only valid if mcast_flood [on | off] is
>> specified first)
>> mcast_flood_rfc4541 on
>>    invalid (mcast_flood_rfc4541 is only valid if mcast_flood [on | off] is
>> specified first)
> 
> A bridge port defaults to having BR_MCAST_FLOOD set - see new_nbp().
> Netlink attributes are only there to _change_ the state of properties in
> the kernel. They don't need to be specified by user space if there's
> nothing to be changed. "Only valid if another netlink attribute comes
> first" makes no sense. You can alter 2 bridge port flags as part of the
> same netlink message, or as part of different netlink messages (sent
> over sockets of other processes).
> 
>>
>> Think of mcast_flood_rfc4541 like a pet door if you will.
> 
> Ultimately, as far as I see it, both the OR-based and the AND-based UAPI
> addition could be made to work in a way that's perhaps similarly backwards
> compatible. It needs to be worked out with the bridge maintainers. Given
> that I'm not doing great with my spare time, I will take a back seat on
> that.

Nik, do you have any objection to the following proposal?

mcast_flood ->          default/    off         on
(existing flag)         missing     (specified/ (specified/
                         (on)        nlmsg)      nlmsg)

mcast_flood_rfc4541
(proposed new flag)
      |
      v
default/                flood all   no flood    flood all
missing
(off)

off                     flood all   no flood    flood all
(specified/nlmsg)

on                      flood all   flood 4541  flood all
(specified/nlmsg)                   ^^^^^^^^^^
                                     only behavior change


Basically the attributes are OR'ed together to form the final flooding 
decision.


> 
> However, what I don't quite understand in your proposal is how many IPv4
> addresses lie beyond the "224.0.0.X" notation? 256? Explain why there is
> such a large discrepancy in the number of IPv4 addresses you are in
> control of (256), vs only 1 IPv6 address with the new rfc4541 flag?

That's straight out of RFC-4541 ("coincidentally", these are also the IP 
addresses for which the bridge will not create mdb's):

2.1.2.

    2) Packets with a destination IP (DIP) address in the 224.0.0.X range
       which are not IGMP must be forwarded on all ports.

       This recommendation is based on the fact that many host systems do
       not send Join IP multicast addresses in this range before sending
       or listening to IP multicast packets.  Furthermore, since the
       224.0.0.X address range is defined as link-local (not to be
       routed), it seems unnecessary to keep the state for each address
       in this range.  Additionally, some routers operate in the
       224.0.0.X address range without issuing IGMP Joins, and these
       applications would break if the switch were to prune them due to
       not having seen a Join Group message from the router.

and

3.

In IPv6, the data forwarding rules are more straight forward because
    MLD is mandated for addresses with scope 2 (link-scope) or greater.
    The only exception is the address FF02::1 which is the all hosts
    link-scope address for which MLD messages are never sent.  Packets
    with the all hosts link-scope address should be forwarded on all
    ports.