DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/9] fix kvargs callback prototype not clearly defined
@ 2023-03-02  7:50 Chengwen Feng
  2023-03-02  7:50 ` [PATCH 1/9] kvargs: detailed definition of callback prototype Chengwen Feng
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function 
parameter 'value' is NULL when parsed 'only keys'.

But where there is no detailed definition of 'value' maybe NULL. this 
leads to a lot of processing errors (some may cause segment errors). 
This patchset fixes some of them.

Chengwen Feng (9):
  kvargs: detailed definition of callback prototype
  compressdev: fix segment fault when parse input args
  compressdev: fix null name when parse input args
  cryptodev: fix segment fault when parse input args
  cryptodev: fix null name when parse input args
  net/hns3: fix segment fault when parse runtime config
  net/virtio: fix segment fault when parse devargs
  dma/skeleton: fix segment fault when parse devargs
  raw/skeleton: fix segment fault when parse devargs

 drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
 drivers/net/hns3/hns3_common.c         |  9 +++++++++
 drivers/net/virtio/virtio_ethdev.c     |  3 +++
 drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
 drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
 lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
 lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
 lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
 8 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/9] kvargs: detailed definition of callback prototype
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-09 13:12   ` Olivier Matz
  2023-03-02  7:50 ` [PATCH 2/9] compressdev: fix segment fault when parse input args Chengwen Feng
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Olivier Matz; +Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
(which prototype is arg_handler_t) parameter 'value' is NULL when
parsed 'only keys'.

But where there is no detailed definition of 'value' maybe NULL, so
this patch adds it.

Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/kvargs/rte_kvargs.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/lib/kvargs/rte_kvargs.h b/lib/kvargs/rte_kvargs.h
index 359a9f5b09..4900b750bc 100644
--- a/lib/kvargs/rte_kvargs.h
+++ b/lib/kvargs/rte_kvargs.h
@@ -36,7 +36,19 @@ extern "C" {
 /** separator character used between key and value */
 #define RTE_KVARGS_KV_DELIM	"="
 
-/** Type of callback function used by rte_kvargs_process() */
+/**
+ * Callback prototype used by rte_kvargs_process().
+ *
+ * @param key
+ *   The key to consider, it will not be NULL.
+ * @param value
+ *   The value corresponding to the key, it may be NULL (e.g. only with key)
+ * @param opaque
+ *   An opaque pointer coming from the caller.
+ * @return
+ *   - >=0 handle key success.
+ *   - <0 on error.
+ */
 typedef int (*arg_handler_t)(const char *key, const char *value, void *opaque);
 
 /** A key/value association */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/9] compressdev: fix segment fault when parse input args
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
  2023-03-02  7:50 ` [PATCH 1/9] kvargs: detailed definition of callback prototype Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  7:50 ` [PATCH 3/9] compressdev: fix null name " Chengwen Feng
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Fan Zhang, Ashish Gupta, Fiona Trahe,
	Shally Verma, Pablo de Lara
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. socket_id).

Fixes: ed7dd94f7f66 ("compressdev: add basic device management")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/compressdev/rte_compressdev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/compressdev/rte_compressdev_pmd.c b/lib/compressdev/rte_compressdev_pmd.c
index e139bc86e7..93c31fbac8 100644
--- a/lib/compressdev/rte_compressdev_pmd.c
+++ b/lib/compressdev/rte_compressdev_pmd.c
@@ -40,6 +40,9 @@ rte_compressdev_pmd_parse_uint_arg(const char *key __rte_unused,
 	int i;
 	char *end;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 	i = strtol(value, &end, 10);
 	if (*end != 0 || errno != 0 || i < 0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/9] compressdev: fix null name when parse input args
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
  2023-03-02  7:50 ` [PATCH 1/9] kvargs: detailed definition of callback prototype Chengwen Feng
  2023-03-02  7:50 ` [PATCH 2/9] compressdev: fix segment fault when parse input args Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  7:50 ` [PATCH 4/9] cryptodev: fix segment fault " Chengwen Feng
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Fan Zhang, Ashish Gupta, Shally Verma,
	Fiona Trahe, Pablo de Lara
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

If the input args is just "name" (which mean only has keys), then it
will get (null) name in rte_compressdev_pmd_parse_name_arg(). This
patch fixes it.

Fixes: ed7dd94f7f66 ("compressdev: add basic device management")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/compressdev/rte_compressdev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/compressdev/rte_compressdev_pmd.c b/lib/compressdev/rte_compressdev_pmd.c
index 93c31fbac8..156bccd972 100644
--- a/lib/compressdev/rte_compressdev_pmd.c
+++ b/lib/compressdev/rte_compressdev_pmd.c
@@ -23,6 +23,9 @@ rte_compressdev_pmd_parse_name_arg(const char *key __rte_unused,
 	struct rte_compressdev_pmd_init_params *params = extra_args;
 	int n;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	n = strlcpy(params->name, value, RTE_COMPRESSDEV_NAME_MAX_LEN);
 	if (n >= RTE_COMPRESSDEV_NAME_MAX_LEN)
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/9] cryptodev: fix segment fault when parse input args
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (2 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 3/9] compressdev: fix null name " Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  8:11   ` [EXT] " Akhil Goyal
  2023-03-02  7:50 ` [PATCH 5/9] cryptodev: fix null name " Chengwen Feng
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Akhil Goyal, Fan Zhang, Declan Doherty,
	Pablo de Lara, Fiona Trahe
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. socket_id,max_nb_queue_pairs).

Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cryptodev/cryptodev_pmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 77b269f312..4122000839 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -38,6 +38,10 @@ rte_cryptodev_pmd_parse_uint_arg(const char *key __rte_unused,
 {
 	int i;
 	char *end;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 
 	i = strtol(value, &end, 10);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 5/9] cryptodev: fix null name when parse input args
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (3 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 4/9] cryptodev: fix segment fault " Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  7:50 ` [PATCH 6/9] net/hns3: fix segment fault when parse runtime config Chengwen Feng
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Akhil Goyal, Fan Zhang, Pablo de Lara,
	Declan Doherty, Fiona Trahe
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

If the input args is just "name" (which mean only has keys), then it
will get (null) name in rte_cryptodev_pmd_parse_name_arg(). This patch
fixes it.

Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cryptodev/cryptodev_pmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 4122000839..d8073a601d 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -22,6 +22,9 @@ rte_cryptodev_pmd_parse_name_arg(const char *key __rte_unused,
 	struct rte_cryptodev_pmd_init_params *params = extra_args;
 	int n;
 
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	n = strlcpy(params->name, value, RTE_CRYPTODEV_NAME_MAX_LEN);
 	if (n >= RTE_CRYPTODEV_NAME_MAX_LEN)
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 6/9] net/hns3: fix segment fault when parse runtime config
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (4 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 5/9] cryptodev: fix null name " Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-08  7:37   ` Dongdong Liu
  2023-03-02  7:50 ` [PATCH 7/9] net/virtio: fix segment fault when parse devargs Chengwen Feng
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Dongdong Liu, Yisen Zhuang, Min Hu (Connor),
	Chengchang Tang
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. rx_func_hint).

Fixes: a124f9e9591b ("net/hns3: add runtime config to select IO burst function")
Fixes: 70791213242e ("net/hns3: support masking device capability")
Fixes: 2fc3e696a7f1 ("net/hns3: add runtime config for mailbox limit time")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/hns3/hns3_common.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/hns3/hns3_common.c b/drivers/net/hns3/hns3_common.c
index a0c9e66c2c..f077ef5057 100644
--- a/drivers/net/hns3/hns3_common.c
+++ b/drivers/net/hns3/hns3_common.c
@@ -163,6 +163,9 @@ hns3_parse_io_hint_func(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	if (strcmp(value, "vec") == 0)
 		hint = HNS3_IO_FUNC_HINT_VEC;
 	else if (strcmp(value, "sve") == 0)
@@ -203,6 +206,9 @@ hns3_parse_dev_caps_mask(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	val = strtoull(value, NULL, HNS3_CONVERT_TO_HEXADECIMAL);
 	*(uint64_t *)extra_args = val;
 
@@ -216,6 +222,9 @@ hns3_parse_mbx_time_limit(const char *key, const char *value, void *extra_args)
 
 	RTE_SET_USED(key);
 
+	if (value == NULL || extra_args == NULL)
+		return 0;
+
 	val = strtoul(value, NULL, HNS3_CONVERT_TO_DECIMAL);
 
 	/*
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 7/9] net/virtio: fix segment fault when parse devargs
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (5 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 6/9] net/hns3: fix segment fault when parse runtime config Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-09 15:21   ` Maxime Coquelin
  2023-03-02  7:50 ` [PATCH 8/9] dma/skeleton: " Chengwen Feng
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Maxime Coquelin, Chenbo Xia, Yong Liu,
	Ivan Dyukov, Ferruh Yigit, Xiao Wang
  Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. vectorized,vdpa).

Fixes: 4710e16a4a7b ("net/virtio: add parameter to enable vectorized path")
Fixes: 44d7b2e87b69 ("net/virtio: refactor devargs parsing")
Fixes: 440f03c25378 ("net/virtio: skip device probe in vDPA mode")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/net/virtio/virtio_ethdev.c     | 3 +++
 drivers/net/virtio/virtio_pci_ethdev.c | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 0103d95920..d10f42bba2 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2054,6 +2054,9 @@ virtio_dev_speed_capa_get(uint32_t speed)
 static int vectorized_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
+	if (value == NULL || ret_val == NULL)
+		return -EINVAL;
+
 	if (strcmp(value, "1") == 0)
 		*(int *)ret_val = 1;
 	else
diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c
index abc63b0935..9b4b846f8a 100644
--- a/drivers/net/virtio/virtio_pci_ethdev.c
+++ b/drivers/net/virtio/virtio_pci_ethdev.c
@@ -148,6 +148,9 @@ eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev)
 static int vdpa_check_handler(__rte_unused const char *key,
 		const char *value, void *ret_val)
 {
+	if (value == NULL || ret_val == NULL)
+		return -EINVAL;
+
 	if (strcmp(value, "1") == 0)
 		*(int *)ret_val = 1;
 	else
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 8/9] dma/skeleton: fix segment fault when parse devargs
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (6 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 7/9] net/virtio: fix segment fault when parse devargs Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  7:50 ` [PATCH 9/9] raw/skeleton: " Chengwen Feng
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Kevin Laatz, Bruce Richardson, Conor Walsh; +Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. lcore).

Fixes: 05d5fc66a269 ("dma/skeleton: introduce skeleton driver")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/dma/skeleton/skeleton_dmadev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/skeleton/skeleton_dmadev.c b/drivers/dma/skeleton/skeleton_dmadev.c
index 9b6da655fd..daf35eccce 100644
--- a/drivers/dma/skeleton/skeleton_dmadev.c
+++ b/drivers/dma/skeleton/skeleton_dmadev.c
@@ -515,9 +515,15 @@ skeldma_parse_lcore(const char *key __rte_unused,
 		    const char *value,
 		    void *opaque)
 {
-	int lcore_id = atoi(value);
+	int lcore_id;
+
+	if (value == NULL || opaque == NULL)
+		return -EINVAL;
+
+	lcore_id = atoi(value);
 	if (lcore_id >= 0 && lcore_id < RTE_MAX_LCORE)
 		*(int *)opaque = lcore_id;
+
 	return 0;
 }
 
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 9/9] raw/skeleton: fix segment fault when parse devargs
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (7 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 8/9] dma/skeleton: " Chengwen Feng
@ 2023-03-02  7:50 ` Chengwen Feng
  2023-03-02  9:17 ` [PATCH 0/9] fix kvargs callback prototype not clearly defined fengchengwen
  2023-03-09 15:19 ` David Marchand
  10 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:50 UTC (permalink / raw)
  To: thomas, ferruh.yigit, Sachin Saxena, Hemant Agrawal, Shreyansh Jain; +Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. self_test).

Fixes: 55ca1b0f2151 ("raw/skeleton: add test cases")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 drivers/raw/skeleton/skeleton_rawdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/raw/skeleton/skeleton_rawdev.c b/drivers/raw/skeleton/skeleton_rawdev.c
index b2ca1cc5cd..53fe49f936 100644
--- a/drivers/raw/skeleton/skeleton_rawdev.c
+++ b/drivers/raw/skeleton/skeleton_rawdev.c
@@ -664,6 +664,8 @@ skeldev_get_selftest(const char *key __rte_unused,
 		     void *opaque)
 {
 	int *flag = opaque;
+	if (value == NULL || opaque == NULL)
+		return -EINVAL;
 	*flag = atoi(value);
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [EXT] [PATCH 4/9] cryptodev: fix segment fault when parse input args
  2023-03-02  7:50 ` [PATCH 4/9] cryptodev: fix segment fault " Chengwen Feng
@ 2023-03-02  8:11   ` Akhil Goyal
  2023-03-02  9:21     ` fengchengwen
  0 siblings, 1 reply; 18+ messages in thread
From: Akhil Goyal @ 2023-03-02  8:11 UTC (permalink / raw)
  To: Chengwen Feng, thomas, ferruh.yigit, Fan Zhang, Declan Doherty,
	Pablo de Lara, Fiona Trahe
  Cc: dev

> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
> 
> This patch fixes segment fault when parse input args with 'only keys'
> (e.g. socket_id,max_nb_queue_pairs).
> 
> Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>

I would say, you can squash your 4/9 and 5/9 patch in a single commit.
As you are doing the same thing in both the patches.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/9] fix kvargs callback prototype not clearly defined
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (8 preceding siblings ...)
  2023-03-02  7:50 ` [PATCH 9/9] raw/skeleton: " Chengwen Feng
@ 2023-03-02  9:17 ` fengchengwen
  2023-03-09 15:19 ` David Marchand
  10 siblings, 0 replies; 18+ messages in thread
From: fengchengwen @ 2023-03-02  9:17 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

Sorry to resend, because the original v1 --cc-cmd failed.

On 2023/3/2 15:50, Chengwen Feng wrote:
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function 
> parameter 'value' is NULL when parsed 'only keys'.
> 
> But where there is no detailed definition of 'value' maybe NULL. this 
> leads to a lot of processing errors (some may cause segment errors). 
> This patchset fixes some of them.
> 
> Chengwen Feng (9):
>   kvargs: detailed definition of callback prototype
>   compressdev: fix segment fault when parse input args
>   compressdev: fix null name when parse input args
>   cryptodev: fix segment fault when parse input args
>   cryptodev: fix null name when parse input args
>   net/hns3: fix segment fault when parse runtime config
>   net/virtio: fix segment fault when parse devargs
>   dma/skeleton: fix segment fault when parse devargs
>   raw/skeleton: fix segment fault when parse devargs
> 
>  drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
>  drivers/net/hns3/hns3_common.c         |  9 +++++++++
>  drivers/net/virtio/virtio_ethdev.c     |  3 +++
>  drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
>  drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
>  lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
>  lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
>  lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
>  8 files changed, 50 insertions(+), 2 deletions(-)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [EXT] [PATCH 4/9] cryptodev: fix segment fault when parse input args
  2023-03-02  8:11   ` [EXT] " Akhil Goyal
@ 2023-03-02  9:21     ` fengchengwen
  0 siblings, 0 replies; 18+ messages in thread
From: fengchengwen @ 2023-03-02  9:21 UTC (permalink / raw)
  To: Akhil Goyal, thomas, ferruh.yigit, Fan Zhang, Declan Doherty,
	Pablo de Lara, Fiona Trahe
  Cc: dev

On 2023/3/2 16:11, Akhil Goyal wrote:
>> The rte_kvargs_process() was used to parse KV pairs, it also supports
>> to parse 'only keys' (e.g. socket_id) type. And the callback function
>> parameter 'value' is NULL when parsed 'only keys'.
>>
>> This patch fixes segment fault when parse input args with 'only keys'
>> (e.g. socket_id,max_nb_queue_pairs).
>>
>> Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> 
> I would say, you can squash your 4/9 and 5/9 patch in a single commit.
> As you are doing the same thing in both the patches.

The cause of the problem is the same but the symptom is different, so I split two commit.

It's okay to squash.

> .
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 6/9] net/hns3: fix segment fault when parse runtime config
  2023-03-02  7:50 ` [PATCH 6/9] net/hns3: fix segment fault when parse runtime config Chengwen Feng
@ 2023-03-08  7:37   ` Dongdong Liu
  0 siblings, 0 replies; 18+ messages in thread
From: Dongdong Liu @ 2023-03-08  7:37 UTC (permalink / raw)
  To: Chengwen Feng, thomas, ferruh.yigit, Yisen Zhuang,
	Min Hu (Connor),
	Chengchang Tang
  Cc: dev


On 2023/3/2 15:50, Chengwen Feng wrote:
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
>
> This patch fixes segment fault when parse input args with 'only keys'
> (e.g. rx_func_hint).
>
> Fixes: a124f9e9591b ("net/hns3: add runtime config to select IO burst function")
> Fixes: 70791213242e ("net/hns3: support masking device capability")
> Fixes: 2fc3e696a7f1 ("net/hns3: add runtime config for mailbox limit time")
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

Acked-by: Dongdong Liu <liudongdong3@huawei.com>

Thanks,
Dongdong

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/9] kvargs: detailed definition of callback prototype
  2023-03-02  7:50 ` [PATCH 1/9] kvargs: detailed definition of callback prototype Chengwen Feng
@ 2023-03-09 13:12   ` Olivier Matz
  0 siblings, 0 replies; 18+ messages in thread
From: Olivier Matz @ 2023-03-09 13:12 UTC (permalink / raw)
  To: Chengwen Feng; +Cc: thomas, ferruh.yigit, dev

Hi Chengwen,

The patch looks good to me.
Please find below few minor style comments.

On Thu, Mar 02, 2023 at 07:50:04AM +0000, Chengwen Feng wrote:
> [PATCH 1/9] kvargs: detailed definition of callback prototype

kvargs: add API documentation for process callback

>
> The rte_kvargs_process() was used to parse KV pairs, it also supports

was -> is

> to parse 'only keys' (e.g. socket_id) type. And the callback function
> (which prototype is arg_handler_t) parameter 'value' is NULL when
> parsed 'only keys'.

parsed -> parsing

> 
> But where there is no detailed definition of 'value' maybe NULL, so

-where

maybe -> may be

> this patch adds it.
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

With this:
Acked-by: Olivier Matz <zer0@droids-corp.org>

Thanks!

> ---
>  lib/kvargs/rte_kvargs.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kvargs/rte_kvargs.h b/lib/kvargs/rte_kvargs.h
> index 359a9f5b09..4900b750bc 100644
> --- a/lib/kvargs/rte_kvargs.h
> +++ b/lib/kvargs/rte_kvargs.h
> @@ -36,7 +36,19 @@ extern "C" {
>  /** separator character used between key and value */
>  #define RTE_KVARGS_KV_DELIM	"="
>  
> -/** Type of callback function used by rte_kvargs_process() */
> +/**
> + * Callback prototype used by rte_kvargs_process().
> + *
> + * @param key
> + *   The key to consider, it will not be NULL.
> + * @param value
> + *   The value corresponding to the key, it may be NULL (e.g. only with key)
> + * @param opaque
> + *   An opaque pointer coming from the caller.
> + * @return
> + *   - >=0 handle key success.
> + *   - <0 on error.
> + */
>  typedef int (*arg_handler_t)(const char *key, const char *value, void *opaque);
>  
>  /** A key/value association */
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/9] fix kvargs callback prototype not clearly defined
  2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
                   ` (9 preceding siblings ...)
  2023-03-02  9:17 ` [PATCH 0/9] fix kvargs callback prototype not clearly defined fengchengwen
@ 2023-03-09 15:19 ` David Marchand
  10 siblings, 0 replies; 18+ messages in thread
From: David Marchand @ 2023-03-09 15:19 UTC (permalink / raw)
  To: Chengwen Feng; +Cc: thomas, ferruh.yigit, dev

On Thu, Mar 2, 2023 at 8:56 AM Chengwen Feng <fengchengwen@huawei.com> wrote:
>
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
>
> But where there is no detailed definition of 'value' maybe NULL. this
> leads to a lot of processing errors (some may cause segment errors).
> This patchset fixes some of them.
>
> Chengwen Feng (9):
>   kvargs: detailed definition of callback prototype
>   compressdev: fix segment fault when parse input args
>   compressdev: fix null name when parse input args
>   cryptodev: fix segment fault when parse input args
>   cryptodev: fix null name when parse input args
>   net/hns3: fix segment fault when parse runtime config
>   net/virtio: fix segment fault when parse devargs
>   dma/skeleton: fix segment fault when parse devargs
>   raw/skeleton: fix segment fault when parse devargs
>
>  drivers/dma/skeleton/skeleton_dmadev.c |  8 +++++++-
>  drivers/net/hns3/hns3_common.c         |  9 +++++++++
>  drivers/net/virtio/virtio_ethdev.c     |  3 +++
>  drivers/net/virtio/virtio_pci_ethdev.c |  3 +++
>  drivers/raw/skeleton/skeleton_rawdev.c |  2 ++
>  lib/compressdev/rte_compressdev_pmd.c  |  6 ++++++
>  lib/cryptodev/cryptodev_pmd.c          |  7 +++++++
>  lib/kvargs/rte_kvargs.h                | 14 +++++++++++++-
>  8 files changed, 50 insertions(+), 2 deletions(-)

Series applied, thanks Chengwen.

-- 
David Marchand


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 7/9] net/virtio: fix segment fault when parse devargs
  2023-03-02  7:50 ` [PATCH 7/9] net/virtio: fix segment fault when parse devargs Chengwen Feng
@ 2023-03-09 15:21   ` Maxime Coquelin
  0 siblings, 0 replies; 18+ messages in thread
From: Maxime Coquelin @ 2023-03-09 15:21 UTC (permalink / raw)
  To: Chengwen Feng, thomas, ferruh.yigit, Chenbo Xia, Yong Liu,
	Ivan Dyukov, Ferruh Yigit, Xiao Wang
  Cc: dev



On 3/2/23 08:50, Chengwen Feng wrote:
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
> 
> This patch fixes segment fault when parse input args with 'only keys'
> (e.g. vectorized,vdpa).
> 
> Fixes: 4710e16a4a7b ("net/virtio: add parameter to enable vectorized path")
> Fixes: 44d7b2e87b69 ("net/virtio: refactor devargs parsing")
> Fixes: 440f03c25378 ("net/virtio: skip device probe in vDPA mode")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>   drivers/net/virtio/virtio_ethdev.c     | 3 +++
>   drivers/net/virtio/virtio_pci_ethdev.c | 3 +++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 0103d95920..d10f42bba2 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -2054,6 +2054,9 @@ virtio_dev_speed_capa_get(uint32_t speed)
>   static int vectorized_check_handler(__rte_unused const char *key,
>   		const char *value, void *ret_val)
>   {
> +	if (value == NULL || ret_val == NULL)
> +		return -EINVAL;
> +
>   	if (strcmp(value, "1") == 0)
>   		*(int *)ret_val = 1;
>   	else
> diff --git a/drivers/net/virtio/virtio_pci_ethdev.c b/drivers/net/virtio/virtio_pci_ethdev.c
> index abc63b0935..9b4b846f8a 100644
> --- a/drivers/net/virtio/virtio_pci_ethdev.c
> +++ b/drivers/net/virtio/virtio_pci_ethdev.c
> @@ -148,6 +148,9 @@ eth_virtio_pci_uninit(struct rte_eth_dev *eth_dev)
>   static int vdpa_check_handler(__rte_unused const char *key,
>   		const char *value, void *ret_val)
>   {
> +	if (value == NULL || ret_val == NULL)
> +		return -EINVAL;
> +
>   	if (strcmp(value, "1") == 0)
>   		*(int *)ret_val = 1;
>   	else

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/9] cryptodev: fix segment fault when parse input args
  2023-03-02  7:48 Chengwen Feng
@ 2023-03-02  7:48 ` Chengwen Feng
  0 siblings, 0 replies; 18+ messages in thread
From: Chengwen Feng @ 2023-03-02  7:48 UTC (permalink / raw)
  To: thomas, ferruh.yigit; +Cc: dev

The rte_kvargs_process() was used to parse KV pairs, it also supports
to parse 'only keys' (e.g. socket_id) type. And the callback function
parameter 'value' is NULL when parsed 'only keys'.

This patch fixes segment fault when parse input args with 'only keys'
(e.g. socket_id,max_nb_queue_pairs).

Fixes: 9e6edea41805 ("cryptodev: add APIs to assist PMD initialisation")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/cryptodev/cryptodev_pmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/cryptodev/cryptodev_pmd.c b/lib/cryptodev/cryptodev_pmd.c
index 77b269f312..4122000839 100644
--- a/lib/cryptodev/cryptodev_pmd.c
+++ b/lib/cryptodev/cryptodev_pmd.c
@@ -38,6 +38,10 @@ rte_cryptodev_pmd_parse_uint_arg(const char *key __rte_unused,
 {
 	int i;
 	char *end;
+
+	if (value == NULL || extra_args == NULL)
+		return -EINVAL;
+
 	errno = 0;
 
 	i = strtol(value, &end, 10);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-03-09 15:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-02  7:50 [PATCH 0/9] fix kvargs callback prototype not clearly defined Chengwen Feng
2023-03-02  7:50 ` [PATCH 1/9] kvargs: detailed definition of callback prototype Chengwen Feng
2023-03-09 13:12   ` Olivier Matz
2023-03-02  7:50 ` [PATCH 2/9] compressdev: fix segment fault when parse input args Chengwen Feng
2023-03-02  7:50 ` [PATCH 3/9] compressdev: fix null name " Chengwen Feng
2023-03-02  7:50 ` [PATCH 4/9] cryptodev: fix segment fault " Chengwen Feng
2023-03-02  8:11   ` [EXT] " Akhil Goyal
2023-03-02  9:21     ` fengchengwen
2023-03-02  7:50 ` [PATCH 5/9] cryptodev: fix null name " Chengwen Feng
2023-03-02  7:50 ` [PATCH 6/9] net/hns3: fix segment fault when parse runtime config Chengwen Feng
2023-03-08  7:37   ` Dongdong Liu
2023-03-02  7:50 ` [PATCH 7/9] net/virtio: fix segment fault when parse devargs Chengwen Feng
2023-03-09 15:21   ` Maxime Coquelin
2023-03-02  7:50 ` [PATCH 8/9] dma/skeleton: " Chengwen Feng
2023-03-02  7:50 ` [PATCH 9/9] raw/skeleton: " Chengwen Feng
2023-03-02  9:17 ` [PATCH 0/9] fix kvargs callback prototype not clearly defined fengchengwen
2023-03-09 15:19 ` David Marchand
  -- strict thread matches above, loose matches on Subject: below --
2023-03-02  7:48 Chengwen Feng
2023-03-02  7:48 ` [PATCH 4/9] cryptodev: fix segment fault when parse input args Chengwen Feng

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