Re: [net-next PATCH v6 1/2] octeontx2-af: Add new mbox to support multicast/mirror offload

[Paolo Abeni]

On Mon, 2023-12-04 at 19:49 +0530, Suman Ghosh wrote:
> A new mailbox is added to support offloading of multicast/mirror
> functionality. The mailbox also supports dynamic updation of the
> multicast/mirror list.
> 
> Signed-off-by: Suman Ghosh <sumang@xxxxxxxxxxx>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@xxxxxxxxx>
> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>

Note that v5 was already applied to net-next. But I still have a
relevant note, see below.

> @@ -5797,3 +6127,337 @@ int rvu_mbox_handler_nix_bandprof_get_hwinfo(struct rvu *rvu, struct msg_req *re
>  
>  	return 0;
>  }
> +
> +static struct nix_mcast_grp_elem *rvu_nix_mcast_find_grp_elem(struct nix_mcast_grp *mcast_grp,
> +							      u32 mcast_grp_idx)
> +{
> +	struct nix_mcast_grp_elem *iter;
> +	bool is_found = false;
> +
> +	mutex_lock(&mcast_grp->mcast_grp_lock);
> +	list_for_each_entry(iter, &mcast_grp->mcast_grp_head, list) {
> +		if (iter->mcast_grp_idx == mcast_grp_idx) {
> +			is_found = true;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mcast_grp->mcast_grp_lock);

AFAICS, at this point another thread/CPU could kick-in and run
rvu_mbox_handler_nix_mcast_grp_destroy() up to completion, freeing
'iter' before it's later used by the current thread.

What prevents such scenario?

_If_ every mcast group manipulation happens under the rtnl lock, then
you could as well completely remove the confusing mcast_grp_lock.

Cheers,

Paolo