From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 11A8AA04C0; Fri, 25 Sep 2020 10:42:35 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D43261D5DA; Fri, 25 Sep 2020 10:42:34 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A1D811D5DA for ; Fri, 25 Sep 2020 10:42:32 +0200 (CEST) IronPort-SDR: HhDO/QPEnbGVW51KksTzWh4iboOakKaC5IUsx5+Q+v1E5h6bilyjJL49gensTTvjhjPIP+wDku qBv3r6ybdgmA== X-IronPort-AV: E=McAfee;i="6000,8403,9754"; a="158859732" X-IronPort-AV: E=Sophos;i="5.77,301,1596524400"; d="scan'208";a="158859732" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2020 01:42:31 -0700 IronPort-SDR: zDKo+zsqfJe/E7EqGGM1yTMSdGu3HWY5qxCmWKgNCTIA2gNOBxoBq7xFoP3p61AAN8fr4Bmjhh bYCwSPSsEGsg== X-IronPort-AV: E=Sophos;i="5.77,301,1596524400"; d="scan'208";a="487384921" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.10.211]) ([10.252.10.211]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 25 Sep 2020 01:42:30 -0700 To: Somnath Kotur Cc: dev , Venkat Duvvuru References: <20200922070632.17706-1-somnath.kotur@broadcom.com> <20200922070632.17706-6-somnath.kotur@broadcom.com> From: Ferruh Yigit Message-ID: Date: Fri, 25 Sep 2020 09:42:26 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 5/8] net/bnxt: add a null ptr check in bnxt PCI probe 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 9/25/2020 3:04 AM, Somnath Kotur wrote: > On Thu, Sep 24, 2020 at 8:17 PM Ferruh Yigit wrote: >> >> On 9/22/2020 8:06 AM, Somnath Kotur wrote: >>> Check for devargs before invoking rep port probe. >>> >>> Fixes: 6dc83230b43b ("net/bnxt: support port representor data path") >>> >>> Signed-off-by: Somnath Kotur >>> Reviewed-by: Venkat Duvvuru >>> --- >>> drivers/net/bnxt/bnxt_ethdev.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c >>> index db2f0dd..84eba0b 100644 >>> --- a/drivers/net/bnxt/bnxt_ethdev.c >>> +++ b/drivers/net/bnxt/bnxt_ethdev.c >>> @@ -6147,6 +6147,10 @@ static int bnxt_pci_probe(struct rte_pci_driver *pci_drv __rte_unused, >>> } >>> PMD_DRV_LOG(DEBUG, "BNXT Port:%d pci probe\n", >>> backing_eth_dev->data->port_id); >>> + >>> + if (!pci_dev->device.devargs) >>> + return ret; >>> + >> >> There is already a null check at the beginning of the function because >> of the same thing (port representors), should they be combined? >> > No, this is to catch the corner case if/when 'backing_eth_dev' is > already allocated , so code would unconditionally call > bnxt_rep_port_probe() > irrespective of devargs being there or not, the check at this point > helps prevent that >> And devargs being not NULL does not really mean it has arguments related >> to the port representors, it may have other device devargs. Perhaps >> 'eth_da' can be used to check? > eth_da is a local var in this function, so perhaps 'num_rep' i.e > invoke bnxt_rep_port_probe only if num_rep > 0 ? +1 > Please let me know if you want me to do a respin of this patch alone > or will you be doing this minor change while merging it in? Please send a new version of this patch alone. Thanks.