From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 264FE1B349 for ; Mon, 5 Feb 2018 13:27:14 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 605983DE31; Mon, 5 Feb 2018 12:27:13 +0000 (UTC) Received: from localhost.localdomain (ovpn-116-10.gru2.redhat.com [10.97.116.10]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F258B3819C; Mon, 5 Feb 2018 12:27:12 +0000 (UTC) Received: by localhost.localdomain (Postfix, from userid 1000) id 9377618106E; Mon, 5 Feb 2018 10:27:10 -0200 (-02) Date: Mon, 5 Feb 2018 10:27:10 -0200 From: Marcelo Ricardo Leitner To: Adrien Mazarguil Cc: Shahaf Shuler , Nelio Laranjeiro , dev@dpdk.org Message-ID: <20180205122710.GC27676@localhost.localdomain> References: <20180202144736.8239-1-adrien.mazarguil@6wind.com> <20180202164050.13017-1-adrien.mazarguil@6wind.com> <20180202164050.13017-2-adrien.mazarguil@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180202164050.13017-2-adrien.mazarguil@6wind.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 05 Feb 2018 12:27:13 +0000 (UTC) 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 12:27:14 -0000 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. > +#endif > mlx4_glue->fork_init(); > rte_pci_register(&mlx4_driver); > } > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c > index 544599b01..050cfac0d 100644 > --- a/drivers/net/mlx5/mlx5.c > +++ b/drivers/net/mlx5/mlx5.c > @@ -1142,6 +1142,15 @@ rte_mlx5_pmd_init(void) > return; > assert(mlx5_glue); > #endif > +#ifndef NDEBUG > + /* Glue structure must not contain any NULL pointers. */ > + { > + unsigned int i; > + > + for (i = 0; i != sizeof(*mlx5_glue) / sizeof(void *); ++i) > + assert(((const void *const *)mlx5_glue)[i]); > + } > +#endif > mlx5_glue->fork_init(); > rte_pci_register(&mlx5_driver); > } > -- > 2.11.0 >