On 05/12/2022 09:42, Ido Schimmel wrote: > This patchset was split from [1] and includes non-functional changes > aimed at making it easier to add additional netlink attributes later on. > Future extensions are available here [2]. > > The idea behind these patches is to create an MDB configuration > structure into which netlink messages are parsed into. The structure is > then passed in the entry creation / deletion call chain instead of > passing the netlink attributes themselves. The same pattern is used by > other rtnetlink objects such as routes and nexthops. > > I initially tried to extend the current code, but it proved to be too > difficult, which is why I decided to refactor it to the extensible and > familiar pattern used by other rtnetlink objects. > > Tested using existing selftests and using a new selftest that will be > submitted together with the planned extensions. > > No changes since initial RFC. > > [1] https://lore.kernel.org/netdev/20221018120420.561846-1-idosch@xxxxxxxxxx/ > [2] https://github.com/idosch/linux/commits/submit/mdb_v1 > > Ido Schimmel (8): > bridge: mcast: Centralize netlink attribute parsing > bridge: mcast: Remove redundant checks > bridge: mcast: Use MDB configuration structure where possible > bridge: mcast: Propagate MDB configuration structure further > bridge: mcast: Use MDB group key from configuration structure > bridge: mcast: Remove br_mdb_parse() > bridge: mcast: Move checks out of critical section > bridge: mcast: Remove redundant function arguments > > net/bridge/br_mdb.c | 312 +++++++++++++++++++--------------------- > net/bridge/br_private.h | 7 + > 2 files changed, 156 insertions(+), 163 deletions(-) > As I also commented on the RFC, nice work! Allowing user-space to manipulate and manually install such entries is a natural extension. One thought (not a big deal) but it would've been ideal if we could initialize the config struct once when parsing and then pass it around as a const argument. I know that its arguments are currently passed to functions that don't expect const, but I *think* it could be a small change. Thanks, Nik
