[dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API calling

[Iremonger, Bernard]

Hi Wenzhuo,


> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Wednesday, January 11, 2017 2:48 AM
> To: dev at dpdk.org
> Cc: Wu, Jingjing <jingjing.wu at intel.com>; Lu, Wenzhuo
> <wenzhuo.lu at intel.com>; stable at dpdk.org
> Subject: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API calling
> 
> Some ixgbe private APIs are added to expose ixgbe specific functions.
> When they're used by testpmd, there's no check for if the NICs are ixgbe.
> Other NICs also have chance to  call these APIs.
> This patch add the check and the feedback print.

I am not sure that testpmd is the right place to do this.
The rte_pmd_ixgbe_* functions are public API's which can be called by other applications.
The checks should be in the rte_pmd_ixgbe_* API's

 
> Fixes: 425781ff5afe ("app/testpmd: add ixgbe VF management")
> 
> CC: stable at dpdk.org
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu at intel.com>
> ---
>  app/test-pmd/cmdline.c | 101
> +++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 81 insertions(+), 20 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> f768b6b..172861a 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -10883,11 +10883,16 @@ 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 ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_vf_vlan_anti_spoof(res->port_id, res-
> >vf_id,
> -			is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	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);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -10897,6 +10902,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));
>  	}
> @@ -10968,11 +10976,16 @@ struct cmd_vf_mac_anti_spoof_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_vf_mac_anti_spoof_result *res = parsed_result;
> -	int ret;
> +	int ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_vf_mac_anti_spoof(res->port_id, res-
> >vf_id,
> -			is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	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);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -10982,6 +10995,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));
>  	}
> @@ -11053,10 +11069,15 @@ struct cmd_vf_vlan_stripq_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_vf_vlan_stripq_result *res = parsed_result;
> -	int ret = 0;
> +	int ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_vf_vlan_stripq(res->port_id, res->vf_id,
> is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> +		ret = rte_pmd_ixgbe_set_vf_vlan_stripq(res->port_id, res-
> >vf_id,
> +						       is_on);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11066,6 +11087,9 @@ struct cmd_vf_vlan_stripq_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));
>  	}
> @@ -11137,9 +11161,14 @@ 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;
> 
> -	ret = rte_pmd_ixgbe_set_vf_vlan_insert(res->port_id, res->vf_id,
> res->vlan_id);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	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);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11149,6 +11178,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));
>  	}
> @@ -11210,10 +11242,14 @@ struct cmd_tx_loopback_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_tx_loopback_result *res = parsed_result;
> -	int ret;
> +	int ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_tx_loopback(res->port_id, is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> +		ret = rte_pmd_ixgbe_set_tx_loopback(res->port_id, is_on);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11223,6 +11259,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));
>  	}
> @@ -11287,10 +11326,14 @@ struct cmd_all_queues_drop_en_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_all_queues_drop_en_result *res = parsed_result;
> -	int ret = 0;
> +	int ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_all_queues_drop_en(res->port_id, is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> +		ret = rte_pmd_ixgbe_set_all_queues_drop_en(res-
> >port_id, is_on);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11300,6 +11343,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));
>  	}
> @@ -11370,11 +11416,16 @@ struct cmd_vf_split_drop_en_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_vf_split_drop_en_result *res = parsed_result;
> -	int ret;
> +	int ret = -ENOTSUP;
>  	int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_vf_split_drop_en(res->port_id, res-
> >vf_id,
> -			is_on);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	if (strstr(dev_info.driver_name, "ixgbe") != NULL)
> +		ret = rte_pmd_ixgbe_set_vf_split_drop_en(res->port_id,
> +							 res->vf_id,
> +							 is_on);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11384,6 +11435,9 @@ struct cmd_vf_split_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));
>  	}
> @@ -11455,10 +11509,14 @@ struct cmd_set_vf_mac_addr_result {
>  	__attribute__((unused)) void *data)
>  {
>  	struct cmd_set_vf_mac_addr_result *res = parsed_result;
> -	int ret;
> +	int ret = -ENOTSUP;
> +	struct rte_eth_dev_info dev_info;
> 
> -	ret = rte_pmd_ixgbe_set_vf_mac_addr(res->port_id, res->vf_id,
> -			&res->mac_addr);
> +	rte_eth_dev_info_get(res->port_id, &dev_info);
> +
> +	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);
>  	switch (ret) {
>  	case 0:
>  		break;
> @@ -11468,6 +11526,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));
>  	}
> --
> 1.9.3

Regards,

Bernard.