[dpdk-dev,v6] 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 paramerters of virtual device.
         (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
 - port_id: port identifier

v5:
- Add testpmd documentation.
  (Thanks to Iremonger, Bernard)
v4:
 - Fix strings of command help.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 app/test-pmd/cmdline.c                      | 133 +++++++++++++++----
 app/test-pmd/config.c                       | 116 +++++++++-------
 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, 415 insertions(+), 130 deletions(-)
  

> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> Sent: Sunday, February 1, 2015 4:02 AM
> To: dev@dpdk.org
> Cc: Qiu, Michael; Iremonger, Bernard; Tetsuya Mukawa
> Subject: [PATCH v6] 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 paramerters of virtual device.
>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>  - port_id: port identifier
> 
> v5:
> - Add testpmd documentation.
>   (Thanks to Iremonger, Bernard)
> v4:
>  - Fix strings of command help.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  app/test-pmd/cmdline.c                      | 133 +++++++++++++++----
>  app/test-pmd/config.c                       | 116 +++++++++-------
>  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, 415 insertions(+), 130 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 4beb404..2f813d8 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -572,6 +572,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"port close (port_id|all)\n"
>  			"    Close all ports or port_id.\n\n"
> 
> +			"port attach (ident)\n"
> +			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
> +
> +			"port detach (port_id)\n"
> +			"    Detach physical or virtual dev by port_id\n\n"
> +
>  			"port config (port_id|all)"
>  			" speed (10|100|1000|10000|40000|auto)"
>  			" duplex (half|full|auto)\n"
> @@ -848,6 +854,89 @@ cmdline_parse_inst_t cmd_operate_specific_port = {
>  	},
>  };
> 
> +/* *** attach a specificied port *** */ struct
> +cmd_operate_attach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	cmdline_fixed_string_t identifier;
> +};
> +
> +static void cmd_operate_attach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data) {
> +	struct cmd_operate_attach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "attach"))
> +		attach_port(res->identifier);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_attach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_attach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			keyword, "attach");
> +cmdline_parse_token_string_t cmd_operate_attach_port_identifier =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			identifier, NULL);
> +
> +cmdline_parse_inst_t cmd_operate_attach_port = {
> +	.f = cmd_operate_attach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port attach identifier, "
> +		"identifier: pci address or virtual dev name",
> +	.tokens = {
> +		(void *)&cmd_operate_attach_port_port,
> +		(void *)&cmd_operate_attach_port_keyword,
> +		(void *)&cmd_operate_attach_port_identifier,
> +		NULL,
> +	},
> +};
> +
> +/* *** detach a specificied port *** */ struct
> +cmd_operate_detach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	uint8_t port_id;
> +};
> +
> +static void cmd_operate_detach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data) {
> +	struct cmd_operate_detach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "detach"))
> +		detach_port(res->port_id);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_detach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			keyword, "detach");
> +cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
> +	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port_id, UINT8);
> +
> +cmdline_parse_inst_t cmd_operate_detach_port = {
> +	.f = cmd_operate_detach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port detach port_id",
> +	.tokens = {
> +		(void *)&cmd_operate_detach_port_port,
> +		(void *)&cmd_operate_detach_port_keyword,
> +		(void *)&cmd_operate_detach_port_port_id,
> +		NULL,
> +	},
> +};
> +
>  /* *** configure speed for all ports *** */  struct cmd_config_speed_all {
>  	cmdline_fixed_string_t port;
> @@ -902,7 +991,7 @@ cmd_config_speed_all_parsed(void *parsed_result,
>  		return;
>  	}
> 
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		ports[pid].dev_conf.link_speed = link_speed;
>  		ports[pid].dev_conf.link_duplex = link_duplex;
>  	}
> @@ -970,10 +1059,8 @@ cmd_config_speed_specific_parsed(void *parsed_result,
>  		return;
>  	}
> 
> -	if (res->id >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->id, nb_ports);
> +	if (port_id_is_invalid(res->id, ENABLED_WARN))
>  		return;
> -	}
> 
>  	if (!strcmp(res->value1, "10"))
>  		link_speed = ETH_LINK_SPEED_10;
> @@ -1533,7 +1620,7 @@ cmd_config_rxtx_queue_parsed(void *parsed_result,
>  		return;
>  	}
> 
> -	if (port_id_is_invalid(res->portid))
> +	if (port_id_is_invalid(res->portid, ENABLED_WARN))
>  		return;
> 
>  	if (port_is_started(res->portid) != 1) { @@ -2876,7 +2963,7 @@ cmd_tx_cksum_parsed(void
> *parsed_result,
>  	uint16_t ol_flags, mask = 0;
>  	struct rte_eth_dev_info dev_info;
> 
> -	if (port_id_is_invalid(res->port_id)) {
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN)) {
>  		printf("invalid port %d\n", res->port_id);
>  		return;
>  	}
> @@ -3003,7 +3090,7 @@ cmd_tso_set_parsed(void *parsed_result,
>  	struct cmd_tso_set_result *res = parsed_result;
>  	struct rte_eth_dev_info dev_info;
> 
> -	if (port_id_is_invalid(res->port_id))
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>  		return;
> 
>  	if (!strcmp(res->mode, "set"))
> @@ -3979,10 +4066,8 @@ static void cmd_set_bond_mac_addr_parsed(void *parsed_result,
>  	struct cmd_set_bond_mac_addr_result *res = parsed_result;
>  	int ret;
> 
> -	if (res->port_num >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->port_num, nb_ports);
> +	if (port_id_is_invalid(res->port_num, ENABLED_WARN))
>  		return;
> -	}
> 
>  	ret = rte_eth_bond_mac_address_set(res->port_num, &res->address);
> 
> @@ -4219,7 +4304,7 @@ static void cmd_set_promisc_mode_parsed(void *parsed_result,
> 
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_promiscuous_enable(i);
>  			else
> @@ -4299,7 +4384,7 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result,
> 
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_allmulticast_enable(i);
>  			else
> @@ -5484,25 +5569,25 @@ static void cmd_showportall_parsed(void *parsed_result,
>  	struct cmd_showportall_result *res = parsed_result;
>  	if (!strcmp(res->show, "clear")) {
>  		if (!strcmp(res->what, "stats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_stats_clear(i);
>  		else if (!strcmp(res->what, "xstats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_xstats_clear(i);
>  	} else if (!strcmp(res->what, "info"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			port_infos_display(i);
>  	else if (!strcmp(res->what, "stats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_display(i);
>  	else if (!strcmp(res->what, "xstats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_xstats_display(i);
>  	else if (!strcmp(res->what, "fdir"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			fdir_get_infos(i);
>  	else if (!strcmp(res->what, "stat_qmap"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_mapping_display(i);
>  }
> 
> @@ -8756,6 +8841,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_set_qmap,
>  	(cmdline_parse_inst_t *)&cmd_operate_port,
>  	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_all,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
>  	(cmdline_parse_inst_t *)&cmd_config_rx_tx, @@ -8830,7 +8917,7 @@ prompt(void)  static
> void  cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)  {
> -	if (id < nb_ports) {
> +	if (!port_id_is_invalid(id, DISABLED_WARN)) {
>  		/* check if need_reconfig has been set to 1 */
>  		if (ports[id].need_reconfig == 0)
>  			ports[id].need_reconfig = dev;
> @@ -8840,7 +8927,7 @@ cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>  	} else {
>  		portid_t pid;
> 
> -		for (pid = 0; pid < nb_ports; pid++) {
> +		FOREACH_PORT(pid, ports) {
>  			/* check if need_reconfig has been set to 1 */
>  			if (ports[pid].need_reconfig == 0)
>  				ports[pid].need_reconfig = dev;
> @@ -8858,10 +8945,8 @@ bypass_is_supported(portid_t port_id)
>  	struct rte_port   *port;
>  	struct rte_pci_id *pci_id;
> 
> -	if (port_id >= nb_ports) {
> -		printf("\tPort id must be less than %d.\n", nb_ports);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 0;
> -	}
> 
>  	/* Get the device id. */
>  	port    = &ports[port_id];
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index c40f819..32d8f9a 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -124,11 +124,15 @@ nic_stats_display(portid_t port_id)
>  	struct rte_eth_stats stats;
>  	struct rte_port *port = &ports[port_id];
>  	uint8_t i;
> +	portid_t pid;
> 
>  	static const char *nic_stats_border = "########################";
> 
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_get(port_id, &stats);
> @@ -201,8 +205,13 @@ nic_stats_display(portid_t port_id)  void  nic_stats_clear(portid_t port_id)  {
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	portid_t pid;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_reset(port_id);
> @@ -249,11 +258,15 @@ nic_stats_mapping_display(portid_t port_id)  {
>  	struct rte_port *port = &ports[port_id];
>  	uint16_t i;
> +	portid_t pid;
> 
>  	static const char *nic_stats_mapping_border = "########################";
> 
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
> 
> @@ -302,9 +315,13 @@ port_infos_display(portid_t port_id)
>  	int vlan_offload;
>  	struct rte_mempool * mp;
>  	static const char *info_border = "*********************";
> +	portid_t pid;
> 
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	port = &ports[port_id];
> @@ -362,11 +379,14 @@ port_infos_display(portid_t port_id)  }
> 
>  int
> -port_id_is_invalid(portid_t port_id)
> +port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> -	if (port_id < nb_ports)
> +	if (ports[port_id].enabled)
>  		return 0;
> -	printf("Invalid port %d (must be < nb_ports=%d)\n", port_id, nb_ports);
> +
> +	if (warning == ENABLED_WARN)
> +		printf("Invalid port %d\n", port_id);
> +
>  	return 1;
>  }
> 
> @@ -425,7 +445,7 @@ port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)
>  	uint32_t reg_v;
> 
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -444,7 +464,7 @@ port_reg_bit_field_display(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -471,7 +491,7 @@ port_reg_display(portid_t port_id, uint32_t reg_off)  {
>  	uint32_t reg_v;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -485,7 +505,7 @@ port_reg_bit_set(portid_t port_id, uint32_t reg_off, uint8_t bit_pos,  {
>  	uint32_t reg_v;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -513,7 +533,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -547,7 +567,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,  void
> port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -560,7 +580,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>  	if (diag == 0)
> @@ -723,7 +743,7 @@ rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id)  {
>  	const struct rte_memzone *rx_mz;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (rx_queue_id_is_invalid(rxq_id))
>  		return;
> @@ -740,7 +760,7 @@ tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id)  {
>  	const struct rte_memzone *tx_mz;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (tx_queue_id_is_invalid(txq_id))
>  		return;
> @@ -796,7 +816,7 @@ port_rss_reta_info(portid_t port_id,
>  	uint16_t i, idx, shift;
>  	int ret;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	ret = rte_eth_dev_rss_reta_query(port_id, reta_conf, nb_entries); @@ -828,7 +848,7 @@
> port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
>  	uint8_t i;
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	/* Get RSS hash key if asked to display it */
>  	rss_conf.rss_key = (show_rss_key) ? rss_key : NULL; @@ -1406,12 +1426,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))
>  			return;
> -		}
>  		if (record_now)
>  			fwd_ports_ids[i] = port_id;
>  	}
> @@ -1569,7 +1585,7 @@ vlan_extend_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1591,7 +1607,7 @@ rx_vlan_strip_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1612,7 +1628,7 @@ rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_set_vlan_strip_on_queue(port_id, queue_id, on); @@ -1627,7 +1643,7
> @@ rx_vlan_filter_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1648,7 +1664,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1665,7 +1681,7 @@ rx_vlan_all_filter_set(portid_t port_id, int on)  {
>  	uint16_t vlan_id;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	for (vlan_id = 0; vlan_id < 4096; vlan_id++)
>  		rx_vft_set(port_id, vlan_id, on);
> @@ -1675,7 +1691,7 @@ void
>  vlan_tpid_set(portid_t port_id, uint16_t tp_id)  {
>  	int diag;
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_set_vlan_ether_type(port_id, tp_id); @@ -1690,7 +1706,7 @@
> vlan_tpid_set(portid_t port_id, uint16_t tp_id)  void  tx_vlan_set(portid_t port_id, uint16_t vlan_id)  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1701,7 +1717,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)  void  tx_vlan_reset(portid_t
> port_id)  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ports[port_id].tx_ol_flags &= ~TESTPMD_TX_OFFLOAD_INSERT_VLAN;  } @@ -1709,7 +1725,7
> @@ tx_vlan_reset(portid_t port_id)  void  tx_vlan_pvid_set(portid_t port_id, uint16_t vlan_id, int on)
> {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	rte_eth_dev_set_vlan_pvid(port_id, vlan_id, on); @@ -1721,7 +1737,7 @@
> set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
>  	uint16_t i;
>  	uint8_t existing_mapping_found = 0;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	if (is_rx ? (rx_queue_id_is_invalid(queue_id)) : (tx_queue_id_is_invalid(queue_id)))
> @@ -1773,7 +1789,7 @@ fdir_add_signature_filter(portid_t port_id, uint8_t queue_id,  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_add_signature_filter(port_id, fdir_filter, @@ -1791,7 +1807,7 @@
> fdir_update_signature_filter(portid_t port_id, uint8_t queue_id,  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_update_signature_filter(port_id, fdir_filter, @@ -1809,7 +1825,7 @@
> fdir_remove_signature_filter(portid_t port_id,  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_remove_signature_filter(port_id, fdir_filter); @@ -1881,7 +1897,7
> @@ fdir_get_infos(portid_t port_id)
> 
>  	static const char *fdir_stats_border = "########################";
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ret = rte_eth_dev_filter_supported(port_id, RTE_ETH_FILTER_FDIR);
>  	if (ret < 0) {
> @@ -1955,7 +1971,7 @@ fdir_add_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
> {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_add_perfect_filter(port_id, fdir_filter, @@ -1973,7 +1989,7 @@
> fdir_update_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_update_perfect_filter(port_id, fdir_filter, @@ -1991,7 +2007,7 @@
> fdir_remove_perfect_filter(portid_t port_id, uint16_t soft_id,  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_remove_perfect_filter(port_id, fdir_filter, @@ -2008,7 +2024,7 @@
> fdir_set_masks(portid_t port_id, struct rte_fdir_masks *fdir_masks)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
> 
>  	diag = rte_eth_dev_fdir_set_masks(port_id, fdir_masks); @@ -2085,7 +2101,7 @@
> set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (is_rx)
>  		diag = rte_eth_dev_set_vf_rx(port_id,vf,on);
> @@ -2107,7 +2123,7 @@ set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t
> on)  {
>  	int diag;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -2124,7 +2140,7 @@ set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate)
>  	int diag;
>  	struct rte_eth_link link;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> @@ -2149,7 +2165,7 @@ set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate, uint64_t
> q_msk)
>  	if (q_msk == 0)
>  		return 0;
> 
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index adf3203..6f2af18 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -376,6 +376,7 @@ parse_portnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
> 
>  	/* reset from value set at definition */
>  	while ((p = strchr(p0,'(')) != NULL) { @@ -397,8 +398,11 @@ parse_portnuma_config(const
> char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET]; @@ -429,6 +433,7 @@
> parse_ringnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
>  	#define RX_RING_ONLY 0x1
>  	#define TX_RING_ONLY 0x2
>  	#define RXTX_RING    0x3
> @@ -453,8 +458,11 @@ parse_ringnuma_config(const char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET]; @@ -626,12 +634,12 @@
> launch_args_parse(int argc, char** argv)  #endif
>  			if (!strcmp(lgopts[opt_idx].name, "nb-ports")) {
>  				n = atoi(optarg);
> -				if (n > 0 && n <= nb_ports)
> +				if (n > 0 &&
> +				    !port_id_is_invalid(n, DISABLED_WARN))
>  					nb_fwd_ports = (uint8_t) n;
>  				else
>  					rte_exit(EXIT_FAILURE,
> -						 "nb-ports should be > 0 and <= %d\n",
> -						 nb_ports);
> +						 "Invalid port %d\n", n);
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "nb-cores")) {
>  				n = atoi(optarg);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 773b8af..c18c1a9 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -71,6 +71,7 @@
>  #include <rte_pci.h>
>  #include <rte_ether.h>
>  #include <rte_ethdev.h>
> +#include <rte_dev.h>
>  #include <rte_string_fns.h>
>  #ifdef RTE_LIBRTE_PMD_XENVIRT
>  #include <rte_eth_xenvirt.h>
> @@ -315,7 +316,7 @@ uint16_t nb_rx_queue_stats_mappings = 0;
> 
>  /* Forward function declarations */
>  static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port); -static void
> check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
> +static void check_all_ports_link_status(uint32_t port_mask);
> 
>  /*
>   * Check if all the ports are started.
> @@ -324,6 +325,20 @@ static void check_all_ports_link_status(uint8_t port_num, uint32_t
> port_mask);  static int all_ports_started(void);
> 
>  /*
> + * Find next enabled port
> + */
> +portid_t
> +find_next_port(portid_t p, struct rte_port *ports, int size) {
> +	if (ports == NULL)
> +		rte_exit(-EINVAL, "failed to find a next port id\n");
> +
> +	while ((ports[p].enabled == 0) && (p < size))
> +		p++;
> +	return p;
> +}
> +
> +/*
>   * Setup default configuration.
>   */
>  static void
> @@ -552,7 +567,8 @@ init_config(void)
>  				+ RTE_TEST_TX_DESC_MAX + MAX_PKT_BURST;
> 
>  		if (!numa_support)
> -			nb_mbuf_per_pool = (nb_mbuf_per_pool * nb_ports);
> +			nb_mbuf_per_pool =
> +				(nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  	}
> 
>  	if (!numa_support) {
> @@ -565,14 +581,19 @@ init_config(void)
> 
>  	/* Configuration of Ethernet ports. */
>  	ports = rte_zmalloc("testpmd: ports",
> -			    sizeof(struct rte_port) * nb_ports,
> +			    sizeof(struct rte_port) * RTE_MAX_ETHPORTS,
>  			    RTE_CACHE_LINE_SIZE);
>  	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_zmalloc(%d struct rte_port) "
> -							"failed\n", nb_ports);
> +		rte_exit(EXIT_FAILURE,
> +				"rte_zmalloc(%d struct rte_port) failed\n",
> +				RTE_MAX_ETHPORTS);
>  	}
> 
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	/* enabled allocated ports */
> +	for (pid = 0; pid < nb_ports; pid++)
> +		ports[pid].enabled = 1;
> +
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		rte_eth_dev_info_get(pid, &port->dev_info);
> 
> @@ -602,8 +623,7 @@ init_config(void)
>  			nb_mbuf_per_pool = nb_mbuf_per_pool/nb_ports;
> 
>  		for (i = 0; i < MAX_SOCKET; i++) {
> -			nb_mbuf = (nb_mbuf_per_pool *
> -						port_per_socket[i]);
> +			nb_mbuf = (nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  			if (nb_mbuf)
>  				mbuf_pool_create(mbuf_data_size,
>  						nb_mbuf,i);
> @@ -635,14 +655,6 @@ reconfig(portid_t new_port_id, unsigned socket_id)
>  	struct rte_port *port;
> 
>  	/* Reconfiguration of Ethernet ports. */
> -	ports = rte_realloc(ports,
> -			    sizeof(struct rte_port) * nb_ports,
> -			    RTE_CACHE_LINE_SIZE);
> -	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_realloc(%d struct rte_port) failed\n",
> -				nb_ports);
> -	}
> -
>  	port = &ports[new_port_id];
>  	rte_eth_dev_info_get(new_port_id, &port->dev_info);
> 
> @@ -663,7 +675,7 @@ init_fwd_streams(void)
>  	streamid_t sm_id, nb_fwd_streams_new;
> 
>  	/* set socket id according to numa or not */
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		if (nb_rxq > port->dev_info.max_rx_queues) {
>  			printf("Fail: nb_rxq(%d) is greater than "
> @@ -1264,7 +1276,7 @@ all_ports_started(void)
>  	portid_t pi;
>  	struct rte_port *port;
> 
> -	for (pi = 0; pi < nb_ports; pi++) {
> +	FOREACH_PORT(pi, ports) {
>  		port = &ports[pi];
>  		/* Check if there is a port which is not started */
>  		if (port->port_status != RTE_PORT_STARTED) @@ -1276,6 +1288,45 @@
> all_ports_started(void)  }
> 
>  int
> +all_ports_stopped(void)
> +{
> +	portid_t pi;
> +	struct rte_port *port;
> +
> +	FOREACH_PORT(pi, ports) {
> +		port = &ports[pi];
> +		if (port->port_status != RTE_PORT_STOPPED)
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +int
> +port_is_started(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_STARTED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int
> +port_is_closed(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +int
>  start_port(portid_t pid)
>  {
>  	int diag, need_check_link_status = 0;
> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
> 
>  	if(dcb_config)
>  		dcb_test = 1;
> -	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)
>  			continue;
> 
>  		port = &ports[pi];
> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>  	}
> 
>  	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);
>  	else
>  		printf("Please stop the ports first\n");
> 
> @@ -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)
>  			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");
>  }
> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
> 
>  	printf("Closing 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)
>  			continue;
> 
>  		port = &ports[pi];
> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>  	printf("Done\n");
>  }
> 
> -int
> -all_ports_stopped(void)
> +void
> +attach_port(char *identifier)
>  {
> -	portid_t pi;
> -	struct rte_port *port;
> +	portid_t i, j, pi = 0;
> 
> -	for (pi = 0; pi < nb_ports; pi++) {
> -		port = &ports[pi];
> -		if (port->port_status != RTE_PORT_STOPPED)
> -			return 0;
> +	printf("Attaching a new port...\n");
> +
> +	if (identifier == NULL) {
> +		printf("Invalid parameters are speficied\n");
> +		return;
>  	}
> 
> -	return 1;
> +	if (test_done == 0) {
> +		printf("Please stop forwarding first\n");
> +		return;
> +	}
> +
> +	if (rte_eal_dev_attach(identifier, &pi))
> +		return;
> +
> +	ports[pi].enabled = 1;
> +	reconfig(pi, rte_eth_dev_socket_id(pi));
> +	rte_eth_promiscuous_enable(pi);
> +
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(j, ports) {
> +		fwd_ports_ids[i] = j;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports++;
> +
> +	ports[pi].port_status = RTE_PORT_STOPPED;
> +
> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
> +	printf("Done\n");
>  }
> 
> -int
> -port_is_started(portid_t port_id)
> +void
> +detach_port(uint8_t port_id)
>  {
> -	if (port_id_is_invalid(port_id))
> -		return -1;
> +	portid_t i, pi = 0;
> +	char name[RTE_ETH_NAME_MAX_LEN];
> 
> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
> -		return 0;
> +	printf("Detaching a port...\n");
> 
> -	return 1;
> +	if (!port_is_closed(port_id)) {
> +		printf("Please close port first\n");
> +		return;
> +	}
> +
> +	rte_eth_promiscuous_disable(port_id);
> +
> +	if (rte_eal_dev_detach(port_id, name))
> +		return;
> +
> +	ports[port_id].enabled = 0;
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(pi, ports) {
> +		fwd_ports_ids[i] = pi;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports--;
> +
> +	printf("Port '%s' is detached. Now total ports is %d\n",
> +			name, nb_ports);
> +	printf("Done\n");
> +	return;
>  }
> 
>  void
> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)  {
>  	portid_t pt_id;
> 
> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
> +	FOREACH_PORT(pt_id, ports) {
>  		printf("Stopping port %d...", pt_id);
>  		fflush(stdout);
>  		rte_eth_dev_close(pt_id);
> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
> 
>  /* Check the link status of all ports in up to 9s, and print them finally */  static void -
> check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
> +check_all_ports_link_status(uint32_t port_mask)
>  {
>  #define CHECK_INTERVAL 100 /* 100ms */
>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */ @@ -1564,7 +1667,7 @@
> check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>  	fflush(stdout);
>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>  		all_ports_up = 1;
> -		for (portid = 0; portid < port_num; portid++) {
> +		FOREACH_PORT(portid, ports) {
>  			if ((port_mask & (1 << portid)) == 0)
>  				continue;
>  			memset(&link, 0, sizeof(link));
> @@ -1688,7 +1791,7 @@ init_port_config(void)
>  	portid_t pid;
>  	struct rte_port *port;
> 
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		port->dev_conf.rxmode = rx_mode;
>  		port->dev_conf.fdir_conf = fdir_conf; @@ -1877,7 +1980,7 @@ main(int argc, char**
> argv)
> 
>  	nb_ports = (portid_t) rte_eth_dev_count();
>  	if (nb_ports == 0)
> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
> 
>  	set_def_fwd_config();
>  	if (nb_lcores == 0)
> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
> 
>  	/* set all ports to promiscuous mode by default */
> -	for (port_id = 0; port_id < nb_ports; port_id++)
> +	FOREACH_PORT(port_id, ports)
>  		rte_eth_promiscuous_enable(port_id);
> 
>  #ifdef RTE_LIBRTE_CMDLINE
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index 8f5e6c7..109c670 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -134,6 +134,7 @@ struct fwd_stream {
>   * The data structure associated with each port.
>   */
>  struct rte_port {
> +	uint8_t                 enabled;    /**< Port enabled or not */
>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
> @@ -159,6 +160,14 @@ struct rte_port {
>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>  };
> 
> +extern portid_t __rte_unused
> +find_next_port(portid_t p, struct rte_port *ports, int size);
> +
> +#define FOREACH_PORT(p, ports) \
> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
> +	    p < RTE_MAX_ETHPORTS; \
> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
> +
>  /**
>   * The data structure associated with each forwarding logical core.
>   * The logical cores are internally numbered by a core index from 0 to @@ -515,6 +524,8 @@ int
> init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);  int start_port(portid_t pid);  void
> stop_port(portid_t pid);  void close_port(portid_t pid);
> +void attach_port(char *identifier);
> +void detach_port(uint8_t port_id);
>  int all_ports_stopped(void);
>  int port_is_started(portid_t port_id);
>  void pmd_test_exit(void);
> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);  void
> get_2tuple_filter(uint8_t port_id, uint16_t index);  void get_5tuple_filter(uint8_t port_id, uint16_t
> index);  void get_flex_filter(uint8_t port_id, uint16_t index); -int port_id_is_invalid(portid_t port_id);
> int rx_queue_id_is_invalid(queueid_t rxq_id);  int tx_queue_id_is_invalid(queueid_t txq_id);
> 
> +enum print_warning {
> +	ENABLED_WARN = 0,
> +	DISABLED_WARN
> +};
> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
> +
>  /*
>   * Work-around of a compilation error with ICC on invocations of the
>   * rte_be_to_cpu_16() function.
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 218835a..1cacbcf 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
Hi Tetsuya,

The doc changes should be in separate commit using the "doc:   explaination"   commit line.

> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
> 
>      Port configuration changes only become active when forwarding is started/restarted.
> 
> +port attach
> +~~~~~~~~~~~
> +
> +Attach a port specified by pci address or virtual device args.
> +
> +To attach a new pci device, the device should be recognized by kernel first.
> +Then it should be moved under DPDK management.
> +Finally the port can be attached to testpmd.
> +On the other hand, to attach a port created by virtual device, above steps are not needed.
> +
> +port attach (identifier)
> +
> +For example, to attach a port that pci address is 0000:02:00.0.

Reword " port that pci address is "  to "port whose pci address is"

> +
> +.. code-block:: console
> +
> +    testpmd> port attach 0000:02:00.0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +For example, to attach a port created by pcap PMD.
> +
> +.. code-block:: console
> +
> +    testpmd> port attach eth_pcap0,iface=eth0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +In this case, identifier is "eth_pcap0,iface=eth0".
> +This identifier format is the same as "--vdev" format of DPDK applications.
> +
> +port detach
> +~~~~~~~~~~~
> +
> +Detach a specific port.
> +
> +Before detaching a port, the port should be closed.
> +Also to remove a pci device completely from the system, first detach the port from testpmd.
> +Then the device should be moved under kernel management.
> +Finally the device can be remove using kernel pci hotplug functionality.

Reword "remove" to "removed"

> +On the other hand, to remove a port created by virtual device, above steps are not needed.

Reword " created by virtual device" to "created by a virtual device"

> +
> +port detach (port_id)
> +
> +For example, to detach a port 0.
> +
> +.. code-block:: console
> +
> +    testpmd> port detach 0
> +    Detaching a port...
> +    ... snip ...
> +    Done
> +
>  port start
>  ~~~~~~~~~~
> 
> --
> 1.9.1

Regards,

Bernard.
  

2015-02-02 11:33, Iremonger, Bernard:
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> Hi Tetsuya,
> 
> The doc changes should be in separate commit using the "doc:   explaination"   commit line.

I agree that new docs should be in a separate commit.
Though when updating some code (like here), it's a good idea to update the doc
in the same commit. Then the change is atomic.

If you want to figure which patches are updating the doc, it should be possible
to have a filter on "+++ b/doc/".
  

On 2015/02/02 20:33, Iremonger, Bernard wrote
>>  /*
>>   * Work-around of a compilation error with ICC on invocations of the
>>   * rte_be_to_cpu_16() function.
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 218835a..1cacbcf 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> Hi Tetsuya,
>
> The doc changes should be in separate commit using the "doc:   explaination"   commit line.
>
>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>
>>      Port configuration changes only become active when forwarding is started/restarted.
>>
>> +port attach
>> +~~~~~~~~~~~
>> +
>> +Attach a port specified by pci address or virtual device args.
>> +
>> +To attach a new pci device, the device should be recognized by kernel first.
>> +Then it should be moved under DPDK management.
>> +Finally the port can be attached to testpmd.
>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>> +
>> +port attach (identifier)
>> +
>> +For example, to attach a port that pci address is 0000:02:00.0.
> Reword " port that pci address is "  to "port whose pci address is"

Hi Bernard,

Thanks, I will fix below comments like your suggestion.

Tetsuya

>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach 0000:02:00.0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +For example, to attach a port created by pcap PMD.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach eth_pcap0,iface=eth0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +In this case, identifier is "eth_pcap0,iface=eth0".
>> +This identifier format is the same as "--vdev" format of DPDK applications.
>> +
>> +port detach
>> +~~~~~~~~~~~
>> +
>> +Detach a specific port.
>> +
>> +Before detaching a port, the port should be closed.
>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>> +Then the device should be moved under kernel management.
>> +Finally the device can be remove using kernel pci hotplug functionality.
> Reword "remove" to "removed"
>
>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
> Reword " created by virtual device" to "created by a virtual device"
>
>> +
>> +port detach (port_id)
>> +
>> +For example, to detach a port 0.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port detach 0
>> +    Detaching a port...
>> +    ... snip ...
>> +    Done
>> +
>>  port start
>>  ~~~~~~~~~~
>>
>> --
>> 1.9.1
> Regards,
>
> Bernard.
>
  

On 2015/02/02 20:57, Thomas Monjalon wrote:
> 2015-02-02 11:33, Iremonger, Bernard:
>> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> Hi Tetsuya,
>>
>> The doc changes should be in separate commit using the "doc:   explaination"   commit line.
> I agree that new docs should be in a separate commit.
> Though when updating some code (like here), it's a good idea to update the doc
> in the same commit. Then the change is atomic.
>
> If you want to figure which patches are updating the doc, it should be possible
> to have a filter on "+++ b/doc/".
>

Hi Thomas,

I appreciate your comment. I will follow this guideline.

Thanks,
Tetsuya
  

On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
> 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 paramerters of virtual device.
>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>  - port_id: port identifier
>
> v5:
> - Add testpmd documentation.
>   (Thanks to Iremonger, Bernard)
> v4:
>  - Fix strings of command help.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  app/test-pmd/cmdline.c                      | 133 +++++++++++++++----
>  app/test-pmd/config.c                       | 116 +++++++++-------
>  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, 415 insertions(+), 130 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 4beb404..2f813d8 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -572,6 +572,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"port close (port_id|all)\n"
>  			"    Close all ports or port_id.\n\n"
>  
> +			"port attach (ident)\n"
> +			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
> +
> +			"port detach (port_id)\n"
> +			"    Detach physical or virtual dev by port_id\n\n"
> +
>  			"port config (port_id|all)"
>  			" speed (10|100|1000|10000|40000|auto)"
>  			" duplex (half|full|auto)\n"
> @@ -848,6 +854,89 @@ cmdline_parse_inst_t cmd_operate_specific_port = {
>  	},
>  };
>  
> +/* *** attach a specificied port *** */
> +struct cmd_operate_attach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	cmdline_fixed_string_t identifier;
> +};
> +
> +static void cmd_operate_attach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data)
> +{
> +	struct cmd_operate_attach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "attach"))
> +		attach_port(res->identifier);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_attach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_attach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			keyword, "attach");
> +cmdline_parse_token_string_t cmd_operate_attach_port_identifier =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			identifier, NULL);
> +
> +cmdline_parse_inst_t cmd_operate_attach_port = {
> +	.f = cmd_operate_attach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port attach identifier, "
> +		"identifier: pci address or virtual dev name",
> +	.tokens = {
> +		(void *)&cmd_operate_attach_port_port,
> +		(void *)&cmd_operate_attach_port_keyword,
> +		(void *)&cmd_operate_attach_port_identifier,
> +		NULL,
> +	},
> +};
> +
> +/* *** detach a specificied port *** */
> +struct cmd_operate_detach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	uint8_t port_id;
> +};
> +
> +static void cmd_operate_detach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data)
> +{
> +	struct cmd_operate_detach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "detach"))
> +		detach_port(res->port_id);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_detach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			keyword, "detach");
> +cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
> +	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port_id, UINT8);
> +
> +cmdline_parse_inst_t cmd_operate_detach_port = {
> +	.f = cmd_operate_detach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port detach port_id",
> +	.tokens = {
> +		(void *)&cmd_operate_detach_port_port,
> +		(void *)&cmd_operate_detach_port_keyword,
> +		(void *)&cmd_operate_detach_port_port_id,
> +		NULL,
> +	},
> +};
> +
>  /* *** configure speed for all ports *** */
>  struct cmd_config_speed_all {
>  	cmdline_fixed_string_t port;
> @@ -902,7 +991,7 @@ cmd_config_speed_all_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		ports[pid].dev_conf.link_speed = link_speed;
>  		ports[pid].dev_conf.link_duplex = link_duplex;
>  	}
> @@ -970,10 +1059,8 @@ cmd_config_speed_specific_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	if (res->id >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->id, nb_ports);
> +	if (port_id_is_invalid(res->id, ENABLED_WARN))
>  		return;
> -	}
>  
>  	if (!strcmp(res->value1, "10"))
>  		link_speed = ETH_LINK_SPEED_10;
> @@ -1533,7 +1620,7 @@ cmd_config_rxtx_queue_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	if (port_id_is_invalid(res->portid))
> +	if (port_id_is_invalid(res->portid, ENABLED_WARN))
>  		return;
>  
>  	if (port_is_started(res->portid) != 1) {
> @@ -2876,7 +2963,7 @@ cmd_tx_cksum_parsed(void *parsed_result,
>  	uint16_t ol_flags, mask = 0;
>  	struct rte_eth_dev_info dev_info;
>  
> -	if (port_id_is_invalid(res->port_id)) {
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN)) {
>  		printf("invalid port %d\n", res->port_id);
>  		return;
>  	}
> @@ -3003,7 +3090,7 @@ cmd_tso_set_parsed(void *parsed_result,
>  	struct cmd_tso_set_result *res = parsed_result;
>  	struct rte_eth_dev_info dev_info;
>  
> -	if (port_id_is_invalid(res->port_id))
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>  		return;
>  
>  	if (!strcmp(res->mode, "set"))
> @@ -3979,10 +4066,8 @@ static void cmd_set_bond_mac_addr_parsed(void *parsed_result,
>  	struct cmd_set_bond_mac_addr_result *res = parsed_result;
>  	int ret;
>  
> -	if (res->port_num >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->port_num, nb_ports);
> +	if (port_id_is_invalid(res->port_num, ENABLED_WARN))
>  		return;
> -	}
>  
>  	ret = rte_eth_bond_mac_address_set(res->port_num, &res->address);
>  
> @@ -4219,7 +4304,7 @@ static void cmd_set_promisc_mode_parsed(void *parsed_result,
>  
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_promiscuous_enable(i);
>  			else
> @@ -4299,7 +4384,7 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result,
>  
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_allmulticast_enable(i);
>  			else
> @@ -5484,25 +5569,25 @@ static void cmd_showportall_parsed(void *parsed_result,
>  	struct cmd_showportall_result *res = parsed_result;
>  	if (!strcmp(res->show, "clear")) {
>  		if (!strcmp(res->what, "stats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_stats_clear(i);
>  		else if (!strcmp(res->what, "xstats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_xstats_clear(i);
>  	} else if (!strcmp(res->what, "info"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			port_infos_display(i);
>  	else if (!strcmp(res->what, "stats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_display(i);
>  	else if (!strcmp(res->what, "xstats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_xstats_display(i);
>  	else if (!strcmp(res->what, "fdir"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			fdir_get_infos(i);
>  	else if (!strcmp(res->what, "stat_qmap"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_mapping_display(i);
>  }
>  
> @@ -8756,6 +8841,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_set_qmap,
>  	(cmdline_parse_inst_t *)&cmd_operate_port,
>  	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_all,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
>  	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
> @@ -8830,7 +8917,7 @@ prompt(void)
>  static void
>  cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>  {
> -	if (id < nb_ports) {
> +	if (!port_id_is_invalid(id, DISABLED_WARN)) {
>  		/* check if need_reconfig has been set to 1 */
>  		if (ports[id].need_reconfig == 0)
>  			ports[id].need_reconfig = dev;
> @@ -8840,7 +8927,7 @@ cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>  	} else {
>  		portid_t pid;
>  
> -		for (pid = 0; pid < nb_ports; pid++) {
> +		FOREACH_PORT(pid, ports) {
>  			/* check if need_reconfig has been set to 1 */
>  			if (ports[pid].need_reconfig == 0)
>  				ports[pid].need_reconfig = dev;
> @@ -8858,10 +8945,8 @@ bypass_is_supported(portid_t port_id)
>  	struct rte_port   *port;
>  	struct rte_pci_id *pci_id;
>  
> -	if (port_id >= nb_ports) {
> -		printf("\tPort id must be less than %d.\n", nb_ports);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 0;
> -	}
>  
>  	/* Get the device id. */
>  	port    = &ports[port_id];
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index c40f819..32d8f9a 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -124,11 +124,15 @@ nic_stats_display(portid_t port_id)
>  	struct rte_eth_stats stats;
>  	struct rte_port *port = &ports[port_id];
>  	uint8_t i;
> +	portid_t pid;
>  
>  	static const char *nic_stats_border = "########################";
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_get(port_id, &stats);
> @@ -201,8 +205,13 @@ nic_stats_display(portid_t port_id)
>  void
>  nic_stats_clear(portid_t port_id)
>  {
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	portid_t pid;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_reset(port_id);
> @@ -249,11 +258,15 @@ nic_stats_mapping_display(portid_t port_id)
>  {
>  	struct rte_port *port = &ports[port_id];
>  	uint16_t i;
> +	portid_t pid;
>  
>  	static const char *nic_stats_mapping_border = "########################";
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  
> @@ -302,9 +315,13 @@ port_infos_display(portid_t port_id)
>  	int vlan_offload;
>  	struct rte_mempool * mp;
>  	static const char *info_border = "*********************";
> +	portid_t pid;
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	port = &ports[port_id];
> @@ -362,11 +379,14 @@ port_infos_display(portid_t port_id)
>  }
>  
>  int
> -port_id_is_invalid(portid_t port_id)
> +port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> -	if (port_id < nb_ports)
> +	if (ports[port_id].enabled)
>  		return 0;
> -	printf("Invalid port %d (must be < nb_ports=%d)\n", port_id, nb_ports);
> +
> +	if (warning == ENABLED_WARN)
> +		printf("Invalid port %d\n", port_id);
> +
>  	return 1;
>  }
>  
> @@ -425,7 +445,7 @@ port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)
>  	uint32_t reg_v;
>  
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -444,7 +464,7 @@ port_reg_bit_field_display(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -471,7 +491,7 @@ port_reg_display(portid_t port_id, uint32_t reg_off)
>  {
>  	uint32_t reg_v;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -485,7 +505,7 @@ port_reg_bit_set(portid_t port_id, uint32_t reg_off, uint8_t bit_pos,
>  {
>  	uint32_t reg_v;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -513,7 +533,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -547,7 +567,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>  void
>  port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -560,7 +580,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>  	if (diag == 0)
> @@ -723,7 +743,7 @@ rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id)
>  {
>  	const struct rte_memzone *rx_mz;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (rx_queue_id_is_invalid(rxq_id))
>  		return;
> @@ -740,7 +760,7 @@ tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id)
>  {
>  	const struct rte_memzone *tx_mz;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (tx_queue_id_is_invalid(txq_id))
>  		return;
> @@ -796,7 +816,7 @@ port_rss_reta_info(portid_t port_id,
>  	uint16_t i, idx, shift;
>  	int ret;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	ret = rte_eth_dev_rss_reta_query(port_id, reta_conf, nb_entries);
> @@ -828,7 +848,7 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
>  	uint8_t i;
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	/* Get RSS hash key if asked to display it */
>  	rss_conf.rss_key = (show_rss_key) ? rss_key : NULL;
> @@ -1406,12 +1426,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))
>  			return;
> -		}
>  		if (record_now)
>  			fwd_ports_ids[i] = port_id;
>  	}
> @@ -1569,7 +1585,7 @@ vlan_extend_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1591,7 +1607,7 @@ rx_vlan_strip_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1612,7 +1628,7 @@ rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_set_vlan_strip_on_queue(port_id, queue_id, on);
> @@ -1627,7 +1643,7 @@ rx_vlan_filter_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1648,7 +1664,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1665,7 +1681,7 @@ rx_vlan_all_filter_set(portid_t port_id, int on)
>  {
>  	uint16_t vlan_id;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	for (vlan_id = 0; vlan_id < 4096; vlan_id++)
>  		rx_vft_set(port_id, vlan_id, on);
> @@ -1675,7 +1691,7 @@ void
>  vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>  {
>  	int diag;
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_set_vlan_ether_type(port_id, tp_id);
> @@ -1690,7 +1706,7 @@ vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>  void
>  tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1701,7 +1717,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  void
>  tx_vlan_reset(portid_t port_id)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ports[port_id].tx_ol_flags &= ~TESTPMD_TX_OFFLOAD_INSERT_VLAN;
>  }
> @@ -1709,7 +1725,7 @@ tx_vlan_reset(portid_t port_id)
>  void
>  tx_vlan_pvid_set(portid_t port_id, uint16_t vlan_id, int on)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	rte_eth_dev_set_vlan_pvid(port_id, vlan_id, on);
> @@ -1721,7 +1737,7 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
>  	uint16_t i;
>  	uint8_t existing_mapping_found = 0;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	if (is_rx ? (rx_queue_id_is_invalid(queue_id)) : (tx_queue_id_is_invalid(queue_id)))
> @@ -1773,7 +1789,7 @@ fdir_add_signature_filter(portid_t port_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_add_signature_filter(port_id, fdir_filter,
> @@ -1791,7 +1807,7 @@ fdir_update_signature_filter(portid_t port_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_update_signature_filter(port_id, fdir_filter,
> @@ -1809,7 +1825,7 @@ fdir_remove_signature_filter(portid_t port_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_remove_signature_filter(port_id, fdir_filter);
> @@ -1881,7 +1897,7 @@ fdir_get_infos(portid_t port_id)
>  
>  	static const char *fdir_stats_border = "########################";
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ret = rte_eth_dev_filter_supported(port_id, RTE_ETH_FILTER_FDIR);
>  	if (ret < 0) {
> @@ -1955,7 +1971,7 @@ fdir_add_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_add_perfect_filter(port_id, fdir_filter,
> @@ -1973,7 +1989,7 @@ fdir_update_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_update_perfect_filter(port_id, fdir_filter,
> @@ -1991,7 +2007,7 @@ fdir_remove_perfect_filter(portid_t port_id, uint16_t soft_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_remove_perfect_filter(port_id, fdir_filter,
> @@ -2008,7 +2024,7 @@ fdir_set_masks(portid_t port_id, struct rte_fdir_masks *fdir_masks)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_set_masks(port_id, fdir_masks);
> @@ -2085,7 +2101,7 @@ set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (is_rx)
>  		diag = rte_eth_dev_set_vf_rx(port_id,vf,on);
> @@ -2107,7 +2123,7 @@ set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -2124,7 +2140,7 @@ set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate)
>  	int diag;
>  	struct rte_eth_link link;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> @@ -2149,7 +2165,7 @@ set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate, uint64_t q_msk)
>  	if (q_msk == 0)
>  		return 0;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index adf3203..6f2af18 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -376,6 +376,7 @@ parse_portnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
>  
>  	/* reset from value set at definition */
>  	while ((p = strchr(p0,'(')) != NULL) {
> @@ -397,8 +398,11 @@ parse_portnuma_config(const char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> @@ -429,6 +433,7 @@ parse_ringnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
>  	#define RX_RING_ONLY 0x1
>  	#define TX_RING_ONLY 0x2
>  	#define RXTX_RING    0x3
> @@ -453,8 +458,11 @@ parse_ringnuma_config(const char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> @@ -626,12 +634,12 @@ launch_args_parse(int argc, char** argv)
>  #endif
>  			if (!strcmp(lgopts[opt_idx].name, "nb-ports")) {
>  				n = atoi(optarg);
> -				if (n > 0 && n <= nb_ports)
> +				if (n > 0 &&
> +				    !port_id_is_invalid(n, DISABLED_WARN))
>  					nb_fwd_ports = (uint8_t) n;
>  				else
>  					rte_exit(EXIT_FAILURE,
> -						 "nb-ports should be > 0 and <= %d\n",
> -						 nb_ports);
> +						 "Invalid port %d\n", n);
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "nb-cores")) {
>  				n = atoi(optarg);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 773b8af..c18c1a9 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -71,6 +71,7 @@
>  #include <rte_pci.h>
>  #include <rte_ether.h>
>  #include <rte_ethdev.h>
> +#include <rte_dev.h>
>  #include <rte_string_fns.h>
>  #ifdef RTE_LIBRTE_PMD_XENVIRT
>  #include <rte_eth_xenvirt.h>
> @@ -315,7 +316,7 @@ uint16_t nb_rx_queue_stats_mappings = 0;
>  
>  /* Forward function declarations */
>  static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port);
> -static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
> +static void check_all_ports_link_status(uint32_t port_mask);
>  
>  /*
>   * Check if all the ports are started.
> @@ -324,6 +325,20 @@ static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
>  static int all_ports_started(void);
>  
>  /*
> + * Find next enabled port
> + */
> +portid_t
> +find_next_port(portid_t p, struct rte_port *ports, int size)
> +{
> +	if (ports == NULL)
> +		rte_exit(-EINVAL, "failed to find a next port id\n");
> +
> +	while ((ports[p].enabled == 0) && (p < size))
> +		p++;
> +	return p;
> +}
> +
> +/*
>   * Setup default configuration.
>   */
>  static void
> @@ -552,7 +567,8 @@ init_config(void)
>  				+ RTE_TEST_TX_DESC_MAX + MAX_PKT_BURST;
>  
>  		if (!numa_support)
> -			nb_mbuf_per_pool = (nb_mbuf_per_pool * nb_ports);
> +			nb_mbuf_per_pool =
> +				(nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  	}
>  
>  	if (!numa_support) {
> @@ -565,14 +581,19 @@ init_config(void)
>  
>  	/* Configuration of Ethernet ports. */
>  	ports = rte_zmalloc("testpmd: ports",
> -			    sizeof(struct rte_port) * nb_ports,
> +			    sizeof(struct rte_port) * RTE_MAX_ETHPORTS,
>  			    RTE_CACHE_LINE_SIZE);
>  	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_zmalloc(%d struct rte_port) "
> -							"failed\n", nb_ports);
> +		rte_exit(EXIT_FAILURE,
> +				"rte_zmalloc(%d struct rte_port) failed\n",
> +				RTE_MAX_ETHPORTS);
>  	}
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	/* enabled allocated ports */
> +	for (pid = 0; pid < nb_ports; pid++)
> +		ports[pid].enabled = 1;
> +
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		rte_eth_dev_info_get(pid, &port->dev_info);
>  
> @@ -602,8 +623,7 @@ init_config(void)
>  			nb_mbuf_per_pool = nb_mbuf_per_pool/nb_ports;
>  
>  		for (i = 0; i < MAX_SOCKET; i++) {
> -			nb_mbuf = (nb_mbuf_per_pool *
> -						port_per_socket[i]);
> +			nb_mbuf = (nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  			if (nb_mbuf)
>  				mbuf_pool_create(mbuf_data_size,
>  						nb_mbuf,i);
> @@ -635,14 +655,6 @@ reconfig(portid_t new_port_id, unsigned socket_id)
>  	struct rte_port *port;
>  
>  	/* Reconfiguration of Ethernet ports. */
> -	ports = rte_realloc(ports,
> -			    sizeof(struct rte_port) * nb_ports,
> -			    RTE_CACHE_LINE_SIZE);
> -	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_realloc(%d struct rte_port) failed\n",
> -				nb_ports);
> -	}
> -
>  	port = &ports[new_port_id];
>  	rte_eth_dev_info_get(new_port_id, &port->dev_info);
>  
> @@ -663,7 +675,7 @@ init_fwd_streams(void)
>  	streamid_t sm_id, nb_fwd_streams_new;
>  
>  	/* set socket id according to numa or not */
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		if (nb_rxq > port->dev_info.max_rx_queues) {
>  			printf("Fail: nb_rxq(%d) is greater than "
> @@ -1264,7 +1276,7 @@ all_ports_started(void)
>  	portid_t pi;
>  	struct rte_port *port;
>  
> -	for (pi = 0; pi < nb_ports; pi++) {
> +	FOREACH_PORT(pi, ports) {
>  		port = &ports[pi];
>  		/* Check if there is a port which is not started */
>  		if (port->port_status != RTE_PORT_STARTED)
> @@ -1276,6 +1288,45 @@ all_ports_started(void)
>  }
>  
>  int
> +all_ports_stopped(void)
> +{
> +	portid_t pi;
> +	struct rte_port *port;
> +
> +	FOREACH_PORT(pi, ports) {
> +		port = &ports[pi];
> +		if (port->port_status != RTE_PORT_STOPPED)
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +int
> +port_is_started(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_STARTED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int
> +port_is_closed(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +int
>  start_port(portid_t pid)
>  {
>  	int diag, need_check_link_status = 0;
> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>  
>  	if(dcb_config)
>  		dcb_test = 1;
> -	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)

Here may it be:

if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid != pi || pid == RET_PORT_ALL))

Otherwise no port will be start by default.


Thanks,
Michael

>  			continue;
>  
>  		port = &ports[pi];
> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>  	}
>  
>  	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);
>  	else
>  		printf("Please stop the ports first\n");
>  
> @@ -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)
>  			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");
>  }
> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>  
>  	printf("Closing 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)
>  			continue;
>  
>  		port = &ports[pi];
> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>  	printf("Done\n");
>  }
>  
> -int
> -all_ports_stopped(void)
> +void
> +attach_port(char *identifier)
>  {
> -	portid_t pi;
> -	struct rte_port *port;
> +	portid_t i, j, pi = 0;
>  
> -	for (pi = 0; pi < nb_ports; pi++) {
> -		port = &ports[pi];
> -		if (port->port_status != RTE_PORT_STOPPED)
> -			return 0;
> +	printf("Attaching a new port...\n");
> +
> +	if (identifier == NULL) {
> +		printf("Invalid parameters are speficied\n");
> +		return;
>  	}
>  
> -	return 1;
> +	if (test_done == 0) {
> +		printf("Please stop forwarding first\n");
> +		return;
> +	}
> +
> +	if (rte_eal_dev_attach(identifier, &pi))
> +		return;
> +
> +	ports[pi].enabled = 1;
> +	reconfig(pi, rte_eth_dev_socket_id(pi));
> +	rte_eth_promiscuous_enable(pi);
> +
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(j, ports) {
> +		fwd_ports_ids[i] = j;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports++;
> +
> +	ports[pi].port_status = RTE_PORT_STOPPED;
> +
> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
> +	printf("Done\n");
>  }
>  
> -int
> -port_is_started(portid_t port_id)
> +void
> +detach_port(uint8_t port_id)
>  {
> -	if (port_id_is_invalid(port_id))
> -		return -1;
> +	portid_t i, pi = 0;
> +	char name[RTE_ETH_NAME_MAX_LEN];
>  
> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
> -		return 0;
> +	printf("Detaching a port...\n");
>  
> -	return 1;
> +	if (!port_is_closed(port_id)) {
> +		printf("Please close port first\n");
> +		return;
> +	}
> +
> +	rte_eth_promiscuous_disable(port_id);
> +
> +	if (rte_eal_dev_detach(port_id, name))
> +		return;
> +
> +	ports[port_id].enabled = 0;
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(pi, ports) {
> +		fwd_ports_ids[i] = pi;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports--;
> +
> +	printf("Port '%s' is detached. Now total ports is %d\n",
> +			name, nb_ports);
> +	printf("Done\n");
> +	return;
>  }
>  
>  void
> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>  {
>  	portid_t pt_id;
>  
> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
> +	FOREACH_PORT(pt_id, ports) {
>  		printf("Stopping port %d...", pt_id);
>  		fflush(stdout);
>  		rte_eth_dev_close(pt_id);
> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>  
>  /* Check the link status of all ports in up to 9s, and print them finally */
>  static void
> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
> +check_all_ports_link_status(uint32_t port_mask)
>  {
>  #define CHECK_INTERVAL 100 /* 100ms */
>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>  	fflush(stdout);
>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>  		all_ports_up = 1;
> -		for (portid = 0; portid < port_num; portid++) {
> +		FOREACH_PORT(portid, ports) {
>  			if ((port_mask & (1 << portid)) == 0)
>  				continue;
>  			memset(&link, 0, sizeof(link));
> @@ -1688,7 +1791,7 @@ init_port_config(void)
>  	portid_t pid;
>  	struct rte_port *port;
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		port->dev_conf.rxmode = rx_mode;
>  		port->dev_conf.fdir_conf = fdir_conf;
> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>  
>  	nb_ports = (portid_t) rte_eth_dev_count();
>  	if (nb_ports == 0)
> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>  
>  	set_def_fwd_config();
>  	if (nb_lcores == 0)
> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>  
>  	/* set all ports to promiscuous mode by default */
> -	for (port_id = 0; port_id < nb_ports; port_id++)
> +	FOREACH_PORT(port_id, ports)
>  		rte_eth_promiscuous_enable(port_id);
>  
>  #ifdef RTE_LIBRTE_CMDLINE
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 8f5e6c7..109c670 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -134,6 +134,7 @@ struct fwd_stream {
>   * The data structure associated with each port.
>   */
>  struct rte_port {
> +	uint8_t                 enabled;    /**< Port enabled or not */
>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
> @@ -159,6 +160,14 @@ struct rte_port {
>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>  };
>  
> +extern portid_t __rte_unused
> +find_next_port(portid_t p, struct rte_port *ports, int size);
> +
> +#define FOREACH_PORT(p, ports) \
> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
> +	    p < RTE_MAX_ETHPORTS; \
> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
> +
>  /**
>   * The data structure associated with each forwarding logical core.
>   * The logical cores are internally numbered by a core index from 0 to
> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>  int start_port(portid_t pid);
>  void stop_port(portid_t pid);
>  void close_port(portid_t pid);
> +void attach_port(char *identifier);
> +void detach_port(uint8_t port_id);
>  int all_ports_stopped(void);
>  int port_is_started(portid_t port_id);
>  void pmd_test_exit(void);
> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>  void get_flex_filter(uint8_t port_id, uint16_t index);
> -int port_id_is_invalid(portid_t port_id);
>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>  int tx_queue_id_is_invalid(queueid_t txq_id);
>  
> +enum print_warning {
> +	ENABLED_WARN = 0,
> +	DISABLED_WARN
> +};
> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
> +
>  /*
>   * Work-around of a compilation error with ICC on invocations of the
>   * rte_be_to_cpu_16() function.
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 218835a..1cacbcf 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>  
>      Port configuration changes only become active when forwarding is started/restarted.
>  
> +port attach
> +~~~~~~~~~~~
> +
> +Attach a port specified by pci address or virtual device args.
> +
> +To attach a new pci device, the device should be recognized by kernel first.
> +Then it should be moved under DPDK management.
> +Finally the port can be attached to testpmd.
> +On the other hand, to attach a port created by virtual device, above steps are not needed.
> +
> +port attach (identifier)
> +
> +For example, to attach a port that pci address is 0000:02:00.0.
> +
> +.. code-block:: console
> +
> +    testpmd> port attach 0000:02:00.0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +For example, to attach a port created by pcap PMD.
> +
> +.. code-block:: console
> +
> +    testpmd> port attach eth_pcap0,iface=eth0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +In this case, identifier is "eth_pcap0,iface=eth0".
> +This identifier format is the same as "--vdev" format of DPDK applications.
> +
> +port detach
> +~~~~~~~~~~~
> +
> +Detach a specific port.
> +
> +Before detaching a port, the port should be closed.
> +Also to remove a pci device completely from the system, first detach the port from testpmd.
> +Then the device should be moved under kernel management.
> +Finally the device can be remove using kernel pci hotplug functionality.
> +On the other hand, to remove a port created by virtual device, above steps are not needed.
> +
> +port detach (port_id)
> +
> +For example, to detach a port 0.
> +
> +.. code-block:: console
> +
> +    testpmd> port detach 0
> +    Detaching a port...
> +    ... snip ...
> +    Done
> +
>  port start
>  ~~~~~~~~~~
>
  

On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
> 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 paramerters of virtual device.
>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>  - port_id: port identifier
>
> v5:
> - Add testpmd documentation.
>   (Thanks to Iremonger, Bernard)
> v4:
>  - Fix strings of command help.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  app/test-pmd/cmdline.c                      | 133 +++++++++++++++----
>  app/test-pmd/config.c                       | 116 +++++++++-------
>  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, 415 insertions(+), 130 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 4beb404..2f813d8 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -572,6 +572,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			"port close (port_id|all)\n"
>  			"    Close all ports or port_id.\n\n"
>  
> +			"port attach (ident)\n"
> +			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
> +
> +			"port detach (port_id)\n"
> +			"    Detach physical or virtual dev by port_id\n\n"
> +
>  			"port config (port_id|all)"
>  			" speed (10|100|1000|10000|40000|auto)"
>  			" duplex (half|full|auto)\n"
> @@ -848,6 +854,89 @@ cmdline_parse_inst_t cmd_operate_specific_port = {
>  	},
>  };
>  
> +/* *** attach a specificied port *** */
> +struct cmd_operate_attach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	cmdline_fixed_string_t identifier;
> +};
> +
> +static void cmd_operate_attach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data)
> +{
> +	struct cmd_operate_attach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "attach"))
> +		attach_port(res->identifier);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_attach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_attach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			keyword, "attach");
> +cmdline_parse_token_string_t cmd_operate_attach_port_identifier =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
> +			identifier, NULL);
> +
> +cmdline_parse_inst_t cmd_operate_attach_port = {
> +	.f = cmd_operate_attach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port attach identifier, "
> +		"identifier: pci address or virtual dev name",
> +	.tokens = {
> +		(void *)&cmd_operate_attach_port_port,
> +		(void *)&cmd_operate_attach_port_keyword,
> +		(void *)&cmd_operate_attach_port_identifier,
> +		NULL,
> +	},
> +};
> +
> +/* *** detach a specificied port *** */
> +struct cmd_operate_detach_port_result {
> +	cmdline_fixed_string_t port;
> +	cmdline_fixed_string_t keyword;
> +	uint8_t port_id;
> +};
> +
> +static void cmd_operate_detach_port_parsed(void *parsed_result,
> +				__attribute__((unused)) struct cmdline *cl,
> +				__attribute__((unused)) void *data)
> +{
> +	struct cmd_operate_detach_port_result *res = parsed_result;
> +
> +	if (!strcmp(res->keyword, "detach"))
> +		detach_port(res->port_id);
> +	else
> +		printf("Unknown parameter\n");
> +}
> +
> +cmdline_parse_token_string_t cmd_operate_detach_port_port =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port, "port");
> +cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
> +			keyword, "detach");
> +cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
> +	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
> +			port_id, UINT8);
> +
> +cmdline_parse_inst_t cmd_operate_detach_port = {
> +	.f = cmd_operate_detach_port_parsed,
> +	.data = NULL,
> +	.help_str = "port detach port_id",
> +	.tokens = {
> +		(void *)&cmd_operate_detach_port_port,
> +		(void *)&cmd_operate_detach_port_keyword,
> +		(void *)&cmd_operate_detach_port_port_id,
> +		NULL,
> +	},
> +};
> +
>  /* *** configure speed for all ports *** */
>  struct cmd_config_speed_all {
>  	cmdline_fixed_string_t port;
> @@ -902,7 +991,7 @@ cmd_config_speed_all_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		ports[pid].dev_conf.link_speed = link_speed;
>  		ports[pid].dev_conf.link_duplex = link_duplex;
>  	}
> @@ -970,10 +1059,8 @@ cmd_config_speed_specific_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	if (res->id >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->id, nb_ports);
> +	if (port_id_is_invalid(res->id, ENABLED_WARN))
>  		return;
> -	}
>  
>  	if (!strcmp(res->value1, "10"))
>  		link_speed = ETH_LINK_SPEED_10;
> @@ -1533,7 +1620,7 @@ cmd_config_rxtx_queue_parsed(void *parsed_result,
>  		return;
>  	}
>  
> -	if (port_id_is_invalid(res->portid))
> +	if (port_id_is_invalid(res->portid, ENABLED_WARN))
>  		return;
>  
>  	if (port_is_started(res->portid) != 1) {
> @@ -2876,7 +2963,7 @@ cmd_tx_cksum_parsed(void *parsed_result,
>  	uint16_t ol_flags, mask = 0;
>  	struct rte_eth_dev_info dev_info;
>  
> -	if (port_id_is_invalid(res->port_id)) {
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN)) {
>  		printf("invalid port %d\n", res->port_id);
>  		return;
>  	}
> @@ -3003,7 +3090,7 @@ cmd_tso_set_parsed(void *parsed_result,
>  	struct cmd_tso_set_result *res = parsed_result;
>  	struct rte_eth_dev_info dev_info;
>  
> -	if (port_id_is_invalid(res->port_id))
> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>  		return;
>  
>  	if (!strcmp(res->mode, "set"))
> @@ -3979,10 +4066,8 @@ static void cmd_set_bond_mac_addr_parsed(void *parsed_result,
>  	struct cmd_set_bond_mac_addr_result *res = parsed_result;
>  	int ret;
>  
> -	if (res->port_num >= nb_ports) {
> -		printf("Port id %d must be less than %d\n", res->port_num, nb_ports);
> +	if (port_id_is_invalid(res->port_num, ENABLED_WARN))
>  		return;
> -	}
>  
>  	ret = rte_eth_bond_mac_address_set(res->port_num, &res->address);
>  
> @@ -4219,7 +4304,7 @@ static void cmd_set_promisc_mode_parsed(void *parsed_result,
>  
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_promiscuous_enable(i);
>  			else
> @@ -4299,7 +4384,7 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result,
>  
>  	/* all ports */
>  	if (allports) {
> -		for (i = 0; i < nb_ports; i++) {
> +		FOREACH_PORT(i, ports) {
>  			if (enable)
>  				rte_eth_allmulticast_enable(i);
>  			else
> @@ -5484,25 +5569,25 @@ static void cmd_showportall_parsed(void *parsed_result,
>  	struct cmd_showportall_result *res = parsed_result;
>  	if (!strcmp(res->show, "clear")) {
>  		if (!strcmp(res->what, "stats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_stats_clear(i);
>  		else if (!strcmp(res->what, "xstats"))
> -			for (i = 0; i < nb_ports; i++)
> +			FOREACH_PORT(i, ports)
>  				nic_xstats_clear(i);
>  	} else if (!strcmp(res->what, "info"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			port_infos_display(i);
>  	else if (!strcmp(res->what, "stats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_display(i);
>  	else if (!strcmp(res->what, "xstats"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_xstats_display(i);
>  	else if (!strcmp(res->what, "fdir"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			fdir_get_infos(i);
>  	else if (!strcmp(res->what, "stat_qmap"))
> -		for (i = 0; i < nb_ports; i++)
> +		FOREACH_PORT(i, ports)
>  			nic_stats_mapping_display(i);
>  }
>  
> @@ -8756,6 +8841,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *)&cmd_set_qmap,
>  	(cmdline_parse_inst_t *)&cmd_operate_port,
>  	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
> +	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_all,
>  	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
>  	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
> @@ -8830,7 +8917,7 @@ prompt(void)
>  static void
>  cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>  {
> -	if (id < nb_ports) {
> +	if (!port_id_is_invalid(id, DISABLED_WARN)) {
>  		/* check if need_reconfig has been set to 1 */
>  		if (ports[id].need_reconfig == 0)
>  			ports[id].need_reconfig = dev;
> @@ -8840,7 +8927,7 @@ cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>  	} else {
>  		portid_t pid;
>  
> -		for (pid = 0; pid < nb_ports; pid++) {
> +		FOREACH_PORT(pid, ports) {
>  			/* check if need_reconfig has been set to 1 */
>  			if (ports[pid].need_reconfig == 0)
>  				ports[pid].need_reconfig = dev;
> @@ -8858,10 +8945,8 @@ bypass_is_supported(portid_t port_id)
>  	struct rte_port   *port;
>  	struct rte_pci_id *pci_id;
>  
> -	if (port_id >= nb_ports) {
> -		printf("\tPort id must be less than %d.\n", nb_ports);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 0;
> -	}
>  
>  	/* Get the device id. */
>  	port    = &ports[port_id];
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index c40f819..32d8f9a 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -124,11 +124,15 @@ nic_stats_display(portid_t port_id)
>  	struct rte_eth_stats stats;
>  	struct rte_port *port = &ports[port_id];
>  	uint8_t i;
> +	portid_t pid;
>  
>  	static const char *nic_stats_border = "########################";
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_get(port_id, &stats);
> @@ -201,8 +205,13 @@ nic_stats_display(portid_t port_id)
>  void
>  nic_stats_clear(portid_t port_id)
>  {
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	portid_t pid;
> +
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	rte_eth_stats_reset(port_id);
> @@ -249,11 +258,15 @@ nic_stats_mapping_display(portid_t port_id)
>  {
>  	struct rte_port *port = &ports[port_id];
>  	uint16_t i;
> +	portid_t pid;
>  
>  	static const char *nic_stats_mapping_border = "########################";
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  
> @@ -302,9 +315,13 @@ port_infos_display(portid_t port_id)
>  	int vlan_offload;
>  	struct rte_mempool * mp;
>  	static const char *info_border = "*********************";
> +	portid_t pid;
>  
> -	if (port_id >= nb_ports) {
> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		printf("Valid port range is [0");
> +		FOREACH_PORT(pid, ports)
> +			printf(", %d", pid);
> +		printf("]\n");
>  		return;
>  	}
>  	port = &ports[port_id];
> @@ -362,11 +379,14 @@ port_infos_display(portid_t port_id)
>  }
>  
>  int
> -port_id_is_invalid(portid_t port_id)
> +port_id_is_invalid(portid_t port_id, enum print_warning warning)
>  {
> -	if (port_id < nb_ports)
> +	if (ports[port_id].enabled)

Here maybe care about overflow,  it could be passed a value of
RTE_PORT_ALL, which is 255.

Thanks,
Michael
>  		return 0;
> -	printf("Invalid port %d (must be < nb_ports=%d)\n", port_id, nb_ports);
> +
> +	if (warning == ENABLED_WARN)
> +		printf("Invalid port %d\n", port_id);
> +
>  	return 1;
>  }
>  
> @@ -425,7 +445,7 @@ port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)
>  	uint32_t reg_v;
>  
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -444,7 +464,7 @@ port_reg_bit_field_display(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -471,7 +491,7 @@ port_reg_display(portid_t port_id, uint32_t reg_off)
>  {
>  	uint32_t reg_v;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -485,7 +505,7 @@ port_reg_bit_set(portid_t port_id, uint32_t reg_off, uint8_t bit_pos,
>  {
>  	uint32_t reg_v;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -513,7 +533,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>  	uint8_t  l_bit;
>  	uint8_t  h_bit;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -547,7 +567,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>  void
>  port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (port_reg_off_is_invalid(port_id, reg_off))
>  		return;
> @@ -560,7 +580,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>  	if (diag == 0)
> @@ -723,7 +743,7 @@ rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id)
>  {
>  	const struct rte_memzone *rx_mz;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (rx_queue_id_is_invalid(rxq_id))
>  		return;
> @@ -740,7 +760,7 @@ tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id)
>  {
>  	const struct rte_memzone *tx_mz;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (tx_queue_id_is_invalid(txq_id))
>  		return;
> @@ -796,7 +816,7 @@ port_rss_reta_info(portid_t port_id,
>  	uint16_t i, idx, shift;
>  	int ret;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	ret = rte_eth_dev_rss_reta_query(port_id, reta_conf, nb_entries);
> @@ -828,7 +848,7 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
>  	uint8_t i;
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	/* Get RSS hash key if asked to display it */
>  	rss_conf.rss_key = (show_rss_key) ? rss_key : NULL;
> @@ -1406,12 +1426,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))
>  			return;
> -		}
>  		if (record_now)
>  			fwd_ports_ids[i] = port_id;
>  	}
> @@ -1569,7 +1585,7 @@ vlan_extend_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1591,7 +1607,7 @@ rx_vlan_strip_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1612,7 +1628,7 @@ rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_set_vlan_strip_on_queue(port_id, queue_id, on);
> @@ -1627,7 +1643,7 @@ rx_vlan_filter_set(portid_t port_id, int on)
>  	int diag;
>  	int vlan_offload;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
> @@ -1648,7 +1664,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1665,7 +1681,7 @@ rx_vlan_all_filter_set(portid_t port_id, int on)
>  {
>  	uint16_t vlan_id;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	for (vlan_id = 0; vlan_id < 4096; vlan_id++)
>  		rx_vft_set(port_id, vlan_id, on);
> @@ -1675,7 +1691,7 @@ void
>  vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>  {
>  	int diag;
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_set_vlan_ether_type(port_id, tp_id);
> @@ -1690,7 +1706,7 @@ vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>  void
>  tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -1701,7 +1717,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>  void
>  tx_vlan_reset(portid_t port_id)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ports[port_id].tx_ol_flags &= ~TESTPMD_TX_OFFLOAD_INSERT_VLAN;
>  }
> @@ -1709,7 +1725,7 @@ tx_vlan_reset(portid_t port_id)
>  void
>  tx_vlan_pvid_set(portid_t port_id, uint16_t vlan_id, int on)
>  {
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	rte_eth_dev_set_vlan_pvid(port_id, vlan_id, on);
> @@ -1721,7 +1737,7 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
>  	uint16_t i;
>  	uint8_t existing_mapping_found = 0;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	if (is_rx ? (rx_queue_id_is_invalid(queue_id)) : (tx_queue_id_is_invalid(queue_id)))
> @@ -1773,7 +1789,7 @@ fdir_add_signature_filter(portid_t port_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_add_signature_filter(port_id, fdir_filter,
> @@ -1791,7 +1807,7 @@ fdir_update_signature_filter(portid_t port_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_update_signature_filter(port_id, fdir_filter,
> @@ -1809,7 +1825,7 @@ fdir_remove_signature_filter(portid_t port_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_remove_signature_filter(port_id, fdir_filter);
> @@ -1881,7 +1897,7 @@ fdir_get_infos(portid_t port_id)
>  
>  	static const char *fdir_stats_border = "########################";
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	ret = rte_eth_dev_filter_supported(port_id, RTE_ETH_FILTER_FDIR);
>  	if (ret < 0) {
> @@ -1955,7 +1971,7 @@ fdir_add_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_add_perfect_filter(port_id, fdir_filter,
> @@ -1973,7 +1989,7 @@ fdir_update_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_update_perfect_filter(port_id, fdir_filter,
> @@ -1991,7 +2007,7 @@ fdir_remove_perfect_filter(portid_t port_id, uint16_t soft_id,
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_remove_perfect_filter(port_id, fdir_filter,
> @@ -2008,7 +2024,7 @@ fdir_set_masks(portid_t port_id, struct rte_fdir_masks *fdir_masks)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  
>  	diag = rte_eth_dev_fdir_set_masks(port_id, fdir_masks);
> @@ -2085,7 +2101,7 @@ set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (is_rx)
>  		diag = rte_eth_dev_set_vf_rx(port_id,vf,on);
> @@ -2107,7 +2123,7 @@ set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
>  {
>  	int diag;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return;
>  	if (vlan_id_is_invalid(vlan_id))
>  		return;
> @@ -2124,7 +2140,7 @@ set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate)
>  	int diag;
>  	struct rte_eth_link link;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> @@ -2149,7 +2165,7 @@ set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate, uint64_t q_msk)
>  	if (q_msk == 0)
>  		return 0;
>  
> -	if (port_id_is_invalid(port_id))
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>  		return 1;
>  	rte_eth_link_get_nowait(port_id, &link);
>  	if (rate > link.link_speed) {
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index adf3203..6f2af18 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -376,6 +376,7 @@ parse_portnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
>  
>  	/* reset from value set at definition */
>  	while ((p = strchr(p0,'(')) != NULL) {
> @@ -397,8 +398,11 @@ parse_portnuma_config(const char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> @@ -429,6 +433,7 @@ parse_ringnuma_config(const char *q_arg)
>  	};
>  	unsigned long int_fld[_NUM_FLD];
>  	char *str_fld[_NUM_FLD];
> +	portid_t pid;
>  	#define RX_RING_ONLY 0x1
>  	#define TX_RING_ONLY 0x2
>  	#define RXTX_RING    0x3
> @@ -453,8 +458,11 @@ parse_ringnuma_config(const char *q_arg)
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id >= nb_ports) {
> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +			printf("Valid port range is [0");
> +			FOREACH_PORT(pid, ports)
> +				printf(", %d", pid);
> +			printf("]\n");
>  			return -1;
>  		}
>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
> @@ -626,12 +634,12 @@ launch_args_parse(int argc, char** argv)
>  #endif
>  			if (!strcmp(lgopts[opt_idx].name, "nb-ports")) {
>  				n = atoi(optarg);
> -				if (n > 0 && n <= nb_ports)
> +				if (n > 0 &&
> +				    !port_id_is_invalid(n, DISABLED_WARN))
>  					nb_fwd_ports = (uint8_t) n;
>  				else
>  					rte_exit(EXIT_FAILURE,
> -						 "nb-ports should be > 0 and <= %d\n",
> -						 nb_ports);
> +						 "Invalid port %d\n", n);
>  			}
>  			if (!strcmp(lgopts[opt_idx].name, "nb-cores")) {
>  				n = atoi(optarg);
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 773b8af..c18c1a9 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -71,6 +71,7 @@
>  #include <rte_pci.h>
>  #include <rte_ether.h>
>  #include <rte_ethdev.h>
> +#include <rte_dev.h>
>  #include <rte_string_fns.h>
>  #ifdef RTE_LIBRTE_PMD_XENVIRT
>  #include <rte_eth_xenvirt.h>
> @@ -315,7 +316,7 @@ uint16_t nb_rx_queue_stats_mappings = 0;
>  
>  /* Forward function declarations */
>  static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port);
> -static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
> +static void check_all_ports_link_status(uint32_t port_mask);
>  
>  /*
>   * Check if all the ports are started.
> @@ -324,6 +325,20 @@ static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
>  static int all_ports_started(void);
>  
>  /*
> + * Find next enabled port
> + */
> +portid_t
> +find_next_port(portid_t p, struct rte_port *ports, int size)
> +{
> +	if (ports == NULL)
> +		rte_exit(-EINVAL, "failed to find a next port id\n");
> +
> +	while ((ports[p].enabled == 0) && (p < size))
> +		p++;
> +	return p;
> +}
> +
> +/*
>   * Setup default configuration.
>   */
>  static void
> @@ -552,7 +567,8 @@ init_config(void)
>  				+ RTE_TEST_TX_DESC_MAX + MAX_PKT_BURST;
>  
>  		if (!numa_support)
> -			nb_mbuf_per_pool = (nb_mbuf_per_pool * nb_ports);
> +			nb_mbuf_per_pool =
> +				(nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  	}
>  
>  	if (!numa_support) {
> @@ -565,14 +581,19 @@ init_config(void)
>  
>  	/* Configuration of Ethernet ports. */
>  	ports = rte_zmalloc("testpmd: ports",
> -			    sizeof(struct rte_port) * nb_ports,
> +			    sizeof(struct rte_port) * RTE_MAX_ETHPORTS,
>  			    RTE_CACHE_LINE_SIZE);
>  	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_zmalloc(%d struct rte_port) "
> -							"failed\n", nb_ports);
> +		rte_exit(EXIT_FAILURE,
> +				"rte_zmalloc(%d struct rte_port) failed\n",
> +				RTE_MAX_ETHPORTS);
>  	}
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	/* enabled allocated ports */
> +	for (pid = 0; pid < nb_ports; pid++)
> +		ports[pid].enabled = 1;
> +
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		rte_eth_dev_info_get(pid, &port->dev_info);
>  
> @@ -602,8 +623,7 @@ init_config(void)
>  			nb_mbuf_per_pool = nb_mbuf_per_pool/nb_ports;
>  
>  		for (i = 0; i < MAX_SOCKET; i++) {
> -			nb_mbuf = (nb_mbuf_per_pool *
> -						port_per_socket[i]);
> +			nb_mbuf = (nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>  			if (nb_mbuf)
>  				mbuf_pool_create(mbuf_data_size,
>  						nb_mbuf,i);
> @@ -635,14 +655,6 @@ reconfig(portid_t new_port_id, unsigned socket_id)
>  	struct rte_port *port;
>  
>  	/* Reconfiguration of Ethernet ports. */
> -	ports = rte_realloc(ports,
> -			    sizeof(struct rte_port) * nb_ports,
> -			    RTE_CACHE_LINE_SIZE);
> -	if (ports == NULL) {
> -		rte_exit(EXIT_FAILURE, "rte_realloc(%d struct rte_port) failed\n",
> -				nb_ports);
> -	}
> -
>  	port = &ports[new_port_id];
>  	rte_eth_dev_info_get(new_port_id, &port->dev_info);
>  
> @@ -663,7 +675,7 @@ init_fwd_streams(void)
>  	streamid_t sm_id, nb_fwd_streams_new;
>  
>  	/* set socket id according to numa or not */
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		if (nb_rxq > port->dev_info.max_rx_queues) {
>  			printf("Fail: nb_rxq(%d) is greater than "
> @@ -1264,7 +1276,7 @@ all_ports_started(void)
>  	portid_t pi;
>  	struct rte_port *port;
>  
> -	for (pi = 0; pi < nb_ports; pi++) {
> +	FOREACH_PORT(pi, ports) {
>  		port = &ports[pi];
>  		/* Check if there is a port which is not started */
>  		if (port->port_status != RTE_PORT_STARTED)
> @@ -1276,6 +1288,45 @@ all_ports_started(void)
>  }
>  
>  int
> +all_ports_stopped(void)
> +{
> +	portid_t pi;
> +	struct rte_port *port;
> +
> +	FOREACH_PORT(pi, ports) {
> +		port = &ports[pi];
> +		if (port->port_status != RTE_PORT_STOPPED)
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +int
> +port_is_started(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_STARTED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +static int
> +port_is_closed(portid_t port_id)
> +{
> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
> +		return 0;
> +
> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
> +		return 0;
> +
> +	return 1;
> +}
> +
> +int
>  start_port(portid_t pid)
>  {
>  	int diag, need_check_link_status = 0;
> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>  
>  	if(dcb_config)
>  		dcb_test = 1;
> -	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)
>  			continue;
>  
>  		port = &ports[pi];
> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>  	}
>  
>  	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);
>  	else
>  		printf("Please stop the ports first\n");
>  
> @@ -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)
>  			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");
>  }
> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>  
>  	printf("Closing 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)
>  			continue;
>  
>  		port = &ports[pi];
> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>  	printf("Done\n");
>  }
>  
> -int
> -all_ports_stopped(void)
> +void
> +attach_port(char *identifier)
>  {
> -	portid_t pi;
> -	struct rte_port *port;
> +	portid_t i, j, pi = 0;
>  
> -	for (pi = 0; pi < nb_ports; pi++) {
> -		port = &ports[pi];
> -		if (port->port_status != RTE_PORT_STOPPED)
> -			return 0;
> +	printf("Attaching a new port...\n");
> +
> +	if (identifier == NULL) {
> +		printf("Invalid parameters are speficied\n");
> +		return;
>  	}
>  
> -	return 1;
> +	if (test_done == 0) {
> +		printf("Please stop forwarding first\n");
> +		return;
> +	}
> +
> +	if (rte_eal_dev_attach(identifier, &pi))
> +		return;
> +
> +	ports[pi].enabled = 1;
> +	reconfig(pi, rte_eth_dev_socket_id(pi));
> +	rte_eth_promiscuous_enable(pi);
> +
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(j, ports) {
> +		fwd_ports_ids[i] = j;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports++;
> +
> +	ports[pi].port_status = RTE_PORT_STOPPED;
> +
> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
> +	printf("Done\n");
>  }
>  
> -int
> -port_is_started(portid_t port_id)
> +void
> +detach_port(uint8_t port_id)
>  {
> -	if (port_id_is_invalid(port_id))
> -		return -1;
> +	portid_t i, pi = 0;
> +	char name[RTE_ETH_NAME_MAX_LEN];
>  
> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
> -		return 0;
> +	printf("Detaching a port...\n");
>  
> -	return 1;
> +	if (!port_is_closed(port_id)) {
> +		printf("Please close port first\n");
> +		return;
> +	}
> +
> +	rte_eth_promiscuous_disable(port_id);
> +
> +	if (rte_eal_dev_detach(port_id, name))
> +		return;
> +
> +	ports[port_id].enabled = 0;
> +	nb_ports = rte_eth_dev_count();
> +
> +	/* set_default_fwd_ports_config(); */
> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
> +	i = 0;
> +	FOREACH_PORT(pi, ports) {
> +		fwd_ports_ids[i] = pi;
> +		i++;
> +	}
> +	nb_cfg_ports = nb_ports;
> +	nb_fwd_ports--;
> +
> +	printf("Port '%s' is detached. Now total ports is %d\n",
> +			name, nb_ports);
> +	printf("Done\n");
> +	return;
>  }
>  
>  void
> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>  {
>  	portid_t pt_id;
>  
> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
> +	FOREACH_PORT(pt_id, ports) {
>  		printf("Stopping port %d...", pt_id);
>  		fflush(stdout);
>  		rte_eth_dev_close(pt_id);
> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>  
>  /* Check the link status of all ports in up to 9s, and print them finally */
>  static void
> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
> +check_all_ports_link_status(uint32_t port_mask)
>  {
>  #define CHECK_INTERVAL 100 /* 100ms */
>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>  	fflush(stdout);
>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>  		all_ports_up = 1;
> -		for (portid = 0; portid < port_num; portid++) {
> +		FOREACH_PORT(portid, ports) {
>  			if ((port_mask & (1 << portid)) == 0)
>  				continue;
>  			memset(&link, 0, sizeof(link));
> @@ -1688,7 +1791,7 @@ init_port_config(void)
>  	portid_t pid;
>  	struct rte_port *port;
>  
> -	for (pid = 0; pid < nb_ports; pid++) {
> +	FOREACH_PORT(pid, ports) {
>  		port = &ports[pid];
>  		port->dev_conf.rxmode = rx_mode;
>  		port->dev_conf.fdir_conf = fdir_conf;
> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>  
>  	nb_ports = (portid_t) rte_eth_dev_count();
>  	if (nb_ports == 0)
> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>  
>  	set_def_fwd_config();
>  	if (nb_lcores == 0)
> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>  
>  	/* set all ports to promiscuous mode by default */
> -	for (port_id = 0; port_id < nb_ports; port_id++)
> +	FOREACH_PORT(port_id, ports)
>  		rte_eth_promiscuous_enable(port_id);
>  
>  #ifdef RTE_LIBRTE_CMDLINE
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 8f5e6c7..109c670 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -134,6 +134,7 @@ struct fwd_stream {
>   * The data structure associated with each port.
>   */
>  struct rte_port {
> +	uint8_t                 enabled;    /**< Port enabled or not */
>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
> @@ -159,6 +160,14 @@ struct rte_port {
>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>  };
>  
> +extern portid_t __rte_unused
> +find_next_port(portid_t p, struct rte_port *ports, int size);
> +
> +#define FOREACH_PORT(p, ports) \
> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
> +	    p < RTE_MAX_ETHPORTS; \
> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
> +
>  /**
>   * The data structure associated with each forwarding logical core.
>   * The logical cores are internally numbered by a core index from 0 to
> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>  int start_port(portid_t pid);
>  void stop_port(portid_t pid);
>  void close_port(portid_t pid);
> +void attach_port(char *identifier);
> +void detach_port(uint8_t port_id);
>  int all_ports_stopped(void);
>  int port_is_started(portid_t port_id);
>  void pmd_test_exit(void);
> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>  void get_flex_filter(uint8_t port_id, uint16_t index);
> -int port_id_is_invalid(portid_t port_id);
>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>  int tx_queue_id_is_invalid(queueid_t txq_id);
>  
> +enum print_warning {
> +	ENABLED_WARN = 0,
> +	DISABLED_WARN
> +};
> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
> +
>  /*
>   * Work-around of a compilation error with ICC on invocations of the
>   * rte_be_to_cpu_16() function.
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 218835a..1cacbcf 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>  
>      Port configuration changes only become active when forwarding is started/restarted.
>  
> +port attach
> +~~~~~~~~~~~
> +
> +Attach a port specified by pci address or virtual device args.
> +
> +To attach a new pci device, the device should be recognized by kernel first.
> +Then it should be moved under DPDK management.
> +Finally the port can be attached to testpmd.
> +On the other hand, to attach a port created by virtual device, above steps are not needed.
> +
> +port attach (identifier)
> +
> +For example, to attach a port that pci address is 0000:02:00.0.
> +
> +.. code-block:: console
> +
> +    testpmd> port attach 0000:02:00.0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +For example, to attach a port created by pcap PMD.
> +
> +.. code-block:: console
> +
> +    testpmd> port attach eth_pcap0,iface=eth0
> +    Attaching a new port...
> +    ... snip ...
> +    Port 0 is attached. Now total ports is 1
> +    Done
> +
> +In this case, identifier is "eth_pcap0,iface=eth0".
> +This identifier format is the same as "--vdev" format of DPDK applications.
> +
> +port detach
> +~~~~~~~~~~~
> +
> +Detach a specific port.
> +
> +Before detaching a port, the port should be closed.
> +Also to remove a pci device completely from the system, first detach the port from testpmd.
> +Then the device should be moved under kernel management.
> +Finally the device can be remove using kernel pci hotplug functionality.
> +On the other hand, to remove a port created by virtual device, above steps are not needed.
> +
> +port detach (port_id)
> +
> +For example, to detach a port 0.
> +
> +.. code-block:: console
> +
> +    testpmd> port detach 0
> +    Detaching a port...
> +    ... snip ...
> +    Done
> +
>  port start
>  ~~~~~~~~~~
>
  

On 2/3/2015 2:16 PM, Qiu, Michael wrote:
> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
>> 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 paramerters of virtual device.
>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>  - port_id: port identifier
>>
>> v5:
>> - Add testpmd documentation.
>>   (Thanks to Iremonger, Bernard)
>> v4:
>>  - Fix strings of command help.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

[...]

>> +static int
>> +port_is_closed(portid_t port_id)
>> +{
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>> +		return 0;
>> +
>> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> +
>> +int
>>  start_port(portid_t pid)
>>  {
>>  	int diag, need_check_link_status = 0;
>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>>  
>>  	if(dcb_config)
>>  		dcb_test = 1;
>> -	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)
> Here may it be:
>
> if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid != pi || pid == RET_PORT_ALL))

Sorry, should be:

if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi && pid != (portid_t)RET_PORT_ALL)

Otherwise, should check for "RET_PORT_ALL" in function port_id_is_invalid()

Thanks,
Michael

> Otherwise no port will be start by default.
>
>
> Thanks,
> Michael
>
>>  			continue;
>>  
>>  		port = &ports[pi];
>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>>  	}
>>  
>>  	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);
>>  	else
>>  		printf("Please stop the ports first\n");
>>  
>> @@ -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)
>>  			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");
>>  }
>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>>  
>>  	printf("Closing 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)
>>  			continue;
>>  
>>  		port = &ports[pi];
>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>>  	printf("Done\n");
>>  }
>>  
>> -int
>> -all_ports_stopped(void)
>> +void
>> +attach_port(char *identifier)
>>  {
>> -	portid_t pi;
>> -	struct rte_port *port;
>> +	portid_t i, j, pi = 0;
>>  
>> -	for (pi = 0; pi < nb_ports; pi++) {
>> -		port = &ports[pi];
>> -		if (port->port_status != RTE_PORT_STOPPED)
>> -			return 0;
>> +	printf("Attaching a new port...\n");
>> +
>> +	if (identifier == NULL) {
>> +		printf("Invalid parameters are speficied\n");
>> +		return;
>>  	}
>>  
>> -	return 1;
>> +	if (test_done == 0) {
>> +		printf("Please stop forwarding first\n");
>> +		return;
>> +	}
>> +
>> +	if (rte_eal_dev_attach(identifier, &pi))
>> +		return;
>> +
>> +	ports[pi].enabled = 1;
>> +	reconfig(pi, rte_eth_dev_socket_id(pi));
>> +	rte_eth_promiscuous_enable(pi);
>> +
>> +	nb_ports = rte_eth_dev_count();
>> +
>> +	/* set_default_fwd_ports_config(); */
>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>> +	i = 0;
>> +	FOREACH_PORT(j, ports) {
>> +		fwd_ports_ids[i] = j;
>> +		i++;
>> +	}
>> +	nb_cfg_ports = nb_ports;
>> +	nb_fwd_ports++;
>> +
>> +	ports[pi].port_status = RTE_PORT_STOPPED;
>> +
>> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
>> +	printf("Done\n");
>>  }
>>  
>> -int
>> -port_is_started(portid_t port_id)
>> +void
>> +detach_port(uint8_t port_id)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> -		return -1;
>> +	portid_t i, pi = 0;
>> +	char name[RTE_ETH_NAME_MAX_LEN];
>>  
>> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
>> -		return 0;
>> +	printf("Detaching a port...\n");
>>  
>> -	return 1;
>> +	if (!port_is_closed(port_id)) {
>> +		printf("Please close port first\n");
>> +		return;
>> +	}
>> +
>> +	rte_eth_promiscuous_disable(port_id);
>> +
>> +	if (rte_eal_dev_detach(port_id, name))
>> +		return;
>> +
>> +	ports[port_id].enabled = 0;
>> +	nb_ports = rte_eth_dev_count();
>> +
>> +	/* set_default_fwd_ports_config(); */
>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>> +	i = 0;
>> +	FOREACH_PORT(pi, ports) {
>> +		fwd_ports_ids[i] = pi;
>> +		i++;
>> +	}
>> +	nb_cfg_ports = nb_ports;
>> +	nb_fwd_ports--;
>> +
>> +	printf("Port '%s' is detached. Now total ports is %d\n",
>> +			name, nb_ports);
>> +	printf("Done\n");
>> +	return;
>>  }
>>  
>>  void
>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>>  {
>>  	portid_t pt_id;
>>  
>> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
>> +	FOREACH_PORT(pt_id, ports) {
>>  		printf("Stopping port %d...", pt_id);
>>  		fflush(stdout);
>>  		rte_eth_dev_close(pt_id);
>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>>  
>>  /* Check the link status of all ports in up to 9s, and print them finally */
>>  static void
>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>> +check_all_ports_link_status(uint32_t port_mask)
>>  {
>>  #define CHECK_INTERVAL 100 /* 100ms */
>>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>  	fflush(stdout);
>>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>>  		all_ports_up = 1;
>> -		for (portid = 0; portid < port_num; portid++) {
>> +		FOREACH_PORT(portid, ports) {
>>  			if ((port_mask & (1 << portid)) == 0)
>>  				continue;
>>  			memset(&link, 0, sizeof(link));
>> @@ -1688,7 +1791,7 @@ init_port_config(void)
>>  	portid_t pid;
>>  	struct rte_port *port;
>>  
>> -	for (pid = 0; pid < nb_ports; pid++) {
>> +	FOREACH_PORT(pid, ports) {
>>  		port = &ports[pid];
>>  		port->dev_conf.rxmode = rx_mode;
>>  		port->dev_conf.fdir_conf = fdir_conf;
>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>>  
>>  	nb_ports = (portid_t) rte_eth_dev_count();
>>  	if (nb_ports == 0)
>> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
>> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>>  
>>  	set_def_fwd_config();
>>  	if (nb_lcores == 0)
>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>>  
>>  	/* set all ports to promiscuous mode by default */
>> -	for (port_id = 0; port_id < nb_ports; port_id++)
>> +	FOREACH_PORT(port_id, ports)
>>  		rte_eth_promiscuous_enable(port_id);
>>  
>>  #ifdef RTE_LIBRTE_CMDLINE
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 8f5e6c7..109c670 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -134,6 +134,7 @@ struct fwd_stream {
>>   * The data structure associated with each port.
>>   */
>>  struct rte_port {
>> +	uint8_t                 enabled;    /**< Port enabled or not */
>>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
>> @@ -159,6 +160,14 @@ struct rte_port {
>>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>>  };
>>  
>> +extern portid_t __rte_unused
>> +find_next_port(portid_t p, struct rte_port *ports, int size);
>> +
>> +#define FOREACH_PORT(p, ports) \
>> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
>> +	    p < RTE_MAX_ETHPORTS; \
>> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
>> +
>>  /**
>>   * The data structure associated with each forwarding logical core.
>>   * The logical cores are internally numbered by a core index from 0 to
>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>>  int start_port(portid_t pid);
>>  void stop_port(portid_t pid);
>>  void close_port(portid_t pid);
>> +void attach_port(char *identifier);
>> +void detach_port(uint8_t port_id);
>>  int all_ports_stopped(void);
>>  int port_is_started(portid_t port_id);
>>  void pmd_test_exit(void);
>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>>  void get_flex_filter(uint8_t port_id, uint16_t index);
>> -int port_id_is_invalid(portid_t port_id);
>>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>>  int tx_queue_id_is_invalid(queueid_t txq_id);
>>  
>> +enum print_warning {
>> +	ENABLED_WARN = 0,
>> +	DISABLED_WARN
>> +};
>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
>> +
>>  /*
>>   * Work-around of a compilation error with ICC on invocations of the
>>   * rte_be_to_cpu_16() function.
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 218835a..1cacbcf 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>  
>>      Port configuration changes only become active when forwarding is started/restarted.
>>  
>> +port attach
>> +~~~~~~~~~~~
>> +
>> +Attach a port specified by pci address or virtual device args.
>> +
>> +To attach a new pci device, the device should be recognized by kernel first.
>> +Then it should be moved under DPDK management.
>> +Finally the port can be attached to testpmd.
>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>> +
>> +port attach (identifier)
>> +
>> +For example, to attach a port that pci address is 0000:02:00.0.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach 0000:02:00.0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +For example, to attach a port created by pcap PMD.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach eth_pcap0,iface=eth0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +In this case, identifier is "eth_pcap0,iface=eth0".
>> +This identifier format is the same as "--vdev" format of DPDK applications.
>> +
>> +port detach
>> +~~~~~~~~~~~
>> +
>> +Detach a specific port.
>> +
>> +Before detaching a port, the port should be closed.
>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>> +Then the device should be moved under kernel management.
>> +Finally the device can be remove using kernel pci hotplug functionality.
>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>> +
>> +port detach (port_id)
>> +
>> +For example, to detach a port 0.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port detach 0
>> +    Detaching a port...
>> +    ... snip ...
>> +    Done
>> +
>>  port start
>>  ~~~~~~~~~~
>>  
>
  

> >> +.. code-block:: console
> >> +
> >> +    testpmd> port attach 0000:02:00.0
> >> +    Attaching a new port...
> >> +    ... snip ...
> >> +    Port 0 is attached. Now total ports is 1
> >> +    Done
> >> +port detach
> >> +~~~~~~~~~~~
> >> +
> >> +Detach a specific port.
> >> +
> >> +Before detaching a port, the port should be closed.
> >> +Also to remove a pci device completely from the system, first detach the port from testpmd.
> >> +Then the device should be moved under kernel management.
> >> +Finally the device can be remove using kernel pci hotplug functionality.

Hi Tetsuya,
Reword "remove" to "removed"

> >> +On the other hand, to remove a port created by virtual device, above steps are not needed.
 Reword " created by virtual device" to "created by a virtual device"

> >
> >> +
> >> +port detach (port_id)
> >> +
> >> +For example, to detach a port 0.
> >> +
> >> +.. code-block:: console
> >> +
> >> +    testpmd> port detach 0
> >> +    Detaching a port...
> >> +    ... snip ...
> >> +    Done
> >> +
> >>  port start
> >>  ~~~~~~~~~~
> >>
> >> --
> >> 1.9.1
> > Regards,
> >
> > Bernard.
> >
  

On 2015/02/03 15:59, Qiu, Michael wrote:
> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
>> 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 paramerters of virtual device.
>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>  - port_id: port identifier
>>
>> v5:
>> - Add testpmd documentation.
>>   (Thanks to Iremonger, Bernard)
>> v4:
>>  - Fix strings of command help.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> ---
>>  app/test-pmd/cmdline.c                      | 133 +++++++++++++++----
>>  app/test-pmd/config.c                       | 116 +++++++++-------
>>  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, 415 insertions(+), 130 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 4beb404..2f813d8 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -572,6 +572,12 @@ static void cmd_help_long_parsed(void *parsed_result,
>>  			"port close (port_id|all)\n"
>>  			"    Close all ports or port_id.\n\n"
>>  
>> +			"port attach (ident)\n"
>> +			"    Attach physical or virtual dev by pci address or virtual device name\n\n"
>> +
>> +			"port detach (port_id)\n"
>> +			"    Detach physical or virtual dev by port_id\n\n"
>> +
>>  			"port config (port_id|all)"
>>  			" speed (10|100|1000|10000|40000|auto)"
>>  			" duplex (half|full|auto)\n"
>> @@ -848,6 +854,89 @@ cmdline_parse_inst_t cmd_operate_specific_port = {
>>  	},
>>  };
>>  
>> +/* *** attach a specificied port *** */
>> +struct cmd_operate_attach_port_result {
>> +	cmdline_fixed_string_t port;
>> +	cmdline_fixed_string_t keyword;
>> +	cmdline_fixed_string_t identifier;
>> +};
>> +
>> +static void cmd_operate_attach_port_parsed(void *parsed_result,
>> +				__attribute__((unused)) struct cmdline *cl,
>> +				__attribute__((unused)) void *data)
>> +{
>> +	struct cmd_operate_attach_port_result *res = parsed_result;
>> +
>> +	if (!strcmp(res->keyword, "attach"))
>> +		attach_port(res->identifier);
>> +	else
>> +		printf("Unknown parameter\n");
>> +}
>> +
>> +cmdline_parse_token_string_t cmd_operate_attach_port_port =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
>> +			port, "port");
>> +cmdline_parse_token_string_t cmd_operate_attach_port_keyword =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
>> +			keyword, "attach");
>> +cmdline_parse_token_string_t cmd_operate_attach_port_identifier =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_attach_port_result,
>> +			identifier, NULL);
>> +
>> +cmdline_parse_inst_t cmd_operate_attach_port = {
>> +	.f = cmd_operate_attach_port_parsed,
>> +	.data = NULL,
>> +	.help_str = "port attach identifier, "
>> +		"identifier: pci address or virtual dev name",
>> +	.tokens = {
>> +		(void *)&cmd_operate_attach_port_port,
>> +		(void *)&cmd_operate_attach_port_keyword,
>> +		(void *)&cmd_operate_attach_port_identifier,
>> +		NULL,
>> +	},
>> +};
>> +
>> +/* *** detach a specificied port *** */
>> +struct cmd_operate_detach_port_result {
>> +	cmdline_fixed_string_t port;
>> +	cmdline_fixed_string_t keyword;
>> +	uint8_t port_id;
>> +};
>> +
>> +static void cmd_operate_detach_port_parsed(void *parsed_result,
>> +				__attribute__((unused)) struct cmdline *cl,
>> +				__attribute__((unused)) void *data)
>> +{
>> +	struct cmd_operate_detach_port_result *res = parsed_result;
>> +
>> +	if (!strcmp(res->keyword, "detach"))
>> +		detach_port(res->port_id);
>> +	else
>> +		printf("Unknown parameter\n");
>> +}
>> +
>> +cmdline_parse_token_string_t cmd_operate_detach_port_port =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
>> +			port, "port");
>> +cmdline_parse_token_string_t cmd_operate_detach_port_keyword =
>> +	TOKEN_STRING_INITIALIZER(struct cmd_operate_detach_port_result,
>> +			keyword, "detach");
>> +cmdline_parse_token_num_t cmd_operate_detach_port_port_id =
>> +	TOKEN_NUM_INITIALIZER(struct cmd_operate_detach_port_result,
>> +			port_id, UINT8);
>> +
>> +cmdline_parse_inst_t cmd_operate_detach_port = {
>> +	.f = cmd_operate_detach_port_parsed,
>> +	.data = NULL,
>> +	.help_str = "port detach port_id",
>> +	.tokens = {
>> +		(void *)&cmd_operate_detach_port_port,
>> +		(void *)&cmd_operate_detach_port_keyword,
>> +		(void *)&cmd_operate_detach_port_port_id,
>> +		NULL,
>> +	},
>> +};
>> +
>>  /* *** configure speed for all ports *** */
>>  struct cmd_config_speed_all {
>>  	cmdline_fixed_string_t port;
>> @@ -902,7 +991,7 @@ cmd_config_speed_all_parsed(void *parsed_result,
>>  		return;
>>  	}
>>  
>> -	for (pid = 0; pid < nb_ports; pid++) {
>> +	FOREACH_PORT(pid, ports) {
>>  		ports[pid].dev_conf.link_speed = link_speed;
>>  		ports[pid].dev_conf.link_duplex = link_duplex;
>>  	}
>> @@ -970,10 +1059,8 @@ cmd_config_speed_specific_parsed(void *parsed_result,
>>  		return;
>>  	}
>>  
>> -	if (res->id >= nb_ports) {
>> -		printf("Port id %d must be less than %d\n", res->id, nb_ports);
>> +	if (port_id_is_invalid(res->id, ENABLED_WARN))
>>  		return;
>> -	}
>>  
>>  	if (!strcmp(res->value1, "10"))
>>  		link_speed = ETH_LINK_SPEED_10;
>> @@ -1533,7 +1620,7 @@ cmd_config_rxtx_queue_parsed(void *parsed_result,
>>  		return;
>>  	}
>>  
>> -	if (port_id_is_invalid(res->portid))
>> +	if (port_id_is_invalid(res->portid, ENABLED_WARN))
>>  		return;
>>  
>>  	if (port_is_started(res->portid) != 1) {
>> @@ -2876,7 +2963,7 @@ cmd_tx_cksum_parsed(void *parsed_result,
>>  	uint16_t ol_flags, mask = 0;
>>  	struct rte_eth_dev_info dev_info;
>>  
>> -	if (port_id_is_invalid(res->port_id)) {
>> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN)) {
>>  		printf("invalid port %d\n", res->port_id);
>>  		return;
>>  	}
>> @@ -3003,7 +3090,7 @@ cmd_tso_set_parsed(void *parsed_result,
>>  	struct cmd_tso_set_result *res = parsed_result;
>>  	struct rte_eth_dev_info dev_info;
>>  
>> -	if (port_id_is_invalid(res->port_id))
>> +	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	if (!strcmp(res->mode, "set"))
>> @@ -3979,10 +4066,8 @@ static void cmd_set_bond_mac_addr_parsed(void *parsed_result,
>>  	struct cmd_set_bond_mac_addr_result *res = parsed_result;
>>  	int ret;
>>  
>> -	if (res->port_num >= nb_ports) {
>> -		printf("Port id %d must be less than %d\n", res->port_num, nb_ports);
>> +	if (port_id_is_invalid(res->port_num, ENABLED_WARN))
>>  		return;
>> -	}
>>  
>>  	ret = rte_eth_bond_mac_address_set(res->port_num, &res->address);
>>  
>> @@ -4219,7 +4304,7 @@ static void cmd_set_promisc_mode_parsed(void *parsed_result,
>>  
>>  	/* all ports */
>>  	if (allports) {
>> -		for (i = 0; i < nb_ports; i++) {
>> +		FOREACH_PORT(i, ports) {
>>  			if (enable)
>>  				rte_eth_promiscuous_enable(i);
>>  			else
>> @@ -4299,7 +4384,7 @@ static void cmd_set_allmulti_mode_parsed(void *parsed_result,
>>  
>>  	/* all ports */
>>  	if (allports) {
>> -		for (i = 0; i < nb_ports; i++) {
>> +		FOREACH_PORT(i, ports) {
>>  			if (enable)
>>  				rte_eth_allmulticast_enable(i);
>>  			else
>> @@ -5484,25 +5569,25 @@ static void cmd_showportall_parsed(void *parsed_result,
>>  	struct cmd_showportall_result *res = parsed_result;
>>  	if (!strcmp(res->show, "clear")) {
>>  		if (!strcmp(res->what, "stats"))
>> -			for (i = 0; i < nb_ports; i++)
>> +			FOREACH_PORT(i, ports)
>>  				nic_stats_clear(i);
>>  		else if (!strcmp(res->what, "xstats"))
>> -			for (i = 0; i < nb_ports; i++)
>> +			FOREACH_PORT(i, ports)
>>  				nic_xstats_clear(i);
>>  	} else if (!strcmp(res->what, "info"))
>> -		for (i = 0; i < nb_ports; i++)
>> +		FOREACH_PORT(i, ports)
>>  			port_infos_display(i);
>>  	else if (!strcmp(res->what, "stats"))
>> -		for (i = 0; i < nb_ports; i++)
>> +		FOREACH_PORT(i, ports)
>>  			nic_stats_display(i);
>>  	else if (!strcmp(res->what, "xstats"))
>> -		for (i = 0; i < nb_ports; i++)
>> +		FOREACH_PORT(i, ports)
>>  			nic_xstats_display(i);
>>  	else if (!strcmp(res->what, "fdir"))
>> -		for (i = 0; i < nb_ports; i++)
>> +		FOREACH_PORT(i, ports)
>>  			fdir_get_infos(i);
>>  	else if (!strcmp(res->what, "stat_qmap"))
>> -		for (i = 0; i < nb_ports; i++)
>> +		FOREACH_PORT(i, ports)
>>  			nic_stats_mapping_display(i);
>>  }
>>  
>> @@ -8756,6 +8841,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>>  	(cmdline_parse_inst_t *)&cmd_set_qmap,
>>  	(cmdline_parse_inst_t *)&cmd_operate_port,
>>  	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
>> +	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
>> +	(cmdline_parse_inst_t *)&cmd_operate_detach_port,
>>  	(cmdline_parse_inst_t *)&cmd_config_speed_all,
>>  	(cmdline_parse_inst_t *)&cmd_config_speed_specific,
>>  	(cmdline_parse_inst_t *)&cmd_config_rx_tx,
>> @@ -8830,7 +8917,7 @@ prompt(void)
>>  static void
>>  cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>>  {
>> -	if (id < nb_ports) {
>> +	if (!port_id_is_invalid(id, DISABLED_WARN)) {
>>  		/* check if need_reconfig has been set to 1 */
>>  		if (ports[id].need_reconfig == 0)
>>  			ports[id].need_reconfig = dev;
>> @@ -8840,7 +8927,7 @@ cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue)
>>  	} else {
>>  		portid_t pid;
>>  
>> -		for (pid = 0; pid < nb_ports; pid++) {
>> +		FOREACH_PORT(pid, ports) {
>>  			/* check if need_reconfig has been set to 1 */
>>  			if (ports[pid].need_reconfig == 0)
>>  				ports[pid].need_reconfig = dev;
>> @@ -8858,10 +8945,8 @@ bypass_is_supported(portid_t port_id)
>>  	struct rte_port   *port;
>>  	struct rte_pci_id *pci_id;
>>  
>> -	if (port_id >= nb_ports) {
>> -		printf("\tPort id must be less than %d.\n", nb_ports);
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return 0;
>> -	}
>>  
>>  	/* Get the device id. */
>>  	port    = &ports[port_id];
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index c40f819..32d8f9a 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -124,11 +124,15 @@ nic_stats_display(portid_t port_id)
>>  	struct rte_eth_stats stats;
>>  	struct rte_port *port = &ports[port_id];
>>  	uint8_t i;
>> +	portid_t pid;
>>  
>>  	static const char *nic_stats_border = "########################";
>>  
>> -	if (port_id >= nb_ports) {
>> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +		printf("Valid port range is [0");
>> +		FOREACH_PORT(pid, ports)
>> +			printf(", %d", pid);
>> +		printf("]\n");
>>  		return;
>>  	}
>>  	rte_eth_stats_get(port_id, &stats);
>> @@ -201,8 +205,13 @@ nic_stats_display(portid_t port_id)
>>  void
>>  nic_stats_clear(portid_t port_id)
>>  {
>> -	if (port_id >= nb_ports) {
>> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +	portid_t pid;
>> +
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +		printf("Valid port range is [0");
>> +		FOREACH_PORT(pid, ports)
>> +			printf(", %d", pid);
>> +		printf("]\n");
>>  		return;
>>  	}
>>  	rte_eth_stats_reset(port_id);
>> @@ -249,11 +258,15 @@ nic_stats_mapping_display(portid_t port_id)
>>  {
>>  	struct rte_port *port = &ports[port_id];
>>  	uint16_t i;
>> +	portid_t pid;
>>  
>>  	static const char *nic_stats_mapping_border = "########################";
>>  
>> -	if (port_id >= nb_ports) {
>> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +		printf("Valid port range is [0");
>> +		FOREACH_PORT(pid, ports)
>> +			printf(", %d", pid);
>> +		printf("]\n");
>>  		return;
>>  	}
>>  
>> @@ -302,9 +315,13 @@ port_infos_display(portid_t port_id)
>>  	int vlan_offload;
>>  	struct rte_mempool * mp;
>>  	static const char *info_border = "*********************";
>> +	portid_t pid;
>>  
>> -	if (port_id >= nb_ports) {
>> -		printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +		printf("Valid port range is [0");
>> +		FOREACH_PORT(pid, ports)
>> +			printf(", %d", pid);
>> +		printf("]\n");
>>  		return;
>>  	}
>>  	port = &ports[port_id];
>> @@ -362,11 +379,14 @@ port_infos_display(portid_t port_id)
>>  }
>>  
>>  int
>> -port_id_is_invalid(portid_t port_id)
>> +port_id_is_invalid(portid_t port_id, enum print_warning warning)
>>  {
>> -	if (port_id < nb_ports)
>> +	if (ports[port_id].enabled)
> Here maybe care about overflow,  it could be passed a value of
> RTE_PORT_ALL, which is 255.

Thanks, I will fix like above.

Tetsuya

> Thanks,
> Michael
>>  		return 0;
>> -	printf("Invalid port %d (must be < nb_ports=%d)\n", port_id, nb_ports);
>> +
>> +	if (warning == ENABLED_WARN)
>> +		printf("Invalid port %d\n", port_id);
>> +
>>  	return 1;
>>  }
>>  
>> @@ -425,7 +445,7 @@ port_reg_bit_display(portid_t port_id, uint32_t reg_off, uint8_t bit_x)
>>  	uint32_t reg_v;
>>  
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -444,7 +464,7 @@ port_reg_bit_field_display(portid_t port_id, uint32_t reg_off,
>>  	uint8_t  l_bit;
>>  	uint8_t  h_bit;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -471,7 +491,7 @@ port_reg_display(portid_t port_id, uint32_t reg_off)
>>  {
>>  	uint32_t reg_v;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -485,7 +505,7 @@ port_reg_bit_set(portid_t port_id, uint32_t reg_off, uint8_t bit_pos,
>>  {
>>  	uint32_t reg_v;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -513,7 +533,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>>  	uint8_t  l_bit;
>>  	uint8_t  h_bit;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -547,7 +567,7 @@ port_reg_bit_field_set(portid_t port_id, uint32_t reg_off,
>>  void
>>  port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (port_reg_off_is_invalid(port_id, reg_off))
>>  		return;
>> @@ -560,7 +580,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	diag = rte_eth_dev_set_mtu(port_id, mtu);
>>  	if (diag == 0)
>> @@ -723,7 +743,7 @@ rx_ring_desc_display(portid_t port_id, queueid_t rxq_id, uint16_t rxd_id)
>>  {
>>  	const struct rte_memzone *rx_mz;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (rx_queue_id_is_invalid(rxq_id))
>>  		return;
>> @@ -740,7 +760,7 @@ tx_ring_desc_display(portid_t port_id, queueid_t txq_id, uint16_t txd_id)
>>  {
>>  	const struct rte_memzone *tx_mz;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (tx_queue_id_is_invalid(txq_id))
>>  		return;
>> @@ -796,7 +816,7 @@ port_rss_reta_info(portid_t port_id,
>>  	uint16_t i, idx, shift;
>>  	int ret;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	ret = rte_eth_dev_rss_reta_query(port_id, reta_conf, nb_entries);
>> @@ -828,7 +848,7 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
>>  	uint8_t i;
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	/* Get RSS hash key if asked to display it */
>>  	rss_conf.rss_key = (show_rss_key) ? rss_key : NULL;
>> @@ -1406,12 +1426,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))
>>  			return;
>> -		}
>>  		if (record_now)
>>  			fwd_ports_ids[i] = port_id;
>>  	}
>> @@ -1569,7 +1585,7 @@ vlan_extend_set(portid_t port_id, int on)
>>  	int diag;
>>  	int vlan_offload;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
>> @@ -1591,7 +1607,7 @@ rx_vlan_strip_set(portid_t port_id, int on)
>>  	int diag;
>>  	int vlan_offload;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
>> @@ -1612,7 +1628,7 @@ rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_set_vlan_strip_on_queue(port_id, queue_id, on);
>> @@ -1627,7 +1643,7 @@ rx_vlan_filter_set(portid_t port_id, int on)
>>  	int diag;
>>  	int vlan_offload;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	vlan_offload = rte_eth_dev_get_vlan_offload(port_id);
>> @@ -1648,7 +1664,7 @@ rx_vft_set(portid_t port_id, uint16_t vlan_id, int on)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (vlan_id_is_invalid(vlan_id))
>>  		return;
>> @@ -1665,7 +1681,7 @@ rx_vlan_all_filter_set(portid_t port_id, int on)
>>  {
>>  	uint16_t vlan_id;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	for (vlan_id = 0; vlan_id < 4096; vlan_id++)
>>  		rx_vft_set(port_id, vlan_id, on);
>> @@ -1675,7 +1691,7 @@ void
>>  vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>>  {
>>  	int diag;
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_set_vlan_ether_type(port_id, tp_id);
>> @@ -1690,7 +1706,7 @@ vlan_tpid_set(portid_t port_id, uint16_t tp_id)
>>  void
>>  tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (vlan_id_is_invalid(vlan_id))
>>  		return;
>> @@ -1701,7 +1717,7 @@ tx_vlan_set(portid_t port_id, uint16_t vlan_id)
>>  void
>>  tx_vlan_reset(portid_t port_id)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	ports[port_id].tx_ol_flags &= ~TESTPMD_TX_OFFLOAD_INSERT_VLAN;
>>  }
>> @@ -1709,7 +1725,7 @@ tx_vlan_reset(portid_t port_id)
>>  void
>>  tx_vlan_pvid_set(portid_t port_id, uint16_t vlan_id, int on)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	rte_eth_dev_set_vlan_pvid(port_id, vlan_id, on);
>> @@ -1721,7 +1737,7 @@ set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_value)
>>  	uint16_t i;
>>  	uint8_t existing_mapping_found = 0;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	if (is_rx ? (rx_queue_id_is_invalid(queue_id)) : (tx_queue_id_is_invalid(queue_id)))
>> @@ -1773,7 +1789,7 @@ fdir_add_signature_filter(portid_t port_id, uint8_t queue_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_add_signature_filter(port_id, fdir_filter,
>> @@ -1791,7 +1807,7 @@ fdir_update_signature_filter(portid_t port_id, uint8_t queue_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_update_signature_filter(port_id, fdir_filter,
>> @@ -1809,7 +1825,7 @@ fdir_remove_signature_filter(portid_t port_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_remove_signature_filter(port_id, fdir_filter);
>> @@ -1881,7 +1897,7 @@ fdir_get_infos(portid_t port_id)
>>  
>>  	static const char *fdir_stats_border = "########################";
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	ret = rte_eth_dev_filter_supported(port_id, RTE_ETH_FILTER_FDIR);
>>  	if (ret < 0) {
>> @@ -1955,7 +1971,7 @@ fdir_add_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_add_perfect_filter(port_id, fdir_filter,
>> @@ -1973,7 +1989,7 @@ fdir_update_perfect_filter(portid_t port_id, uint16_t soft_id, uint8_t queue_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_update_perfect_filter(port_id, fdir_filter,
>> @@ -1991,7 +2007,7 @@ fdir_remove_perfect_filter(portid_t port_id, uint16_t soft_id,
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_remove_perfect_filter(port_id, fdir_filter,
>> @@ -2008,7 +2024,7 @@ fdir_set_masks(portid_t port_id, struct rte_fdir_masks *fdir_masks)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  
>>  	diag = rte_eth_dev_fdir_set_masks(port_id, fdir_masks);
>> @@ -2085,7 +2101,7 @@ set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (is_rx)
>>  		diag = rte_eth_dev_set_vf_rx(port_id,vf,on);
>> @@ -2107,7 +2123,7 @@ set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
>>  {
>>  	int diag;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return;
>>  	if (vlan_id_is_invalid(vlan_id))
>>  		return;
>> @@ -2124,7 +2140,7 @@ set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate)
>>  	int diag;
>>  	struct rte_eth_link link;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return 1;
>>  	rte_eth_link_get_nowait(port_id, &link);
>>  	if (rate > link.link_speed) {
>> @@ -2149,7 +2165,7 @@ set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate, uint64_t q_msk)
>>  	if (q_msk == 0)
>>  		return 0;
>>  
>> -	if (port_id_is_invalid(port_id))
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>  		return 1;
>>  	rte_eth_link_get_nowait(port_id, &link);
>>  	if (rate > link.link_speed) {
>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>> index adf3203..6f2af18 100644
>> --- a/app/test-pmd/parameters.c
>> +++ b/app/test-pmd/parameters.c
>> @@ -376,6 +376,7 @@ parse_portnuma_config(const char *q_arg)
>>  	};
>>  	unsigned long int_fld[_NUM_FLD];
>>  	char *str_fld[_NUM_FLD];
>> +	portid_t pid;
>>  
>>  	/* reset from value set at definition */
>>  	while ((p = strchr(p0,'(')) != NULL) {
>> @@ -397,8 +398,11 @@ parse_portnuma_config(const char *q_arg)
>>  				return -1;
>>  		}
>>  		port_id = (uint8_t)int_fld[FLD_PORT];
>> -		if (port_id >= nb_ports) {
>> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +			printf("Valid port range is [0");
>> +			FOREACH_PORT(pid, ports)
>> +				printf(", %d", pid);
>> +			printf("]\n");
>>  			return -1;
>>  		}
>>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
>> @@ -429,6 +433,7 @@ parse_ringnuma_config(const char *q_arg)
>>  	};
>>  	unsigned long int_fld[_NUM_FLD];
>>  	char *str_fld[_NUM_FLD];
>> +	portid_t pid;
>>  	#define RX_RING_ONLY 0x1
>>  	#define TX_RING_ONLY 0x2
>>  	#define RXTX_RING    0x3
>> @@ -453,8 +458,11 @@ parse_ringnuma_config(const char *q_arg)
>>  				return -1;
>>  		}
>>  		port_id = (uint8_t)int_fld[FLD_PORT];
>> -		if (port_id >= nb_ports) {
>> -			printf("Invalid port, range is [0, %d]\n", nb_ports - 1);
>> +		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
>> +			printf("Valid port range is [0");
>> +			FOREACH_PORT(pid, ports)
>> +				printf(", %d", pid);
>> +			printf("]\n");
>>  			return -1;
>>  		}
>>  		socket_id = (uint8_t)int_fld[FLD_SOCKET];
>> @@ -626,12 +634,12 @@ launch_args_parse(int argc, char** argv)
>>  #endif
>>  			if (!strcmp(lgopts[opt_idx].name, "nb-ports")) {
>>  				n = atoi(optarg);
>> -				if (n > 0 && n <= nb_ports)
>> +				if (n > 0 &&
>> +				    !port_id_is_invalid(n, DISABLED_WARN))
>>  					nb_fwd_ports = (uint8_t) n;
>>  				else
>>  					rte_exit(EXIT_FAILURE,
>> -						 "nb-ports should be > 0 and <= %d\n",
>> -						 nb_ports);
>> +						 "Invalid port %d\n", n);
>>  			}
>>  			if (!strcmp(lgopts[opt_idx].name, "nb-cores")) {
>>  				n = atoi(optarg);
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 773b8af..c18c1a9 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -71,6 +71,7 @@
>>  #include <rte_pci.h>
>>  #include <rte_ether.h>
>>  #include <rte_ethdev.h>
>> +#include <rte_dev.h>
>>  #include <rte_string_fns.h>
>>  #ifdef RTE_LIBRTE_PMD_XENVIRT
>>  #include <rte_eth_xenvirt.h>
>> @@ -315,7 +316,7 @@ uint16_t nb_rx_queue_stats_mappings = 0;
>>  
>>  /* Forward function declarations */
>>  static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port);
>> -static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
>> +static void check_all_ports_link_status(uint32_t port_mask);
>>  
>>  /*
>>   * Check if all the ports are started.
>> @@ -324,6 +325,20 @@ static void check_all_ports_link_status(uint8_t port_num, uint32_t port_mask);
>>  static int all_ports_started(void);
>>  
>>  /*
>> + * Find next enabled port
>> + */
>> +portid_t
>> +find_next_port(portid_t p, struct rte_port *ports, int size)
>> +{
>> +	if (ports == NULL)
>> +		rte_exit(-EINVAL, "failed to find a next port id\n");
>> +
>> +	while ((ports[p].enabled == 0) && (p < size))
>> +		p++;
>> +	return p;
>> +}
>> +
>> +/*
>>   * Setup default configuration.
>>   */
>>  static void
>> @@ -552,7 +567,8 @@ init_config(void)
>>  				+ RTE_TEST_TX_DESC_MAX + MAX_PKT_BURST;
>>  
>>  		if (!numa_support)
>> -			nb_mbuf_per_pool = (nb_mbuf_per_pool * nb_ports);
>> +			nb_mbuf_per_pool =
>> +				(nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>>  	}
>>  
>>  	if (!numa_support) {
>> @@ -565,14 +581,19 @@ init_config(void)
>>  
>>  	/* Configuration of Ethernet ports. */
>>  	ports = rte_zmalloc("testpmd: ports",
>> -			    sizeof(struct rte_port) * nb_ports,
>> +			    sizeof(struct rte_port) * RTE_MAX_ETHPORTS,
>>  			    RTE_CACHE_LINE_SIZE);
>>  	if (ports == NULL) {
>> -		rte_exit(EXIT_FAILURE, "rte_zmalloc(%d struct rte_port) "
>> -							"failed\n", nb_ports);
>> +		rte_exit(EXIT_FAILURE,
>> +				"rte_zmalloc(%d struct rte_port) failed\n",
>> +				RTE_MAX_ETHPORTS);
>>  	}
>>  
>> -	for (pid = 0; pid < nb_ports; pid++) {
>> +	/* enabled allocated ports */
>> +	for (pid = 0; pid < nb_ports; pid++)
>> +		ports[pid].enabled = 1;
>> +
>> +	FOREACH_PORT(pid, ports) {
>>  		port = &ports[pid];
>>  		rte_eth_dev_info_get(pid, &port->dev_info);
>>  
>> @@ -602,8 +623,7 @@ init_config(void)
>>  			nb_mbuf_per_pool = nb_mbuf_per_pool/nb_ports;
>>  
>>  		for (i = 0; i < MAX_SOCKET; i++) {
>> -			nb_mbuf = (nb_mbuf_per_pool *
>> -						port_per_socket[i]);
>> +			nb_mbuf = (nb_mbuf_per_pool * RTE_MAX_ETHPORTS);
>>  			if (nb_mbuf)
>>  				mbuf_pool_create(mbuf_data_size,
>>  						nb_mbuf,i);
>> @@ -635,14 +655,6 @@ reconfig(portid_t new_port_id, unsigned socket_id)
>>  	struct rte_port *port;
>>  
>>  	/* Reconfiguration of Ethernet ports. */
>> -	ports = rte_realloc(ports,
>> -			    sizeof(struct rte_port) * nb_ports,
>> -			    RTE_CACHE_LINE_SIZE);
>> -	if (ports == NULL) {
>> -		rte_exit(EXIT_FAILURE, "rte_realloc(%d struct rte_port) failed\n",
>> -				nb_ports);
>> -	}
>> -
>>  	port = &ports[new_port_id];
>>  	rte_eth_dev_info_get(new_port_id, &port->dev_info);
>>  
>> @@ -663,7 +675,7 @@ init_fwd_streams(void)
>>  	streamid_t sm_id, nb_fwd_streams_new;
>>  
>>  	/* set socket id according to numa or not */
>> -	for (pid = 0; pid < nb_ports; pid++) {
>> +	FOREACH_PORT(pid, ports) {
>>  		port = &ports[pid];
>>  		if (nb_rxq > port->dev_info.max_rx_queues) {
>>  			printf("Fail: nb_rxq(%d) is greater than "
>> @@ -1264,7 +1276,7 @@ all_ports_started(void)
>>  	portid_t pi;
>>  	struct rte_port *port;
>>  
>> -	for (pi = 0; pi < nb_ports; pi++) {
>> +	FOREACH_PORT(pi, ports) {
>>  		port = &ports[pi];
>>  		/* Check if there is a port which is not started */
>>  		if (port->port_status != RTE_PORT_STARTED)
>> @@ -1276,6 +1288,45 @@ all_ports_started(void)
>>  }
>>  
>>  int
>> +all_ports_stopped(void)
>> +{
>> +	portid_t pi;
>> +	struct rte_port *port;
>> +
>> +	FOREACH_PORT(pi, ports) {
>> +		port = &ports[pi];
>> +		if (port->port_status != RTE_PORT_STOPPED)
>> +			return 0;
>> +	}
>> +
>> +	return 1;
>> +}
>> +
>> +int
>> +port_is_started(portid_t port_id)
>> +{
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>> +		return 0;
>> +
>> +	if (ports[port_id].port_status != RTE_PORT_STARTED)
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> +
>> +static int
>> +port_is_closed(portid_t port_id)
>> +{
>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>> +		return 0;
>> +
>> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> +
>> +int
>>  start_port(portid_t pid)
>>  {
>>  	int diag, need_check_link_status = 0;
>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>>  
>>  	if(dcb_config)
>>  		dcb_test = 1;
>> -	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)
>>  			continue;
>>  
>>  		port = &ports[pi];
>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>>  	}
>>  
>>  	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);
>>  	else
>>  		printf("Please stop the ports first\n");
>>  
>> @@ -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)
>>  			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");
>>  }
>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>>  
>>  	printf("Closing 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)
>>  			continue;
>>  
>>  		port = &ports[pi];
>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>>  	printf("Done\n");
>>  }
>>  
>> -int
>> -all_ports_stopped(void)
>> +void
>> +attach_port(char *identifier)
>>  {
>> -	portid_t pi;
>> -	struct rte_port *port;
>> +	portid_t i, j, pi = 0;
>>  
>> -	for (pi = 0; pi < nb_ports; pi++) {
>> -		port = &ports[pi];
>> -		if (port->port_status != RTE_PORT_STOPPED)
>> -			return 0;
>> +	printf("Attaching a new port...\n");
>> +
>> +	if (identifier == NULL) {
>> +		printf("Invalid parameters are speficied\n");
>> +		return;
>>  	}
>>  
>> -	return 1;
>> +	if (test_done == 0) {
>> +		printf("Please stop forwarding first\n");
>> +		return;
>> +	}
>> +
>> +	if (rte_eal_dev_attach(identifier, &pi))
>> +		return;
>> +
>> +	ports[pi].enabled = 1;
>> +	reconfig(pi, rte_eth_dev_socket_id(pi));
>> +	rte_eth_promiscuous_enable(pi);
>> +
>> +	nb_ports = rte_eth_dev_count();
>> +
>> +	/* set_default_fwd_ports_config(); */
>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>> +	i = 0;
>> +	FOREACH_PORT(j, ports) {
>> +		fwd_ports_ids[i] = j;
>> +		i++;
>> +	}
>> +	nb_cfg_ports = nb_ports;
>> +	nb_fwd_ports++;
>> +
>> +	ports[pi].port_status = RTE_PORT_STOPPED;
>> +
>> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
>> +	printf("Done\n");
>>  }
>>  
>> -int
>> -port_is_started(portid_t port_id)
>> +void
>> +detach_port(uint8_t port_id)
>>  {
>> -	if (port_id_is_invalid(port_id))
>> -		return -1;
>> +	portid_t i, pi = 0;
>> +	char name[RTE_ETH_NAME_MAX_LEN];
>>  
>> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
>> -		return 0;
>> +	printf("Detaching a port...\n");
>>  
>> -	return 1;
>> +	if (!port_is_closed(port_id)) {
>> +		printf("Please close port first\n");
>> +		return;
>> +	}
>> +
>> +	rte_eth_promiscuous_disable(port_id);
>> +
>> +	if (rte_eal_dev_detach(port_id, name))
>> +		return;
>> +
>> +	ports[port_id].enabled = 0;
>> +	nb_ports = rte_eth_dev_count();
>> +
>> +	/* set_default_fwd_ports_config(); */
>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>> +	i = 0;
>> +	FOREACH_PORT(pi, ports) {
>> +		fwd_ports_ids[i] = pi;
>> +		i++;
>> +	}
>> +	nb_cfg_ports = nb_ports;
>> +	nb_fwd_ports--;
>> +
>> +	printf("Port '%s' is detached. Now total ports is %d\n",
>> +			name, nb_ports);
>> +	printf("Done\n");
>> +	return;
>>  }
>>  
>>  void
>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>>  {
>>  	portid_t pt_id;
>>  
>> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
>> +	FOREACH_PORT(pt_id, ports) {
>>  		printf("Stopping port %d...", pt_id);
>>  		fflush(stdout);
>>  		rte_eth_dev_close(pt_id);
>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>>  
>>  /* Check the link status of all ports in up to 9s, and print them finally */
>>  static void
>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>> +check_all_ports_link_status(uint32_t port_mask)
>>  {
>>  #define CHECK_INTERVAL 100 /* 100ms */
>>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>  	fflush(stdout);
>>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>>  		all_ports_up = 1;
>> -		for (portid = 0; portid < port_num; portid++) {
>> +		FOREACH_PORT(portid, ports) {
>>  			if ((port_mask & (1 << portid)) == 0)
>>  				continue;
>>  			memset(&link, 0, sizeof(link));
>> @@ -1688,7 +1791,7 @@ init_port_config(void)
>>  	portid_t pid;
>>  	struct rte_port *port;
>>  
>> -	for (pid = 0; pid < nb_ports; pid++) {
>> +	FOREACH_PORT(pid, ports) {
>>  		port = &ports[pid];
>>  		port->dev_conf.rxmode = rx_mode;
>>  		port->dev_conf.fdir_conf = fdir_conf;
>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>>  
>>  	nb_ports = (portid_t) rte_eth_dev_count();
>>  	if (nb_ports == 0)
>> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
>> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>>  
>>  	set_def_fwd_config();
>>  	if (nb_lcores == 0)
>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>>  
>>  	/* set all ports to promiscuous mode by default */
>> -	for (port_id = 0; port_id < nb_ports; port_id++)
>> +	FOREACH_PORT(port_id, ports)
>>  		rte_eth_promiscuous_enable(port_id);
>>  
>>  #ifdef RTE_LIBRTE_CMDLINE
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 8f5e6c7..109c670 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -134,6 +134,7 @@ struct fwd_stream {
>>   * The data structure associated with each port.
>>   */
>>  struct rte_port {
>> +	uint8_t                 enabled;    /**< Port enabled or not */
>>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
>> @@ -159,6 +160,14 @@ struct rte_port {
>>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>>  };
>>  
>> +extern portid_t __rte_unused
>> +find_next_port(portid_t p, struct rte_port *ports, int size);
>> +
>> +#define FOREACH_PORT(p, ports) \
>> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
>> +	    p < RTE_MAX_ETHPORTS; \
>> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
>> +
>>  /**
>>   * The data structure associated with each forwarding logical core.
>>   * The logical cores are internally numbered by a core index from 0 to
>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>>  int start_port(portid_t pid);
>>  void stop_port(portid_t pid);
>>  void close_port(portid_t pid);
>> +void attach_port(char *identifier);
>> +void detach_port(uint8_t port_id);
>>  int all_ports_stopped(void);
>>  int port_is_started(portid_t port_id);
>>  void pmd_test_exit(void);
>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>>  void get_flex_filter(uint8_t port_id, uint16_t index);
>> -int port_id_is_invalid(portid_t port_id);
>>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>>  int tx_queue_id_is_invalid(queueid_t txq_id);
>>  
>> +enum print_warning {
>> +	ENABLED_WARN = 0,
>> +	DISABLED_WARN
>> +};
>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
>> +
>>  /*
>>   * Work-around of a compilation error with ICC on invocations of the
>>   * rte_be_to_cpu_16() function.
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 218835a..1cacbcf 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>  
>>      Port configuration changes only become active when forwarding is started/restarted.
>>  
>> +port attach
>> +~~~~~~~~~~~
>> +
>> +Attach a port specified by pci address or virtual device args.
>> +
>> +To attach a new pci device, the device should be recognized by kernel first.
>> +Then it should be moved under DPDK management.
>> +Finally the port can be attached to testpmd.
>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>> +
>> +port attach (identifier)
>> +
>> +For example, to attach a port that pci address is 0000:02:00.0.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach 0000:02:00.0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +For example, to attach a port created by pcap PMD.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port attach eth_pcap0,iface=eth0
>> +    Attaching a new port...
>> +    ... snip ...
>> +    Port 0 is attached. Now total ports is 1
>> +    Done
>> +
>> +In this case, identifier is "eth_pcap0,iface=eth0".
>> +This identifier format is the same as "--vdev" format of DPDK applications.
>> +
>> +port detach
>> +~~~~~~~~~~~
>> +
>> +Detach a specific port.
>> +
>> +Before detaching a port, the port should be closed.
>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>> +Then the device should be moved under kernel management.
>> +Finally the device can be remove using kernel pci hotplug functionality.
>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>> +
>> +port detach (port_id)
>> +
>> +For example, to detach a port 0.
>> +
>> +.. code-block:: console
>> +
>> +    testpmd> port detach 0
>> +    Detaching a port...
>> +    ... snip ...
>> +    Done
>> +
>>  port start
>>  ~~~~~~~~~~
>>
  

On 2015/02/03 18:14, Qiu, Michael wrote:
> On 2/3/2015 2:16 PM, Qiu, Michael wrote:
>> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
>>> 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 paramerters of virtual device.
>>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>>  - port_id: port identifier
>>>
>>> v5:
>>> - Add testpmd documentation.
>>>   (Thanks to Iremonger, Bernard)
>>> v4:
>>>  - Fix strings of command help.
>>>
>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> [...]
>
>>> +static int
>>> +port_is_closed(portid_t port_id)
>>> +{
>>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>> +		return 0;
>>> +
>>> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
>>> +		return 0;
>>> +
>>> +	return 1;
>>> +}
>>> +
>>> +int
>>>  start_port(portid_t pid)
>>>  {
>>>  	int diag, need_check_link_status = 0;
>>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>>>  
>>>  	if(dcb_config)
>>>  		dcb_test = 1;
>>> -	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)
>> Here may it be:
>>
>> if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid != pi || pid == RET_PORT_ALL))
> Sorry, should be:
>
> if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi && pid != (portid_t)RET_PORT_ALL)
>
> Otherwise, should check for "RET_PORT_ALL" in function port_id_is_invalid()

Thanks for comment. I've found 2 issues.
(I guess the original code has same issue.)
One is that "port_id_is_invalid" should receives "pi" instead of "pid".
The other is if statement is wrong as you said.

I guess following statement will be good.

if (port_id_is_invalid(pi, DISABLED_WARN) || (pid != pi && pid !=
(portid_t)RTE_PORT_ALL))

How about it?

Thanks,
Tetsuya


> Thanks,
> Michael
>
>> Otherwise no port will be start by default.
>>
>>
>> Thanks,
>> Michael
>>
>>>  			continue;
>>>  
>>>  		port = &ports[pi];
>>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>>>  	}
>>>  
>>>  	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);
>>>  	else
>>>  		printf("Please stop the ports first\n");
>>>  
>>> @@ -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)
>>>  			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");
>>>  }
>>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>>>  
>>>  	printf("Closing 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)
>>>  			continue;
>>>  
>>>  		port = &ports[pi];
>>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>>>  	printf("Done\n");
>>>  }
>>>  
>>> -int
>>> -all_ports_stopped(void)
>>> +void
>>> +attach_port(char *identifier)
>>>  {
>>> -	portid_t pi;
>>> -	struct rte_port *port;
>>> +	portid_t i, j, pi = 0;
>>>  
>>> -	for (pi = 0; pi < nb_ports; pi++) {
>>> -		port = &ports[pi];
>>> -		if (port->port_status != RTE_PORT_STOPPED)
>>> -			return 0;
>>> +	printf("Attaching a new port...\n");
>>> +
>>> +	if (identifier == NULL) {
>>> +		printf("Invalid parameters are speficied\n");
>>> +		return;
>>>  	}
>>>  
>>> -	return 1;
>>> +	if (test_done == 0) {
>>> +		printf("Please stop forwarding first\n");
>>> +		return;
>>> +	}
>>> +
>>> +	if (rte_eal_dev_attach(identifier, &pi))
>>> +		return;
>>> +
>>> +	ports[pi].enabled = 1;
>>> +	reconfig(pi, rte_eth_dev_socket_id(pi));
>>> +	rte_eth_promiscuous_enable(pi);
>>> +
>>> +	nb_ports = rte_eth_dev_count();
>>> +
>>> +	/* set_default_fwd_ports_config(); */
>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>> +	i = 0;
>>> +	FOREACH_PORT(j, ports) {
>>> +		fwd_ports_ids[i] = j;
>>> +		i++;
>>> +	}
>>> +	nb_cfg_ports = nb_ports;
>>> +	nb_fwd_ports++;
>>> +
>>> +	ports[pi].port_status = RTE_PORT_STOPPED;
>>> +
>>> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
>>> +	printf("Done\n");
>>>  }
>>>  
>>> -int
>>> -port_is_started(portid_t port_id)
>>> +void
>>> +detach_port(uint8_t port_id)
>>>  {
>>> -	if (port_id_is_invalid(port_id))
>>> -		return -1;
>>> +	portid_t i, pi = 0;
>>> +	char name[RTE_ETH_NAME_MAX_LEN];
>>>  
>>> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
>>> -		return 0;
>>> +	printf("Detaching a port...\n");
>>>  
>>> -	return 1;
>>> +	if (!port_is_closed(port_id)) {
>>> +		printf("Please close port first\n");
>>> +		return;
>>> +	}
>>> +
>>> +	rte_eth_promiscuous_disable(port_id);
>>> +
>>> +	if (rte_eal_dev_detach(port_id, name))
>>> +		return;
>>> +
>>> +	ports[port_id].enabled = 0;
>>> +	nb_ports = rte_eth_dev_count();
>>> +
>>> +	/* set_default_fwd_ports_config(); */
>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>> +	i = 0;
>>> +	FOREACH_PORT(pi, ports) {
>>> +		fwd_ports_ids[i] = pi;
>>> +		i++;
>>> +	}
>>> +	nb_cfg_ports = nb_ports;
>>> +	nb_fwd_ports--;
>>> +
>>> +	printf("Port '%s' is detached. Now total ports is %d\n",
>>> +			name, nb_ports);
>>> +	printf("Done\n");
>>> +	return;
>>>  }
>>>  
>>>  void
>>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>>>  {
>>>  	portid_t pt_id;
>>>  
>>> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
>>> +	FOREACH_PORT(pt_id, ports) {
>>>  		printf("Stopping port %d...", pt_id);
>>>  		fflush(stdout);
>>>  		rte_eth_dev_close(pt_id);
>>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>>>  
>>>  /* Check the link status of all ports in up to 9s, and print them finally */
>>>  static void
>>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>> +check_all_ports_link_status(uint32_t port_mask)
>>>  {
>>>  #define CHECK_INTERVAL 100 /* 100ms */
>>>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
>>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>>  	fflush(stdout);
>>>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>>>  		all_ports_up = 1;
>>> -		for (portid = 0; portid < port_num; portid++) {
>>> +		FOREACH_PORT(portid, ports) {
>>>  			if ((port_mask & (1 << portid)) == 0)
>>>  				continue;
>>>  			memset(&link, 0, sizeof(link));
>>> @@ -1688,7 +1791,7 @@ init_port_config(void)
>>>  	portid_t pid;
>>>  	struct rte_port *port;
>>>  
>>> -	for (pid = 0; pid < nb_ports; pid++) {
>>> +	FOREACH_PORT(pid, ports) {
>>>  		port = &ports[pid];
>>>  		port->dev_conf.rxmode = rx_mode;
>>>  		port->dev_conf.fdir_conf = fdir_conf;
>>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>>>  
>>>  	nb_ports = (portid_t) rte_eth_dev_count();
>>>  	if (nb_ports == 0)
>>> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
>>> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>>>  
>>>  	set_def_fwd_config();
>>>  	if (nb_lcores == 0)
>>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>>>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>>>  
>>>  	/* set all ports to promiscuous mode by default */
>>> -	for (port_id = 0; port_id < nb_ports; port_id++)
>>> +	FOREACH_PORT(port_id, ports)
>>>  		rte_eth_promiscuous_enable(port_id);
>>>  
>>>  #ifdef RTE_LIBRTE_CMDLINE
>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>> index 8f5e6c7..109c670 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -134,6 +134,7 @@ struct fwd_stream {
>>>   * The data structure associated with each port.
>>>   */
>>>  struct rte_port {
>>> +	uint8_t                 enabled;    /**< Port enabled or not */
>>>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>>>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>>>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
>>> @@ -159,6 +160,14 @@ struct rte_port {
>>>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>>>  };
>>>  
>>> +extern portid_t __rte_unused
>>> +find_next_port(portid_t p, struct rte_port *ports, int size);
>>> +
>>> +#define FOREACH_PORT(p, ports) \
>>> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
>>> +	    p < RTE_MAX_ETHPORTS; \
>>> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
>>> +
>>>  /**
>>>   * The data structure associated with each forwarding logical core.
>>>   * The logical cores are internally numbered by a core index from 0 to
>>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>>>  int start_port(portid_t pid);
>>>  void stop_port(portid_t pid);
>>>  void close_port(portid_t pid);
>>> +void attach_port(char *identifier);
>>> +void detach_port(uint8_t port_id);
>>>  int all_ports_stopped(void);
>>>  int port_is_started(portid_t port_id);
>>>  void pmd_test_exit(void);
>>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>>>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>>>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>>>  void get_flex_filter(uint8_t port_id, uint16_t index);
>>> -int port_id_is_invalid(portid_t port_id);
>>>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>>>  int tx_queue_id_is_invalid(queueid_t txq_id);
>>>  
>>> +enum print_warning {
>>> +	ENABLED_WARN = 0,
>>> +	DISABLED_WARN
>>> +};
>>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
>>> +
>>>  /*
>>>   * Work-around of a compilation error with ICC on invocations of the
>>>   * rte_be_to_cpu_16() function.
>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> index 218835a..1cacbcf 100644
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>>  
>>>      Port configuration changes only become active when forwarding is started/restarted.
>>>  
>>> +port attach
>>> +~~~~~~~~~~~
>>> +
>>> +Attach a port specified by pci address or virtual device args.
>>> +
>>> +To attach a new pci device, the device should be recognized by kernel first.
>>> +Then it should be moved under DPDK management.
>>> +Finally the port can be attached to testpmd.
>>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>>> +
>>> +port attach (identifier)
>>> +
>>> +For example, to attach a port that pci address is 0000:02:00.0.
>>> +
>>> +.. code-block:: console
>>> +
>>> +    testpmd> port attach 0000:02:00.0
>>> +    Attaching a new port...
>>> +    ... snip ...
>>> +    Port 0 is attached. Now total ports is 1
>>> +    Done
>>> +
>>> +For example, to attach a port created by pcap PMD.
>>> +
>>> +.. code-block:: console
>>> +
>>> +    testpmd> port attach eth_pcap0,iface=eth0
>>> +    Attaching a new port...
>>> +    ... snip ...
>>> +    Port 0 is attached. Now total ports is 1
>>> +    Done
>>> +
>>> +In this case, identifier is "eth_pcap0,iface=eth0".
>>> +This identifier format is the same as "--vdev" format of DPDK applications.
>>> +
>>> +port detach
>>> +~~~~~~~~~~~
>>> +
>>> +Detach a specific port.
>>> +
>>> +Before detaching a port, the port should be closed.
>>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>>> +Then the device should be moved under kernel management.
>>> +Finally the device can be remove using kernel pci hotplug functionality.
>>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>>> +
>>> +port detach (port_id)
>>> +
>>> +For example, to detach a port 0.
>>> +
>>> +.. code-block:: console
>>> +
>>> +    testpmd> port detach 0
>>> +    Detaching a port...
>>> +    ... snip ...
>>> +    Done
>>> +
>>>  port start
>>>  ~~~~~~~~~~
>>>
  

Hi Bernard,

I appreciate your checking.
I will fix like below.

Tetsuya

On 2015/02/03 19:03, Iremonger, Bernard wrote:
>>>> +.. code-block:: console
>>>> +
>>>> +    testpmd> port attach 0000:02:00.0
>>>> +    Attaching a new port...
>>>> +    ... snip ...
>>>> +    Port 0 is attached. Now total ports is 1
>>>> +    Done
>>>> +port detach
>>>> +~~~~~~~~~~~
>>>> +
>>>> +Detach a specific port.
>>>> +
>>>> +Before detaching a port, the port should be closed.
>>>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>>>> +Then the device should be moved under kernel management.
>>>> +Finally the device can be remove using kernel pci hotplug functionality.
> Hi Tetsuya,
> Reword "remove" to "removed"
>
>>>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>  Reword " created by virtual device" to "created by a virtual device"
>
>>>> +
>>>> +port detach (port_id)
>>>> +
>>>> +For example, to detach a port 0.
>>>> +
>>>> +.. code-block:: console
>>>> +
>>>> +    testpmd> port detach 0
>>>> +    Detaching a port...
>>>> +    ... snip ...
>>>> +    Done
>>>> +
>>>>  port start
>>>>  ~~~~~~~~~~
>>>>
>>>> --
>>>> 1.9.1
>>> Regards,
>>>
>>> Bernard.
>>>
  

On 2/3/2015 6:30 PM, Tetsuya Mukawa wrote:
> On 2015/02/03 18:14, Qiu, Michael wrote:
>> On 2/3/2015 2:16 PM, Qiu, Michael wrote:
>>> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
>>>> 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 paramerters of virtual device.
>>>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>>>  - port_id: port identifier
>>>>
>>>> v5:
>>>> - Add testpmd documentation.
>>>>   (Thanks to Iremonger, Bernard)
>>>> v4:
>>>>  - Fix strings of command help.
>>>>
>>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>> [...]
>>
>>>> +static int
>>>> +port_is_closed(portid_t port_id)
>>>> +{
>>>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>>> +		return 0;
>>>> +
>>>> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
>>>> +		return 0;
>>>> +
>>>> +	return 1;
>>>> +}
>>>> +
>>>> +int
>>>>  start_port(portid_t pid)
>>>>  {
>>>>  	int diag, need_check_link_status = 0;
>>>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>>>>  
>>>>  	if(dcb_config)
>>>>  		dcb_test = 1;
>>>> -	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)
>>> Here may it be:
>>>
>>> if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid != pi || pid == RET_PORT_ALL))
>> Sorry, should be:
>>
>> if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi && pid != (portid_t)RET_PORT_ALL)
>>
>> Otherwise, should check for "RET_PORT_ALL" in function port_id_is_invalid()
> Thanks for comment. I've found 2 issues.
> (I guess the original code has same issue.)

Original code may not have this issue, cause RET_PORT_ALL is always
greater than nb_ports, so "continue" will not exec. The logic may be 
right, but it is a little hard to understand.

> One is that "port_id_is_invalid" should receives "pi" instead of "pid".
> The other is if statement is wrong as you said.
>
> I guess following statement will be good.
>
> if (port_id_is_invalid(pi, DISABLED_WARN) || (pid != pi && pid !=
> (portid_t)RTE_PORT_ALL))

Actually, "port_id_is_invalid(pi, DISABLED_WARN)" could be removed as
"FOREACH_PORT" will find a valid port.

So it could be:

if (pid != pi && pid != (portid_t)RTE_PORT_ALL)

What do you think?

Thanks,
Michael
> How about it?
>
> Thanks,
> Tetsuya
>
>
>> Thanks,
>> Michael
>>
>>> Otherwise no port will be start by default.
>>>
>>>
>>> Thanks,
>>> Michael
>>>
>>>>  			continue;
>>>>  
>>>>  		port = &ports[pi];
>>>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>>>>  	}
>>>>  
>>>>  	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);
>>>>  	else
>>>>  		printf("Please stop the ports first\n");
>>>>  
>>>> @@ -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)
>>>>  			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");
>>>>  }
>>>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>>>>  
>>>>  	printf("Closing 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)
>>>>  			continue;
>>>>  
>>>>  		port = &ports[pi];
>>>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>>>>  	printf("Done\n");
>>>>  }
>>>>  
>>>> -int
>>>> -all_ports_stopped(void)
>>>> +void
>>>> +attach_port(char *identifier)
>>>>  {
>>>> -	portid_t pi;
>>>> -	struct rte_port *port;
>>>> +	portid_t i, j, pi = 0;
>>>>  
>>>> -	for (pi = 0; pi < nb_ports; pi++) {
>>>> -		port = &ports[pi];
>>>> -		if (port->port_status != RTE_PORT_STOPPED)
>>>> -			return 0;
>>>> +	printf("Attaching a new port...\n");
>>>> +
>>>> +	if (identifier == NULL) {
>>>> +		printf("Invalid parameters are speficied\n");
>>>> +		return;
>>>>  	}
>>>>  
>>>> -	return 1;
>>>> +	if (test_done == 0) {
>>>> +		printf("Please stop forwarding first\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	if (rte_eal_dev_attach(identifier, &pi))
>>>> +		return;
>>>> +
>>>> +	ports[pi].enabled = 1;
>>>> +	reconfig(pi, rte_eth_dev_socket_id(pi));
>>>> +	rte_eth_promiscuous_enable(pi);
>>>> +
>>>> +	nb_ports = rte_eth_dev_count();
>>>> +
>>>> +	/* set_default_fwd_ports_config(); */
>>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>>> +	i = 0;
>>>> +	FOREACH_PORT(j, ports) {
>>>> +		fwd_ports_ids[i] = j;
>>>> +		i++;
>>>> +	}
>>>> +	nb_cfg_ports = nb_ports;
>>>> +	nb_fwd_ports++;
>>>> +
>>>> +	ports[pi].port_status = RTE_PORT_STOPPED;
>>>> +
>>>> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
>>>> +	printf("Done\n");
>>>>  }
>>>>  
>>>> -int
>>>> -port_is_started(portid_t port_id)
>>>> +void
>>>> +detach_port(uint8_t port_id)
>>>>  {
>>>> -	if (port_id_is_invalid(port_id))
>>>> -		return -1;
>>>> +	portid_t i, pi = 0;
>>>> +	char name[RTE_ETH_NAME_MAX_LEN];
>>>>  
>>>> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
>>>> -		return 0;
>>>> +	printf("Detaching a port...\n");
>>>>  
>>>> -	return 1;
>>>> +	if (!port_is_closed(port_id)) {
>>>> +		printf("Please close port first\n");
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	rte_eth_promiscuous_disable(port_id);
>>>> +
>>>> +	if (rte_eal_dev_detach(port_id, name))
>>>> +		return;
>>>> +
>>>> +	ports[port_id].enabled = 0;
>>>> +	nb_ports = rte_eth_dev_count();
>>>> +
>>>> +	/* set_default_fwd_ports_config(); */
>>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>>> +	i = 0;
>>>> +	FOREACH_PORT(pi, ports) {
>>>> +		fwd_ports_ids[i] = pi;
>>>> +		i++;
>>>> +	}
>>>> +	nb_cfg_ports = nb_ports;
>>>> +	nb_fwd_ports--;
>>>> +
>>>> +	printf("Port '%s' is detached. Now total ports is %d\n",
>>>> +			name, nb_ports);
>>>> +	printf("Done\n");
>>>> +	return;
>>>>  }
>>>>  
>>>>  void
>>>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>>>>  {
>>>>  	portid_t pt_id;
>>>>  
>>>> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
>>>> +	FOREACH_PORT(pt_id, ports) {
>>>>  		printf("Stopping port %d...", pt_id);
>>>>  		fflush(stdout);
>>>>  		rte_eth_dev_close(pt_id);
>>>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>>>>  
>>>>  /* Check the link status of all ports in up to 9s, and print them finally */
>>>>  static void
>>>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>>> +check_all_ports_link_status(uint32_t port_mask)
>>>>  {
>>>>  #define CHECK_INTERVAL 100 /* 100ms */
>>>>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
>>>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>>>  	fflush(stdout);
>>>>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>>>>  		all_ports_up = 1;
>>>> -		for (portid = 0; portid < port_num; portid++) {
>>>> +		FOREACH_PORT(portid, ports) {
>>>>  			if ((port_mask & (1 << portid)) == 0)
>>>>  				continue;
>>>>  			memset(&link, 0, sizeof(link));
>>>> @@ -1688,7 +1791,7 @@ init_port_config(void)
>>>>  	portid_t pid;
>>>>  	struct rte_port *port;
>>>>  
>>>> -	for (pid = 0; pid < nb_ports; pid++) {
>>>> +	FOREACH_PORT(pid, ports) {
>>>>  		port = &ports[pid];
>>>>  		port->dev_conf.rxmode = rx_mode;
>>>>  		port->dev_conf.fdir_conf = fdir_conf;
>>>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>>>>  
>>>>  	nb_ports = (portid_t) rte_eth_dev_count();
>>>>  	if (nb_ports == 0)
>>>> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
>>>> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>>>>  
>>>>  	set_def_fwd_config();
>>>>  	if (nb_lcores == 0)
>>>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>>>>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>>>>  
>>>>  	/* set all ports to promiscuous mode by default */
>>>> -	for (port_id = 0; port_id < nb_ports; port_id++)
>>>> +	FOREACH_PORT(port_id, ports)
>>>>  		rte_eth_promiscuous_enable(port_id);
>>>>  
>>>>  #ifdef RTE_LIBRTE_CMDLINE
>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>>> index 8f5e6c7..109c670 100644
>>>> --- a/app/test-pmd/testpmd.h
>>>> +++ b/app/test-pmd/testpmd.h
>>>> @@ -134,6 +134,7 @@ struct fwd_stream {
>>>>   * The data structure associated with each port.
>>>>   */
>>>>  struct rte_port {
>>>> +	uint8_t                 enabled;    /**< Port enabled or not */
>>>>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>>>>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>>>>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
>>>> @@ -159,6 +160,14 @@ struct rte_port {
>>>>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>>>>  };
>>>>  
>>>> +extern portid_t __rte_unused
>>>> +find_next_port(portid_t p, struct rte_port *ports, int size);
>>>> +
>>>> +#define FOREACH_PORT(p, ports) \
>>>> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
>>>> +	    p < RTE_MAX_ETHPORTS; \
>>>> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
>>>> +
>>>>  /**
>>>>   * The data structure associated with each forwarding logical core.
>>>>   * The logical cores are internally numbered by a core index from 0 to
>>>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>>>>  int start_port(portid_t pid);
>>>>  void stop_port(portid_t pid);
>>>>  void close_port(portid_t pid);
>>>> +void attach_port(char *identifier);
>>>> +void detach_port(uint8_t port_id);
>>>>  int all_ports_stopped(void);
>>>>  int port_is_started(portid_t port_id);
>>>>  void pmd_test_exit(void);
>>>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>>>>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>>>>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>>>>  void get_flex_filter(uint8_t port_id, uint16_t index);
>>>> -int port_id_is_invalid(portid_t port_id);
>>>>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>>>>  int tx_queue_id_is_invalid(queueid_t txq_id);
>>>>  
>>>> +enum print_warning {
>>>> +	ENABLED_WARN = 0,
>>>> +	DISABLED_WARN
>>>> +};
>>>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
>>>> +
>>>>  /*
>>>>   * Work-around of a compilation error with ICC on invocations of the
>>>>   * rte_be_to_cpu_16() function.
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index 218835a..1cacbcf 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>>>  
>>>>      Port configuration changes only become active when forwarding is started/restarted.
>>>>  
>>>> +port attach
>>>> +~~~~~~~~~~~
>>>> +
>>>> +Attach a port specified by pci address or virtual device args.
>>>> +
>>>> +To attach a new pci device, the device should be recognized by kernel first.
>>>> +Then it should be moved under DPDK management.
>>>> +Finally the port can be attached to testpmd.
>>>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>>>> +
>>>> +port attach (identifier)
>>>> +
>>>> +For example, to attach a port that pci address is 0000:02:00.0.
>>>> +
>>>> +.. code-block:: console
>>>> +
>>>> +    testpmd> port attach 0000:02:00.0
>>>> +    Attaching a new port...
>>>> +    ... snip ...
>>>> +    Port 0 is attached. Now total ports is 1
>>>> +    Done
>>>> +
>>>> +For example, to attach a port created by pcap PMD.
>>>> +
>>>> +.. code-block:: console
>>>> +
>>>> +    testpmd> port attach eth_pcap0,iface=eth0
>>>> +    Attaching a new port...
>>>> +    ... snip ...
>>>> +    Port 0 is attached. Now total ports is 1
>>>> +    Done
>>>> +
>>>> +In this case, identifier is "eth_pcap0,iface=eth0".
>>>> +This identifier format is the same as "--vdev" format of DPDK applications.
>>>> +
>>>> +port detach
>>>> +~~~~~~~~~~~
>>>> +
>>>> +Detach a specific port.
>>>> +
>>>> +Before detaching a port, the port should be closed.
>>>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>>>> +Then the device should be moved under kernel management.
>>>> +Finally the device can be remove using kernel pci hotplug functionality.
>>>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>>>> +
>>>> +port detach (port_id)
>>>> +
>>>> +For example, to detach a port 0.
>>>> +
>>>> +.. code-block:: console
>>>> +
>>>> +    testpmd> port detach 0
>>>> +    Detaching a port...
>>>> +    ... snip ...
>>>> +    Done
>>>> +
>>>>  port start
>>>>  ~~~~~~~~~~
>>>>  
>
>
  

On 2015/02/04 10:44, Qiu, Michael wrote:
> On 2/3/2015 6:30 PM, Tetsuya Mukawa wrote:
>> On 2015/02/03 18:14, Qiu, Michael wrote:
>>> On 2/3/2015 2:16 PM, Qiu, Michael wrote:
>>>> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:
>>>>> 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 paramerters of virtual device.
>>>>>          (ex. 0000:02:00.0, eth_pcap0,iface=eth0)
>>>>>  - port_id: port identifier
>>>>>
>>>>> v5:
>>>>> - Add testpmd documentation.
>>>>>   (Thanks to Iremonger, Bernard)
>>>>> v4:
>>>>>  - Fix strings of command help.
>>>>>
>>>>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
>>> [...]
>>>
>>>>> +static int
>>>>> +port_is_closed(portid_t port_id)
>>>>> +{
>>>>> +	if (port_id_is_invalid(port_id, ENABLED_WARN))
>>>>> +		return 0;
>>>>> +
>>>>> +	if (ports[port_id].port_status != RTE_PORT_CLOSED)
>>>>> +		return 0;
>>>>> +
>>>>> +	return 1;
>>>>> +}
>>>>> +
>>>>> +int
>>>>>  start_port(portid_t pid)
>>>>>  {
>>>>>  	int diag, need_check_link_status = 0;
>>>>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)
>>>>>  
>>>>>  	if(dcb_config)
>>>>>  		dcb_test = 1;
>>>>> -	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)
>>>> Here may it be:
>>>>
>>>> if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid != pi || pid == RET_PORT_ALL))
>>> Sorry, should be:
>>>
>>> if (!port_id_is_invalid(pid, DISABLED_WARN) && pid != pi && pid != (portid_t)RET_PORT_ALL)
>>>
>>> Otherwise, should check for "RET_PORT_ALL" in function port_id_is_invalid()
>> Thanks for comment. I've found 2 issues.
>> (I guess the original code has same issue.)
> Original code may not have this issue, cause RET_PORT_ALL is always
> greater than nb_ports, so "continue" will not exec. The logic may be 
> right, but it is a little hard to understand.
>
>> One is that "port_id_is_invalid" should receives "pi" instead of "pid".
>> The other is if statement is wrong as you said.
>>
>> I guess following statement will be good.
>>
>> if (port_id_is_invalid(pi, DISABLED_WARN) || (pid != pi && pid !=
>> (portid_t)RTE_PORT_ALL))
> Actually, "port_id_is_invalid(pi, DISABLED_WARN)" could be removed as
> "FOREACH_PORT" will find a valid port.

Good point!

> So it could be:
>
> if (pid != pi && pid != (portid_t)RTE_PORT_ALL)
>
> What do you think?

I agree with you.
I will change like above.

Thanks,
Tetsuya

> Thanks,
> Michael
>> How about it?
>>
>> Thanks,
>> Tetsuya
>>
>>
>>> Thanks,
>>> Michael
>>>
>>>> Otherwise no port will be start by default.
>>>>
>>>>
>>>> Thanks,
>>>> Michael
>>>>
>>>>>  			continue;
>>>>>  
>>>>>  		port = &ports[pi];
>>>>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)
>>>>>  	}
>>>>>  
>>>>>  	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);
>>>>>  	else
>>>>>  		printf("Please stop the ports first\n");
>>>>>  
>>>>> @@ -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)
>>>>>  			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");
>>>>>  }
>>>>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)
>>>>>  
>>>>>  	printf("Closing 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)
>>>>>  			continue;
>>>>>  
>>>>>  		port = &ports[pi];
>>>>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)
>>>>>  	printf("Done\n");
>>>>>  }
>>>>>  
>>>>> -int
>>>>> -all_ports_stopped(void)
>>>>> +void
>>>>> +attach_port(char *identifier)
>>>>>  {
>>>>> -	portid_t pi;
>>>>> -	struct rte_port *port;
>>>>> +	portid_t i, j, pi = 0;
>>>>>  
>>>>> -	for (pi = 0; pi < nb_ports; pi++) {
>>>>> -		port = &ports[pi];
>>>>> -		if (port->port_status != RTE_PORT_STOPPED)
>>>>> -			return 0;
>>>>> +	printf("Attaching a new port...\n");
>>>>> +
>>>>> +	if (identifier == NULL) {
>>>>> +		printf("Invalid parameters are speficied\n");
>>>>> +		return;
>>>>>  	}
>>>>>  
>>>>> -	return 1;
>>>>> +	if (test_done == 0) {
>>>>> +		printf("Please stop forwarding first\n");
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	if (rte_eal_dev_attach(identifier, &pi))
>>>>> +		return;
>>>>> +
>>>>> +	ports[pi].enabled = 1;
>>>>> +	reconfig(pi, rte_eth_dev_socket_id(pi));
>>>>> +	rte_eth_promiscuous_enable(pi);
>>>>> +
>>>>> +	nb_ports = rte_eth_dev_count();
>>>>> +
>>>>> +	/* set_default_fwd_ports_config(); */
>>>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>>>> +	i = 0;
>>>>> +	FOREACH_PORT(j, ports) {
>>>>> +		fwd_ports_ids[i] = j;
>>>>> +		i++;
>>>>> +	}
>>>>> +	nb_cfg_ports = nb_ports;
>>>>> +	nb_fwd_ports++;
>>>>> +
>>>>> +	ports[pi].port_status = RTE_PORT_STOPPED;
>>>>> +
>>>>> +	printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports);
>>>>> +	printf("Done\n");
>>>>>  }
>>>>>  
>>>>> -int
>>>>> -port_is_started(portid_t port_id)
>>>>> +void
>>>>> +detach_port(uint8_t port_id)
>>>>>  {
>>>>> -	if (port_id_is_invalid(port_id))
>>>>> -		return -1;
>>>>> +	portid_t i, pi = 0;
>>>>> +	char name[RTE_ETH_NAME_MAX_LEN];
>>>>>  
>>>>> -	if (ports[port_id].port_status != RTE_PORT_STARTED)
>>>>> -		return 0;
>>>>> +	printf("Detaching a port...\n");
>>>>>  
>>>>> -	return 1;
>>>>> +	if (!port_is_closed(port_id)) {
>>>>> +		printf("Please close port first\n");
>>>>> +		return;
>>>>> +	}
>>>>> +
>>>>> +	rte_eth_promiscuous_disable(port_id);
>>>>> +
>>>>> +	if (rte_eal_dev_detach(port_id, name))
>>>>> +		return;
>>>>> +
>>>>> +	ports[port_id].enabled = 0;
>>>>> +	nb_ports = rte_eth_dev_count();
>>>>> +
>>>>> +	/* set_default_fwd_ports_config(); */
>>>>> +	bzero(fwd_ports_ids, sizeof(fwd_ports_ids));
>>>>> +	i = 0;
>>>>> +	FOREACH_PORT(pi, ports) {
>>>>> +		fwd_ports_ids[i] = pi;
>>>>> +		i++;
>>>>> +	}
>>>>> +	nb_cfg_ports = nb_ports;
>>>>> +	nb_fwd_ports--;
>>>>> +
>>>>> +	printf("Port '%s' is detached. Now total ports is %d\n",
>>>>> +			name, nb_ports);
>>>>> +	printf("Done\n");
>>>>> +	return;
>>>>>  }
>>>>>  
>>>>>  void
>>>>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)
>>>>>  {
>>>>>  	portid_t pt_id;
>>>>>  
>>>>> -	for (pt_id = 0; pt_id < nb_ports; pt_id++) {
>>>>> +	FOREACH_PORT(pt_id, ports) {
>>>>>  		printf("Stopping port %d...", pt_id);
>>>>>  		fflush(stdout);
>>>>>  		rte_eth_dev_close(pt_id);
>>>>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {
>>>>>  
>>>>>  /* Check the link status of all ports in up to 9s, and print them finally */
>>>>>  static void
>>>>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>>>> +check_all_ports_link_status(uint32_t port_mask)
>>>>>  {
>>>>>  #define CHECK_INTERVAL 100 /* 100ms */
>>>>>  #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */
>>>>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)
>>>>>  	fflush(stdout);
>>>>>  	for (count = 0; count <= MAX_CHECK_TIME; count++) {
>>>>>  		all_ports_up = 1;
>>>>> -		for (portid = 0; portid < port_num; portid++) {
>>>>> +		FOREACH_PORT(portid, ports) {
>>>>>  			if ((port_mask & (1 << portid)) == 0)
>>>>>  				continue;
>>>>>  			memset(&link, 0, sizeof(link));
>>>>> @@ -1688,7 +1791,7 @@ init_port_config(void)
>>>>>  	portid_t pid;
>>>>>  	struct rte_port *port;
>>>>>  
>>>>> -	for (pid = 0; pid < nb_ports; pid++) {
>>>>> +	FOREACH_PORT(pid, ports) {
>>>>>  		port = &ports[pid];
>>>>>  		port->dev_conf.rxmode = rx_mode;
>>>>>  		port->dev_conf.fdir_conf = fdir_conf;
>>>>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)
>>>>>  
>>>>>  	nb_ports = (portid_t) rte_eth_dev_count();
>>>>>  	if (nb_ports == 0)
>>>>> -		rte_exit(EXIT_FAILURE, "No probed ethernet device\n");
>>>>> +		RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");
>>>>>  
>>>>>  	set_def_fwd_config();
>>>>>  	if (nb_lcores == 0)
>>>>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)
>>>>>  		rte_exit(EXIT_FAILURE, "Start ports failed\n");
>>>>>  
>>>>>  	/* set all ports to promiscuous mode by default */
>>>>> -	for (port_id = 0; port_id < nb_ports; port_id++)
>>>>> +	FOREACH_PORT(port_id, ports)
>>>>>  		rte_eth_promiscuous_enable(port_id);
>>>>>  
>>>>>  #ifdef RTE_LIBRTE_CMDLINE
>>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>>>> index 8f5e6c7..109c670 100644
>>>>> --- a/app/test-pmd/testpmd.h
>>>>> +++ b/app/test-pmd/testpmd.h
>>>>> @@ -134,6 +134,7 @@ struct fwd_stream {
>>>>>   * The data structure associated with each port.
>>>>>   */
>>>>>  struct rte_port {
>>>>> +	uint8_t                 enabled;    /**< Port enabled or not */
>>>>>  	struct rte_eth_dev_info dev_info;   /**< PCI info + driver name */
>>>>>  	struct rte_eth_conf     dev_conf;   /**< Port configuration. */
>>>>>  	struct ether_addr       eth_addr;   /**< Port ethernet address */
>>>>> @@ -159,6 +160,14 @@ struct rte_port {
>>>>>  	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
>>>>>  };
>>>>>  
>>>>> +extern portid_t __rte_unused
>>>>> +find_next_port(portid_t p, struct rte_port *ports, int size);
>>>>> +
>>>>> +#define FOREACH_PORT(p, ports) \
>>>>> +	for (p = find_next_port(0, ports, RTE_MAX_ETHPORTS); \
>>>>> +	    p < RTE_MAX_ETHPORTS; \
>>>>> +	    p = find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))
>>>>> +
>>>>>  /**
>>>>>   * The data structure associated with each forwarding logical core.
>>>>>   * The logical cores are internally numbered by a core index from 0 to
>>>>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_config *dcb_conf);
>>>>>  int start_port(portid_t pid);
>>>>>  void stop_port(portid_t pid);
>>>>>  void close_port(portid_t pid);
>>>>> +void attach_port(char *identifier);
>>>>> +void detach_port(uint8_t port_id);
>>>>>  int all_ports_stopped(void);
>>>>>  int port_is_started(portid_t port_id);
>>>>>  void pmd_test_exit(void);
>>>>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint16_t index);
>>>>>  void get_2tuple_filter(uint8_t port_id, uint16_t index);
>>>>>  void get_5tuple_filter(uint8_t port_id, uint16_t index);
>>>>>  void get_flex_filter(uint8_t port_id, uint16_t index);
>>>>> -int port_id_is_invalid(portid_t port_id);
>>>>>  int rx_queue_id_is_invalid(queueid_t rxq_id);
>>>>>  int tx_queue_id_is_invalid(queueid_t txq_id);
>>>>>  
>>>>> +enum print_warning {
>>>>> +	ENABLED_WARN = 0,
>>>>> +	DISABLED_WARN
>>>>> +};
>>>>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);
>>>>> +
>>>>>  /*
>>>>>   * Work-around of a compilation error with ICC on invocations of the
>>>>>   * rte_be_to_cpu_16() function.
>>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> index 218835a..1cacbcf 100644
>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> @@ -808,6 +808,63 @@ The following sections show functions for configuring ports.
>>>>>  
>>>>>      Port configuration changes only become active when forwarding is started/restarted.
>>>>>  
>>>>> +port attach
>>>>> +~~~~~~~~~~~
>>>>> +
>>>>> +Attach a port specified by pci address or virtual device args.
>>>>> +
>>>>> +To attach a new pci device, the device should be recognized by kernel first.
>>>>> +Then it should be moved under DPDK management.
>>>>> +Finally the port can be attached to testpmd.
>>>>> +On the other hand, to attach a port created by virtual device, above steps are not needed.
>>>>> +
>>>>> +port attach (identifier)
>>>>> +
>>>>> +For example, to attach a port that pci address is 0000:02:00.0.
>>>>> +
>>>>> +.. code-block:: console
>>>>> +
>>>>> +    testpmd> port attach 0000:02:00.0
>>>>> +    Attaching a new port...
>>>>> +    ... snip ...
>>>>> +    Port 0 is attached. Now total ports is 1
>>>>> +    Done
>>>>> +
>>>>> +For example, to attach a port created by pcap PMD.
>>>>> +
>>>>> +.. code-block:: console
>>>>> +
>>>>> +    testpmd> port attach eth_pcap0,iface=eth0
>>>>> +    Attaching a new port...
>>>>> +    ... snip ...
>>>>> +    Port 0 is attached. Now total ports is 1
>>>>> +    Done
>>>>> +
>>>>> +In this case, identifier is "eth_pcap0,iface=eth0".
>>>>> +This identifier format is the same as "--vdev" format of DPDK applications.
>>>>> +
>>>>> +port detach
>>>>> +~~~~~~~~~~~
>>>>> +
>>>>> +Detach a specific port.
>>>>> +
>>>>> +Before detaching a port, the port should be closed.
>>>>> +Also to remove a pci device completely from the system, first detach the port from testpmd.
>>>>> +Then the device should be moved under kernel management.
>>>>> +Finally the device can be remove using kernel pci hotplug functionality.
>>>>> +On the other hand, to remove a port created by virtual device, above steps are not needed.
>>>>> +
>>>>> +port detach (port_id)
>>>>> +
>>>>> +For example, to detach a port 0.
>>>>> +
>>>>> +.. code-block:: console
>>>>> +
>>>>> +    testpmd> port detach 0
>>>>> +    Detaching a port...
>>>>> +    ... snip ...
>>>>> +    Done
>>>>> +
>>>>>  port start
>>>>>  ~~~~~~~~~~
>>>>>  
>>