patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] examples/pipeline: fix include path for rte_log.h
@ 2024-02-13 14:52 Cristian Dumitrescu
  2024-02-13 16:18 ` David Marchand
  2024-02-13 17:38 ` [PATCH V2] " Cristian Dumitrescu
  0 siblings, 2 replies; 14+ messages in thread
From: Cristian Dumitrescu @ 2024-02-13 14:52 UTC (permalink / raw)
  To: dev; +Cc: stable

When rte_log.h was moved to a new directory, the include path was not
updated for the generated C code produced by the pipeline library,
which results in build failure for this code.

Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
Cc: stable@dpdk.org

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 examples/pipeline/cli.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
index 2ae6cc579f..afb143c01f 100644
--- a/examples/pipeline/cli.c
+++ b/examples/pipeline/cli.c
@@ -714,6 +714,7 @@ cmd_pipeline_libbuild(char **tokens,
 		 "-I %s/lib/eal/include "
 		 "-I %s/lib/eal/x86/include "
 		 "-I %s/lib/eal/include/generic "
+		 "-I %s/lib/log "
 		 "-I %s/lib/meter "
 		 "-I %s/lib/port "
 		 "-I %s/lib/table "
@@ -738,6 +739,7 @@ cmd_pipeline_libbuild(char **tokens,
 		 install_dir,
 		 install_dir,
 		 install_dir,
+		 install_dir,
 		 log_file,
 		 obj_file,
 		 lib_file,
-- 
2.34.1


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

* Re: [PATCH] examples/pipeline: fix include path for rte_log.h
  2024-02-13 14:52 [PATCH] examples/pipeline: fix include path for rte_log.h Cristian Dumitrescu
@ 2024-02-13 16:18 ` David Marchand
  2024-02-13 17:22   ` Dumitrescu, Cristian
  2024-02-13 17:38 ` [PATCH V2] " Cristian Dumitrescu
  1 sibling, 1 reply; 14+ messages in thread
From: David Marchand @ 2024-02-13 16:18 UTC (permalink / raw)
  To: Cristian Dumitrescu; +Cc: dev, stable

On Tue, Feb 13, 2024 at 3:52 PM Cristian Dumitrescu
<cristian.dumitrescu@intel.com> wrote:
>
> When rte_log.h was moved to a new directory, the include path was not
> updated for the generated C code produced by the pipeline library,
> which results in build failure for this code.
>
> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
> Cc: stable@dpdk.org
>
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---
>  examples/pipeline/cli.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
> index 2ae6cc579f..afb143c01f 100644
> --- a/examples/pipeline/cli.c
> +++ b/examples/pipeline/cli.c
> @@ -714,6 +714,7 @@ cmd_pipeline_libbuild(char **tokens,
>                  "-I %s/lib/eal/include "
>                  "-I %s/lib/eal/x86/include "
>                  "-I %s/lib/eal/include/generic "
> +                "-I %s/lib/log "
>                  "-I %s/lib/meter "
>                  "-I %s/lib/port "
>                  "-I %s/lib/table "
> @@ -738,6 +739,7 @@ cmd_pipeline_libbuild(char **tokens,
>                  install_dir,
>                  install_dir,
>                  install_dir,
> +                install_dir,
>                  log_file,
>                  obj_file,
>                  lib_file,

Wrt $Subject, copy/paste code in drivers/net/softnic is broken too.

Some comments on this code:
- putting compilation commands seems a rather strange idea in C code,
but at least, why not invoke an external tool/script that makes use of
existing build framework?
- RTE_INSTALL_DIR is undocumented,
- this implementation (passing -I <some_directory>/lib/eal/include) is
broken with a system-installed dpdk,


-- 
David Marchand


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

* RE: [PATCH] examples/pipeline: fix include path for rte_log.h
  2024-02-13 16:18 ` David Marchand
@ 2024-02-13 17:22   ` Dumitrescu, Cristian
  0 siblings, 0 replies; 14+ messages in thread
From: Dumitrescu, Cristian @ 2024-02-13 17:22 UTC (permalink / raw)
  To: Marchand, David; +Cc: dev, stable

Hi David,

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, February 13, 2024 4:19 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: Re: [PATCH] examples/pipeline: fix include path for rte_log.h
> 
> 
> Wrt $Subject, copy/paste code in drivers/net/softnic is broken too.

You're right, let me fix this in V2.

> 
> Some comments on this code:
> - putting compilation commands seems a rather strange idea in C code,
> but at least, why not invoke an external tool/script that makes use of
> existing build framework?

Yes, I am thinking of transitioning the pipeline app from the examples
folder to the apps folder, as it is way more than an example at this point,
and also add a script/tool to generate the C code in a more configurable
way. For now, the examples/pipeline contains just the default way to do
this, but of course a standalone tool would be better. On my ToDo list.
Thanks for providing this input!

> - RTE_INSTALL_DIR is undocumented,

This option is not supposed to be broadly used, but it is there just in case.
When the tool mentioned above will be ready, this will be moved into that
tool.

> - this implementation (passing -I <some_directory>/lib/eal/include) is
> broken with a system-installed dpdk,

Yes, this is expected. For the purpose of the pipeline C code generation,
we need the path to the full DPDK folder, not just the public header files.
There is one internal DPDK internal header file (rte_pipeline_internal.h)
which is needed for code generation purposes, but it should not be
made a public header, as it does not contain the pipeline API that apps
are using.

> 
 
> --
> David Marchand

Regard,
Cristian

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

* [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-13 14:52 [PATCH] examples/pipeline: fix include path for rte_log.h Cristian Dumitrescu
  2024-02-13 16:18 ` David Marchand
@ 2024-02-13 17:38 ` Cristian Dumitrescu
  2024-02-14 11:22   ` Ferruh Yigit
  2024-02-15 13:32   ` Ferruh Yigit
  1 sibling, 2 replies; 14+ messages in thread
From: Cristian Dumitrescu @ 2024-02-13 17:38 UTC (permalink / raw)
  To: dev; +Cc: stable

When rte_log.h was moved to a new directory, the include path was not
updated for the generated C code produced by the pipeline library,
which results in build failure for this code.

Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
Cc: stable@dpdk.org

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
---
 drivers/net/softnic/rte_eth_softnic_cli.c | 2 ++
 examples/pipeline/cli.c                   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/softnic/rte_eth_softnic_cli.c b/drivers/net/softnic/rte_eth_softnic_cli.c
index 085523fe03..95e705c553 100644
--- a/drivers/net/softnic/rte_eth_softnic_cli.c
+++ b/drivers/net/softnic/rte_eth_softnic_cli.c
@@ -337,6 +337,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
 		 "-I %s/lib/eal/include "
 		 "-I %s/lib/eal/x86/include "
 		 "-I %s/lib/eal/include/generic "
+		 "-I %s/lib/log "
 		 "-I %s/lib/meter "
 		 "-I %s/lib/port "
 		 "-I %s/lib/table "
@@ -361,6 +362,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
 		 install_dir,
 		 install_dir,
 		 install_dir,
+		 install_dir,
 		 log_file,
 		 obj_file,
 		 lib_file,
diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
index 2ae6cc579f..afb143c01f 100644
--- a/examples/pipeline/cli.c
+++ b/examples/pipeline/cli.c
@@ -714,6 +714,7 @@ cmd_pipeline_libbuild(char **tokens,
 		 "-I %s/lib/eal/include "
 		 "-I %s/lib/eal/x86/include "
 		 "-I %s/lib/eal/include/generic "
+		 "-I %s/lib/log "
 		 "-I %s/lib/meter "
 		 "-I %s/lib/port "
 		 "-I %s/lib/table "
@@ -738,6 +739,7 @@ cmd_pipeline_libbuild(char **tokens,
 		 install_dir,
 		 install_dir,
 		 install_dir,
+		 install_dir,
 		 log_file,
 		 obj_file,
 		 lib_file,
-- 
2.34.1


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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-13 17:38 ` [PATCH V2] " Cristian Dumitrescu
@ 2024-02-14 11:22   ` Ferruh Yigit
  2024-02-14 16:25     ` Aaron Conole
  2024-02-14 17:22     ` Dumitrescu, Cristian
  2024-02-15 13:32   ` Ferruh Yigit
  1 sibling, 2 replies; 14+ messages in thread
From: Ferruh Yigit @ 2024-02-14 11:22 UTC (permalink / raw)
  To: Cristian Dumitrescu, dev
  Cc: stable, David Marchand, Aaron Conole, Bruce Richardson

On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
> When rte_log.h was moved to a new directory, the include path was not
> updated for the generated C code produced by the pipeline library,
> which results in build failure for this code.
> 
> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
> Cc: stable@dpdk.org
> 

Hi Cristian,

How can I verify the fix? Can you please list the required steps?


And I wonder how this skipped the testing, I guess v23.11 released with
this defect. Is there a gap in the CI or internal build/test scripts?


> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> ---
>  drivers/net/softnic/rte_eth_softnic_cli.c | 2 ++
>  examples/pipeline/cli.c                   | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/softnic/rte_eth_softnic_cli.c b/drivers/net/softnic/rte_eth_softnic_cli.c
> index 085523fe03..95e705c553 100644
> --- a/drivers/net/softnic/rte_eth_softnic_cli.c
> +++ b/drivers/net/softnic/rte_eth_softnic_cli.c
> @@ -337,6 +337,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
>  		 "-I %s/lib/eal/include "
>  		 "-I %s/lib/eal/x86/include "
>  		 "-I %s/lib/eal/include/generic "
> +		 "-I %s/lib/log "
>  		 "-I %s/lib/meter "
>  		 "-I %s/lib/port "
>  		 "-I %s/lib/table "
> @@ -361,6 +362,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
>  		 install_dir,
>  		 install_dir,
>  		 install_dir,
> +		 install_dir,
>  		 log_file,
>  		 obj_file,
>  		 lib_file,
> diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
> index 2ae6cc579f..afb143c01f 100644
> --- a/examples/pipeline/cli.c
> +++ b/examples/pipeline/cli.c
> @@ -714,6 +714,7 @@ cmd_pipeline_libbuild(char **tokens,
>  		 "-I %s/lib/eal/include "
>  		 "-I %s/lib/eal/x86/include "
>  		 "-I %s/lib/eal/include/generic "
> +		 "-I %s/lib/log "
>  		 "-I %s/lib/meter "
>  		 "-I %s/lib/port "
>  		 "-I %s/lib/table "
> @@ -738,6 +739,7 @@ cmd_pipeline_libbuild(char **tokens,
>  		 install_dir,
>  		 install_dir,
>  		 install_dir,
> +		 install_dir,
>  		 log_file,
>  		 obj_file,
>  		 lib_file,


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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 11:22   ` Ferruh Yigit
@ 2024-02-14 16:25     ` Aaron Conole
  2024-02-14 19:32       ` Patrick Robb
  2024-02-14 17:22     ` Dumitrescu, Cristian
  1 sibling, 1 reply; 14+ messages in thread
From: Aaron Conole @ 2024-02-14 16:25 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Cristian Dumitrescu, dev, stable, David Marchand,
	Bruce Richardson, Patrick Robb

Ferruh Yigit <ferruh.yigit@amd.com> writes:

> On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
>> When rte_log.h was moved to a new directory, the include path was not
>> updated for the generated C code produced by the pipeline library,
>> which results in build failure for this code.
>> 
>> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
>> Cc: stable@dpdk.org
>> 
>
> Hi Cristian,
>
> How can I verify the fix? Can you please list the required steps?

I guess maybe (?) with the pipeline DTS case, but I'm not sure that
would be sufficient.

> And I wonder how this skipped the testing, I guess v23.11 released with
> this defect. Is there a gap in the CI or internal build/test scripts?

I don't know that softnic driver is used in the lab.  Actually, would
DTS suite even have triggered this issue?  I'm not sure if there is a
set of tests which covers the case.  Maybe Patrick can confirm about the
pipeline test?

>> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>> ---
>>  drivers/net/softnic/rte_eth_softnic_cli.c | 2 ++
>>  examples/pipeline/cli.c                   | 2 ++
>>  2 files changed, 4 insertions(+)
>> 
>> diff --git a/drivers/net/softnic/rte_eth_softnic_cli.c b/drivers/net/softnic/rte_eth_softnic_cli.c
>> index 085523fe03..95e705c553 100644
>> --- a/drivers/net/softnic/rte_eth_softnic_cli.c
>> +++ b/drivers/net/softnic/rte_eth_softnic_cli.c
>> @@ -337,6 +337,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
>>  		 "-I %s/lib/eal/include "
>>  		 "-I %s/lib/eal/x86/include "
>>  		 "-I %s/lib/eal/include/generic "
>> +		 "-I %s/lib/log "
>>  		 "-I %s/lib/meter "
>>  		 "-I %s/lib/port "
>>  		 "-I %s/lib/table "
>> @@ -361,6 +362,7 @@ cmd_softnic_pipeline_libbuild(struct pmd_internals *softnic __rte_unused,
>>  		 install_dir,
>>  		 install_dir,
>>  		 install_dir,
>> +		 install_dir,
>>  		 log_file,
>>  		 obj_file,
>>  		 lib_file,
>> diff --git a/examples/pipeline/cli.c b/examples/pipeline/cli.c
>> index 2ae6cc579f..afb143c01f 100644
>> --- a/examples/pipeline/cli.c
>> +++ b/examples/pipeline/cli.c
>> @@ -714,6 +714,7 @@ cmd_pipeline_libbuild(char **tokens,
>>  		 "-I %s/lib/eal/include "
>>  		 "-I %s/lib/eal/x86/include "
>>  		 "-I %s/lib/eal/include/generic "
>> +		 "-I %s/lib/log "
>>  		 "-I %s/lib/meter "
>>  		 "-I %s/lib/port "
>>  		 "-I %s/lib/table "
>> @@ -738,6 +739,7 @@ cmd_pipeline_libbuild(char **tokens,
>>  		 install_dir,
>>  		 install_dir,
>>  		 install_dir,
>> +		 install_dir,
>>  		 log_file,
>>  		 obj_file,
>>  		 lib_file,


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

* RE: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 11:22   ` Ferruh Yigit
  2024-02-14 16:25     ` Aaron Conole
@ 2024-02-14 17:22     ` Dumitrescu, Cristian
  2024-02-15 12:17       ` Ferruh Yigit
  1 sibling, 1 reply; 14+ messages in thread
From: Dumitrescu, Cristian @ 2024-02-14 17:22 UTC (permalink / raw)
  To: Ferruh Yigit, dev
  Cc: stable, Marchand, David, Aaron Conole, Richardson, Bruce

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Wednesday, February 14, 2024 11:22 AM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Marchand, David <david.marchand@redhat.com>; Aaron
> Conole <aconole@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
> 
> On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
> > When rte_log.h was moved to a new directory, the include path was not
> > updated for the generated C code produced by the pipeline library,
> > which results in build failure for this code.
> >
> > Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
> > Cc: stable@dpdk.org
> >
> 
> Hi Cristian,
> 
> How can I verify the fix? Can you please list the required steps?
> 

Just build the examples/pipeline app and the run this app with the command line below (you can use l2fwd.cli or any other .cli file from the same folder):
	./build/examples/dpdk-pipeline -l0-1 -- -s ./examples/pipeline/examples/l2fwd.cli

If everything works, then you should:
	i) not get any error message, especially for the "pipeline libbuild /tmp/l2fwd.c /tmp/l2fwd.so" line from l2fwd.cli script).
	ii) have the file "/tmp/l2fwd.so" created.
	iii) get the traffic flowing (if you have the NICs working)

Note: you would need to adjust the ethdev BFDs to those in your setup in the .cli file and the ethdev.io file from the same folder. Just read through the .cli file.

Ping me if you get into trouble.

> 
> And I wonder how this skipped the testing, I guess v23.11 released with
> this defect. Is there a gap in the CI or internal build/test scripts?
> 

Regards,
Cristian

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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 16:25     ` Aaron Conole
@ 2024-02-14 19:32       ` Patrick Robb
  2024-02-14 20:00         ` Dumitrescu, Cristian
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Robb @ 2024-02-14 19:32 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Ferruh Yigit, Cristian Dumitrescu, dev, stable, David Marchand,
	Bruce Richardson

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

Hi Aaron/Cristian,

On Wed, Feb 14, 2024 at 11:25 AM Aaron Conole <aconole@redhat.com> wrote:

> Ferruh Yigit <ferruh.yigit@amd.com> writes:
>
> > On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
> >> When rte_log.h was moved to a new directory, the include path was not
> >> updated for the generated C code produced by the pipeline library,
> >> which results in build failure for this code.
> >>
> >> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
> >> Cc: stable@dpdk.org
> >>
> >
> > Hi Cristian,
> >
> > How can I verify the fix? Can you please list the required steps?
>
> I guess maybe (?) with the pipeline DTS case, but I'm not sure that
> would be sufficient.
>
> > And I wonder how this skipped the testing, I guess v23.11 released with
> > this defect. Is there a gap in the CI or internal build/test scripts?
>
> I don't know that softnic driver is used in the lab.  Actually, would
> DTS suite even have triggered this issue?  I'm not sure if there is a
> set of tests which covers the case.  Maybe Patrick can confirm about the
> pipeline test?
>

So, based on what Cristian stated (passing in any of the .cli files when
starting the pipeline example app would show it is fixed), yes I assume the
DTS testsuite would have caught this, as I can see the testsuite does do
that. But, yes it's also true that the pipeline testsuites are not run at
UNH or the Intel Lab (the two labs which publicly report DTS results), so
that's how this gets through CI Testing. It is not possible (testing
capacity wise) to run every testsuite, and I don't think there has been
conversation between the lab and our vendor contacts about this specific
coverage (at least not while I've been working here).

However, based on the physical testplan requirements (4 10G tester ports to
4 10G DUT ports), we could bring the testsuite online if there is
interest(and the testsuite hasn't broken since it dropped in 2020). One of
our Intel testbeds which we run currently has exactly that NIC topology,
and it also doesn't have bad testing capacity concerns as compared to some
other testbeds. Let me know if there is an interest in this coverage and
I'll make a ticket for the team to take a look. At a minimum we could dry
run the framework on the testbed I'm thinking of and provide feedback,
which I suppose would take only a couple minutes/hours. Christian let me
know if that's a value - it's a low barrier of entry to dry run.

https://git.dpdk.org/tools/dts/tree/test_plans/pipeline_test_plan.rst

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

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

* RE: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 19:32       ` Patrick Robb
@ 2024-02-14 20:00         ` Dumitrescu, Cristian
  2024-02-14 20:17           ` Patrick Robb
  0 siblings, 1 reply; 14+ messages in thread
From: Dumitrescu, Cristian @ 2024-02-14 20:00 UTC (permalink / raw)
  To: Patrick Robb, Aaron Conole
  Cc: Ferruh Yigit, dev, stable, Marchand, David, Richardson, Bruce

Hi Patrick,

See my comment below under your reply.

From: Patrick Robb <probb@iol.unh.edu> 
Sent: Wednesday, February 14, 2024 7:32 PM
To: Aaron Conole <aconole@redhat.com>
Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org; stable@dpdk.org; Marchand, David <david.marchand@redhat.com>; Richardson, Bruce <bruce.richardson@intel.com>
Subject: Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h

Hi Aaron/Cristian,

On Wed, Feb 14, 2024 at 11:25 AM Aaron Conole <mailto:aconole@redhat.com> wrote:
Ferruh Yigit <mailto:ferruh.yigit@amd.com> writes:

> On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
>> When rte_log.h was moved to a new directory, the include path was not
>> updated for the generated C code produced by the pipeline library,
>> which results in build failure for this code.
>> 
>> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
>> Cc: mailto:stable@dpdk.org
>> 
>
> Hi Cristian,
>
> How can I verify the fix? Can you please list the required steps?

I guess maybe (?) with the pipeline DTS case, but I'm not sure that
would be sufficient.

> And I wonder how this skipped the testing, I guess v23.11 released with
> this defect. Is there a gap in the CI or internal build/test scripts?

I don't know that softnic driver is used in the lab.  Actually, would
DTS suite even have triggered this issue?  I'm not sure if there is a
set of tests which covers the case.  Maybe Patrick can confirm about the
pipeline test?

So, based on what Cristian stated (passing in any of the .cli files when starting the pipeline example app would show it is fixed), yes I assume the DTS testsuite would have caught this, as I can see the testsuite does do that. But, yes it's also true that the pipeline testsuites are not run at UNH or the Intel Lab (the two labs which publicly report DTS results), so that's how this gets through CI Testing. It is not possible (testing capacity wise) to run every testsuite, and I don't think there has been conversation between the lab and our vendor contacts about this specific coverage (at least not while I've been working here).

However, based on the physical testplan requirements (4 10G tester ports to 4 10G DUT ports), we could bring the testsuite online if there is interest(and the testsuite hasn't broken since it dropped in 2020). One of our Intel testbeds which we run currently has exactly that NIC topology, and it also doesn't have bad testing capacity concerns as compared to some other testbeds. Let me know if there is an interest in this coverage and I'll make a ticket for the team to take a look. At a minimum we could dry run the framework on the testbed I'm thinking of and provide feedback, which I suppose would take only a couple minutes/hours. Christian let me know if that's a value - it's a low barrier of entry to dry run.

https://git.dpdk.org/tools/dts/tree/test_plans/pipeline_test_plan.rst


[Cristian]
Yes, you are right, we do have a DTS test suite for the pipeline library.

It would be great to run it automatically as part of the CI testing. Just a word of caution though: I am not the owner of those tests, and also not intimately familiar with them, so it might be hard to find volunteers to setup the test suite, that the only problem that I see unfortunately. But we should try and see if there are any issues at all.

Thanks,
Cristian

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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 20:00         ` Dumitrescu, Cristian
@ 2024-02-14 20:17           ` Patrick Robb
  2024-07-16 16:33             ` Patrick Robb
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Robb @ 2024-02-14 20:17 UTC (permalink / raw)
  To: Dumitrescu, Cristian
  Cc: Aaron Conole, Ferruh Yigit, dev, stable, Marchand, David,
	Richardson, Bruce

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

On Wed, Feb 14, 2024 at 3:00 PM Dumitrescu, Cristian <
cristian.dumitrescu@intel.com> wrote:

>
>
> [Cristian]
> Yes, you are right, we do have a DTS test suite for the pipeline library.
>
> It would be great to run it automatically as part of the CI testing. Just
> a word of caution though: I am not the owner of those tests, and also not
> intimately familiar with them, so it might be hard to find volunteers to
> setup the test suite, that the only problem that I see unfortunately. But
> we should try and see if there are any issues at all.
>
> Thanks,
> Cristian
>

Okay, the setup doesn't look too burdensome anyhow so we should be alright
with just the team here at UNH. I'll let you know how it goes.

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

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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 17:22     ` Dumitrescu, Cristian
@ 2024-02-15 12:17       ` Ferruh Yigit
  0 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2024-02-15 12:17 UTC (permalink / raw)
  To: Dumitrescu, Cristian, dev
  Cc: stable, Marchand, David, Aaron Conole, Richardson, Bruce

On 2/14/2024 5:22 PM, Dumitrescu, Cristian wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Wednesday, February 14, 2024 11:22 AM
>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; dev@dpdk.org
>> Cc: stable@dpdk.org; Marchand, David <david.marchand@redhat.com>; Aaron
>> Conole <aconole@redhat.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>
>> Subject: Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
>>
>> On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
>>> When rte_log.h was moved to a new directory, the include path was not
>>> updated for the generated C code produced by the pipeline library,
>>> which results in build failure for this code.
>>>
>>> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
>>> Cc: stable@dpdk.org
>>>
>>
>> Hi Cristian,
>>
>> How can I verify the fix? Can you please list the required steps?
>>
> 
> Just build the examples/pipeline app and the run this app with the command line below (you can use l2fwd.cli or any other .cli file from the same folder):
> 	./build/examples/dpdk-pipeline -l0-1 -- -s ./examples/pipeline/examples/l2fwd.cli
> 
> If everything works, then you should:
> 	i) not get any error message, especially for the "pipeline libbuild /tmp/l2fwd.c /tmp/l2fwd.so" line from l2fwd.cli script).
> 	ii) have the file "/tmp/l2fwd.so" created.
> 	iii) get the traffic flowing (if you have the NICs working)
> 
> Note: you would need to adjust the ethdev BFDs to those in your setup in the .cli file and the ethdev.io file from the same folder. Just read through the .cli file.
> 

Thanks, I just tested the /tmp/l2fwd.so build, and confirmed the fix.

But it is hard to script this testcase. If 'dpdk-pipeline' exits with
failure when building ' /tmp/l2fwd.so' fails, at least this can be
tested easier in a script, does this make sense?


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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-13 17:38 ` [PATCH V2] " Cristian Dumitrescu
  2024-02-14 11:22   ` Ferruh Yigit
@ 2024-02-15 13:32   ` Ferruh Yigit
  1 sibling, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2024-02-15 13:32 UTC (permalink / raw)
  To: Cristian Dumitrescu; +Cc: stable, dev, Thomas Monjalon, David Marchand

On 2/13/2024 5:38 PM, Cristian Dumitrescu wrote:
> When rte_log.h was moved to a new directory, the include path was not
> updated for the generated C code produced by the pipeline library,
> which results in build failure for this code.
> 
> Fixes: 09ce41310930 ("log: separate logging functions out of EAL")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 

Tested-by: Ferruh Yigit <ferruh.yigit@amd.com>


Patch has both net/softnic and examples/pipeline update, since update is
trivial and for exact same issue, instead of requesting a patch split I
am getting it directly to next-net to not create more overhead.

Applied to dpdk-next-net/main, thanks.


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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-02-14 20:17           ` Patrick Robb
@ 2024-07-16 16:33             ` Patrick Robb
  2024-07-17 10:10               ` Ferruh Yigit
  0 siblings, 1 reply; 14+ messages in thread
From: Patrick Robb @ 2024-07-16 16:33 UTC (permalink / raw)
  To: Dumitrescu, Cristian
  Cc: Aaron Conole, Ferruh Yigit, dev, stable, Marchand, David,
	Richardson, Bruce

On Wed, Feb 14, 2024 at 3:17 PM Patrick Robb <probb@iol.unh.edu> wrote:
>
>
>
> On Wed, Feb 14, 2024 at 3:00 PM Dumitrescu, Cristian <cristian.dumitrescu@intel.com> wrote:
>>
>>
>>
>> [Cristian]
>> Yes, you are right, we do have a DTS test suite for the pipeline library.
>>
>> It would be great to run it automatically as part of the CI testing. Just a word of caution though: I am not the owner of those tests, and also not intimately familiar with them, so it might be hard to find volunteers to setup the test suite, that the only problem that I see unfortunately. But we should try and see if there are any issues at all.
>>
>> Thanks,
>> Cristian
>
>
> Okay, the setup doesn't look too burdensome anyhow so we should be alright with just the team here at UNH. I'll let you know how it goes.

Sorry I realized today I never provided an update here. Back in
February I did wire a testbed according to the testplan, insalled the
dependencies and gave it a run, which was not passing.

It would have been possible to pick apart the testsuite, manually
running the example app to see whether the issue laid in the testbed
setup, testsuite implementation, or somewhere else. But, it would have
taken time and the premise of this effort was that it would be a quick
setup, so I gave up as our efforts should go towards making additions
to new DTS now, instead of debugging old DTS.

I'm sorry that there isn't pipeline coverage in CI currently though -
something to target going forward.

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

* Re: [PATCH V2] examples/pipeline: fix include path for rte_log.h
  2024-07-16 16:33             ` Patrick Robb
@ 2024-07-17 10:10               ` Ferruh Yigit
  0 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2024-07-17 10:10 UTC (permalink / raw)
  To: Patrick Robb, Dumitrescu, Cristian
  Cc: Aaron Conole, dev, stable, Marchand, David, Richardson, Bruce

On 7/16/2024 5:33 PM, Patrick Robb wrote:
> On Wed, Feb 14, 2024 at 3:17 PM Patrick Robb <probb@iol.unh.edu> wrote:
>>
>>
>>
>> On Wed, Feb 14, 2024 at 3:00 PM Dumitrescu, Cristian <cristian.dumitrescu@intel.com> wrote:
>>>
>>>
>>>
>>> [Cristian]
>>> Yes, you are right, we do have a DTS test suite for the pipeline library.
>>>
>>> It would be great to run it automatically as part of the CI testing. Just a word of caution though: I am not the owner of those tests, and also not intimately familiar with them, so it might be hard to find volunteers to setup the test suite, that the only problem that I see unfortunately. But we should try and see if there are any issues at all.
>>>
>>> Thanks,
>>> Cristian
>>
>>
>> Okay, the setup doesn't look too burdensome anyhow so we should be alright with just the team here at UNH. I'll let you know how it goes.
> 
> Sorry I realized today I never provided an update here. Back in
> February I did wire a testbed according to the testplan, insalled the
> dependencies and gave it a run, which was not passing.
> 
> It would have been possible to pick apart the testsuite, manually
> running the example app to see whether the issue laid in the testbed
> setup, testsuite implementation, or somewhere else. But, it would have
> taken time and the premise of this effort was that it would be a quick
> setup, so I gave up as our efforts should go towards making additions
> to new DTS now, instead of debugging old DTS.
> 
> I'm sorry that there isn't pipeline coverage in CI currently though -
> something to target going forward.
>

Thanks for the update Patrick.

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

end of thread, other threads:[~2024-07-17 10:11 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 14:52 [PATCH] examples/pipeline: fix include path for rte_log.h Cristian Dumitrescu
2024-02-13 16:18 ` David Marchand
2024-02-13 17:22   ` Dumitrescu, Cristian
2024-02-13 17:38 ` [PATCH V2] " Cristian Dumitrescu
2024-02-14 11:22   ` Ferruh Yigit
2024-02-14 16:25     ` Aaron Conole
2024-02-14 19:32       ` Patrick Robb
2024-02-14 20:00         ` Dumitrescu, Cristian
2024-02-14 20:17           ` Patrick Robb
2024-07-16 16:33             ` Patrick Robb
2024-07-17 10:10               ` Ferruh Yigit
2024-02-14 17:22     ` Dumitrescu, Cristian
2024-02-15 12:17       ` Ferruh Yigit
2024-02-15 13:32   ` Ferruh Yigit

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).