On Wed, 5 Feb 2025 16:36:11 +0000 Shani Peretz <[email protected]> wrote:
> > -----Original Message----- > > From: Stephen Hemminger <[email protected]> > > Sent: Wednesday, 29 January 2025 18:25 > > To: Shani Peretz <[email protected]> > > Cc: [email protected]; NBU-Contact-Thomas Monjalon (EXTERNAL) > > <[email protected]>; Tyler Retzlaff <[email protected]>; Parav > > Pandit <[email protected]>; Xueming Li <[email protected]>; Nipun Gupta > > <[email protected]>; Nikhil Agarwal <[email protected]>; Hemant > > Agrawal <[email protected]>; Sachin Saxena > > <[email protected]>; Rosen Xu <[email protected]>; Chenbo Xia > > <[email protected]>; Tomasz Duszynski <[email protected]>; > > Chengwen Feng <[email protected]>; NBU-Contact-longli > > (EXTERNAL) <[email protected]>; Wei Hu <[email protected]>; Bruce > > Richardson <[email protected]>; Kevin Laatz > > <[email protected]>; Jan Blunck <[email protected]> > > 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 <[email protected]> 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.
