From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44]) by dpdk.org (Postfix) with ESMTP id C717E7CA9 for ; Wed, 26 Jul 2017 14:55:21 +0200 (CEST) Received: by mail-wm0-f44.google.com with SMTP id t201so71803513wmt.1 for ; Wed, 26 Jul 2017 05:55:21 -0700 (PDT) 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:content-transfer-encoding:in-reply-to :user-agent; bh=ek9p834BlzZ7s/KlktlV9IJSrygzEQQ4mc/1eq9IIlQ=; b=LJ+4V/a43l+unGFlpaFwgUb/LAo1OAD6Z8b7vR4Eam4hAV95j+CqtGEeOHP2Utjjs5 eTy42xDCN/d0WHAsyZtyPEip9CXpKe2fb0x0xHngxWjZ+SsAmtbj822ltI64nm1v85n4 +gsSzPsobiDZWLxczi3B8xxj8WWF7p7E23UZN/WNxzD+iL3rsbVrFbZ/uspv1oz45RE3 /OW1HWIYeJ48Fnc+v5qiOFGDTsk7Ggq4EuEWnCB4I1XRnk80SnN6Q4w8zhw05OYkVRNx 1ds6HW9i20l1DLyUrzLSgOCASVh1kdOJEBmXHTFrh1sV5csGR23YC8z301YnLRpQ9uQG 7AbQ== 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:content-transfer-encoding :in-reply-to:user-agent; bh=ek9p834BlzZ7s/KlktlV9IJSrygzEQQ4mc/1eq9IIlQ=; b=RCIj7457j739g34KqUQP1MXzhbc7h9VTVxrNm9JyKq2s6L0m5tBIsgfp9F/qn+thUv x/IPvo8zLVZ4TkaQ+xdSX/i2yuqmRmauRE8xZo3crDerFS5HxEq7QWMgUT0sxgCDbuww FLcGoFwlVOEYVne5nY47r3/HEANA5x45Hqo1xIDCt2Fz+zm8/koogfft6jrqyuyar4Im eZ2Zl6Wxok3LDaCtOMLr/Jchh2EAt3r4mtyQ25pllWG5w9KsnlC49YstNvgJU8Gj466r wZwG091PccoqHJn7pN3Rfz56EdKmHn2oUjPatKW5b/Ia3XXen5i9bRw7Q1SfgAlxPAcC j+Hg== X-Gm-Message-State: AIVw113ydCYZjGa9YI8w3MwMHJRW62ngtLPPqnpj6ze+EgtzHXrQ+gSw co11u9PvnOMNdnYR X-Received: by 10.28.198.206 with SMTP id w197mr738009wmf.86.1501073721186; Wed, 26 Jul 2017 05:55:21 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g18sm32378926wrg.69.2017.07.26.05.55.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 26 Jul 2017 05:55:20 -0700 (PDT) Date: Wed, 26 Jul 2017 14:55:11 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Shachar Beiser Cc: Adrien Mazarguil , "dev@dpdk.org" , =?iso-8859-1?Q?N=E9lio?= Laranjeiro , Shahaf Shuler Message-ID: <20170726125511.GE11154@bidouze.vm.6wind.com> References: <5ec0604196fb087a42fa75e6ddc2a04aab293593.1501047767.git.shacharbe@mellanox.com> <20170726090613.GT19852@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix verification of device context 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: Wed, 26 Jul 2017 12:55:22 -0000 Hi Shachar, On Wed, Jul 26, 2017 at 09:21:27AM +0000, Shachar Beiser wrote: > Hi , > > When I say close I mean : " mlx5_dev_close" . This function set the priv->ctx to NULL. > We think this patch is required because we have an open bug of seg fault while accessing priv->ctx == NULL. > > -Shachar Beiser. > This patch does not fix the root cause of the issue. It is a bug in the ether layer, and missing flags within MLX5 PMD. So NACK on this patch, I will send shortly the proper fix. Best regards, > > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Wednesday, July 26, 2017 12:06 PM > To: Shachar Beiser > Cc: dev@dpdk.org; Nélio Laranjeiro ; stable@dpdk.org > Subject: Re: [PATCH] net/mlx5: fix verification of device context > > Hi Shachar, > > On Wed, Jul 26, 2017 at 05:43:24AM +0000, Shachar Beiser wrote: > > Get interface name function lacks verification of device context. > > It might lead to segmentation fault when trying to query the name > > after the device is closed.fixing it by adding the missing > > verification > > > > Thanks, however if by "close" you mean it may occur when applications use ethdev callbacks after a call to rte_eth_dev_close(), I do not think PMDs have to protect themselves against bad application behavior, otherwise there is no end to such fixes. > > The reverse of rte_eth_dev_close() is not rte_eth_dev_configure() nor any other ethdev callback (see documentation), but a bus probe operation. > > Perhaps I've missed something, so in case a crash occurs *while* calling > rte_eth_dev_close() I guess this patch is fine, but then please describe the reason. > > > Fixes: cd89f22a1e9770 ("net/mlx5: remove unused interface name query") > > This commit doesn't look like the root cause of that issue? > > > Cc: stable@dpdk.org > > > > Signed-off-by: Shachar Beiser > > --- > > drivers/net/mlx5/mlx5_ethdev.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/net/mlx5/mlx5_ethdev.c > > b/drivers/net/mlx5/mlx5_ethdev.c index b70b7b9..6e67461 100644 > > --- a/drivers/net/mlx5/mlx5_ethdev.c > > +++ b/drivers/net/mlx5/mlx5_ethdev.c > > @@ -173,6 +173,10 @@ struct priv * > > char match[IF_NAMESIZE] = ""; > > > > { > > + if (priv->ctx == NULL) { > > + DEBUG("The device is closed, cannot query interface name "); > > + return -1; > > + } > > MKSTR() is at the beginning of this block because it defines a new variable (path). For coding style consistency you should not put any code before variable declarations, or at least insert an empty line between them. Otherwise you could move this check to the parent block. > > > MKSTR(path, "%s/device/net", priv->ctx->device->ibdev_path); > > > > dir = opendir(path); > > -- > > 1.8.3.1 > > > > I think this patch is not necessary unless proved otherwise, have you > actually observed a crash addressed by it? > > -- > Adrien Mazarguil > 6WIND -- Gaëtan Rivet 6WIND