[v2,1/6] eal: reset lcore function pointer and argument

  In the rte_eal_remote_launch function, the lcore function
pointer is checked for NULL. However, the pointer is never
reset to NULL. Reset the lcore function pointer and argument
after the worker has completed executing the lcore function.

Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang@arm.com>
---
 lib/eal/freebsd/eal_thread.c | 2 ++
 lib/eal/linux/eal_thread.c   | 2 ++
 lib/eal/windows/eal_thread.c | 2 ++
 3 files changed, 6 insertions(+)
  

On Thu, Sep 09, 2021 at 06:13:07PM -0500, Honnappa Nagarahalli wrote:
> In the rte_eal_remote_launch function, the lcore function pointer is
> checked for NULL. However, the pointer is never reset to NULL. Reset the
> lcore function pointer and argument after the worker has completed
> executing the lcore function.
> 
No problems with this patch, but just in general observation.  It would be
good if we had test cases to cover these sorts of problems. If a test were
added to the autotests for this issue we could observe the issue reproduced
and easily verify it were fixed by this patch, giving us a high degree of
confidence in the fix.

Regards,
/Bruce
  

On Fri, Sep 10, 2021 at 9:49 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Sep 09, 2021 at 06:13:07PM -0500, Honnappa Nagarahalli wrote:
> > In the rte_eal_remote_launch function, the lcore function pointer is
> > checked for NULL. However, the pointer is never reset to NULL. Reset the
> > lcore function pointer and argument after the worker has completed
> > executing the lcore function.
> >
> No problems with this patch, but just in general observation.  It would be
> good if we had test cases to cover these sorts of problems. If a test were
> added to the autotests for this issue we could observe the issue reproduced
> and easily verify it were fixed by this patch, giving us a high degree of
> confidence in the fix.

+1
  

<snip>

> 
> On Fri, Sep 10, 2021 at 9:49 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Thu, Sep 09, 2021 at 06:13:07PM -0500, Honnappa Nagarahalli wrote:
> > > In the rte_eal_remote_launch function, the lcore function pointer is
> > > checked for NULL. However, the pointer is never reset to NULL. Reset
> > > the lcore function pointer and argument after the worker has
> > > completed executing the lcore function.
> > >
> > No problems with this patch, but just in general observation.  It
> > would be good if we had test cases to cover these sorts of problems.
> > If a test were added to the autotests for this issue we could observe
> > the issue reproduced and easily verify it were fixed by this patch,
> > giving us a high degree of confidence in the fix.
> 
> +1
Thanks for the comments.
Currently, the eal_thread_loop function calls rte_panic if the function pointer is set to NULL. This needs to be changed to return an error code. However, we need to distinguish between the error codes from EAL and the error codes from the function the lcore would run. Currently, there is no mechanism to return the error codes from EAL.

We could add a new member to " struct lcore_config" to capture the error codes from EAL. We need to add a new API to return the EAL error code to the application.

With this we should be able to add a new test case.

> 
> 
> --
> David Marchand