mbuf: allow dynamic flags to be used by secondary process

Message ID 20201015172019.3181-1-stephen@networkplumber.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series mbuf: allow dynamic flags to be used by secondary process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Functional success Functional Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Stephen Hemminger Oct. 15, 2020, 5:20 p.m. UTC
  The dynamic flag management is broken for multi-process environment.
All calls to lookup dynamic flags or fields will fail in secondary process.
This is because the local pointer to the memzone is not ever initialized.

Fix it by using the same checks as dynfield_register().
I.e if shared memory zone has not been looked up already,
then discover it.

Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
Cc: olivier.matz@6wind.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_mbuf/rte_mbuf_dyn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

Olivier Matz Oct. 20, 2020, 12:18 p.m. UTC | #1
Hi Stephen,

On Thu, Oct 15, 2020 at 10:20:19AM -0700, Stephen Hemminger wrote:
> mbuf: allow dynamic flags to be used by secondary process

Suggested title:
mbuf: fix dynamic flags lookup from secondary process

>
> The dynamic flag management is broken for multi-process environment.
> All calls to lookup dynamic flags or fields will fail in secondary process.
> This is because the local pointer to the memzone is not ever initialized.
> 
> Fix it by using the same checks as dynfield_register().
> I.e if shared memory zone has not been looked up already,
> then discover it.
> 
> Fixes: 4958ca3a443a ("mbuf: support dynamic fields and flags")
> Cc: olivier.matz@6wind.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Good catch, please Cc stable for v2.

> ---
>  lib/librte_mbuf/rte_mbuf_dyn.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
> index 538a43f6959f..ac4801b4d770 100644
> --- a/lib/librte_mbuf/rte_mbuf_dyn.c
> +++ b/lib/librte_mbuf/rte_mbuf_dyn.c
> @@ -185,7 +185,7 @@ rte_mbuf_dynfield_lookup(const char *name, struct rte_mbuf_dynfield *params)
>  {
>  	struct mbuf_dynfield_elt *mbuf_dynfield;
>  
> -	if (shm == NULL) {
> +	if (shm == NULL && init_shared_mem() < 0) {
>  		rte_errno = ENOENT;
>  		return -1;
>  	}

init_shared_mem() is supposed to be called with tailq lock held.

I think the test (shm == NULL && init_shared_mem() < 0) should be
moved in __mbuf_dynfield_lookup().

> @@ -384,7 +384,7 @@ rte_mbuf_dynflag_lookup(const char *name,
>  {
>  	struct mbuf_dynflag_elt *mbuf_dynflag;
>  
> -	if (shm == NULL) {
> +	if (shm == NULL && init_shared_mem() < 0) {
>  		rte_errno = ENOENT;
>  		return -1;
>  	}

Same here


Thanks,
Olivier
  

Patch

diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c
index 538a43f6959f..ac4801b4d770 100644
--- a/lib/librte_mbuf/rte_mbuf_dyn.c
+++ b/lib/librte_mbuf/rte_mbuf_dyn.c
@@ -185,7 +185,7 @@  rte_mbuf_dynfield_lookup(const char *name, struct rte_mbuf_dynfield *params)
 {
 	struct mbuf_dynfield_elt *mbuf_dynfield;
 
-	if (shm == NULL) {
+	if (shm == NULL && init_shared_mem() < 0) {
 		rte_errno = ENOENT;
 		return -1;
 	}
@@ -384,7 +384,7 @@  rte_mbuf_dynflag_lookup(const char *name,
 {
 	struct mbuf_dynflag_elt *mbuf_dynflag;
 
-	if (shm == NULL) {
+	if (shm == NULL && init_shared_mem() < 0) {
 		rte_errno = ENOENT;
 		return -1;
 	}