Hi Stephen, On Fri, Oct 23, 2020 at 05:43:31PM -0700, Stephen Hemminger wrote: > The dynamic flag management is broken if rte_mbuf_dynflag_lookup() > is done in a secondary process 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.m...@6wind.com > Signed-off-by: Stephen Hemminger <step...@networkplumber.org> > --- > > v3 - change title, fix one extra whitespace > > lib/librte_mbuf/rte_mbuf_dyn.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/lib/librte_mbuf/rte_mbuf_dyn.c b/lib/librte_mbuf/rte_mbuf_dyn.c > index 538a43f6959f..554ec5a1ca4f 100644 > --- a/lib/librte_mbuf/rte_mbuf_dyn.c > +++ b/lib/librte_mbuf/rte_mbuf_dyn.c > @@ -185,13 +185,11 @@ rte_mbuf_dynfield_lookup(const char *name, struct > rte_mbuf_dynfield *params) > { > struct mbuf_dynfield_elt *mbuf_dynfield; > > - if (shm == NULL) { > - rte_errno = ENOENT; > - return -1; > - } > - > rte_mcfg_tailq_read_lock(); > - mbuf_dynfield = __mbuf_dynfield_lookup(name); > + if (shm == NULL && init_shared_mem() < 0) > + mbuf_dynfield = NULL; > + else > + mbuf_dynfield = __mbuf_dynfield_lookup(name); > rte_mcfg_tailq_read_unlock(); > > if (mbuf_dynfield == NULL) { > rte_errno = ENOENT; > return -1;
There is still a small corner case here: on a primary process, init_shared_mem() can return -1 in case rte_memzone_reserve_aligned() returns a NULL memzone. In this situation, rte_errno is set by the memzone layer by overriden to ENOENT. Maybe something like this is better, what do you think? @@ -172,7 +172,7 @@ __mbuf_dynfield_lookup(const char *name) break; } - if (te == NULL) { + if (te == NULL || mbuf_dynfield == NULL) { rte_errno = ENOENT; return NULL; } @@ -185,19 +185,15 @@ rte_mbuf_dynfield_lookup(const char *name, struct rte_mbuf_dynfield *params) { struct mbuf_dynfield_elt *mbuf_dynfield; - if (shm == NULL) { - rte_errno = ENOENT; - return -1; - } - rte_mcfg_tailq_read_lock(); - mbuf_dynfield = __mbuf_dynfield_lookup(name); + if (shm == NULL && init_shared_mem() < 0) + mbuf_dynfield = NULL; + else + mbuf_dynfield = __mbuf_dynfield_lookup(name); rte_mcfg_tailq_read_unlock(); - if (mbuf_dynfield == NULL) { - rte_errno = ENOENT; + if (mbuf_dynfield == NULL) return -1; - } if (params != NULL) memcpy(params, &mbuf_dynfield->params, sizeof(*params)); Thanks, Olivier > @@ -384,13 +382,11 @@ rte_mbuf_dynflag_lookup(const char *name, > { > struct mbuf_dynflag_elt *mbuf_dynflag; > > - if (shm == NULL) { > - rte_errno = ENOENT; > - return -1; > - } > - > rte_mcfg_tailq_read_lock(); > - mbuf_dynflag = __mbuf_dynflag_lookup(name); > + if (shm == NULL && init_shared_mem() < 0) > + mbuf_dynflag = NULL; > + else > + mbuf_dynflag = __mbuf_dynflag_lookup(name); > rte_mcfg_tailq_read_unlock(); > > if (mbuf_dynflag == NULL) { > -- > 2.27.0 >