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 ?
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?
Thanks
Som
