Skip to contentSkip to conversation

eal: parse args before any kinds of init

PATCH In review Wang Sheng-Hui 2 participants 5 messages Apr 15, 2014 – May 7, 2014 1 message not in the archive

lore
Share & tools
lore

Wang Sheng-Hui

author
#0 lore raw

PATCH 1/1 eal: parse args before any kinds of init

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 <[email protected]>
---
 lib/librte_eal/linuxapp/eal/eal.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
lib/librte_eal/linuxapp/eal/eal.c +9 -4
index 3ded563..9d20718 100644
@@ -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 &&
Signature · 1 line
1.8.3.2
#2 lore raw
In reply to a message not in the archive

Hi,

2014-04-15 11:03, Wang Sheng-Hui:

Quoted 5 lines

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 <[email protected]>

[...]

Quoted 4 lines
lib/librte_eal/linuxapp/eal/eal.c +4 -0
+       fctret = eal_parse_args(argc, argv);
+       if (fctret < 0)
+               exit(1);
+

lib/librte_eal/linuxapp/eal/eal.c rte_eal_init(int argc, char **argv) · line 976

Quoted 10 lines
lib/librte_eal/linuxapp/eal/eal.c +0 -4
        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.

Signature · 1 line
Thomas
author
#3 lore raw

Thanks, Thomas.

2014-04-18 5:58 GMT+08:00 Thomas Monjalon <[email protected]>:

Quoted 36 lines

Hi,

2014-04-15 11:03, Wang Sheng-Hui:

Quoted 1 line

Parse args first, to resolve any invalid args and give out the usage

string.

Quoted 4 lines

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 <[email protected]>

[...]

Quoted 4 lines
lib/librte_eal/linuxapp/eal/eal.c +4 -0
+       fctret = eal_parse_args(argc, argv);
+       if (fctret < 0)
+               exit(1);
+

lib/librte_eal/linuxapp/eal/eal.c rte_eal_init(int argc, char **argv) · line 976

Quoted 10 lines
lib/librte_eal/linuxapp/eal/eal.c +0 -4
        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

#4 lore raw
In reply to a message not in the archive

2014-04-15 11:03, Wang Sheng-Hui:

Quoted 5 lines

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 <[email protected]>

[...]

lib/librte_eal/linuxapp/eal/eal.c eal_parse_args(int argc, char **argv) · line 645

Quoted 10 lines
lib/librte_eal/linuxapp/eal/eal.c +5 -0
@@ -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.

Quoted 20 lines
lib/librte_eal/linuxapp/eal/eal.c +4 -4
@@ -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
Quoted 1 line

&& 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

Signature · 1 line
Thomas
#5 lore raw

2014-05-05 17:50, Thomas Monjalon:

Quoted 25 lines

2014-04-15 11:03, Wang Sheng-Hui:

Quoted 5 lines

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 <[email protected]>

[...]

Quoted 17 lines
lib/librte_eal/linuxapp/eal/eal.c +4 -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");
Quoted 7 lines

lib/librte_eal/linuxapp/eal/eal.c rte_eal_init(int argc, char **argv) · line 976

Quoted 4 lines
lib/librte_eal/linuxapp/eal/eal.c +0 -4
-       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.

Signature · 1 line
Thomas