On 10/29/20 3:18 PM, wangyunjian wrote: >> -----Original Message----- >> From: Thomas Monjalon [mailto:tho...@monjalon.net] >> Sent: Tuesday, October 27, 2020 10:29 PM >> To: wangyunjian <wangyunj...@huawei.com> >> Cc: dev@dpdk.org; ferruh.yi...@intel.com; andrew.rybche...@oktetlabs.ru; >> Lilijun (Jerry) <jerry.lili...@huawei.com>; xudingke <xudin...@huawei.com>; >> wangyunjian <wangyunj...@huawei.com>; sta...@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH v2] ethdev: fix data type for port id >> >> 27/10/2020 14:41, wangyunjian: >>> From: Yunjian Wang <wangyunj...@huawei.com> >>> >>> The ethdev port id should be 16 bits now. This patch fixes the data >>> type of the variable for 'pid', changing from uint32_t to uint16_t. >>> >>> We also need use RTE_BUILD_BUG_ON() to ensure that >> RTE_MAX_ETHPORTS is >>> less or equal to UINT16_MAX. >> >> Actually the need is to check that we have room for an increment after >> RTE_MAX_ETHPORTS, meaning RTE_MAX_ETHPORTS < UINT16_MAX. > > OK. > >> >>> Fixes: 5b7ba31148a8 ("ethdev: add port ownership") >>> Cc: sta...@dpdk.org >>> >>> Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> >>> --- >>> v2: >>> add RTE_BUILD_BUG_ON() check for RTE_MAX_ETHPORTS validity >>> --- >>> rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id) { >>> - uint32_t pid; >>> + uint16_t pid; >>> >>> if (name == NULL) { >>> RTE_ETHDEV_LOG(ERR, "Null pointer is specified\n"); @@ -4292,6 >>> +4292,8 @@ RTE_INIT(eth_dev_init_cb_lists) { >>> int i; >>> >>> + RTE_BUILD_BUG_ON(RTE_MAX_ETHPORTS > UINT16_MAX); >> >> As explained above, should check >= >> >> This check is global for ethdev. >> Please could you move it somewhere else? >> >> > > What about following: > > # git diff > diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c > index ccea60afe7..ff878f4d4d 100644 > --- a/lib/librte_eal/freebsd/eal.c > +++ b/lib/librte_eal/freebsd/eal.c > @@ -672,6 +672,8 @@ rte_eal_init(int argc, char **argv) > struct internal_config *internal_conf = > eal_get_internal_configuration(); > > + RTE_BUILD_BUG_ON(RTE_MAX_ETHPORTS >= UINT16_MAX); > + > /* checks if the machine is adequate */ > if (!rte_cpu_is_supported()) { > rte_eal_init_alert("unsupported cpu type."); > diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c > index 9b579b8200..bd7fb7b5f1 100644 > --- a/lib/librte_eal/linux/eal.c > +++ b/lib/librte_eal/linux/eal.c > @@ -970,6 +970,8 @@ rte_eal_init(int argc, char **argv) > struct internal_config *internal_conf = > eal_get_internal_configuration(); > > + RTE_BUILD_BUG_ON(RTE_MAX_ETHPORTS >= UINT16_MAX); > + > /* checks if the machine is adequate */ > if (!rte_cpu_is_supported()) { > rte_eal_init_alert("unsupported cpu type."); > diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c > index 141f22adb7..27c2ebd114 100644 > --- a/lib/librte_eal/windows/eal.c > +++ b/lib/librte_eal/windows/eal.c > @@ -265,6 +265,8 @@ rte_eal_init(int argc, char **argv) > struct internal_config *internal_conf = > eal_get_internal_configuration(); > > + RTE_BUILD_BUG_ON(RTE_MAX_ETHPORTS >= UINT16_MAX); > + > rte_eal_log_init(NULL, 0); >
If RTE_MAX_ETHPORTS is a maximum number of ports starting from 0 and loops are port_id < RTE_MAX_ETHPORTS, RTE_MAX_ETHPORTS may be UINT16_MAX and it avoid 16-bit unsigned integer overflow, but may be it is safer to cut one more port from space do checks as above.
