DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC 0/4] Enforce flag checking in API's
@ 2020-02-12 23:08 Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 1/4] ring: future proof flag settings Stephen Hemminger
                   ` (5 more replies)
  0 siblings, 6 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-02-12 23:08 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Since the DPDK api's are lazy about checking for flag values
it means the flags can never be extended in future releases
without causing ABI breakage. This means we end up doing unnecessary
symbol versioning just to work around applications that might
pass in naughty bits.

This is the DPDK analog of the Linux kernel openat() problem.
Openat api was added but since kernel did not check flags it
ended up that another syscall openat2() was necessary before
the flags could be used.

Stephen Hemminger (4):
  ring: future proof flag settings
  hash: check flags on creation
  stack: check flags on creation
  cfgfile: check flags value

 lib/librte_cfgfile/rte_cfgfile.c  | 4 ++++
 lib/librte_hash/rte_cuckoo_hash.c | 9 +++++++++
 lib/librte_ring/rte_ring.c        | 7 +++++++
 lib/librte_stack/rte_stack.c      | 5 +++++
 4 files changed, 25 insertions(+)

-- 
2.20.1


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

* [dpdk-dev] [RFC 1/4] ring: future proof flag settings
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
@ 2020-02-12 23:08 ` Stephen Hemminger
  2020-02-13 11:12   ` Ananyev, Konstantin
  2020-02-12 23:08 ` [dpdk-dev] [RFC 2/4] hash: check flags on creation Stephen Hemminger
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-02-12 23:08 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ring/rte_ring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index 77e5de099b81..6cd8831649ea 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -100,6 +100,13 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
 	RTE_BUILD_BUG_ON((offsetof(struct rte_ring, prod) &
 			  RTE_CACHE_LINE_MASK) != 0);
 
+	/* future proof flags, only allow supported values */
+	if (flags & ~(RING_F_SP_ENQ | RING_F_SC_DEQ)) {
+		RTE_LOG(ERR, RING,
+			"Unsupported flags requested %d\n", flags);
+		return -EINVAL;
+	}
+		
 	/* init the ring structure */
 	memset(r, 0, sizeof(*r));
 	ret = strlcpy(r->name, name, sizeof(r->name));
-- 
2.20.1


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

* [dpdk-dev] [RFC 2/4] hash: check flags on creation
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-02-12 23:08 ` Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 3/4] stack: " Stephen Hemminger
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-02-12 23:08 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_hash/rte_cuckoo_hash.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 6af8ca42e94f..ab9cd91b4955 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -32,6 +32,9 @@
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
 
+/* Mask of all flags supported by this version */
+#define RTE_HASH_EXTRA_FLAGS_MASK 0x2f
+
 #define FOR_EACH_BUCKET(CURRENT_BKT, START_BUCKET)                            \
 	for (CURRENT_BKT = START_BUCKET;                                      \
 		CURRENT_BKT != NULL;                                          \
@@ -164,6 +167,12 @@ rte_hash_create(const struct rte_hash_parameters *params)
 		return NULL;
 	}
 
+	if (params->extra_flag & ~RTE_HASH_EXTRA_FLAGS_MASK) {
+		rte_errno = EINVAL;
+		RTE_LOG(ERR, HASH, "rte_hash_create: unsupported extra flags\n");
+		return NULL;
+	}
+
 	/* Validate correct usage of extra options */
 	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) &&
 	    (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)) {
-- 
2.20.1


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

* [dpdk-dev] [RFC 3/4] stack: check flags on creation
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 1/4] ring: future proof flag settings Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 2/4] hash: check flags on creation Stephen Hemminger
@ 2020-02-12 23:08 ` Stephen Hemminger
  2020-02-12 23:08 ` [dpdk-dev] [RFC 4/4] cfgfile: check flags value Stephen Hemminger
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-02-12 23:08 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_stack/rte_stack.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_stack/rte_stack.c b/lib/librte_stack/rte_stack.c
index d19824f00439..4b09ff30d6bd 100644
--- a/lib/librte_stack/rte_stack.c
+++ b/lib/librte_stack/rte_stack.c
@@ -59,6 +59,11 @@ rte_stack_create(const char *name, unsigned int count, int socket_id,
 	unsigned int sz;
 	int ret;
 
+	if (flags & ~(RTE_STACK_F_LF)) {
+		STACK_LOG_ERR("Unsupported stack flags %#x\n", flags);
+		return NULL;
+	}
+	
 #ifdef RTE_ARCH_64
 	RTE_BUILD_BUG_ON(sizeof(struct rte_stack_lf_head) != 16);
 #else
-- 
2.20.1


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

* [dpdk-dev] [RFC 4/4] cfgfile: check flags value
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
                   ` (2 preceding siblings ...)
  2020-02-12 23:08 ` [dpdk-dev] [RFC 3/4] stack: " Stephen Hemminger
@ 2020-02-12 23:08 ` Stephen Hemminger
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
  5 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-02-12 23:08 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 9049fd9c2319..cec82c82ee20 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -272,6 +272,10 @@ rte_cfgfile_create(int flags)
 	int i;
 	struct rte_cfgfile *cfg;
 
+	/* future proof flags usage */
+	if (flags & ~(CFG_FLAG_GLOBAL_SECTION | CFG_FLAG_EMPTY_VALUES))
+		return NULL;
+	
 	cfg = malloc(sizeof(*cfg));
 
 	if (cfg == NULL)
-- 
2.20.1


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

* Re: [dpdk-dev] [RFC 1/4] ring: future proof flag settings
  2020-02-12 23:08 ` [dpdk-dev] [RFC 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-02-13 11:12   ` Ananyev, Konstantin
  0 siblings, 0 replies; 26+ messages in thread
From: Ananyev, Konstantin @ 2020-02-13 11:12 UTC (permalink / raw)
  To: Stephen Hemminger, dev



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Wednesday, February 12, 2020 11:08 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Subject: [dpdk-dev] [RFC 1/4] ring: future proof flag settings
> 
> All API's should check that they support the flag values
> passed. If an application passes an invalid flag it could
> cause problems in later ABI.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_ring/rte_ring.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
> index 77e5de099b81..6cd8831649ea 100644
> --- a/lib/librte_ring/rte_ring.c
> +++ b/lib/librte_ring/rte_ring.c
> @@ -100,6 +100,13 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
>  	RTE_BUILD_BUG_ON((offsetof(struct rte_ring, prod) &
>  			  RTE_CACHE_LINE_MASK) != 0);
> 
> +	/* future proof flags, only allow supported values */
> +	if (flags & ~(RING_F_SP_ENQ | RING_F_SC_DEQ)) {
> +		RTE_LOG(ERR, RING,
> +			"Unsupported flags requested %d\n", flags);
> +		return -EINVAL;
> +	}
> +

Good idea, I think, but seems you forgot RING_F_EXACT_SZ.

>  	/* init the ring structure */
>  	memset(r, 0, sizeof(*r));
>  	ret = strlcpy(r->name, name, sizeof(r->name));
> --
> 2.20.1


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

* [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
                   ` (3 preceding siblings ...)
  2020-02-12 23:08 ` [dpdk-dev] [RFC 4/4] cfgfile: check flags value Stephen Hemminger
@ 2020-04-24 17:15 ` Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings Stephen Hemminger
                     ` (3 more replies)
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
  5 siblings, 4 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 17:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

Since the DPDK api's are lazy about checking for flag values
it means the flags can never be extended in future releases
without causing ABI breakage. This means we end up doing unnecessary
symbol versioning just to work around applications that might
pass in naughty bits.

This is the DPDK analog of the Linux kernel openat() problem.
Openat api was added but since kernel did not check flags it
ended up that another syscall openat2() was necessary before
the flags could be used.

v2 - rebase and fix checkpatch warnings from RFC

Stephen Hemminger (4):
  ring: future proof flag settings
  hash: check flags on creation
  stack: check flags on creation
  cfgfile: check flags value

 lib/librte_cfgfile/rte_cfgfile.c  |  4 ++++
 lib/librte_hash/rte_cuckoo_hash.c |  9 +++++++++
 lib/librte_ring/rte_ring.c        | 10 ++++++++++
 lib/librte_stack/rte_stack.c      |  5 +++++
 4 files changed, 28 insertions(+)

-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
@ 2020-04-24 17:15   ` Stephen Hemminger
  2020-04-24 18:07     ` Honnappa Nagarahalli
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 2/4] hash: check flags on creation Stephen Hemminger
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 17:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values passed.
These checks ensure that the extra bits can safely be used
without risk of ABI breakage.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ring/rte_ring.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index ebe5ccf0de68..70685121581f 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = {
 };
 EAL_REGISTER_TAILQ(rte_ring_tailq)
 
+/* mask of all valid flag values to ring_create() */
+#define RING_F_MASK	0x007F
+
 /* true if x is a power of 2 */
 #define POWEROF2(x) ((((x)-1) & (x)) == 0)
 
@@ -197,6 +200,13 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
 	RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
 		offsetof(struct rte_ring_rts_headtail, tail.val.pos));
 
+	/* future proof flags, only allow supported values */
+	if (flags & ~RING_F_MASK) {
+		RTE_LOG(ERR, RING,
+			"Unsupported flags requested %#x\n", flags);
+		return -EINVAL;
+	}
+
 	/* init the ring structure */
 	memset(r, 0, sizeof(*r));
 	ret = strlcpy(r->name, name, sizeof(r->name));
-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 2/4] hash: check flags on creation
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-04-24 17:15   ` Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 3/4] stack: " Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 4/4] cfgfile: check flags value Stephen Hemminger
  3 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 17:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_hash/rte_cuckoo_hash.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 6af8ca42e94f..ab9cd91b4955 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -32,6 +32,9 @@
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
 
+/* Mask of all flags supported by this version */
+#define RTE_HASH_EXTRA_FLAGS_MASK 0x2f
+
 #define FOR_EACH_BUCKET(CURRENT_BKT, START_BUCKET)                            \
 	for (CURRENT_BKT = START_BUCKET;                                      \
 		CURRENT_BKT != NULL;                                          \
@@ -164,6 +167,12 @@ rte_hash_create(const struct rte_hash_parameters *params)
 		return NULL;
 	}
 
+	if (params->extra_flag & ~RTE_HASH_EXTRA_FLAGS_MASK) {
+		rte_errno = EINVAL;
+		RTE_LOG(ERR, HASH, "rte_hash_create: unsupported extra flags\n");
+		return NULL;
+	}
+
 	/* Validate correct usage of extra options */
 	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) &&
 	    (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)) {
-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 3/4] stack: check flags on creation
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 2/4] hash: check flags on creation Stephen Hemminger
@ 2020-04-24 17:15   ` Stephen Hemminger
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 4/4] cfgfile: check flags value Stephen Hemminger
  3 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 17:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_stack/rte_stack.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_stack/rte_stack.c b/lib/librte_stack/rte_stack.c
index d19824f00439..e58fa545fca4 100644
--- a/lib/librte_stack/rte_stack.c
+++ b/lib/librte_stack/rte_stack.c
@@ -59,6 +59,11 @@ rte_stack_create(const char *name, unsigned int count, int socket_id,
 	unsigned int sz;
 	int ret;
 
+	if (flags & ~(RTE_STACK_F_LF)) {
+		STACK_LOG_ERR("Unsupported stack flags %#x\n", flags);
+		return NULL;
+	}
+
 #ifdef RTE_ARCH_64
 	RTE_BUILD_BUG_ON(sizeof(struct rte_stack_lf_head) != 16);
 #else
-- 
2.20.1


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

* [dpdk-dev] [PATCH v2 4/4] cfgfile: check flags value
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 3/4] stack: " Stephen Hemminger
@ 2020-04-24 17:15   ` Stephen Hemminger
  3 siblings, 0 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 17:15 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 9049fd9c2319..714717dd9007 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -272,6 +272,10 @@ rte_cfgfile_create(int flags)
 	int i;
 	struct rte_cfgfile *cfg;
 
+	/* future proof flags usage */
+	if (flags & ~(CFG_FLAG_GLOBAL_SECTION | CFG_FLAG_EMPTY_VALUES))
+		return NULL;
+
 	cfg = malloc(sizeof(*cfg));
 
 	if (cfg == NULL)
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
  2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-04-24 18:07     ` Honnappa Nagarahalli
  2020-04-24 19:02       ` Stephen Hemminger
  0 siblings, 1 reply; 26+ messages in thread
From: Honnappa Nagarahalli @ 2020-04-24 18:07 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: nd, Honnappa Nagarahalli, nd

<snip>

> 
> All API's should check that they support the flag values passed.
> These checks ensure that the extra bits can safely be used without risk of ABI
> breakage.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
>  lib/librte_ring/rte_ring.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> ebe5ccf0de68..70685121581f 100644
> --- a/lib/librte_ring/rte_ring.c
> +++ b/lib/librte_ring/rte_ring.c
> @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = {  };
>  EAL_REGISTER_TAILQ(rte_ring_tailq)
> 
> +/* mask of all valid flag values to ring_create() */
> +#define RING_F_MASK	0x007F
Is it better to construct this using the actual flag #defines?

> +
>  /* true if x is a power of 2 */
>  #define POWEROF2(x) ((((x)-1) & (x)) == 0)
> 
> @@ -197,6 +200,13 @@ rte_ring_init(struct rte_ring *r, const char *name,
> unsigned count,
>  	RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
>  		offsetof(struct rte_ring_rts_headtail, tail.val.pos));
> 
> +	/* future proof flags, only allow supported values */
> +	if (flags & ~RING_F_MASK) {
> +		RTE_LOG(ERR, RING,
> +			"Unsupported flags requested %#x\n", flags);
> +		return -EINVAL;
> +	}
> +
>  	/* init the ring structure */
>  	memset(r, 0, sizeof(*r));
>  	ret = strlcpy(r->name, name, sizeof(r->name));
> --
> 2.20.1


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

* Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
  2020-04-24 18:07     ` Honnappa Nagarahalli
@ 2020-04-24 19:02       ` Stephen Hemminger
  2020-04-25  9:20         ` Ananyev, Konstantin
  0 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-24 19:02 UTC (permalink / raw)
  To: Honnappa Nagarahalli; +Cc: dev, nd

On Fri, 24 Apr 2020 18:07:15 +0000
Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:

> <snip>
> 
> > 
> > All API's should check that they support the flag values passed.
> > These checks ensure that the extra bits can safely be used without risk of ABI
> > breakage.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> >  lib/librte_ring/rte_ring.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> > ebe5ccf0de68..70685121581f 100644
> > --- a/lib/librte_ring/rte_ring.c
> > +++ b/lib/librte_ring/rte_ring.c
> > @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = {  };
> >  EAL_REGISTER_TAILQ(rte_ring_tailq)
> > 
> > +/* mask of all valid flag values to ring_create() */
> > +#define RING_F_MASK	0x007F  
> Is it better to construct this using the actual flag #defines?

sure, but it gets long


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

* Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
  2020-04-24 19:02       ` Stephen Hemminger
@ 2020-04-25  9:20         ` Ananyev, Konstantin
  0 siblings, 0 replies; 26+ messages in thread
From: Ananyev, Konstantin @ 2020-04-25  9:20 UTC (permalink / raw)
  To: Stephen Hemminger, Honnappa Nagarahalli; +Cc: dev, nd



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Stephen Hemminger
> Sent: Friday, April 24, 2020 8:02 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings
> 
> On Fri, 24 Apr 2020 18:07:15 +0000
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com> wrote:
> 
> > <snip>
> >
> > >
> > > All API's should check that they support the flag values passed.
> > > These checks ensure that the extra bits can safely be used without risk of ABI
> > > breakage.
> > >
> > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > > ---
> > >  lib/librte_ring/rte_ring.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > >
> > > diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> > > ebe5ccf0de68..70685121581f 100644
> > > --- a/lib/librte_ring/rte_ring.c
> > > +++ b/lib/librte_ring/rte_ring.c
> > > @@ -42,6 +42,9 @@ static struct rte_tailq_elem rte_ring_tailq = {  };
> > >  EAL_REGISTER_TAILQ(rte_ring_tailq)
> > >
> > > +/* mask of all valid flag values to ring_create() */
> > > +#define RING_F_MASK	0x007F
> > Is it better to construct this using the actual flag #defines?
> 
> sure, but it gets long

+1 to use public defines here.



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

* [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's
  2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
                   ` (4 preceding siblings ...)
  2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
@ 2020-04-27 23:16 ` Stephen Hemminger
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings Stephen Hemminger
                     ` (5 more replies)
  5 siblings, 6 replies; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger

The DPDK API's are lax about checking for undefined flag values.
This makes it impossible to add new bits to existing API's
without causing ABI breakage. This means we end up doing unnecessary
symbol versioning just to work around applications that might
pass in naughty bits.

This is the DPDK analog of the Linux kernel openat() problem.
Openat api was added but since kernel did not check flags it
ended up that another syscall openat2() was necessary before
the flags could be used.

v3 - define mask based on existing defines for ring and hash

Stephen Hemminger (4):
  ring: future proof flag settings
  hash: check flags on creation
  stack: check flags on creation
  cfgfile: check flags value

 lib/librte_cfgfile/rte_cfgfile.c  |  4 ++++
 lib/librte_hash/rte_cuckoo_hash.c | 14 ++++++++++++++
 lib/librte_ring/rte_ring.c        | 12 ++++++++++++
 lib/librte_stack/rte_stack.c      |  5 +++++
 4 files changed, 35 insertions(+)

-- 
2.20.1


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

* [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
@ 2020-04-27 23:16   ` Stephen Hemminger
  2020-04-28  4:12     ` Honnappa Nagarahalli
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation Stephen Hemminger
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Honnappa Nagarahalli, Konstantin Ananyev

All API's should check that they support the flag values passed.
These checks ensure that the extra bits can safely be used
without risk of ABI breakage.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_ring/rte_ring.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c
index ebe5ccf0de68..e16dd45a82de 100644
--- a/lib/librte_ring/rte_ring.c
+++ b/lib/librte_ring/rte_ring.c
@@ -42,6 +42,11 @@ static struct rte_tailq_elem rte_ring_tailq = {
 };
 EAL_REGISTER_TAILQ(rte_ring_tailq)
 
+/* mask of all valid flag values to ring_create() */
+#define RING_F_MASK (RING_F_SP_ENQ | RING_F_SC_DEQ | RING_F_EXACT_SZ | \
+		     RING_F_MP_RTS_ENQ | RING_F_MC_RTS_DEQ |	       \
+		     RING_F_MP_HTS_ENQ | RING_F_MC_HTS_DEQ )
+
 /* true if x is a power of 2 */
 #define POWEROF2(x) ((((x)-1) & (x)) == 0)
 
@@ -197,6 +202,13 @@ rte_ring_init(struct rte_ring *r, const char *name, unsigned count,
 	RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
 		offsetof(struct rte_ring_rts_headtail, tail.val.pos));
 
+	/* future proof flags, only allow supported values */
+	if (flags & ~RING_F_MASK) {
+		RTE_LOG(ERR, RING,
+			"Unsupported flags requested %#x\n", flags);
+		return -EINVAL;
+	}
+
 	/* init the ring structure */
 	memset(r, 0, sizeof(*r));
 	ret = strlcpy(r->name, name, sizeof(r->name));
-- 
2.20.1


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

* [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-04-27 23:16   ` Stephen Hemminger
  2020-04-29  1:08     ` Wang, Yipeng1
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 3/4] stack: " Stephen Hemminger
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Yipeng Wang, Sameh Gobriel, Bruce Richardson

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_hash/rte_cuckoo_hash.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 38767a8a164f..d54bbb47edcb 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -32,6 +32,14 @@
 #include "rte_hash.h"
 #include "rte_cuckoo_hash.h"
 
+/* Mask of all flags supported by this version */
+#define RTE_HASH_EXTRA_FLAGS_MASK (RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | \
+				   RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD | \
+				   RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | \
+				   RTE_HASH_EXTRA_FLAGS_EXT_TABLE |	\
+				   RTE_HASH_EXTRA_FLAGS_NO_FREE_ON_DEL | \
+				   RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)
+
 #define FOR_EACH_BUCKET(CURRENT_BKT, START_BUCKET)                            \
 	for (CURRENT_BKT = START_BUCKET;                                      \
 		CURRENT_BKT != NULL;                                          \
@@ -164,6 +172,12 @@ rte_hash_create(const struct rte_hash_parameters *params)
 		return NULL;
 	}
 
+	if (params->extra_flag & ~RTE_HASH_EXTRA_FLAGS_MASK) {
+		rte_errno = EINVAL;
+		RTE_LOG(ERR, HASH, "rte_hash_create: unsupported extra flags\n");
+		return NULL;
+	}
+
 	/* Validate correct usage of extra options */
 	if ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) &&
 	    (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)) {
-- 
2.20.1


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

* [dpdk-dev] [PATCH v3 3/4] stack: check flags on creation
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings Stephen Hemminger
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation Stephen Hemminger
@ 2020-04-27 23:16   ` Stephen Hemminger
  2020-05-02  2:36     ` Eads, Gage
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value Stephen Hemminger
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Gage Eads, Olivier Matz

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_stack/rte_stack.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/librte_stack/rte_stack.c b/lib/librte_stack/rte_stack.c
index d19824f00439..e58fa545fca4 100644
--- a/lib/librte_stack/rte_stack.c
+++ b/lib/librte_stack/rte_stack.c
@@ -59,6 +59,11 @@ rte_stack_create(const char *name, unsigned int count, int socket_id,
 	unsigned int sz;
 	int ret;
 
+	if (flags & ~(RTE_STACK_F_LF)) {
+		STACK_LOG_ERR("Unsupported stack flags %#x\n", flags);
+		return NULL;
+	}
+
 #ifdef RTE_ARCH_64
 	RTE_BUILD_BUG_ON(sizeof(struct rte_stack_lf_head) != 16);
 #else
-- 
2.20.1


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

* [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
                     ` (2 preceding siblings ...)
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 3/4] stack: " Stephen Hemminger
@ 2020-04-27 23:16   ` Stephen Hemminger
  2020-04-28 10:24     ` Bruce Richardson
  2020-04-28 10:28   ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Bruce Richardson
  2020-04-28 11:04   ` Ananyev, Konstantin
  5 siblings, 1 reply; 26+ messages in thread
From: Stephen Hemminger @ 2020-04-27 23:16 UTC (permalink / raw)
  To: dev; +Cc: Stephen Hemminger, Cristian Dumitrescu

All API's should check that they support the flag values
passed. If an application passes an invalid flag it could
cause problems in later ABI.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_cfgfile/rte_cfgfile.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/librte_cfgfile/rte_cfgfile.c b/lib/librte_cfgfile/rte_cfgfile.c
index 9049fd9c2319..714717dd9007 100644
--- a/lib/librte_cfgfile/rte_cfgfile.c
+++ b/lib/librte_cfgfile/rte_cfgfile.c
@@ -272,6 +272,10 @@ rte_cfgfile_create(int flags)
 	int i;
 	struct rte_cfgfile *cfg;
 
+	/* future proof flags usage */
+	if (flags & ~(CFG_FLAG_GLOBAL_SECTION | CFG_FLAG_EMPTY_VALUES))
+		return NULL;
+
 	cfg = malloc(sizeof(*cfg));
 
 	if (cfg == NULL)
-- 
2.20.1


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

* Re: [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings Stephen Hemminger
@ 2020-04-28  4:12     ` Honnappa Nagarahalli
  0 siblings, 0 replies; 26+ messages in thread
From: Honnappa Nagarahalli @ 2020-04-28  4:12 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Konstantin Ananyev, nd, Honnappa Nagarahalli, nd

<snip>

> 
> All API's should check that they support the flag values passed.
> These checks ensure that the extra bits can safely be used without risk of ABI
> breakage.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> ---
>  lib/librte_ring/rte_ring.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index
> ebe5ccf0de68..e16dd45a82de 100644
> --- a/lib/librte_ring/rte_ring.c
> +++ b/lib/librte_ring/rte_ring.c
> @@ -42,6 +42,11 @@ static struct rte_tailq_elem rte_ring_tailq = {  };
>  EAL_REGISTER_TAILQ(rte_ring_tailq)
> 
> +/* mask of all valid flag values to ring_create() */ #define
> +RING_F_MASK (RING_F_SP_ENQ | RING_F_SC_DEQ | RING_F_EXACT_SZ | \
> +		     RING_F_MP_RTS_ENQ | RING_F_MC_RTS_DEQ |	       \
> +		     RING_F_MP_HTS_ENQ | RING_F_MC_HTS_DEQ )
> +
>  /* true if x is a power of 2 */
>  #define POWEROF2(x) ((((x)-1) & (x)) == 0)
> 
> @@ -197,6 +202,13 @@ rte_ring_init(struct rte_ring *r, const char *name,
> unsigned count,
>  	RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=
>  		offsetof(struct rte_ring_rts_headtail, tail.val.pos));
> 
> +	/* future proof flags, only allow supported values */
> +	if (flags & ~RING_F_MASK) {
> +		RTE_LOG(ERR, RING,
> +			"Unsupported flags requested %#x\n", flags);
> +		return -EINVAL;
> +	}
> +
>  	/* init the ring structure */
>  	memset(r, 0, sizeof(*r));
>  	ret = strlcpy(r->name, name, sizeof(r->name));
> --
> 2.20.1


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

* Re: [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value Stephen Hemminger
@ 2020-04-28 10:24     ` Bruce Richardson
  0 siblings, 0 replies; 26+ messages in thread
From: Bruce Richardson @ 2020-04-28 10:24 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Cristian Dumitrescu

On Mon, Apr 27, 2020 at 04:16:25PM -0700, Stephen Hemminger wrote:
> All API's should check that they support the flag values
> passed. If an application passes an invalid flag it could
> cause problems in later ABI.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
                     ` (3 preceding siblings ...)
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value Stephen Hemminger
@ 2020-04-28 10:28   ` Bruce Richardson
  2020-06-16 15:47     ` Thomas Monjalon
  2020-04-28 11:04   ` Ananyev, Konstantin
  5 siblings, 1 reply; 26+ messages in thread
From: Bruce Richardson @ 2020-04-28 10:28 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Mon, Apr 27, 2020 at 04:16:21PM -0700, Stephen Hemminger wrote:
> The DPDK API's are lax about checking for undefined flag values.
> This makes it impossible to add new bits to existing API's
> without causing ABI breakage. This means we end up doing unnecessary
> symbol versioning just to work around applications that might
> pass in naughty bits.
> 
> This is the DPDK analog of the Linux kernel openat() problem.
> Openat api was added but since kernel did not check flags it
> ended up that another syscall openat2() was necessary before
> the flags could be used.
> 
> v3 - define mask based on existing defines for ring and hash
> 
> Stephen Hemminger (4):
>   ring: future proof flag settings
>   hash: check flags on creation
>   stack: check flags on creation
>   cfgfile: check flags value
> 
>  lib/librte_cfgfile/rte_cfgfile.c  |  4 ++++
>  lib/librte_hash/rte_cuckoo_hash.c | 14 ++++++++++++++
>  lib/librte_ring/rte_ring.c        | 12 ++++++++++++
>  lib/librte_stack/rte_stack.c      |  5 +++++
>  4 files changed, 35 insertions(+)
> 
I think this is a good idea to do in DPDK

Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's
  2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
                     ` (4 preceding siblings ...)
  2020-04-28 10:28   ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Bruce Richardson
@ 2020-04-28 11:04   ` Ananyev, Konstantin
  5 siblings, 0 replies; 26+ messages in thread
From: Ananyev, Konstantin @ 2020-04-28 11:04 UTC (permalink / raw)
  To: Stephen Hemminger, dev

> The DPDK API's are lax about checking for undefined flag values.
> This makes it impossible to add new bits to existing API's
> without causing ABI breakage. This means we end up doing unnecessary
> symbol versioning just to work around applications that might
> pass in naughty bits.
> 
> This is the DPDK analog of the Linux kernel openat() problem.
> Openat api was added but since kernel did not check flags it
> ended up that another syscall openat2() was necessary before
> the flags could be used.
> 
> v3 - define mask based on existing defines for ring and hash
> 
> Stephen Hemminger (4):
>   ring: future proof flag settings
>   hash: check flags on creation
>   stack: check flags on creation
>   cfgfile: check flags value
> 
>  lib/librte_cfgfile/rte_cfgfile.c  |  4 ++++
>  lib/librte_hash/rte_cuckoo_hash.c | 14 ++++++++++++++
>  lib/librte_ring/rte_ring.c        | 12 ++++++++++++
>  lib/librte_stack/rte_stack.c      |  5 +++++
>  4 files changed, 35 insertions(+)
> 
> --

Series Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.20.1


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

* Re: [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation Stephen Hemminger
@ 2020-04-29  1:08     ` Wang, Yipeng1
  0 siblings, 0 replies; 26+ messages in thread
From: Wang, Yipeng1 @ 2020-04-29  1:08 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Gobriel, Sameh, Richardson, Bruce

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, April 27, 2020 4:16 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH v3 2/4] hash: check flags on creation
> 
> All API's should check that they support the flag values passed. If an application
> passes an invalid flag it could cause problems in later ABI.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 3/4] stack: check flags on creation
  2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 3/4] stack: " Stephen Hemminger
@ 2020-05-02  2:36     ` Eads, Gage
  0 siblings, 0 replies; 26+ messages in thread
From: Eads, Gage @ 2020-05-02  2:36 UTC (permalink / raw)
  To: Stephen Hemminger, dev; +Cc: Olivier Matz

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, April 27, 2020 6:16 PM
> To: dev@dpdk.org
> Cc: Stephen Hemminger <stephen@networkplumber.org>; Eads, Gage
> <gage.eads@intel.com>; Olivier Matz <olivier.matz@6wind.com>
> Subject: [PATCH v3 3/4] stack: check flags on creation
> 
> All API's should check that they support the flag values passed. If an application
> passes an invalid flag it could cause problems in later ABI.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
Acked-by: Gage Eads <gage.eads@intel.com>

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

* Re: [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's
  2020-04-28 10:28   ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Bruce Richardson
@ 2020-06-16 15:47     ` Thomas Monjalon
  0 siblings, 0 replies; 26+ messages in thread
From: Thomas Monjalon @ 2020-06-16 15:47 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Bruce Richardson, konstantin.ananyev

28/04/2020 12:28, Bruce Richardson:
> On Mon, Apr 27, 2020 at 04:16:21PM -0700, Stephen Hemminger wrote:
> > The DPDK API's are lax about checking for undefined flag values.
> > This makes it impossible to add new bits to existing API's
> > without causing ABI breakage. This means we end up doing unnecessary
> > symbol versioning just to work around applications that might
> > pass in naughty bits.
> > 
> > This is the DPDK analog of the Linux kernel openat() problem.
> > Openat api was added but since kernel did not check flags it
> > ended up that another syscall openat2() was necessary before
> > the flags could be used.
> > 
> > v3 - define mask based on existing defines for ring and hash
> > 
> > Stephen Hemminger (4):
> >   ring: future proof flag settings
> >   hash: check flags on creation
> >   stack: check flags on creation
> >   cfgfile: check flags value
> > 
> I think this is a good idea to do in DPDK
> 
> Series-acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks




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

end of thread, other threads:[~2020-06-16 15:47 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-12 23:08 [dpdk-dev] [RFC 0/4] Enforce flag checking in API's Stephen Hemminger
2020-02-12 23:08 ` [dpdk-dev] [RFC 1/4] ring: future proof flag settings Stephen Hemminger
2020-02-13 11:12   ` Ananyev, Konstantin
2020-02-12 23:08 ` [dpdk-dev] [RFC 2/4] hash: check flags on creation Stephen Hemminger
2020-02-12 23:08 ` [dpdk-dev] [RFC 3/4] stack: " Stephen Hemminger
2020-02-12 23:08 ` [dpdk-dev] [RFC 4/4] cfgfile: check flags value Stephen Hemminger
2020-04-24 17:15 ` [dpdk-dev] [PATCH v2 0/4] enforce checking on flag values in API's Stephen Hemminger
2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 1/4] ring: future proof flag settings Stephen Hemminger
2020-04-24 18:07     ` Honnappa Nagarahalli
2020-04-24 19:02       ` Stephen Hemminger
2020-04-25  9:20         ` Ananyev, Konstantin
2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 2/4] hash: check flags on creation Stephen Hemminger
2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 3/4] stack: " Stephen Hemminger
2020-04-24 17:15   ` [dpdk-dev] [PATCH v2 4/4] cfgfile: check flags value Stephen Hemminger
2020-04-27 23:16 ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Stephen Hemminger
2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 1/4] ring: future proof flag settings Stephen Hemminger
2020-04-28  4:12     ` Honnappa Nagarahalli
2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 2/4] hash: check flags on creation Stephen Hemminger
2020-04-29  1:08     ` Wang, Yipeng1
2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 3/4] stack: " Stephen Hemminger
2020-05-02  2:36     ` Eads, Gage
2020-04-27 23:16   ` [dpdk-dev] [PATCH v3 4/4] cfgfile: check flags value Stephen Hemminger
2020-04-28 10:24     ` Bruce Richardson
2020-04-28 10:28   ` [dpdk-dev] [PATCH v3 0/4] Enforce checking on flag values in API's Bruce Richardson
2020-06-16 15:47     ` Thomas Monjalon
2020-04-28 11:04   ` Ananyev, Konstantin

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