> -----Original Message-----
> From: Stephen Hemminger <step...@networkplumber.org>
> Sent: Wednesday, 5 February 2025 20:46
> To: Shani Peretz <shper...@nvidia.com>
> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <tho...@monjalon.net>; 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 v4] bus: fix inconsistent representation of PCI numbers
>
> External email: Use caution opening links or attachments
>
>
> On Wed, 5 Feb 2025 17:37:54 +0000
> Shani Peretz <shper...@nvidia.com> wrote:
>
> > > -----Original Message-----
> > > From: Stephen Hemminger <step...@networkplumber.org>
> > > Sent: Wednesday, 5 February 2025 18:42
> > > To: Shani Peretz <shper...@nvidia.com>
> > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > > <tho...@monjalon.net>; 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 v4] bus: fix inconsistent representation of PCI
> > > numbers
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Wed, 5 Feb 2025 16:36:11 +0000
> > > Shani Peretz <shper...@nvidia.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Stephen Hemminger <step...@networkplumber.org>
> > > > > Sent: Wednesday, 29 January 2025 18:25
> > > > > To: Shani Peretz <shper...@nvidia.com>
> > > > > Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon (EXTERNAL)
> > > > > <tho...@monjalon.net>; 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 v4] bus: fix inconsistent representation of
> > > > > PCI numbers
> > > > >
> > > > > External email: Use caution opening links or attachments
> > > > >
> > > > >
> > > > > On Wed, 29 Jan 2025 10:54:16 +0200 Shani Peretz
> > > > > <shper...@nvidia.com> wrote:
> > > > >
> > > > > > +create_pci_dev(const char *name) {
> > > > > > + int port_id;
> > > > > > + uint8_t slave_mac1[] = {0x00, 0xFF, 0x00, 0xFF, 0x00, 0x00 };
> > > > > > + struct rte_ether_addr *mac_addr = (struct rte_ether_addr
> > > > > > +*)slave_mac1;
> > > > >
> > > > > Use different initializer and you can avoid the need for cast here.
> > > > >
> > > > >
> > > > > >
> > > > > > +/**
> > > > > > + * General device name comparison. Will compare by using the
> > > > > > +specific bus
> > > > > > + * compare function or by comparing the names directly.
> > > > > > + *
> > > > > > + * @param dev
> > > > > > + * Device handle.
> > > > > > + * @param name
> > > > > > + * Name to compare against.
> > > > > > + * @return
> > > > > > + * 0 if the device matches the name. Nonzero otherwise.
> > > > > > + */
> > > > > > +__rte_internal
> > > > > > +int rte_cmp_dev_name(const struct rte_device *dev, const void
> > > > > > +*name);
> > > > >
> > > > > It would make more sense to me if name was a character not void
> pointer.
> > > > >
> > > > > The design might be clearer if bus address was more of an
> > > > > typedef with a pointer and size together. Treat it more like an
> > > > > object.
> > > >
> > > >
> > > > Okay so just to understand,
> > > > this is the struct I am adding:
> > > >
> > > > struct rte_bus_address {
> > > > const void *addr;
> > > > size_t size;
> > > > };
> > > >
> > > > This is how I pass it to find_device:
> > > >
> > > > struct rte_bus_address dev_addr = {
> > > > .addr = da->name,
> > > > .size = RTE_DEV_NAME_MAX_LEN
> > > > };
> > > > dev = da->bus->find_device(NULL, rte_cmp_dev_name,
> > > > &dev_addr);
> > > >
> > > > And this is how I use it in rte_cmp_dev_name:
> > > >
> > > > rte_cmp_dev_name(const struct rte_device *dev1, const void *addr)
> > > > {
> > > > const struct rte_bus_address *dev2_addr = addr;
> > > > …
> > > > }
> > > >
> > > > Is that what you meant?
> > > > Also, I'm not sure if the size is really needed, because we check
> > > > the size of the parsed name, which may be different than the size
> > > > of the original name
> > > >
> > >
> > > It would be best to pass rte_bus_address to rte_cmp_dev_name rather
> > > than having implied cast.
> > > Not sure if that is possible without breaking API though.
> >
> > I think you are right, also it will require to change the signature of
> > find_device which will make it even bigger change
>
> Could you post patch with something "good enough" that builds and passes
> tests?
You mean with the rte_bus_address change or without?
Without it it was tested and passed. With it I only ran few tests locally and
they passed
