14/07/2021 16:16, Matan Azrad:
> From: Thomas Monjalon
> > 13/07/2021 15:42, Matan Azrad:
> > > 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: sta...@dpdk.org
> > > >
> > > > Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
> > > > ---
> > > > --- a/lib/ethdev/rte_ethdev.c
> > > > +++ b/lib/ethdev/rte_ethdev.c
> > > > @@ -4649,8 +4649,6 @@ rte_eth_dev_callback_register(uint16_t
> > > > } else {
> > > > rte_spinlock_unlock(ð_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?
We don't call rte_exit in the lib, so the app can do whatever it wants.
> That way or others, I think the behavior should be a convention for all the
> file functions(at least).
What do you mean "all the file functions"?
> I tend to do cleanup on any error.
I would like to hear opinions from others as well.
