Re: [dpdk-dev] [PATCH v4 10/11] eal: replace rte_panic instances in init sequence

[Arnon Warshavsky <arnon@qwilt.com>]

>
> Looking at the eal_thread_init_master, I think it's probably a
> recoverable condition.  For instance, perhaps the core mask was wrong,
> and could be corrected by re-attempting the initialization.  Just
> suggesting that it's probably okay to allow a re-attempt here.  I would
> suggest:
>
> -       eal_thread_init_master(rte_config.master_lcore);
> +       if (eal_thread_init_master(rte_config.master_lcore) != 0) {
> +               rte_eal_init_alert("Cannot assign master lcore\n");
> +               rte_errno = EINVAL;
> +               return -1;
> +       }
>
> if you agree.
>
> Yes. This is indeed the way to go.

>
> --
>
> In the above I hope it illustrates what you'll need - a way to signal to
> each side that initialization phase is happening, and that
> initialization was successful / failed, and to clean up in the failure
> case.
>
> Just meant for illustration so feel free to ignore / flame, but that's
> how I would go about removing the rte_panic() calls.
>

Thanks.
Indeed I did not consider recovery and clean up from here

>
> >  This seems to only exist as a way of triggering the run_once check in
> >  the eal_init.  It doesn't add anything except one more state variable to
> >  check against.  What is the purpose?
> >
> >
> > Actually this is not a run-once in purpose, rather an attempt to define
> a state for the device
> > and on the way work around breaking abi on the the void function called
> before that.
>
> I think it's a way to try and track state for initialization and to
> prevent future inits.  Which ABI are you worried about?  rte_panic()?
> I'm not sure how this is an ABI work around, but I'm probably not
> thinking about it hard enough.
>

I now see the mess I did and why wasn't it clear.
I initially changed the api of eal_thread_init_master() from void to int.
Having had the ABI check failed, I only fixed the linuxapp back to void and
added this workaround to prevent ABI break on this patch.

I will align both linuxapp and bsd to remain at panic for this function as
well , and address it together with the thread itself




-- 

*Arnon Warshavsky*
*Qwilt | work: +972-72-2221634 | mobile: +972-50-8583058 | arnon@qwilt.com
<arnon@qwilt.com>*