[dpdk-dev] [PATCH 5/8] net/bnxt: add a null ptr check in bnxt PCI probe

[Ferruh Yigit]

On 9/25/2020 3:04 AM, Somnath Kotur wrote:
> On Thu, Sep 24, 2020 at 8:17 PM Ferruh Yigit <ferruh.yigit at 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.kotur at broadcom.com>
>>> Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru at 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.