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

[De Lara Guarch, Pablo]

Hi Tetsuya,

> -----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 ++++++++

[...]

> @@ -1446,8 +1497,8 @@ stop_port(portid_t pid)
>  	}
>  	printf("Stopping ports...\n");
> 
> -	for (pi = 0; pi < nb_ports; pi++) {
> -		if (pid < nb_ports && pid != pi)
> +	FOREACH_PORT(pi, ports) {
> +		if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi)

If using "port stop all", this function does not work as it should.
Problem is that pid = RTE_PORT_ALL = 255, and then ports[255].enabled is undefined, 
as ports array is allocated only for RTE_MAX_ETHPORTS (32 by default).

So, the solution could be either increasing the ports array to 256 items,
or check if we are passing RTE_PORT_ALL in pid.

What do you think?

Thanks,
Pablo

>  			continue;
> 
>  		port = &ports[pi];
> @@ -1463,7 +1514,7 @@ stop_port(portid_t pid)
>  		need_check_link_status = 1;
>  	}
>  	if (need_check_link_status && !no_link_check)
> -		check_all_ports_link_status(nb_ports, RTE_PORT_ALL);
> +		check_all_ports_link_status(RTE_PORT_ALL);
> 
>  	printf("Done\n");
>  }