09/05/2018 16:03, Gaëtan Rivet: > On Wed, May 09, 2018 at 03:43:31PM +0200, Thomas Monjalon wrote: > > 09/05/2018 15:30, Gaëtan Rivet: > > > On Wed, May 09, 2018 at 01:01:58PM +0000, Matan Azrad wrote: > > > > Regarding uint32 > > > > The maximum port id number can be 0xffff. > > > > In this case the loop will be infinite if we use uint16 to iterate over > > > > all the ports. > > > > > > If RTE_MAX_ETHPORTS is set to 0xffff, an array rte_eth_devices[0xffff] > > > would be defined statically, and I think other issues would arise > > > before our being stuck in an infinite loop? > > > > > > In any case, if this had to be fixed, then there should be a > > > BUILD_BUG_ON RTE_MAX_ETHPORTS being 0xffff, in the relevant part of > > > librte_ethdev, instead of relying on librte_ethdev users skirting > > > shortfalls of the library. Anyone iterating on port IDs should expect the > > > port_id type to be sufficient to hold this information. > > > > Interesting thought. > > I vote for keeping Matan's option as it is correct, > > and will accept a patch in 18.08 for your option (BUILD_BUG_ON). > > Maybe we should send a deprecation notice before limiting the max > > number of ports to 0xfffe? Or is it ridiculous for such unlikely constraint? > > > > > > No actually the issue is when RTE_MAX_ETHPORTS is equal (or superior) to > 0x10000. > > If this is an issue that you think is worth having a workaround here, > you should also consider that rte_eth_find_next_owned_by (and > rte_eth_find_next, even if this one should be phased out), would also > result in an overflow and an infinite loop.
You get a point. I will remove the workaround uint32_t in v2, so all the related issues can be fixed at once in a separate patch using BUILD_BUG_ON.