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 6E56FA04C0; Fri, 25 Sep 2020 12:46:26 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 468EF1E88C; Fri, 25 Sep 2020 12:46:26 +0200 (CEST) Received: from mail-pf1-f196.google.com (mail-pf1-f196.google.com [209.85.210.196]) by dpdk.org (Postfix) with ESMTP id 538C41E887 for ; Fri, 25 Sep 2020 12:46:24 +0200 (CEST) Received: by mail-pf1-f196.google.com with SMTP id b124so2753887pfg.13 for ; Fri, 25 Sep 2020 03:46:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mWpZocisY9F+/WQFW7csM81iv1sJUpPxFN0Am63p6+E=; b=aWeh7jS2iIBSgz0BTjJDAyEsdlrl3PxMcZWkMMu6M8f/rMW49o83vNftXrBINqjGbZ XlJgXo5pHnYVrrAdW82n11kZJEFNZuE7y3Z++NaY1xUagF+gC3FhAr8+fCffk3+mJCQJ 8Glmrjtpq3sYIFHD/3Stk3teshSKPxD/hrrS8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mWpZocisY9F+/WQFW7csM81iv1sJUpPxFN0Am63p6+E=; b=BAn2P5tt+2xSORDxt5vAOIZfiKNMttzpGyVVziIGirxjG/Ha94e81U0KEY23M5BPlY /Th31SmSLUoyZeQRQaXxdAzq2N+IZm+LziPC0Ug0dBpcNwGqR0AflPQQmdWMBwqxQay7 ef6h1Zuff/GUJe8KiAG4tyTUg7ReY8azyAG2DzFp+gt0s5j0+dK6hjC24CKR0Fqyba5S fCzQBrAxiFe6kseTXF5nt/Z2segSrUamjIbzKbHy+NUjqFtgxsKkj47wRSoz5rlLul7n dwrlOCt8zqxJCznqeeHcraNbm4XaSIwCSjQMR2uGUMfKIJf/ZTIJgKdGZAUkLWFT4dF0 pd+g== X-Gm-Message-State: AOAM532Ult71rZTEKtK8ellJDWGnaRQIHEZSVONpKCo6vFjaA06mYmRY M35rZPara69y2u3+7Z7m2TCRswLevkODYnvMr6MMd7wuyuE= X-Google-Smtp-Source: ABdhPJzVsS6YpWiDi4hG/i9jk5KFS38zPuGMnoOo6/2gTF689rfsZZa0zyn6+cMRwFZzpxOKwVU8e6m1B+U6lcoUXg8= X-Received: by 2002:a17:902:a9c3:b029:d1:ece5:a1d0 with SMTP id b3-20020a170902a9c3b02900d1ece5a1d0mr3650114plr.78.1601030783360; Fri, 25 Sep 2020 03:46:23 -0700 (PDT) MIME-Version: 1.0 References: <20200922070632.17706-1-somnath.kotur@broadcom.com> <20200922070632.17706-6-somnath.kotur@broadcom.com> In-Reply-To: From: Somnath Kotur Date: Fri, 25 Sep 2020 16:16:11 +0530 Message-ID: To: Ferruh Yigit Cc: dev , Venkat Duvvuru Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 Fri, Sep 25, 2020 at 2:12 PM Ferruh Yigit wrote: > > 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. Thanks Ferruh, Done