* [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
* [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 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
* 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).