On Fri, Sep 25, 2020 at 4:16 PM Somnath Kotur
<somnath.ko...@broadcom.com> wrote:
>
> On Fri, Sep 25, 2020 at 2:12 PM Ferruh Yigit <ferruh.yi...@intel.com> wrote:
> >
> > On 9/25/2020 3:04 AM, Somnath Kotur wrote:
> > > On Thu, Sep 24, 2020 at 8:17 PM Ferruh Yigit <ferruh.yi...@intel.com>
> > > 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 <somnath.ko...@broadcom.com>
> > >>> Reviewed-by: Venkat Duvvuru <venkatkumar.duvv...@broadcom.com>
> > >>> ---
> > >>> 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
Sorry Ferruh, the first one i sent did not have the in-reply-to,
re-sent it with the correct in-reply-to format again, so please pick
that up
Thanks a lot
-Som
