Re: [PATCH v2] net/bonding: support bifurcated driver in eal cli using --vdev

[gowrishankar muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>]

On Friday 07 July 2017 09:08 PM, Declan Doherty wrote:
> On 04/07/2017 12:57 PM, Gowrishankar wrote:
>> From: Gowrishankar Muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
>>
>> At present, creating bonding devices using --vdev is broken for PMD like
>> mlx5 as it is neither UIO nor VFIO based and hence PMD driver is unknown
>> to find_port_id_by_pci_addr(), as below.
>>
>> testpmd <EAL args> --vdev 'net_bonding0,mode=1,slave=<PCI>,socket_id=0'
>>
>> PMD: bond_ethdev_parse_slave_port_kvarg(150) - Invalid slave port value
>>  (<PCI ID>) specified
>> EAL: Failed to parse slave ports for bonded device net_bonding0
>>
>> This patch fixes parsing PCI ID from bonding device params by verifying
>> it in RTE PCI bus, rather than checking dev->kdrv.
>>
>> Changes:
>>   v2 - revisit fix by iterating rte_pci_bus
>>
>> Signed-off-by: Gowrishankar Muthukrishnan 
>> <gowrishankar.m@linux.vnet.ibm.com>
>> ---
> ...
>>
>
> Hey Gowrishankar,
>
> I was having a look at this patch and there is the following 
> checkpatch error.
>
> _coding style issues_
>
>
> WARNING:AVOID_EXTERNS: externs should be avoided in .c files
> #48: FILE: drivers/net/bonding/rte_eth_bond_args.c:43:
> +extern struct rte_pci_bus rte_pci_bus;
>
Hi Declan,
Thank you for your review.
Yes, but I also saw some references like above in older code.

>
> Looking at bit closer at the issue I think there is a simpler 
> solution, the bonding driver really shouldn't be parsing the PCI bus 
> directly, and since PCI devices use the PCI DBF as their name we can 
> simply replace the all the scanning code with a simple call to 
> rte_eth_dev_get_port_by_name API.
>

But you are removing an option to mention ports by PCI addresses right  
(as I see parse_port_id() completely removed in your patch) ?.
IMO, we just need to check if given eth pci id (incase we mention ports 
ib PCI ID) is one of what EAL scanned in PCI. Also, slaves should not be 
from any blacklisted PCI ids (as we test with -b or -w).

With your patch, it failed to parse params as below.

testpmd -l 0,8,16 --socket-mem 512,512 --vdev 
'net_bonding0,mode=1,slave=0002:01:00.0,slave=0002:01:00.1,primary=0002:01:00.0,socket_id=0' 


Configuring Port 0 (socket 1)
PMD: net_mlx5: 0x4a7f8f80: TX queues number update: 0 -> 1
PMD: net_mlx5: 0x4a7f8f80: RX queues number update: 0 -> 1
Port 0: E4:1D:2D:C9:C7:56
Configuring Port 1 (socket 1)
PMD: net_mlx5: 0x4a7fd000: TX queues number update: 0 -> 1
PMD: net_mlx5: 0x4a7fd000: RX queues number update: 0 -> 1
Port 1: E4:1D:2D:C9:C7:57
Configuring Port 2 (socket 0)
EAL: Failed to parse slave ports for bonded device net_bonding0
Fail to configure port 2
EAL: Error - exiting with code: 1
   Cause: Start ports failed

With my patch, I have tested with -w and -b options in testpmd as well.

Please let me know if I am wrong on anything of above.

Thanks,
Gowrishankar