[dpdk-dev] [PATCH v3] mbuf: fix dynamic flags lookup from secondary process
Olivier Matz
olivier.matz at 6wind.com
Mon Oct 26 11:39:35 CET 2020
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 at 6wind.com
> Signed-off-by: Stephen Hemminger <stephen at 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
>
More information about the dev
mailing list