diff mbox series

[1/3] app/testpmd: add/remove multiple VLAN filter IDs at once

Message ID 20250411081005.1133509-2-chaoyong.he@corigine.com (mailing list archive)
State Changes Requested
Delegated to: Stephen Hemminger
Headers
Series enhance the vlan filter feature |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Chaoyong He April 11, 2025, 8:10 a.m. UTC 
From: Long Wu <long.wu@corigine.com>

The command of setting VLAN filter IDs can only set one VLAN filter
ID at once. For example:
testpmd> rx_vlan add 100 0

This is very inefficient in some case because it forces the
user type this command as many times as the VLAN filter IDs
they want to set.

This patch supports the user set any number valid VLAN filter
IDs just through one command and it is also backwards compatible.
For example:
testpmd> rx_vlan add 100,102-103,200 0
The VLAN filter IDs 100, 102, 103, 200 will be added to port 0.

Signed-off-by: Long Wu <long.wu@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
---
 app/test-pmd/cmdline.c                      | 46 ++++++++++-----
 app/test-pmd/config.c                       | 64 +++++++++++++++++++++
 app/test-pmd/testpmd.h                      |  1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 20 +++++--
 4 files changed, 112 insertions(+), 19 deletions(-)

Comments

Stephen Hemminger April 11, 2025, 4:27 p.m. UTC | #1 
On Fri, 11 Apr 2025 16:10:03 +0800
Chaoyong He <chaoyong.he@corigine.com> wrote:

> From: Long Wu <long.wu@corigine.com>
> 
> The command of setting VLAN filter IDs can only set one VLAN filter
> ID at once. For example:
> testpmd> rx_vlan add 100 0  
> 
> This is very inefficient in some case because it forces the
> user type this command as many times as the VLAN filter IDs
> they want to set.
> 
> This patch supports the user set any number valid VLAN filter
> IDs just through one command and it is also backwards compatible.
> For example:
> testpmd> rx_vlan add 100,102-103,200 0  
> The VLAN filter IDs 100, 102, 103, 200 will be added to port 0.
> 
> Signed-off-by: Long Wu <long.wu@corigine.com>
> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>

Not sure if this is really worth the effort.
Testpmd is intended to exercise drivers, not as a production forwarding application
though users have tendency to be lazy and use it for that.

If consensus is to implement this, then DPDK should really have a way to parse
a range of values (already needed for cores). Then the library code could be used
here, rather than special parsing code only for VLAN's in testpmd.
Chaoyong He April 14, 2025, 6:26 a.m. UTC | #2 
> On Fri, 11 Apr 2025 16:10:03 +0800
> Chaoyong He <chaoyong.he@corigine.com> wrote:
> 
> > From: Long Wu <long.wu@corigine.com>
> >
> > The command of setting VLAN filter IDs can only set one VLAN filter ID
> > at once. For example:
> > testpmd> rx_vlan add 100 0
> >
> > This is very inefficient in some case because it forces the user type
> > this command as many times as the VLAN filter IDs they want to set.
> >
> > This patch supports the user set any number valid VLAN filter IDs just
> > through one command and it is also backwards compatible.
> > For example:
> > testpmd> rx_vlan add 100,102-103,200 0
> > The VLAN filter IDs 100, 102, 103, 200 will be added to port 0.
> >
> > Signed-off-by: Long Wu <long.wu@corigine.com>
> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
> 
> Not sure if this is really worth the effort.
> Testpmd is intended to exercise drivers, not as a production forwarding
> application though users have tendency to be lazy and use it for that.
> 
> If consensus is to implement this, then DPDK should really have a way to parse
> a range of values (already needed for cores). Then the library code could be
> used here, rather than special parsing code only for VLAN's in testpmd.

Okay, we can wait for this one.

Then how about the 2/3 and 3/3 patch?
Which are add a 'show' command for the VLAN filter IDs and I think they can split from this one.
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d4bb3ec998..5a627a9bc6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -411,12 +411,12 @@  static void cmd_help_long_parsed(void *parsed_result,
 			"    Set the VLAN TPID for Packet Filtering on"
 			" a port\n\n"
 
-			"rx_vlan add (vlan_id|all) (port_id)\n"
-			"    Add a vlan_id, or all identifiers, to the set"
+			"rx_vlan add (vlan_id_list|all) (port_id)\n"
+			"    Add a list of vlan_id, or all identifiers, to the set"
 			" of VLAN identifiers filtered by port_id.\n\n"
 
-			"rx_vlan rm (vlan_id|all) (port_id)\n"
-			"    Remove a vlan_id, or all identifiers, from the set"
+			"rx_vlan rm (vlan_id_list|all) (port_id)\n"
+			"    Remove a list of vlan_id, or all identifiers, from the set"
 			" of VLAN identifiers filtered by port_id.\n\n"
 
 			"rx_vlan add (vlan_id) port (port_id) vf (vf_mask)\n"
@@ -4653,11 +4653,11 @@  static cmdline_parse_inst_t cmd_vlan_tpid = {
 	},
 };
 
-/* *** ADD/REMOVE A VLAN IDENTIFIER TO/FROM A PORT VLAN RX FILTER *** */
+/* *** ADD/REMOVE A LIST OF VLAN IDENTIFIERS TO/FROM A PORT VLAN RX FILTER *** */
 struct cmd_rx_vlan_filter_result {
 	cmdline_fixed_string_t rx_vlan;
 	cmdline_fixed_string_t what;
-	uint16_t vlan_id;
+	cmdline_fixed_string_t vlan_id_list;
 	portid_t port_id;
 };
 
@@ -4666,12 +4666,28 @@  cmd_rx_vlan_filter_parsed(void *parsed_result,
 			  __rte_unused struct cmdline *cl,
 			  __rte_unused void *data)
 {
+	int i;
+	int on = 0;
+	int vlan_id_count;
+	uint16_t vlan_id[4096];
 	struct cmd_rx_vlan_filter_result *res = parsed_result;
 
-	if (!strcmp(res->what, "add"))
-		rx_vft_set(res->port_id, res->vlan_id, 1);
-	else
-		rx_vft_set(res->port_id, res->vlan_id, 0);
+	vlan_id_count = rx_vft_list_parse(vlan_id, res->vlan_id_list);
+	if (vlan_id_count <= 0) {
+		fprintf(stderr, "Please input correct vlan id list\n");
+		return;
+	}
+
+	if (strcmp(res->what, "add") == 0)
+		on = 1;
+
+	for (i = 0; i < vlan_id_count; i++) {
+		if (rx_vft_set(res->port_id, vlan_id[i], on) != 0) {
+			fprintf(stderr, "Failed to %s vlan id %d\n",
+					res->what, vlan_id[i]);
+			break;
+		}
+	}
 }
 
 static cmdline_parse_token_string_t cmd_rx_vlan_filter_rx_vlan =
@@ -4680,9 +4696,9 @@  static cmdline_parse_token_string_t cmd_rx_vlan_filter_rx_vlan =
 static cmdline_parse_token_string_t cmd_rx_vlan_filter_what =
 	TOKEN_STRING_INITIALIZER(struct cmd_rx_vlan_filter_result,
 				 what, "add#rm");
-static cmdline_parse_token_num_t cmd_rx_vlan_filter_vlanid =
-	TOKEN_NUM_INITIALIZER(struct cmd_rx_vlan_filter_result,
-			      vlan_id, RTE_UINT16);
+static cmdline_parse_token_string_t cmd_rx_vlan_filter_vlanid =
+	TOKEN_STRING_INITIALIZER(struct cmd_rx_vlan_filter_result,
+				 vlan_id_list, NULL);
 static cmdline_parse_token_num_t cmd_rx_vlan_filter_portid =
 	TOKEN_NUM_INITIALIZER(struct cmd_rx_vlan_filter_result,
 			      port_id, RTE_UINT16);
@@ -4690,8 +4706,8 @@  static cmdline_parse_token_num_t cmd_rx_vlan_filter_portid =
 static cmdline_parse_inst_t cmd_rx_vlan_filter = {
 	.f = cmd_rx_vlan_filter_parsed,
 	.data = NULL,
-	.help_str = "rx_vlan add|rm <vlan_id> <port_id>: "
-		"Add/Remove a VLAN identifier to/from the set of VLAN "
+	.help_str = "rx_vlan add|rm <vlan_id_list0[,vlan_id_list1]*> <port_id>: "
+		"Add/Remove a list of VLAN identifiers to/from the set of VLAN "
 		"identifiers filtered by a port",
 	.tokens = {
 		(void *)&cmd_rx_vlan_filter_rx_vlan,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index e89af21cec..61f93a9ca5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -6612,6 +6612,70 @@  rx_vlan_all_filter_set(portid_t port_id, int on)
 	}
 }
 
+int
+rx_vft_list_parse(uint16_t *vlan_id, char *vlan_id_list)
+{
+	int i;
+	int min;
+	int max;
+	bool dup;
+	int index;
+	int count = 0;
+	char *end = NULL;
+
+	/*
+	 * The acceptable VLAN ID list format should be like: 1,3-4,7.
+	 * Use `,` to separate VLAN ID list.
+	 * Use `-` to represent range.
+	 */
+	min = -1;
+	do {
+		while (isblank(*vlan_id_list))
+			vlan_id_list++;
+		if (*vlan_id_list == '\0')
+			return -1;
+
+		errno = 0;
+		index = strtol(vlan_id_list, &end, 10);
+		if (errno || end == NULL)
+			return -1;
+		if (index < 0)
+			return -1;
+
+		while (isblank(*end))
+			end++;
+
+		if ((*end == ',') || (*end == '\0')) {
+			max = index;
+			if (min == -1)
+				min = index;
+			for (index = min; index <= max; index++) {
+				dup = false;
+
+				for (i = 0; i < count; i++) {
+					if (vlan_id[i] == index)
+						dup = true;
+				}
+				if (dup)
+					continue;
+				if (count >= 4096)
+					return -1;
+
+				vlan_id[count++] = index;
+			}
+			min = -1;
+		} else if (*end == '-') {
+			min = index;
+		} else {
+			return -1;
+		}
+
+		vlan_id_list = end + 1;
+	} while (*end != '\0');
+
+	return count;
+}
+
 void
 vlan_tpid_set(portid_t port_id, enum rte_vlan_type vlan_type, uint16_t tp_id)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 05ccdec8d2..ac4f716408 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1103,6 +1103,7 @@  void rx_vlan_strip_set_on_queue(portid_t port_id, uint16_t queue_id, int on);
 void rx_vlan_filter_set(portid_t port_id, int on);
 void rx_vlan_all_filter_set(portid_t port_id, int on);
 void rx_vlan_qinq_strip_set(portid_t port_id, int on);
+int rx_vft_list_parse(uint16_t *vlan_id, char *vlan_id_list);
 int rx_vft_set(portid_t port_id, uint16_t vlan_id, int on);
 void vlan_extend_set(portid_t port_id, int on);
 void vlan_tpid_set(portid_t port_id, enum rte_vlan_type vlan_type,
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index eeef49500f..b35722d2f5 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1035,9 +1035,15 @@  Set the inner or outer VLAN TPID for packet filtering on a port::
 rx_vlan add
 ~~~~~~~~~~~
 
-Add a VLAN ID, or all identifiers, to the set of VLAN identifiers filtered by port ID::
+Add a list of VLAN IDs, or all identifiers, to the set of VLAN identifiers filtered by port ID::
 
-   testpmd> rx_vlan add (vlan_id|all) (port_id)
+   testpmd> rx_vlan add (<vlan_id_list0[,vlan_id_list1]*>|all) (port_id)
+
+For example, to add some VLAN identifiers filtered by port:
+
+.. code-block:: console
+
+   testpmd> rx_vlan add 1,3-4,7 0
 
 .. note::
 
@@ -1048,9 +1054,15 @@  Add a VLAN ID, or all identifiers, to the set of VLAN identifiers filtered by po
 rx_vlan rm
 ~~~~~~~~~~
 
-Remove a VLAN ID, or all identifiers, from the set of VLAN identifiers filtered by port ID::
+Remove a list of VLAN IDs, or all identifiers, from the set of VLAN identifiers filtered by port ID::
+
+   testpmd> rx_vlan rm (<vlan_id_list0[,vlan_id_list1]*>|all) (port_id)
+
+For example, to remove some VLAN identifiers filtered by port:
+
+.. code-block:: console
 
-   testpmd> rx_vlan rm (vlan_id|all) (port_id)
+   testpmd> rx_vlan rm 1,3-4,7 0
 
 rx_vlan add (for VF)
 ~~~~~~~~~~~~~~~~~~~~