[dpdk-dev] [PATCH v15] testpmd: Add port hotplug support

Tetsuya Mukawa mukawa at igel.co.jp
Fri Feb 27 07:14:59 CET 2015


On 2015/02/27 3:49, De Lara Guarch, Pablo wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Tetsuya Mukawa
>> Sent: Wednesday, February 25, 2015 7:32 PM
>> To: dev at dpdk.org
>> Subject: [dpdk-dev] [PATCH v15] testpmd: Add port hotplug support
>>
>> The patch introduces following commands.
>> - port attach [ident]
>> - port detach [port_id]
>>  - attach: attaching a port
>>  - detach: detaching a port
>>  - ident: pci address of physical device.
>>           Or device name and parameters of virtual device.
>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>  - port_id: port identifier
>>
>> v15:
>> - Replace rte_eal_dev_attach() by rte_eth_dev_attach()
>> - Replace rte_eal_dev_detach() by rte_eth_dev_detach()
>>
>> v7:
>> - Fix doc.
>>   (Thanks to Iremonger, Bernard)
>> - Fix port checking implementation of star_port();
>>   (Thanks to Qiu, Michael)
>> v5:
>> - Add testpmd documentation.
>>   (Thanks to Iremonger, Bernard)
>> v4:
>>  - Fix strings of command help.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp>
>> ---
>>  app/test-pmd/cmdline.c                      | 137 +++++++++++++++----
>>  app/test-pmd/config.c                       | 102 ++++++++------
>>  app/test-pmd/parameters.c                   |  22 ++-
>>  app/test-pmd/testpmd.c                      | 199 +++++++++++++++++++++-------
>>  app/test-pmd/testpmd.h                      |  18 ++-
>>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  57 ++++++++
>>  6 files changed, 409 insertions(+), 126 deletions(-)
>>
> [...]
>
>> @@ -1423,12 +1443,8 @@ set_fwd_ports_list(unsigned int *portlist,
>> unsigned int nb_pt)
>>   again:
>>  	for (i = 0; i < nb_pt; i++) {
>>  		port_id = (portid_t) portlist[i];
>> -		if (port_id >= nb_ports) {
>> -			printf("Invalid port id %u >= %u\n",
>> -			       (unsigned int) port_id,
>> -			       (unsigned int) nb_ports);
>> +		if (port_id_is_invalid(port_id, ENABLED_WARN))
> Sorry for catching this late, but there is a segmentation fault when this function gets called 
> when parsing "portmask" at testpmd initialization, since port_id_is_invalid function needs to have array "ports" initialized.
> I would revert the change, but not sure if this is need for other reasons.

Hi Pablo,

I appreciate for your reporting.
I could reproduce the issue with portmask option like below
$ sudo ./testpmd -c f -n 1 -- --portmask 0x1 -i

As a result of investigation, when 'launch_args_parse()' is called, port
structure hasn't been allocated yet.
As you said, this causes the issue.

I guess we can have 2 options to fix the issue.

Option1:
Fix 'set_fwd_ports_list()' to work even when port structure hasn't been
initialized yet.
I guess this implementation is much complex for readers of test-pmd code.

Option2:
Move initialization code of ports like below.

int main () {

        init_port(); /* only initialize port structure here*/
        launch_args_parse();
        init_config(); /* So far, port initialization code is
implemented in init_config() */

}

I guess 2nd option may be better.
How do you think?
I will send a patch to implement 2nd option.
if you are ok to fix like above, could you please check and acked it?

Thanks,
Tetsuya

> Thanks,
> Pablo
>>  			return;
>> -		}
>>  		if (record_now)
>>  			fwd_ports_ids[i] = port_id;
>>  	}




More information about the dev mailing list