mbuf: enforce no option for dynamic fields and flags

Message ID 20211012193957.21972-1-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series mbuf: enforce no option for dynamic fields and flags |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

David Marchand Oct. 12, 2021, 7:39 p.m. UTC
  As stated in the API, dynamic field and flags should be created with no
additional flag (simply in the API for future changes).

Fix the dynamic flag register helper which was not enforcing it and add
unit tests.

Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 app/test/test_mbuf.c    | 18 ++++++++++++++++++
 lib/mbuf/rte_mbuf_dyn.c |  4 ++++
 2 files changed, 22 insertions(+)
  

Comments

Stephen Hemminger Oct. 12, 2021, 8:14 p.m. UTC | #1
On Tue, 12 Oct 2021 21:39:57 +0200
David Marchand <david.marchand@redhat.com> wrote:

> As stated in the API, dynamic field and flags should be created with no
> additional flag (simply in the API for future changes).
> 
> Fix the dynamic flag register helper which was not enforcing it and add
> unit tests.
> 
> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>
  
Andrew Rybchenko Oct. 13, 2021, 7:06 a.m. UTC | #2
On 10/12/21 11:14 PM, Stephen Hemminger wrote:
> On Tue, 12 Oct 2021 21:39:57 +0200
> David Marchand <david.marchand@redhat.com> wrote:
> 
>> As stated in the API, dynamic field and flags should be created with no
>> additional flag (simply in the API for future changes).
>>
>> Fix the dynamic flag register helper which was not enforcing it and add
>> unit tests.
>>
>> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
> 
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 

Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
  
Ray Kinsella Oct. 13, 2021, 8:37 a.m. UTC | #3
On 12/10/2021 20:39, David Marchand wrote:
> As stated in the API, dynamic field and flags should be created with no
> additional flag (simply in the API for future changes).
> 
> Fix the dynamic flag register helper which was not enforcing it and add
> unit tests.
> 
> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  app/test/test_mbuf.c    | 18 ++++++++++++++++++
>  lib/mbuf/rte_mbuf_dyn.c |  4 ++++
>  2 files changed, 22 insertions(+)

Acked-by: Ray Kinsella <mdr@ashroe.eu>
  
David Marchand Oct. 15, 2021, 8:28 a.m. UTC | #4
On Wed, Oct 13, 2021 at 9:06 AM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
> On 10/12/21 11:14 PM, Stephen Hemminger wrote:
> > On Tue, 12 Oct 2021 21:39:57 +0200
> > David Marchand <david.marchand@redhat.com> wrote:
> >
> >> As stated in the API, dynamic field and flags should be created with no
> >> additional flag (simply in the API for future changes).
> >>
> >> Fix the dynamic flag register helper which was not enforcing it and add
> >> unit tests.
> >>
> >> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
> >>
> >> Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ray Kinsella <mdr@ashroe.eu>

Applied, thanks.
  

Patch

diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 9a248dfaea..82777109dc 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -2577,6 +2577,16 @@  test_mbuf_dyn(struct rte_mempool *pktmbuf_pool)
 		.align = 3,
 		.flags = 0,
 	};
+	const struct rte_mbuf_dynfield dynfield_fail_flag = {
+		.name = "test-dynfield",
+		.size = sizeof(uint8_t),
+		.align = __alignof__(uint8_t),
+		.flags = 1,
+	};
+	const struct rte_mbuf_dynflag dynflag_fail_flag = {
+		.name = "test-dynflag",
+		.flags = 1,
+	};
 	const struct rte_mbuf_dynflag dynflag = {
 		.name = "test-dynflag",
 		.flags = 0,
@@ -2638,6 +2648,14 @@  test_mbuf_dyn(struct rte_mempool *pktmbuf_pool)
 	if (ret != -1)
 		GOTO_FAIL("dynamic field creation should fail (not avail)");
 
+	ret = rte_mbuf_dynfield_register(&dynfield_fail_flag);
+	if (ret != -1)
+		GOTO_FAIL("dynamic field creation should fail (invalid flag)");
+
+	ret = rte_mbuf_dynflag_register(&dynflag_fail_flag);
+	if (ret != -1)
+		GOTO_FAIL("dynamic flag creation should fail (invalid flag)");
+
 	flag = rte_mbuf_dynflag_register(&dynflag);
 	if (flag == -1)
 		GOTO_FAIL("failed to register dynamic flag, flag=%d: %s",
diff --git a/lib/mbuf/rte_mbuf_dyn.c b/lib/mbuf/rte_mbuf_dyn.c
index ca46eb279e..d55e162a68 100644
--- a/lib/mbuf/rte_mbuf_dyn.c
+++ b/lib/mbuf/rte_mbuf_dyn.c
@@ -498,6 +498,10 @@  rte_mbuf_dynflag_register_bitnum(const struct rte_mbuf_dynflag *params,
 {
 	int ret;
 
+	if (params->flags != 0) {
+		rte_errno = EINVAL;
+		return -1;
+	}
 	if (req >= RTE_SIZEOF_FIELD(struct rte_mbuf, ol_flags) * CHAR_BIT &&
 			req != UINT_MAX) {
 		rte_errno = EINVAL;