ethdev: fix data type for port id
Checks
Commit Message
From: Yunjian Wang <wangyunjian@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.
Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
Cc: stable@dpdk.org
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
lib/librte_ethdev/rte_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
26/10/2020 13:24, wangyunjian:
> From: Yunjian Wang <wangyunjian@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.
>
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
It was 32-bit on purpose, to avoid overflow in this loop:
for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++)
It is now replaced by RTE_ETH_FOREACH_VALID_DEV,
but I wonder whether we still have this theoritical overflow risk.
If yes, we should change more variables to 32-bit.
On 10/26/20 3:24 PM, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@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.
>
> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> Cc: stable@dpdk.org
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
On 10/26/20 3:29 PM, Thomas Monjalon wrote:
> 26/10/2020 13:24, wangyunjian:
>> From: Yunjian Wang <wangyunjian@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.
>>
>> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
>
> It was 32-bit on purpose, to avoid overflow in this loop:
> for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++)
>
> It is now replaced by RTE_ETH_FOREACH_VALID_DEV,
> but I wonder whether we still have this theoritical overflow risk.
> If yes, we should change more variables to 32-bit.
Ah, it is too tricky. May be it is better to ensure that
RTE_MAX_ETHPORTS is less or equal to UINT16_MAX?
26/10/2020 13:33, Andrew Rybchenko:
> On 10/26/20 3:29 PM, Thomas Monjalon wrote:
> > 26/10/2020 13:24, wangyunjian:
> >> From: Yunjian Wang <wangyunjian@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.
> >>
> >> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> >
> > It was 32-bit on purpose, to avoid overflow in this loop:
> > for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++)
> >
> > It is now replaced by RTE_ETH_FOREACH_VALID_DEV,
> > but I wonder whether we still have this theoritical overflow risk.
> > If yes, we should change more variables to 32-bit.
>
> Ah, it is too tricky. May be it is better to ensure that
> RTE_MAX_ETHPORTS is less or equal to UINT16_MAX?
Yes could be another option.
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, October 26, 2020 8:34 PM
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org;
> ferruh.yigit@intel.com; Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
> <xudingke@huawei.com>; matan@nvidia.com
> Subject: Re: [dpdk-dev] [PATCH] ethdev: fix data type for port id
>
> 26/10/2020 13:33, Andrew Rybchenko:
> > On 10/26/20 3:29 PM, Thomas Monjalon wrote:
> > > 26/10/2020 13:24, wangyunjian:
> > >> From: Yunjian Wang <wangyunjian@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.
> > >>
> > >> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> > >
> > > It was 32-bit on purpose, to avoid overflow in this loop:
> > > for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++)
> > >
> > > It is now replaced by RTE_ETH_FOREACH_VALID_DEV, but I wonder
> > > whether we still have this theoritical overflow risk.
> > > If yes, we should change more variables to 32-bit.
> >
> > Ah, it is too tricky. May be it is better to ensure that
> > RTE_MAX_ETHPORTS is less or equal to UINT16_MAX?
>
> Yes could be another option.
>
Add a check on RTE_MAX_ETHPORT in rte_eal_init()?
27/10/2020 03:46, wangyunjian:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 26/10/2020 13:33, Andrew Rybchenko:
> > > On 10/26/20 3:29 PM, Thomas Monjalon wrote:
> > > > 26/10/2020 13:24, wangyunjian:
> > > >> From: Yunjian Wang <wangyunjian@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.
> > > >>
> > > >> Fixes: 5b7ba31148a8 ("ethdev: add port ownership")
> > > >
> > > > It was 32-bit on purpose, to avoid overflow in this loop:
> > > > for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++)
> > > >
> > > > It is now replaced by RTE_ETH_FOREACH_VALID_DEV, but I wonder
> > > > whether we still have this theoritical overflow risk.
> > > > If yes, we should change more variables to 32-bit.
> > >
> > > Ah, it is too tricky. May be it is better to ensure that
> > > RTE_MAX_ETHPORTS is less or equal to UINT16_MAX?
> >
> > Yes could be another option.
> >
>
> Add a check on RTE_MAX_ETHPORT in rte_eal_init()?
Could be a compilation check done with RTE_BUILD_BUG_ON.
@@ -816,7 +816,7 @@ rte_eth_dev_get_name_by_port(uint16_t port_id, char *name)
int
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");