Re: [Bridge] [PATCH net-next 12/14] bridge: mcast: Support replacement of MDB port group entries

[Nikolay Aleksandrov <razor@blackwall.org>]

On 08/12/2022 17:28, Ido Schimmel wrote:
> Now that user space can specify additional attributes of port group
> entries such as filter mode and source list, it makes sense to allow
> user space to atomically modify these attributes by replacing entries
> instead of forcing user space to delete the entries and add them back.
> 
> Replace MDB port group entries when the 'NLM_F_REPLACE' flag is
> specified in the netlink message header.
> 
> When a (*, G) entry is replaced, update the following attributes: Source
> list, state, filter mode, protocol and flags. If the entry is temporary
> and in EXCLUDE mode, reset the group timer to the group membership
> interval. If the entry is temporary and in INCLUDE mode, reset the
> source timers of associated sources to the group membership interval.
> 
> Examples:
> 
>  # bridge mdb replace dev br0 port dummy10 grp 239.1.1.1 permanent source_list 192.0.2.1,192.0.2.2 filter_mode include
>  # bridge -d -s mdb show
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.2 permanent filter_mode include proto static     0.00
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.1 permanent filter_mode include proto static     0.00
>  dev br0 port dummy10 grp 239.1.1.1 permanent filter_mode include source_list 192.0.2.2/0.00,192.0.2.1/0.00 proto static     0.00
> 
>  # bridge mdb replace dev br0 port dummy10 grp 239.1.1.1 permanent source_list 192.0.2.1,192.0.2.3 filter_mode exclude proto zebra
>  # bridge -d -s mdb show
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.3 permanent filter_mode include proto zebra  blocked    0.00
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.1 permanent filter_mode include proto zebra  blocked    0.00
>  dev br0 port dummy10 grp 239.1.1.1 permanent filter_mode exclude source_list 192.0.2.3/0.00,192.0.2.1/0.00 proto zebra     0.00
> 
>  # bridge mdb replace dev br0 port dummy10 grp 239.1.1.1 temp source_list 192.0.2.4,192.0.2.3 filter_mode include proto bgp
>  # bridge -d -s mdb show
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.4 temp filter_mode include proto bgp     0.00
>  dev br0 port dummy10 grp 239.1.1.1 src 192.0.2.3 temp filter_mode include proto bgp     0.00
>  dev br0 port dummy10 grp 239.1.1.1 temp filter_mode include source_list 192.0.2.4/259.44,192.0.2.3/259.44 proto bgp     0.00
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>  net/bridge/br_mdb.c     | 103 ++++++++++++++++++++++++++++++++++++++--
>  net/bridge/br_private.h |   1 +
>  2 files changed, 99 insertions(+), 5 deletions(-)
> 
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index 72d4e53193e5..98d899427c03 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -802,6 +802,28 @@ __br_mdb_choose_context(struct net_bridge *br,
>  	return brmctx;
>  }
>  
> +static int br_mdb_replace_group_sg(const struct br_mdb_config *cfg,
> +				   struct net_bridge_mdb_entry *mp,
> +				   struct net_bridge_port_group *pg,
> +				   struct net_bridge_mcast *brmctx,
> +				   unsigned char flags,
> +				   struct netlink_ext_ack *extack)

extack seems unused here

> +{
> +	unsigned long now = jiffies;
> +
> +	pg->flags = flags;
> +	pg->rt_protocol = cfg->rt_protocol;
> +	if (!(flags & MDB_PG_FLAGS_PERMANENT) && !cfg->src_entry)
> +		mod_timer(&pg->timer,
> +			  now + brmctx->multicast_membership_interval);
> +	else
> +		del_timer(&pg->timer);
> +
> +	br_mdb_notify(cfg->br->dev, mp, pg, RTM_NEWMDB);
> +
> +	return 0;
> +}
> +
>  static int br_mdb_add_group_sg(const struct br_mdb_config *cfg,
>  			       struct net_bridge_mdb_entry *mp,
>  			       struct net_bridge_mcast *brmctx,
[snip]
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index cdc9e040f1f6..2473add41e16 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -107,6 +107,7 @@ struct br_mdb_config {
>  	struct br_mdb_src_entry		*src_entries;
>  	int				num_src_entries;
>  	u8				rt_protocol;
> +	u32				nlflags;

nlmsg_flags is u16 (__u16), also I'd add it before rt_protocol

>  };
>  #endif
>