[dpdk-dev,v2,04/18] eal: add lightweight kvarg parsing utility

Message ID a2487b6a8d1ba3f062f50e1f00727006d96a8feb.1521652453.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Gaëtan Rivet March 21, 2018, 5:15 p.m. UTC
  This library offers a quick way to parse parameters passed with a
key=value syntax.

A single function is needed and finds the relevant element within the
text. No dynamic allocation is performed. It is possible to chain the
parsing of each pairs for quickly scanning a list.

This utility is private to the EAL and should allow avoiding having to
move around the more complete librte_kvargs.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 38 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_private.h    | 34 ++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
  

Comments

Wiles, Keith March 21, 2018, 5:32 p.m. UTC | #1
> On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> 
> This library offers a quick way to parse parameters passed with a
> key=value syntax.
> 
> A single function is needed and finds the relevant element within the
> text. No dynamic allocation is performed. It is possible to chain the
> parsing of each pairs for quickly scanning a list.
> 
> This utility is private to the EAL and should allow avoiding having to
> move around the more complete librte_kvargs.

What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?

My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)

> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Regards,
Keith
  
Gaëtan Rivet March 21, 2018, 5:58 p.m. UTC | #2
Hello Keith,

On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> 
> 
> > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > 
> > This library offers a quick way to parse parameters passed with a
> > key=value syntax.
> > 
> > A single function is needed and finds the relevant element within the
> > text. No dynamic allocation is performed. It is possible to chain the
> > parsing of each pairs for quickly scanning a list.
> > 
> > This utility is private to the EAL and should allow avoiding having to
> > move around the more complete librte_kvargs.
> 
> What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> 

Pretty much, yes. I needed something lean and simple.

> My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> 

I hear you. I decided this was worth the trade-off.

librte_kvargs does the iteration on a kvlist in one-line.
It processes the input text and mangle as needed to be able to do so.
Drivers using it will be able to declare first a set of acceptable keys,
meaning that it is complementary to having a usage-help and more stringent
input validation.

rte_kvargs does none of it. This means that each user would have to
re-implement checks by hand, and reviewers would need to enforce that
they are done, etc.

So, IMO, different use-cases. In this context, I think making the effort
of moving around librte_kvargs within the EAL, tweaking its use for my
needs, does not make much sense. It would complicate its
implementation only for one less private function.

If people do not like using librte_kvargs, it could maybe evolve, be
rewritten, then all drivers would need to follow suit, etc. But I
haven't seen the need personally.

Regards,
  
Neil Horman March 22, 2018, 2:10 p.m. UTC | #3
On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> 
> 
> > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > 
> > This library offers a quick way to parse parameters passed with a
> > key=value syntax.
> > 
> > A single function is needed and finds the relevant element within the
> > text. No dynamic allocation is performed. It is possible to chain the
> > parsing of each pairs for quickly scanning a list.
> > 
> > This utility is private to the EAL and should allow avoiding having to
> > move around the more complete librte_kvargs.
> 
> What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> 
> My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> 
+1, this really doesn't make much sense to me.  Two parsing routines seems like
its just asking for us to have to fix parsing bugs in two places.  If allocation
is a concern, I don't see why you can't just change the malloc in
rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
of kvargs that can be shared from init time.  librte_kvargs isn't necessecarily
the best parsing library ever, but its not bad, and it just seems wrong to go
re-inventing the wheel.

Neil

> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> 
> Regards,
> Keith
> 
>
  
Gaëtan Rivet March 22, 2018, 4:27 p.m. UTC | #4
On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > 
> > 
> > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > 
> > > This library offers a quick way to parse parameters passed with a
> > > key=value syntax.
> > > 
> > > A single function is needed and finds the relevant element within the
> > > text. No dynamic allocation is performed. It is possible to chain the
> > > parsing of each pairs for quickly scanning a list.
> > > 
> > > This utility is private to the EAL and should allow avoiding having to
> > > move around the more complete librte_kvargs.
> > 
> > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > 
> > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > 
> +1, this really doesn't make much sense to me.  Two parsing routines seems like
> its just asking for us to have to fix parsing bugs in two places.  If allocation
> is a concern, I don't see why you can't just change the malloc in
> rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> of kvargs that can be shared from init time.

I think the existing allocation scheme is fine for other usages (in
drivers and so on). Not for what I wanted to do.

>                                               librte_kvargs isn't necessecarily
> the best parsing library ever, but its not bad, and it just seems wrong to go
> re-inventing the wheel.
> 

It serves a different purpose than the one I'm pursuing.

This helper is lightweight and private. If I wanted to integrate my
needs with librte_kvargs, I would be adding new functionalities, making
it more complex, and for a use-case that is useless for the vast
majority of users of the lib.

If that's really an issue, I'm better off simply removing rte_parse_kv
and writing the parsing by hand within my function. This would be ugly
and tedious, but less than moving librte_kvargs within EAL and changing
it to my needs.
  
Neil Horman March 23, 2018, 12:53 a.m. UTC | #5
On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:
> On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> > On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > > 
> > > 
> > > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > > 
> > > > This library offers a quick way to parse parameters passed with a
> > > > key=value syntax.
> > > > 
> > > > A single function is needed and finds the relevant element within the
> > > > text. No dynamic allocation is performed. It is possible to chain the
> > > > parsing of each pairs for quickly scanning a list.
> > > > 
> > > > This utility is private to the EAL and should allow avoiding having to
> > > > move around the more complete librte_kvargs.
> > > 
> > > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > > 
> > > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > > 
> > +1, this really doesn't make much sense to me.  Two parsing routines seems like
> > its just asking for us to have to fix parsing bugs in two places.  If allocation
> > is a concern, I don't see why you can't just change the malloc in
> > rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> > of kvargs that can be shared from init time.
> 
> I think the existing allocation scheme is fine for other usages (in
> drivers and so on). Not for what I wanted to do.
> 
Ok, but thats an adressable issue.  you can bifurcate the parse function to an
internal function that accepts any preallocated kvargs struct, and export two
wrapper functions, one which allocates the struct from the heap, another which
allocated automatically on the stack.

> >                                               librte_kvargs isn't necessecarily
> > the best parsing library ever, but its not bad, and it just seems wrong to go
> > re-inventing the wheel.
> > 
> 
> It serves a different purpose than the one I'm pursuing.
> 
> This helper is lightweight and private. If I wanted to integrate my
> needs with librte_kvargs, I would be adding new functionalities, making
> it more complex, and for a use-case that is useless for the vast
> majority of users of the lib.
> 
Ok, to that end:

1) Privacy is not an issue (at least from my understanding of what your doing).
If we start with the assumption that librte_kvargs is capable of satisfying your
needs (even if its not done in an optimal way), the fact that your version of
the function is internal to the library doesn't seem overly relevant, unless
theres something critical to that privacy that I'm missing.

2) Lightweight function  seems like something that can be integrated with
librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently
non-performant for your needs, specifically?  We talked about the heap
allocation above, is there something else? The string duplication perhaps?


> If that's really an issue, I'm better off simply removing rte_parse_kv
> and writing the parsing by hand within my function. This would be ugly
> and tedious, but less than moving librte_kvargs within EAL and changing
> it to my needs.
I don't think thats necessecary, I just think if you can ennumerate the items
that are non-performant for your needs we can make some changes to librte_kvargs
to optimize around them, or offer parsing options to avoid them, and in the
process avoid some code duplication

Neil

> 
> -- 
> Gaëtan Rivet
> 6WIND
>
  
Gaëtan Rivet March 23, 2018, 9:31 a.m. UTC | #6
On Thu, Mar 22, 2018 at 08:53:49PM -0400, Neil Horman wrote:
> On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:
> > On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> > > On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > > > 
> > > > 
> > > > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > > > 
> > > > > This library offers a quick way to parse parameters passed with a
> > > > > key=value syntax.
> > > > > 
> > > > > A single function is needed and finds the relevant element within the
> > > > > text. No dynamic allocation is performed. It is possible to chain the
> > > > > parsing of each pairs for quickly scanning a list.
> > > > > 
> > > > > This utility is private to the EAL and should allow avoiding having to
> > > > > move around the more complete librte_kvargs.
> > > > 
> > > > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > > > 
> > > > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > > > 
> > > +1, this really doesn't make much sense to me.  Two parsing routines seems like
> > > its just asking for us to have to fix parsing bugs in two places.  If allocation
> > > is a concern, I don't see why you can't just change the malloc in
> > > rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> > > of kvargs that can be shared from init time.
> > 
> > I think the existing allocation scheme is fine for other usages (in
> > drivers and so on). Not for what I wanted to do.
> > 
> Ok, but thats an adressable issue.  you can bifurcate the parse function to an
> internal function that accepts any preallocated kvargs struct, and export two
> wrapper functions, one which allocates the struct from the heap, another which
> allocated automatically on the stack.
> 

Sure, everything is possible.

> > >                                               librte_kvargs isn't necessecarily
> > > the best parsing library ever, but its not bad, and it just seems wrong to go
> > > re-inventing the wheel.
> > > 
> > 
> > It serves a different purpose than the one I'm pursuing.
> > 
> > This helper is lightweight and private. If I wanted to integrate my
> > needs with librte_kvargs, I would be adding new functionalities, making
> > it more complex, and for a use-case that is useless for the vast
> > majority of users of the lib.
> > 
> Ok, to that end:
> 
> 1) Privacy is not an issue (at least from my understanding of what your doing).
> If we start with the assumption that librte_kvargs is capable of satisfying your
> needs (even if its not done in an optimal way), the fact that your version of
> the function is internal to the library doesn't seem overly relevant, unless
> theres something critical to that privacy that I'm missing.
> 

Privacy is only a point I brought up to say that the impact of this
function is minimal. People looking to parse their kvargs should not
have any ambiguity regarding how they should do so. Only librte_kvargs
is available.

> 2) Lightweight function  seems like something that can be integrated with
> librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently
> non-performant for your needs, specifically?  We talked about the heap
> allocation above, is there something else? The string duplication perhaps?
> 
> 

Mostly the way to use it.
The filter strings are
bus=value,.../class=value,...

where either bus= list or class= list can be omitted, but at least one
must appear.

I want to read a single kvarg. I do not want to parse the whole string.
the '/' signifies the end of the current layer.

librte_kvargs does not care about those points. I cannot ask it to only
read either bus or class, as it would then throw an error for all the
other keys (which the EAL has necessarily no knowledge of).

So I would need to:

  * Add a custom storage scheme
  * Add a custom parsing mode stopping at the first kvarg
  * Add an edge-case to ignore the '/', so as not to throw off the rest
    of the parsing (least it be considered part of the previous kvarg
    value field).

Seeing this, does adding those really specifics functionality help
librte_kvargs to be more useful and usable? I do not think so.

It would only serve to disrupt the library for a marginal use-case, with
the introduction of edge-cases that will blur the specs of the lib's
API, making it harder to avoid subtle bugs.

Only way to do so sanely would be to add rte_parse_kv as part of
librte_kvargs, as is. But then the whole thing does not make sense IMO:
no one would care to use it, the maintainance effort is the same, the
likelyhood of bugs as well (but in the process we would disrupt the
distribution of librte_kvargs by moving it within the EAL).

I see no benefit to either solution.

> > If that's really an issue, I'm better off simply removing rte_parse_kv
> > and writing the parsing by hand within my function. This would be ugly
> > and tedious, but less than moving librte_kvargs within EAL and changing
> > it to my needs.
> I don't think thats necessecary, I just think if you can ennumerate the items
> that are non-performant for your needs we can make some changes to librte_kvargs
> to optimize around them, or offer parsing options to avoid them, and in the
> process avoid some code duplication
> 

I think it makes sense to have specialized functions for specialized
use-cases, and forcing the code to be generic and work with all of them
will make it more complicated.

The genericity would only be worth it if people actually needed to parse
the device strings the same way I do. No one has any business doing so.
This genericity adds complexity and issues, without even being useful in
the first place.
  
Neil Horman March 23, 2018, 11:54 a.m. UTC | #7
On Fri, Mar 23, 2018 at 10:31:22AM +0100, Gaëtan Rivet wrote:
> On Thu, Mar 22, 2018 at 08:53:49PM -0400, Neil Horman wrote:
> > On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:
> > > On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> > > > On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > > > > 
> > > > > 
> > > > > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > > > > 
> > > > > > This library offers a quick way to parse parameters passed with a
> > > > > > key=value syntax.
> > > > > > 
> > > > > > A single function is needed and finds the relevant element within the
> > > > > > text. No dynamic allocation is performed. It is possible to chain the
> > > > > > parsing of each pairs for quickly scanning a list.
> > > > > > 
> > > > > > This utility is private to the EAL and should allow avoiding having to
> > > > > > move around the more complete librte_kvargs.
> > > > > 
> > > > > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > > > > 
> > > > > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > > > > 
> > > > +1, this really doesn't make much sense to me.  Two parsing routines seems like
> > > > its just asking for us to have to fix parsing bugs in two places.  If allocation
> > > > is a concern, I don't see why you can't just change the malloc in
> > > > rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> > > > of kvargs that can be shared from init time.
> > > 
> > > I think the existing allocation scheme is fine for other usages (in
> > > drivers and so on). Not for what I wanted to do.
> > > 
> > Ok, but thats an adressable issue.  you can bifurcate the parse function to an
> > internal function that accepts any preallocated kvargs struct, and export two
> > wrapper functions, one which allocates the struct from the heap, another which
> > allocated automatically on the stack.
> > 
> 
> Sure, everything is possible.
> 
Ok.

> > > >                                               librte_kvargs isn't necessecarily
> > > > the best parsing library ever, but its not bad, and it just seems wrong to go
> > > > re-inventing the wheel.
> > > > 
> > > 
> > > It serves a different purpose than the one I'm pursuing.
> > > 
> > > This helper is lightweight and private. If I wanted to integrate my
> > > needs with librte_kvargs, I would be adding new functionalities, making
> > > it more complex, and for a use-case that is useless for the vast
> > > majority of users of the lib.
> > > 
> > Ok, to that end:
> > 
> > 1) Privacy is not an issue (at least from my understanding of what your doing).
> > If we start with the assumption that librte_kvargs is capable of satisfying your
> > needs (even if its not done in an optimal way), the fact that your version of
> > the function is internal to the library doesn't seem overly relevant, unless
> > theres something critical to that privacy that I'm missing.
> > 
> 
> Privacy is only a point I brought up to say that the impact of this
> function is minimal. People looking to parse their kvargs should not
> have any ambiguity regarding how they should do so. Only librte_kvargs
> is available.
> 
Ok, would you also council others developing dpdk apps to write their own
parsing routines when what they needed was trivial for the existing library?
You are people too :)

> > 2) Lightweight function  seems like something that can be integrated with
> > librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently
> > non-performant for your needs, specifically?  We talked about the heap
> > allocation above, is there something else? The string duplication perhaps?
> > 
> > 
> 
> Mostly the way to use it.
> The filter strings are
> bus=value,.../class=value,...
> 
> where either bus= list or class= list can be omitted, but at least one
> must appear.
> 
Ok, so whats the problem with using librte_kvargs for that?  Is it that the list
that acts as the value to the key isn't parsed out into its own set of tokens?
That seems entirely addressable.

> I want to read a single kvarg. I do not want to parse the whole string.
> the '/' signifies the end of the current layer.
> 
This makes it seem like librte_kvargs can handle this as a trivial case of its
functionality.

> librte_kvargs does not care about those points. I cannot ask it to only
> read either bus or class, as it would then throw an error for all the
> other keys (which the EAL has necessarily no knowledge of).
> 
But you can ask it to read both, and within your libraries logic make the
determination as to the validitiy of receiving both.  Alternatively you can
modify the valid_keys check in kvargs to be a regex that matches on either bus
or class, or accept an ignore parameter for keys that may appear but should be
ignored in the light of other keys.  Theres lots of options here.

> So I would need to:
> 
>   * Add a custom storage scheme
>   * Add a custom parsing mode stopping at the first kvarg
>   * Add an edge-case to ignore the '/', so as not to throw off the rest
>     of the parsing (least it be considered part of the previous kvarg
>     value field).
> 
> Seeing this, does adding those really specifics functionality help
> librte_kvargs to be more useful and usable? I do not think so.
> 
I think you're overcomplicating this.
How about enhancing librte_kvargs to make parsing configurable
such that invalid keys get ignored rather than generate errors?

> It would only serve to disrupt the library for a marginal use-case, with
> the introduction of edge-cases that will blur the specs of the lib's
> API, making it harder to avoid subtle bugs.
> 
What do you mean "disrupt the library"?  What is its purpose of a library if not
to do the jobs we want it to?  If everyone created their own routine to do
something that a library could do with some modifications, we'd be left with
1000 versions of the same routine.  If the existing library does 99% of what you
want it to, lets ennumerate what the missing %1 is and make the change, not
throw the baby out with the bathwater.

> Only way to do so sanely would be to add rte_parse_kv as part of
> librte_kvargs, as is. But then the whole thing does not make sense IMO:
> no one would care to use it, the maintainance effort is the same, the
> likelyhood of bugs as well (but in the process we would disrupt the
> distribution of librte_kvargs by moving it within the EAL).
> 
> I see no benefit to either solution.
> 
Again, thats an overcomplication.  As I read it, you have a need to interrogate
a key/value string, whos contents may contain invalid keys (for your parsing
purposes), and whos values may themselves be lists, correct?  If so, I don't see
the problem in enhancing libkvargs to:

1) Allow for the parsing routine to ignore invalid keys (or even ignore specific
invalid keys, and trigger on any unknown keys)

2) Allows for the subparsing of lists into their own set of tokens.

> > > If that's really an issue, I'm better off simply removing rte_parse_kv
> > > and writing the parsing by hand within my function. This would be ugly
> > > and tedious, but less than moving librte_kvargs within EAL and changing
> > > it to my needs.
> > I don't think thats necessecary, I just think if you can ennumerate the items
> > that are non-performant for your needs we can make some changes to librte_kvargs
> > to optimize around them, or offer parsing options to avoid them, and in the
> > process avoid some code duplication
> > 
> 
> I think it makes sense to have specialized functions for specialized
> use-cases, and forcing the code to be generic and work with all of them
> will make it more complicated.
> 
This isn't specialized, its trivial.  Its just a trivial case that libkvargs
isn't built to handle at the moment.  Lets get it there.

> The genericity would only be worth it if people actually needed to parse
> the device strings the same way I do. No one has any business doing so.
> This genericity adds complexity and issues, without even being useful in
> the first place.
> 
I really think you're trying to take a short cut here where none is needed, and
I'm sorry, but I can't support that.

Neil

> -- 
> Gaëtan Rivet
> 6WIND
>
  
Gaëtan Rivet March 23, 2018, 1:12 p.m. UTC | #8
On Fri, Mar 23, 2018 at 07:54:11AM -0400, Neil Horman wrote:
> On Fri, Mar 23, 2018 at 10:31:22AM +0100, Gaëtan Rivet wrote:
> > On Thu, Mar 22, 2018 at 08:53:49PM -0400, Neil Horman wrote:
> > > On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:
> > > > On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> > > > > On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > > > > > 
> > > > > > 
> > > > > > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > > > > > 
> > > > > > > This library offers a quick way to parse parameters passed with a
> > > > > > > key=value syntax.
> > > > > > > 
> > > > > > > A single function is needed and finds the relevant element within the
> > > > > > > text. No dynamic allocation is performed. It is possible to chain the
> > > > > > > parsing of each pairs for quickly scanning a list.
> > > > > > > 
> > > > > > > This utility is private to the EAL and should allow avoiding having to
> > > > > > > move around the more complete librte_kvargs.
> > > > > > 
> > > > > > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > > > > > 
> > > > > > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > > > > > 
> > > > > +1, this really doesn't make much sense to me.  Two parsing routines seems like
> > > > > its just asking for us to have to fix parsing bugs in two places.  If allocation
> > > > > is a concern, I don't see why you can't just change the malloc in
> > > > > rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> > > > > of kvargs that can be shared from init time.
> > > > 
> > > > I think the existing allocation scheme is fine for other usages (in
> > > > drivers and so on). Not for what I wanted to do.
> > > > 
> > > Ok, but thats an adressable issue.  you can bifurcate the parse function to an
> > > internal function that accepts any preallocated kvargs struct, and export two
> > > wrapper functions, one which allocates the struct from the heap, another which
> > > allocated automatically on the stack.
> > > 
> > 
> > Sure, everything is possible.
> > 
> Ok.
> 
> > > > >                                               librte_kvargs isn't necessecarily
> > > > > the best parsing library ever, but its not bad, and it just seems wrong to go
> > > > > re-inventing the wheel.
> > > > > 
> > > > 
> > > > It serves a different purpose than the one I'm pursuing.
> > > > 
> > > > This helper is lightweight and private. If I wanted to integrate my
> > > > needs with librte_kvargs, I would be adding new functionalities, making
> > > > it more complex, and for a use-case that is useless for the vast
> > > > majority of users of the lib.
> > > > 
> > > Ok, to that end:
> > > 
> > > 1) Privacy is not an issue (at least from my understanding of what your doing).
> > > If we start with the assumption that librte_kvargs is capable of satisfying your
> > > needs (even if its not done in an optimal way), the fact that your version of
> > > the function is internal to the library doesn't seem overly relevant, unless
> > > theres something critical to that privacy that I'm missing.
> > > 
> > 
> > Privacy is only a point I brought up to say that the impact of this
> > function is minimal. People looking to parse their kvargs should not
> > have any ambiguity regarding how they should do so. Only librte_kvargs
> > is available.
> > 
> Ok, would you also council others developing dpdk apps to write their own
> parsing routines when what they needed was trivial for the existing library?
> You are people too :)
> 
> > > 2) Lightweight function  seems like something that can be integrated with
> > > librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently
> > > non-performant for your needs, specifically?  We talked about the heap
> > > allocation above, is there something else? The string duplication perhaps?
> > > 
> > > 
> > 
> > Mostly the way to use it.
> > The filter strings are
> > bus=value,.../class=value,...
> > 
> > where either bus= list or class= list can be omitted, but at least one
> > must appear.
> > 
> Ok, so whats the problem with using librte_kvargs for that?  Is it that the list
> that acts as the value to the key isn't parsed out into its own set of tokens?
> That seems entirely addressable.
> 
> > I want to read a single kvarg. I do not want to parse the whole string.
> > the '/' signifies the end of the current layer.
> > 
> This makes it seem like librte_kvargs can handle this as a trivial case of its
> functionality.
> 
> > librte_kvargs does not care about those points. I cannot ask it to only
> > read either bus or class, as it would then throw an error for all the
> > other keys (which the EAL has necessarily no knowledge of).
> > 
> But you can ask it to read both, and within your libraries logic make the
> determination as to the validitiy of receiving both.  Alternatively you can
> modify the valid_keys check in kvargs to be a regex that matches on either bus
> or class, or accept an ignore parameter for keys that may appear but should be
> ignored in the light of other keys.  Theres lots of options here.
> 

No, I will not be adding regex parsing to express a set of acceptable
token :)

> > So I would need to:
> > 
> >   * Add a custom storage scheme
> >   * Add a custom parsing mode stopping at the first kvarg
> >   * Add an edge-case to ignore the '/', so as not to throw off the rest
> >     of the parsing (least it be considered part of the previous kvarg
> >     value field).
> > 
> > Seeing this, does adding those really specifics functionality help
> > librte_kvargs to be more useful and usable? I do not think so.
> > 
> I think you're overcomplicating this.
> How about enhancing librte_kvargs to make parsing configurable
> such that invalid keys get ignored rather than generate errors?
> 

Invalid keys can already be ignored, it's not an issue.

> > It would only serve to disrupt the library for a marginal use-case, with
> > the introduction of edge-cases that will blur the specs of the lib's
> > API, making it harder to avoid subtle bugs.
> > 
> What do you mean "disrupt the library"?  What is its purpose of a library if not
> to do the jobs we want it to?  If everyone created their own routine to do
> something that a library could do with some modifications, we'd be left with
> 1000 versions of the same routine.  If the existing library does 99% of what you
> want it to, lets ennumerate what the missing %1 is and make the change, not
> throw the baby out with the bathwater.
> 
> > Only way to do so sanely would be to add rte_parse_kv as part of
> > librte_kvargs, as is. But then the whole thing does not make sense IMO:
> > no one would care to use it, the maintainance effort is the same, the
> > likelyhood of bugs as well (but in the process we would disrupt the
> > distribution of librte_kvargs by moving it within the EAL).
> > 
> > I see no benefit to either solution.
> > 
> Again, thats an overcomplication.  As I read it, you have a need to interrogate
> a key/value string, whos contents may contain invalid keys (for your parsing
> purposes), and whos values may themselves be lists, correct?  If so, I don't see
> the problem in enhancing libkvargs to:
> 
> 1) Allow for the parsing routine to ignore invalid keys (or even ignore specific
> invalid keys, and trigger on any unknown keys)
> 
> 2) Allows for the subparsing of lists into their own set of tokens.
> 

What I would do if I wanted to use librte_kvargs, would be to duplicate
the input text, mangle it to respect the intended format, and feed it to
librte_kvargs for parsing. Then I would iterate over the pairs, and stop
on the two I'm concerned about.

What I dislike here:

  * I actually do the bulk of the parsing by hand (recognizing first a
    valid input, modifying it to respect the lib, and after iterating on
    the list of pairs and strcmp-ing the ones I want). This is approximately
    as complicated as the current helper function.

  * I have to move librte_kvargs in the EAL.

> > > > If that's really an issue, I'm better off simply removing rte_parse_kv
> > > > and writing the parsing by hand within my function. This would be ugly
> > > > and tedious, but less than moving librte_kvargs within EAL and changing
> > > > it to my needs.
> > > I don't think thats necessecary, I just think if you can ennumerate the items
> > > that are non-performant for your needs we can make some changes to librte_kvargs
> > > to optimize around them, or offer parsing options to avoid them, and in the
> > > process avoid some code duplication
> > > 
> > 
> > I think it makes sense to have specialized functions for specialized
> > use-cases, and forcing the code to be generic and work with all of them
> > will make it more complicated.
> > 
> This isn't specialized, its trivial.  Its just a trivial case that libkvargs
> isn't built to handle at the moment.  Lets get it there.
> 

My use-case is trivial. Putting it within the librte_kvargs makes it
more complicated to write. I hate this.

> > The genericity would only be worth it if people actually needed to parse
> > the device strings the same way I do. No one has any business doing so.
> > This genericity adds complexity and issues, without even being useful in
> > the first place.
> > 
> I really think you're trying to take a short cut here where none is needed, and
> I'm sorry, but I can't support that.

There are countably infinite solutions, we will probably reach one (and
it might even do what we want).
  
Wiles, Keith March 23, 2018, 1:15 p.m. UTC | #9
> On Mar 23, 2018, at 4:31 AM, Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:

> 

> On Thu, Mar 22, 2018 at 08:53:49PM -0400, Neil Horman wrote:

>> On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:

>>> On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:

>>>> On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:

>>>>> 

>>>>> 

>>>>>> On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:

>>>>>> 

>>>>>> This library offers a quick way to parse parameters passed with a

>>>>>> key=value syntax.

>>>>>> 

>>>>>> A single function is needed and finds the relevant element within the

>>>>>> text. No dynamic allocation is performed. It is possible to chain the

>>>>>> parsing of each pairs for quickly scanning a list.

>>>>>> 

>>>>>> This utility is private to the EAL and should allow avoiding having to

>>>>>> move around the more complete librte_kvargs.

>>>>> 

>>>>> What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?

>>>>> 

>>>>> My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)

>>>>> 

>>>> +1, this really doesn't make much sense to me.  Two parsing routines seems like

>>>> its just asking for us to have to fix parsing bugs in two places.  If allocation

>>>> is a concern, I don't see why you can't just change the malloc in

>>>> rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set

>>>> of kvargs that can be shared from init time.

>>> 

>>> I think the existing allocation scheme is fine for other usages (in

>>> drivers and so on). Not for what I wanted to do.

>>> 

>> Ok, but thats an adressable issue.  you can bifurcate the parse function to an

>> internal function that accepts any preallocated kvargs struct, and export two

>> wrapper functions, one which allocates the struct from the heap, another which

>> allocated automatically on the stack.

>> 

> 

> Sure, everything is possible.

> 

>>>>                                              librte_kvargs isn't necessecarily

>>>> the best parsing library ever, but its not bad, and it just seems wrong to go

>>>> re-inventing the wheel.

>>>> 

>>> 

>>> It serves a different purpose than the one I'm pursuing.

>>> 

>>> This helper is lightweight and private. If I wanted to integrate my

>>> needs with librte_kvargs, I would be adding new functionalities, making

>>> it more complex, and for a use-case that is useless for the vast

>>> majority of users of the lib.

>>> 

>> Ok, to that end:

>> 

>> 1) Privacy is not an issue (at least from my understanding of what your doing).

>> If we start with the assumption that librte_kvargs is capable of satisfying your

>> needs (even if its not done in an optimal way), the fact that your version of

>> the function is internal to the library doesn't seem overly relevant, unless

>> theres something critical to that privacy that I'm missing.

>> 

> 

> Privacy is only a point I brought up to say that the impact of this

> function is minimal. People looking to parse their kvargs should not

> have any ambiguity regarding how they should do so. Only librte_kvargs

> is available.

> 

>> 2) Lightweight function  seems like something that can be integrated with

>> librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently

>> non-performant for your needs, specifically?  We talked about the heap

>> allocation above, is there something else? The string duplication perhaps?

>> 

>> 

> 

> Mostly the way to use it.

> The filter strings are

> bus=value,.../class=value,...

> 

> where either bus= list or class= list can be omitted, but at least one

> must appear.

> 

> I want to read a single kvarg. I do not want to parse the whole string.

> the '/' signifies the end of the current layer.

> 

> librte_kvargs does not care about those points. I cannot ask it to only

> read either bus or class, as it would then throw an error for all the

> other keys (which the EAL has necessarily no knowledge of).

> 

> So I would need to:

> 

>  * Add a custom storage scheme

>  * Add a custom parsing mode stopping at the first kvarg

>  * Add an edge-case to ignore the '/', so as not to throw off the rest

>    of the parsing (least it be considered part of the previous kvarg

>    value field).

> 

> Seeing this, does adding those really specifics functionality help

> librte_kvargs to be more useful and usable? I do not think so.

> 

> It would only serve to disrupt the library for a marginal use-case, with

> the introduction of edge-cases that will blur the specs of the lib's

> API, making it harder to avoid subtle bugs.

> 

> Only way to do so sanely would be to add rte_parse_kv as part of

> librte_kvargs, as is. But then the whole thing does not make sense IMO:

> no one would care to use it, the maintainance effort is the same, the

> likelyhood of bugs as well (but in the process we would disrupt the

> distribution of librte_kvargs by moving it within the EAL).


It seems to me you could layer the new functionality on top of rte_kvargs to provide the new layering you want and still provide the old API for current usage. The allocation of memory is internal to rte_kvargs and it really should not allocate memory or allocate on the stack. This change is something we should most likely do for rte_kvargs anyway. I do not like routines that allocate memory for me then expect me to free the memory later only because it needed some internal working space, but that is just me.

In my previous coding needs I had layered a structure and routine on top of rte_kvargs to provide something close to your needs, but in the long run I did not need the code as the design changed. I could look at the again and see if it would help here.

> 

> I see no benefit to either solution.

> 

>>> If that's really an issue, I'm better off simply removing rte_parse_kv

>>> and writing the parsing by hand within my function. This would be ugly

>>> and tedious, but less than moving librte_kvargs within EAL and changing

>>> it to my needs.

>> I don't think thats necessecary, I just think if you can ennumerate the items

>> that are non-performant for your needs we can make some changes to librte_kvargs

>> to optimize around them, or offer parsing options to avoid them, and in the

>> process avoid some code duplication

>> 

> 

> I think it makes sense to have specialized functions for specialized

> use-cases, and forcing the code to be generic and work with all of them

> will make it more complicated.

> 

> The genericity would only be worth it if people actually needed to parse

> the device strings the same way I do. No one has any business doing so.

> This genericity adds complexity and issues, without even being useful in

> the first place.

> 

> -- 

> Gaëtan Rivet

> 6WIND


Regards,
Keith
  
Neil Horman March 26, 2018, 11:23 a.m. UTC | #10
On Fri, Mar 23, 2018 at 02:12:36PM +0100, Gaëtan Rivet wrote:
> On Fri, Mar 23, 2018 at 07:54:11AM -0400, Neil Horman wrote:
> > On Fri, Mar 23, 2018 at 10:31:22AM +0100, Gaëtan Rivet wrote:
> > > On Thu, Mar 22, 2018 at 08:53:49PM -0400, Neil Horman wrote:
> > > > On Thu, Mar 22, 2018 at 05:27:51PM +0100, Gaëtan Rivet wrote:
> > > > > On Thu, Mar 22, 2018 at 10:10:37AM -0400, Neil Horman wrote:
> > > > > > On Wed, Mar 21, 2018 at 05:32:24PM +0000, Wiles, Keith wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > > On Mar 21, 2018, at 12:15 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > > > > > > > 
> > > > > > > > This library offers a quick way to parse parameters passed with a
> > > > > > > > key=value syntax.
> > > > > > > > 
> > > > > > > > A single function is needed and finds the relevant element within the
> > > > > > > > text. No dynamic allocation is performed. It is possible to chain the
> > > > > > > > parsing of each pairs for quickly scanning a list.
> > > > > > > > 
> > > > > > > > This utility is private to the EAL and should allow avoiding having to
> > > > > > > > move around the more complete librte_kvargs.
> > > > > > > 
> > > > > > > What is the big advantage with this code and the librte_kvargs code. Is it just no allocation, rte_kvargs needs to be build before parts of EAL or what?
> > > > > > > 
> > > > > > > My concern is we have now two flavors one in EAL and one in librte_kvargs, would it not be more reasonable to improve rte_kvargs to remove your objections? I am all for fast, better, stronger code :-)
> > > > > > > 
> > > > > > +1, this really doesn't make much sense to me.  Two parsing routines seems like
> > > > > > its just asking for us to have to fix parsing bugs in two places.  If allocation
> > > > > > is a concern, I don't see why you can't just change the malloc in
> > > > > > rte_kvargs_parse to an automatic allocation on the stack, or a preallocation set
> > > > > > of kvargs that can be shared from init time.
> > > > > 
> > > > > I think the existing allocation scheme is fine for other usages (in
> > > > > drivers and so on). Not for what I wanted to do.
> > > > > 
> > > > Ok, but thats an adressable issue.  you can bifurcate the parse function to an
> > > > internal function that accepts any preallocated kvargs struct, and export two
> > > > wrapper functions, one which allocates the struct from the heap, another which
> > > > allocated automatically on the stack.
> > > > 
> > > 
> > > Sure, everything is possible.
> > > 
> > Ok.
> > 
> > > > > >                                               librte_kvargs isn't necessecarily
> > > > > > the best parsing library ever, but its not bad, and it just seems wrong to go
> > > > > > re-inventing the wheel.
> > > > > > 
> > > > > 
> > > > > It serves a different purpose than the one I'm pursuing.
> > > > > 
> > > > > This helper is lightweight and private. If I wanted to integrate my
> > > > > needs with librte_kvargs, I would be adding new functionalities, making
> > > > > it more complex, and for a use-case that is useless for the vast
> > > > > majority of users of the lib.
> > > > > 
> > > > Ok, to that end:
> > > > 
> > > > 1) Privacy is not an issue (at least from my understanding of what your doing).
> > > > If we start with the assumption that librte_kvargs is capable of satisfying your
> > > > needs (even if its not done in an optimal way), the fact that your version of
> > > > the function is internal to the library doesn't seem overly relevant, unless
> > > > theres something critical to that privacy that I'm missing.
> > > > 
> > > 
> > > Privacy is only a point I brought up to say that the impact of this
> > > function is minimal. People looking to parse their kvargs should not
> > > have any ambiguity regarding how they should do so. Only librte_kvargs
> > > is available.
> > > 
> > Ok, would you also council others developing dpdk apps to write their own
> > parsing routines when what they needed was trivial for the existing library?
> > You are people too :)
> > 
> > > > 2) Lightweight function  seems like something that can be integrated with
> > > > librte_kvargs.  Looking at it, what may I ask in librte_kvargs is insufficiently
> > > > non-performant for your needs, specifically?  We talked about the heap
> > > > allocation above, is there something else? The string duplication perhaps?
> > > > 
> > > > 
> > > 
> > > Mostly the way to use it.
> > > The filter strings are
> > > bus=value,.../class=value,...
> > > 
> > > where either bus= list or class= list can be omitted, but at least one
> > > must appear.
> > > 
> > Ok, so whats the problem with using librte_kvargs for that?  Is it that the list
> > that acts as the value to the key isn't parsed out into its own set of tokens?
> > That seems entirely addressable.
> > 
> > > I want to read a single kvarg. I do not want to parse the whole string.
> > > the '/' signifies the end of the current layer.
> > > 
> > This makes it seem like librte_kvargs can handle this as a trivial case of its
> > functionality.
> > 
> > > librte_kvargs does not care about those points. I cannot ask it to only
> > > read either bus or class, as it would then throw an error for all the
> > > other keys (which the EAL has necessarily no knowledge of).
> > > 
> > But you can ask it to read both, and within your libraries logic make the
> > determination as to the validitiy of receiving both.  Alternatively you can
> > modify the valid_keys check in kvargs to be a regex that matches on either bus
> > or class, or accept an ignore parameter for keys that may appear but should be
> > ignored in the light of other keys.  Theres lots of options here.
> > 
> 
> No, I will not be adding regex parsing to express a set of acceptable
> token :)
> 
Thats fine, Just making suggestions as to how to add the functionality you need
to the command line parsing library.  I'm not sure what you're opposision is,
but there are lots of other ways to skin this cat.


> > > So I would need to:
> > > 
> > >   * Add a custom storage scheme
> > >   * Add a custom parsing mode stopping at the first kvarg
> > >   * Add an edge-case to ignore the '/', so as not to throw off the rest
> > >     of the parsing (least it be considered part of the previous kvarg
> > >     value field).
> > > 
> > > Seeing this, does adding those really specifics functionality help
> > > librte_kvargs to be more useful and usable? I do not think so.
> > > 
> > I think you're overcomplicating this.
> > How about enhancing librte_kvargs to make parsing configurable
> > such that invalid keys get ignored rather than generate errors?
> > 
> 
> Invalid keys can already be ignored, it's not an issue.
> 
Ok, that eliminates one problem and seems like it makes even more sense to use
the standard parsing library.

> > > It would only serve to disrupt the library for a marginal use-case, with
> > > the introduction of edge-cases that will blur the specs of the lib's
> > > API, making it harder to avoid subtle bugs.
> > > 
> > What do you mean "disrupt the library"?  What is its purpose of a library if not
> > to do the jobs we want it to?  If everyone created their own routine to do
> > something that a library could do with some modifications, we'd be left with
> > 1000 versions of the same routine.  If the existing library does 99% of what you
> > want it to, lets ennumerate what the missing %1 is and make the change, not
> > throw the baby out with the bathwater.
> > 
> > > Only way to do so sanely would be to add rte_parse_kv as part of
> > > librte_kvargs, as is. But then the whole thing does not make sense IMO:
> > > no one would care to use it, the maintainance effort is the same, the
> > > likelyhood of bugs as well (but in the process we would disrupt the
> > > distribution of librte_kvargs by moving it within the EAL).
> > > 
> > > I see no benefit to either solution.
> > > 
> > Again, thats an overcomplication.  As I read it, you have a need to interrogate
> > a key/value string, whos contents may contain invalid keys (for your parsing
> > purposes), and whos values may themselves be lists, correct?  If so, I don't see
> > the problem in enhancing libkvargs to:
> > 
> > 1) Allow for the parsing routine to ignore invalid keys (or even ignore specific
> > invalid keys, and trigger on any unknown keys)
> > 
> > 2) Allows for the subparsing of lists into their own set of tokens.
> > 
> 
> What I would do if I wanted to use librte_kvargs, would be to duplicate
> the input text, mangle it to respect the intended format, and feed it to
> librte_kvargs for parsing. Then I would iterate over the pairs, and stop
> on the two I'm concerned about.
> 
> What I dislike here:
> 
>   * I actually do the bulk of the parsing by hand (recognizing first a
>     valid input, modifying it to respect the lib, and after iterating on
>     the list of pairs and strcmp-ing the ones I want). This is approximately
>     as complicated as the current helper function.
> 
Or, just extend the kvargs string format to allow for nested lits (which I think
is the main sticking point here) and adjust your input format accordingly.

>   * I have to move librte_kvargs in the EAL.
> 
Why?  Theres no reason you can't make eal dependent on kvargs, I see no circular
dependency there.

> > > > > If that's really an issue, I'm better off simply removing rte_parse_kv
> > > > > and writing the parsing by hand within my function. This would be ugly
> > > > > and tedious, but less than moving librte_kvargs within EAL and changing
> > > > > it to my needs.
> > > > I don't think thats necessecary, I just think if you can ennumerate the items
> > > > that are non-performant for your needs we can make some changes to librte_kvargs
> > > > to optimize around them, or offer parsing options to avoid them, and in the
> > > > process avoid some code duplication
> > > > 
> > > 
> > > I think it makes sense to have specialized functions for specialized
> > > use-cases, and forcing the code to be generic and work with all of them
> > > will make it more complicated.
> > > 
> > This isn't specialized, its trivial.  Its just a trivial case that libkvargs
> > isn't built to handle at the moment.  Lets get it there.
> > 
> 
> My use-case is trivial. Putting it within the librte_kvargs makes it
> more complicated to write. I hate this.
> 
If librte_kvargs can't handle a trivial use case, then kvargs needs to be
improved.

> > > The genericity would only be worth it if people actually needed to parse
> > > the device strings the same way I do. No one has any business doing so.
> > > This genericity adds complexity and issues, without even being useful in
> > > the first place.
> > > 
> > I really think you're trying to take a short cut here where none is needed, and
> > I'm sorry, but I can't support that.
> 
> There are countably infinite solutions, we will probably reach one (and
> it might even do what we want).
I'm not really sure what you mean by this, but the bottom line for me is that
there is no reason to invent a new wheel when you have a library who purpose is
to do the kind of things you want.

Neil
  

Patch

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index cd071442f..4032f1bd8 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -13,10 +13,48 @@ 
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_debug.h>
+#include <rte_errno.h>
 #include <rte_log.h>
 
 #include "eal_private.h"
 
+/* EAL-private function. */
+int
+rte_parse_kv(const char *str, struct rte_kvarg *kv)
+{
+	const char *equal;
+	const char *end;
+
+	if (str == NULL || str[0] == '\0')
+		return 1;
+	equal = strchr(str, '=');
+	if (equal == NULL) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+	end = strchr(equal + 1, ',');
+	end = end ? end : strchr(equal + 1, '/');
+	end = end ? end : strchr(equal + 1, '\0');
+	if (end == NULL) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+	if (kv == NULL)
+		return 0;
+	snprintf(kv->data, sizeof(kv->data), "%s", str);
+	kv->key = &kv->data[0];
+	strchr(kv->data, end[0])[0] = '\0';
+	if (strchr(kv->data, '=')) {
+		kv->value = strchr(kv->data, '=') + 1;
+		strchr(kv->data, '=')[0] = '\0';
+	}
+	if (end[0] == '\0')
+		kv->next = NULL;
+	else
+		kv->next = end + 1;
+	return 0;
+}
+
 static int cmp_detached_dev_name(const struct rte_device *dev,
 	const void *_name)
 {
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 0b2877000..d2774a3ad 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -205,4 +205,38 @@  struct rte_bus *rte_bus_find_by_device_name(const char *str);
 
 int rte_mp_channel_init(void);
 
+/*
+ * Lightweight kvarg parsing library.
+ */
+
+#define RTE_MAX_KVARG_LEN 64
+
+/**
+ * Kvarg representation.
+ */
+struct rte_kvarg {
+	char *key; /**< points the key in the data. */
+	char *value; /**< points the value in the data. */
+	const char *next; /**< next token to parse, if any. */
+	char data[RTE_MAX_KVARG_LEN + 1]; /**< local copy of key and value. */
+};
+
+/**
+ * Parse one kvarg.
+ *
+ * The key-value pair must be shorter than the rte_kvarg data size.
+ *
+ * @param[in] str
+ *   text to parse.
+ *
+ * @param[out] kv
+ *   kvarg structure to fill.
+ *
+ * @return
+ *   0 if parsing succeeded.
+ *   >0 if there was nothing to parse.
+ *   <0 on error, rte_errno is set.
+ */
+int rte_parse_kv(const char *str, struct rte_kvarg *kv);
+
 #endif /* _EAL_PRIVATE_H_ */