DPDK usage discussions
 help / color / mirror / Atom feed
* Generic flow string parser
@ 2023-04-26  4:46 Cliff Burdick
  2023-04-26  5:47 ` David Marchand
  0 siblings, 1 reply; 14+ messages in thread
From: Cliff Burdick @ 2023-04-26  4:46 UTC (permalink / raw)
  To: users

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

Does anyone know if a generic parser for flow strings exists anywhere? The
one inside of testpmd is ideal, but unfortunately it's self-contained and
not distributed as part of a normal DPDK install. This seems like something
that is likely reinvented over and over and it would be useful if there was
a single API to take in strings and generate flows.

[-- Attachment #2: Type: text/html, Size: 380 bytes --]

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

* Re: Generic flow string parser
  2023-04-26  4:46 Generic flow string parser Cliff Burdick
@ 2023-04-26  5:47 ` David Marchand
  2023-04-27  8:37   ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: David Marchand @ 2023-04-26  5:47 UTC (permalink / raw)
  To: Cliff Burdick; +Cc: users, Thomas Monjalon, Ori Kam

On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com> wrote:
>
> Does anyone know if a generic parser for flow strings exists anywhere? The one inside of testpmd is ideal, but unfortunately it's self-contained and not distributed as part of a normal DPDK install. This seems like something that is likely reinvented over and over and it would be useful if there was a single API to take in strings and generate flows.

I heard this same question in the past, but I don't remember the answer.
Copying Thomas and Ori who might know.


-- 
David Marchand


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

* Re: Generic flow string parser
  2023-04-26  5:47 ` David Marchand
@ 2023-04-27  8:37   ` Thomas Monjalon
  2023-04-27 13:19     ` Cliff Burdick
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2023-04-27  8:37 UTC (permalink / raw)
  To: Cliff Burdick, David Marchand; +Cc: users, Ori Kam

26/04/2023 07:47, David Marchand:
> On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com> wrote:
> >
> > Does anyone know if a generic parser for flow strings exists anywhere? The one inside of testpmd is ideal, but unfortunately it's self-contained and not distributed as part of a normal DPDK install. This seems like something that is likely reinvented over and over and it would be useful if there was a single API to take in strings and generate flows.
> 
> I heard this same question in the past, but I don't remember the answer.
> Copying Thomas and Ori who might know.

I'm not sure how the testpmd code could help another application.
And in general, if your application has a CLI,
you need to integrate the flow commands in a broader context.



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

* Re: Generic flow string parser
  2023-04-27  8:37   ` Thomas Monjalon
@ 2023-04-27 13:19     ` Cliff Burdick
  2023-04-28 17:36       ` Tom Barbette
  0 siblings, 1 reply; 14+ messages in thread
From: Cliff Burdick @ 2023-04-27 13:19 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: David Marchand, users, Ori Kam

[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]

Hi Thomas, testpmd has a 12,000 line parser just for taking in strings and
converting it to flow rules. This is obviously useful for testing flows,
but it also is an interface for any type of flow imaginable since this is
where they're tested.

Now imagine you're developing an application that allows the user to
specify custom flows in a config. Your only option is to make your own flow
string input (json, etc) and convert that to to the flow spec. This is
reinventing almost all of what testpmd already does, and it's extremely
error-prone. I think it would be very useful to have this as an API call
rather than a user constructing each flow by hand so that all these other
applications can benefit and not be worries about bugs during conversions.



On Thu, Apr 27, 2023, 01:37 Thomas Monjalon <thomas@monjalon.net> wrote:

> 26/04/2023 07:47, David Marchand:
> > On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com>
> wrote:
> > >
> > > Does anyone know if a generic parser for flow strings exists anywhere?
> The one inside of testpmd is ideal, but unfortunately it's self-contained
> and not distributed as part of a normal DPDK install. This seems like
> something that is likely reinvented over and over and it would be useful if
> there was a single API to take in strings and generate flows.
> >
> > I heard this same question in the past, but I don't remember the answer.
> > Copying Thomas and Ori who might know.
>
> I'm not sure how the testpmd code could help another application.
> And in general, if your application has a CLI,
> you need to integrate the flow commands in a broader context.
>
>
>

[-- Attachment #2: Type: text/html, Size: 2151 bytes --]

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

* Re: Generic flow string parser
  2023-04-27 13:19     ` Cliff Burdick
@ 2023-04-28 17:36       ` Tom Barbette
  2023-04-28 18:09         ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Tom Barbette @ 2023-04-28 17:36 UTC (permalink / raw)
  To: Cliff Burdick, Thomas Monjalon; +Cc: David Marchand, users, Ori Kam

[-- Attachment #1: Type: text/plain, Size: 2596 bytes --]

Hi all!

I totally agree with this.

In FastClick we link against a copy of the test-pmd source code to call the parser externally. We just have to patch a bit some files (see https://github.com/tbarbette/fastclick/blob/main/userlevel/rte_parse.mk, and used here : https://github.com/tbarbette/fastclick/blob/main/lib/flowruleparser.cc). It actually worked fairly well until a structure named "template" appeared, which is a registered keyword in C++, and prevent compilation now even under extern "C". This can be patched too but did not find the time yet.

So a clean solution would be more than nice. It's not only the 12K lines of codes, it's also the "testpmd syntax" which is known, and appears in a lot of examples here and there.

Given the relatively easy (but hacky) integration we have, a clean library wouldn't probably be very difficult.


Tom

Le 27/04/23 à 15:19, Cliff Burdick a écrit :
Hi Thomas, testpmd has a 12,000 line parser just for taking in strings and converting it to flow rules. This is obviously useful for testing flows, but it also is an interface for any type of flow imaginable since this is where they're tested.

Now imagine you're developing an application that allows the user to specify custom flows in a config. Your only option is to make your own flow string input (json, etc) and convert that to to the flow spec. This is reinventing almost all of what testpmd already does, and it's extremely error-prone. I think it would be very useful to have this as an API call rather than a user constructing each flow by hand so that all these other applications can benefit and not be worries about bugs during conversions.



On Thu, Apr 27, 2023, 01:37 Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
26/04/2023 07:47, David Marchand:
> On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com<mailto:shaklee3@gmail.com>> wrote:
> >
> > Does anyone know if a generic parser for flow strings exists anywhere? The one inside of testpmd is ideal, but unfortunately it's self-contained and not distributed as part of a normal DPDK install. This seems like something that is likely reinvented over and over and it would be useful if there was a single API to take in strings and generate flows.
>
> I heard this same question in the past, but I don't remember the answer.
> Copying Thomas and Ori who might know.

I'm not sure how the testpmd code could help another application.
And in general, if your application has a CLI,
you need to integrate the flow commands in a broader context.




[-- Attachment #2: Type: text/html, Size: 4281 bytes --]

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

* Re: Generic flow string parser
  2023-04-28 17:36       ` Tom Barbette
@ 2023-04-28 18:09         ` Stephen Hemminger
  2023-04-28 19:13           ` Cliff Burdick
  0 siblings, 1 reply; 14+ messages in thread
From: Stephen Hemminger @ 2023-04-28 18:09 UTC (permalink / raw)
  To: Tom Barbette
  Cc: Cliff Burdick, Thomas Monjalon, David Marchand, users, Ori Kam

On Fri, 28 Apr 2023 17:36:51 +0000
Tom Barbette <tom.barbette@uclouvain.be> wrote:

> Hi all!
> 
> I totally agree with this.
> 
> In FastClick we link against a copy of the test-pmd source code to call the parser externally. We just have to patch a bit some files (see https://github.com/tbarbette/fastclick/blob/main/userlevel/rte_parse.mk, and used here : https://github.com/tbarbette/fastclick/blob/main/lib/flowruleparser.cc). It actually worked fairly well until a structure named "template" appeared, which is a registered keyword in C++, and prevent compilation now even under extern "C". This can be patched too but did not find the time yet.
> 
> So a clean solution would be more than nice. It's not only the 12K lines of codes, it's also the "testpmd syntax" which is known, and appears in a lot of examples here and there.
> 
> Given the relatively easy (but hacky) integration we have, a clean library wouldn't probably be very difficult.
> 
> 
> Tom
> 
> Le 27/04/23 à 15:19, Cliff Burdick a écrit :
> Hi Thomas, testpmd has a 12,000 line parser just for taking in strings and converting it to flow rules. This is obviously useful for testing flows, but it also is an interface for any type of flow imaginable since this is where they're tested.
> 
> Now imagine you're developing an application that allows the user to specify custom flows in a config. Your only option is to make your own flow string input (json, etc) and convert that to to the flow spec. This is reinventing almost all of what testpmd already does, and it's extremely error-prone. I think it would be very useful to have this as an API call rather than a user constructing each flow by hand so that all these other applications can benefit and not be worries about bugs during conversions.
> 
> 
> 
> On Thu, Apr 27, 2023, 01:37 Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
> 26/04/2023 07:47, David Marchand:
> > On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com<mailto:shaklee3@gmail.com>> wrote:  
> > >
> > > Does anyone know if a generic parser for flow strings exists anywhere? The one inside of testpmd is ideal, but unfortunately it's self-contained and not distributed as part of a normal DPDK install. This seems like something that is likely reinvented over and over and it would be useful if there was a single API to take in strings and generate flows.  
> >
> > I heard this same question in the past, but I don't remember the answer.
> > Copying Thomas and Ori who might know.  
> 
> I'm not sure how the testpmd code could help another application.
> And in general, if your application has a CLI,
> you need to integrate the flow commands in a broader context.

Exposing the parser for use, would require some renaming of functions, documentation
and a test suite. The testing would be the hardest part.

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

* Re: Generic flow string parser
  2023-04-28 18:09         ` Stephen Hemminger
@ 2023-04-28 19:13           ` Cliff Burdick
  2023-04-29  0:04             ` Stephen Hemminger
  0 siblings, 1 reply; 14+ messages in thread
From: Cliff Burdick @ 2023-04-28 19:13 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Tom Barbette, Thomas Monjalon, David Marchand, users, Ori Kam

[-- Attachment #1: Type: text/plain, Size: 4135 bytes --]

Hi Stephen, it would definitely not be worthwhile to repeat everything
that's already tested with testpmd. I was thinking that given that there
already is a "flow_parse" function that does almost everything needed,
something like that could be exposed. If we think of the testpmd flow
string as a sort of "IR" for string flow specification, that would allow
others to implement higher-level transform of a schema like JSON or YAML
into the testpmd language. Due to the complexity of testpmd and how it's
the source of true for testing flows, I think it's too great of an ask to
have testpmd support a new type of parsing. My only suggestion would be to
take what already exists and expose it in a public API that is included in
a DPDK install.

If you look at the "flow_classify" example in DPDK you can already see that
for that application someone had to write another flow text parser for a
format they made up. Instead, that example could be converted over to this
other API as well.

On Fri, Apr 28, 2023 at 11:09 AM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri, 28 Apr 2023 17:36:51 +0000
> Tom Barbette <tom.barbette@uclouvain.be> wrote:
>
> > Hi all!
> >
> > I totally agree with this.
> >
> > In FastClick we link against a copy of the test-pmd source code to call
> the parser externally. We just have to patch a bit some files (see
> https://github.com/tbarbette/fastclick/blob/main/userlevel/rte_parse.mk,
> and used here :
> https://github.com/tbarbette/fastclick/blob/main/lib/flowruleparser.cc).
> It actually worked fairly well until a structure named "template" appeared,
> which is a registered keyword in C++, and prevent compilation now even
> under extern "C". This can be patched too but did not find the time yet.
> >
> > So a clean solution would be more than nice. It's not only the 12K lines
> of codes, it's also the "testpmd syntax" which is known, and appears in a
> lot of examples here and there.
> >
> > Given the relatively easy (but hacky) integration we have, a clean
> library wouldn't probably be very difficult.
> >
> >
> > Tom
> >
> > Le 27/04/23 à 15:19, Cliff Burdick a écrit :
> > Hi Thomas, testpmd has a 12,000 line parser just for taking in strings
> and converting it to flow rules. This is obviously useful for testing
> flows, but it also is an interface for any type of flow imaginable since
> this is where they're tested.
> >
> > Now imagine you're developing an application that allows the user to
> specify custom flows in a config. Your only option is to make your own flow
> string input (json, etc) and convert that to to the flow spec. This is
> reinventing almost all of what testpmd already does, and it's extremely
> error-prone. I think it would be very useful to have this as an API call
> rather than a user constructing each flow by hand so that all these other
> applications can benefit and not be worries about bugs during conversions.
> >
> >
> >
> > On Thu, Apr 27, 2023, 01:37 Thomas Monjalon <thomas@monjalon.net<mailto:
> thomas@monjalon.net>> wrote:
> > 26/04/2023 07:47, David Marchand:
> > > On Wed, Apr 26, 2023 at 6:47 AM Cliff Burdick <shaklee3@gmail.com
> <mailto:shaklee3@gmail.com>> wrote:
> > > >
> > > > Does anyone know if a generic parser for flow strings exists
> anywhere? The one inside of testpmd is ideal, but unfortunately it's
> self-contained and not distributed as part of a normal DPDK install. This
> seems like something that is likely reinvented over and over and it would
> be useful if there was a single API to take in strings and generate flows.
> > >
> > > I heard this same question in the past, but I don't remember the
> answer.
> > > Copying Thomas and Ori who might know.
> >
> > I'm not sure how the testpmd code could help another application.
> > And in general, if your application has a CLI,
> > you need to integrate the flow commands in a broader context.
>
> Exposing the parser for use, would require some renaming of functions,
> documentation
> and a test suite. The testing would be the hardest part.
>

[-- Attachment #2: Type: text/html, Size: 5224 bytes --]

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

* Re: Generic flow string parser
  2023-04-28 19:13           ` Cliff Burdick
@ 2023-04-29  0:04             ` Stephen Hemminger
  2023-04-29  0:08               ` Stephen Hemminger
  2023-04-29 14:23               ` Cliff Burdick
  0 siblings, 2 replies; 14+ messages in thread
From: Stephen Hemminger @ 2023-04-29  0:04 UTC (permalink / raw)
  To: Cliff Burdick
  Cc: Tom Barbette, Thomas Monjalon, David Marchand, users, Ori Kam

On Fri, 28 Apr 2023 12:13:26 -0700
Cliff Burdick <shaklee3@gmail.com> wrote:

> Hi Stephen, it would definitely not be worthwhile to repeat everything
> that's already tested with testpmd. I was thinking that given that there
> already is a "flow_parse" function that does almost everything needed,
> something like that could be exposed. If we think of the testpmd flow
> string as a sort of "IR" for string flow specification, that would allow
> others to implement higher-level transform of a schema like JSON or YAML
> into the testpmd language. Due to the complexity of testpmd and how it's
> the source of true for testing flows, I think it's too great of an ask to
> have testpmd support a new type of parsing. My only suggestion would be to
> take what already exists and expose it in a public API that is included in
> a DPDK install.
> 
> If you look at the "flow_classify" example in DPDK you can already see that
> for that application someone had to write another flow text parser for a
> format they made up. Instead, that example could be converted over to this
> other API as well.

Please don't top post.

The naming issue is that almost all libraries in DPDK start with rte_ prefix
and the testpmd functions do not.

The flow_classify example is pretty much abandonware at this point.
Code is not updated, other than build breakages.
Last time I looked at it noticed lots of code reinvention useless code,
and only supports IPv4. It really needs a rewrite.

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

* Re: Generic flow string parser
  2023-04-29  0:04             ` Stephen Hemminger
@ 2023-04-29  0:08               ` Stephen Hemminger
  2023-04-29 14:23               ` Cliff Burdick
  1 sibling, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2023-04-29  0:08 UTC (permalink / raw)
  To: Cliff Burdick
  Cc: Tom Barbette, Thomas Monjalon, David Marchand, users, Ori Kam

On Fri, 28 Apr 2023 17:04:46 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:

> On Fri, 28 Apr 2023 12:13:26 -0700
> Cliff Burdick <shaklee3@gmail.com> wrote:
> 
> > Hi Stephen, it would definitely not be worthwhile to repeat everything
> > that's already tested with testpmd. I was thinking that given that there
> > already is a "flow_parse" function that does almost everything needed,
> > something like that could be exposed. If we think of the testpmd flow
> > string as a sort of "IR" for string flow specification, that would allow
> > others to implement higher-level transform of a schema like JSON or YAML
> > into the testpmd language. Due to the complexity of testpmd and how it's
> > the source of true for testing flows, I think it's too great of an ask to
> > have testpmd support a new type of parsing. My only suggestion would be to
> > take what already exists and expose it in a public API that is included in
> > a DPDK install.
> > 
> > If you look at the "flow_classify" example in DPDK you can already see that
> > for that application someone had to write another flow text parser for a
> > format they made up. Instead, that example could be converted over to this
> > other API as well.  
> 
> Please don't top post.
> 
> The naming issue is that almost all libraries in DPDK start with rte_ prefix
> and the testpmd functions do not.
> 
> The flow_classify example is pretty much abandonware at this point.
> Code is not updated, other than build breakages.
> Last time I looked at it noticed lots of code reinvention useless code,
> and only supports IPv4. It really needs a rewrite.

Would rather the flow parser was rewritten as well. Doing open coded
parser is much more error prone and hard to extend versus writing the
parser in yacc/lex (ie bison/flex). 

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

* Re: Generic flow string parser
  2023-04-29  0:04             ` Stephen Hemminger
  2023-04-29  0:08               ` Stephen Hemminger
@ 2023-04-29 14:23               ` Cliff Burdick
  2023-04-29 21:39                 ` Thomas Monjalon
  1 sibling, 1 reply; 14+ messages in thread
From: Cliff Burdick @ 2023-04-29 14:23 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Tom Barbette, Thomas Monjalon, David Marchand, users, Ori Kam

[-- Attachment #1: Type: text/plain, Size: 2701 bytes --]

> Would rather the flow parser was rewritten as well. Doing open coded
> parser is much more error prone and hard to extend versus writing the
> parser in yacc/lex (ie bison/flex).

I agree, and that's kind of why the original suggestion of using testpmd
came from. Writing a new parser is obviously the better choice and would
have been great if testpmd started that way, but a significant amount of
time was invested in that method. Since it works and is tested, it didn't
seem like a bad request to build off that and bring that code into an rte_
API. I'd imagine building a proper parser would not just require the parser
piece, but also making sure all the tests now use that, and also the legacy
testpmd was converted. It seemed unlikely all of this could be done in a
reasonable amount of time and a lot of input from many people. Given the
amount of debugging I (and others) have spent on figuring on why a flow
spec didn't work properly, this could be a huge timesaver for new projects
like Tom mentioned.

On Fri, Apr 28, 2023 at 5:04 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri, 28 Apr 2023 12:13:26 -0700
> Cliff Burdick <shaklee3@gmail.com> wrote:
>
> > Hi Stephen, it would definitely not be worthwhile to repeat everything
> > that's already tested with testpmd. I was thinking that given that there
> > already is a "flow_parse" function that does almost everything needed,
> > something like that could be exposed. If we think of the testpmd flow
> > string as a sort of "IR" for string flow specification, that would allow
> > others to implement higher-level transform of a schema like JSON or YAML
> > into the testpmd language. Due to the complexity of testpmd and how it's
> > the source of true for testing flows, I think it's too great of an ask to
> > have testpmd support a new type of parsing. My only suggestion would be
> to
> > take what already exists and expose it in a public API that is included
> in
> > a DPDK install.
> >
> > If you look at the "flow_classify" example in DPDK you can already see
> that
> > for that application someone had to write another flow text parser for a
> > format they made up. Instead, that example could be converted over to
> this
> > other API as well.
>
> Please don't top post.
>
> The naming issue is that almost all libraries in DPDK start with rte_
> prefix
> and the testpmd functions do not.
>
> The flow_classify example is pretty much abandonware at this point.
> Code is not updated, other than build breakages.
> Last time I looked at it noticed lots of code reinvention useless code,
> and only supports IPv4. It really needs a rewrite.
>

[-- Attachment #2: Type: text/html, Size: 3227 bytes --]

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

* Re: Generic flow string parser
  2023-04-29 14:23               ` Cliff Burdick
@ 2023-04-29 21:39                 ` Thomas Monjalon
  2023-04-29 21:49                   ` Cliff Burdick
  2023-06-05 16:36                   ` kumaraparameshwaran rathinavel
  0 siblings, 2 replies; 14+ messages in thread
From: Thomas Monjalon @ 2023-04-29 21:39 UTC (permalink / raw)
  To: Cliff Burdick, Tom Barbette
  Cc: Stephen Hemminger, David Marchand, users, Ori Kam, dev

This thread is an API suggestion, it should be discussed in
the developer mailing list (did the Cc here).

29/04/2023 16:23, Cliff Burdick:
> > Would rather the flow parser was rewritten as well. Doing open coded
> > parser is much more error prone and hard to extend versus writing the
> > parser in yacc/lex (ie bison/flex).
> 
> I agree, and that's kind of why the original suggestion of using testpmd
> came from. Writing a new parser is obviously the better choice and would
> have been great if testpmd started that way, but a significant amount of
> time was invested in that method. Since it works and is tested, it didn't
> seem like a bad request to build off that and bring that code into an rte_
> API. I'd imagine building a proper parser would not just require the parser
> piece, but also making sure all the tests now use that, and also the legacy
> testpmd was converted. It seemed unlikely all of this could be done in a
> reasonable amount of time and a lot of input from many people. Given the
> amount of debugging I (and others) have spent on figuring on why a flow
> spec didn't work properly, this could be a huge timesaver for new projects
> like Tom mentioned.
> 
> On Fri, Apr 28, 2023 at 5:04 PM Stephen Hemminger <
> stephen@networkplumber.org> wrote:
> 
> > On Fri, 28 Apr 2023 12:13:26 -0700
> > Cliff Burdick <shaklee3@gmail.com> wrote:
> >
> > > Hi Stephen, it would definitely not be worthwhile to repeat everything
> > > that's already tested with testpmd. I was thinking that given that there
> > > already is a "flow_parse" function that does almost everything needed,
> > > something like that could be exposed. If we think of the testpmd flow
> > > string as a sort of "IR" for string flow specification, that would allow
> > > others to implement higher-level transform of a schema like JSON or YAML
> > > into the testpmd language. Due to the complexity of testpmd and how it's
> > > the source of true for testing flows, I think it's too great of an ask to
> > > have testpmd support a new type of parsing. My only suggestion would be
> > > to take what already exists and expose it in a public API that is included
> > > in a DPDK install.

So the only things we need are 2 functions, if I understand well:

int rte_flow_to_text(const struct rte_flow*);
struct rte_flow *rte_flow_from_text(const char *);

Here I assume the output of rte_flow_from_text() would be a created flow,
meaning it calls rte_flow_create() under the hood.
Is it what you wish?
Or should it fill port ID, attributes, patterns and actions?



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

* Re: Generic flow string parser
  2023-04-29 21:39                 ` Thomas Monjalon
@ 2023-04-29 21:49                   ` Cliff Burdick
  2023-05-26 22:35                     ` Cliff Burdick
  2023-06-05 16:36                   ` kumaraparameshwaran rathinavel
  1 sibling, 1 reply; 14+ messages in thread
From: Cliff Burdick @ 2023-04-29 21:49 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Tom Barbette, Stephen Hemminger, David Marchand, users, Ori Kam, dev

[-- Attachment #1: Type: text/plain, Size: 4321 bytes --]

> So the only things we need are 2 functions, if I understand well:
>
> int rte_flow_to_text(const struct rte_flow*);
> struct rte_flow *rte_flow_from_text(const char *);
>
> Here I assume the output of rte_flow_from_text() would be a created flow,
> meaning it calls rte_flow_create() under the hood.
> Is it what you wish?
> Or should it fill port ID, attributes, patterns and actions?

I think it should follow closely with what "flow_parse" already does:
https://github.com/DPDK/dpdk/blob/d03446724972d2a1bb645ce7f3e64f5ef0203d61/app/test-pmd/cmdline_flow.c#L11304

Namely, just do the part of populating attributes, patterns, and actions
from a string. It's then up to the user if they want to create, validate,
or do something else with it (like see how it populated the structures).
The flow_parse function appears to take an opaque pointer that's specific
to a structure inside of cmdline_flow.c and assign the attributes, actions,
and patterns to members of that result struct. I don't know the reason for
this, but when calling the function the user doesn't know how many patterns
or actions their string will generate. They would either need to pass in
structures that are allocated larger than needed, or have a separate API
that returns how many actions and patterns are needed for a string, then
they need to allocate the correct size themselves. I'm assuming it's not
ideal to have the library itself do dynamic memory allocations for the
correct size.

On Sat, Apr 29, 2023 at 2:40 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> This thread is an API suggestion, it should be discussed in
> the developer mailing list (did the Cc here).
>
> 29/04/2023 16:23, Cliff Burdick:
> > > Would rather the flow parser was rewritten as well. Doing open coded
> > > parser is much more error prone and hard to extend versus writing the
> > > parser in yacc/lex (ie bison/flex).
> >
> > I agree, and that's kind of why the original suggestion of using testpmd
> > came from. Writing a new parser is obviously the better choice and would
> > have been great if testpmd started that way, but a significant amount of
> > time was invested in that method. Since it works and is tested, it didn't
> > seem like a bad request to build off that and bring that code into an
> rte_
> > API. I'd imagine building a proper parser would not just require the
> parser
> > piece, but also making sure all the tests now use that, and also the
> legacy
> > testpmd was converted. It seemed unlikely all of this could be done in a
> > reasonable amount of time and a lot of input from many people. Given the
> > amount of debugging I (and others) have spent on figuring on why a flow
> > spec didn't work properly, this could be a huge timesaver for new
> projects
> > like Tom mentioned.
> >
> > On Fri, Apr 28, 2023 at 5:04 PM Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> >
> > > On Fri, 28 Apr 2023 12:13:26 -0700
> > > Cliff Burdick <shaklee3@gmail.com> wrote:
> > >
> > > > Hi Stephen, it would definitely not be worthwhile to repeat
> everything
> > > > that's already tested with testpmd. I was thinking that given that
> there
> > > > already is a "flow_parse" function that does almost everything
> needed,
> > > > something like that could be exposed. If we think of the testpmd flow
> > > > string as a sort of "IR" for string flow specification, that would
> allow
> > > > others to implement higher-level transform of a schema like JSON or
> YAML
> > > > into the testpmd language. Due to the complexity of testpmd and how
> it's
> > > > the source of true for testing flows, I think it's too great of an
> ask to
> > > > have testpmd support a new type of parsing. My only suggestion would
> be
> > > > to take what already exists and expose it in a public API that is
> included
> > > > in a DPDK install.
>
> So the only things we need are 2 functions, if I understand well:
>
> int rte_flow_to_text(const struct rte_flow*);
> struct rte_flow *rte_flow_from_text(const char *);
>
> Here I assume the output of rte_flow_from_text() would be a created flow,
> meaning it calls rte_flow_create() under the hood.
> Is it what you wish?
> Or should it fill port ID, attributes, patterns and actions?
>
>
>

[-- Attachment #2: Type: text/html, Size: 5266 bytes --]

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

* Re: Generic flow string parser
  2023-04-29 21:49                   ` Cliff Burdick
@ 2023-05-26 22:35                     ` Cliff Burdick
  0 siblings, 0 replies; 14+ messages in thread
From: Cliff Burdick @ 2023-05-26 22:35 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Tom Barbette, Stephen Hemminger, David Marchand, users, Ori Kam, dev

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On Sat, Apr 29, 2023 at 2:49 PM Cliff Burdick <shaklee3@gmail.com> wrote:

> > So the only things we need are 2 functions, if I understand well:
> >
> > int rte_flow_to_text(const struct rte_flow*);
> > struct rte_flow *rte_flow_from_text(const char *);
> >
> > Here I assume the output of rte_flow_from_text() would be a created flow,
> > meaning it calls rte_flow_create() under the hood.
> > Is it what you wish?
> > Or should it fill port ID, attributes, patterns and actions?
> >
> > I think it should follow closely with what "flow_parse" already does:
> https://github.com/DPDK/dpdk/blob/d03446724972d2a1bb645ce7f3e64f5ef0203d61/app/test-pmd/cmdline_flow.c#L11304
> >
> > Namely, just do the part of populating attributes, patterns, and actions
> from a string. It's then up to the user if they want to create, validate,
> or do something else with it (like see how it populate> d the structures).
> The flow_parse function appears to take an opaque pointer that's specific
> to a structure inside of cmdline_flow.c and assign the attributes, actions,
> and patterns to members of that result struct. I don't know the reason for
> this, but when >calling the function the user doesn't know how many
> patterns or actions their string will generate. They would either need to
> pass in structures that are allocated larger than needed, or have a
> separate API that returns how many actions and patterns are needed for a
> string, then they need to allocate the correct size themselves. I'm
> assuming it's not ideal to have the library itself do dynamic memory
> allocations for the correct size.
>
>>
>> Tom, for what it's worth I'm on a quest to get this to work since I think
it's necessary. I'm just hacking through it like you did and I ran into the
same "template" keyword error. It's probably worthwhile to fix that
anyways. I'm maintaining a set of patches as I go. The general strategy has
been to remove testpmd's main function, compile it as a library, and link
against that.

[-- Attachment #2: Type: text/html, Size: 2769 bytes --]

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

* Re: Generic flow string parser
  2023-04-29 21:39                 ` Thomas Monjalon
  2023-04-29 21:49                   ` Cliff Burdick
@ 2023-06-05 16:36                   ` kumaraparameshwaran rathinavel
  1 sibling, 0 replies; 14+ messages in thread
From: kumaraparameshwaran rathinavel @ 2023-06-05 16:36 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: Cliff Burdick, Tom Barbette, Stephen Hemminger, David Marchand,
	users, Ori Kam, dev

[-- Attachment #1: Type: text/plain, Size: 3040 bytes --]

On Sun, Apr 30, 2023 at 3:10 AM Thomas Monjalon <thomas@monjalon.net> wrote:

> This thread is an API suggestion, it should be discussed in
> the developer mailing list (did the Cc here).
>
> 29/04/2023 16:23, Cliff Burdick:
> > > Would rather the flow parser was rewritten as well. Doing open coded
> > > parser is much more error prone and hard to extend versus writing the
> > > parser in yacc/lex (ie bison/flex).
> >
> > I agree, and that's kind of why the original suggestion of using testpmd
> > came from. Writing a new parser is obviously the better choice and would
> > have been great if testpmd started that way, but a significant amount of
> > time was invested in that method. Since it works and is tested, it didn't
> > seem like a bad request to build off that and bring that code into an
> rte_
> > API. I'd imagine building a proper parser would not just require the
> parser
> > piece, but also making sure all the tests now use that, and also the
> legacy
> > testpmd was converted. It seemed unlikely all of this could be done in a
> > reasonable amount of time and a lot of input from many people. Given the
> > amount of debugging I (and others) have spent on figuring on why a flow
> > spec didn't work properly, this could be a huge timesaver for new
> projects
> > like Tom mentioned.
> >
> > On Fri, Apr 28, 2023 at 5:04 PM Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> >
> > > On Fri, 28 Apr 2023 12:13:26 -0700
> > > Cliff Burdick <shaklee3@gmail.com> wrote:
> > >
> > > > Hi Stephen, it would definitely not be worthwhile to repeat
> everything
> > > > that's already tested with testpmd. I was thinking that given that
> there
> > > > already is a "flow_parse" function that does almost everything
> needed,
> > > > something like that could be exposed. If we think of the testpmd flow
> > > > string as a sort of "IR" for string flow specification, that would
> allow
> > > > others to implement higher-level transform of a schema like JSON or
> YAML
> > > > into the testpmd language. Due to the complexity of testpmd and how
> it's
> > > > the source of true for testing flows, I think it's too great of an
> ask to
> > > > have testpmd support a new type of parsing. My only suggestion would
> be
> > > > to take what already exists and expose it in a public API that is
> included
> > > > in a DPDK install.
>
> So the only things we need are 2 functions, if I understand well:
>
> int rte_flow_to_text(const struct rte_flow*);
> struct rte_flow *rte_flow_from_text(const char *);
>
> Here I assume the output of rte_flow_from_text() would be a created flow,
> meaning it calls rte_flow_create() under the hood.
> Is it what you wish?
> Or should it fill port ID, attributes, patterns and actions?
>
>
>> +1 It would be definitely useful to have a generic parser which could
>> re-use the test-pmd parser code as it has already done the heavy lifting. I
>> would be happy to contribute/help to get this going.
>>
>

[-- Attachment #2: Type: text/html, Size: 3910 bytes --]

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

end of thread, other threads:[~2023-06-05 16:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-26  4:46 Generic flow string parser Cliff Burdick
2023-04-26  5:47 ` David Marchand
2023-04-27  8:37   ` Thomas Monjalon
2023-04-27 13:19     ` Cliff Burdick
2023-04-28 17:36       ` Tom Barbette
2023-04-28 18:09         ` Stephen Hemminger
2023-04-28 19:13           ` Cliff Burdick
2023-04-29  0:04             ` Stephen Hemminger
2023-04-29  0:08               ` Stephen Hemminger
2023-04-29 14:23               ` Cliff Burdick
2023-04-29 21:39                 ` Thomas Monjalon
2023-04-29 21:49                   ` Cliff Burdick
2023-05-26 22:35                     ` Cliff Burdick
2023-06-05 16:36                   ` kumaraparameshwaran rathinavel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).