On Wed, 30 Apr 2025 11:12:51 +0000 Shani Peretz <shper...@nvidia.com> wrote:
> Hey Stephan, > Apologize for the delayed response and appreciate your assistance. > I prepared a document outlining the bug and its flow, along with the two > solutions we discussed. > I hope this document provides a comprehensive overview. Can you take a look? > > Thank you! > > https://docs.google.com/document/d/1LmMlJ31P1G77K0TGkBfXWz0DEj6zdprP0bG5p_wu77w/edit?usp=sharing > > > > -----Original Message----- > > From: Stephen Hemminger <step...@networkplumber.org> > > Sent: Monday, 17 March 2025 16:11 > > To: Shani Peretz <shper...@nvidia.com> > > Cc: dev@dpdk.org; Tyler Retzlaff <roret...@linux.microsoft.com>; Parav > > Pandit > > <pa...@nvidia.com>; Xueming Li <xuemi...@nvidia.com>; Nipun Gupta > > <nipun.gu...@amd.com>; Nikhil Agarwal <nikhil.agar...@amd.com>; Hemant > > Agrawal <hemant.agra...@nxp.com>; Sachin Saxena > > <sachin.sax...@nxp.com>; Rosen Xu <rosen...@intel.com>; Chenbo Xia > > <chen...@nvidia.com>; Tomasz Duszynski <tduszyn...@marvell.com>; > > Chengwen Feng <fengcheng...@huawei.com>; NBU-Contact-longli > > (EXTERNAL) <lon...@microsoft.com>; Wei Hu <w...@microsoft.com>; Bruce > > Richardson <bruce.richard...@intel.com>; Kevin Laatz > > <kevin.la...@intel.com>; Jan Blunck <jblu...@infradead.org> > > Subject: Re: [PATCH v7 2/4] lib: fix comparison between devices > > > > External email: Use caution opening links or attachments > > > > > > On Thu, 6 Mar 2025 16:26:50 +0000 > > Shani Peretz <shper...@nvidia.com> wrote: > > > > > > -----Original Message----- > > > > From: Stephen Hemminger <step...@networkplumber.org> > > > > Sent: Thursday, 20 February 2025 20:33 > > > > To: Shani Peretz <shper...@nvidia.com> > > > > Cc: dev@dpdk.org; Tyler Retzlaff <roret...@linux.microsoft.com>; > > > > Parav Pandit <pa...@nvidia.com>; Xueming Li <xuemi...@nvidia.com>; > > > > Nipun Gupta <nipun.gu...@amd.com>; Nikhil Agarwal > > > > <nikhil.agar...@amd.com>; Hemant Agrawal <hemant.agra...@nxp.com>; > > > > Sachin Saxena <sachin.sax...@nxp.com>; Rosen Xu > > > > <rosen...@intel.com>; Chenbo Xia <chen...@nvidia.com>; Tomasz > > > > Duszynski <tduszyn...@marvell.com>; Chengwen Feng > > > > <fengcheng...@huawei.com>; NBU-Contact-longli > > > > (EXTERNAL) <lon...@microsoft.com>; Wei Hu <w...@microsoft.com>; Bruce > > > > Richardson <bruce.richard...@intel.com>; Kevin Laatz > > > > <kevin.la...@intel.com>; Jan Blunck <jblu...@infradead.org> > > > > Subject: Re: [PATCH v7 2/4] lib: fix comparison between devices > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > On Wed, 12 Feb 2025 18:38:33 +0200 > > > > Shani Peretz <shper...@nvidia.com> wrote: > > > > > > > > > DPDK supports multiple formats for specifying buses, (such as > > > > > "0000:08:00.0" and "08:00.0" for PCI). > > > > > This flexibility can lead to inconsistencies when using one format > > > > > while running testpmd, then attempts to use the other format in a > > > > > later command, resulting in a failure. > > > > > > > > > > The issue arises from the find_device function, which compares the > > > > > user-provided string directly with the device->name in the > > > > > rte_device structure. > > > > > If we want to accurately compare these names, we'll need to bring > > > > > both sides to the same representation by invoking the parse > > > > > function on the user input. > > > > > > > > Could you give an example where this happens please? > > > > Shouldn't find_device string always be changed into canonical form > > > > in find_device handler? > > > > > > The flow I was dealing with was attach_port -> rte_dev_probe - > > > local_dev_probe -> find_device. > > > The string passed to attach_port was the short version, directly from the > > > user. > > > > > > So, to clarify - you're saying that find_device simply need to accept the > > > string > > in its canonical form? Which means we'll only need to fix local_dev_probe to > > bring it to the canonical form before calling find_device? > > > I tried it but then I noticed that there's no function that gets the > > > user- > > provided string and returns it's string canonical form. The closest to this > > is > > parse, but what it eventually returns is not necessarily a string - it can > > be > > anything - for instance pci_parse will give you back a struct rte_pci_addr. > > > > Not sure at this point. There are two options. One would be fixup in > > attach_port > > the other would be allowing short form in PCI part of find_device. Since the > > strings from command line are put in canonical form for devargs, it seems > > logical to do it in attach_port path. What about if testpmd just did it first? diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index b5f0c02261..a324225e26 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -3413,17 +3413,18 @@ void attach_port(char *identifier) { portid_t pi; + struct rte_devargs devargs = { 0 }; struct rte_dev_iterator iterator; printf("Attaching a new port...\n"); - if (identifier == NULL) { + if (rte_devargs_parse(&devargs, identifier) != 0) { fprintf(stderr, "Invalid parameters are specified\n"); return; } - if (rte_dev_probe(identifier) < 0) { - TESTPMD_LOG(ERR, "Failed to attach port %s\n", identifier); + if (rte_dev_probe(devargs.name) < 0) { + TESTPMD_LOG(ERR, "Failed to attach port %s\n", devargs.name); return; } @@ -3435,14 +3436,14 @@ attach_port(char *identifier) } /* second attach mode: iterator */ - RTE_ETH_FOREACH_MATCHING_DEV(pi, identifier, &iterator) { + RTE_ETH_FOREACH_MATCHING_DEV(pi, devargs.name, &iterator) { /* setup ports matching the devargs used for probing */ if (port_is_forwarding(pi)) continue; /* port was already attached before */ setup_attached_port((void *)(intptr_t)pi); } out: - printf("Port %s is attached.\n", identifier); + printf("Port %s is attached.\n", devargs.name); printf("Done\n"); }
