Re: [dpdk-dev] [PATCH v9 12/18] net/ipn3ke: remove configuration for i40e port bonding

["Pei, Andy" <andy.pei@intel.com>]

Hi Conole,

Thanks.
It will be included in next version.

-----Original Message-----
From: Aaron Conole [mailto:aconole@redhat.com] 
Sent: Monday, October 14, 2019 10:40 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Zhang, Tianfei <tianfei.zhang@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v9 12/18] net/ipn3ke: remove configuration for i40e port bonding

Andy Pei <andy.pei@intel.com> writes:

> From: Rosen Xu <rosen.xu@intel.com>
>
> The ipn3ke board FPGA and i40e BDF scan has added in ifpga_rawdev, so 
> it doesn't need to provide configuration for i40e port bonding.
>
> Signed-off-by: Rosen Xu <rosen.xu@intel.com>
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>  drivers/net/ipn3ke/Makefile             |   2 +

You'll also need to include a change to the meson file to ensure the include path will work for meson based builds.

>  drivers/net/ipn3ke/ipn3ke_ethdev.c      | 289 ++++----------------------------
>  drivers/net/ipn3ke/ipn3ke_representor.c |   8 +-
>  3 files changed, 44 insertions(+), 255 deletions(-)
>
> diff --git a/drivers/net/ipn3ke/Makefile b/drivers/net/ipn3ke/Makefile 
> index 8c3ae37..2c65e49 100644
> --- a/drivers/net/ipn3ke/Makefile
> +++ b/drivers/net/ipn3ke/Makefile
> @@ -19,6 +19,8 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API  CFLAGS += -O3  
> CFLAGS += $(WERROR_FLAGS)  CFLAGS += -I$(RTE_SDK)/drivers/bus/ifpga
> +CFLAGS += -I$(RTE_SDK)/drivers/raw/ifpga CFLAGS += 
> +-I$(RTE_SDK)/drivers/net/i40e
>  LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring  LDLIBS += 
> -lrte_ethdev -lrte_net -lrte_kvargs  LDLIBS += -lrte_bus_ifpga diff 
> --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c 
> b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> index 28d8aaf..3051cdf 100644
> --- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
> +++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
> @@ -19,6 +19,7 @@
>  #include <rte_bus_ifpga.h>
>  #include <ifpga_common.h>
>  #include <ifpga_logs.h>
> +#include <ifpga_rawdev.h>
>  
>  #include "ipn3ke_rawdev_api.h"
>  #include "ipn3ke_flow.h"
> @@ -324,7 +325,8 @@
>  				"LineSideMACType", &mac_type);
>  	hw->retimer.mac_type = (int)mac_type;
>  
> -	IPN3KE_AFU_PMD_DEBUG("UPL_version is 0x%x\n", IPN3KE_READ_REG(hw, 0));
> +	hw->acc_tm = 0;
> +	hw->acc_flow = 0;
>  
>  	if (afu_dev->id.uuid.uuid_low == IPN3KE_UUID_VBNG_LOW &&
>  		afu_dev->id.uuid.uuid_high == IPN3KE_UUID_VBNG_HIGH) { @@ -342,6 
> +344,12 @@
>  		/* After reset, wait until init done */
>  		if (ipn3ke_vbng_init_done(hw))
>  			return -1;
> +
> +		hw->acc_tm = 1;
> +		hw->acc_flow = 1;
> +
> +		IPN3KE_AFU_PMD_DEBUG("UPL_version is 0x%x\n",
> +			IPN3KE_READ_REG(hw, 0));
>  	}
>  
>  	if (hw->retimer.mac_type == IFPGA_RAWDEV_RETIMER_MAC_TYPE_10GE_XFI) 
> { @@ -409,9 +417,6 @@
>  		hw->flow_hw_enable = 1;
>  	}
>  
> -	hw->acc_tm = 0;
> -	hw->acc_flow = 0;
> -
>  	return 0;
>  }
>  
> @@ -462,7 +467,11 @@ static int ipn3ke_vswitch_probe(struct 
> rte_afu_device *afu_dev)  {
>  	char name[RTE_ETH_NAME_MAX_LEN];
>  	struct ipn3ke_hw *hw;
> -	int i, retval;
> +	struct rte_eth_dev *i40e_eth;
> +	struct ifpga_rawdev *ifpga_dev;
> +	uint16_t port_id;
> +	int i, j, retval;
> +	char *fvl_bdf;
>  
>  	/* check if the AFU device has been probed already */
>  	/* allocate shared mcp_vswitch structure */ @@ -489,7 +498,12 @@ 
> static int ipn3ke_vswitch_probe(struct rte_afu_device *afu_dev)
>  	if (retval)
>  		return retval;
>  
> +	ifpga_dev = ifpga_rawdev_get(hw->rawdev);
> +		if (!ifpga_dev)
> +			IPN3KE_AFU_PMD_ERR("failed to find ifpga_device.");
> +
>  	/* probe representor ports */
> +	j = 0;
>  	for (i = 0; i < hw->port_num; i++) {
>  		struct ipn3ke_rpst rpst = {
>  			.port_id = i,
> @@ -501,6 +515,22 @@ static int ipn3ke_vswitch_probe(struct rte_afu_device *afu_dev)
>  		snprintf(name, sizeof(name), "net_%s_representor_%d",
>  			afu_dev->device.name, i);
>  
> +		for (; j < 8; j++) {
> +			fvl_bdf = ifpga_dev->fvl_bdf[j];
> +			retval = rte_eth_dev_get_port_by_name(fvl_bdf,
> +				&port_id);
> +			if (retval) {
> +				continue;
> +			} else {
> +				i40e_eth = &rte_eth_devices[port_id];
> +				rpst.i40e_pf_eth = i40e_eth;
> +				rpst.i40e_pf_eth_port_id = port_id;
> +
> +				j++;
> +				break;
> +			}
> +		}
> +
>  		retval = rte_eth_dev_create(&afu_dev->device, name,
>  			sizeof(struct ipn3ke_rpst), NULL, NULL,
>  			ipn3ke_rpst_init, &rpst);
> @@ -508,6 +538,7 @@ static int ipn3ke_vswitch_probe(struct rte_afu_device *afu_dev)
>  		if (retval)
>  			IPN3KE_AFU_PMD_ERR("failed to create ipn3ke representor %s.",
>  								name);
> +
>  	}
>  
>  	return 0;
> @@ -553,254 +584,6 @@ static int ipn3ke_vswitch_remove(struct 
> rte_afu_device *afu_dev)
>  
>  RTE_PMD_REGISTER_AFU(net_ipn3ke_afu, afu_ipn3ke_driver);
>  
> -static const char * const valid_args[] = {
> -#define IPN3KE_AFU_NAME         "afu"
> -		IPN3KE_AFU_NAME,
> -#define IPN3KE_FPGA_ACCELERATION_LIST     "fpga_acc"
> -		IPN3KE_FPGA_ACCELERATION_LIST,
> -#define IPN3KE_I40E_PF_LIST     "i40e_pf"
> -		IPN3KE_I40E_PF_LIST,
> -		NULL
> -};
> -
> -static int
> -ipn3ke_cfg_parse_acc_list(const char *afu_name,
> -	const char *acc_list_name)
> -{
> -	struct rte_afu_device *afu_dev;
> -	struct ipn3ke_hw *hw;
> -	const char *p_source;
> -	char *p_start;
> -	char name[RTE_ETH_NAME_MAX_LEN];
> -
> -	afu_dev = rte_ifpga_find_afu_by_name(afu_name);
> -	if (!afu_dev)
> -		return -1;
> -	hw = afu_dev->shared.data;
> -	if (!hw)
> -		return -1;
> -
> -	p_source = acc_list_name;
> -	while (*p_source) {
> -		while ((*p_source == '{') || (*p_source == '|'))
> -			p_source++;
> -		p_start = name;
> -		while ((*p_source != '|') && (*p_source != '}'))
> -			*p_start++ = *p_source++;
> -		*p_start = 0;
> -		if (!strcmp(name, "tm") && hw->tm_hw_enable)
> -			hw->acc_tm = 1;
> -
> -		if (!strcmp(name, "flow") && hw->flow_hw_enable)
> -			hw->acc_flow = 1;
> -
> -		if (*p_source == '}')
> -			return 0;
> -	}
> -
> -	return 0;
> -}
> -
> -static int
> -ipn3ke_cfg_parse_i40e_pf_ethdev(const char *afu_name,
> -	const char *pf_name)
> -{
> -	struct rte_eth_dev *i40e_eth, *rpst_eth;
> -	struct rte_afu_device *afu_dev;
> -	struct ipn3ke_rpst *rpst;
> -	struct ipn3ke_hw *hw;
> -	const char *p_source;
> -	char *p_start;
> -	char name[RTE_ETH_NAME_MAX_LEN];
> -	uint16_t port_id;
> -	int i;
> -	int ret = -1;
> -
> -	afu_dev = rte_ifpga_find_afu_by_name(afu_name);
> -	if (!afu_dev)
> -		return -1;
> -	hw = afu_dev->shared.data;
> -	if (!hw)
> -		return -1;
> -
> -	p_source = pf_name;
> -	for (i = 0; i < hw->port_num; i++) {
> -		snprintf(name, sizeof(name), "net_%s_representor_%d",
> -			afu_name, i);
> -		ret = rte_eth_dev_get_port_by_name(name, &port_id);
> -		if (ret)
> -			return -1;
> -		rpst_eth = &rte_eth_devices[port_id];
> -		rpst = IPN3KE_DEV_PRIVATE_TO_RPST(rpst_eth);
> -
> -		while ((*p_source == '{') || (*p_source == '|'))
> -			p_source++;
> -		p_start = name;
> -		while ((*p_source != '|') && (*p_source != '}'))
> -			*p_start++ = *p_source++;
> -		*p_start = 0;
> -
> -		ret = rte_eth_dev_get_port_by_name(name, &port_id);
> -		if (ret)
> -			return -1;
> -		i40e_eth = &rte_eth_devices[port_id];
> -
> -		rpst->i40e_pf_eth = i40e_eth;
> -		rpst->i40e_pf_eth_port_id = port_id;
> -
> -		if ((*p_source == '}') || !(*p_source))
> -			break;
> -	}
> -
> -	return 0;
> -}
> -
> -static int
> -ipn3ke_cfg_probe(struct rte_vdev_device *dev) -{
> -	struct rte_devargs *devargs;
> -	struct rte_kvargs *kvlist = NULL;
> -	char *afu_name = NULL;
> -	char *acc_name = NULL;
> -	char *pf_name = NULL;
> -	int afu_name_en = 0;
> -	int acc_list_en = 0;
> -	int pf_list_en = 0;
> -	int ret = -1;
> -
> -	devargs = dev->device.devargs;
> -
> -	kvlist = rte_kvargs_parse(devargs->args, valid_args);
> -	if (!kvlist) {
> -		IPN3KE_AFU_PMD_ERR("error when parsing param");
> -		goto end;
> -	}
> -
> -	if (rte_kvargs_count(kvlist, IPN3KE_AFU_NAME) == 1) {
> -		if (rte_kvargs_process(kvlist, IPN3KE_AFU_NAME,
> -				       &rte_ifpga_get_string_arg,
> -				       &afu_name) < 0) {
> -			IPN3KE_AFU_PMD_ERR("error to parse %s",
> -				     IPN3KE_AFU_NAME);
> -			goto end;
> -		} else {
> -			afu_name_en = 1;
> -		}
> -	}
> -
> -	if (rte_kvargs_count(kvlist, IPN3KE_FPGA_ACCELERATION_LIST) == 1) {
> -		if (rte_kvargs_process(kvlist, IPN3KE_FPGA_ACCELERATION_LIST,
> -				       &rte_ifpga_get_string_arg,
> -				       &acc_name) < 0) {
> -			IPN3KE_AFU_PMD_ERR("error to parse %s",
> -				     IPN3KE_FPGA_ACCELERATION_LIST);
> -			goto end;
> -		} else {
> -			acc_list_en = 1;
> -		}
> -	}
> -
> -	if (rte_kvargs_count(kvlist, IPN3KE_I40E_PF_LIST) == 1) {
> -		if (rte_kvargs_process(kvlist, IPN3KE_I40E_PF_LIST,
> -				       &rte_ifpga_get_string_arg,
> -				       &pf_name) < 0) {
> -			IPN3KE_AFU_PMD_ERR("error to parse %s",
> -				     IPN3KE_I40E_PF_LIST);
> -			goto end;
> -		} else {
> -			pf_list_en = 1;
> -		}
> -	}
> -
> -	if (!afu_name_en) {
> -		IPN3KE_AFU_PMD_ERR("arg %s is mandatory for ipn3ke",
> -			  IPN3KE_AFU_NAME);
> -		goto end;
> -	}
> -
> -	if (!pf_list_en) {
> -		IPN3KE_AFU_PMD_ERR("arg %s is mandatory for ipn3ke",
> -			  IPN3KE_I40E_PF_LIST);
> -		goto end;
> -	}
> -
> -	if (acc_list_en) {
> -		ret = ipn3ke_cfg_parse_acc_list(afu_name, acc_name);
> -		if (ret) {
> -			IPN3KE_AFU_PMD_ERR("arg %s parse error for ipn3ke",
> -			  IPN3KE_FPGA_ACCELERATION_LIST);
> -			goto end;
> -		}
> -	} else {
> -		IPN3KE_AFU_PMD_INFO("arg %s is optional for ipn3ke, using i40e acc",
> -			  IPN3KE_FPGA_ACCELERATION_LIST);
> -	}
> -
> -	ret = ipn3ke_cfg_parse_i40e_pf_ethdev(afu_name, pf_name);
> -	if (ret)
> -		goto end;
> -end:
> -	if (kvlist)
> -		rte_kvargs_free(kvlist);
> -	if (afu_name)
> -		free(afu_name);
> -	if (acc_name)
> -		free(acc_name);
> -
> -	return ret;
> -}
> -
> -static int
> -ipn3ke_cfg_remove(struct rte_vdev_device *dev) -{
> -	struct rte_devargs *devargs;
> -	struct rte_kvargs *kvlist = NULL;
> -	char *afu_name = NULL;
> -	struct rte_afu_device *afu_dev;
> -	int ret = -1;
> -
> -	devargs = dev->device.devargs;
> -
> -	kvlist = rte_kvargs_parse(devargs->args, valid_args);
> -	if (!kvlist) {
> -		IPN3KE_AFU_PMD_ERR("error when parsing param");
> -		goto end;
> -	}
> -
> -	if (rte_kvargs_count(kvlist, IPN3KE_AFU_NAME) == 1) {
> -		if (rte_kvargs_process(kvlist, IPN3KE_AFU_NAME,
> -				       &rte_ifpga_get_string_arg,
> -				       &afu_name) < 0) {
> -			IPN3KE_AFU_PMD_ERR("error to parse %s",
> -				     IPN3KE_AFU_NAME);
> -		} else {
> -			afu_dev = rte_ifpga_find_afu_by_name(afu_name);
> -			if (!afu_dev)
> -				goto end;
> -			ret = ipn3ke_vswitch_remove(afu_dev);
> -		}
> -	} else {
> -		IPN3KE_AFU_PMD_ERR("Remove ipn3ke_cfg %p error", dev);
> -	}
> -
> -end:
> -	if (kvlist)
> -		rte_kvargs_free(kvlist);
> -
> -	return ret;
> -}
> -
> -static struct rte_vdev_driver ipn3ke_cfg_driver = {
> -	.probe = ipn3ke_cfg_probe,
> -	.remove = ipn3ke_cfg_remove,
> -};
> -
> -RTE_PMD_REGISTER_VDEV(ipn3ke_cfg, ipn3ke_cfg_driver); 
> -RTE_PMD_REGISTER_PARAM_STRING(ipn3ke_cfg,
> -	"afu=<string> "
> -	"fpga_acc=<string>"
> -	"i40e_pf=<string>");
> -
>  RTE_INIT(ipn3ke_afu_init_log)
>  {
>  	ipn3ke_afu_logtype = rte_log_register("pmd.afu.ipn3ke");
> diff --git a/drivers/net/ipn3ke/ipn3ke_representor.c 
> b/drivers/net/ipn3ke/ipn3ke_representor.c
> index d37f5e2..7e5d29d 100644
> --- a/drivers/net/ipn3ke/ipn3ke_representor.c
> +++ b/drivers/net/ipn3ke/ipn3ke_representor.c
> @@ -20,6 +20,7 @@
>  #include <rte_rawdev_pmd.h>
>  #include <rte_bus_ifpga.h>
>  #include <ifpga_logs.h>
> +#include <rte_pmd_i40e.h>
>  
>  #include "ipn3ke_rawdev_api.h"
>  #include "ipn3ke_flow.h"
> @@ -2918,8 +2919,11 @@ static uint16_t ipn3ke_rpst_recv_pkts(__rte_unused void *rx_q,
>  	rpst->switch_domain_id = representor_param->switch_domain_id;
>  	rpst->port_id = representor_param->port_id;
>  	rpst->hw = representor_param->hw;
> -	rpst->i40e_pf_eth = NULL;
> -	rpst->i40e_pf_eth_port_id = 0xFFFF;
> +	rpst->i40e_pf_eth = representor_param->i40e_pf_eth;
> +	rpst->i40e_pf_eth_port_id = representor_param->i40e_pf_eth_port_id;
> +	if (rpst->i40e_pf_eth)
> +		rte_pmd_i40e_set_switch_dev(rpst->i40e_pf_eth_port_id,
> +					    rpst->ethdev);
>  
>  	ethdev->data->mac_addrs = rte_zmalloc("ipn3ke", RTE_ETHER_ADDR_LEN, 0);
>  	if (!ethdev->data->mac_addrs) {