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

2015-02-05 Thread Tetsuya Mukawa 
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. :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 
>>> [...]
>>>
> +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 = [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 = [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 = [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 = [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;
>   }
>  
> -

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

2015-02-04 Thread Qiu, Michael 
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. :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 
>> [...]
>>
 +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 = [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 = [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 = [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 = [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, ))
 +  return;
 +
 +

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

2015-02-03 Thread Tetsuya Mukawa 
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 :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.
>>>

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

2015-02-03 Thread Tetsuya Mukawa 
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. :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 
> [...]
>
>>> +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 = [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 = [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 = [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 = [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, ))
>>> +   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

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

2015-02-03 Thread Tetsuya Mukawa 
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. :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 
>> ---
>>  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|1|4|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 *)_operate_attach_port_port,
>> +(void *)_operate_attach_port_keyword,
>> +(void *)_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 =

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

2015-02-03 Thread Tetsuya Mukawa 
On 2015/02/02 20:57, Thomas Monjalon wrote:
> 2015-02-02 11:33, Iremonger, Bernard:
>> From: Tetsuya Mukawa [mailto:mukawa at 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

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

2015-02-03 Thread Tetsuya Mukawa 
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 :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 :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.
>

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

2015-02-03 Thread Iremonger, Bernard 

> >> +.. code-block:: console
> >> +
> >> +testpmd> port attach :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.
> >

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

2015-02-03 Thread Qiu, Michael 
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. :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 

[...]

>> +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 = [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 = [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 = [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 = [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, ))
>> +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)) {
>> +

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

2015-02-03 Thread Qiu, Michael 
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. :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 
> ---
>  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|1|4|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 *)_operate_attach_port_port,
> + (void *)_operate_attach_port_keyword,
> + (void *)_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 *)_operate_detach_port_port,
> +

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

2015-02-03 Thread Qiu, Michael 
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. :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 
> ---
>  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|1|4|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 *)_operate_attach_port_port,
> + (void *)_operate_attach_port_keyword,
> + (void *)_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 *)_operate_detach_port_port,
> +

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

2015-02-01 Thread Tetsuya Mukawa 
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. :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 
---
 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|1|4|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 *)_operate_attach_port_port,
+   (void *)_operate_attach_port_keyword,
+   (void *)_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 *)_operate_detach_port_port,
+   (void *)_operate_detach_port_keyword,
+   (void *)_operate_detach_port_port_id,
+   NULL,
+   },
+};
+
 /* *** configure speed for all ports *** */
 struct