From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f175.google.com (mail-wr0-f175.google.com [209.85.128.175]) by dpdk.org (Postfix) with ESMTP id 102064C71 for ; Tue, 20 Mar 2018 15:02:42 +0100 (CET) Received: by mail-wr0-f175.google.com with SMTP id o1so1794081wro.10 for ; Tue, 20 Mar 2018 07:02:41 -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=UBVWOmiY1rD2hXLsZhElLrqexY6ZxGJh1awkFy+2kSs=; b=CKxuUJ6iS/Dlywndu3bVd/oJLusqUwp0Q85PYvETYONl0foXnBZnC1Uqy1xmjHdbUF eJ2QToP8p0gp+APjAxCQZ95sml2PSw56PzmuzeFRBkTRRbNdkntkqbHEI70DO/y+tQ5G BE/ICS/kwojKMswVpsGzJniuic5caMeAxoINPahV4wQYzOemcUyN+daOfHILl9n8p6Xu 5ZfopxFCvlaT+nSNMddFdFqpuO2ZeWy6WSBSGRkl/94o8730GhT3Zpg+NO/k2QX79tUG PzGFxU3AfPTwu9qADxm4ztaKVkX+65HVEvp4tvNJwV0NgLnAZQofjITthRFfv9APgY+G GFBg== 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=UBVWOmiY1rD2hXLsZhElLrqexY6ZxGJh1awkFy+2kSs=; b=aF8W1ZqFo7MpYofUYaOA3jN22iXcmCQ54bmLpju4/uF4pXfhtCncGu/qa9jQBx+lbT jNoZz8ZVKGXuFZ7JQu7RaceyCbI8I6+D7LNSjZxuHR8xOQbPTxszqEVDm4887Fb8GKGT dFeL2+npXg3x00GK4HGIJ41EtlxE4IxmkmlpYofCWiEEJVbDEdJAZ+Xch2A8Eoxi6Spi kNnH9zd3N7DZp+KThfa36iUTOTu9jtB79MQtLnddCMt+WHJckQXZNd2O9GPwB74xW7op 1OJovrO+BtFwAP3A6M9ZZ8zmBm5JLJIAOe6CmiD2t89SzG7IN16wWgOyU0+C4njg78OU CSbA== X-Gm-Message-State: AElRT7E82lbvWoCsSfD8SkEo91kz5WF6ly1Mvl/WR2+EB3GtIK4fdZjm PQw7g4sfbHa/UMHemwaXRd5xAA== X-Google-Smtp-Source: AG47ELuVTGqNxRgDVvOYhKraSnixiw3uAz7QDiZFY0quM1pRfEMkWBWc7Jf0bcR7bTk65F/E/V9NcA== X-Received: by 10.223.171.213 with SMTP id s79mr12328370wrc.52.1521554561162; Tue, 20 Mar 2018 07:02:41 -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 q9sm1539857wrf.11.2018.03.20.07.02.38 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 20 Mar 2018 07:02:39 -0700 (PDT) Date: Tue, 20 Mar 2018 15:02:25 +0100 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Adrien Mazarguil Cc: Ophir Munk , dev@dpdk.org, Thomas Monjalon , Olga Shern , stable@dpdk.org Message-ID: <20180320140225.i2nenxxpj4ojtc6w@bidouze.vm.6wind.com> References: <1519836450-11895-1-git-send-email-ophirmu@mellanox.com> <20180302191258.GF4256@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180302191258.GF4256@6wind.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH v1] net/mlx4: fix 'show port info all' during detach 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: Tue, 20 Mar 2018 14:02:42 -0000 Hi Ophir, Adrien, On Fri, Mar 02, 2018 at 08:12:58PM +0100, Adrien Mazarguil wrote: > On Wed, Feb 28, 2018 at 04:47:30PM +0000, Ophir Munk wrote: > > The following scenario causes a crash in function mlx4_get_ifname > > 1. On testpmd startup mlx4 device is probed and started > > 2. mlx4 sriov is disabled. As a result an RMV event is sent to > > testpmd which closes the device and nullify the priv struct > > members. > > 3. Running 'show port info all' in testpmd results in segmentation > > fault because of accessing NULL pointer priv->ctx > > > > The fix is to return with an error from mlx4_get_ifname() if priv->ctx > > member is NULL. > > So priv->ctx is NULL after mlx4_dev_close() was called. In my opinion the > fact testpmd is allowed to call rte_eth_dev_info_get() on a closed port is > fishy, there are quite a few other ethdev callbacks that may end up > crashing in such a scenario. > > Since commit 284c908cc588 ("app/testpmd: request device removal interrupt"), > testpmd automatically calls rte_eth_dev_close() and rte_eal_dev_detach() > after receiving a removal event on a port. > > rte_eth_dev_close() documentation says: > > "Close a stopped Ethernet device. The device cannot be restarted! > The function frees all resources except for needed by the > closed state. To free these resources, call rte_eth_dev_detach()." > > Unfortunately testpmd calls rte_*eal*_dev_detach() not > rte_*eth*_dev_detach(), the former doesn't release ethdev ports while the > latter does. I think it's a mistake, there's no point in keeping a closed > device around after it's been physically unplugged. > > In short, rmv_event_callback() should call detach_port() instead of > rte_eal_dev_detach(). > This is correct, the issue is actually testpmd making a mistake when calling directly rte_eal_dev_detach(). The fix should thus be simply to change this call. > > Fixes: 61cbdd419478 ("net/mlx4: separate device control functions") > > Cc: stable@dpdk.org > > > > Signed-off-by: Ophir Munk > > The above suggests the problem is actually in testpmd and was introduced in > v17.05 by commit 284c908cc588. The proposed patch is a workaround that > doesn't address the underlying issue, thus NACK unless proven otherwise :) > > > --- > > drivers/net/mlx4/mlx4_ethdev.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/mlx4/mlx4_ethdev.c b/drivers/net/mlx4/mlx4_ethdev.c > > index 3bc6927..cca5223 100644 > > --- a/drivers/net/mlx4/mlx4_ethdev.c > > +++ b/drivers/net/mlx4/mlx4_ethdev.c > > @@ -67,6 +67,9 @@ mlx4_get_ifname(const struct priv *priv, char (*ifname)[IF_NAMESIZE]) > > char match[IF_NAMESIZE] = ""; > > > > { > > + if (priv->ctx == NULL) > > + return -ENOENT; > > + > > MKSTR(path, "%s/device/net", priv->ctx->device->ibdev_path); > > > > dir = opendir(path); > > -- > > 2.7.4 > > > > -- > Adrien Mazarguil > 6WIND -- Gaëtan Rivet 6WIND