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 at 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 >>> ~~~~~~~~~~ >>>