From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 17756A04B5; Mon, 26 Oct 2020 11:39:41 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7DF452BA3; Mon, 26 Oct 2020 11:39:38 +0100 (CET) Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by dpdk.org (Postfix) with ESMTP id A429B2BA2 for ; Mon, 26 Oct 2020 11:39:37 +0100 (CET) Received: by mail-wm1-f66.google.com with SMTP id 13so11085850wmf.0 for ; Mon, 26 Oct 2020 03:39:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=vyU+5eKQlbFGFbU4IQbhWHuzeLHcQPj9k5bjr3MzgPQ=; b=etyZV35V64z7B/RnH+q+zu8gN8EJ3SXCTbCd+wdWhXt5J/UIgmJ8+tni4f2IJi2bAe k0H6OupznRqOS8UulfJw0zpeliziG/eugcqsZbfZw6BdU3XIHujQbLRsQWmJLreMD2hz Y/RBordFmOyNwzB7/7//mHzlvLqQqu6ZoVTM2yvN8D0feskR4YN/lejGFxQd+GpsUSUH 60jZX+m6o8/4lkLKOik7b+lv9uQwEqjZLWRQ3X0ZstBs+cP4XjC5VEMNNO478K0J2H9V GbiXhGmol3MnT0n2Sy05XyvLBaVSr+OlhxLv+MZdcgSJ5KkUWhfLLFvrfHti2w8zvgp5 gspw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=vyU+5eKQlbFGFbU4IQbhWHuzeLHcQPj9k5bjr3MzgPQ=; b=cSvUFbG8P0j86CCV4qE85oOwTDFPkttKO3QqYnBRIGh09tIsj/RCg5Tao7ot4KohxC OWVnPIhd2OKMb+sW/6z7v/BB0yZmdSB/sGdw44Jt021qzLLK2qj5WFMFv/YV9Squ6mbY 3yKGMIOxxyvUsGqrhmtXLlLZt7Q8xs1qeGl6Zs40zm9IScTfixXmnspmE1YPcqdR+GGx UTSCKgLMeMr2d7zhim9BsyVGA2T+Ve4hMBkyokf4V6FDJAydsns9TtmpWPkcHJdwpFc3 X7OXU5whoW8BoScTUN+PxKBAp4zg+KUf8tFhDJGbgdxLoLAls2FQ2kr72GNU3BEz1zLJ KjIw== X-Gm-Message-State: AOAM533KqbXYWtBEK+SROoYcErp/14Lb/tEal6bsUV1pfE48J8ufYE7+ psw/S3HVldFimo5xsX87qIKTCtIzzZfCx6zT X-Google-Smtp-Source: ABdhPJwZDwLXnWzEQ7Q2lt/9F+eobPxJoNB5Jqx2DpZ5K3r+dzFRFkyX8vRy0B6Q5K1fyY3MJ5O5VQ== X-Received: by 2002:a1c:2803:: with SMTP id o3mr14689165wmo.97.1603708776329; Mon, 26 Oct 2020 03:39:36 -0700 (PDT) Received: from 6wind.com (2a01cb0c0005a600345636f7e65ed1a0.ipv6.abo.wanadoo.fr. [2a01:cb0c:5:a600:3456:36f7:e65e:d1a0]) by smtp.gmail.com with ESMTPSA id n9sm24157849wrq.72.2020.10.26.03.39.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Oct 2020 03:39:35 -0700 (PDT) Date: Mon, 26 Oct 2020 11:39:35 +0100 From: Olivier Matz To: Stephen Hemminger Cc: dev@dpdk.org, stable@dpdk.org Message-ID: <20201026103935.GL1898@platinum> References: <20201015172019.3181-1-stephen@networkplumber.org> <20201024004331.25043-1-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201024004331.25043-1-stephen@networkplumber.org> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v3] mbuf: fix dynamic flags lookup from secondary process X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > > 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 >