From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 2257F2A58 for ; Fri, 14 Oct 2016 10:23:42 +0200 (CEST) Received: from lfbn-1-5996-232.w90-110.abo.wanadoo.fr ([90.110.195.232] helo=[192.168.1.13]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1buxpM-0006na-G5; Fri, 14 Oct 2016 10:26:53 +0200 To: jean.tourrilhes@hpe.com, dev@dpdk.org, Thomas Monjalon , David Marchand , Sergio Gonzalez Monroy References: <20161012200445.GA10029@labs.hpe.com> From: Olivier Matz Message-ID: Date: Fri, 14 Oct 2016 10:23:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <20161012200445.GA10029@labs.hpe.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] mempool: Add sanity check when secondary link in less mempools than primary X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Oct 2016 08:23:42 -0000 Hi Jean, On 10/12/2016 10:04 PM, Jean Tourrilhes wrote: > mempool: Add sanity check when secondary link in less mempools than primary > > If the primary and secondary process were build using different build > systems, the list of constructors included by the linker in each > binary might be different. Mempools are registered via constructors, so > the linker magic will directly impact which tailqs are registered with > the primary and the secondary. > > DPDK currently assumes that the secondary has a superset of the > mempools registered at the primary, and they are in the same order > (same index in primary and secondary). In some build scenario, the > secondary might not initialise any mempools at all. This would result > in an obscure segfault when trying to use the mempool. Instead, fail > early with a more explicit error message. > > Signed-off-by: Jean Tourrilhes > --- > lib/librte_mempool/rte_mempool.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 2e28e2e..4fe9158 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -1275,6 +1275,16 @@ rte_mempool_lookup(const char *name) > return NULL; > } > > + /* Sanity check : secondary may have initialised less mempools > + * than primary due to linker and constructor magic. Note that > + * this does not address the case where the constructor order > + * is different between primary and secondary and where the index > + * points to the wrong ops. Jean II */ > + if(mp->ops_index >= (int32_t) rte_mempool_ops_table.num_ops) { > + /* Do not dump mempool list, it will segfault. */ > + rte_panic("Cannot find ops for mempool, ops_index %d, num_ops %d - maybe due to build process or linker configuration\n", mp->ops_index, rte_mempool_ops_table.num_ops); > + } > + > return mp; > } > > I'm not really fan of this. I think the configuration and build system of primary and secondaries should be the same to avoid this kind of issues. Some other issues may happen if the configuration is different, for instance the size of structures may be different. There is already a lot of mess due to primary/secondary at many places in the code, I'm not sure adding more is really desirable. Regards, Olivier