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

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Sun, Dec 13, 2020 at 01:08:55AM +0100, Andrew Lunn wrote:
> > > And you need some way to cleanup the allocated memory when the commit
> > > never happens because some other layer has said No!
> >
> > So this would be a fatal problem with the switchdev transactional model
> > if I am not misunderstanding it. On one hand there's this nice, bubbly
> > idea that you should preallocate memory in the prepare phase, so that
> > there's one reason less to fail at commit time. But on the other hand,
> > if "the commit phase might never happen" is even a remove possibility,
> > all of that goes to trash - how are you even supposed to free the
> > preallocated memory.
>
> It can definitely happen, that commit is never called:
>
> static int switchdev_port_obj_add_now(struct net_device *dev,
>                                       const struct switchdev_obj *obj,
>                                       struct netlink_ext_ack *extack)
> {
>
>        /* Phase I: prepare for obj add. Driver/device should fail
>          * here if there are going to be issues in the commit phase,
>          * such as lack of resources or support.  The driver/device
>          * should reserve resources needed for the commit phase here,
>          * but should not commit the obj.
>          */
>
>         trans.ph_prepare = true;
>         err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
>                                         dev, obj, &trans, extack);
>         if (err)
>                 return err;
>
>         /* Phase II: commit obj add.  This cannot fail as a fault
>          * of driver/device.  If it does, it's a bug in the driver/device
>          * because the driver said everythings was OK in phase I.
>          */
>
>         trans.ph_prepare = false;
>         err = switchdev_port_obj_notify(SWITCHDEV_PORT_OBJ_ADD,
>                                         dev, obj, &trans, extack);
>         WARN(err, "%s: Commit of object (id=%d) failed.\n", dev->name, obj->id);
>
>         return err;
>
> So if any notifier returns an error during prepare, the commit is
> never called.
>
> So the memory you allocated and added to the list may never get
> used. Its refcount stays zero.  Which is why i suggested making the
> MDB remove call do a general garbage collect. It is not perfect, the
> cleanup could be deferred a long time, but is should get removed
> eventually.

What would the garbage collection look like?