From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 5F6E9AD96 for ; Wed, 4 Feb 2015 02:44:13 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 03 Feb 2015 17:39:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,516,1418112000"; d="scan'208";a="646962743" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by orsmga001.jf.intel.com with ESMTP; 03 Feb 2015 17:44:11 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by PGSMSX108.gar.corp.intel.com (10.221.44.103) with Microsoft SMTP Server (TLS) id 14.3.195.1; Wed, 4 Feb 2015 09:44:08 +0800 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.253]) by shsmsx102.ccr.corp.intel.com ([169.254.2.124]) with mapi id 14.03.0195.001; Wed, 4 Feb 2015 09:44:07 +0800 From: "Qiu, Michael" To: Tetsuya Mukawa , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v6] testpmd: Add port hotplug support Thread-Index: AQHQPdP23bysSjCN/EiSLvqSV3tO8A== Date: Wed, 4 Feb 2015 01:44:06 +0000 Message-ID: <533710CFB86FA344BFBF2D6802E60286CD420D@SHSMSX101.ccr.corp.intel.com> References: <1421664027-17971-9-git-send-email-mukawa@igel.co.jp> <1422763322-13742-1-git-send-email-mukawa@igel.co.jp> <1422763322-13742-16-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286CD3E56@SHSMSX101.ccr.corp.intel.com> <533710CFB86FA344BFBF2D6802E60286CD3F7D@SHSMSX101.ccr.corp.intel.com> <54D0A31C.2030903@igel.co.jp> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6] testpmd: Add port hotplug support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Feb 2015 01:44:17 -0000 On 2/3/2015 6:30 PM, Tetsuya Mukawa wrote:=0A= > On 2015/02/03 18:14, Qiu, Michael wrote:=0A= >> On 2/3/2015 2:16 PM, Qiu, Michael wrote:=0A= >>> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote:=0A= >>>> The patch introduces following commands.=0A= >>>> - port attach [ident]=0A= >>>> - port detach [port_id]=0A= >>>> - attach: attaching a port=0A= >>>> - detach: detaching a port=0A= >>>> - ident: pci address of physical device.=0A= >>>> Or device name and paramerters of virtual device.=0A= >>>> (ex. 0000:02:00.0, eth_pcap0,iface=3Deth0)=0A= >>>> - port_id: port identifier=0A= >>>>=0A= >>>> v5:=0A= >>>> - Add testpmd documentation.=0A= >>>> (Thanks to Iremonger, Bernard)=0A= >>>> v4:=0A= >>>> - Fix strings of command help.=0A= >>>>=0A= >>>> Signed-off-by: Tetsuya Mukawa =0A= >> [...]=0A= >>=0A= >>>> +static int=0A= >>>> +port_is_closed(portid_t port_id)=0A= >>>> +{=0A= >>>> + if (port_id_is_invalid(port_id, ENABLED_WARN))=0A= >>>> + return 0;=0A= >>>> +=0A= >>>> + if (ports[port_id].port_status !=3D RTE_PORT_CLOSED)=0A= >>>> + return 0;=0A= >>>> +=0A= >>>> + return 1;=0A= >>>> +}=0A= >>>> +=0A= >>>> +int=0A= >>>> start_port(portid_t pid)=0A= >>>> {=0A= >>>> int diag, need_check_link_status =3D 0;=0A= >>>> @@ -1296,8 +1347,8 @@ start_port(portid_t pid)=0A= >>>> =0A= >>>> if(dcb_config)=0A= >>>> dcb_test =3D 1;=0A= >>>> - for (pi =3D 0; pi < nb_ports; pi++) {=0A= >>>> - if (pid < nb_ports && pid !=3D pi)=0A= >>>> + FOREACH_PORT(pi, ports) {=0A= >>>> + if (!port_id_is_invalid(pid, DISABLED_WARN) && pid !=3D pi)=0A= >>> Here may it be:=0A= >>>=0A= >>> if (!port_id_is_invalid(pid, DISABLED_WARN) && (pid !=3D pi || pid =3D= =3D RET_PORT_ALL))=0A= >> Sorry, should be:=0A= >>=0A= >> if (!port_id_is_invalid(pid, DISABLED_WARN) && pid !=3D pi && pid !=3D (= portid_t)RET_PORT_ALL)=0A= >>=0A= >> Otherwise, should check for "RET_PORT_ALL" in function port_id_is_invali= d()=0A= > Thanks for comment. I've found 2 issues.=0A= > (I guess the original code has same issue.)=0A= =0A= Original code may not have this issue, cause RET_PORT_ALL is always=0A= greater than nb_ports, so "continue" will not exec. The logic may be =0A= right, but it is a little hard to understand.=0A= =0A= > One is that "port_id_is_invalid" should receives "pi" instead of "pid".= =0A= > The other is if statement is wrong as you said.=0A= >=0A= > I guess following statement will be good.=0A= >=0A= > if (port_id_is_invalid(pi, DISABLED_WARN) || (pid !=3D pi && pid !=3D=0A= > (portid_t)RTE_PORT_ALL))=0A= =0A= Actually, "port_id_is_invalid(pi, DISABLED_WARN)" could be removed as=0A= "FOREACH_PORT" will find a valid port.=0A= =0A= So it could be:=0A= =0A= if (pid !=3D pi && pid !=3D (portid_t)RTE_PORT_ALL)=0A= =0A= What do you think?=0A= =0A= Thanks,=0A= Michael=0A= > How about it?=0A= >=0A= > Thanks,=0A= > Tetsuya=0A= >=0A= >=0A= >> Thanks,=0A= >> Michael=0A= >>=0A= >>> Otherwise no port will be start by default.=0A= >>>=0A= >>>=0A= >>> Thanks,=0A= >>> Michael=0A= >>>=0A= >>>> continue;=0A= >>>> =0A= >>>> port =3D &ports[pi];=0A= >>>> @@ -1421,7 +1472,7 @@ start_port(portid_t pid)=0A= >>>> }=0A= >>>> =0A= >>>> if (need_check_link_status && !no_link_check)=0A= >>>> - check_all_ports_link_status(nb_ports, RTE_PORT_ALL);=0A= >>>> + check_all_ports_link_status(RTE_PORT_ALL);=0A= >>>> else=0A= >>>> printf("Please stop the ports first\n");=0A= >>>> =0A= >>>> @@ -1446,8 +1497,8 @@ stop_port(portid_t pid)=0A= >>>> }=0A= >>>> printf("Stopping ports...\n");=0A= >>>> =0A= >>>> - for (pi =3D 0; pi < nb_ports; pi++) {=0A= >>>> - if (pid < nb_ports && pid !=3D pi)=0A= >>>> + FOREACH_PORT(pi, ports) {=0A= >>>> + if (!port_id_is_invalid(pid, DISABLED_WARN) && pid !=3D pi)=0A= >>>> continue;=0A= >>>> =0A= >>>> port =3D &ports[pi];=0A= >>>> @@ -1463,7 +1514,7 @@ stop_port(portid_t pid)=0A= >>>> need_check_link_status =3D 1;=0A= >>>> }=0A= >>>> if (need_check_link_status && !no_link_check)=0A= >>>> - check_all_ports_link_status(nb_ports, RTE_PORT_ALL);=0A= >>>> + check_all_ports_link_status(RTE_PORT_ALL);=0A= >>>> =0A= >>>> printf("Done\n");=0A= >>>> }=0A= >>>> @@ -1481,8 +1532,8 @@ close_port(portid_t pid)=0A= >>>> =0A= >>>> printf("Closing ports...\n");=0A= >>>> =0A= >>>> - for (pi =3D 0; pi < nb_ports; pi++) {=0A= >>>> - if (pid < nb_ports && pid !=3D pi)=0A= >>>> + FOREACH_PORT(pi, ports) {=0A= >>>> + if (!port_id_is_invalid(pid, DISABLED_WARN) && pid !=3D pi)=0A= >>>> continue;=0A= >>>> =0A= >>>> port =3D &ports[pi];=0A= >>>> @@ -1502,31 +1553,83 @@ close_port(portid_t pid)=0A= >>>> printf("Done\n");=0A= >>>> }=0A= >>>> =0A= >>>> -int=0A= >>>> -all_ports_stopped(void)=0A= >>>> +void=0A= >>>> +attach_port(char *identifier)=0A= >>>> {=0A= >>>> - portid_t pi;=0A= >>>> - struct rte_port *port;=0A= >>>> + portid_t i, j, pi =3D 0;=0A= >>>> =0A= >>>> - for (pi =3D 0; pi < nb_ports; pi++) {=0A= >>>> - port =3D &ports[pi];=0A= >>>> - if (port->port_status !=3D RTE_PORT_STOPPED)=0A= >>>> - return 0;=0A= >>>> + printf("Attaching a new port...\n");=0A= >>>> +=0A= >>>> + if (identifier =3D=3D NULL) {=0A= >>>> + printf("Invalid parameters are speficied\n");=0A= >>>> + return;=0A= >>>> }=0A= >>>> =0A= >>>> - return 1;=0A= >>>> + if (test_done =3D=3D 0) {=0A= >>>> + printf("Please stop forwarding first\n");=0A= >>>> + return;=0A= >>>> + }=0A= >>>> +=0A= >>>> + if (rte_eal_dev_attach(identifier, &pi))=0A= >>>> + return;=0A= >>>> +=0A= >>>> + ports[pi].enabled =3D 1;=0A= >>>> + reconfig(pi, rte_eth_dev_socket_id(pi));=0A= >>>> + rte_eth_promiscuous_enable(pi);=0A= >>>> +=0A= >>>> + nb_ports =3D rte_eth_dev_count();=0A= >>>> +=0A= >>>> + /* set_default_fwd_ports_config(); */=0A= >>>> + bzero(fwd_ports_ids, sizeof(fwd_ports_ids));=0A= >>>> + i =3D 0;=0A= >>>> + FOREACH_PORT(j, ports) {=0A= >>>> + fwd_ports_ids[i] =3D j;=0A= >>>> + i++;=0A= >>>> + }=0A= >>>> + nb_cfg_ports =3D nb_ports;=0A= >>>> + nb_fwd_ports++;=0A= >>>> +=0A= >>>> + ports[pi].port_status =3D RTE_PORT_STOPPED;=0A= >>>> +=0A= >>>> + printf("Port %d is attached. Now total ports is %d\n", pi, nb_ports)= ;=0A= >>>> + printf("Done\n");=0A= >>>> }=0A= >>>> =0A= >>>> -int=0A= >>>> -port_is_started(portid_t port_id)=0A= >>>> +void=0A= >>>> +detach_port(uint8_t port_id)=0A= >>>> {=0A= >>>> - if (port_id_is_invalid(port_id))=0A= >>>> - return -1;=0A= >>>> + portid_t i, pi =3D 0;=0A= >>>> + char name[RTE_ETH_NAME_MAX_LEN];=0A= >>>> =0A= >>>> - if (ports[port_id].port_status !=3D RTE_PORT_STARTED)=0A= >>>> - return 0;=0A= >>>> + printf("Detaching a port...\n");=0A= >>>> =0A= >>>> - return 1;=0A= >>>> + if (!port_is_closed(port_id)) {=0A= >>>> + printf("Please close port first\n");=0A= >>>> + return;=0A= >>>> + }=0A= >>>> +=0A= >>>> + rte_eth_promiscuous_disable(port_id);=0A= >>>> +=0A= >>>> + if (rte_eal_dev_detach(port_id, name))=0A= >>>> + return;=0A= >>>> +=0A= >>>> + ports[port_id].enabled =3D 0;=0A= >>>> + nb_ports =3D rte_eth_dev_count();=0A= >>>> +=0A= >>>> + /* set_default_fwd_ports_config(); */=0A= >>>> + bzero(fwd_ports_ids, sizeof(fwd_ports_ids));=0A= >>>> + i =3D 0;=0A= >>>> + FOREACH_PORT(pi, ports) {=0A= >>>> + fwd_ports_ids[i] =3D pi;=0A= >>>> + i++;=0A= >>>> + }=0A= >>>> + nb_cfg_ports =3D nb_ports;=0A= >>>> + nb_fwd_ports--;=0A= >>>> +=0A= >>>> + printf("Port '%s' is detached. Now total ports is %d\n",=0A= >>>> + name, nb_ports);=0A= >>>> + printf("Done\n");=0A= >>>> + return;=0A= >>>> }=0A= >>>> =0A= >>>> void=0A= >>>> @@ -1534,7 +1637,7 @@ pmd_test_exit(void)=0A= >>>> {=0A= >>>> portid_t pt_id;=0A= >>>> =0A= >>>> - for (pt_id =3D 0; pt_id < nb_ports; pt_id++) {=0A= >>>> + FOREACH_PORT(pt_id, ports) {=0A= >>>> printf("Stopping port %d...", pt_id);=0A= >>>> fflush(stdout);=0A= >>>> rte_eth_dev_close(pt_id);=0A= >>>> @@ -1553,7 +1656,7 @@ struct pmd_test_command {=0A= >>>> =0A= >>>> /* Check the link status of all ports in up to 9s, and print them fin= ally */=0A= >>>> static void=0A= >>>> -check_all_ports_link_status(uint8_t port_num, uint32_t port_mask)=0A= >>>> +check_all_ports_link_status(uint32_t port_mask)=0A= >>>> {=0A= >>>> #define CHECK_INTERVAL 100 /* 100ms */=0A= >>>> #define MAX_CHECK_TIME 90 /* 9s (90 * 100ms) in total */=0A= >>>> @@ -1564,7 +1667,7 @@ check_all_ports_link_status(uint8_t port_num, ui= nt32_t port_mask)=0A= >>>> fflush(stdout);=0A= >>>> for (count =3D 0; count <=3D MAX_CHECK_TIME; count++) {=0A= >>>> all_ports_up =3D 1;=0A= >>>> - for (portid =3D 0; portid < port_num; portid++) {=0A= >>>> + FOREACH_PORT(portid, ports) {=0A= >>>> if ((port_mask & (1 << portid)) =3D=3D 0)=0A= >>>> continue;=0A= >>>> memset(&link, 0, sizeof(link));=0A= >>>> @@ -1688,7 +1791,7 @@ init_port_config(void)=0A= >>>> portid_t pid;=0A= >>>> struct rte_port *port;=0A= >>>> =0A= >>>> - for (pid =3D 0; pid < nb_ports; pid++) {=0A= >>>> + FOREACH_PORT(pid, ports) {=0A= >>>> port =3D &ports[pid];=0A= >>>> port->dev_conf.rxmode =3D rx_mode;=0A= >>>> port->dev_conf.fdir_conf =3D fdir_conf;=0A= >>>> @@ -1877,7 +1980,7 @@ main(int argc, char** argv)=0A= >>>> =0A= >>>> nb_ports =3D (portid_t) rte_eth_dev_count();=0A= >>>> if (nb_ports =3D=3D 0)=0A= >>>> - rte_exit(EXIT_FAILURE, "No probed ethernet device\n");=0A= >>>> + RTE_LOG(WARNING, EAL, "No probed ethernet devices\n");=0A= >>>> =0A= >>>> set_def_fwd_config();=0A= >>>> if (nb_lcores =3D=3D 0)=0A= >>>> @@ -1899,7 +2002,7 @@ main(int argc, char** argv)=0A= >>>> rte_exit(EXIT_FAILURE, "Start ports failed\n");=0A= >>>> =0A= >>>> /* set all ports to promiscuous mode by default */=0A= >>>> - for (port_id =3D 0; port_id < nb_ports; port_id++)=0A= >>>> + FOREACH_PORT(port_id, ports)=0A= >>>> rte_eth_promiscuous_enable(port_id);=0A= >>>> =0A= >>>> #ifdef RTE_LIBRTE_CMDLINE=0A= >>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h=0A= >>>> index 8f5e6c7..109c670 100644=0A= >>>> --- a/app/test-pmd/testpmd.h=0A= >>>> +++ b/app/test-pmd/testpmd.h=0A= >>>> @@ -134,6 +134,7 @@ struct fwd_stream {=0A= >>>> * The data structure associated with each port.=0A= >>>> */=0A= >>>> struct rte_port {=0A= >>>> + uint8_t enabled; /**< Port enabled or not */=0A= >>>> struct rte_eth_dev_info dev_info; /**< PCI info + driver name */= =0A= >>>> struct rte_eth_conf dev_conf; /**< Port configuration. */=0A= >>>> struct ether_addr eth_addr; /**< Port ethernet address */=0A= >>>> @@ -159,6 +160,14 @@ struct rte_port {=0A= >>>> struct rte_eth_txconf tx_conf; /**< tx configuration */=0A= >>>> };=0A= >>>> =0A= >>>> +extern portid_t __rte_unused=0A= >>>> +find_next_port(portid_t p, struct rte_port *ports, int size);=0A= >>>> +=0A= >>>> +#define FOREACH_PORT(p, ports) \=0A= >>>> + for (p =3D find_next_port(0, ports, RTE_MAX_ETHPORTS); \=0A= >>>> + p < RTE_MAX_ETHPORTS; \=0A= >>>> + p =3D find_next_port(p + 1, ports, RTE_MAX_ETHPORTS))=0A= >>>> +=0A= >>>> /**=0A= >>>> * The data structure associated with each forwarding logical core.= =0A= >>>> * The logical cores are internally numbered by a core index from 0 t= o=0A= >>>> @@ -515,6 +524,8 @@ int init_port_dcb_config(portid_t pid,struct dcb_c= onfig *dcb_conf);=0A= >>>> int start_port(portid_t pid);=0A= >>>> void stop_port(portid_t pid);=0A= >>>> void close_port(portid_t pid);=0A= >>>> +void attach_port(char *identifier);=0A= >>>> +void detach_port(uint8_t port_id);=0A= >>>> int all_ports_stopped(void);=0A= >>>> int port_is_started(portid_t port_id);=0A= >>>> void pmd_test_exit(void);=0A= >>>> @@ -558,10 +569,15 @@ void get_ethertype_filter(uint8_t port_id, uint1= 6_t index);=0A= >>>> void get_2tuple_filter(uint8_t port_id, uint16_t index);=0A= >>>> void get_5tuple_filter(uint8_t port_id, uint16_t index);=0A= >>>> void get_flex_filter(uint8_t port_id, uint16_t index);=0A= >>>> -int port_id_is_invalid(portid_t port_id);=0A= >>>> int rx_queue_id_is_invalid(queueid_t rxq_id);=0A= >>>> int tx_queue_id_is_invalid(queueid_t txq_id);=0A= >>>> =0A= >>>> +enum print_warning {=0A= >>>> + ENABLED_WARN =3D 0,=0A= >>>> + DISABLED_WARN=0A= >>>> +};=0A= >>>> +int port_id_is_invalid(portid_t port_id, enum print_warning warning);= =0A= >>>> +=0A= >>>> /*=0A= >>>> * Work-around of a compilation error with ICC on invocations of the= =0A= >>>> * rte_be_to_cpu_16() function.=0A= >>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/= testpmd_app_ug/testpmd_funcs.rst=0A= >>>> index 218835a..1cacbcf 100644=0A= >>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst=0A= >>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst=0A= >>>> @@ -808,6 +808,63 @@ The following sections show functions for configu= ring ports.=0A= >>>> =0A= >>>> Port configuration changes only become active when forwarding is = started/restarted.=0A= >>>> =0A= >>>> +port attach=0A= >>>> +~~~~~~~~~~~=0A= >>>> +=0A= >>>> +Attach a port specified by pci address or virtual device args.=0A= >>>> +=0A= >>>> +To attach a new pci device, the device should be recognized by kernel= first.=0A= >>>> +Then it should be moved under DPDK management.=0A= >>>> +Finally the port can be attached to testpmd.=0A= >>>> +On the other hand, to attach a port created by virtual device, above = steps are not needed.=0A= >>>> +=0A= >>>> +port attach (identifier)=0A= >>>> +=0A= >>>> +For example, to attach a port that pci address is 0000:02:00.0.=0A= >>>> +=0A= >>>> +.. code-block:: console=0A= >>>> +=0A= >>>> + testpmd> port attach 0000:02:00.0=0A= >>>> + Attaching a new port...=0A= >>>> + ... snip ...=0A= >>>> + Port 0 is attached. Now total ports is 1=0A= >>>> + Done=0A= >>>> +=0A= >>>> +For example, to attach a port created by pcap PMD.=0A= >>>> +=0A= >>>> +.. code-block:: console=0A= >>>> +=0A= >>>> + testpmd> port attach eth_pcap0,iface=3Deth0=0A= >>>> + Attaching a new port...=0A= >>>> + ... snip ...=0A= >>>> + Port 0 is attached. Now total ports is 1=0A= >>>> + Done=0A= >>>> +=0A= >>>> +In this case, identifier is "eth_pcap0,iface=3Deth0".=0A= >>>> +This identifier format is the same as "--vdev" format of DPDK applica= tions.=0A= >>>> +=0A= >>>> +port detach=0A= >>>> +~~~~~~~~~~~=0A= >>>> +=0A= >>>> +Detach a specific port.=0A= >>>> +=0A= >>>> +Before detaching a port, the port should be closed.=0A= >>>> +Also to remove a pci device completely from the system, first detach = the port from testpmd.=0A= >>>> +Then the device should be moved under kernel management.=0A= >>>> +Finally the device can be remove using kernel pci hotplug functionali= ty.=0A= >>>> +On the other hand, to remove a port created by virtual device, above = steps are not needed.=0A= >>>> +=0A= >>>> +port detach (port_id)=0A= >>>> +=0A= >>>> +For example, to detach a port 0.=0A= >>>> +=0A= >>>> +.. code-block:: console=0A= >>>> +=0A= >>>> + testpmd> port detach 0=0A= >>>> + Detaching a port...=0A= >>>> + ... snip ...=0A= >>>> + Done=0A= >>>> +=0A= >>>> port start=0A= >>>> ~~~~~~~~~~=0A= >>>> =0A= >=0A= >=0A= =0A=