* [dpdk-dev] [PATCH 0/2] ethdev: minor bugfixes @ 2020-06-19 3:42 Wei Hu (Xavier) 2020-06-19 3:42 ` [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup Wei Hu (Xavier) 2020-06-19 3:42 ` [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities Wei Hu (Xavier) 0 siblings, 2 replies; 6+ messages in thread From: Wei Hu (Xavier) @ 2020-06-19 3:42 UTC (permalink / raw) To: dev; +Cc: xavier.huwei This series are minor bugfixes for rte_ethdev.c. Chengchang Tang (2): ethdev: fix data room size verification in Rx queue setup ethdev: fix VLAN offloads set if no relative capabilities lib/librte_ethdev/rte_ethdev.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup 2020-06-19 3:42 [dpdk-dev] [PATCH 0/2] ethdev: minor bugfixes Wei Hu (Xavier) @ 2020-06-19 3:42 ` Wei Hu (Xavier) 2020-06-19 8:21 ` Andrew Rybchenko 2020-06-19 3:42 ` [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities Wei Hu (Xavier) 1 sibling, 1 reply; 6+ messages in thread From: Wei Hu (Xavier) @ 2020-06-19 3:42 UTC (permalink / raw) To: dev; +Cc: xavier.huwei From: Chengchang Tang <tangchengchang@huawei.com> In the rte_eth_rx_queue_setup API function, the local variable named mbp_buf_size, which is the data room size of the input parameter mp, is checked to guarantee that each memory chunck used for net device in the mbuf is bigger than the min_rx_bufsize. But if mbp_buf_size is less than RTE_PKTMBUF_HEADROOM, the value of the following statement will be a large number since the mbp_buf_size is a unsigned value. mbp_buf_size - RTE_PKTMBUF_HEADROOM As a result, it will cause a segment fault in this situation. This patch fixes it by adding a check condition to guarantee that the local varibale named mbp_buf_size is bigger than RTE_PKTMBUF_HEADROOM. Fixes: af75078fece3 ("first public release") Cc: stable@dpdk.org Signed-off-by: Chengchang Tang <tangchengchang@huawei.com> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com> --- lib/librte_ethdev/rte_ethdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 8e10a6f..122fd2a 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1822,7 +1822,8 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id, } mbp_buf_size = rte_pktmbuf_data_room_size(mp); - if ((mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) { + if (mbp_buf_size < RTE_PKTMBUF_HEADROOM || + (mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) { RTE_ETHDEV_LOG(ERR, "%s mbuf_data_room_size %d < %d (RTE_PKTMBUF_HEADROOM=%d + min_rx_bufsize(dev)=%d)\n", mp->name, (int)mbp_buf_size, -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup 2020-06-19 3:42 ` [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup Wei Hu (Xavier) @ 2020-06-19 8:21 ` Andrew Rybchenko 0 siblings, 0 replies; 6+ messages in thread From: Andrew Rybchenko @ 2020-06-19 8:21 UTC (permalink / raw) To: Wei Hu (Xavier), dev; +Cc: Thomas Monjalon, Ferruh Yigit On 6/19/20 6:42 AM, Wei Hu (Xavier) wrote: > From: Chengchang Tang <tangchengchang@huawei.com> > > In the rte_eth_rx_queue_setup API function, the local variable named > mbp_buf_size, which is the data room size of the input parameter mp, > is checked to guarantee that each memory chunck used for net device > in the mbuf is bigger than the min_rx_bufsize. But if mbp_buf_size is > less than RTE_PKTMBUF_HEADROOM, the value of the following statement > will be a large number since the mbp_buf_size is a unsigned value. > mbp_buf_size - RTE_PKTMBUF_HEADROOM > As a result, it will cause a segment fault in this situation. > > This patch fixes it by adding a check condition to guarantee that the > local varibale named mbp_buf_size is bigger than RTE_PKTMBUF_HEADROOM. > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org > > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com> > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com> > --- > lib/librte_ethdev/rte_ethdev.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index 8e10a6f..122fd2a 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -1822,7 +1822,8 @@ rte_eth_rx_queue_setup(uint16_t port_id, uint16_t rx_queue_id, > } > mbp_buf_size = rte_pktmbuf_data_room_size(mp); > > - if ((mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) { > + if (mbp_buf_size < RTE_PKTMBUF_HEADROOM || > + (mbp_buf_size - RTE_PKTMBUF_HEADROOM) < dev_info.min_rx_bufsize) { May be it could be shorten to mbp_buf_size < dev_info.min_rx_bufsize + RTE_PKTMBUF_HEADROOM However, way suggested in the patch is 100% safe against integer overflow. > RTE_ETHDEV_LOG(ERR, > "%s mbuf_data_room_size %d < %d (RTE_PKTMBUF_HEADROOM=%d + min_rx_bufsize(dev)=%d)\n", > mp->name, (int)mbp_buf_size, > Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities 2020-06-19 3:42 [dpdk-dev] [PATCH 0/2] ethdev: minor bugfixes Wei Hu (Xavier) 2020-06-19 3:42 ` [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup Wei Hu (Xavier) @ 2020-06-19 3:42 ` Wei Hu (Xavier) 2020-06-19 8:37 ` Andrew Rybchenko 1 sibling, 1 reply; 6+ messages in thread From: Wei Hu (Xavier) @ 2020-06-19 3:42 UTC (permalink / raw) To: dev; +Cc: xavier.huwei From: Chengchang Tang <tangchengchang@huawei.com> Currently, there is a potential problem that calling the API function rte_eth_dev_set_vlan_offload to start a vlan hardware offloads which the driver does not support. if the PMD driver does not support the relative hardware offloads and does not check for it, the hardware setting will not change, but the relative offload in dev->data->dev_conf.rxmode.offloads will be turned on. It is supposed to check the hardware capabilities to decide whether the relative callback needs to be called just like the behavior in the API function named rte_eth_dev_configure. Fixes: 81f9db8ecc2c ("ethdev: add vlan offload support") Cc: stable@dpdk.org Signed-off-by: Chengchang Tang <tangchengchang@huawei.com> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com> --- lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index 122fd2a..5176a0e 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -3261,12 +3261,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t port_id, int rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) { + struct rte_eth_dev_info dev_info; struct rte_eth_dev *dev; int ret = 0; int mask = 0; int cur, org = 0; uint64_t orig_offloads; uint64_t dev_offloads; + uint64_t new_offloads; RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); dev = &rte_eth_devices[port_id]; @@ -3320,6 +3322,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) if (mask == 0) return ret; + ret = rte_eth_dev_info_get(port_id, &dev_info); + if (ret != 0) + return ret; + + /* + * New added Rx vlan offloading which are not enabled in + * rte_eth_dev_configure() must be within its device capabilities + */ + if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) { + new_offloads = dev_offloads & ~orig_offloads; + RTE_ETHDEV_LOG(ERR, + "Ethdev port_id=%u requested new added vlan offloads " + "0x%" PRIx64 " must be within Rx offloads capabilities " + "0x%"PRIx64 " in %s()\n", + port_id, new_offloads, dev_info.rx_offload_capa, + __func__); + return -EINVAL; + } + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); dev->data->dev_conf.rxmode.offloads = dev_offloads; ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities 2020-06-19 3:42 ` [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities Wei Hu (Xavier) @ 2020-06-19 8:37 ` Andrew Rybchenko 2020-06-20 6:42 ` Wei Hu (Xavier) 0 siblings, 1 reply; 6+ messages in thread From: Andrew Rybchenko @ 2020-06-19 8:37 UTC (permalink / raw) To: Wei Hu (Xavier), dev; +Cc: Thomas Monjalon, Ferruh Yigit On 6/19/20 6:42 AM, Wei Hu (Xavier) wrote: > From: Chengchang Tang <tangchengchang@huawei.com> > > Currently, there is a potential problem that calling the API function > rte_eth_dev_set_vlan_offload to start a vlan hardware offloads which the > driver does not support. if the PMD driver does not support the relative > hardware offloads and does not check for it, the hardware setting will > not change, but the relative offload in dev->data->dev_conf.rxmode.offloads > will be turned on. I'm sorry, but I don't understand what 'relative' means in the context. Also, please, use 'VLAN' instead of 'vlan' in the description and log message below. > It is supposed to check the hardware capabilities to decide whether the > relative callback needs to be called just like the behavior in the API > function named rte_eth_dev_configure. I think the direction of the fix is right, but it definitely duplicates checks which are done in some PMDs. I think that it would be good to do the cleanup in follow up patches. > Fixes: 81f9db8ecc2c ("ethdev: add vlan offload support") Most likely it is incorrect, since generic Rx offloads were introduced later. > Cc: stable@dpdk.org > > Signed-off-by: Chengchang Tang <tangchengchang@huawei.com> > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com> > --- > lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index 122fd2a..5176a0e 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -3261,12 +3261,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t port_id, > int > rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) > { > + struct rte_eth_dev_info dev_info; > struct rte_eth_dev *dev; > int ret = 0; > int mask = 0; > int cur, org = 0; > uint64_t orig_offloads; > uint64_t dev_offloads; > + uint64_t new_offloads; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > @@ -3320,6 +3322,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) > if (mask == 0) > return ret; > > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + if (ret != 0) > + return ret; > + > + /* > + * New added Rx vlan offloading which are not enabled in > + * rte_eth_dev_configure() must be within its device capabilities > + */ > + if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) { > + new_offloads = dev_offloads & ~orig_offloads; > + RTE_ETHDEV_LOG(ERR, > + "Ethdev port_id=%u requested new added vlan offloads " > + "0x%" PRIx64 " must be within Rx offloads capabilities " > + "0x%"PRIx64 " in %s()\n", > + port_id, new_offloads, dev_info.rx_offload_capa, > + __func__); > + return -EINVAL; > + } > + > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); > dev->data->dev_conf.rxmode.offloads = dev_offloads; > ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities 2020-06-19 8:37 ` Andrew Rybchenko @ 2020-06-20 6:42 ` Wei Hu (Xavier) 0 siblings, 0 replies; 6+ messages in thread From: Wei Hu (Xavier) @ 2020-06-20 6:42 UTC (permalink / raw) To: Andrew Rybchenko; +Cc: dev, Thomas Monjalon, Ferruh Yigit Hi, Andrew Rybchenko On 2020/6/19 16:37, Andrew Rybchenko wrote: > On 6/19/20 6:42 AM, Wei Hu (Xavier) wrote: >> From: Chengchang Tang <tangchengchang@huawei.com> >> >> Currently, there is a potential problem that calling the API function >> rte_eth_dev_set_vlan_offload to start a vlan hardware offloads which the >> driver does not support. if the PMD driver does not support the relative >> hardware offloads and does not check for it, the hardware setting will >> not change, but the relative offload in dev->data->dev_conf.rxmode.offloads >> will be turned on. > I'm sorry, but I don't understand what 'relative' means in the > context. > > Also, please, use 'VLAN' instead of 'vlan' in the description > and log message below. OK, I will fix it in V2. >> It is supposed to check the hardware capabilities to decide whether the >> relative callback needs to be called just like the behavior in the API >> function named rte_eth_dev_configure. > I think the direction of the fix is right, but it definitely > duplicates checks which are done in some PMDs. I think that it > would be good to do the cleanup in follow up patches. I will clean them in V2. >> Fixes: 81f9db8ecc2c ("ethdev: add vlan offload support") > Most likely it is incorrect, since generic Rx offloads were > introduced later. OK, I will replace it with the following statement in V2. Fixes: a4996bd89c42 ("ethdev: new Rx/Tx offloads API") Thanks, Xavier > >> Cc: stable@dpdk.org >> >> Signed-off-by: Chengchang Tang <tangchengchang@huawei.com> >> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com> >> --- >> lib/librte_ethdev/rte_ethdev.c | 21 +++++++++++++++++++++ >> 1 file changed, 21 insertions(+) >> >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >> index 122fd2a..5176a0e 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -3261,12 +3261,14 @@ rte_eth_dev_set_vlan_ether_type(uint16_t port_id, >> int >> rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) >> { >> + struct rte_eth_dev_info dev_info; >> struct rte_eth_dev *dev; >> int ret = 0; >> int mask = 0; >> int cur, org = 0; >> uint64_t orig_offloads; >> uint64_t dev_offloads; >> + uint64_t new_offloads; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> @@ -3320,6 +3322,25 @@ rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask) >> if (mask == 0) >> return ret; >> >> + ret = rte_eth_dev_info_get(port_id, &dev_info); >> + if (ret != 0) >> + return ret; >> + >> + /* >> + * New added Rx vlan offloading which are not enabled in >> + * rte_eth_dev_configure() must be within its device capabilities >> + */ >> + if ((dev_offloads & dev_info.rx_offload_capa) != dev_offloads) { >> + new_offloads = dev_offloads & ~orig_offloads; >> + RTE_ETHDEV_LOG(ERR, >> + "Ethdev port_id=%u requested new added vlan offloads " >> + "0x%" PRIx64 " must be within Rx offloads capabilities " >> + "0x%"PRIx64 " in %s()\n", >> + port_id, new_offloads, dev_info.rx_offload_capa, >> + __func__); >> + return -EINVAL; >> + } >> + >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_offload_set, -ENOTSUP); >> dev->data->dev_conf.rxmode.offloads = dev_offloads; >> ret = (*dev->dev_ops->vlan_offload_set)(dev, mask); >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-20 6:42 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-19 3:42 [dpdk-dev] [PATCH 0/2] ethdev: minor bugfixes Wei Hu (Xavier) 2020-06-19 3:42 ` [dpdk-dev] [PATCH 1/2] ethdev: fix data room size verification in Rx queue setup Wei Hu (Xavier) 2020-06-19 8:21 ` Andrew Rybchenko 2020-06-19 3:42 ` [dpdk-dev] [PATCH 2/2] ethdev: fix VLAN offloads set if no relative capabilities Wei Hu (Xavier) 2020-06-19 8:37 ` Andrew Rybchenko 2020-06-20 6:42 ` Wei Hu (Xavier)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).