[dpdk-dev,v5,23/29] app/testpmd: handle i40e in VF VLAN filter command

Message ID 20161216190257.6921-24-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation fail apply patch file failure

Commit Message

Ferruh Yigit Dec. 16, 2016, 7:02 p.m. UTC
  From: Bernard Iremonger <bernard.iremonger@intel.com>

modify set_vf_rx_vlan function to handle the i40e PMD.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 app/test-pmd/cmdline.c | 40 +++++++++++++++++++++++++++++++++-------
 app/test-pmd/config.c  | 13 -------------
 app/test-pmd/testpmd.h |  2 --
 3 files changed, 33 insertions(+), 22 deletions(-)
  

Comments

Vincent Jardin Dec. 16, 2016, 8:31 p.m. UTC | #1
Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
> +#ifdef RTE_LIBRTE_IXGBE_PMD
> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> +		ret = rte_pmd_ixgbe_set_vf_vlan_filter(res->port_id,
> +				res->vlan_id, res->vf_mask, is_add);
> +#endif
> +#ifdef RTE_LIBRTE_I40E_PMD
> +	if (strstr(dev_info.driver_name, "i40e") != NULL)
> +		ret = rte_pmd_i40e_set_vf_vlan_filter(res->port_id,
> +				res->vlan_id, res->vf_mask, is_add);
> +#endif

That's exactly what we need to avoid, it won't scale to many PMDs.
  
Ferruh Yigit Dec. 19, 2016, 11:13 a.m. UTC | #2
On 12/16/2016 8:31 PM, Vincent JARDIN wrote:
> Le 16/12/2016 à 20:02, Ferruh Yigit a écrit :
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
>> +		ret = rte_pmd_ixgbe_set_vf_vlan_filter(res->port_id,
>> +				res->vlan_id, res->vf_mask, is_add);
>> +#endif
>> +#ifdef RTE_LIBRTE_I40E_PMD
>> +	if (strstr(dev_info.driver_name, "i40e") != NULL)
>> +		ret = rte_pmd_i40e_set_vf_vlan_filter(res->port_id,
>> +				res->vlan_id, res->vf_mask, is_add);
>> +#endif
> 
> That's exactly what we need to avoid, it won't scale to many PMDs.
> 

For a generic PMD feature, completely agree with you. Application
shouldn't know/worry about underlying hardware. eth_dev layer should be
used.
But above usage is for an application that knows the hardware, and
knowing that it is losing all the benefits of the portability and
explicitly including the PMD header to use those PMD specific APIs.

Thanks,
ferruh
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 2b17925..81273b6 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6835,7 +6835,6 @@  cmdline_parse_inst_t cmd_vf_mac_addr_filter = {
 	},
 };
 
-#ifdef RTE_LIBRTE_IXGBE_PMD
 /* *** ADD/REMOVE A VLAN IDENTIFIER TO/FROM A PORT VLAN RX FILTER *** */
 struct cmd_vf_rx_vlan_filter {
 	cmdline_fixed_string_t rx_vlan;
@@ -6853,11 +6852,39 @@  cmd_vf_rx_vlan_filter_parsed(void *parsed_result,
 			  __attribute__((unused)) void *data)
 {
 	struct cmd_vf_rx_vlan_filter *res = parsed_result;
+	int ret = -ENOTSUP;
+	__rte_unused int is_add = (strcmp(res->what, "add") == 0) ? 1 : 0;
+	struct rte_eth_dev_info dev_info;
 
-	if (!strcmp(res->what, "add"))
-		set_vf_rx_vlan(res->port_id, res->vlan_id,res->vf_mask, 1);
-	else
-		set_vf_rx_vlan(res->port_id, res->vlan_id,res->vf_mask, 0);
+	rte_eth_dev_info_get(res->port_id, &dev_info);
+
+#ifdef RTE_LIBRTE_IXGBE_PMD
+	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
+		ret = rte_pmd_ixgbe_set_vf_vlan_filter(res->port_id,
+				res->vlan_id, res->vf_mask, is_add);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (strstr(dev_info.driver_name, "i40e") != NULL)
+		ret = rte_pmd_i40e_set_vf_vlan_filter(res->port_id,
+				res->vlan_id, res->vf_mask, is_add);
+#endif
+
+	switch (ret) {
+	case 0:
+		break;
+	case -EINVAL:
+		printf("invalid vlan_id %d or vf_mask %lu\n",
+				res->vlan_id, res->vf_mask);
+		break;
+	case -ENODEV:
+		printf("invalid port_id %d\n", res->port_id);
+		break;
+	case -ENOTSUP:
+		printf("function not implemented or supported\n");
+		break;
+	default:
+		printf("programming error: (%s)\n", strerror(-ret));
+	}
 }
 
 cmdline_parse_token_string_t cmd_vf_rx_vlan_filter_rx_vlan =
@@ -6898,7 +6925,6 @@  cmdline_parse_inst_t cmd_vf_rxvlan_filter = {
 		NULL,
 	},
 };
-#endif
 
 /* *** SET RATE LIMIT FOR A QUEUE OF A PORT *** */
 struct cmd_queue_rate_limit_result {
@@ -12085,9 +12111,9 @@  cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
 	(cmdline_parse_inst_t *)&cmd_set_vf_rxmode,
 	(cmdline_parse_inst_t *)&cmd_set_vf_traffic,
-	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
 	(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
 #endif
+	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
 	(cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
 	(cmdline_parse_inst_t *)&cmd_set_vf_promisc,
 	(cmdline_parse_inst_t *)&cmd_set_vf_allmulti,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index fc0424a..38aa0b9 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2348,19 +2348,6 @@  set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on)
 	       		"diag=%d\n", port_id, diag);
 
 }
-
-void
-set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id, uint64_t vf_mask, uint8_t on)
-{
-	int diag;
-
-	diag = rte_pmd_ixgbe_set_vf_vlan_filter(port_id, vlan_id, vf_mask, on);
-
-	if (diag == 0)
-		return;
-	printf("rte_pmd_ixgbe_set_vf_vlan_filter for port_id=%d failed "
-	       "diag=%d\n", port_id, diag);
-}
 #endif
 
 int
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 9c1e703..5d104bd 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -571,8 +571,6 @@  void port_rss_reta_info(portid_t port_id,
 			uint16_t nb_entries);
 
 void set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on);
-void set_vf_rx_vlan(portid_t port_id, uint16_t vlan_id,
-		uint64_t vf_mask, uint8_t on);
 
 int set_queue_rate_limit(portid_t port_id, uint16_t queue_idx, uint16_t rate);
 int set_vf_rate_limit(portid_t port_id, uint16_t vf, uint16_t rate,