From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id EC84A1B2EA for ; Mon, 5 Feb 2018 14:31:34 +0100 (CET) Received: by mail-wm0-f65.google.com with SMTP id b21so25922461wme.4 for ; Mon, 05 Feb 2018 05:31:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=HPpiKxkh/nJ2qBpcasLfJRqu5ZASWtmHCsooGwE+bRc=; b=U44muQRo99qwBIMpAiAxXEMpCGi5n4Fv6lP5woeP539PgWaqhzFtOr7zBOfDWeIeQN A9PKkZ1JbCKrPsJkNtLWztUe9XlhOCt92yPQ7Olo31rR4GPVxNPiOTqbIk74EroWAccr mgPeuySCMPX9JbFs5eZvS9MpTnxV8eWJi+0xcch+mx+nGE7jb9bZEo3hFPDK3OM0JvAy oLGYOQE3BjQfJJbUSeIDEIH2bqgv6fjoxG/HXbedtj4nmFQIN3sdE/xxeiUvrw56FIBM 2K/pQijHAcL6EaKmxk6K+hqEqzip+aLNz61E7pow1mk679LPF8oib8UBAs/tnVyvSyJg 2MUw== 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; bh=HPpiKxkh/nJ2qBpcasLfJRqu5ZASWtmHCsooGwE+bRc=; b=DiQvxWwrb0nsl6VKp+kIM+MIbADtHhY19Qa47FmOvFcShCn4j3OPdnfA0q3ttYVNQX nJshyKGSFN6YRU3hSaU2Rz4D1rzgoBGIBR+Mv6haElFP4rAi/GYCDcO6XtumIgcfyFAR ioe4N2qq2H42WJl/WrI+A3IGl8XPYp1QKpMiHJi9Wx6ERz+ijOAl1IDGFPbuA4sYy5sd fh+sJMlyQ5jSY8nQYRzV/n7Wo5LlRVLRNCXxegPiCyApAHhc/0Zrp/LqotCGq0lj3I3s 20JAJkpTwnr7IREkKJ16uneR8rbKLVwF3MIFQe8oO+DHph7YLU+mTGadhfwfjLDOUBzM DnDw== X-Gm-Message-State: AKwxytfgoEUFGME4AwFj8jPFnCUl2l6EgEmTBOq1JyCUMvL2YKonK0SM xG37nH5rfUXBmDl8dn+B33z2pA== X-Google-Smtp-Source: AH8x224V+wK5yhQbAtasOVcJq9WkCbDNqqPztU/WMY+nxgtyULI8WzTdVI6eIUrRJFgj7c8Yp3OG4Q== X-Received: by 10.28.232.72 with SMTP id f69mr32593477wmh.110.1517837494675; Mon, 05 Feb 2018 05:31:34 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id v80sm4862590wmv.17.2018.02.05.05.31.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Feb 2018 05:31:33 -0800 (PST) Date: Mon, 5 Feb 2018 14:31:21 +0100 From: Adrien Mazarguil To: Marcelo Ricardo Leitner Cc: Shahaf Shuler , Nelio Laranjeiro , dev@dpdk.org Message-ID: <20180205133121.GF4256@6wind.com> References: <20180202144736.8239-1-adrien.mazarguil@6wind.com> <20180202164050.13017-1-adrien.mazarguil@6wind.com> <20180202164050.13017-2-adrien.mazarguil@6wind.com> <20180205122710.GC27676@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180205122710.GC27676@localhost.localdomain> Subject: Re: [dpdk-dev] [PATCH v2 1/4] net/mlx: add debug checks to glue structure 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: , X-List-Received-Date: Mon, 05 Feb 2018 13:31:35 -0000 On Mon, Feb 05, 2018 at 10:27:10AM -0200, Marcelo Ricardo Leitner wrote: > On Fri, Feb 02, 2018 at 05:46:12PM +0100, Adrien Mazarguil wrote: > > This code should catch mistakes early if a glue structure member is added > > without a corresponding implementation in the library. > > > > Signed-off-by: Adrien Mazarguil > > --- > > drivers/net/mlx4/mlx4.c | 9 +++++++++ > > drivers/net/mlx5/mlx5.c | 9 +++++++++ > > 2 files changed, 18 insertions(+) > > > > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > > index 50a55ee52..201d39b6e 100644 > > --- a/drivers/net/mlx4/mlx4.c > > +++ b/drivers/net/mlx4/mlx4.c > > @@ -799,6 +799,15 @@ rte_mlx4_pmd_init(void) > > return; > > assert(mlx4_glue); > > #endif > > +#ifndef NDEBUG > > + /* Glue structure must not contain any NULL pointers. */ > > + { > > + unsigned int i; > > + > > + for (i = 0; i != sizeof(*mlx4_glue) / sizeof(void *); ++i) > > + assert(((const void *const *)mlx4_glue)[i]); > > + } > > This code will not catch the case on which mlx4_glue on PMD is smaller > than the one on the glue lib. Although this would be safe, as the PMD > won't place calls for functions that it is not aware of, I guess we > don't want to allow such situation anyhow. > > One way to handle this is to add a size field and let the PMD check if > the size is the same. As this code is walking through it as an array > of pointers, an union around it to keep the alignment may be welcomed. > > Also, this code would do read beyond buffer in the opposite case, when > mlx4_glue for the PMD is larger than the one on the glue lib. While I generally agree, this block is pure debugging code not compiled in by default and meant for PMD developers, not end users. It happened to me while moving all these calls to the glue structure where I missed a couple of functions and is a way to ensure such mistakes would be caught early on. The version check that comes afterward is actually is the safe check you're thinking about. Based on that, the PMD can be confident the symbol in question has the expected properties. No need to check for NULLs. If you think this code doesn't have its place in a PMD, I can remove it; the version check should be enough. In my opinion it's better to keep it for safety though, please confirm. -- Adrien Mazarguil 6WIND