Re: [dpdk-dev] [PATCH v2] ethdev: fix data type for port id

[wangyunjian]

> -----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);