[dpdk-dev,v8,18/25] app/testpmd: use VFD APIs on i40e
Checks
Commit Message
The new VF Daemon (VFD) APIs is implemented on i40e. Change
testpmd code to use them, including VF MAC anti-spoofing,
VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN
insert.
Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
app/test-pmd/Makefile | 3 +
app/test-pmd/cmdline.c | 154 +++++++++++++++++++++++++++++++++++++++----------
2 files changed, 126 insertions(+), 31 deletions(-)
Comments
On 1/10/2017 7:16 AM, Wenzhuo Lu wrote:
> The new VF Daemon (VFD) APIs is implemented on i40e. Change
> testpmd code to use them, including VF MAC anti-spoofing,
> VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN
> insert.
>
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
<...>
> +#ifdef RTE_LIBRTE_IXGBE_PMD
Within this ifdef, there is following call:
ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
It is not safe to make that call directly, port should be verified first
if it is ixgbe port, as done in other functions.
> /* all queues drop enable configuration */
>
> /* Common result structure for all queues drop enable */
> @@ -11277,6 +11351,9 @@ struct cmd_all_queues_drop_en_result {
> case -ENODEV:
> printf("invalid port_id %d\n", res->port_id);
> break;
> + case -ENOTSUP:
> + printf("function not implemented\n");
> + break;
> default:
> printf("programming error: (%s)\n", strerror(-ret));
> }
> @@ -11381,6 +11458,7 @@ struct cmd_vf_split_drop_en_result {
> NULL,
> },
> };
> +#endif
>
<...>
> @@ -11619,20 +11711,20 @@ struct cmd_set_vf_mac_addr_result {
> (cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis,
> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add,
> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del,
> -#ifdef RTE_LIBRTE_IXGBE_PMD
> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
> (cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_insert,
> (cmdline_parse_inst_t *)&cmd_set_tx_loopback,
> +#ifdef RTE_LIBRTE_IXGBE_PMD
Overall, since port will be verified within the function, why not remove
this ifdef completely?
So all functions will be visible to everyone, but only the PMDs support
it will be called, others will return "not supported", as done in some
functions supports bot i40e and ixgbe pmd specific APIs.
> (cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en,
> (cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
> - (cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
> (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_set_vf_mac_addr,
> NULL,
> };
>
>
On 1/10/2017 11:29 AM, Ferruh Yigit wrote:
> On 1/10/2017 7:16 AM, Wenzhuo Lu wrote:
>> The new VF Daemon (VFD) APIs is implemented on i40e. Change
>> testpmd code to use them, including VF MAC anti-spoofing,
>> VF VLAN anti-spoofing, TX loopback, VF VLAN strip, VF VLAN
>> insert.
>>
>> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
>> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
>> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>> ---
>
> <...>
>
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>
> Within this ifdef, there is following call:
>
> ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
>
> It is not safe to make that call directly, port should be verified first
> if it is ixgbe port, as done in other functions.
With a second thought, although this part is not correct, it is out of
this patches scope, because it concerns ixgbe related part. So below
comment also become invalid.
Wenzhuo,
What do you think not changing this patch, but fixing this in a separate
patch?
Thanks,
ferruh
>
>> /* all queues drop enable configuration */
>>
>> /* Common result structure for all queues drop enable */
>> @@ -11277,6 +11351,9 @@ struct cmd_all_queues_drop_en_result {
>> case -ENODEV:
>> printf("invalid port_id %d\n", res->port_id);
>> break;
>> + case -ENOTSUP:
>> + printf("function not implemented\n");
>> + break;
>> default:
>> printf("programming error: (%s)\n", strerror(-ret));
>> }
>> @@ -11381,6 +11458,7 @@ struct cmd_vf_split_drop_en_result {
>> NULL,
>> },
>> };
>> +#endif
>>
>
> <...>
>
>> @@ -11619,20 +11711,20 @@ struct cmd_set_vf_mac_addr_result {
>> (cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis,
>> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add,
>> (cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del,
>> -#ifdef RTE_LIBRTE_IXGBE_PMD
>> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
>> (cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
>> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
>> (cmdline_parse_inst_t *)&cmd_set_vf_vlan_insert,
>> (cmdline_parse_inst_t *)&cmd_set_tx_loopback,
>> +#ifdef RTE_LIBRTE_IXGBE_PMD
>
> Overall, since port will be verified within the function, why not remove
> this ifdef completely?
>
> So all functions will be visible to everyone, but only the PMDs support
> it will be called, others will return "not supported", as done in some
> functions supports bot i40e and ixgbe pmd specific APIs.
>
>> (cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en,
>> (cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
>> - (cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
>> (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_set_vf_mac_addr,
>> NULL,
>> };
>>
>>
>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, January 11, 2017 4:09 AM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: Chen, Jing D; Iremonger, Bernard
> Subject: Re: [dpdk-dev] [PATCH v8 18/25] app/testpmd: use VFD APIs on i40e
>
> On 1/10/2017 11:29 AM, Ferruh Yigit wrote:
> > On 1/10/2017 7:16 AM, Wenzhuo Lu wrote:
> >> The new VF Daemon (VFD) APIs is implemented on i40e. Change testpmd
> >> code to use them, including VF MAC anti-spoofing, VF VLAN
> >> anti-spoofing, TX loopback, VF VLAN strip, VF VLAN insert.
> >>
> >> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
> >> Signed-off-by: Chen Jing D(Mark) <jing.d.chen@intel.com>
> >> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> >> ---
> >
> > <...>
> >
> >> +#ifdef RTE_LIBRTE_IXGBE_PMD
> >
> > Within this ifdef, there is following call:
> >
> > ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
> >
> > It is not safe to make that call directly, port should be verified
> > first if it is ixgbe port, as done in other functions.
>
> With a second thought, although this part is not correct, it is out of this
> patches scope, because it concerns ixgbe related part. So below comment also
> become invalid.
>
> Wenzhuo,
>
> What do you think not changing this patch, but fixing this in a separate patch?
Agree. I'll not change this patch but fix this issue with a separate patch.
>
> Thanks,
> ferruh
@@ -58,7 +58,10 @@ SRCS-y += csumonly.c
SRCS-y += icmpecho.c
SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c
+ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe
+_LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e
+endif
CFLAGS_cmdline.o := -D_GNU_SOURCE
@@ -90,6 +90,9 @@
#ifdef RTE_LIBRTE_IXGBE_PMD
#include <rte_pmd_ixgbe.h>
#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+#include <rte_pmd_i40e.h>
+#endif
#include "testpmd.h"
static struct cmdline *testpmd_cl;
@@ -262,19 +265,19 @@ static void cmd_help_long_parsed(void *parsed_result,
"set portlist (x[,y]*)\n"
" Set the list of forwarding ports.\n\n"
-#ifdef RTE_LIBRTE_IXGBE_PMD
"set tx loopback (port_id) (on|off)\n"
" Enable or disable tx loopback.\n\n"
+#ifdef RTE_LIBRTE_IXGBE_PMD
"set all queues drop (port_id) (on|off)\n"
" Set drop enable bit for all queues.\n\n"
"set vf split drop (port_id) (vf_id) (on|off)\n"
" Set split drop enable bit for a VF from the PF.\n\n"
+#endif
"set vf mac antispoof (port_id) (vf_id) (on|off).\n"
" Set MAC antispoof for a VF from the PF.\n\n"
-#endif
"vlan set strip (on|off) (port_id)\n"
" Set the VLAN strip on a port.\n\n"
@@ -282,7 +285,6 @@ static void cmd_help_long_parsed(void *parsed_result,
"vlan set stripq (on|off) (port_id,queue_id)\n"
" Set the VLAN strip for a queue on a port.\n\n"
-#ifdef RTE_LIBRTE_IXGBE_PMD
"set vf vlan stripq (port_id) (vf_id) (on|off)\n"
" Set the VLAN strip for all queues in a pool for a VF from the PF.\n\n"
@@ -291,7 +293,6 @@ static void cmd_help_long_parsed(void *parsed_result,
"set vf vlan antispoof (port_id) (vf_id) (on|off)\n"
" Set VLAN antispoof for a VF from the PF.\n\n"
-#endif
"vlan set filter (on|off) (port_id)\n"
" Set the VLAN filter on a port.\n\n"
@@ -386,10 +387,8 @@ static void cmd_help_long_parsed(void *parsed_result,
"mac_addr add port (port_id) vf (vf_id) (mac_address)\n"
" Add a MAC address for a VF on the port.\n\n"
-#ifdef RTE_LIBRTE_IXGBE_PMD
"set vf mac addr (port_id) (vf_id) (XX:XX:XX:XX:XX:XX)\n"
" Set the MAC address for a VF from the PF.\n\n"
-#endif
"set port (port_id) uta (mac_address|all) (on|off)\n"
" Add/Remove a or all unicast hash filter(s)"
@@ -6675,9 +6674,7 @@ struct cmd_set_vf_traffic {
NULL,
},
};
-#endif
-#ifdef RTE_LIBRTE_IXGBE_PMD
/* *** CONFIGURE VF RECEIVE MODE *** */
struct cmd_set_vf_rxmode {
cmdline_fixed_string_t set;
@@ -10808,7 +10805,6 @@ struct cmd_config_e_tag_result {
NULL,
},
};
-#ifdef RTE_LIBRTE_IXGBE_PMD
/* vf vlan anti spoof configuration */
@@ -10860,11 +10856,24 @@ struct cmd_vf_vlan_anti_spoof_result {
__attribute__((unused)) void *data)
{
struct cmd_vf_vlan_anti_spoof_result *res = parsed_result;
- int ret = 0;
- int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ int ret = -ENOTSUP;
+
+ __rte_unused int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ struct rte_eth_dev_info dev_info;
+
+ 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_anti_spoof(res->port_id,
+ res->vf_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_vf_vlan_anti_spoof(res->port_id,
+ res->vf_id, is_on);
+#endif
- ret = rte_pmd_ixgbe_set_vf_vlan_anti_spoof(res->port_id, res->vf_id,
- is_on);
switch (ret) {
case 0:
break;
@@ -10874,6 +10883,9 @@ struct cmd_vf_vlan_anti_spoof_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -10945,11 +10957,24 @@ struct cmd_vf_mac_anti_spoof_result {
__attribute__((unused)) void *data)
{
struct cmd_vf_mac_anti_spoof_result *res = parsed_result;
- int ret;
- int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ int ret = -ENOTSUP;
+
+ __rte_unused int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ struct rte_eth_dev_info dev_info;
+
+ 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_mac_anti_spoof(res->port_id,
+ res->vf_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_vf_mac_anti_spoof(res->port_id,
+ res->vf_id, is_on);
+#endif
- ret = rte_pmd_ixgbe_set_vf_mac_anti_spoof(res->port_id, res->vf_id,
- is_on);
switch (ret) {
case 0:
break;
@@ -10959,6 +10984,9 @@ struct cmd_vf_mac_anti_spoof_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -11030,10 +11058,24 @@ struct cmd_vf_vlan_stripq_result {
__attribute__((unused)) void *data)
{
struct cmd_vf_vlan_stripq_result *res = parsed_result;
- int ret = 0;
- int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ int ret = -ENOTSUP;
+
+ __rte_unused int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ struct rte_eth_dev_info dev_info;
+
+ 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_stripq(res->port_id,
+ res->vf_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_vf_vlan_stripq(res->port_id,
+ res->vf_id, is_on);
+#endif
- ret = rte_pmd_ixgbe_set_vf_vlan_stripq(res->port_id, res->vf_id, is_on);
switch (ret) {
case 0:
break;
@@ -11114,9 +11156,22 @@ struct cmd_vf_vlan_insert_result {
__attribute__((unused)) void *data)
{
struct cmd_vf_vlan_insert_result *res = parsed_result;
- int ret;
+ int ret = -ENOTSUP;
+ struct rte_eth_dev_info dev_info;
+
+ 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_insert(res->port_id, res->vf_id,
+ res->vlan_id);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_vf_vlan_insert(res->port_id, res->vf_id,
+ res->vlan_id);
+#endif
- ret = rte_pmd_ixgbe_set_vf_vlan_insert(res->port_id, res->vf_id, res->vlan_id);
switch (ret) {
case 0:
break;
@@ -11126,6 +11181,9 @@ struct cmd_vf_vlan_insert_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -11187,10 +11245,22 @@ struct cmd_tx_loopback_result {
__attribute__((unused)) void *data)
{
struct cmd_tx_loopback_result *res = parsed_result;
- int ret;
- int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ int ret = -ENOTSUP;
+
+ __rte_unused int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
+ struct rte_eth_dev_info dev_info;
+
+ 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_tx_loopback(res->port_id, is_on);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_tx_loopback(res->port_id, is_on);
+#endif
- ret = rte_pmd_ixgbe_set_tx_loopback(res->port_id, is_on);
switch (ret) {
case 0:
break;
@@ -11200,6 +11270,9 @@ struct cmd_tx_loopback_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -11219,6 +11292,7 @@ struct cmd_tx_loopback_result {
},
};
+#ifdef RTE_LIBRTE_IXGBE_PMD
/* all queues drop enable configuration */
/* Common result structure for all queues drop enable */
@@ -11277,6 +11351,9 @@ struct cmd_all_queues_drop_en_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -11381,6 +11458,7 @@ struct cmd_vf_split_drop_en_result {
NULL,
},
};
+#endif
/* vf mac address configuration */
@@ -11432,10 +11510,22 @@ struct cmd_set_vf_mac_addr_result {
__attribute__((unused)) void *data)
{
struct cmd_set_vf_mac_addr_result *res = parsed_result;
- int ret;
+ struct rte_eth_dev_info dev_info;
+ int ret = -ENOTSUP;
+
+ 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_mac_addr(res->port_id, res->vf_id,
+ &res->mac_addr);
+#endif
+#ifdef RTE_LIBRTE_I40E_PMD
+ if (strstr(dev_info.driver_name, "i40e") != NULL)
+ ret = rte_pmd_i40e_set_vf_mac_addr(res->port_id, res->vf_id,
+ &res->mac_addr);
+#endif
- ret = rte_pmd_ixgbe_set_vf_mac_addr(res->port_id, res->vf_id,
- &res->mac_addr);
switch (ret) {
case 0:
break;
@@ -11445,6 +11535,9 @@ struct cmd_set_vf_mac_addr_result {
case -ENODEV:
printf("invalid port_id %d\n", res->port_id);
break;
+ case -ENOTSUP:
+ printf("function not implemented\n");
+ break;
default:
printf("programming error: (%s)\n", strerror(-ret));
}
@@ -11465,7 +11558,6 @@ struct cmd_set_vf_mac_addr_result {
NULL,
},
};
-#endif
/* ******************************************************************************** */
@@ -11619,20 +11711,20 @@ struct cmd_set_vf_mac_addr_result {
(cmdline_parse_inst_t *)&cmd_config_e_tag_forwarding_en_dis,
(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_add,
(cmdline_parse_inst_t *)&cmd_config_e_tag_filter_del,
-#ifdef RTE_LIBRTE_IXGBE_PMD
(cmdline_parse_inst_t *)&cmd_set_vf_vlan_anti_spoof,
(cmdline_parse_inst_t *)&cmd_set_vf_mac_anti_spoof,
(cmdline_parse_inst_t *)&cmd_set_vf_vlan_stripq,
(cmdline_parse_inst_t *)&cmd_set_vf_vlan_insert,
(cmdline_parse_inst_t *)&cmd_set_tx_loopback,
+#ifdef RTE_LIBRTE_IXGBE_PMD
(cmdline_parse_inst_t *)&cmd_set_all_queues_drop_en,
(cmdline_parse_inst_t *)&cmd_set_vf_split_drop_en,
- (cmdline_parse_inst_t *)&cmd_set_vf_mac_addr,
(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_set_vf_mac_addr,
NULL,
};