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

Share & tools

Wang Sheng-Hui

author

2014-04-15 03:03 UTC #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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Thomas Monjalon

2014-04-17 21:58 UTC #2 lore raw

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

Wang Sheng-Hui

author

2014-04-18 00:30 UTC #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

Thomas Monjalon

2014-05-05 15:50 UTC #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

Thomas Monjalon

2014-05-07 13:06 UTC #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