Re: [Bridge] [PATCH net-next 09/13] bridge: mcast: Add MDB get support

[Nikolay Aleksandrov <razor@blackwall.org>]

On 10/16/23 16:12, Ido Schimmel wrote:
> Implement support for MDB get operation by looking up a matching MDB
> entry, allocating the skb according to the entry's size and then filling
> in the response. The operation is performed under the bridge multicast
> lock to ensure that the entry does not change between the time the reply
> size is determined and when the reply is filled in.
> 
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> ---
>   net/bridge/br_device.c  |   1 +
>   net/bridge/br_mdb.c     | 154 ++++++++++++++++++++++++++++++++++++++++
>   net/bridge/br_private.h |   9 +++
>   3 files changed, 164 insertions(+)
> 
[snip]
> +int br_mdb_get(struct net_device *dev, struct nlattr *tb[], u32 portid, u32 seq,
> +	       struct netlink_ext_ack *extack)
> +{
> +	struct net_bridge *br = netdev_priv(dev);
> +	struct net_bridge_mdb_entry *mp;
> +	struct sk_buff *skb;
> +	struct br_ip group;
> +	int err;
> +
> +	err = br_mdb_get_parse(dev, tb, &group, extack);
> +	if (err)
> +		return err;
> +
> +	spin_lock_bh(&br->multicast_lock);

Since this is only reading, could we use rcu to avoid blocking mcast 
processing?

> +
> +	mp = br_mdb_ip_get(br, &group);
> +	if (!mp) {
> +		NL_SET_ERR_MSG_MOD(extack, "MDB entry not found");
> +		err = -ENOENT;
> +		goto unlock;
> +	}
> +
> +	skb = br_mdb_get_reply_alloc(mp);
> +	if (!skb) {
> +		err = -ENOMEM;
> +		goto unlock;
> +	}
> +
> +	err = br_mdb_get_reply_fill(skb, mp, portid, seq);
> +	if (err) {
> +		NL_SET_ERR_MSG_MOD(extack, "Failed to fill MDB get reply");
> +		goto free;
> +	}
> +
> +	spin_unlock_bh(&br->multicast_lock);
> +
> +	return rtnl_unicast(skb, dev_net(dev), portid);
> +
> +free:
> +	kfree_skb(skb);
> +unlock:
> +	spin_unlock_bh(&br->multicast_lock);
> +	return err;
> +}