[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Keith Wiles]

Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal.c         | 20 ++++++++++++++++++++
 lib/librte_eal/common/include/rte_eal.h | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 43e8a47..a228576 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -557,6 +557,26 @@ rte_eal_init(int argc, char **argv)
 	return fctret;
 }
 
+/* Launch threads, called at application init() and parse app args. */
+int
+rte_eal_init_parse(int argc, char **argv,
+		int (*parse)(int, char **))
+{
+	int	ret;
+
+	ret = rte_eal_init(argc, argv);
+	if ((ret >= 0) && (parse != NULL)) {
+		argc -= ret;
+		argv += ret;
+
+		int rval = parse(argc, argv);
+		if (rval < 0)
+			return rval;
+		ret += rval;		/* Return the total args parsed */
+	}
+	return ret;
+}
+
 /* get core role */
 enum rte_lcore_role_t
 rte_eal_lcore_role(unsigned lcore_id)
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 1385a73..c04f295 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -153,6 +153,38 @@ int rte_eal_iopl_init(void);
  *   - On failure, a negative error value.
  */
 int rte_eal_init(int argc, char **argv);
+
+/**
+ * Initialize the Environment Abstraction Layer (EAL) and parse local args.
+ *
+ * This function is to be executed on the MASTER lcore only, as soon
+ * as possible in the application's main() function.
+ *
+ * The function finishes the initialization process before main() is called.
+ * It puts the SLAVE lcores in the WAIT state.
+ *
+ * When the multi-partition feature is supported, depending on the
+ * configuration (if CONFIG_RTE_EAL_MAIN_PARTITION is disabled), this
+ * function waits to ensure that the magic number is set before
+ * returning. See also the rte_eal_get_configuration() function. Note:
+ * This behavior may change in the future.
+ *
+ * @param argc
+ *   The argc argument that was given to the main() function.
+ * @param argv
+ *   The argv argument that was given to the main() function.
+ * @param parse
+ *   The parse function pointer from user int (*parse)(int, char **);
+ * @return
+ *   - On success, the number of parsed arguments, which is greater or
+ *     equal to zero. After the call to rte_eal_init(),
+ *     all arguments argv[x] with x < ret may be modified and should
+ *     not be accessed by the application.
+ *   - On failure, a negative error value.
+ */
+int rte_eal_init_parse(int argc, char **argv,
+		int (*parse)(int, char **));
+
 /**
  * Usage function typedef used by the application usage function.
  *
-- 
2.3.0

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Thomas Monjalon]

Hi Keith,

2015-06-03 13:49, Keith Wiles:
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c         | 20 ++++++++++++++++++++
>  lib/librte_eal/common/include/rte_eal.h | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)

These comments would be useful:
As a RFC patch, it is not complete yet.
This new API may be used for <reason>.

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Wiles, Keith]

On 6/3/15, 2:24 PM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>Hi Keith,
>
>2015-06-03 13:49, Keith Wiles:
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>>  lib/librte_eal/bsdapp/eal/eal.c         | 20 ++++++++++++++++++++
>>  lib/librte_eal/common/include/rte_eal.h | 32
>>++++++++++++++++++++++++++++++++
>>  2 files changed, 52 insertions(+)
>
>These comments would be useful:
>As a RFC patch, it is not complete yet.
>This new API may be used for <reason>.

I was going to add this to the real patch:

The new API is to reduce some of the clutter in the applications. Most of
the applications need to do the following:

    int ret = rte_eal_init(argc, argv);
    if ( ret < 0 )
        rte_exit(EXIT_FAILURE, "rte_eal_init: failed!");
    argc -= ret;
    argv += ret;

    ret = parse_args(argc, argv);
    if ( ret < 0 )
        rte_exit(EXIT_FAILURE, "parse_args:failed!");

With this new API:

    int ret = rte_eal_init_parse(argc, argv, parse_args);
    if ( ret < 0 )
        rte_exit(EXIT_FAILURE, "rte_eal_init_parse: Failed!");


>

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Stephen Hemminger]

On Wed,  3 Jun 2015 13:49:53 -0500
Keith Wiles <keith.wiles@intel.com> wrote:

> +/* Launch threads, called at application init() and parse app args. */
> +int
> +rte_eal_init_parse(int argc, char **argv,
> +		int (*parse)(int, char **))
> +{
> +	int	ret;
> +
> +	ret = rte_eal_init(argc, argv);
> +	if ((ret >= 0) && (parse != NULL)) {
> +		argc -= ret;
> +		argv += ret;

This won't work C is call by value.

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Wiles, Keith]

Hi Stephen

On 6/3/15, 7:12 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:

>On Wed,  3 Jun 2015 13:49:53 -0500
>Keith Wiles <keith.wiles@intel.com> wrote:
>
>> +/* Launch threads, called at application init() and parse app args. */
>> +int
>> +rte_eal_init_parse(int argc, char **argv,
>> +		int (*parse)(int, char **))
>> +{
>> +	int	ret;
>> +
>> +	ret = rte_eal_init(argc, argv);
>> +	if ((ret >= 0) && (parse != NULL)) {
>> +		argc -= ret;
>> +		argv += ret;
>
>This won't work C is call by value.

I tested this routine with Pktgen (again), which has a number of
application options and it appears to work correctly. Can you explain why
this will not work?

Regards,
++Keith
>

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Neil Horman]

On Thu, Jun 04, 2015 at 11:50:33AM +0000, Wiles, Keith wrote:
> Hi Stephen
> 
> On 6/3/15, 7:12 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:
> 
> >On Wed,  3 Jun 2015 13:49:53 -0500
> >Keith Wiles <keith.wiles@intel.com> wrote:
> >
> >> +/* Launch threads, called at application init() and parse app args. */
> >> +int
> >> +rte_eal_init_parse(int argc, char **argv,
> >> +		int (*parse)(int, char **))
> >> +{
> >> +	int	ret;
> >> +
> >> +	ret = rte_eal_init(argc, argv);
> >> +	if ((ret >= 0) && (parse != NULL)) {
> >> +		argc -= ret;
> >> +		argv += ret;
> >
> >This won't work C is call by value.
> 
> I tested this routine with Pktgen (again), which has a number of
> application options and it appears to work correctly. Can you explain why
> this will not work?
> 
> Regards,
> ++Keith
> >
> 
> 

Stephen was thinking that your intent was to have argc, and argv modified at the
call site of this function (i.e. if you called rte_eal_init_parse from main(),
then after the call to rte_ela_init_parse, argc would be reduced by ret and argv
would point forward in memory ret bytes in the main function, but they wont.  It
doesn't matter though, because you return ret, so the caller can do that
movement themselves.  As you note, it works.

Note that if it was your intention to have argc and argv modified at the call
site, then Stephen is right and this is broken, you need to modify the prototype
to be:
int rte_eal_init_parse(int *argc, char ***argv)

and do a dereference when modifying the parameters so the change is seen at the
call site.

That said, I'm not sure theres much value in adding this to the API.  For one,
it implies that dpdk arguments need to come first on the command line.  While
all the example applications do that, theres no requirement that they do so, and
this function silently implies that they have to, so any existing applications
in the wild that violate that assumption are enjoined from using this

It also doesn't really save any code.  If we pick an example app (I'll us
l2fwd-jobstats), We currently have this:

	/* init EAL */
        ret = rte_eal_init(argc, argv);
        if (ret < 0)
                rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
        argc -= ret;
        argv += ret;

        /* parse application arguments (after the EAL ones) */
        ret = l2fwd_parse_args(argc, argv);
	if (ret < 0)
                rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");

With your new API we would get this:

	ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
        if (ret < 0)
                rte_exit(EXIT_FAILURE, "Invalid arguments - not sure what\n");

Its definately 5 fewer lines of source, but it doesn't save any execution
instructions, and for the effort of that, you loose the ability to determine if
it was a DPDK argument or an application argument that failed.

Its not a bad addition, I'm just not sure its worth having to take on the
additional API surface to include.  I'd be more supportive if you could enhance
the function to allow the previously mentioned before/after flexibiilty.  Then
we could just deprecate rte_eal_init as an API call entirely, and use this
instead.

Neil

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Wiles, Keith]

Hi Neil and Stephen,

On 6/4/15, 8:55 AM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>On Thu, Jun 04, 2015 at 11:50:33AM +0000, Wiles, Keith wrote:
>> Hi Stephen
>> 
>> On 6/3/15, 7:12 PM, "Stephen Hemminger" <stephen@networkplumber.org>
>>wrote:
>> 
>> >On Wed,  3 Jun 2015 13:49:53 -0500
>> >Keith Wiles <keith.wiles@intel.com> wrote:
>> >
>> >> +/* Launch threads, called at application init() and parse app args.
>>*/
>> >> +int
>> >> +rte_eal_init_parse(int argc, char **argv,
>> >> +		int (*parse)(int, char **))
>> >> +{
>> >> +	int	ret;
>> >> +
>> >> +	ret = rte_eal_init(argc, argv);
>> >> +	if ((ret >= 0) && (parse != NULL)) {
>> >> +		argc -= ret;
>> >> +		argv += ret;
>> >
>> >This won't work C is call by value.
>> 
>> I tested this routine with Pktgen (again), which has a number of
>> application options and it appears to work correctly. Can you explain
>>why
>> this will not work?
>> 
>> Regards,
>> ++Keith
>> >
>> 
>> 
>
>Stephen was thinking that your intent was to have argc, and argv modified
>at the
>call site of this function (i.e. if you called rte_eal_init_parse from
>main(),
>then after the call to rte_ela_init_parse, argc would be reduced by ret
>and argv
>would point forward in memory ret bytes in the main function, but they
>wont.  It
>doesn't matter though, because you return ret, so the caller can do that
>movement themselves.  As you note, it works.
>
>Note that if it was your intention to have argc and argv modified at the
>call
>site, then Stephen is right and this is broken, you need to modify the
>prototype
>to be:
>int rte_eal_init_parse(int *argc, char ***argv)

My intent was not to alter the argc and argv values as that is not a
reasonable use case, correct?

>
>and do a dereference when modifying the parameters so the change is seen
>at the
>call site.
>
>That said, I'm not sure theres much value in adding this to the API.  For
>one,
>it implies that dpdk arguments need to come first on the command line.
>While
>all the example applications do that, theres no requirement that they do
>so, and
>this function silently implies that they have to, so any existing
>applications
>in the wild that violate that assumption are enjoined from using this
>
>It also doesn't really save any code.  If we pick an example app (I'll us
>l2fwd-jobstats), We currently have this:
>
>	/* init EAL */
>        ret = rte_eal_init(argc, argv);
>        if (ret < 0)
>                rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
>        argc -= ret;
>        argv += ret;
>
>        /* parse application arguments (after the EAL ones) */
>        ret = l2fwd_parse_args(argc, argv);
>	if (ret < 0)
>                rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
>
>With your new API we would get this:
>
>	ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
>        if (ret < 0)
>                rte_exit(EXIT_FAILURE, "Invalid arguments - not sure
>what\n");
>
>Its definately 5 fewer lines of source, but it doesn't save any execution
>instructions, and for the effort of that, you loose the ability to
>determine if
>it was a DPDK argument or an application argument that failed.

I agree this is not saving instructions and adding performance, but of
code clutter and providing a layered model for the developer. The
rte_eal_init() routine still exists and I was not trying to remove that
API only layer a convenient API for common constructs.
>
>Its not a bad addition, I'm just not sure its worth having to take on the
>additional API surface to include.  I'd be more supportive if you could
>enhance
>the function to allow the previously mentioned before/after flexibiilty.
>Then
>we could just deprecate rte_eal_init as an API call entirely, and use this
>instead.

I can see we can create an API to add support for doing the applications
args first or after, but would that even be acceptable?

++Keith
>
>Neil
>

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[David Marchand]

On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith <keith.wiles@intel.com> wrote:

> Hi Neil and Stephen,
>
> I agree this is not saving instructions and adding performance, but of
> code clutter and providing a layered model for the developer. The
> rte_eal_init() routine still exists and I was not trying to remove that
> API only layer a convenient API for common constructs.
> >
> >Its not a bad addition, I'm just not sure its worth having to take on the
> >additional API surface to include.  I'd be more supportive if you could
> >enhance
> >the function to allow the previously mentioned before/after flexibiilty.
> >Then
> >we could just deprecate rte_eal_init as an API call entirely, and use this
> >instead.
>
> I can see we can create an API to add support for doing the applications
> args first or after, but would that even be acceptable?
>

What's the point ?
Adding stuff just for saving lines ?
Are you serious about this ?


-- 
David Marchand

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Stephen Hemminger]

On Thu, 4 Jun 2015 09:55:42 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> That said, I'm not sure theres much value in adding this to the API.  For one,
> it implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so, and
> this function silently implies that they have to, so any existing applications
> in the wild that violate that assumption are enjoined from using this

I found that the only way to support daemon command line args is to put local
arguments first, then call EAL to parse it's args.

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Wiles, Keith]

From: David Marchand <david.marchand@6wind.com<mailto:david.marchand@6wind.com>>
Date: Thursday, June 4, 2015 at 9:43 AM
To: Keith Wiles <keith.wiles@intel.com<mailto:keith.wiles@intel.com>>
Cc: Neil Horman <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>>, "dev@dpdk.org<mailto:dev@dpdk.org>" <dev@dpdk.org<mailto:dev@dpdk.org>>
Subject: Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith <keith.wiles@intel.com<mailto:keith.wiles@intel.com>> wrote:
Hi Neil and Stephen,

I agree this is not saving instructions and adding performance, but of
code clutter and providing a layered model for the developer. The
rte_eal_init() routine still exists and I was not trying to remove that
API only layer a convenient API for common constructs.
>
>Its not a bad addition, I'm just not sure its worth having to take on the
>additional API surface to include.  I'd be more supportive if you could
>enhance
>the function to allow the previously mentioned before/after flexibiilty.
>Then
>we could just deprecate rte_eal_init as an API call entirely, and use this
>instead.

I can see we can create an API to add support for doing the applications
args first or after, but would that even be acceptable?

What's the point ?
Adding stuff just for saving lines ?
Are you serious about this ?

Wow, OK dropped!


--
David Marchand

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Wiles, Keith]

Hmmm, replied in HTML.

>On Thu, Jun 4, 2015 at 4:27 PM, Wiles, Keith
><keith.wiles@intel.com> wrote:
>
>Hi Neil and Stephen,
>
>I agree this is not saving instructions and adding performance, but of
>code clutter and providing a layered model for the developer. The
>rte_eal_init() routine still exists and I was not trying to remove that
>API only layer a convenient API for common constructs.
>>
>>Its not a bad addition, I'm just not sure its worth having to take on the
>>additional API surface to include.  I'd be more supportive if you could
>>enhance
>>the function to allow the previously mentioned before/after flexibiilty.
>>Then
>>we could just deprecate rte_eal_init as an API call entirely, and use
>>this
>>instead.
>
>I can see we can create an API to add support for doing the applications
>args first or after, but would that even be acceptable?
>
>
>
>What's the point ?
>Adding stuff just for saving lines ?
>Are you serious about this ?

Wow, OK it is dropped.
>
>
>-- 
>
>David Marchand

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Thomas F Herbert]

On 6/4/15 9:55 AM, Neil Horman wrote:
> On Thu, Jun 04, 2015 at 11:50:33AM +0000, Wiles, Keith wrote:
>> Hi Stephen
>>
>> On 6/3/15, 7:12 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:
>>
>>> On Wed,  3 Jun 2015 13:49:53 -0500
>>> Keith Wiles <keith.wiles@intel.com> wrote:
>>>
>>>> +/* Launch threads, called at application init() and parse app args. */
>>>> +int
>>>> +rte_eal_init_parse(int argc, char **argv,
>>>> +		int (*parse)(int, char **))
>>>> +{
>>>> +	int	ret;
>>>> +
>>>> +	ret = rte_eal_init(argc, argv);
>>>> +	if ((ret >= 0) && (parse != NULL)) {
>>>> +		argc -= ret;
>>>> +		argv += ret;
>>>
>>> This won't work C is call by value.
>>
>> I tested this routine with Pktgen (again), which has a number of
>> application options and it appears to work correctly. Can you explain why
>> this will not work?
>>
>> Regards,
>> ++Keith
>>>
>>
>>
>
> Stephen was thinking that your intent was to have argc, and argv modified at the
> call site of this function (i.e. if you called rte_eal_init_parse from main(),
> then after the call to rte_ela_init_parse, argc would be reduced by ret and argv
> would point forward in memory ret bytes in the main function, but they wont.  It
> doesn't matter though, because you return ret, so the caller can do that
> movement themselves.  As you note, it works.
>
> Note that if it was your intention to have argc and argv modified at the call
> site, then Stephen is right and this is broken, you need to modify the prototype
> to be:
> int rte_eal_init_parse(int *argc, char ***argv)
>
> and do a dereference when modifying the parameters so the change is seen at the
> call site.
>
> That said, I'm not sure theres much value in adding this to the API.  For one,
> it implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so, and
> this function silently implies that they have to, so any existing applications
> in the wild that violate that assumption are enjoined from using this
>
> It also doesn't really save any code.  If we pick an example app (I'll us
> l2fwd-jobstats), We currently have this:
>
> 	/* init EAL */
>          ret = rte_eal_init(argc, argv);
>          if (ret < 0)
>                  rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
>          argc -= ret;
>          argv += ret;
>
>          /* parse application arguments (after the EAL ones) */
>          ret = l2fwd_parse_args(argc, argv);
> 	if (ret < 0)
>                  rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
>
> With your new API we would get this:
>
> 	ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
>          if (ret < 0)
>                  rte_exit(EXIT_FAILURE, "Invalid arguments - not sure what\n");
>
> Its definately 5 fewer lines of source, but it doesn't save any execution
> instructions, and for the effort of that, you loose the ability to determine if
> it was a DPDK argument or an application argument that failed.
>
> Its not a bad addition, I'm just not sure its worth having to take on the
> additional API surface to include.  I'd be more supportive if you could enhance
> the function to allow the previously mentioned before/after flexibiilty.  Then
> we could just deprecate rte_eal_init as an API call entirely, and use this
> instead.
+1.

Also, I think rte_set_application_usage_hook() callback could be used by 
app writers for implementing usage() for a conventional  "<program> -h" 
like capability to print all usage including both eal and app specific 
args even if the eal args are not correct. This is an alternative to 
calling eal_init() first and bombing before printing all usage.

--TFH

> Neil
>

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Chilikin, Andrey]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Thursday, June 4, 2015 2:56 PM
> To: Wiles, Keith
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at
> rte_eal_init time
> 
> On Thu, Jun 04, 2015 at 11:50:33AM +0000, Wiles, Keith wrote:
> > Hi Stephen
> >
> > On 6/3/15, 7:12 PM, "Stephen Hemminger" <stephen@networkplumber.org>
> wrote:
> >
> > >On Wed,  3 Jun 2015 13:49:53 -0500
> > >Keith Wiles <keith.wiles@intel.com> wrote:
> > >
> > >> +/* Launch threads, called at application init() and parse app
> > >> +args. */ int rte_eal_init_parse(int argc, char **argv,
> > >> +		int (*parse)(int, char **))
> > >> +{
> > >> +	int	ret;
> > >> +
> > >> +	ret = rte_eal_init(argc, argv);
> > >> +	if ((ret >= 0) && (parse != NULL)) {
> > >> +		argc -= ret;
> > >> +		argv += ret;
> > >
> > >This won't work C is call by value.
> >
> > I tested this routine with Pktgen (again), which has a number of
> > application options and it appears to work correctly. Can you explain
> > why this will not work?
> >
> > Regards,
> > ++Keith
> > >
> >
> >
> 
> Stephen was thinking that your intent was to have argc, and argv modified at
> the call site of this function (i.e. if you called rte_eal_init_parse from main(),
> then after the call to rte_ela_init_parse, argc would be reduced by ret and argv
> would point forward in memory ret bytes in the main function, but they wont.
> It doesn't matter though, because you return ret, so the caller can do that
> movement themselves.  As you note, it works.
> 
> Note that if it was your intention to have argc and argv modified at the call site,
> then Stephen is right and this is broken, you need to modify the prototype to be:
> int rte_eal_init_parse(int *argc, char ***argv)
> 
> and do a dereference when modifying the parameters so the change is seen at
> the call site.
> 
> That said, I'm not sure theres much value in adding this to the API.  For one, it
> implies that dpdk arguments need to come first on the command line.  While
> all the example applications do that, theres no requirement that they do so,
> and this function silently implies that they have to, so any existing applications
> in the wild that violate that assumption are enjoined from using this
> 
> It also doesn't really save any code.  If we pick an example app (I'll us l2fwd-
> jobstats), We currently have this:
> 
> 	/* init EAL */
>         ret = rte_eal_init(argc, argv);
>         if (ret < 0)
>                 rte_exit(EXIT_FAILURE, "Invalid EAL arguments\n");
>         argc -= ret;
>         argv += ret;
> 
>         /* parse application arguments (after the EAL ones) */
>         ret = l2fwd_parse_args(argc, argv);
> 	if (ret < 0)
>                 rte_exit(EXIT_FAILURE, "Invalid L2FWD arguments\n");
> 
> With your new API we would get this:
> 
> 	ret = rte_eal_init_parse(argc, argv, l2fwd_parse_args)
>         if (ret < 0)
>                 rte_exit(EXIT_FAILURE, "Invalid arguments - not sure what\n");
> 
> Its definately 5 fewer lines of source, but it doesn't save any execution
> instructions, and for the effort of that, you loose the ability to determine if it
> was a DPDK argument or an application argument that failed.
> 
> Its not a bad addition, I'm just not sure its worth having to take on the
> additional API surface to include.  I'd be more supportive if you could enhance
> the function to allow the previously mentioned before/after flexibiilty.  Then we
> could just deprecate rte_eal_init as an API call entirely, and use this instead.

Before/after would be very useful, a lot of applications use only "-c" and "-n" EAL command line parameters and "-c" in many cases is redundant as application can calculate core mask from its own parameters, and "-n" just a required parameter which can be defaulted to a platform specific value. So in addition to rte_set_application_usage_hook() it would be nice to have some more general way of overwriting eal initialization parameters. 

> 
> Neil

Re: [dpdk-dev] [RFC PATCH] eal:Add new API for parsing args at rte_eal_init time

[Simon Kågström]

<Snipping lots of stuff>

On 2015-06-04 23:27, Chilikin, Andrey wrote:

>> Its not a bad addition, I'm just not sure its worth having to take on the
>> additional API surface to include.  I'd be more supportive if you could enhance
>> the function to allow the previously mentioned before/after flexibiilty.  Then we
>> could just deprecate rte_eal_init as an API call entirely, and use this instead.
> 
> Before/after would be very useful, a lot of applications use only "-c" and "-n" EAL command line parameters and "-c" in many cases is redundant as application can calculate core mask from its own parameters, and "-n" just a required parameter which can be defaulted to a platform specific value. So in addition to rte_set_application_usage_hook() it would be nice to have some more general way of overwriting eal initialization parameters. 

I've always found it a bit strange that DPDK forces argv handling this
way. The application will anyway have to setup system-specific stuff
(buffer count etc) for the ports to use, so special-casing memory and
core setup seems strange. I think it would be more logical to have EAL
configuration from a structure like for the ports:

  struct dpdk_conf conf =
  {
        .core_mask = 0x7,
        .huge_pages = 1,
        [...]
  };

  rte_eal_init(&conf);

And make the current argv parser optional, i.e., something like

  int main(argc, argv)
  {
        struct dpdk_conf conf;

	ret = rte_eal_parse_argv(&conf);
        rte_eal_init(&conf);
        argc -= ret; ...
  }

// Simon