Comments inline.

Best regards,
Kuba Kozak

-----Original Message-----
From: Wu, Jingjing
Sent: Tuesday, June 20, 2017 04:14
To: Piasecki, JacekX <redacted>; dev@dpdk.org
Cc: Richardson, Bruce <redacted>; Jain, Deepak K <redacted>;
Jastrzebski, MichalX K [off-list ref]; Kozak, KubaX [off-list ref]
Subject: RE: [dpdk-dev] [PATCH 3/3] app/testpmd: changed example to parse options from cfg file

quoted

+
+#ifdef RTE_LIBRTE_CFGFILE
+/* Load config file path from command line */ static char *
+cfgfile_load_path(int argc, char **argv) {
+	int i;
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp("--cfgfile-path", argv[i])) {
+			if (i < argc)
+				return strdup(argv[i+1]);
+		}
+	}
+	return 0;
+}
+#endif
+

It is a little confused. Is the cfgfile-path is application's argument or EAL's?
Where is it supposed to be located?

It is an application argument.

quoted

 int
 main(int argc, char** argv)
 {
 	int  diag;
 	uint8_t port_id;
+	struct rte_cfgfile *cfg = NULL;
+	char *config_file = NULL;

 	signal(SIGINT, signal_handler);
 	signal(SIGTERM, signal_handler);

+#ifdef RTE_LIBRTE_CFGFILE
+	/* load --cfgfile-path argument from argv */
+	config_file = cfgfile_load_path(argc, argv);
+
+	if (config_file) {
+		printf("Info: found cfgfile-path \"%s\"\n", config_file);
+	} else {
+		printf("Info: not found cfgfile-path parameter "
+				"(searching for cfgfile "
+				"in default directory)\n");
+		config_file = strdup("config.ini");
+	}
+	cfg = rte_cfgfile_load(config_file, CFG_FLAG_EMPTY_VALUES);
+	if (cfg == NULL)
+		printf("Info: Valid cfgfile not found\n");
+	rte_eal_configure(cfg);
+#endif
+

Does it mean if RTE_LIBRTE_CFGFILE is defined, then the arguments are passed by Cfgfile, if no cfgfile
will use config.ini by default? How about the legacy command lines?
I think even cfgfile is good, we still need to keep the command line way.

If RTE_LIBRTE_CFGFILE is defined then arguments are passed both by cfgfile and command line. Command line arguments have priority over cfgfile arguments and they override their values.

quoted

 	diag = rte_eal_init(argc, argv);
 	if (diag < 0)
 		rte_panic("Cannot init EAL\n");

@@ -2289,7 +2329,16 @@ uint8_t port_is_bonding_slave(portid_t slave_pid)
 	argc -= diag;
 	argv += diag;
 	if (argc > 1)
-		launch_args_parse(argc, argv);
+		launch_args_parse(argc, argv, cfg);
+

The argc and argv have been overwritten by cfgfile, right? Does it make sense to Pass cfg to
launch_args_parse?

This function translate argv arguments into configuration file form and append/override current cfgfile.
Finally it parse application arguments from cfgfile. 
Intention was to parse arguments from single source (configuration file form).

And you also need to update the testpmd doc to describe this new use.

Thanks
Jingjing