Re: [dpdk-dev] [PATCH] ethdev: avoid unregistering a non-allocated callback

[Matan Azrad <matan@nvidia.com>]

From: Thomas Monjalon
> 13/07/2021 15:42, Matan Azrad:
> > Hi Thomas
> >
> > From: Thomas Monjalon
> > > When registering a new event callback, if allocation fails, there is
> > > no need for unregistering the callback, because it is not registered.
> > >
> > > Fixes: 9ec0b3869d8d ("ethdev: allow event registration for all
> > > ports")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  lib/ethdev/rte_ethdev.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> > > 9d95cd11e1..1731854628 100644
> > > --- a/lib/ethdev/rte_ethdev.c
> > > +++ b/lib/ethdev/rte_ethdev.c
> > > @@ -4649,8 +4649,6 @@ rte_eth_dev_callback_register(uint16_t
> port_id,
> > >                                                   user_cb, next);
> > >                         } else {
> > >                                 rte_spinlock_unlock(&eth_dev_cb_lock);
> > > -                               rte_eth_dev_callback_unregister(port_id, event,
> > > -                                                               cb_fn, cb_arg);
> >
> > Please pay attention to the case of port_id=RTE_ETH_ALL where the user
> wants to register the event for all the ports.
> >
> > In this case, when a failure happens for one of the ports, this unregister call
> cleans the callback from all the ports.
> 
> Yes I missed it. Now I better understand the intent, thanks.
> 
> Next question: do we really want to rollback already registered ports?
> Anyway, if we are out of memory, I think it is better not doing more
> operations.
> There can be various opinions on this topic, please give yours.

Sure,
I understand that memory error is serious,
Do you think it is a fatal error? If so, maybe we should use rte_exit?

That way or others, I think the behavior should be a convention for all the file functions(at least).

I tend to do cleanup on any error.

Matan