patches for DPDK stable branches
 help / color / mirror / Atom feed
From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: Patrick Robb <probb@iol.unh.edu>, Aaron Conole <aconole@redhat.com>
Cc: Ferruh Yigit <ferruh.yigit@amd.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"stable@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
Date: Wed, 14 Feb 2024 20:00:08 +0000	[thread overview]
Message-ID: <DS0PR11MB74427F5AB7045990915EA4A3EB4E2@DS0PR11MB7442.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CAJvnSUAwg+h25uxVqRDD=qGnKkTE_Cwu3V=OY3SAiVq0r2WYuQ@mail.gmail.com>

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

  reply	other threads:[~2024-02-14 20:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-13 14:52 [PATCH] " 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 [this message]
2024-02-14 20:17           ` Patrick Robb
2024-02-14 17:22     ` Dumitrescu, Cristian
2024-02-15 12:17       ` Ferruh Yigit
2024-02-15 13:32   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DS0PR11MB74427F5AB7045990915EA4A3EB4E2@DS0PR11MB7442.namprd11.prod.outlook.com \
    --to=cristian.dumitrescu@intel.com \
    --cc=aconole@redhat.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=probb@iol.unh.edu \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).