DPDK patches and discussions
 help / color / 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
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ 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] 6+ 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
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ 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] 6+ 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
  2020-02-12 23:08 ` [dpdk-dev] [RFC 4/4] cfgfile: check flags value Stephen Hemminger
  3 siblings, 0 replies; 6+ 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] 6+ 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
  3 siblings, 0 replies; 6+ 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] 6+ 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
  3 siblings, 0 replies; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ 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

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox