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

[Thomas Monjalon <thomas@monjalon.net>]

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.