Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Joachim Wiberg <troglobit@gmail.com>
To:     Nikolay Aleksandrov <nikolay@nvidia.com>,
        Horatiu Vultur <horatiu.vultur@microchip.com>,
        davem@davemloft.net, kuba@kernel.org, roopa@nvidia.com,
        allan.nielsen@microchip.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, bridge@lists.linux-foundation.org
Subject: Re: [RFC PATCH v2] net: bridge: igmp: Extend IGMP query to be per vlan
In-Reply-To: <32bf6a72-6aff-5e36-fb02-333f3c450f49@nvidia.com>
References: <20210112135903.3730765-1-horatiu.vultur@microchip.com> <32bf6a72-6aff-5e36-fb02-333f3c450f49@nvidia.com>
Date:   Sat, 16 Jan 2021 16:39:24 +0100
Message-ID: <8735z0zyab.fsf@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, Jan 13, 2021 at 14:15, Nikolay Aleksandrov <nikolay@nvidia.com> wrote:
> On 12/01/2021 15:59, Horatiu Vultur wrote:
>> Based on the comments of the previous version, we started to work on a
>> new version, so it would be possible to enable/disable queries per vlan.
>> [snip]
>> We were wondering if this what you had in mind when you proposed to have
>> this per vlan? Or we are completely off? Or we should fix some of the
>> issues that I mentioned, before you can see more clearly the direction?
> No, unfortunately not even close. We already have per-port per-vlan and global per-vlan
> contexts which are also linked together for each vlan, those must be used for any vlan
> configuration and state. The problem is that you'd have to mix igmp and vlan code and
> those two live under two different kconfig options, and worse rely on different locks, so
> extra care must be taken.
> [snip]
> If you don't need this asap, I'll probably get to it in two months
> after EHT and the new bridge flush api, even we are still carrying an out-of-tree patch
> for this which someone (not from cumulus) tried to upstream a few years back, but it also has
> wrong design in general. :)

Hi,

very interesting thread this!  I believe I may be the one who posted the
patch[1] a few years ago, and I fully agree with Nik here.  We developed
the basic concepts further at Westermo, but it's been really difficult
to get it stable.

We have discussed at length at work if an IGMP snooping implementation
really belongs in the bridge, or if it's better suited as a user space
daemon?  Similar to what was decided for RSTP/MSTP support, i.e., the
bridge only has STP and RSTP/MSTP is handled by mstpd[2].

Most of what's required for a user space implementation is available,
but it would've been nice if a single AF_PACKET socket on br0 could be
used to catch what brport (ifindex) a query or report comes in on.  As
it is now that information is lost/replaced with the ifindex of br0.
And then there's the issue of detecting and forwarding to a multicast
routing daemon on top of br0.  That br0 is not a brport in the MDB, or
that host_joined cannot be set/seen with iproute2 is quite limiting.
These issues can of course be addressed, but are they of interest to
the community at large?


Best regards
 /Joachim

[1]: https://lore.kernel.org/netdev/20180418120713.GA10742@troglobit/
[2]: https://github.com/mstpd/mstpd