[dpdk-dev,v4,5/5] app/test-pmd: add Port Representor commands

Message ID 20180108143720.7994-6-remy.horton@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Remy Horton Jan. 8, 2018, 2:37 p.m. UTC
  Port Representors provide a logical presentation in DPDK of VF (virtual
function) ports for the purposes of control and monitoring. Each port
representor device represents a single VF and is associated with it's
parent physical function (PF) PMD which provides the back-end hooks for
the representor device ops and defines the control domain to which that
port belongs. This allows to use existing DPDK APIs to monitor and control
the port without the need to create and maintain VF specific APIs.

This patch adds the 'add representor' and 'del representor' commands
to test-pmd, which respectively allow the adding and removing of
port representors.

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 app/test-pmd/cmdline.c                      | 88 +++++++++++++++++++++++++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 25 ++++++++
 2 files changed, 113 insertions(+)
  

Comments

Ferruh Yigit Jan. 9, 2018, 10:10 p.m. UTC | #1
On 1/8/2018 2:37 PM, Remy Horton wrote:
> Port Representors provide a logical presentation in DPDK of VF (virtual
> function) ports for the purposes of control and monitoring. Each port
> representor device represents a single VF and is associated with it's
> parent physical function (PF) PMD which provides the back-end hooks for
> the representor device ops and defines the control domain to which that
> port belongs. This allows to use existing DPDK APIs to monitor and control
> the port without the need to create and maintain VF specific APIs.
> 
> This patch adds the 'add representor' and 'del representor' commands
> to test-pmd, which respectively allow the adding and removing of
> port representors.
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>

<...>

> +
> +	rte_log(RTE_LOG_INFO, RTE_LOGTYPE_USER1, "%s(): addr:%s vport:%i\n",
> +		__func__, res->pf, res->vport);

this file tends to use printf for logs.
<...>

> @@ -15576,6 +15662,8 @@ cmdline_parse_ctx_t main_ctx[] = {
>  	(cmdline_parse_inst_t *) &cmd_show_bonding_config,
>  	(cmdline_parse_inst_t *) &cmd_set_bonding_primary,
>  	(cmdline_parse_inst_t *) &cmd_add_bonding_slave,
> +	(cmdline_parse_inst_t *) &cmd_add_representor,
> +	(cmdline_parse_inst_t *) &cmd_del_representor,

Please update cmd_help_long_parsed() to add new commands.

<...>

> +Adding a representor for a VF requires specifying the PF in
> +``Bus_DomBDF`` format alongside the index number of the VF::
> +
> +   testpmd> add representor pci_0000:81:00.0 0

I am for grouping port related commands under "port" command, these are all OK
in their context, but when you look into all testpmd command it turns into mess.

What do you think for:
port representor add <pf_address> <vport_id>
port representor del <pf_address> <vport_id>
  
Remy Horton Jan. 10, 2018, 7:22 a.m. UTC | #2
On 09/01/2018 22:10, Ferruh Yigit wrote:
[..]
>> +Adding a representor for a VF requires specifying the PF in
>> +``Bus_DomBDF`` format alongside the index number of the VF::
>> +
>> +   testpmd> add representor pci_0000:81:00.0 0
>
> I am for grouping port related commands under "port" command, these are all OK
> in their context, but when you look into all testpmd command it turns into mess.
>
> What do you think for:
> port representor add <pf_address> <vport_id>
> port representor del <pf_address> <vport_id>

Seems good to me - did notice some of the interactive help screens were 
a bit on the long side..
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f71d963..1a831ba 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -76,6 +76,7 @@ 
 #include <rte_eth_ctrl.h>
 #include <rte_flow.h>
 #include <rte_gro.h>
+#include <rte_port_representor.h>
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -15535,6 +15536,91 @@  cmdline_parse_inst_t cmd_load_from_file = {
 	},
 };
 
+struct cmd_add_representor_result {
+	cmdline_fixed_string_t cmd;
+	cmdline_fixed_string_t representor;
+	cmdline_fixed_string_t pf;
+	uint16_t vport;
+};
+
+cmdline_parse_token_string_t cmd_addrepresentor_add =
+TOKEN_STRING_INITIALIZER(struct cmd_add_representor_result,
+	cmd, "add");
+cmdline_parse_token_string_t cmd_addrepresentor_del =
+TOKEN_STRING_INITIALIZER(struct cmd_add_representor_result,
+	cmd, "del");
+cmdline_parse_token_string_t cmd_addrepresentor_rep =
+TOKEN_STRING_INITIALIZER(struct cmd_add_representor_result,
+	representor, "representor");
+cmdline_parse_token_string_t cmd_addrepresentor_pf =
+TOKEN_STRING_INITIALIZER(struct cmd_add_representor_result,
+	pf, NULL);
+cmdline_parse_token_num_t cmd_addrepresentor_vport =
+TOKEN_NUM_INITIALIZER(struct cmd_add_representor_result,
+	vport, UINT16);
+
+static void cmd_add_representor_callback(void *parsed_result,
+		__attribute__((unused))  struct cmdline *cl,
+		__attribute__((unused)) void *data)
+{
+	struct cmd_add_representor_result *res = parsed_result;
+	uint16_t port_id;
+	int ret;
+
+	rte_log(RTE_LOG_INFO, RTE_LOGTYPE_USER1, "%s(): addr:%s vport:%i\n",
+		__func__, res->pf, res->vport);
+
+	ret = rte_representor_port_register(res->pf, res->vport, &port_id);
+	if (ret != 0)
+		printf("Registering port representor failed\n");
+	else
+		printf("Port Representor registered with port id %i\n",
+			port_id);
+}
+
+static void cmd_del_representor_callback(void *parsed_result,
+		__attribute__((unused))  struct cmdline *cl,
+		__attribute__((unused)) void *data)
+{
+	struct cmd_add_representor_result *res = parsed_result;
+	int ret;
+
+	rte_log(RTE_LOG_INFO, RTE_LOGTYPE_USER1, "%s(): port:%i\n", __func__,
+		res->vport);
+	ret = rte_representor_port_unregister(res->pf, res->vport);
+	if (ret != 0)
+		printf("Port %i is not a valid port representor.\n",
+			res->vport);
+}
+
+cmdline_parse_inst_t cmd_add_representor = {
+	.f = cmd_add_representor_callback,
+	.help_str = "add representor <BusName_DomBDF> <vport_id> "
+		"Add a Port Representor",
+	.data = NULL,
+	.tokens = {
+		(void *)&cmd_addrepresentor_add,
+		(void *)&cmd_addrepresentor_rep,
+		(void *)&cmd_addrepresentor_pf,
+		(void *)&cmd_addrepresentor_vport,
+		NULL
+	}
+};
+
+cmdline_parse_inst_t cmd_del_representor = {
+	.f = cmd_del_representor_callback,
+	.help_str = "del representor <BusName_DomBDF> <vport_id> "
+		"Delete a Port Representor",
+	.data = NULL,
+	.tokens = {
+		(void *)&cmd_addrepresentor_del,
+		(void *)&cmd_addrepresentor_rep,
+		(void *)&cmd_addrepresentor_pf,
+		(void *)&cmd_addrepresentor_vport,
+		NULL
+	}
+};
+
 /* ******************************************************************************** */
 
 /* list of instructions */
@@ -15576,6 +15662,8 @@  cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *) &cmd_show_bonding_config,
 	(cmdline_parse_inst_t *) &cmd_set_bonding_primary,
 	(cmdline_parse_inst_t *) &cmd_add_bonding_slave,
+	(cmdline_parse_inst_t *) &cmd_add_representor,
+	(cmdline_parse_inst_t *) &cmd_del_representor,
 	(cmdline_parse_inst_t *) &cmd_remove_bonding_slave,
 	(cmdline_parse_inst_t *) &cmd_create_bonded_device,
 	(cmdline_parse_inst_t *) &cmd_set_bond_mac_addr,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 9789139..0eaeb38 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -3481,3 +3481,28 @@  Validate and create a QinQ rule on port 0 to steer traffic to a queue on the hos
    ID      Group   Prio    Attr    Rule
    0       0       0       i-      ETH VLAN VLAN=>VF QUEUE
    1       0       0       i-      ETH VLAN VLAN=>PF QUEUE
+
+Port Representors
+-----------------
+
+Adding and removal of port representors (*rte_representor*) is done via the
+``add representor`` and ``del representor`` commands. Once created these
+can take the same control commands as the underlying VF (Virtual Function).
+
+
+Adding a representor
+~~~~~~~~~~~~~~~~~~~~
+
+Adding a representor for a VF requires specifying the PF in
+``Bus_DomBDF`` format alongside the index number of the VF::
+
+   testpmd> add representor pci_0000:81:00.0 0
+
+
+Removing a representor
+~~~~~~~~~~~~~~~~~~~~~~
+
+To remove a representor, the same parameters are required as were used to
+create it::
+
+    testpmd> del representor pci_0000:81:00.0 0