Re: [PATCH v3 net-next] net: dsa: reference count the host mdb addresses

From: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Tobias Waldekranz <tobias@waldekranz.com>
Subject: Re: [PATCH v3 net-next] net: dsa: reference count the host mdb addresses
Date: Sat, 12 Dec 2020 23:06:41 +0100	[thread overview]
Message-ID: <20201212220641.GA2781095@lunn.ch> (raw)
In-Reply-To: <20201212203901.351331-1-vladimir.oltean@nxp.com>

> +	/* Complication created by the fact that addition has two phases, but
> +	 * deletion only has one phase, and we need reference counting.
> +	 * The strategy is to do the memory allocation in the prepare phase,
> +	 * but initialize the refcount in the commit phase.
> +	 *
> +	 * Have mdb	| mdb has refcount > 0	| Commit phase	| Resolution
> +	 * -------------+-----------------------+---------------+---------------
> +	 * no		| -			| no		| Alloc & proceed

This does not look right.

The point of the prepare phase is to allow all the different layers
involved to allocate whatever they need and to validate they can do
the requested action. Any layer can say, No, stop, i cannot do
this. The commit phase will then not happen. But that also means the
prepare phase should not do any state changes. So you should not be
proceeding here, just allocating.

And you need some way to cleanup the allocated memory when the commit
never happens because some other layer has said No!

     Andrew