* [dpdk-stable] [PATCH v2] mbuf: allow dynamic flags to be used by secondary process [not found] <20201015172019.3181-1-stephen@networkplumber.org> @ 2020-10-20 20:58 ` Stephen Hemminger 2020-10-24 0:43 ` [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from " Stephen Hemminger 1 sibling, 0 replies; 4+ messages in thread From: Stephen Hemminger @ 2020-10-20 20:58 UTC (permalink / raw) To: dev; +Cc: stable, Stephen Hemminger, olivier.matz 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.matz@6wind.com Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- v2 - move check to inside locked region 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..717898c0df02 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) { @@ -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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from secondary process [not found] <20201015172019.3181-1-stephen@networkplumber.org> 2020-10-20 20:58 ` [dpdk-stable] [PATCH v2] mbuf: allow dynamic flags to be used by secondary process Stephen Hemminger @ 2020-10-24 0:43 ` Stephen Hemminger 2020-10-26 10:39 ` Olivier Matz 1 sibling, 1 reply; 4+ messages in thread From: Stephen Hemminger @ 2020-10-24 0:43 UTC (permalink / raw) To: dev; +Cc: stable, Stephen Hemminger, olivier.matz 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.matz@6wind.com Signed-off-by: Stephen Hemminger <stephen@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) { @@ -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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from secondary process 2020-10-24 0:43 ` [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from " Stephen Hemminger @ 2020-10-26 10:39 ` Olivier Matz 2020-10-26 14:49 ` Stephen Hemminger 0 siblings, 1 reply; 4+ messages in thread From: Olivier Matz @ 2020-10-26 10:39 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, stable 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.matz@6wind.com > Signed-off-by: Stephen Hemminger <stephen@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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from secondary process 2020-10-26 10:39 ` Olivier Matz @ 2020-10-26 14:49 ` Stephen Hemminger 0 siblings, 0 replies; 4+ messages in thread From: Stephen Hemminger @ 2020-10-26 14:49 UTC (permalink / raw) To: Olivier Matz; +Cc: dev, stable On Mon, 26 Oct 2020 11:39:35 +0100 Olivier Matz <olivier.matz@6wind.com> wrote: > 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.matz@6wind.com > > Signed-off-by: Stephen Hemminger <stephen@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? Sure, for what I was using rte_errno was not important. And since it was previously broken lets get it fixed. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-26 14:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20201015172019.3181-1-stephen@networkplumber.org>
2020-10-20 20:58 ` [dpdk-stable] [PATCH v2] mbuf: allow dynamic flags to be used by secondary process Stephen Hemminger
2020-10-24 0:43 ` [dpdk-stable] [PATCH v3] mbuf: fix dynamic flags lookup from " Stephen Hemminger
2020-10-26 10:39 ` Olivier Matz
2020-10-26 14:49 ` Stephen Hemminger
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).