[dpdk-dev] [PATCH] eal: parse args before any kinds of init

[dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Wang Sheng-Hui]

Parse args first, to resolve any invalid args and give out the usage string.
E.g './helloworld --invalid', the '--invalid' will be checked before any init.
After the options are checked, take any init actions.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 3ded563..9d20718 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -637,6 +637,11 @@ eal_parse_args(int argc, char **argv)
        };
        struct shared_driver *solib;

+       /* check the no args case */
+       if (argc == 1) {
+               eal_usage(prgname);
+               return (-1);
+       }
        argvopt = argv;

        internal_config.memory = 0;
@@ -964,16 +969,16 @@ rte_eal_init(int argc, char **argv)

        thread_id = pthread_self();

+       fctret = eal_parse_args(argc, argv);
+       if (fctret < 0)
+               exit(1);
+
        if (rte_eal_log_early_init() < 0)
                rte_panic("Cannot init early logs\n");

        if (rte_eal_cpu_init() < 0)
                rte_panic("Cannot detect lcores\n");

-       fctret = eal_parse_args(argc, argv);
-       if (fctret < 0)
-               exit(1);
-
        if (internal_config.no_hugetlbfs == 0 &&
                        internal_config.process_type != RTE_PROC_SECONDARY &&
                        internal_config.xen_dom0_support == 0 &&
-- 
1.8.3.2

Re: [dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Thomas Monjalon]

Hi,

2014-04-15 11:03, Wang Sheng-Hui:
> Parse args first, to resolve any invalid args and give out the usage string.
> E.g './helloworld --invalid', the '--invalid' will be checked before any
> init. After the options are checked, take any init actions.
> 
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
[...]
> +       fctret = eal_parse_args(argc, argv);
> +       if (fctret < 0)
> +               exit(1);
> +
>         if (rte_eal_log_early_init() < 0)
>                 rte_panic("Cannot init early logs\n");
> 
>         if (rte_eal_cpu_init() < 0)
>                 rte_panic("Cannot detect lcores\n");
> 
> -       fctret = eal_parse_args(argc, argv);
> -       if (fctret < 0)
> -               exit(1);
> -

Thank you for trying to improve this part.

I think you cannot move eal_parse_args before rte_eal_log_early_init because 
eal_parse_args uses RTE_LOG.
I cannot see why rte_eal_cpu_init is call before argument parsing but we 
should double check it.

-- 
Thomas

Re: [dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Wang Sheng-Hui]

Thanks, Thomas.



2014-04-18 5:58 GMT+08:00 Thomas Monjalon <thomas.monjalon@6wind.com>:

> Hi,
>
> 2014-04-15 11:03, Wang Sheng-Hui:
> > Parse args first, to resolve any invalid args and give out the usage
> string.
> > E.g './helloworld --invalid', the '--invalid' will be checked before any
> > init. After the options are checked, take any init actions.
> >
> > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> [...]
> > +       fctret = eal_parse_args(argc, argv);
> > +       if (fctret < 0)
> > +               exit(1);
> > +
> >         if (rte_eal_log_early_init() < 0)
> >                 rte_panic("Cannot init early logs\n");
> >
> >         if (rte_eal_cpu_init() < 0)
> >                 rte_panic("Cannot detect lcores\n");
> >
> > -       fctret = eal_parse_args(argc, argv);
> > -       if (fctret < 0)
> > -               exit(1);
> > -
>
> Thank you for trying to improve this part.
>
> I think you cannot move eal_parse_args before rte_eal_log_early_init
> because
> eal_parse_args uses RTE_LOG.
> I cannot see why rte_eal_cpu_init is call before argument parsing but we
> should double check it.
>
> --
> Thomas
>

Re: [dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Thomas Monjalon]

2014-04-15 11:03, Wang Sheng-Hui:
> Parse args first, to resolve any invalid args and give out the usage string.
> E.g './helloworld --invalid', the '--invalid' will be checked before any
> init. After the options are checked, take any init actions.
> 
> Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
[...]
> @@ -637,6 +637,11 @@ eal_parse_args(int argc, char **argv)
>         };
>         struct shared_driver *solib;
> 
> +       /* check the no args case */
> +       if (argc == 1) {
> +               eal_usage(prgname);
> +               return (-1);
> +       }
>         argvopt = argv;

I wonder if there are some use cases where default values could be used for 
first tests.
If we can set some default values for coremask and memory channels, then we 
can accept having no argument.
Maybe that an option -h would be needed.

> @@ -964,16 +969,16 @@ rte_eal_init(int argc, char **argv)
> 
>         thread_id = pthread_self();
> 
> +       fctret = eal_parse_args(argc, argv);
> +       if (fctret < 0)
> +               exit(1);
> +
>         if (rte_eal_log_early_init() < 0)
>                 rte_panic("Cannot init early logs\n");
> 
>         if (rte_eal_cpu_init() < 0)
>                 rte_panic("Cannot detect lcores\n");
> 
> -       fctret = eal_parse_args(argc, argv);
> -       if (fctret < 0)
> -               exit(1);
> -
>         if (internal_config.no_hugetlbfs == 0 &&
>                         internal_config.process_type != RTE_PROC_SECONDARY
> && internal_config.xen_dom0_support == 0 &&

You should move eal_parse_args() just after rte_eal_log_early_init() in order 
to have logs available.

Could you send a v2 patch with this kind of change for BSD also.

Thank you
-- 
Thomas

Re: [dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Thomas Monjalon]

2014-05-05 17:50, Thomas Monjalon:
> 2014-04-15 11:03, Wang Sheng-Hui:
> > Parse args first, to resolve any invalid args and give out the usage
> > string. E.g './helloworld --invalid', the '--invalid' will be checked
> > before any init. After the options are checked, take any init actions.
> > 
> > Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
> 
> [...]
> > @@ -964,16 +969,16 @@ rte_eal_init(int argc, char **argv)
> > 
> >         thread_id = pthread_self();
> > 
> > +       fctret = eal_parse_args(argc, argv);
> > +       if (fctret < 0)
> > +               exit(1);
> > +
> > 
> >         if (rte_eal_log_early_init() < 0)
> >         
> >                 rte_panic("Cannot init early logs\n");
> >         
> >         if (rte_eal_cpu_init() < 0)
> >         
> >                 rte_panic("Cannot detect lcores\n");
> > 
> > -       fctret = eal_parse_args(argc, argv);
> > -       if (fctret < 0)
> > -               exit(1);
> > -
> 
> You should move eal_parse_args() just after rte_eal_log_early_init() in
> order to have logs available.

When double checking, I saw this commit which justify why rte_eal_cpu_init() 
is before eal_parse_args():
	http://dpdk.org/browse/dpdk/commit/?id=f563a3727b5dba

If the goal is to move debug lines in cpu_init, you should split 
rte_eal_log_early_init() in 2 functions: 1 to detect cores and 1 for debug 
summary.
By the way, these are debug logs which should be disabled by default.

-- 
Thomas

[dpdk-dev] [PATCH] eal: parse args before any kinds of init

[Wang Sheng-Hui]

Parse args first, to resolve any invalid args and give out the usage string.
E.g './helloworld --invalid', the '--invalid' will be checked before any init.
After the options are checked, take any init actions.

Signed-off-by: Wang Sheng-Hui <shhuiw@gmail.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 3ded563..9d20718 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -637,6 +637,11 @@ eal_parse_args(int argc, char **argv)
        };
        struct shared_driver *solib;

+       /* check the no args case */
+       if (argc == 1) {
+               eal_usage(prgname);
+               return (-1);
+       }
        argvopt = argv;

        internal_config.memory = 0;
@@ -964,16 +969,16 @@ rte_eal_init(int argc, char **argv)

        thread_id = pthread_self();

+       fctret = eal_parse_args(argc, argv);
+       if (fctret < 0)
+               exit(1);
+
        if (rte_eal_log_early_init() < 0)
                rte_panic("Cannot init early logs\n");

        if (rte_eal_cpu_init() < 0)
                rte_panic("Cannot detect lcores\n");

-       fctret = eal_parse_args(argc, argv);
-       if (fctret < 0)
-               exit(1);
-
        if (internal_config.no_hugetlbfs == 0 &&
                        internal_config.process_type != RTE_PROC_SECONDARY &&
                        internal_config.xen_dom0_support == 0 &&
-- 
1.8.3.2