All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] Yet another option for DPDK options
@ 2016-06-01 15:00 Wiles, Keith
  2016-06-01 15:46 ` Matthew Hall
                   ` (4 more replies)
  0 siblings, 5 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-01 15:00 UTC (permalink / raw)
  To: Yuanhan Liu, Thomas Monjalon
  Cc: dev, Richardson, Bruce, Tan, Jianfeng, Stephen Hemminger,
	Christian Ehrhardt, Panu Matilainen, Olivier Matz

Started from the link below, but did not want to highjack the thread.
http://dpdk.org/ml/archives/dev/2016-June/040021.html

I was thinking about this problem from a user perspective and command line options are very difficult to manage specifically when you have a large number of options as we have in dpdk. I see all of these options as a type of database of information for the DPDK and the application, because the application command line options are also getting very complex as well.

I have been looking at a number of different options here and the direction I was thinking was using a file for the options and configurations with the data in a clean format. It could have been a INI file or JSON or XML, but they all seem to have some problems I do not like. The INI file is too flat and I wanted a hierarchy in the data, the JSON data is similar and XML is just hard to read. I wanted to be able to manage multiple applications and possible system the DPDK/app runs. The problem with the above formats is they are just data and not easy to make decisions about the system and applications at runtime.

If the “database” of information could be queried by the EAL, drivers and application then we do not need to try and create a complex command line. It would be nice to execute a DPDK applications like this:

./some_dpdk_app –config-file dpdk-config-filename

The dpdk-config-filename could contain a lot of information and be able to startup multiple different applications. The dpdk-config-file could also include other config files to complete the configuration. The format of the data in the config file needs to be readable, but allow the user to put in new options, needs to be hierarchical in nature and have some simple functions to execute if required.

The solution I was thinking is the file information is really just a fragment of a scripting language, which the DPDK application contains this scripting language interpreter. I was looking at using Lua lua.org as the scripting language interpreter it is small and easy to understand. Python and others are very big and require a lot of resources and Lua requires very few system resources. Also I did not want to have to write a parser (lex/yacc). The other nice feature of Lua is you can create a sandbox for the code to run in and limit the type of system resources and APIs that can be accessed by the application and configuration. Lua can be trimmed down to a fairly small size and builds on just about any system or we can just install Lua on the system without changes from a rpm or deb.

I use Lua in pktgen at this time and the interface between ‘C’ and Lua is very simple and easy. Currently I include Lua in Pktgen, but I could have just used a system library.

The data in the config file can be data statements along with some limited code to make some data changes at run time without having to modify the real application. Here is a simple config file I wrote: Some of the options do not make sense to be in the file at the same time, but wanted to see all of the options. The mk_lcore_list() and mk_coremap() are just Lua functions we can preload to help convert the simple strings into real data in this case tables of information. The application could be something like pktgen = { map = { … }, more_options = 1, } this allows the same file to possible contain many application configurations. Needs a bit more work.

dpdk_default = {
    lcore_mask = 0xFF00,
    lcore_list = mk_lcore_list("0-7", 10, "14-16"),
    coremap = mk_coremap("(0-7)@0,10,(14-16)@1"),
    master_lcore = 1,
    log_level = 7,
    ranks = 4,
    channels = 2,
    memory = 512,
    socket_mem = { 512, 512 },
    huge_dir = "/mnt/huge",
    base_virtaddr = 0,
    create_uio_dev = true,
    vfio_intr = "legacy",
    xen_dom0 = false,
    proc_type = "auto",
    pci_blacklist = {
        "08:00.0",
        "08:00.1",
        "09:00.0",
        "09:00.1",
        "83:00.1",
        "87:00.0",
        "87:00.1",
        "89:00.0",
        "89:00.1"
    },
    pci_whitelist = {
    },
    vdev = {
        eth_pcap0 = { iface = "eth2" },
        eth_pcap1 = { iface = "eth3" },
    },
    driver = { },
    syslog = true,
    vmware_tsc_map = false,
    file_prefix = "pg",
    huge_unlink = true,
    no_huge = false,
    no_pci = false,
    no_hpet = false,
    no_shconf = false,
}

pktgen_data = {
   map = { … },
   more-data = 1,
}

The EAL, driver, application, … would query an API to access the data and the application can change his options quickly without modifying the code.

Anyway comments are welcome.
 
Regards,
Keith






^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
@ 2016-06-01 15:46 ` Matthew Hall
  2016-06-01 16:08   ` Wiles, Keith
  2016-06-01 15:58 ` Jay Rolette
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 49+ messages in thread
From: Matthew Hall @ 2016-06-01 15:46 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Yuanhan Liu, Thomas Monjalon, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Wed, Jun 01, 2016 at 03:00:11PM +0000, Wiles, Keith wrote:
> The INI file is too flat and I wanted a hierarchy in the data, the JSON data 
> is similar and XML is just hard to read.

I don't think it's fair to say JSON lacks hierarchy. Personally it is working 
great in my current application. The main "bug" is that the spec designers 
intentionally and idiotically left out the ability to make comments. But there 
are some smarter JSON parsers such as json-c and the Perl JSON parser which 
will allow them using either "#" or "//".

You can also build JSON in memory pretty nicely using json-c. It has a simple 
DOM-like API for this.

I am using it in the config file for my app right now, and passing a fake argc 
and argv to DPDK using wordexp() to prevent it from munging the argc and argv 
of my application.

> It would be nice to execute a DPDK applications like this:
> 
> ./some_dpdk_app ???config-file dpdk-config-filename

FYI, I think you used Outlook with some of MS's bad defaults and it mangled 
all your special characters...

> The dpdk-config-filename could contain a lot of information and be able to 
> startup multiple different applications. The dpdk-config-file could also 
> include other config files to complete the configuration. The format of the 
> data in the config file needs to be readable, but allow the user to put in 
> new options, needs to be hierarchical in nature and have some simple 
> functions to execute if required.

To me, this is way too complicated and includes a lot of features I'm not 
convinced we actually need or want. I'd really prefer if we just have one file 
per app. I don't want a super complicated way to configure it replacing an 
already super complicated way to configure it.

> The solution I was thinking is the file information is really just a 
> fragment of a scripting language, which the DPDK application contains this 
> scripting language interpreter. I was looking at using Lua lua.org as the 
> scripting language interpreter it is small and easy to understand.

If we're stuck doing this Lua is the best option but I'd still rather avoid 
it. I like the fact that DPDK is a lot of clean C code, this is why I find it 
so much easier to read and code than the awful kernel network stacks.

>     lcore_list = mk_lcore_list("0-7", 10, "14-16"),
>     coremap = mk_coremap("(0-7)@0,10,(14-16)@1"),

These magical functions feel weird compared to just having some simple 
functions that take them as JSON strings and validate them. Which is what I'm 
doing in my app right now with minimal pain.

> The EAL, driver, application, ??? would query an API to access the data and 
> the application can change his options quickly without modifying the code.

I don't want to have to use somebody else's API to get to the config of my app 
if I can avoid it. I like the approach of json-c where I can lay it out how I 
want, and pass the parts I want DPDK to have over to DPDK. I don't necessarily 
want to have to go through DPDK to get to my own config stuff. Which is what I 
am stuck doing if we put a weird proprietary DPDK specific file format or 
scripting environment in these files.

> Keith

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
  2016-06-01 15:46 ` Matthew Hall
@ 2016-06-01 15:58 ` Jay Rolette
  2016-06-01 16:18   ` Bruce Richardson
  2016-06-01 18:51 ` Thomas Monjalon
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 49+ messages in thread
From: Jay Rolette @ 2016-06-01 15:58 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Yuanhan Liu, Thomas Monjalon, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com> wrote:

> Started from the link below, but did not want to highjack the thread.
> http://dpdk.org/ml/archives/dev/2016-June/040021.html
>
> I was thinking about this problem from a user perspective and command line
> options are very difficult to manage specifically when you have a large
> number of options as we have in dpdk. I see all of these options as a type
> of database of information for the DPDK and the application, because the
> application command line options are also getting very complex as well.
>
> I have been looking at a number of different options here and the
> direction I was thinking was using a file for the options and
> configurations with the data in a clean format. It could have been a INI
> file or JSON or XML, but they all seem to have some problems I do not like.
> The INI file is too flat and I wanted a hierarchy in the data, the JSON
> data is similar and XML is just hard to read. I wanted to be able to manage
> multiple applications and possible system the DPDK/app runs. The problem
> with the above formats is they are just data and not easy to make decisions
> about the system and applications at runtime.
>

INI format is simplest for users to read, but if you really need hierarchy,
JSON will do that just fine. Not sure what you mean by "JSON data is
similar"...


> If the “database” of information could be queried by the EAL, drivers and
> application then we do not need to try and create a complex command line.
> It would be nice to execute a DPDK applications like this:
>
> ./some_dpdk_app –config-file dpdk-config-filename
>

+1 much nicer than the mess that is EAL command line args today.


> The dpdk-config-filename could contain a lot of information and be able to
> startup multiple different applications. The dpdk-config-file could also
> include other config files to complete the configuration. The format of the
> data in the config file needs to be readable, but allow the user to put in
> new options, needs to be hierarchical in nature and have some simple
> functions to execute if required.
>
> The solution I was thinking is the file information is really just a
> fragment of a scripting language, which the DPDK application contains this
> scripting language interpreter. I was looking at using Lua lua.org as the
> scripting language interpreter it is small and easy to understand. Python
> and others are very big and require a lot of resources and Lua requires
> very few system resources. Also I did not want to have to write a parser
> (lex/yacc). The other nice feature of Lua is you can create a sandbox for
> the code to run in and limit the type of system resources and APIs that can
> be accessed by the application and configuration. Lua can be trimmed down
> to a fairly small size and builds on just about any system or we can just
> install Lua on the system without changes from a rpm or deb.
>

There are JSON and INI file parser libraries for pretty much any language
you care to use. That shouldn't be a factor in choosing file format.

The argument about "Python and others are very big and require a lot of
resources" doesn't end up mattering much since it is already required by a
couple of the DPDK tools (in particular, dpdk_nic_bind.py).


> I use Lua in pktgen at this time and the interface between ‘C’ and Lua is
> very simple and easy. Currently I include Lua in Pktgen, but I could have
> just used a system library.
>
> The data in the config file can be data statements along with some limited
> code to make some data changes at run time without having to modify the
> real application. Here is a simple config file I wrote: Some of the options
> do not make sense to be in the file at the same time, but wanted to see all
> of the options. The mk_lcore_list() and mk_coremap() are just Lua functions
> we can preload to help convert the simple strings into real data in this
> case tables of information. The application could be something like pktgen
> = { map = { … }, more_options = 1, } this allows the same file to possible
> contain many application configurations. Needs a bit more work.
>
> dpdk_default = {
>
<snip>

> }
>
> The EAL, driver, application, … would query an API to access the data and
> the application can change his options quickly without modifying the code.
>
> Anyway comments are welcome.
>
> Regards,
> Keith
>

I like the concept overall. I'd suggest separating out the Lua thing. Lua's
fine for scripting, but nothing here really requires it or saves a lot of
development work.

Jay

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:46 ` Matthew Hall
@ 2016-06-01 16:08   ` Wiles, Keith
  0 siblings, 0 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-01 16:08 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Yuanhan Liu, Thomas Monjalon, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz



Regards,
Keith



On 6/1/16, 10:46 AM, "Matthew Hall" <mhall@mhcomputing.net> wrote:

>On Wed, Jun 01, 2016 at 03:00:11PM +0000, Wiles, Keith wrote:
>> The INI file is too flat and I wanted a hierarchy in the data, the JSON data 
>> is similar and XML is just hard to read.
>
>I don't think it's fair to say JSON lacks hierarchy. Personally it is working 
>great in my current application. The main "bug" is that the spec designers 
>intentionally and idiotically left out the ability to make comments. But there 
>are some smarter JSON parsers such as json-c and the Perl JSON parser which 
>will allow them using either "#" or "//".

JSON-C could work and Lua has its comments too. I just wanted more from the configuration then just data. You still need some code to parse the JSON format in the application, right?

>
>You can also build JSON in memory pretty nicely using json-c. It has a simple 
>DOM-like API for this.
>
>I am using it in the config file for my app right now, and passing a fake argc 
>and argv to DPDK using wordexp() to prevent it from munging the argc and argv 
>of my application.
>
>> It would be nice to execute a DPDK applications like this:
>> 
>> ./some_dpdk_app ???config-file dpdk-config-filename
>
>FYI, I think you used Outlook with some of MS's bad defaults and it mangled 
>all your special characters...

Yes, I hate MS Outlook. I have tried to fix the options, but it never seems to work out. Until someone create a really good email application for OS X that works with exchange I will stuck with this one. I have tried a number of different ones, they all have limitations or problems ☹

>
>> The dpdk-config-filename could contain a lot of information and be able to 
>> startup multiple different applications. The dpdk-config-file could also 
>> include other config files to complete the configuration. The format of the 
>> data in the config file needs to be readable, but allow the user to put in 
>> new options, needs to be hierarchical in nature and have some simple 
>> functions to execute if required.
>
>To me, this is way too complicated and includes a lot of features I'm not 
>convinced we actually need or want. I'd really prefer if we just have one file 
>per app. I don't want a super complicated way to configure it replacing an 
>already super complicated way to configure it.

I do not see it being too complexed I think it is what you have used before and not that it is to complex of a solution.

>
>> The solution I was thinking is the file information is really just a 
>> fragment of a scripting language, which the DPDK application contains this 
>> scripting language interpreter. I was looking at using Lua lua.org as the 
>> scripting language interpreter it is small and easy to understand.
>
>If we're stuck doing this Lua is the best option but I'd still rather avoid 
>it. I like the fact that DPDK is a lot of clean C code, this is why I find it 
>so much easier to read and code than the awful kernel network stacks.

We do not need to understand Lua interpreter or compiler only the simple Lua scripting code.

>
>>     lcore_list = mk_lcore_list("0-7", 10, "14-16"),
>>     coremap = mk_coremap("(0-7)@0,10,(14-16)@1"),
>
>These magical functions feel weird compared to just having some simple 
>functions that take them as JSON strings and validate them. Which is what I'm 
>doing in my app right now with minimal pain.

These are just examples to convert to tables from strings and could have not used them.

>
>> The EAL, driver, application, ??? would query an API to access the data and 
>> the application can change his options quickly without modifying the code.
>
>I don't want to have to use somebody else's API to get to the config of my app 
>if I can avoid it. I like the approach of json-c where I can lay it out how I 
>want, and pass the parts I want DPDK to have over to DPDK. I don't necessarily 
>want to have to go through DPDK to get to my own config stuff. Which is what I 
>am stuck doing if we put a weird proprietary DPDK specific file format or 
>scripting environment in these files.

Not sure the meaning of someone else’s API, we already have a difficult configuration structure and command line interface, just trying to create a common set up database like APIs to access the data. Now maybe JSON-c has some already I do not know. Removing the command line options for just one option ☺ seems like a good thing.

>
>> Keith
>
>Matthew.
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:58 ` Jay Rolette
@ 2016-06-01 16:18   ` Bruce Richardson
  2016-06-01 16:21     ` Arnon Warshavsky
                       ` (3 more replies)
  0 siblings, 4 replies; 49+ messages in thread
From: Bruce Richardson @ 2016-06-01 16:18 UTC (permalink / raw)
  To: Jay Rolette
  Cc: Wiles, Keith, Yuanhan Liu, Thomas Monjalon, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Wed, Jun 01, 2016 at 10:58:41AM -0500, Jay Rolette wrote:
> On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> 
> > Started from the link below, but did not want to highjack the thread.
> > http://dpdk.org/ml/archives/dev/2016-June/040021.html
> >
> > I was thinking about this problem from a user perspective and command line
> > options are very difficult to manage specifically when you have a large
> > number of options as we have in dpdk. I see all of these options as a type
> > of database of information for the DPDK and the application, because the
> > application command line options are also getting very complex as well.
> >
> > I have been looking at a number of different options here and the
> > direction I was thinking was using a file for the options and
> > configurations with the data in a clean format. It could have been a INI
> > file or JSON or XML, but they all seem to have some problems I do not like.
> > The INI file is too flat and I wanted a hierarchy in the data, the JSON
> > data is similar and XML is just hard to read. I wanted to be able to manage
> > multiple applications and possible system the DPDK/app runs. The problem
> > with the above formats is they are just data and not easy to make decisions
> > about the system and applications at runtime.
> >
> 
> INI format is simplest for users to read, but if you really need hierarchy,
> JSON will do that just fine. Not sure what you mean by "JSON data is
> similar"...
> 
> 
I'd be quite concerned if we start needing lots of hierarchies for configuration.

I'd really just like to see ini file format used for this because:
* it's a well understood, simple format
* very easily human readable and editable
* lots of support for it in lots of languages
* hierarchies are possible in it too - just not as easy as in other formats
  though. [In a previous life I worked with ini files which had address
  hierarchies 6-levels deep in them. It wasn't hard to work with]
* it works well with grep since you must have one value per-line
* it allows comments
* we already have a DPDK library for parsing them

However, for me the biggest advantage of using something like ini is that it
would force us to keep things simple!

I'd stay away from formats like json or XML that are designed for serializing
entire objects or structures, and look for something that allows us to just
specify configuration values.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 16:18   ` Bruce Richardson
@ 2016-06-01 16:21     ` Arnon Warshavsky
  2016-06-01 18:13     ` Wiles, Keith
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 49+ messages in thread
From: Arnon Warshavsky @ 2016-06-01 16:21 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Jay Rolette, Wiles, Keith, Yuanhan Liu, Thomas Monjalon, dev,
	Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Wed, Jun 1, 2016 at 7:18 PM, Bruce Richardson <bruce.richardson@intel.com
> wrote:

> On Wed, Jun 01, 2016 at 10:58:41AM -0500, Jay Rolette wrote:
> > On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com>
> wrote:
> >
> > > Started from the link below, but did not want to highjack the thread.
> > > http://dpdk.org/ml/archives/dev/2016-June/040021.html
> > >
> > > I was thinking about this problem from a user perspective and command
> line
> > > options are very difficult to manage specifically when you have a large
> > > number of options as we have in dpdk. I see all of these options as a
> type
> > > of database of information for the DPDK and the application, because
> the
> > > application command line options are also getting very complex as well.
> > >
> > > I have been looking at a number of different options here and the
> > > direction I was thinking was using a file for the options and
> > > configurations with the data in a clean format. It could have been a
> INI
> > > file or JSON or XML, but they all seem to have some problems I do not
> like.
> > > The INI file is too flat and I wanted a hierarchy in the data, the JSON
> > > data is similar and XML is just hard to read. I wanted to be able to
> manage
> > > multiple applications and possible system the DPDK/app runs. The
> problem
> > > with the above formats is they are just data and not easy to make
> decisions
> > > about the system and applications at runtime.
> > >
> >
> > INI format is simplest for users to read, but if you really need
> hierarchy,
> > JSON will do that just fine. Not sure what you mean by "JSON data is
> > similar"...
> >
> >
> I'd be quite concerned if we start needing lots of hierarchies for
> configuration.
>
> I'd really just like to see ini file format used for this because:
> * it's a well understood, simple format
> * very easily human readable and editable
> * lots of support for it in lots of languages
> * hierarchies are possible in it too - just not as easy as in other formats
>   though. [In a previous life I worked with ini files which had address
>   hierarchies 6-levels deep in them. It wasn't hard to work with]
> * it works well with grep since you must have one value per-line
> * it allows comments
> * we already have a DPDK library for parsing them
>
> However, for me the biggest advantage of using something like ini is that
> it
> would force us to keep things simple!
>
> I'd stay away from formats like json or XML that are designed for
> serializing
> entire objects or structures, and look for something that allows us to just
> specify configuration values.
>
> Regards,
> /Bruce
>
>

 +1 for a single cfg file parameter
 +1 for ini simplicity
/Arnon

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 16:18   ` Bruce Richardson
  2016-06-01 16:21     ` Arnon Warshavsky
@ 2016-06-01 18:13     ` Wiles, Keith
  2016-06-01 18:31     ` Stephen Hemminger
  2016-06-03 10:07     ` Yerden Zhumabekov
  3 siblings, 0 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-01 18:13 UTC (permalink / raw)
  To: Richardson, Bruce, Jay Rolette
  Cc: Yuanhan Liu, Thomas Monjalon, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz


On 6/1/16, 11:18 AM, "Richardson, Bruce" <bruce.richardson@intel.com> wrote:

>On Wed, Jun 01, 2016 at 10:58:41AM -0500, Jay Rolette wrote:
>> On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>> 
>> > Started from the link below, but did not want to highjack the thread.
>> > http://dpdk.org/ml/archives/dev/2016-June/040021.html
>> >
>> > I was thinking about this problem from a user perspective and command line
>> > options are very difficult to manage specifically when you have a large
>> > number of options as we have in dpdk. I see all of these options as a type
>> > of database of information for the DPDK and the application, because the
>> > application command line options are also getting very complex as well.
>> >
>> > I have been looking at a number of different options here and the
>> > direction I was thinking was using a file for the options and
>> > configurations with the data in a clean format. It could have been a INI
>> > file or JSON or XML, but they all seem to have some problems I do not like.
>> > The INI file is too flat and I wanted a hierarchy in the data, the JSON
>> > data is similar and XML is just hard to read. I wanted to be able to manage
>> > multiple applications and possible system the DPDK/app runs. The problem
>> > with the above formats is they are just data and not easy to make decisions
>> > about the system and applications at runtime.
>> >
>> 
>> INI format is simplest for users to read, but if you really need hierarchy,
>> JSON will do that just fine. Not sure what you mean by "JSON data is
>> similar"...
>> 
>> 
>I'd be quite concerned if we start needing lots of hierarchies for configuration.
>
>I'd really just like to see ini file format used for this because:
>* it's a well understood, simple format
>* very easily human readable and editable
>* lots of support for it in lots of languages
>* hierarchies are possible in it too - just not as easy as in other formats
>  though. [In a previous life I worked with ini files which had address
>  hierarchies 6-levels deep in them. It wasn't hard to work with]

Maybe INI will work for hierarchies, but I bet it was not super obvious without a lot of comments.

>* it works well with grep since you must have one value per-line
>* it allows comments

We can have comments in any format not really a deciding factor IMHO.

>* we already have a DPDK library for parsing them
>
>However, for me the biggest advantage of using something like ini is that it
>would force us to keep things simple!

Simple is good and with any of these formats you can be simple or complex, just depends on the usage.

If all I wanted was to run a few examples then I would say INI is just fine. I would like to have a configuration file that can help me understand the system and pick the right options for a two socket system or one socket or system with 4 cores or 16 or running in a VM/container or not or FreeBSD or Linux or … you get the picture all with a single image (maybe not with the FreeBSD/Linux).

In a static data format you do not get these easily (maybe if you added a bunch of different options and then made the code figure out which ones to use), in a interpreted language you do get them for free.

This is why I do not want just a database of options.

>
>I'd stay away from formats like json or XML that are designed for serializing
>entire objects or structures, and look for something that allows us to just
>specify configuration values.
>
>Regards,
>/Bruce
>
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 16:18   ` Bruce Richardson
  2016-06-01 16:21     ` Arnon Warshavsky
  2016-06-01 18:13     ` Wiles, Keith
@ 2016-06-01 18:31     ` Stephen Hemminger
  2016-06-03 10:07     ` Yerden Zhumabekov
  3 siblings, 0 replies; 49+ messages in thread
From: Stephen Hemminger @ 2016-06-01 18:31 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Jay Rolette, Wiles, Keith, Yuanhan Liu, Thomas Monjalon, dev,
	Tan, Jianfeng, Christian Ehrhardt, Panu Matilainen, Olivier Matz

On Wed, 1 Jun 2016 17:18:26 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Jun 01, 2016 at 10:58:41AM -0500, Jay Rolette wrote:
> > On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> > 
> > > Started from the link below, but did not want to highjack the thread.
> > > http://dpdk.org/ml/archives/dev/2016-June/040021.html
> > >
> > > I was thinking about this problem from a user perspective and command line
> > > options are very difficult to manage specifically when you have a large
> > > number of options as we have in dpdk. I see all of these options as a type
> > > of database of information for the DPDK and the application, because the
> > > application command line options are also getting very complex as well.
> > >
> > > I have been looking at a number of different options here and the
> > > direction I was thinking was using a file for the options and
> > > configurations with the data in a clean format. It could have been a INI
> > > file or JSON or XML, but they all seem to have some problems I do not like.
> > > The INI file is too flat and I wanted a hierarchy in the data, the JSON
> > > data is similar and XML is just hard to read. I wanted to be able to manage
> > > multiple applications and possible system the DPDK/app runs. The problem
> > > with the above formats is they are just data and not easy to make decisions
> > > about the system and applications at runtime.
> > >
> > 
> > INI format is simplest for users to read, but if you really need hierarchy,
> > JSON will do that just fine. Not sure what you mean by "JSON data is
> > similar"...
> > 
> > 
> I'd be quite concerned if we start needing lots of hierarchies for configuration.
> 
> I'd really just like to see ini file format used for this because:
> * it's a well understood, simple format
> * very easily human readable and editable
> * lots of support for it in lots of languages
> * hierarchies are possible in it too - just not as easy as in other formats
>   though. [In a previous life I worked with ini files which had address
>   hierarchies 6-levels deep in them. It wasn't hard to work with]
> * it works well with grep since you must have one value per-line
> * it allows comments
> * we already have a DPDK library for parsing them
> 
> However, for me the biggest advantage of using something like ini is that it
> would force us to keep things simple!

Agreed, INI is much easier to deal with than JSON.

Also, lets still keep the configuration options to a minimum. And the defaults
must still work.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
  2016-06-01 15:46 ` Matthew Hall
  2016-06-01 15:58 ` Jay Rolette
@ 2016-06-01 18:51 ` Thomas Monjalon
  2016-06-02  9:19   ` Marc
  2016-06-02  7:56 ` Yuanhan Liu
  2016-06-02 10:41 ` Neil Horman
  4 siblings, 1 reply; 49+ messages in thread
From: Thomas Monjalon @ 2016-06-01 18:51 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Yuanhan Liu, dev, Richardson, Bruce, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

Hi Keith,

I'll try to bring more context to this discussion below.

2016-06-01 15:00, Wiles, Keith:
> Started from the link below, but did not want to highjack the thread.
> http://dpdk.org/ml/archives/dev/2016-June/040021.html
> 
> I was thinking about this problem from a user perspective and command
> line options are very difficult to manage specifically when you have
> a large number of options as we have in dpdk.

The user uses an application.
It is up to the application to let users do some configuration.

> I see all of these options as a type of database of information for
> the DPDK and the application, because the application command line
> options are also getting very complex as well.

DPDK is a collection of libraries.
There is no command line options in a library.
So we should not be talking about such issue. But...

... configuration of the DPDK libraries must be improved.
We need some clean API to let the application configure a lot of things
at runtime (during initialization or after).
Ideally the API should not use an argc/argv format.

We also have a lot of applications for tests or examples which use a
common configuration scheme based on command line options.
It is only for test and demonstration purpose. So it is not so important
and must not be complex to maintain.
I also think that we should avoid having to modify a configuration file
for test applications. I like launching a freshly built testpmd with a
copy-pasted command line without having to create a temporary
configuration file.

Instead of wrapping a messy configuration interface, we should proceed
with this steps (in this order):
- implement clean configuration API
- move command line options parsing in a separate library
- implement an alternative to the options parsing library, as an example
- remove the options parsing library if the alternative is better

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
                   ` (2 preceding siblings ...)
  2016-06-01 18:51 ` Thomas Monjalon
@ 2016-06-02  7:56 ` Yuanhan Liu
  2016-06-02 10:41 ` Neil Horman
  4 siblings, 0 replies; 49+ messages in thread
From: Yuanhan Liu @ 2016-06-02  7:56 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Thomas Monjalon, dev, Richardson, Bruce, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz, Traynor, Kevin

On Wed, Jun 01, 2016 at 03:00:11PM +0000, Wiles, Keith wrote:
> I have been looking at a number of different options here and the direction I was thinking was using a file for the options and configurations with the data in a clean format.

It should be helpful and handy for productive usage. But for development
and debugging, I'd say CLI options is more convenient and flexible. I
would be more willing to fiddle with CLI options than editing config files.

In another word, +1, but I would also assume that we will keep the CLI
options.

> It could have been a INI file or JSON or XML, but they all seem to have some problems I do not like. The INI file is too flat and I wanted a hierarchy in the data, the JSON data is similar and XML is just hard to read. I wanted to be able to manage multiple applications and possible system the DPDK/app runs. The problem with the above formats is they are just data and not easy to make decisions about the system and applications at runtime.


__Just__ want to increase the chaos a bit, here is another option:
YAML, which supports comments.

> 
> If the “database” of information could be queried by the EAL, drivers and application then we do not need to try and create a complex command line. It would be nice to execute a DPDK applications like this:
> 
> ./some_dpdk_app –config-file dpdk-config-filename

It could be simpler if you hardcode a default config file, say
/etc/dpdk.conf.

I'm thinking OVS guys would be happy to see that? :)

	--yliu

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 18:51 ` Thomas Monjalon
@ 2016-06-02  9:19   ` Marc
  0 siblings, 0 replies; 49+ messages in thread
From: Marc @ 2016-06-02  9:19 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Wiles, Keith, Yuanhan Liu, dev, Richardson, Bruce, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On 1 June 2016 at 20:51, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> Hi Keith,
>
> I'll try to bring more context to this discussion below.
>
> 2016-06-01 15:00, Wiles, Keith:
> > Started from the link below, but did not want to highjack the thread.
> > http://dpdk.org/ml/archives/dev/2016-June/040021.html
> >
> > I was thinking about this problem from a user perspective and command
> > line options are very difficult to manage specifically when you have
> > a large number of options as we have in dpdk.
>
> The user uses an application.
> It is up to the application to let users do some configuration.
>
> > I see all of these options as a type of database of information for
> > the DPDK and the application, because the application command line
> > options are also getting very complex as well.
>
> DPDK is a collection of libraries.
> There is no command line options in a library.
> So we should not be talking about such issue. But...
>
> ... configuration of the DPDK libraries must be improved.
> We need some clean API to let the application configure a lot of things
> at runtime (during initialization or after).
> Ideally the API should not use an argc/argv format.
>
> We also have a lot of applications for tests or examples which use a
> common configuration scheme based on command line options.
> It is only for test and demonstration purpose. So it is not so important
> and must not be complex to maintain.
> I also think that we should avoid having to modify a configuration file
> for test applications. I like launching a freshly built testpmd with a
> copy-pasted command line without having to create a temporary
> configuration file.
>
> Instead of wrapping a messy configuration interface, we should proceed
> with this steps (in this order):
> - implement clean configuration API
> - move command line options parsing in a separate library
> - implement an alternative to the options parsing library, as an example
> - remove the options parsing library if the alternative is better
>
>
Fully agree on all that you say. To me:

* +1 on staying away from XML and JSON.

* INI is an option, but if there is the need of hierarchical another option
is libconfig
(
http://www.hyperrealm.com/libconfig/libconfig_manual.html#Configuration-Files)
that to me is more readable and user-friendly than JSON (not to mention
XML).

* As you, Thomas say, and as it has been discussed previously [1]; it would
be good that eal_init was not depending on argv and had a _simple_, and
with reasonable defaults, struct-based init API, and build wrapper
libraries on top of that, one being the command-line and another being a
configuration file (although they would be connected somehow).

Marc

[1] http://dpdk.org/ml/archives/dev/2013-August/000374.html

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
                   ` (3 preceding siblings ...)
  2016-06-02  7:56 ` Yuanhan Liu
@ 2016-06-02 10:41 ` Neil Horman
  2016-06-02 13:19   ` Thomas Monjalon
  4 siblings, 1 reply; 49+ messages in thread
From: Neil Horman @ 2016-06-02 10:41 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Yuanhan Liu, Thomas Monjalon, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Wed, Jun 01, 2016 at 03:00:11PM +0000, Wiles, Keith wrote:
> Started from the link below, but did not want to highjack the thread.
> http://dpdk.org/ml/archives/dev/2016-June/040021.html
> 
> I was thinking about this problem from a user perspective and command line options are very difficult to manage specifically when you have a large number of options as we have in dpdk. I see all of these options as a type of database of information for the DPDK and the application, because the application command line options are also getting very complex as well.
> 
> I have been looking at a number of different options here and the direction I was thinking was using a file for the options and configurations with the data in a clean format. It could have been a INI file or JSON or XML, but they all seem to have some problems I do not like. The INI file is too flat and I wanted a hierarchy in the data, the JSON data is similar and XML is just hard to read. I wanted to be able to manage multiple applications and possible system the DPDK/app runs. The problem with the above formats is they are just data and not easy to make decisions about the system and applications at runtime.
> 
> If the “database” of information could be queried by the EAL, drivers and application then we do not need to try and create a complex command line. It would be nice to execute a DPDK applications like this:
> 
> ./some_dpdk_app –config-file dpdk-config-filename
> 
> The dpdk-config-filename could contain a lot of information and be able to startup multiple different applications. The dpdk-config-file could also include other config files to complete the configuration. The format of the data in the config file needs to be readable, but allow the user to put in new options, needs to be hierarchical in nature and have some simple functions to execute if required.
> 
> The solution I was thinking is the file information is really just a fragment of a scripting language, which the DPDK application contains this scripting language interpreter. I was looking at using Lua lua.org as the scripting language interpreter it is small and easy to understand. Python and others are very big and require a lot of resources and Lua requires very few system resources. Also I did not want to have to write a parser (lex/yacc). The other nice feature of Lua is you can create a sandbox for the code to run in and limit the type of system resources and APIs that can be accessed by the application and configuration. Lua can be trimmed down to a fairly small size and builds on just about any system or we can just install Lua on the system without changes from a rpm or deb.
> 
> I use Lua in pktgen at this time and the interface between ‘C’ and Lua is very simple and easy. Currently I include Lua in Pktgen, but I could have just used a system library.
> 
> The data in the config file can be data statements along with some limited code to make some data changes at run time without having to modify the real application. Here is a simple config file I wrote: Some of the options do not make sense to be in the file at the same time, but wanted to see all of the options. The mk_lcore_list() and mk_coremap() are just Lua functions we can preload to help convert the simple strings into real data in this case tables of information. The application could be something like pktgen = { map = { … }, more_options = 1, } this allows the same file to possible contain many application configurations. Needs a bit more work.
> 
> dpdk_default = {
>     lcore_mask = 0xFF00,
>     lcore_list = mk_lcore_list("0-7", 10, "14-16"),
>     coremap = mk_coremap("(0-7)@0,10,(14-16)@1"),
>     master_lcore = 1,
>     log_level = 7,
>     ranks = 4,
>     channels = 2,
>     memory = 512,
>     socket_mem = { 512, 512 },
>     huge_dir = "/mnt/huge",
>     base_virtaddr = 0,
>     create_uio_dev = true,
>     vfio_intr = "legacy",
>     xen_dom0 = false,
>     proc_type = "auto",
>     pci_blacklist = {
>         "08:00.0",
>         "08:00.1",
>         "09:00.0",
>         "09:00.1",
>         "83:00.1",
>         "87:00.0",
>         "87:00.1",
>         "89:00.0",
>         "89:00.1"
>     },
>     pci_whitelist = {
>     },
>     vdev = {
>         eth_pcap0 = { iface = "eth2" },
>         eth_pcap1 = { iface = "eth3" },
>     },
>     driver = { },
>     syslog = true,
>     vmware_tsc_map = false,
>     file_prefix = "pg",
>     huge_unlink = true,
>     no_huge = false,
>     no_pci = false,
>     no_hpet = false,
>     no_shconf = false,
> }
> 
> pktgen_data = {
>    map = { … },
>    more-data = 1,
> }
> 
> The EAL, driver, application, … would query an API to access the data and the application can change his options quickly without modifying the code.
> 
> Anyway comments are welcome.
>  
> Regards,
> Keith

I'm not sure why you're focusing no selecting a config file format at all.  Why
not just focus on removing the argument parsing from the core rte_eal_init code,
instead passing in a configuration struct that is stored and queried per
application.  Leave the parsing of a config file and population of that config
struct as an exercize to the application developer.  That way a given
application can use command line options, config files, or whatever method they
choose, which would be in keeping with traditional application design.

For the purposes of the example apps, it would seem that either JSON, YAML, or
the above Lua format would work just fine.

Neil

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 10:41 ` Neil Horman
@ 2016-06-02 13:19   ` Thomas Monjalon
  2016-06-02 13:53     ` Wiles, Keith
  0 siblings, 1 reply; 49+ messages in thread
From: Thomas Monjalon @ 2016-06-02 13:19 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Yuanhan Liu, dev, Richardson, Bruce, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

2016-06-02 06:41, Neil Horman:
> I'm not sure why you're focusing no selecting a config file format at all.  Why
> not just focus on removing the argument parsing from the core rte_eal_init code,
> instead passing in a configuration struct that is stored and queried per
> application.  Leave the parsing of a config file and population of that config
> struct as an exercize to the application developer.  That way a given
> application can use command line options, config files, or whatever method they
> choose, which would be in keeping with traditional application design.
> 
> For the purposes of the example apps, it would seem that either JSON, YAML, or
> the above Lua format would work just fine.

+1

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 13:19   ` Thomas Monjalon
@ 2016-06-02 13:53     ` Wiles, Keith
  2016-06-02 17:11       ` Neil Horman
  0 siblings, 1 reply; 49+ messages in thread
From: Wiles, Keith @ 2016-06-02 13:53 UTC (permalink / raw)
  To: Thomas Monjalon, Neil Horman
  Cc: Yuanhan Liu, dev, Richardson, Bruce, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz


On 6/2/16, 8:19 AM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>2016-06-02 06:41, Neil Horman:
>> I'm not sure why you're focusing no selecting a config file format at all.  Why

The reason is I am on now looking at formats is because I have been thinking about this issue for some time and already understand your comments. I agree with you and Thomas, which to me would be the details needing to be done as part of the project. I guess I found the config file format a lot more fun to define. ☺

>> not just focus on removing the argument parsing from the core rte_eal_init code,
>> instead passing in a configuration struct that is stored and queried per
>> application.  Leave the parsing of a config file and population of that config
>> struct as an exercize to the application developer.  That way a given
>> application can use command line options, config files, or whatever method they
>> choose, which would be in keeping with traditional application design.

Moving the code out of DPDK into multiple different libraries one for converting command line to config structure (support the current options) and possibly some config file format library to config structure would give options for the developers. DPDK just needs to be driven by a configuration structure or set of APIs and not use args/argv directly.

Moving the current args/argv code out of DPDK into a library should be easy (I guess) and I am willing to do that work if we think it is needed today.

>> 
>> For the purposes of the example apps, it would seem that either JSON, YAML, or
>> the above Lua format would work just fine.
>
>+1
>

Regards,
++Keith



^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 13:53     ` Wiles, Keith
@ 2016-06-02 17:11       ` Neil Horman
  2016-06-02 19:33         ` Wiles, Keith
  2016-06-02 19:41         ` Wiles, Keith
  0 siblings, 2 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-02 17:11 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Thomas Monjalon, Yuanhan Liu, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Thu, Jun 02, 2016 at 01:53:32PM +0000, Wiles, Keith wrote:
> 
> On 6/2/16, 8:19 AM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
> 
> >2016-06-02 06:41, Neil Horman:
> >> I'm not sure why you're focusing no selecting a config file format at all.  Why
> 
> The reason is I am on now looking at formats is because I have been thinking about this issue for some time and already understand your comments. I agree with you and Thomas, which to me would be the details needing to be done as part of the project. I guess I found the config file format a lot more fun to define. ☺
> 

Sure, it is more fun to define, but I think its likely the wrong problem to
solve (or perhaps not even the wrong problem, but rather the less pressing
problem).

> >> not just focus on removing the argument parsing from the core rte_eal_init code,
> >> instead passing in a configuration struct that is stored and queried per
> >> application.  Leave the parsing of a config file and population of that config
> >> struct as an exercize to the application developer.  That way a given
> >> application can use command line options, config files, or whatever method they
> >> choose, which would be in keeping with traditional application design.
> 
> Moving the code out of DPDK into multiple different libraries one for converting command line to config structure (support the current options) and possibly some config file format library to config structure would give options for the developers. DPDK just needs to be driven by a configuration structure or set of APIs and not use args/argv directly.

Yes.  So we agree?

> 
> Moving the current args/argv code out of DPDK into a library should be easy (I guess) and I am willing to do that work if we think it is needed today.
> 
Yes, I think thats the more pressing problem.  To ennumerate, I think whats
really called for is:

1) The definition of a config structure that can be passed to rte_eal_init,
defining the configuration for that running process

2) The creation and use of an API that various DPDK libraries can use to
retrieve that structure (or elements thereof), based on some explicit or imlicit
id, so that the configuration can be used (I'm thinking here specifically of
multiple dpdk applications using a dpdk shared library)

3) The removal of the eal_parse_args code from the core dpdk library entirely,
packaging it instead as its own library that interprets command line arguments
as currently defined, and populates an instance of the structure defined in (1)

4) Altering the Makefiles, so that the example apps link against the new library
in (3), altering the app source code to work with the config structure defined
in (1)

With those steps, I think we will remove the command line bits from the dpdk
core, and do so without altering the user experience for any of the sample apps
(which will demonstrate to other developers that the same can be done with their
applications).  From there we will be free to create alternate methods of
populating the config struct defined in (1) (via JSON file, YAML, XML, or
whatever).

Neil

> >> 
> >> For the purposes of the example apps, it would seem that either JSON, YAML, or
> >> the above Lua format would work just fine.
> >
> >+1
> >
> 
> Regards,
> ++Keith
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 17:11       ` Neil Horman
@ 2016-06-02 19:33         ` Wiles, Keith
  2016-06-02 19:41         ` Wiles, Keith
  1 sibling, 0 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-02 19:33 UTC (permalink / raw)
  To: Neil Horman
  Cc: Thomas Monjalon, Yuanhan Liu, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>On Thu, Jun 02, 2016 at 01:53:32PM +0000, Wiles, Keith wrote:
>> 
>> On 6/2/16, 8:19 AM, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
>> 
>> >2016-06-02 06:41, Neil Horman:
>> >> I'm not sure why you're focusing no selecting a config file format at all.  Why
>> 
>> The reason is I am on now looking at formats is because I have been thinking about this issue for some time and already understand your comments. I agree with you and Thomas, which to me would be the details needing to be done as part of the project. I guess I found the config file format a lot more fun to define. ☺
>> 
>
>Sure, it is more fun to define, but I think its likely the wrong problem to
>solve (or perhaps not even the wrong problem, but rather the less pressing
>problem).

It is not the wrong problem, just a different starting point in the overall problem from the changes below. Now, I believe we have come full circle to identify the whole problem.

Let me look at the problem some and see if I can identify a configuration structure.
>> 
>> Regards,
>> ++Keith




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 17:11       ` Neil Horman
  2016-06-02 19:33         ` Wiles, Keith
@ 2016-06-02 19:41         ` Wiles, Keith
  2016-06-02 20:08           ` Neil Horman
  2016-06-02 20:51           ` Matthew Hall
  1 sibling, 2 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-02 19:41 UTC (permalink / raw)
  To: Neil Horman
  Cc: Thomas Monjalon, Yuanhan Liu, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz


On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>
>1) The definition of a config structure that can be passed to rte_eal_init,
>defining the configuration for that running process

Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.

Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)

Would this work better in the long run, does a fixed structure still make sense?

>
>2) The creation and use of an API that various DPDK libraries can use to
>retrieve that structure (or elements thereof), based on some explicit or imlicit
>id, so that the configuration can be used (I'm thinking here specifically of
>multiple dpdk applications using a dpdk shared library)
>
>3) The removal of the eal_parse_args code from the core dpdk library entirely,
>packaging it instead as its own library that interprets command line arguments
>as currently defined, and populates an instance of the structure defined in (1)
>
>4) Altering the Makefiles, so that the example apps link against the new library
>in (3), altering the app source code to work with the config structure defined
>in (1)
>
>With those steps, I think we will remove the command line bits from the dpdk
>core, and do so without altering the user experience for any of the sample apps
>(which will demonstrate to other developers that the same can be done with their
>applications).  From there we will be free to create alternate methods of
>populating the config struct defined in (1) (via JSON file, YAML, XML, or
>whatever).
>
>Neil
>
>> >> 
>> >> For the purposes of the example apps, it would seem that either JSON, YAML, or
>> >> the above Lua format would work just fine.
>> >
>> >+1
>> >
>> 
>> Regards,
>> ++Keith
>> 
>> 
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 19:41         ` Wiles, Keith
@ 2016-06-02 20:08           ` Neil Horman
  2016-06-02 20:53             ` Matthew Hall
  2016-06-03 10:29             ` Bruce Richardson
  2016-06-02 20:51           ` Matthew Hall
  1 sibling, 2 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-02 20:08 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Thomas Monjalon, Yuanhan Liu, dev, Richardson, Bruce, Tan,
	Jianfeng, Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> 
> On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> 
> >
> >1) The definition of a config structure that can be passed to rte_eal_init,
> >defining the configuration for that running process
> 
> Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.

Thats a fair point.  A decent starting point is likely a simple struct that
looks like this:

struct key_vals {
	char *key;
	union {
		ulong longval;
		void *ptrval;
	} value;
};

struct config {
	size_t count;
	struct key_vals kvp[0];
};

> 
> Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)
> 
Well, if you have config sections that require mulitiple elements, I'd handle
that with naming, i.e. if you have a config group that has an int and char
value, I'd name them "group.intval", and "group.charval", so they are
independently searchable, but linked from a nomenclature standpoint.

> Would this work better in the long run, does a fixed structure still make sense?
> 
No. I think you're ABI concerns are valid, but the above is likely a good
starting point to address them.

Best
Neil


> >
> >2) The creation and use of an API that various DPDK libraries can use to
> >retrieve that structure (or elements thereof), based on some explicit or imlicit
> >id, so that the configuration can be used (I'm thinking here specifically of
> >multiple dpdk applications using a dpdk shared library)
> >
> >3) The removal of the eal_parse_args code from the core dpdk library entirely,
> >packaging it instead as its own library that interprets command line arguments
> >as currently defined, and populates an instance of the structure defined in (1)
> >
> >4) Altering the Makefiles, so that the example apps link against the new library
> >in (3), altering the app source code to work with the config structure defined
> >in (1)
> >
> >With those steps, I think we will remove the command line bits from the dpdk
> >core, and do so without altering the user experience for any of the sample apps
> >(which will demonstrate to other developers that the same can be done with their
> >applications).  From there we will be free to create alternate methods of
> >populating the config struct defined in (1) (via JSON file, YAML, XML, or
> >whatever).
> >
> >Neil
> >
> >> >> 
> >> >> For the purposes of the example apps, it would seem that either JSON, YAML, or
> >> >> the above Lua format would work just fine.
> >> >
> >> >+1
> >> >
> >> 
> >> Regards,
> >> ++Keith
> >> 
> >> 
> >
> 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 19:41         ` Wiles, Keith
  2016-06-02 20:08           ` Neil Horman
@ 2016-06-02 20:51           ` Matthew Hall
  1 sibling, 0 replies; 49+ messages in thread
From: Matthew Hall @ 2016-06-02 20:51 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Neil Horman, Thomas Monjalon, Yuanhan Liu, dev, Richardson,
	Bruce, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> Would this work better in the long run, does a fixed structure still make sense?

This right here is why I suggested libjson-c as an example. It has a nice API 
like this already:

http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 20:08           ` Neil Horman
@ 2016-06-02 20:53             ` Matthew Hall
  2016-06-02 22:34               ` Neil Horman
  2016-06-03 10:29             ` Bruce Richardson
  1 sibling, 1 reply; 49+ messages in thread
From: Matthew Hall @ 2016-06-02 20:53 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Richardson,
	Bruce, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> struct key_vals {
> 	char *key;
> 	union {
> 		ulong longval;
> 		void *ptrval;
> 	} value;
> };
> 
> struct config {
> 	size_t count;
> 	struct key_vals kvp[0];
> };

This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
performance advantage because it's just for config.

Something that looks more like sysctl MIBs with hierarchical names or like 
JSON w/ a hierarchy of hash tables and arrays is much less user-hostile.

https://www.freebsd.org/cgi/man.cgi?sysctl(3)

http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 20:53             ` Matthew Hall
@ 2016-06-02 22:34               ` Neil Horman
  2016-06-03  2:17                 ` Matthew Hall
  0 siblings, 1 reply; 49+ messages in thread
From: Neil Horman @ 2016-06-02 22:34 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Richardson,
	Bruce, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 01:53:55PM -0700, Matthew Hall wrote:
> On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> > struct key_vals {
> > 	char *key;
> > 	union {
> > 		ulong longval;
> > 		void *ptrval;
> > 	} value;
> > };
> > 
> > struct config {
> > 	size_t count;
> > 	struct key_vals kvp[0];
> > };
> 
> This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
> performance advantage because it's just for config.
> 
What!?  I can't even parse that sentence.  Of course its just for config, we're
talking about a configuration structure. If you want to make it more
complex/heirarchical/whatever, fine, propose a way to do that that isnt ABI
variant in response to config additions.  Its just a starting point.

> Something that looks more like sysctl MIBs with hierarchical names or like 
> JSON w/ a hierarchy of hash tables and arrays is much less user-hostile.
> 

> https://www.freebsd.org/cgi/man.cgi?sysctl(3)
> 
I can't even begin to understand what you're after here.  sysctl provides a
heirarchy in _exactly_ the same way that I just proposed, by texual consistency
in naming.

> http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html
> 
So, this is a fine interface to convert text config to a code format, but thats
a decision that application should be making, not something dpdk should mandate

Neil

> Matthew.
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 22:34               ` Neil Horman
@ 2016-06-03  2:17                 ` Matthew Hall
  2016-06-03  9:57                   ` Bruce Richardson
  2016-06-03 12:03                   ` Neil Horman
  0 siblings, 2 replies; 49+ messages in thread
From: Matthew Hall @ 2016-06-03  2:17 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Richardson,
	Bruce, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 06:34:58PM -0400, Neil Horman wrote:
> > This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
> > performance advantage because it's just for config.
> > 
> What!?  I can't even parse that sentence.

I would not want to have to use the structure you proposed in user-readable 
code. It looked a lot like ugly ioctl stuff and I found the sysctl style 
interface easier to read. I don't see why that would be hard for anyone to 
parse but nevertheless.

> > https://www.freebsd.org/cgi/man.cgi?sysctl(3)
> > 
> I can't even begin to understand what you're after here.  sysctl provides a
> heirarchy in _exactly_ the same way that I just proposed, by texual consistency
> in naming.

I didn't object to the hierarchy part, but the user hostility of the example 
proposed.

> > http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html
> > 
> So, this is a fine interface to convert text config to a code format, but thats
> a decision that application should be making, not something dpdk should mandate

You're thinking way too narrowly here for what I am working to convey. I 
wasn't meaning to say JSON had to be used. I was saying, the kind of 
lightweight object-based API they used for modeling JSON has worked very well 
for modeling config data inside of my app. IE, simple functions for working 
with the following sort of entities (which are used in many file / interchange 
systems like JSON, MsgPack, YAML, etc.):

Objects:
* hashes, arbitrarily nested
* arrays, arbitrarily nested

Atoms:
* strings - textual
* strings - binary (something we should add for DPDK)
* integers
* floats / doubles
* booleans

In general I am seeing two good approaches for nesting:

1. name nesting like MIB variable "x.y.z.a.b.c" - this is how sysctl works
2. object nesting- this is how JSON, YAML, MsgPack, INI (implicitly w/ section 
names), XML etc. work...

to express this in the Python / Ruby / JS style syntax it would be:

config['x']['y']['z']['a']['b']['c']
using json-c it would be like

json_object_object_get()... until a json_object_TYPE_get().

What I've done for these in the past, is to make something that can parse the 
sysctl-style name x.y.z.0.a.b.c, detect if each dotted-item is a string, in 
which case reach inside the dict for the string or return NULL if not found, 
and if it's a number reach inside the array for that index and return NULL if 
not found. Here is a Python example how to take the sysctl style and look it 
up inside some objects. The same thing could be done using anything with at 
least as rich of features as what json-c provides...

RE_IS_INT = re.compile('^[0-9]+$')
def retrieve_path(data, path):
    if isinstance(path, basestring):
        path = path.split('.')

    if isinstance(data, Mapping):
        result = data.get(path[0])
    else:
        if not RE_IS_INT.match(str(path[0])):
            return None
        i = int(path[0])
        result = data[i] if len(data) > i else None

    if len(path) == 1:
        return result
    else:
        if result:
            return fetch(result, path[1:])
        else:
            return None

> Neil

Matthew

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03  2:17                 ` Matthew Hall
@ 2016-06-03  9:57                   ` Bruce Richardson
  2016-06-03 10:06                     ` Bruce Richardson
  2016-06-03 12:03                   ` Neil Horman
  1 sibling, 1 reply; 49+ messages in thread
From: Bruce Richardson @ 2016-06-03  9:57 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Neil Horman, Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev,
	Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 07:17:05PM -0700, Matthew Hall wrote:
> On Thu, Jun 02, 2016 at 06:34:58PM -0400, Neil Horman wrote:
> > > This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
> > > performance advantage because it's just for config.
> > > 
> > What!?  I can't even parse that sentence.
> 
> I would not want to have to use the structure you proposed in user-readable 
> code. It looked a lot like ugly ioctl stuff and I found the sysctl style 
> interface easier to read. I don't see why that would be hard for anyone to 
> parse but nevertheless.
> 
> > > https://www.freebsd.org/cgi/man.cgi?sysctl(3)
> > > 
> > I can't even begin to understand what you're after here.  sysctl provides a
> > heirarchy in _exactly_ the same way that I just proposed, by texual consistency
> > in naming.
> 
> I didn't object to the hierarchy part, but the user hostility of the example 
> proposed.
> 
> > > http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html
> > > 
> > So, this is a fine interface to convert text config to a code format, but thats
> > a decision that application should be making, not something dpdk should mandate
> 
> You're thinking way too narrowly here for what I am working to convey. I 
> wasn't meaning to say JSON had to be used. I was saying, the kind of 
> lightweight object-based API they used for modeling JSON has worked very well 
> for modeling config data inside of my app. IE, simple functions for working 
> with the following sort of entities (which are used in many file / interchange 
> systems like JSON, MsgPack, YAML, etc.):
> 
> Objects:
> * hashes, arbitrarily nested
> * arrays, arbitrarily nested
> 
> Atoms:
> * strings - textual
> * strings - binary (something we should add for DPDK)
> * integers
> * floats / doubles
> * booleans
> 
> In general I am seeing two good approaches for nesting:
> 
> 1. name nesting like MIB variable "x.y.z.a.b.c" - this is how sysctl works
> 2. object nesting- this is how JSON, YAML, MsgPack, INI (implicitly w/ section 
> names), XML etc. work...
> 
> to express this in the Python / Ruby / JS style syntax it would be:
> 
> config['x']['y']['z']['a']['b']['c']
> using json-c it would be like
> 
> json_object_object_get()... until a json_object_TYPE_get().
> 
> What I've done for these in the past, is to make something that can parse the 
> sysctl-style name x.y.z.0.a.b.c, detect if each dotted-item is a string, in 
> which case reach inside the dict for the string or return NULL if not found, 
> and if it's a number reach inside the array for that index and return NULL if 
> not found. Here is a Python example how to take the sysctl style and look it 
> up inside some objects. The same thing could be done using anything with at 
> least as rich of features as what json-c provides...
> 
> RE_IS_INT = re.compile('^[0-9]+$')
> def retrieve_path(data, path):
>     if isinstance(path, basestring):
>         path = path.split('.')
> 
>     if isinstance(data, Mapping):
>         result = data.get(path[0])
>     else:
>         if not RE_IS_INT.match(str(path[0])):
>             return None
>         i = int(path[0])
>         result = data[i] if len(data) > i else None
> 
>     if len(path) == 1:
>         return result
>     else:
>         if result:
>             return fetch(result, path[1:])
>         else:
>             return None
> 
> > Neil
> 
> Matthew

I'm afraid I don't see the need to expand out to such a large range of types, or
to add object-type nesting. I'm a big fan of simplicity, and I think Neils
original suggestion of basic name-value pairs is a good one to start with. The
dot notation should work fine for any hierarchies we want to have. If we get
beyond having 2 levels in a hierarchy of config, I think we may have gone
overboard in making things too fine-grained configurable!

/Bruce

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03  9:57                   ` Bruce Richardson
@ 2016-06-03 10:06                     ` Bruce Richardson
  0 siblings, 0 replies; 49+ messages in thread
From: Bruce Richardson @ 2016-06-03 10:06 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Neil Horman, Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev,
	Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Fri, Jun 03, 2016 at 10:57:22AM +0100, Bruce Richardson wrote:
> On Thu, Jun 02, 2016 at 07:17:05PM -0700, Matthew Hall wrote:
> > On Thu, Jun 02, 2016 at 06:34:58PM -0400, Neil Horman wrote:
> > > > This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
> > > > performance advantage because it's just for config.
> > > > 
> > > What!?  I can't even parse that sentence.
> > 
> > I would not want to have to use the structure you proposed in user-readable 
> > code. It looked a lot like ugly ioctl stuff and I found the sysctl style 
> > interface easier to read. I don't see why that would be hard for anyone to 
> > parse but nevertheless.
> > 
> > > > https://www.freebsd.org/cgi/man.cgi?sysctl(3)
> > > > 
> > > I can't even begin to understand what you're after here.  sysctl provides a
> > > heirarchy in _exactly_ the same way that I just proposed, by texual consistency
> > > in naming.
> > 
> > I didn't object to the hierarchy part, but the user hostility of the example 
> > proposed.
> > 
> > > > http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html
> > > > 
> > > So, this is a fine interface to convert text config to a code format, but thats
> > > a decision that application should be making, not something dpdk should mandate
> > 
> > You're thinking way too narrowly here for what I am working to convey. I 
> > wasn't meaning to say JSON had to be used. I was saying, the kind of 
> > lightweight object-based API they used for modeling JSON has worked very well 
> > for modeling config data inside of my app. IE, simple functions for working 
> > with the following sort of entities (which are used in many file / interchange 
> > systems like JSON, MsgPack, YAML, etc.):
> > 
> > Objects:
> > * hashes, arbitrarily nested
> > * arrays, arbitrarily nested
> > 
> > Atoms:
> > * strings - textual
> > * strings - binary (something we should add for DPDK)
> > * integers
> > * floats / doubles
> > * booleans
> > 
> > In general I am seeing two good approaches for nesting:
> > 
> > 1. name nesting like MIB variable "x.y.z.a.b.c" - this is how sysctl works
> > 2. object nesting- this is how JSON, YAML, MsgPack, INI (implicitly w/ section 
> > names), XML etc. work...
> > 
> > to express this in the Python / Ruby / JS style syntax it would be:
> > 
> > config['x']['y']['z']['a']['b']['c']
> > using json-c it would be like
> > 
> > json_object_object_get()... until a json_object_TYPE_get().
> > 
> > What I've done for these in the past, is to make something that can parse the 
> > sysctl-style name x.y.z.0.a.b.c, detect if each dotted-item is a string, in 
> > which case reach inside the dict for the string or return NULL if not found, 
> > and if it's a number reach inside the array for that index and return NULL if 
> > not found. Here is a Python example how to take the sysctl style and look it 
> > up inside some objects. The same thing could be done using anything with at 
> > least as rich of features as what json-c provides...
> > 
> > RE_IS_INT = re.compile('^[0-9]+$')
> > def retrieve_path(data, path):
> >     if isinstance(path, basestring):
> >         path = path.split('.')
> > 
> >     if isinstance(data, Mapping):
> >         result = data.get(path[0])
> >     else:
> >         if not RE_IS_INT.match(str(path[0])):
> >             return None
> >         i = int(path[0])
> >         result = data[i] if len(data) > i else None
> > 
> >     if len(path) == 1:
> >         return result
> >     else:
> >         if result:
> >             return fetch(result, path[1:])
> >         else:
> >             return None
> > 
> > > Neil
> > 
> > Matthew
> 
> I'm afraid I don't see the need to expand out to such a large range of types, or
> to add object-type nesting. I'm a big fan of simplicity, and I think Neils
> original suggestion of basic name-value pairs is a good one to start with. The
> dot notation should work fine for any hierarchies we want to have. If we get
> beyond having 2 levels in a hierarchy of config, I think we may have gone
> overboard in making things too fine-grained configurable!
> 
Minor correction: re-reading my mail afterwards, I realise I actually meant 
2 dots in the name, ie. 2 sublevels, or 3 levels, rather than 2 levels! Must
proofread more.

However, I expect most folks still got my point despite the typo! :-)

/Bruce

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-01 16:18   ` Bruce Richardson
                       ` (2 preceding siblings ...)
  2016-06-01 18:31     ` Stephen Hemminger
@ 2016-06-03 10:07     ` Yerden Zhumabekov
  3 siblings, 0 replies; 49+ messages in thread
From: Yerden Zhumabekov @ 2016-06-03 10:07 UTC (permalink / raw)
  To: dev; +Cc: bruce.richardson

+1

We're using INI in our app, turned out to be quite simple, like this:

[eal]
;; EAL common options:
;;   -c COREMASK         Hexadecimal bitmask of cores to run on
# coremask = fff

;;   -l CORELIST         List of cores to run on
corelist = 3,4,5

;;   --lcores COREMAP    Map lcore set to physical cpu set
; coremap =

;;   --master-lcore ID   Core ID that is used as master
; master-lcore-id = 0

;;   -n CHANNELS         Number of memory channels
memory-channels = 4


On 01.06.2016 22:18, Bruce Richardson wrote:
> On Wed, Jun 01, 2016 at 10:58:41AM -0500, Jay Rolette wrote:
>> On Wed, Jun 1, 2016 at 10:00 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>
>>> Started from the link below, but did not want to highjack the thread.
>>> http://dpdk.org/ml/archives/dev/2016-June/040021.html
>>>
>>> I was thinking about this problem from a user perspective and command line
>>> options are very difficult to manage specifically when you have a large
>>> number of options as we have in dpdk. I see all of these options as a type
>>> of database of information for the DPDK and the application, because the
>>> application command line options are also getting very complex as well.
>>>
>>> I have been looking at a number of different options here and the
>>> direction I was thinking was using a file for the options and
>>> configurations with the data in a clean format. It could have been a INI
>>> file or JSON or XML, but they all seem to have some problems I do not like.
>>> The INI file is too flat and I wanted a hierarchy in the data, the JSON
>>> data is similar and XML is just hard to read. I wanted to be able to manage
>>> multiple applications and possible system the DPDK/app runs. The problem
>>> with the above formats is they are just data and not easy to make decisions
>>> about the system and applications at runtime.
>>>
>> INI format is simplest for users to read, but if you really need hierarchy,
>> JSON will do that just fine. Not sure what you mean by "JSON data is
>> similar"...
>>
>>
> I'd be quite concerned if we start needing lots of hierarchies for configuration.
>
> I'd really just like to see ini file format used for this because:
> * it's a well understood, simple format
> * very easily human readable and editable
> * lots of support for it in lots of languages
> * hierarchies are possible in it too - just not as easy as in other formats
>    though. [In a previous life I worked with ini files which had address
>    hierarchies 6-levels deep in them. It wasn't hard to work with]
> * it works well with grep since you must have one value per-line
> * it allows comments
> * we already have a DPDK library for parsing them
>
> However, for me the biggest advantage of using something like ini is that it
> would force us to keep things simple!
>
> I'd stay away from formats like json or XML that are designed for serializing
> entire objects or structures, and look for something that allows us to just
> specify configuration values.
>
> Regards,
> /Bruce
>

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-02 20:08           ` Neil Horman
  2016-06-02 20:53             ` Matthew Hall
@ 2016-06-03 10:29             ` Bruce Richardson
  2016-06-03 11:01               ` Bruce Richardson
  1 sibling, 1 reply; 49+ messages in thread
From: Bruce Richardson @ 2016-06-03 10:29 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> > 
> > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> > 
> > >
> > >1) The definition of a config structure that can be passed to rte_eal_init,
> > >defining the configuration for that running process
> > 
> > Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.
> 
> Thats a fair point.  A decent starting point is likely a simple struct that
> looks like this:
> 
> struct key_vals {
> 	char *key;
> 	union {
> 		ulong longval;
> 		void *ptrval;
> 	} value;
> };
> 
> struct config {
> 	size_t count;
> 	struct key_vals kvp[0];
> };
> 
> > 
> > Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)
> > 
> Well, if you have config sections that require mulitiple elements, I'd handle
> that with naming, i.e. if you have a config group that has an int and char
> value, I'd name them "group.intval", and "group.charval", so they are
> independently searchable, but linked from a nomenclature standpoint.
> 
> > Would this work better in the long run, does a fixed structure still make sense?
> > 
> No. I think you're ABI concerns are valid, but the above is likely a good
> starting point to address them.
> 
> Best
> Neil

I'll throw out one implementation idea here that I looked at previously, for
the reason that it was simple enough implement with existing code.

We already have the cfgfile library which works with name/value pairs read from
ini files on disk. However, it would be easy enough to add couple of APIs to
that to allow the user to "set" values inside an ini structure as well. With
that done we can then just add a new eal_init api which takes a single
"struct rte_cfgfile *" as parameter. For those apps that want to just use
inifiles for configuration straight, they can then do:

cfg = rte_cfgfile_load("my_cfg_file");
rte_eal_newinit(cfg);

Those who want a different config can instead do:

cfg = rte_cfgfile_new();
rte_cfgfile_add_section(cfg, "dpdk");
foreach_eal_setting_wanted:
	rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
rte_eal_newinit(cfg);

We can standardize on a sectionname, or a couple of standard section names that
are used by DPDK, so that the rest of the config file can contain other data
for the app itself.

What do people think. I mainly like it because it gives us good reuse of what
is already there, and enhances our existing library. As well as this it makes
it trivially easy for apps to use ini files - which seem to be very popular here
- while still giving flexibility for others to use whatever other config format
their app prefers.

/Bruce

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 10:29             ` Bruce Richardson
@ 2016-06-03 11:01               ` Bruce Richardson
  2016-06-03 11:50                 ` Neil Horman
  0 siblings, 1 reply; 49+ messages in thread
From: Bruce Richardson @ 2016-06-03 11:01 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
> On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> > On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> > > 
> > > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> > > 
> > > >
> > > >1) The definition of a config structure that can be passed to rte_eal_init,
> > > >defining the configuration for that running process
> > > 
> > > Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.
> > 
> > Thats a fair point.  A decent starting point is likely a simple struct that
> > looks like this:
> > 
> > struct key_vals {
> > 	char *key;
> > 	union {
> > 		ulong longval;
> > 		void *ptrval;
> > 	} value;
> > };
> > 
> > struct config {
> > 	size_t count;
> > 	struct key_vals kvp[0];
> > };
> > 
> > > 
> > > Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)
> > > 
> > Well, if you have config sections that require mulitiple elements, I'd handle
> > that with naming, i.e. if you have a config group that has an int and char
> > value, I'd name them "group.intval", and "group.charval", so they are
> > independently searchable, but linked from a nomenclature standpoint.
> > 
> > > Would this work better in the long run, does a fixed structure still make sense?
> > > 
> > No. I think you're ABI concerns are valid, but the above is likely a good
> > starting point to address them.
> > 
> > Best
> > Neil
> 
> I'll throw out one implementation idea here that I looked at previously, for
> the reason that it was simple enough implement with existing code.
> 
> We already have the cfgfile library which works with name/value pairs read from
> ini files on disk. However, it would be easy enough to add couple of APIs to
> that to allow the user to "set" values inside an ini structure as well. With
> that done we can then just add a new eal_init api which takes a single
> "struct rte_cfgfile *" as parameter. For those apps that want to just use
> inifiles for configuration straight, they can then do:
> 
> cfg = rte_cfgfile_load("my_cfg_file");
> rte_eal_newinit(cfg);
> 
> Those who want a different config can instead do:
> 
> cfg = rte_cfgfile_new();
> rte_cfgfile_add_section(cfg, "dpdk");
> foreach_eal_setting_wanted:
> 	rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
> rte_eal_newinit(cfg);
> 
>From chatting to a couple of other DPDK dev's here I suspect I may not have
been entirely clear here with this example. What is being shown above is building
up a "config-file" in memory - or rather a config structure which happens to
have the idea of sections and values as an ini file has. There is no actual
file ever being written to disk, and for those using any non-ini config file
structure for their app, the code overhead of using the APIs above should be 
pretty much the same as building up any other set of key-value pairs in
memory to pass to an init function.

Hope this is a little clearer now.

/Bruce

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 11:01               ` Bruce Richardson
@ 2016-06-03 11:50                 ` Neil Horman
  2016-06-03 12:01                   ` Arnon Warshavsky
  2016-06-03 12:14                   ` Panu Matilainen
  0 siblings, 2 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-03 11:50 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Panu Matilainen,
	Olivier Matz

On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
> On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
> > On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> > > On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> > > > 
> > > > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> > > > 
> > > > >
> > > > >1) The definition of a config structure that can be passed to rte_eal_init,
> > > > >defining the configuration for that running process
> > > > 
> > > > Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.
> > > 
> > > Thats a fair point.  A decent starting point is likely a simple struct that
> > > looks like this:
> > > 
> > > struct key_vals {
> > > 	char *key;
> > > 	union {
> > > 		ulong longval;
> > > 		void *ptrval;
> > > 	} value;
> > > };
> > > 
> > > struct config {
> > > 	size_t count;
> > > 	struct key_vals kvp[0];
> > > };
> > > 
> > > > 
> > > > Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)
> > > > 
> > > Well, if you have config sections that require mulitiple elements, I'd handle
> > > that with naming, i.e. if you have a config group that has an int and char
> > > value, I'd name them "group.intval", and "group.charval", so they are
> > > independently searchable, but linked from a nomenclature standpoint.
> > > 
> > > > Would this work better in the long run, does a fixed structure still make sense?
> > > > 
> > > No. I think you're ABI concerns are valid, but the above is likely a good
> > > starting point to address them.
> > > 
> > > Best
> > > Neil
> > 
> > I'll throw out one implementation idea here that I looked at previously, for
> > the reason that it was simple enough implement with existing code.
> > 
> > We already have the cfgfile library which works with name/value pairs read from
> > ini files on disk. However, it would be easy enough to add couple of APIs to
> > that to allow the user to "set" values inside an ini structure as well. With
> > that done we can then just add a new eal_init api which takes a single
> > "struct rte_cfgfile *" as parameter. For those apps that want to just use
> > inifiles for configuration straight, they can then do:
> > 
> > cfg = rte_cfgfile_load("my_cfg_file");
> > rte_eal_newinit(cfg);
> > 
> > Those who want a different config can instead do:
> > 
> > cfg = rte_cfgfile_new();
> > rte_cfgfile_add_section(cfg, "dpdk");
> > foreach_eal_setting_wanted:
> > 	rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
> > rte_eal_newinit(cfg);
> > 
> From chatting to a couple of other DPDK dev's here I suspect I may not have
> been entirely clear here with this example. What is being shown above is building
> up a "config-file" in memory - or rather a config structure which happens to
> have the idea of sections and values as an ini file has. There is no actual
> file ever being written to disk, and for those using any non-ini config file
> structure for their app, the code overhead of using the APIs above should be 
> pretty much the same as building up any other set of key-value pairs in
> memory to pass to an init function.
> 
> Hope this is a little clearer now.
> 
I'm fine with the idea of reusing the config file library that currently exists,
or more to the point, modifying it to be usable as a configuration API, rather
than a configuration file parser.  My primary interest is in separating the user
configuration mechanism from the internal library configuration lookup
mechanism.  What I would really like to be able to see is application developers
have the flexibiilty to choose their own configuration method and format, and
programatically build a configuration for the dpdk on a per-instance basis prior
to calling rte_eal_init

It seems like this approach satisfies that requirement
Neil

> /Bruce
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 11:50                 ` Neil Horman
@ 2016-06-03 12:01                   ` Arnon Warshavsky
  2016-06-03 12:53                     ` Panu Matilainen
  2016-06-03 12:14                   ` Panu Matilainen
  1 sibling, 1 reply; 49+ messages in thread
From: Arnon Warshavsky @ 2016-06-03 12:01 UTC (permalink / raw)
  To: Neil Horman
  Cc: Bruce Richardson, Wiles, Keith, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Fri, Jun 3, 2016 at 2:50 PM, Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
> > On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
> > > On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
> > > > On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
> > > > >
> > > > > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> > > > >
> > > > > >
> > > > > >1) The definition of a config structure that can be passed to
> rte_eal_init,
> > > > > >defining the configuration for that running process
> > > > >
> > > > > Having a configuration structure means we have to have an ABI
> change to that structure anytime we add or remove an option. I was thinking
> a very simple DB of some kind would be better. Have the code query the DB
> to obtain the needed information. The APIs used to query and set the DB
> needs to be very easy to use as well.
> > > >
> > > > Thats a fair point.  A decent starting point is likely a simple
> struct that
> > > > looks like this:
> > > >
> > > > struct key_vals {
> > > >   char *key;
> > > >   union {
> > > >           ulong longval;
> > > >           void *ptrval;
> > > >   } value;
> > > > };
> > > >
> > > > struct config {
> > > >   size_t count;
> > > >   struct key_vals kvp[0];
> > > > };
> > > >
> > > > >
> > > > > Maybe each option can define its own structure if needed or just a
> simple variable type can be used for the basic types (int, string, bool, …)
> > > > >
> > > > Well, if you have config sections that require mulitiple elements,
> I'd handle
> > > > that with naming, i.e. if you have a config group that has an int
> and char
> > > > value, I'd name them "group.intval", and "group.charval", so they are
> > > > independently searchable, but linked from a nomenclature standpoint.
> > > >
> > > > > Would this work better in the long run, does a fixed structure
> still make sense?
> > > > >
> > > > No. I think you're ABI concerns are valid, but the above is likely a
> good
> > > > starting point to address them.
> > > >
> > > > Best
> > > > Neil
> > >
> > > I'll throw out one implementation idea here that I looked at
> previously, for
> > > the reason that it was simple enough implement with existing code.
> > >
> > > We already have the cfgfile library which works with name/value pairs
> read from
> > > ini files on disk. However, it would be easy enough to add couple of
> APIs to
> > > that to allow the user to "set" values inside an ini structure as
> well. With
> > > that done we can then just add a new eal_init api which takes a single
> > > "struct rte_cfgfile *" as parameter. For those apps that want to just
> use
> > > inifiles for configuration straight, they can then do:
> > >
> > > cfg = rte_cfgfile_load("my_cfg_file");
> > > rte_eal_newinit(cfg);
> > >
> > > Those who want a different config can instead do:
> > >
> > > cfg = rte_cfgfile_new();
> > > rte_cfgfile_add_section(cfg, "dpdk");
> > > foreach_eal_setting_wanted:
> > >     rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
> > > rte_eal_newinit(cfg);
> > >
> > From chatting to a couple of other DPDK dev's here I suspect I may not
> have
> > been entirely clear here with this example. What is being shown above is
> building
> > up a "config-file" in memory - or rather a config structure which
> happens to
> > have the idea of sections and values as an ini file has. There is no
> actual
> > file ever being written to disk, and for those using any non-ini config
> file
> > structure for their app, the code overhead of using the APIs above
> should be
> > pretty much the same as building up any other set of key-value pairs in
> > memory to pass to an init function.
> >
> > Hope this is a little clearer now.
> >
> I'm fine with the idea of reusing the config file library that currently
> exists,
> or more to the point, modifying it to be usable as a configuration API,
> rather
> than a configuration file parser.  My primary interest is in separating
> the user
> configuration mechanism from the internal library configuration lookup
> mechanism.  What I would really like to be able to see is application
> developers
> have the flexibiilty to choose their own configuration method and format,
> and
> programatically build a configuration for the dpdk on a per-instance basis
> prior
> to calling rte_eal_init
>
> It seems like this approach satisfies that requirement
> Neil
>
>
If the there is no configuration structure , rather an opaque configuration
key/value store ,
the user applications can set and get knobs that are not seen by anyone
(library) that does not know them by name

e.g

int num_nodes = getCfgInt ( cfgObject , "eal" , "num_numa_nodes");
int delay = getCfgInt ( cfgObject , "drivers.ixgbe" , "some_delay");
setCfgInt (cfgObject , "my_app" , "num_days" , 7);
setCfgString (cfgObject , "my_app" , "best_day" , "Wednesday");

/Arnon

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03  2:17                 ` Matthew Hall
  2016-06-03  9:57                   ` Bruce Richardson
@ 2016-06-03 12:03                   ` Neil Horman
  1 sibling, 0 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-03 12:03 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Richardson,
	Bruce, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Panu Matilainen, Olivier Matz

On Thu, Jun 02, 2016 at 07:17:05PM -0700, Matthew Hall wrote:
> On Thu, Jun 02, 2016 at 06:34:58PM -0400, Neil Horman wrote:
> > > This sort of code is very 1970s / ioctl / messy binary. And doesn't buy any 
> > > performance advantage because it's just for config.
> > > 
> > What!?  I can't even parse that sentence.
> 
> I would not want to have to use the structure you proposed in user-readable 
> code. It looked a lot like ugly ioctl stuff and I found the sysctl style 
> interface easier to read. I don't see why that would be hard for anyone to 
> parse but nevertheless.
> 
I still don't understand how you are likening a typed variable length array to
an ioctl that uses unsigned long a single all encompasing argument, or how you
see a difference between the ASN.1 style naming that sysctl uses and the ASN.1
style naming that  I proposed.  But I suppose thats neither here nor there, we
can agree to disagree on those points.

> > > https://www.freebsd.org/cgi/man.cgi?sysctl(3)
> > > 
> > I can't even begin to understand what you're after here.  sysctl provides a
> > heirarchy in _exactly_ the same way that I just proposed, by texual consistency
> > in naming.
> 
> I didn't object to the hierarchy part, but the user hostility of the example 
> proposed.
> 
I take issue with that, I fail to see anything hostile about variable length
arrays of unioned types.  And if it iss to much, wrap a get/set api around it,
which I would expect anyway to ease lookup tasks.

> > > http://json-c.github.io/json-c/json-c-0.12/doc/html/json__object_8h.html
> > > 
> > So, this is a fine interface to convert text config to a code format, but thats
> > a decision that application should be making, not something dpdk should mandate
> 
> You're thinking way too narrowly here for what I am working to convey. I 
> wasn't meaning to say JSON had to be used. I was saying, the kind of 
> lightweight object-based API they used for modeling JSON has worked very well 
> for modeling config data inside of my app. IE, simple functions for working 
> with the following sort of entities (which are used in many file / interchange 
> systems like JSON, MsgPack, YAML, etc.):
> 
> Objects:
> * hashes, arbitrarily nested
> * arrays, arbitrarily nested
> 
> Atoms:
> * strings - textual
> * strings - binary (something we should add for DPDK)
> * integers
> * floats / doubles
> * booleans
> 
> In general I am seeing two good approaches for nesting:
> 
> 1. name nesting like MIB variable "x.y.z.a.b.c" - this is how sysctl works
> 2. object nesting- this is how JSON, YAML, MsgPack, INI (implicitly w/ section 
> names), XML etc. work...
> 
> to express this in the Python / Ruby / JS style syntax it would be:
> 
> config['x']['y']['z']['a']['b']['c']
> using json-c it would be like
> 
> json_object_object_get()... until a json_object_TYPE_get().
> 

soo, you want to borrow a parsing api for the sole purpose of using it as an
in-memory configuration mechanism?  I suppose thats fine, but as Bruce points
out in his note, the dpdk already has an api that can be repurposed for that
use.

> What I've done for these in the past, is to make something that can parse the 
> sysctl-style name x.y.z.0.a.b.c, detect if each dotted-item is a string, in 
> which case reach inside the dict for the string or return NULL if not found, 
> and if it's a number reach inside the array for that index and return NULL if 
> not found. Here is a Python example how to take the sysctl style and look it 
> up inside some objects. The same thing could be done using anything with at 
> least as rich of features as what json-c provides...
> 
> RE_IS_INT = re.compile('^[0-9]+$')
> def retrieve_path(data, path):
>     if isinstance(path, basestring):
>         path = path.split('.')
> 
>     if isinstance(data, Mapping):
>         result = data.get(path[0])
>     else:
>         if not RE_IS_INT.match(str(path[0])):
>             return None
>         i = int(path[0])
>         result = data[i] if len(data) > i else None
> 
>     if len(path) == 1:
>         return result
>     else:
>         if result:
>             return fetch(result, path[1:])
>         else:
>             return None
> 
> > Neil
> 

That seems....500% more complicated than what dpdk really needs.  All it really
needs is key/value storage, with some minor semblance of a grouping mechanism,
and the abiilty to store some basic types (void * at
a minimum).  The caller should know implicitly based on the key lookup/set what
the type of the value is.  The API could be as simple as:

int setkey(char *key, void *value);
void *getkey(char *key);
void delkey(char *key);


It could be more complex of course, but at a minuimum, thats really enough, if
you didn't just want to expose a data structure to directly alter.

Neil

> Matthew
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 11:50                 ` Neil Horman
  2016-06-03 12:01                   ` Arnon Warshavsky
@ 2016-06-03 12:14                   ` Panu Matilainen
  1 sibling, 0 replies; 49+ messages in thread
From: Panu Matilainen @ 2016-06-03 12:14 UTC (permalink / raw)
  To: Neil Horman, Bruce Richardson
  Cc: Wiles, Keith, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On 06/03/2016 02:50 PM, Neil Horman wrote:
> On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
>> On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
>>> On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
>>>> On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
>>>>>
>>>>> On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
>>>>>
>>>>>>
>>>>>> 1) The definition of a config structure that can be passed to rte_eal_init,
>>>>>> defining the configuration for that running process
>>>>>
>>>>> Having a configuration structure means we have to have an ABI change to that structure anytime we add or remove an option. I was thinking a very simple DB of some kind would be better. Have the code query the DB to obtain the needed information. The APIs used to query and set the DB needs to be very easy to use as well.
>>>>
>>>> Thats a fair point.  A decent starting point is likely a simple struct that
>>>> looks like this:
>>>>
>>>> struct key_vals {
>>>> 	char *key;
>>>> 	union {
>>>> 		ulong longval;
>>>> 		void *ptrval;
>>>> 	} value;
>>>> };
>>>>
>>>> struct config {
>>>> 	size_t count;
>>>> 	struct key_vals kvp[0];
>>>> };
>>>>
>>>>>
>>>>> Maybe each option can define its own structure if needed or just a simple variable type can be used for the basic types (int, string, bool, …)
>>>>>
>>>> Well, if you have config sections that require mulitiple elements, I'd handle
>>>> that with naming, i.e. if you have a config group that has an int and char
>>>> value, I'd name them "group.intval", and "group.charval", so they are
>>>> independently searchable, but linked from a nomenclature standpoint.
>>>>
>>>>> Would this work better in the long run, does a fixed structure still make sense?
>>>>>
>>>> No. I think you're ABI concerns are valid, but the above is likely a good
>>>> starting point to address them.
>>>>
>>>> Best
>>>> Neil
>>>
>>> I'll throw out one implementation idea here that I looked at previously, for
>>> the reason that it was simple enough implement with existing code.
>>>
>>> We already have the cfgfile library which works with name/value pairs read from
>>> ini files on disk. However, it would be easy enough to add couple of APIs to
>>> that to allow the user to "set" values inside an ini structure as well. With
>>> that done we can then just add a new eal_init api which takes a single
>>> "struct rte_cfgfile *" as parameter. For those apps that want to just use
>>> inifiles for configuration straight, they can then do:
>>>
>>> cfg = rte_cfgfile_load("my_cfg_file");
>>> rte_eal_newinit(cfg);
>>>
>>> Those who want a different config can instead do:
>>>
>>> cfg = rte_cfgfile_new();
>>> rte_cfgfile_add_section(cfg, "dpdk");
>>> foreach_eal_setting_wanted:
>>> 	rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
>>> rte_eal_newinit(cfg);
>>>
>> From chatting to a couple of other DPDK dev's here I suspect I may not have
>> been entirely clear here with this example. What is being shown above is building
>> up a "config-file" in memory - or rather a config structure which happens to
>> have the idea of sections and values as an ini file has. There is no actual
>> file ever being written to disk, and for those using any non-ini config file
>> structure for their app, the code overhead of using the APIs above should be
>> pretty much the same as building up any other set of key-value pairs in
>> memory to pass to an init function.

/me nods.

This is pretty much exactly what I suggested (only in much less detail) 
last year :) http://dpdk.org/ml/archives/dev/2015-October/024803.html

>> Hope this is a little clearer now.
> I'm fine with the idea of reusing the config file library that currently exists,
> or more to the point, modifying it to be usable as a configuration API, rather
> than a configuration file parser.  My primary interest is in separating the user
> configuration mechanism from the internal library configuration lookup
> mechanism.  What I would really like to be able to see is application developers
> have the flexibiilty to choose their own configuration method and format, and
> programatically build a configuration for the dpdk on a per-instance basis prior
> to calling rte_eal_init
>
> It seems like this approach satisfies that requirement

/me nods some more.

What the key-value config also can buy us is a direct mapping to cli 
options (which is something Keith has been looking into IIRC), at which 
point I think all the bases are quite nicely covered.

	- Panu -

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 12:01                   ` Arnon Warshavsky
@ 2016-06-03 12:53                     ` Panu Matilainen
  2016-06-03 14:31                       ` Arnon Warshavsky
  0 siblings, 1 reply; 49+ messages in thread
From: Panu Matilainen @ 2016-06-03 12:53 UTC (permalink / raw)
  To: Arnon Warshavsky, Neil Horman
  Cc: Bruce Richardson, Wiles, Keith, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Olivier Matz

On 06/03/2016 03:01 PM, Arnon Warshavsky wrote:
>
>
> On Fri, Jun 3, 2016 at 2:50 PM, Neil Horman <nhorman@tuxdriver.com
> <mailto:nhorman@tuxdriver.com>> wrote:
>
>     On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
>     > On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
>     > > On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
>     > > > On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
>     > > > >
>     > > > > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com
>     <mailto:nhorman@tuxdriver.com>> wrote:
>     > > > >
>     > > > > >
>     > > > > >1) The definition of a config structure that can be passed
>     to rte_eal_init,
>     > > > > >defining the configuration for that running process
>     > > > >
>     > > > > Having a configuration structure means we have to have an
>     ABI change to that structure anytime we add or remove an option. I
>     was thinking a very simple DB of some kind would be better. Have the
>     code query the DB to obtain the needed information. The APIs used to
>     query and set the DB needs to be very easy to use as well.
>     > > >
>     > > > Thats a fair point.  A decent starting point is likely a
>     simple struct that
>     > > > looks like this:
>     > > >
>     > > > struct key_vals {
>     > > >   char *key;
>     > > >   union {
>     > > >           ulong longval;
>     > > >           void *ptrval;
>     > > >   } value;
>     > > > };
>     > > >
>     > > > struct config {
>     > > >   size_t count;
>     > > >   struct key_vals kvp[0];
>     > > > };
>     > > >
>     > > > >
>     > > > > Maybe each option can define its own structure if needed or
>     just a simple variable type can be used for the basic types (int,
>     string, bool, …)
>     > > > >
>     > > > Well, if you have config sections that require mulitiple
>     elements, I'd handle
>     > > > that with naming, i.e. if you have a config group that has an
>     int and char
>     > > > value, I'd name them "group.intval", and "group.charval", so
>     they are
>     > > > independently searchable, but linked from a nomenclature
>     standpoint.
>     > > >
>     > > > > Would this work better in the long run, does a fixed
>     structure still make sense?
>     > > > >
>     > > > No. I think you're ABI concerns are valid, but the above is
>     likely a good
>     > > > starting point to address them.
>     > > >
>     > > > Best
>     > > > Neil
>     > >
>     > > I'll throw out one implementation idea here that I looked at
>     previously, for
>     > > the reason that it was simple enough implement with existing code.
>     > >
>     > > We already have the cfgfile library which works with name/value
>     pairs read from
>     > > ini files on disk. However, it would be easy enough to add
>     couple of APIs to
>     > > that to allow the user to "set" values inside an ini structure
>     as well. With
>     > > that done we can then just add a new eal_init api which takes a
>     single
>     > > "struct rte_cfgfile *" as parameter. For those apps that want to
>     just use
>     > > inifiles for configuration straight, they can then do:
>     > >
>     > > cfg = rte_cfgfile_load("my_cfg_file");
>     > > rte_eal_newinit(cfg);
>     > >
>     > > Those who want a different config can instead do:
>     > >
>     > > cfg = rte_cfgfile_new();
>     > > rte_cfgfile_add_section(cfg, "dpdk");
>     > > foreach_eal_setting_wanted:
>     > >     rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
>     > > rte_eal_newinit(cfg);
>     > >
>     > From chatting to a couple of other DPDK dev's here I suspect I may
>     not have
>     > been entirely clear here with this example. What is being shown
>     above is building
>     > up a "config-file" in memory - or rather a config structure which
>     happens to
>     > have the idea of sections and values as an ini file has. There is
>     no actual
>     > file ever being written to disk, and for those using any non-ini
>     config file
>     > structure for their app, the code overhead of using the APIs above
>     should be
>     > pretty much the same as building up any other set of key-value
>     pairs in
>     > memory to pass to an init function.
>     >
>     > Hope this is a little clearer now.
>     >
>     I'm fine with the idea of reusing the config file library that
>     currently exists,
>     or more to the point, modifying it to be usable as a configuration
>     API, rather
>     than a configuration file parser.  My primary interest is in
>     separating the user
>     configuration mechanism from the internal library configuration lookup
>     mechanism.  What I would really like to be able to see is
>     application developers
>     have the flexibiilty to choose their own configuration method and
>     format, and
>     programatically build a configuration for the dpdk on a per-instance
>     basis prior
>     to calling rte_eal_init
>
>     It seems like this approach satisfies that requirement
>     Neil
>
>
> If the there is no configuration structure , rather an opaque
> configuration key/value store ,
> the user applications can set and get knobs that are not seen by anyone
> (library) that does not know them by name
>
> e.g
>
> int num_nodes = getCfgInt ( cfgObject , "eal" , "num_numa_nodes");
> int delay = getCfgInt ( cfgObject , "drivers.ixgbe" , "some_delay");
> setCfgInt (cfgObject , "my_app" , "num_days" , 7);
> setCfgString (cfgObject , "my_app" , "best_day" , "Wednesday");

I dont see why it would not be possible to have the libraries export 
their known config keys in one way or the other. Or more.

One aspect is runtime queries which would need an API of some kind. 
Being able to query default values should work for that purpose and be 
handy for various other uses as well.

Another one is build-time sanity checking which could be doen by 
auto-generating header(s) from the library known keys, eg

#define CFG_NUM_NUMA_NODES "num_numa_nodes"

so if you use the macro instead of the actual string, you'll get a 
compiler error in case of unknown key instead of runtime misbehavior in 
case of typoed values etc. Whether that's worth it is an entirely 
different question.

	- Panu -





> /Arnon
>

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 12:53                     ` Panu Matilainen
@ 2016-06-03 14:31                       ` Arnon Warshavsky
  2016-06-03 16:04                         ` Wiles, Keith
  0 siblings, 1 reply; 49+ messages in thread
From: Arnon Warshavsky @ 2016-06-03 14:31 UTC (permalink / raw)
  To: Panu Matilainen
  Cc: Neil Horman, Bruce Richardson, Wiles, Keith, Thomas Monjalon,
	Yuanhan Liu, dev, Tan, Jianfeng, Stephen Hemminger,
	Christian Ehrhardt, Olivier Matz

On Fri, Jun 3, 2016 at 3:53 PM, Panu Matilainen <pmatilai@redhat.com> wrote:

> On 06/03/2016 03:01 PM, Arnon Warshavsky wrote:
>
>>
>>
>> On Fri, Jun 3, 2016 at 2:50 PM, Neil Horman <nhorman@tuxdriver.com
>> <mailto:nhorman@tuxdriver.com>> wrote:
>>
>>     On Fri, Jun 03, 2016 at 12:01:30PM +0100, Bruce Richardson wrote:
>>     > On Fri, Jun 03, 2016 at 11:29:43AM +0100, Bruce Richardson wrote:
>>     > > On Thu, Jun 02, 2016 at 04:08:37PM -0400, Neil Horman wrote:
>>     > > > On Thu, Jun 02, 2016 at 07:41:10PM +0000, Wiles, Keith wrote:
>>     > > > >
>>     > > > > On 6/2/16, 12:11 PM, "Neil Horman" <nhorman@tuxdriver.com
>>     <mailto:nhorman@tuxdriver.com>> wrote:
>>     > > > >
>>     > > > > >
>>     > > > > >1) The definition of a config structure that can be passed
>>     to rte_eal_init,
>>     > > > > >defining the configuration for that running process
>>     > > > >
>>     > > > > Having a configuration structure means we have to have an
>>     ABI change to that structure anytime we add or remove an option. I
>>     was thinking a very simple DB of some kind would be better. Have the
>>     code query the DB to obtain the needed information. The APIs used to
>>     query and set the DB needs to be very easy to use as well.
>>     > > >
>>     > > > Thats a fair point.  A decent starting point is likely a
>>     simple struct that
>>     > > > looks like this:
>>     > > >
>>     > > > struct key_vals {
>>     > > >   char *key;
>>     > > >   union {
>>     > > >           ulong longval;
>>     > > >           void *ptrval;
>>     > > >   } value;
>>     > > > };
>>     > > >
>>     > > > struct config {
>>     > > >   size_t count;
>>     > > >   struct key_vals kvp[0];
>>     > > > };
>>     > > >
>>     > > > >
>>     > > > > Maybe each option can define its own structure if needed or
>>     just a simple variable type can be used for the basic types (int,
>>     string, bool, …)
>>     > > > >
>>     > > > Well, if you have config sections that require mulitiple
>>     elements, I'd handle
>>     > > > that with naming, i.e. if you have a config group that has an
>>     int and char
>>     > > > value, I'd name them "group.intval", and "group.charval", so
>>     they are
>>     > > > independently searchable, but linked from a nomenclature
>>     standpoint.
>>     > > >
>>     > > > > Would this work better in the long run, does a fixed
>>     structure still make sense?
>>     > > > >
>>     > > > No. I think you're ABI concerns are valid, but the above is
>>     likely a good
>>     > > > starting point to address them.
>>     > > >
>>     > > > Best
>>     > > > Neil
>>     > >
>>     > > I'll throw out one implementation idea here that I looked at
>>     previously, for
>>     > > the reason that it was simple enough implement with existing code.
>>     > >
>>     > > We already have the cfgfile library which works with name/value
>>     pairs read from
>>     > > ini files on disk. However, it would be easy enough to add
>>     couple of APIs to
>>     > > that to allow the user to "set" values inside an ini structure
>>     as well. With
>>     > > that done we can then just add a new eal_init api which takes a
>>     single
>>     > > "struct rte_cfgfile *" as parameter. For those apps that want to
>>     just use
>>     > > inifiles for configuration straight, they can then do:
>>     > >
>>     > > cfg = rte_cfgfile_load("my_cfg_file");
>>     > > rte_eal_newinit(cfg);
>>     > >
>>     > > Those who want a different config can instead do:
>>     > >
>>     > > cfg = rte_cfgfile_new();
>>     > > rte_cfgfile_add_section(cfg, "dpdk");
>>     > > foreach_eal_setting_wanted:
>>     > >     rte_cfgfile_set(cfg, "dpdk", mysetting, myvalue);
>>     > > rte_eal_newinit(cfg);
>>     > >
>>     > From chatting to a couple of other DPDK dev's here I suspect I may
>>     not have
>>     > been entirely clear here with this example. What is being shown
>>     above is building
>>     > up a "config-file" in memory - or rather a config structure which
>>     happens to
>>     > have the idea of sections and values as an ini file has. There is
>>     no actual
>>     > file ever being written to disk, and for those using any non-ini
>>     config file
>>     > structure for their app, the code overhead of using the APIs above
>>     should be
>>     > pretty much the same as building up any other set of key-value
>>     pairs in
>>     > memory to pass to an init function.
>>     >
>>     > Hope this is a little clearer now.
>>     >
>>     I'm fine with the idea of reusing the config file library that
>>     currently exists,
>>     or more to the point, modifying it to be usable as a configuration
>>     API, rather
>>     than a configuration file parser.  My primary interest is in
>>     separating the user
>>     configuration mechanism from the internal library configuration lookup
>>     mechanism.  What I would really like to be able to see is
>>     application developers
>>     have the flexibiilty to choose their own configuration method and
>>     format, and
>>     programatically build a configuration for the dpdk on a per-instance
>>     basis prior
>>     to calling rte_eal_init
>>
>>     It seems like this approach satisfies that requirement
>>     Neil
>>
>>
>> If the there is no configuration structure , rather an opaque
>> configuration key/value store ,
>> the user applications can set and get knobs that are not seen by anyone
>> (library) that does not know them by name
>>
>> e.g
>>
>> int num_nodes = getCfgInt ( cfgObject , "eal" , "num_numa_nodes");
>> int delay = getCfgInt ( cfgObject , "drivers.ixgbe" , "some_delay");
>> setCfgInt (cfgObject , "my_app" , "num_days" , 7);
>> setCfgString (cfgObject , "my_app" , "best_day" , "Wednesday");
>>
>
> I dont see why it would not be possible to have the libraries export their
> known config keys in one way or the other. Or more.
>
> One aspect is runtime queries which would need an API of some kind. Being
> able to query default values should work for that purpose and be handy for
> various other uses as well.
>
> Another one is build-time sanity checking which could be doen by
> auto-generating header(s) from the library known keys, eg
>
> #define CFG_NUM_NUMA_NODES "num_numa_nodes"
>
> so if you use the macro instead of the actual string, you'll get a
> compiler error in case of unknown key instead of runtime misbehavior in
> case of typoed values etc. Whether that's worth it is an entirely different
> question.
>
>         - Panu -



Thanks Panu .
I was not clear here.
Naturally libraries are better off accessed using well known macro keys.
The other way around that does not require the library to know the keys of
the applications.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 14:31                       ` Arnon Warshavsky
@ 2016-06-03 16:04                         ` Wiles, Keith
  2016-06-03 16:10                           ` Wiles, Keith
  2016-06-03 17:44                           ` Neil Horman
  0 siblings, 2 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 16:04 UTC (permalink / raw)
  To: Arnon Warshavsky, Panu Matilainen
  Cc: Neil Horman, Richardson, Bruce, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Olivier Matz

Sorry, I deleted all of the text as it was getting a bit long.

Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.

- Break out the current command line options out of the DPDK common code and move into a new lib.
  - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
     - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
  - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
  - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.

- I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
  - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
- The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺

- Next is the data storage and how we can access the data in a clean simple way.
- I want to have some simple level of hierarchy in the data.
  - Having a string containing at least two levels “primary:secondary”.
     - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
        - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
     - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”

  - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
     - Key value pairs (KVP) or a hashmap data store.
        - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
           - If we want to have the two split I am ok with that as well meaning the API would be:
             rte_map_get(mapObj, “EAL”, “foo.bar”);
             rte_map_set(mapObj, “EAL”, “foo.bar”, value);
           - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
              - I am leaning toward
     - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
- Use a code design to make the strings simple to use without having typos be a problem.
   - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.

This is as far as I have gotten and got tired of typing ☺

I hope this will satisfy most everyone’s needs for now.


Regards,
Keith




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 16:04                         ` Wiles, Keith
@ 2016-06-03 16:10                           ` Wiles, Keith
  2016-06-03 17:44                           ` Neil Horman
  1 sibling, 0 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 16:10 UTC (permalink / raw)
  To: Arnon Warshavsky, Panu Matilainen
  Cc: Neil Horman, Richardson, Bruce, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Olivier Matz



Regards,
Keith



On 6/3/16, 11:04 AM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>Sorry, I deleted all of the text as it was getting a bit long.
>
>Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
>
>- Break out the current command line options out of the DPDK common code and move into a new lib.
>  - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
>     - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
>  - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
>  - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
>
>- I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
>  - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
>- The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
>
>- Next is the data storage and how we can access the data in a clean simple way.
>- I want to have some simple level of hierarchy in the data.
>  - Having a string containing at least two levels “primary:secondary”.
>     - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
>        - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
>     - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
>
>  - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
>     - Key value pairs (KVP) or a hashmap data store.
>        - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
>           - If we want to have the two split I am ok with that as well meaning the API would be:
>             rte_map_get(mapObj, “EAL”, “foo.bar”);
>             rte_map_set(mapObj, “EAL”, “foo.bar”, value);
>           - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
>              - I am leaning toward
A single string, but let me know your thoughts.
>     - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
>- Use a code design to make the strings simple to use without having typos be a problem.
>   - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
>
>This is as far as I have gotten and got tired of typing ☺
>
>I hope this will satisfy most everyone’s needs for now.
>
>
>Regards,
>Keith
>
>
>
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 16:04                         ` Wiles, Keith
  2016-06-03 16:10                           ` Wiles, Keith
@ 2016-06-03 17:44                           ` Neil Horman
  2016-06-03 18:29                             ` Wiles, Keith
  1 sibling, 1 reply; 49+ messages in thread
From: Neil Horman @ 2016-06-03 17:44 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Arnon Warshavsky, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> Sorry, I deleted all of the text as it was getting a bit long.
> 
> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
> 
> - Break out the current command line options out of the DPDK common code and move into a new lib.
>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
These three items seem to be the exact opposite of my suggestion.  The point of
this change was to segregate the parsing of configuration away from the
initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
way that the command line is directly passed into it, you've not changed that
implicit binding to command line options.

I can understand if you want to keep rte_eal_init as is for ABI purposes, but
then you should create an rte_eal_init2(foo), where foo is some handle to in
memory parsed configuration, so that applications can preform that separation.

Neil

>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
> 
> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
> 
> - Next is the data storage and how we can access the data in a clean simple way.
> - I want to have some simple level of hierarchy in the data.
>   - Having a string containing at least two levels “primary:secondary”.
>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> 
>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
>      - Key value pairs (KVP) or a hashmap data store.
>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
>            - If we want to have the two split I am ok with that as well meaning the API would be:
>              rte_map_get(mapObj, “EAL”, “foo.bar”);
>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
>               - I am leaning toward
>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
> - Use a code design to make the strings simple to use without having typos be a problem.
>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
> 
> This is as far as I have gotten and got tired of typing ☺
> 
> I hope this will satisfy most everyone’s needs for now.
> 
> 
> Regards,
> Keith
> 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 17:44                           ` Neil Horman
@ 2016-06-03 18:29                             ` Wiles, Keith
  2016-06-03 18:38                               ` Neil Horman
  0 siblings, 1 reply; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 18:29 UTC (permalink / raw)
  To: Neil Horman
  Cc: Arnon Warshavsky, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz


On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
>> Sorry, I deleted all of the text as it was getting a bit long.
>> 
>> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
>> 
>> - Break out the current command line options out of the DPDK common code and move into a new lib.
>>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
>>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
>>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
>These three items seem to be the exact opposite of my suggestion.  The point of
>this change was to segregate the parsing of configuration away from the
>initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
>way that the command line is directly passed into it, you've not changed that
>implicit binding to command line options.

Neil,

You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.

>
>I can understand if you want to keep rte_eal_init as is for ABI purposes, but
>then you should create an rte_eal_init2(foo), where foo is some handle to in
>memory parsed configuration, so that applications can preform that separation.

I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.

I hope that clears that up, but let me know.

++Keith

>
>Neil
>
>>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
>> 
>> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
>>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
>> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
>> 
>> - Next is the data storage and how we can access the data in a clean simple way.
>> - I want to have some simple level of hierarchy in the data.
>>   - Having a string containing at least two levels “primary:secondary”.
>>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
>>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
>>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
>> 
>>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
>>      - Key value pairs (KVP) or a hashmap data store.
>>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
>>            - If we want to have the two split I am ok with that as well meaning the API would be:
>>              rte_map_get(mapObj, “EAL”, “foo.bar”);
>>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
>>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
>>               - I am leaning toward
>>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
>> - Use a code design to make the strings simple to use without having typos be a problem.
>>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
>> 
>> This is as far as I have gotten and got tired of typing ☺
>> 
>> I hope this will satisfy most everyone’s needs for now.
>> 
>> 
>> Regards,
>> Keith
>> 
>> 
>> 
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 18:29                             ` Wiles, Keith
@ 2016-06-03 18:38                               ` Neil Horman
  2016-06-03 18:52                                 ` Arnon Warshavsky
  0 siblings, 1 reply; 49+ messages in thread
From: Neil Horman @ 2016-06-03 18:38 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Arnon Warshavsky, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
> 
> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> 
> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> >> Sorry, I deleted all of the text as it was getting a bit long.
> >> 
> >> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
> >> 
> >> - Break out the current command line options out of the DPDK common code and move into a new lib.
> >>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
> >>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
> >>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
> >These three items seem to be the exact opposite of my suggestion.  The point of
> >this change was to segregate the parsing of configuration away from the
> >initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
> >way that the command line is directly passed into it, you've not changed that
> >implicit binding to command line options.
> 
> Neil,
> 
> You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.
> 
> >
> >I can understand if you want to keep rte_eal_init as is for ABI purposes, but
> >then you should create an rte_eal_init2(foo), where foo is some handle to in
> >memory parsed configuration, so that applications can preform that separation.
> 
> I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.
> 
> I hope that clears that up, but let me know.
> 
yes, that clarifies your thinking, and I agree with it.  Thank you!
Neil

> ++Keith
> 
> >
> >Neil
> >
> >>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
> >> 
> >> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
> >>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
> >> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
> >> 
> >> - Next is the data storage and how we can access the data in a clean simple way.
> >> - I want to have some simple level of hierarchy in the data.
> >>   - Having a string containing at least two levels “primary:secondary”.
> >>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
> >>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
> >>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> >> 
> >>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
> >>      - Key value pairs (KVP) or a hashmap data store.
> >>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
> >>            - If we want to have the two split I am ok with that as well meaning the API would be:
> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
> >>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
> >>               - I am leaning toward
> >>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
> >> - Use a code design to make the strings simple to use without having typos be a problem.
> >>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
> >> 
> >> This is as far as I have gotten and got tired of typing ☺
> >> 
> >> I hope this will satisfy most everyone’s needs for now.
> >> 
> >> 
> >> Regards,
> >> Keith
> >> 
> >> 
> >> 
> >
> 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 18:38                               ` Neil Horman
@ 2016-06-03 18:52                                 ` Arnon Warshavsky
  2016-06-03 19:00                                   ` Wiles, Keith
  2016-06-03 21:38                                   ` Matthew Hall
  0 siblings, 2 replies; 49+ messages in thread
From: Arnon Warshavsky @ 2016-06-03 18:52 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com> wrote:

> On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
> >
> > On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> >
> > >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> > >> Sorry, I deleted all of the text as it was getting a bit long.
> > >>
> > >> Here are my thoughts as of now, which is a combination of many
> suggestions I read from everyone’s emails. I hope this is not too hard to
> understand.
> > >>
> > >> - Break out the current command line options out of the DPDK common
> code and move into a new lib.
> > >>   - At this point I was thinking of keeping the rte_eal_init(args,
> argv) API and just have it pass the args/argv to the new lib to create the
> data storage.
> > >>      - Maybe move the rte_eal_init() API to the new lib or keep it in
> the common eal code. Do not want to go hog wild.
> > >>   - The rte_eal_init(args, argv) would then call to the new API
> rte_eal_initialize(void), which in turn queries the data storage. (still
> thinking here)
> > >These three items seem to be the exact opposite of my suggestion.  The
> point of
> > >this change was to segregate the parsing of configuration away from the
> > >initalization dpdk using that configurtion.  By keeping rte_eal_init in
> such a
> > >way that the command line is directly passed into it, you've not
> changed that
> > >implicit binding to command line options.
> >
> > Neil,
> >
> > You maybe reading the above wrong or I wrote it wrong, which is a high
> possibility. I want to move the command line parsing out of DPDK an into a
> library, but I still believe I need to provide some backward compatibility
> for ABI and to reduce the learning curve. The current applications can
> still call the rte_eal_init(), which then calls the new lib parser for dpdk
> command line options and then calls rte_eal_initialize() or move to the new
> API rte_eal_initialize() preceded by a new library call to parse the old
> command line args. At some point we can deprecate the rte_eal_init() if we
> think it is reasonable.
> >
> > >
> > >I can understand if you want to keep rte_eal_init as is for ABI
> purposes, but
> > >then you should create an rte_eal_init2(foo), where foo is some handle
> to in
> > >memory parsed configuration, so that applications can preform that
> separation.
> >
> > I think you describe what I had planned here. The rte_eal_initialize()
> routine is the new rte_eal_init2() API and the rte_eal_init() was only for
> backward compatibility was my thinking. I figured the argument to
> rte_eal_initialize() would be something to be decided, but it will mostly
> likely be some type of pointer to the storage.
> >
> > I hope that clears that up, but let me know.
> >
> yes, that clarifies your thinking, and I agree with it.  Thank you!
> Neil
>
> > ++Keith
> >
> > >
> > >Neil
> > >
> > >>   - The example apps args needs to be passed to the examples as is
> for now, then we can convert them one at a time if needed.
> > >>
> > >> - I would like to keep the storage of the data separate from the file
> parser as they can use the ‘set’ routines to build the data storage up.
> > >>   - Keeping them split allows for new parsers to be created, while
> keeping the data storage from changing.
> > >> - The rte_cfg code could be modified to use the new configuration if
> someone wants to take on that task ☺
> > >>
> > >> - Next is the data storage and how we can access the data in a clean
> simple way.
> > >> - I want to have some simple level of hierarchy in the data.
> > >>   - Having a string containing at least two levels
> “primary:secondary”.
> > >>      - Primary string is something like “EAL” or “Pktgen” or
> “testpmd” to divide the data storage into logical major groups.
> > >>         - The primary allows us to have groups and then we can have
> common secondary strings in different groups if needed.
> > >>      - Secondary string can be whatever the developer of that group
> would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> > >>
> > >>   - The secondary string is treated as a single string if it has a
> hierarchy or not, but referencing a single value in the data storage.
> > >>      - Key value pairs (KVP) or a hashmap data store.
> > >>         - The key here is the whole string “EAL:foobar” not just
> “foobar” secondary string.
> > >>            - If we want to have the two split I am ok with that as
> well meaning the API would be:
> > >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
> > >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
> > >>            - Have the primary as a different section in the data
> store, would allow for dumping that section maybe easier, not sure.
> > >>               - I am leaning toward
> > >>      - Not going to try splitting up the string or parse it as it is
> up to the developer to make it unique in the data store.
> > >> - Use a code design to make the strings simple to use without having
> typos be a problem.
> > >>    - Not sure what the design is yet, but I do not want to have to
> concat two string or split strings in the code.
> > >>
> > >> This is as far as I have gotten and got tired of typing ☺
> > >>
> > >> I hope this will satisfy most everyone’s needs for now.
> > >>
> > >>
> > >> Regards,
> > >> Keith
> > >>
> > >>
> > >>
> > >
> >
> >
> >
>

Keith

What about the data types of the values?
I would assume that as a library it can provide the service of typed
get/set and not leave conversion and validation to the app.

rte_map_get_int(map,section,key)
rte_map_get_double(...)
rte_map_get_string(...)
rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key

This may also allow some basic validity of the configuration file

Another point I forgot about is default values.
We sometimes use a notation where the app also specifies a default value in
case the configuration did not specify it
  rte_map_get_int(map,section,key , defaultValue )
and specify if this was a mandatory that has no default
  rte_map_get_int_crash_if_missing (map,section,key)


/Arnon

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 18:52                                 ` Arnon Warshavsky
@ 2016-06-03 19:00                                   ` Wiles, Keith
  2016-06-03 19:07                                     ` Wiles, Keith
  2016-06-03 21:38                                   ` Matthew Hall
  1 sibling, 1 reply; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 19:00 UTC (permalink / raw)
  To: Arnon Warshavsky, Neil Horman
  Cc: Panu Matilainen, Richardson, Bruce, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Olivier Matz

On 6/3/16, 1:52 PM, "Arnon Warshavsky" <arnon@qwilt.com<mailto:arnon@qwilt.com>> wrote:



On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
>
> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
>
> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> >> Sorry, I deleted all of the text as it was getting a bit long.
> >>
> >> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
> >>
> >> - Break out the current command line options out of the DPDK common code and move into a new lib.
> >>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
> >>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
> >>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
> >These three items seem to be the exact opposite of my suggestion.  The point of
> >this change was to segregate the parsing of configuration away from the
> >initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
> >way that the command line is directly passed into it, you've not changed that
> >implicit binding to command line options.
>
> Neil,
>
> You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.
>
> >
> >I can understand if you want to keep rte_eal_init as is for ABI purposes, but
> >then you should create an rte_eal_init2(foo), where foo is some handle to in
> >memory parsed configuration, so that applications can preform that separation.
>
> I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.
>
> I hope that clears that up, but let me know.
>
yes, that clarifies your thinking, and I agree with it.  Thank you!
Neil

> ++Keith
>
> >
> >Neil
> >
> >>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
> >>
> >> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
> >>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
> >> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
> >>
> >> - Next is the data storage and how we can access the data in a clean simple way.
> >> - I want to have some simple level of hierarchy in the data.
> >>   - Having a string containing at least two levels “primary:secondary”.
> >>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
> >>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
> >>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> >>
> >>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
> >>      - Key value pairs (KVP) or a hashmap data store.
> >>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
> >>            - If we want to have the two split I am ok with that as well meaning the API would be:
> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
> >>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
> >>               - I am leaning toward
> >>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
> >> - Use a code design to make the strings simple to use without having typos be a problem.
> >>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
> >>
> >> This is as far as I have gotten and got tired of typing ☺
> >>
> >> I hope this will satisfy most everyone’s needs for now.
> >>
> >>
> >> Regards,
> >> Keith
> >>
> >>
> >>
> >
>
>
>

Keith
What about the data types of the values?
I would assume that as a library it can provide the service of typed get/set and not leave conversion and validation to the app.

rte_map_get_int(map,section,key)
rte_map_get_double(...)
rte_map_get_string(...)
rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key
This may also allow some basic validity of the configuration file
Another point I forgot about is default values.
We sometimes use a notation where the app also specifies a default value in case the configuration did not specify it
  rte_map_get_int(map,section,key , defaultValue )
and specify if this was a mandatory that has no default
  rte_map_get_int_crash_if_missing (map,section,key)




/Arnon

Arnon,

Yes, I too was thinking about access type APIs, but had not come to a full conclusion yet. As long as the API for get/put can return any value, we can add a layer on top of these primary get/put APIs to do some basic type checking. This way the developer can add his/her own type checking APIs or we provide a couple basic types for simple values.

Does that make sense?

++Keith

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:00                                   ` Wiles, Keith
@ 2016-06-03 19:07                                     ` Wiles, Keith
  2016-06-03 19:18                                       ` Neil Horman
  2016-06-03 21:40                                       ` Matthew Hall
  0 siblings, 2 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 19:07 UTC (permalink / raw)
  To: Arnon Warshavsky, Neil Horman
  Cc: Panu Matilainen, Richardson, Bruce, Thomas Monjalon, Yuanhan Liu,
	dev, Tan, Jianfeng, Stephen Hemminger, Christian Ehrhardt,
	Olivier Matz

On 6/3/16, 2:00 PM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:

>On 6/3/16, 1:52 PM, "Arnon Warshavsky" <arnon@qwilt.com<mailto:arnon@qwilt.com>> wrote:
>
>
>
>On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
>On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
>>
>> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
>>
>> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
>> >> Sorry, I deleted all of the text as it was getting a bit long.
>> >>
>> >> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
>> >>
>> >> - Break out the current command line options out of the DPDK common code and move into a new lib.
>> >>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
>> >>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
>> >>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
>> >These three items seem to be the exact opposite of my suggestion.  The point of
>> >this change was to segregate the parsing of configuration away from the
>> >initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
>> >way that the command line is directly passed into it, you've not changed that
>> >implicit binding to command line options.
>>
>> Neil,
>>
>> You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.
>>
>> >
>> >I can understand if you want to keep rte_eal_init as is for ABI purposes, but
>> >then you should create an rte_eal_init2(foo), where foo is some handle to in
>> >memory parsed configuration, so that applications can preform that separation.
>>
>> I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.
>>
>> I hope that clears that up, but let me know.
>>
>yes, that clarifies your thinking, and I agree with it.  Thank you!
>Neil
>
>> ++Keith
>>
>> >
>> >Neil
>> >
>> >>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
>> >>
>> >> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
>> >>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
>> >> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
>> >>
>> >> - Next is the data storage and how we can access the data in a clean simple way.
>> >> - I want to have some simple level of hierarchy in the data.
>> >>   - Having a string containing at least two levels “primary:secondary”.
>> >>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
>> >>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
>> >>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
>> >>
>> >>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
>> >>      - Key value pairs (KVP) or a hashmap data store.
>> >>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
>> >>            - If we want to have the two split I am ok with that as well meaning the API would be:
>> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
>> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
>> >>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
>> >>               - I am leaning toward
>> >>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
>> >> - Use a code design to make the strings simple to use without having typos be a problem.
>> >>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
>> >>
>> >> This is as far as I have gotten and got tired of typing ☺
>> >>
>> >> I hope this will satisfy most everyone’s needs for now.
>> >>
>> >>
>> >> Regards,
>> >> Keith
>> >>
>> >>
>> >>
>> >
>>
>>
>>
>
>Keith
>What about the data types of the values?
>I would assume that as a library it can provide the service of typed get/set and not leave conversion and validation to the app.
>
>rte_map_get_int(map,section,key)
>rte_map_get_double(...)
>rte_map_get_string(...)
>rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key
>This may also allow some basic validity of the configuration file
>Another point I forgot about is default values.
>We sometimes use a notation where the app also specifies a default value in case the configuration did not specify it
>  rte_map_get_int(map,section,key , defaultValue )
>and specify if this was a mandatory that has no default
>  rte_map_get_int_crash_if_missing (map,section,key)
>
>
>
>
>/Arnon
>
>Arnon,
>
>Yes, I too was thinking about access type APIs, but had not come to a full conclusion yet. As long as the API for get/put can return any value, we can add a layer on top of these primary get/put APIs to do some basic type checking. This way the developer can add his/her own type checking APIs or we provide a couple basic types for simple values.

One more thing. I had not thought about default values as the defaults are handle directly by the code when an option is not applied. I think it should be left up to the developer to add default values to the storage or handle it when an option is not found in the storage.

If I understand your code above the API would pass in a default value if one did not exist in the storage, which I guess is reasonable. Anyone think this is a good idea or not?

>
>Does that make sense?
>
>++Keith
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:07                                     ` Wiles, Keith
@ 2016-06-03 19:18                                       ` Neil Horman
  2016-06-03 19:23                                         ` Wiles, Keith
  2016-06-03 21:41                                         ` Matthew Hall
  2016-06-03 21:40                                       ` Matthew Hall
  1 sibling, 2 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-03 19:18 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Arnon Warshavsky, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 07:07:50PM +0000, Wiles, Keith wrote:
> On 6/3/16, 2:00 PM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:
> 
> >On 6/3/16, 1:52 PM, "Arnon Warshavsky" <arnon@qwilt.com<mailto:arnon@qwilt.com>> wrote:
> >
> >
> >
> >On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
> >On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
> >>
> >> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
> >>
> >> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> >> >> Sorry, I deleted all of the text as it was getting a bit long.
> >> >>
> >> >> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
> >> >>
> >> >> - Break out the current command line options out of the DPDK common code and move into a new lib.
> >> >>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
> >> >>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
> >> >>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
> >> >These three items seem to be the exact opposite of my suggestion.  The point of
> >> >this change was to segregate the parsing of configuration away from the
> >> >initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
> >> >way that the command line is directly passed into it, you've not changed that
> >> >implicit binding to command line options.
> >>
> >> Neil,
> >>
> >> You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.
> >>
> >> >
> >> >I can understand if you want to keep rte_eal_init as is for ABI purposes, but
> >> >then you should create an rte_eal_init2(foo), where foo is some handle to in
> >> >memory parsed configuration, so that applications can preform that separation.
> >>
> >> I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.
> >>
> >> I hope that clears that up, but let me know.
> >>
> >yes, that clarifies your thinking, and I agree with it.  Thank you!
> >Neil
> >
> >> ++Keith
> >>
> >> >
> >> >Neil
> >> >
> >> >>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
> >> >>
> >> >> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
> >> >>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
> >> >> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
> >> >>
> >> >> - Next is the data storage and how we can access the data in a clean simple way.
> >> >> - I want to have some simple level of hierarchy in the data.
> >> >>   - Having a string containing at least two levels “primary:secondary”.
> >> >>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
> >> >>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
> >> >>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> >> >>
> >> >>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
> >> >>      - Key value pairs (KVP) or a hashmap data store.
> >> >>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
> >> >>            - If we want to have the two split I am ok with that as well meaning the API would be:
> >> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
> >> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
> >> >>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
> >> >>               - I am leaning toward
> >> >>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
> >> >> - Use a code design to make the strings simple to use without having typos be a problem.
> >> >>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
> >> >>
> >> >> This is as far as I have gotten and got tired of typing ☺
> >> >>
> >> >> I hope this will satisfy most everyone’s needs for now.
> >> >>
> >> >>
> >> >> Regards,
> >> >> Keith
> >> >>
> >> >>
> >> >>
> >> >
> >>
> >>
> >>
> >
> >Keith
> >What about the data types of the values?
> >I would assume that as a library it can provide the service of typed get/set and not leave conversion and validation to the app.
> >
> >rte_map_get_int(map,section,key)
> >rte_map_get_double(...)
> >rte_map_get_string(...)
> >rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key
> >This may also allow some basic validity of the configuration file
> >Another point I forgot about is default values.
> >We sometimes use a notation where the app also specifies a default value in case the configuration did not specify it
> >  rte_map_get_int(map,section,key , defaultValue )
> >and specify if this was a mandatory that has no default
> >  rte_map_get_int_crash_if_missing (map,section,key)
> >
> >
> >
> >
> >/Arnon
> >
> >Arnon,
> >
> >Yes, I too was thinking about access type APIs, but had not come to a full conclusion yet. As long as the API for get/put can return any value, we can add a layer on top of these primary get/put APIs to do some basic type checking. This way the developer can add his/her own type checking APIs or we provide a couple basic types for simple values.
> 
> One more thing. I had not thought about default values as the defaults are handle directly by the code when an option is not applied. I think it should be left up to the developer to add default values to the storage or handle it when an option is not found in the storage.
> 
> If I understand your code above the API would pass in a default value if one did not exist in the storage, which I guess is reasonable. Anyone think this is a good idea or not?
> 

I'm not opposed to default values, but it seems to me that if we are splitting
out a configuration storage library from dpdk, part of the initzliation of that
library can be installing default values.  That is to say, instead of having the
code specific areas assume a default value if none is present in the config, an
init function for the configuration storage library would just populate the
keystore.  That way all the dpdk itself has to do is a key lookup.

Neil

> >
> >Does that make sense?
> >
> >++Keith
> >
> 
> 
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:18                                       ` Neil Horman
@ 2016-06-03 19:23                                         ` Wiles, Keith
  2016-06-03 19:28                                           ` Arnon Warshavsky
  2016-06-03 21:42                                           ` Matthew Hall
  2016-06-03 21:41                                         ` Matthew Hall
  1 sibling, 2 replies; 49+ messages in thread
From: Wiles, Keith @ 2016-06-03 19:23 UTC (permalink / raw)
  To: Neil Horman
  Cc: Arnon Warshavsky, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz


On 6/3/16, 2:18 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:

>On Fri, Jun 03, 2016 at 07:07:50PM +0000, Wiles, Keith wrote:
>> On 6/3/16, 2:00 PM, "dev on behalf of Wiles, Keith" <dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:
>> 
>> >On 6/3/16, 1:52 PM, "Arnon Warshavsky" <arnon@qwilt.com<mailto:arnon@qwilt.com>> wrote:
>> >
>> >
>> >
>> >On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
>> >On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
>> >>
>> >> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com<mailto:nhorman@tuxdriver.com>> wrote:
>> >>
>> >> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
>> >> >> Sorry, I deleted all of the text as it was getting a bit long.
>> >> >>
>> >> >> Here are my thoughts as of now, which is a combination of many suggestions I read from everyone’s emails. I hope this is not too hard to understand.
>> >> >>
>> >> >> - Break out the current command line options out of the DPDK common code and move into a new lib.
>> >> >>   - At this point I was thinking of keeping the rte_eal_init(args, argv) API and just have it pass the args/argv to the new lib to create the data storage.
>> >> >>      - Maybe move the rte_eal_init() API to the new lib or keep it in the common eal code. Do not want to go hog wild.
>> >> >>   - The rte_eal_init(args, argv) would then call to the new API rte_eal_initialize(void), which in turn queries the data storage. (still thinking here)
>> >> >These three items seem to be the exact opposite of my suggestion.  The point of
>> >> >this change was to segregate the parsing of configuration away from the
>> >> >initalization dpdk using that configurtion.  By keeping rte_eal_init in such a
>> >> >way that the command line is directly passed into it, you've not changed that
>> >> >implicit binding to command line options.
>> >>
>> >> Neil,
>> >>
>> >> You maybe reading the above wrong or I wrote it wrong, which is a high possibility. I want to move the command line parsing out of DPDK an into a library, but I still believe I need to provide some backward compatibility for ABI and to reduce the learning curve. The current applications can still call the rte_eal_init(), which then calls the new lib parser for dpdk command line options and then calls rte_eal_initialize() or move to the new API rte_eal_initialize() preceded by a new library call to parse the old command line args. At some point we can deprecate the rte_eal_init() if we think it is reasonable.
>> >>
>> >> >
>> >> >I can understand if you want to keep rte_eal_init as is for ABI purposes, but
>> >> >then you should create an rte_eal_init2(foo), where foo is some handle to in
>> >> >memory parsed configuration, so that applications can preform that separation.
>> >>
>> >> I think you describe what I had planned here. The rte_eal_initialize() routine is the new rte_eal_init2() API and the rte_eal_init() was only for backward compatibility was my thinking. I figured the argument to rte_eal_initialize() would be something to be decided, but it will mostly likely be some type of pointer to the storage.
>> >>
>> >> I hope that clears that up, but let me know.
>> >>
>> >yes, that clarifies your thinking, and I agree with it.  Thank you!
>> >Neil
>> >
>> >> ++Keith
>> >>
>> >> >
>> >> >Neil
>> >> >
>> >> >>   - The example apps args needs to be passed to the examples as is for now, then we can convert them one at a time if needed.
>> >> >>
>> >> >> - I would like to keep the storage of the data separate from the file parser as they can use the ‘set’ routines to build the data storage up.
>> >> >>   - Keeping them split allows for new parsers to be created, while keeping the data storage from changing.
>> >> >> - The rte_cfg code could be modified to use the new configuration if someone wants to take on that task ☺
>> >> >>
>> >> >> - Next is the data storage and how we can access the data in a clean simple way.
>> >> >> - I want to have some simple level of hierarchy in the data.
>> >> >>   - Having a string containing at least two levels “primary:secondary”.
>> >> >>      - Primary string is something like “EAL” or “Pktgen” or “testpmd” to divide the data storage into logical major groups.
>> >> >>         - The primary allows us to have groups and then we can have common secondary strings in different groups if needed.
>> >> >>      - Secondary string can be whatever the developer of that group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
>> >> >>
>> >> >>   - The secondary string is treated as a single string if it has a hierarchy or not, but referencing a single value in the data storage.
>> >> >>      - Key value pairs (KVP) or a hashmap data store.
>> >> >>         - The key here is the whole string “EAL:foobar” not just “foobar” secondary string.
>> >> >>            - If we want to have the two split I am ok with that as well meaning the API would be:
>> >> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
>> >> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
>> >> >>            - Have the primary as a different section in the data store, would allow for dumping that section maybe easier, not sure.
>> >> >>               - I am leaning toward
>> >> >>      - Not going to try splitting up the string or parse it as it is up to the developer to make it unique in the data store.
>> >> >> - Use a code design to make the strings simple to use without having typos be a problem.
>> >> >>    - Not sure what the design is yet, but I do not want to have to concat two string or split strings in the code.
>> >> >>
>> >> >> This is as far as I have gotten and got tired of typing ☺
>> >> >>
>> >> >> I hope this will satisfy most everyone’s needs for now.
>> >> >>
>> >> >>
>> >> >> Regards,
>> >> >> Keith
>> >> >>
>> >> >>
>> >> >>
>> >> >
>> >>
>> >>
>> >>
>> >
>> >Keith
>> >What about the data types of the values?
>> >I would assume that as a library it can provide the service of typed get/set and not leave conversion and validation to the app.
>> >
>> >rte_map_get_int(map,section,key)
>> >rte_map_get_double(...)
>> >rte_map_get_string(...)
>> >rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key
>> >This may also allow some basic validity of the configuration file
>> >Another point I forgot about is default values.
>> >We sometimes use a notation where the app also specifies a default value in case the configuration did not specify it
>> >  rte_map_get_int(map,section,key , defaultValue )
>> >and specify if this was a mandatory that has no default
>> >  rte_map_get_int_crash_if_missing (map,section,key)
>> >
>> >
>> >
>> >
>> >/Arnon
>> >
>> >Arnon,
>> >
>> >Yes, I too was thinking about access type APIs, but had not come to a full conclusion yet. As long as the API for get/put can return any value, we can add a layer on top of these primary get/put APIs to do some basic type checking. This way the developer can add his/her own type checking APIs or we provide a couple basic types for simple values.
>> 
>> One more thing. I had not thought about default values as the defaults are handle directly by the code when an option is not applied. I think it should be left up to the developer to add default values to the storage or handle it when an option is not found in the storage.
>> 
>> If I understand your code above the API would pass in a default value if one did not exist in the storage, which I guess is reasonable. Anyone think this is a good idea or not?
>> 
>
>I'm not opposed to default values, but it seems to me that if we are splitting
>out a configuration storage library from dpdk, part of the initzliation of that
>library can be installing default values.  That is to say, instead of having the
>code specific areas assume a default value if none is present in the config, an
>init function for the configuration storage library would just populate the
>keystore.  That way all the dpdk itself has to do is a key lookup.

+1

If someone needs or wants default values in the API call then a wrapper functions around the basic keystore APIs can be done by the developer or we can add a new set of APIs to provide that type of feature, just like the variable type APIs. Just as long as the basic APIs do not exclude we can add it later.

>
>Neil
>
>> >
>> >Does that make sense?
>> >
>> >++Keith
>> >
>> 
>> 
>> 
>




^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:23                                         ` Wiles, Keith
@ 2016-06-03 19:28                                           ` Arnon Warshavsky
  2016-06-03 21:42                                           ` Matthew Hall
  1 sibling, 0 replies; 49+ messages in thread
From: Arnon Warshavsky @ 2016-06-03 19:28 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Neil Horman, Panu Matilainen, Richardson, Bruce, Thomas Monjalon,
	Yuanhan Liu, dev, Tan, Jianfeng, Stephen Hemminger,
	Christian Ehrhardt, Olivier Matz

I

On Fri, Jun 3, 2016 at 10:23 PM, Wiles, Keith <keith.wiles@intel.com> wrote:

>
> On 6/3/16, 2:18 PM, "Neil Horman" <nhorman@tuxdriver.com> wrote:
>
> >On Fri, Jun 03, 2016 at 07:07:50PM +0000, Wiles, Keith wrote:
> >> On 6/3/16, 2:00 PM, "dev on behalf of Wiles, Keith" <
> dev-bounces@dpdk.org on behalf of keith.wiles@intel.com> wrote:
> >>
> >> >On 6/3/16, 1:52 PM, "Arnon Warshavsky" <arnon@qwilt.com<mailto:
> arnon@qwilt.com>> wrote:
> >> >
> >> >
> >> >
> >> >On Fri, Jun 3, 2016 at 9:38 PM, Neil Horman <nhorman@tuxdriver.com
> <mailto:nhorman@tuxdriver.com>> wrote:
> >> >On Fri, Jun 03, 2016 at 06:29:13PM +0000, Wiles, Keith wrote:
> >> >>
> >> >> On 6/3/16, 12:44 PM, "Neil Horman" <nhorman@tuxdriver.com<mailto:
> nhorman@tuxdriver.com>> wrote:
> >> >>
> >> >> >On Fri, Jun 03, 2016 at 04:04:14PM +0000, Wiles, Keith wrote:
> >> >> >> Sorry, I deleted all of the text as it was getting a bit long.
> >> >> >>
> >> >> >> Here are my thoughts as of now, which is a combination of many
> suggestions I read from everyone’s emails. I hope this is not too hard to
> understand.
> >> >> >>
> >> >> >> - Break out the current command line options out of the DPDK
> common code and move into a new lib.
> >> >> >>   - At this point I was thinking of keeping the
> rte_eal_init(args, argv) API and just have it pass the args/argv to the new
> lib to create the data storage.
> >> >> >>      - Maybe move the rte_eal_init() API to the new lib or keep
> it in the common eal code. Do not want to go hog wild.
> >> >> >>   - The rte_eal_init(args, argv) would then call to the new API
> rte_eal_initialize(void), which in turn queries the data storage. (still
> thinking here)
> >> >> >These three items seem to be the exact opposite of my suggestion.
> The point of
> >> >> >this change was to segregate the parsing of configuration away from
> the
> >> >> >initalization dpdk using that configurtion.  By keeping
> rte_eal_init in such a
> >> >> >way that the command line is directly passed into it, you've not
> changed that
> >> >> >implicit binding to command line options.
> >> >>
> >> >> Neil,
> >> >>
> >> >> You maybe reading the above wrong or I wrote it wrong, which is a
> high possibility. I want to move the command line parsing out of DPDK an
> into a library, but I still believe I need to provide some backward
> compatibility for ABI and to reduce the learning curve. The current
> applications can still call the rte_eal_init(), which then calls the new
> lib parser for dpdk command line options and then calls
> rte_eal_initialize() or move to the new API rte_eal_initialize() preceded
> by a new library call to parse the old command line args. At some point we
> can deprecate the rte_eal_init() if we think it is reasonable.
> >> >>
> >> >> >
> >> >> >I can understand if you want to keep rte_eal_init as is for ABI
> purposes, but
> >> >> >then you should create an rte_eal_init2(foo), where foo is some
> handle to in
> >> >> >memory parsed configuration, so that applications can preform that
> separation.
> >> >>
> >> >> I think you describe what I had planned here. The
> rte_eal_initialize() routine is the new rte_eal_init2() API and the
> rte_eal_init() was only for backward compatibility was my thinking. I
> figured the argument to rte_eal_initialize() would be something to be
> decided, but it will mostly likely be some type of pointer to the storage.
> >> >>
> >> >> I hope that clears that up, but let me know.
> >> >>
> >> >yes, that clarifies your thinking, and I agree with it.  Thank you!
> >> >Neil
> >> >
> >> >> ++Keith
> >> >>
> >> >> >
> >> >> >Neil
> >> >> >
> >> >> >>   - The example apps args needs to be passed to the examples as
> is for now, then we can convert them one at a time if needed.
> >> >> >>
> >> >> >> - I would like to keep the storage of the data separate from the
> file parser as they can use the ‘set’ routines to build the data storage up.
> >> >> >>   - Keeping them split allows for new parsers to be created,
> while keeping the data storage from changing.
> >> >> >> - The rte_cfg code could be modified to use the new configuration
> if someone wants to take on that task ☺
> >> >> >>
> >> >> >> - Next is the data storage and how we can access the data in a
> clean simple way.
> >> >> >> - I want to have some simple level of hierarchy in the data.
> >> >> >>   - Having a string containing at least two levels
> “primary:secondary”.
> >> >> >>      - Primary string is something like “EAL” or “Pktgen” or
> “testpmd” to divide the data storage into logical major groups.
> >> >> >>         - The primary allows us to have groups and then we can
> have common secondary strings in different groups if needed.
> >> >> >>      - Secondary string can be whatever the developer of that
> group would like e.g. simple “EAL:foobar”, two levels “testpmd:foo.bar”
> >> >> >>
> >> >> >>   - The secondary string is treated as a single string if it has
> a hierarchy or not, but referencing a single value in the data storage.
> >> >> >>      - Key value pairs (KVP) or a hashmap data store.
> >> >> >>         - The key here is the whole string “EAL:foobar” not just
> “foobar” secondary string.
> >> >> >>            - If we want to have the two split I am ok with that
> as well meaning the API would be:
> >> >> >>              rte_map_get(mapObj, “EAL”, “foo.bar”);
> >> >> >>              rte_map_set(mapObj, “EAL”, “foo.bar”, value);
> >> >> >>            - Have the primary as a different section in the data
> store, would allow for dumping that section maybe easier, not sure.
> >> >> >>               - I am leaning toward
> >> >> >>      - Not going to try splitting up the string or parse it as it
> is up to the developer to make it unique in the data store.
> >> >> >> - Use a code design to make the strings simple to use without
> having typos be a problem.
> >> >> >>    - Not sure what the design is yet, but I do not want to have
> to concat two string or split strings in the code.
> >> >> >>
> >> >> >> This is as far as I have gotten and got tired of typing ☺
> >> >> >>
> >> >> >> I hope this will satisfy most everyone’s needs for now.
> >> >> >>
> >> >> >>
> >> >> >> Regards,
> >> >> >> Keith
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >
> >> >>
> >> >>
> >> >>
> >> >
> >> >Keith
> >> >What about the data types of the values?
> >> >I would assume that as a library it can provide the service of typed
> get/set and not leave conversion and validation to the app.
> >> >
> >> >rte_map_get_int(map,section,key)
> >> >rte_map_get_double(...)
> >> >rte_map_get_string(...)
> >> >rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS
> key
> >> >This may also allow some basic validity of the configuration file
> >> >Another point I forgot about is default values.
> >> >We sometimes use a notation where the app also specifies a default
> value in case the configuration did not specify it
> >> >  rte_map_get_int(map,section,key , defaultValue )
> >> >and specify if this was a mandatory that has no default
> >> >  rte_map_get_int_crash_if_missing (map,section,key)
> >> >
> >> >
> >> >
> >> >
> >> >/Arnon
> >> >
> >> >Arnon,
> >> >
> >> >Yes, I too was thinking about access type APIs, but had not come to a
> full conclusion yet. As long as the API for get/put can return any value,
> we can add a layer on top of these primary get/put APIs to do some basic
> type checking. This way the developer can add his/her own type checking
> APIs or we provide a couple basic types for simple values.
> >>
> >> One more thing. I had not thought about default values as the defaults
> are handle directly by the code when an option is not applied. I think it
> should be left up to the developer to add default values to the storage or
> handle it when an option is not found in the storage.
> >>
> >> If I understand your code above the API would pass in a default value
> if one did not exist in the storage, which I guess is reasonable. Anyone
> think this is a good idea or not?
> >>
> >
> >I'm not opposed to default values, but it seems to me that if we are
> splitting
> >out a configuration storage library from dpdk, part of the initzliation
> of that
> >library can be installing default values.  That is to say, instead of
> having the
> >code specific areas assume a default value if none is present in the
> config, an
> >init function for the configuration storage library would just populate
> the
> >keystore.  That way all the dpdk itself has to do is a key lookup.
>
> +1
>
> If someone needs or wants default values in the API call then a wrapper
> functions around the basic keystore APIs can be done by the developer or we
> can add a new set of APIs to provide that type of feature, just like the
> variable type APIs. Just as long as the basic APIs do not exclude we can
> add it later.
>
> >
> >Neil
> >
> >> >
> >> >Does that make sense?
> >> >
> >> >++Keith
> >> >
> >>
> >>
> >>
> >
>
>
>
>
Yes.
I like to use the the getValue(myAlternativeDefault) concept when I have
different granularity defaults coming from different hierarchies,
but per dpdk as a single configuration separation to an init phase indeed
makes more sense, so +1 here too

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 18:52                                 ` Arnon Warshavsky
  2016-06-03 19:00                                   ` Wiles, Keith
@ 2016-06-03 21:38                                   ` Matthew Hall
  1 sibling, 0 replies; 49+ messages in thread
From: Matthew Hall @ 2016-06-03 21:38 UTC (permalink / raw)
  To: Arnon Warshavsky
  Cc: Neil Horman, Wiles, Keith, Panu Matilainen, Richardson, Bruce,
	Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 09:52:40PM +0300, Arnon Warshavsky wrote:
> What about the data types of the values?
> I would assume that as a library it can provide the service of typed
> get/set and not leave conversion and validation to the app.
> 
> rte_map_get_int(map,section,key)
> rte_map_get_double(...)
> rte_map_get_string(...)
> rte_map_get_bytes(...,destBuff , destBuffSize) //e.g byte array of RSS key
> 
> This may also allow some basic validity of the configuration file
> 
> Another point I forgot about is default values.
> We sometimes use a notation where the app also specifies a default value in
> case the configuration did not specify it
>   rte_map_get_int(map,section,key , defaultValue )
> and specify if this was a mandatory that has no default
>   rte_map_get_int_crash_if_missing (map,section,key)

This is why I was advising to use something similar to json-c's API with some 
type specific functions to make things easier for the users, with the 
dot-terminated naming made popular by sysctl such as 
"debug.intel.ctr_KMD_CTR_RPUPEI" and so forth.

> /Arnon

Matthew

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:07                                     ` Wiles, Keith
  2016-06-03 19:18                                       ` Neil Horman
@ 2016-06-03 21:40                                       ` Matthew Hall
  1 sibling, 0 replies; 49+ messages in thread
From: Matthew Hall @ 2016-06-03 21:40 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Arnon Warshavsky, Neil Horman, Panu Matilainen, Richardson,
	Bruce, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 07:07:50PM +0000, Wiles, Keith wrote:
> If I understand your code above the API would pass in a default value if one 
> did not exist in the storage, which I guess is reasonable. Anyone think this 
> is a good idea or not?

This model has worked very well in my code at least. It keeps good reference 
locality between where the option is accessed and how it is configured and 
what it is called... all to the same line of code.

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:18                                       ` Neil Horman
  2016-06-03 19:23                                         ` Wiles, Keith
@ 2016-06-03 21:41                                         ` Matthew Hall
  2016-06-05  0:19                                           ` Neil Horman
  1 sibling, 1 reply; 49+ messages in thread
From: Matthew Hall @ 2016-06-03 21:41 UTC (permalink / raw)
  To: Neil Horman
  Cc: Wiles, Keith, Arnon Warshavsky, Panu Matilainen, Richardson,
	Bruce, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 03:18:04PM -0400, Neil Horman wrote:
> I'm not opposed to default values, but it seems to me that if we are splitting
> out a configuration storage library from dpdk, part of the initzliation of that
> library can be installing default values.  That is to say, instead of having the
> code specific areas assume a default value if none is present in the config, an
> init function for the configuration storage library would just populate the
> keystore.  That way all the dpdk itself has to do is a key lookup.
> 
> Neil

I don't think this provides as much mental locality of reference for people 
reading the code. But, an unwanted default argument can be filled with 0 / 
NULL / false as needed.

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 19:23                                         ` Wiles, Keith
  2016-06-03 19:28                                           ` Arnon Warshavsky
@ 2016-06-03 21:42                                           ` Matthew Hall
  1 sibling, 0 replies; 49+ messages in thread
From: Matthew Hall @ 2016-06-03 21:42 UTC (permalink / raw)
  To: Wiles, Keith
  Cc: Neil Horman, Arnon Warshavsky, Panu Matilainen, Richardson,
	Bruce, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 07:23:39PM +0000, Wiles, Keith wrote:
> If someone needs or wants default values in the API call then a wrapper 
> functions around the basic keystore APIs can be done by the developer or we 
> can add a new set of APIs to provide that type of feature, just like the 
> variable type APIs. Just as long as the basic APIs do not exclude we can add 
> it later.

Please make them part of the default, so this is easy to use and we don't get 
bifurcated and duplicated boilerplate code.

Matthew.

^ permalink raw reply	[flat|nested] 49+ messages in thread

* Re: [RFC] Yet another option for DPDK options
  2016-06-03 21:41                                         ` Matthew Hall
@ 2016-06-05  0:19                                           ` Neil Horman
  0 siblings, 0 replies; 49+ messages in thread
From: Neil Horman @ 2016-06-05  0:19 UTC (permalink / raw)
  To: Matthew Hall
  Cc: Wiles, Keith, Arnon Warshavsky, Panu Matilainen, Richardson,
	Bruce, Thomas Monjalon, Yuanhan Liu, dev, Tan, Jianfeng,
	Stephen Hemminger, Christian Ehrhardt, Olivier Matz

On Fri, Jun 03, 2016 at 02:41:37PM -0700, Matthew Hall wrote:
> On Fri, Jun 03, 2016 at 03:18:04PM -0400, Neil Horman wrote:
> > I'm not opposed to default values, but it seems to me that if we are splitting
> > out a configuration storage library from dpdk, part of the initzliation of that
> > library can be installing default values.  That is to say, instead of having the
> > code specific areas assume a default value if none is present in the config, an
> > init function for the configuration storage library would just populate the
> > keystore.  That way all the dpdk itself has to do is a key lookup.
> > 
> > Neil
> 
> I don't think this provides as much mental locality of reference for people 
> reading the code. But, an unwanted default argument can be filled with 0 / 
> NULL / false as needed.
> 
Well, I'm operating under the assumption that default values are used in cases
where a lack of any value (i.e. the unset case), creates a state in DPDK where
it cannot continue (i.e. it would crash).  If a unset state is for some reason
required on a key that is otherwise populated with a default, then I would
propose that an UnsetKey api call be added.

Neil

> Matthew.
> 

^ permalink raw reply	[flat|nested] 49+ messages in thread

end of thread, other threads:[~2016-06-05  0:20 UTC | newest]

Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-01 15:00 [RFC] Yet another option for DPDK options Wiles, Keith
2016-06-01 15:46 ` Matthew Hall
2016-06-01 16:08   ` Wiles, Keith
2016-06-01 15:58 ` Jay Rolette
2016-06-01 16:18   ` Bruce Richardson
2016-06-01 16:21     ` Arnon Warshavsky
2016-06-01 18:13     ` Wiles, Keith
2016-06-01 18:31     ` Stephen Hemminger
2016-06-03 10:07     ` Yerden Zhumabekov
2016-06-01 18:51 ` Thomas Monjalon
2016-06-02  9:19   ` Marc
2016-06-02  7:56 ` Yuanhan Liu
2016-06-02 10:41 ` Neil Horman
2016-06-02 13:19   ` Thomas Monjalon
2016-06-02 13:53     ` Wiles, Keith
2016-06-02 17:11       ` Neil Horman
2016-06-02 19:33         ` Wiles, Keith
2016-06-02 19:41         ` Wiles, Keith
2016-06-02 20:08           ` Neil Horman
2016-06-02 20:53             ` Matthew Hall
2016-06-02 22:34               ` Neil Horman
2016-06-03  2:17                 ` Matthew Hall
2016-06-03  9:57                   ` Bruce Richardson
2016-06-03 10:06                     ` Bruce Richardson
2016-06-03 12:03                   ` Neil Horman
2016-06-03 10:29             ` Bruce Richardson
2016-06-03 11:01               ` Bruce Richardson
2016-06-03 11:50                 ` Neil Horman
2016-06-03 12:01                   ` Arnon Warshavsky
2016-06-03 12:53                     ` Panu Matilainen
2016-06-03 14:31                       ` Arnon Warshavsky
2016-06-03 16:04                         ` Wiles, Keith
2016-06-03 16:10                           ` Wiles, Keith
2016-06-03 17:44                           ` Neil Horman
2016-06-03 18:29                             ` Wiles, Keith
2016-06-03 18:38                               ` Neil Horman
2016-06-03 18:52                                 ` Arnon Warshavsky
2016-06-03 19:00                                   ` Wiles, Keith
2016-06-03 19:07                                     ` Wiles, Keith
2016-06-03 19:18                                       ` Neil Horman
2016-06-03 19:23                                         ` Wiles, Keith
2016-06-03 19:28                                           ` Arnon Warshavsky
2016-06-03 21:42                                           ` Matthew Hall
2016-06-03 21:41                                         ` Matthew Hall
2016-06-05  0:19                                           ` Neil Horman
2016-06-03 21:40                                       ` Matthew Hall
2016-06-03 21:38                                   ` Matthew Hall
2016-06-03 12:14                   ` Panu Matilainen
2016-06-02 20:51           ` Matthew Hall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.