[dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the
specific FPGA that i40e PF is connected to.
i40e PF get link status update via the connected
FPGA.

Fixes: c60869e2b742 ("net/i40e: fix link status update")
Cc: roy.fan.zhang@intel.com
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 128 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 122 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index cab440f..9873ea0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -39,11 +39,12 @@
 #include "i40e_regs.h"
 #include "rte_pmd_i40e.h"
 
-#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
-#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
-#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
-#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
-#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
+#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
+#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
+#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
+#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -2784,6 +2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	*(char **)extra_args = strdup(value);
+
+	if (!*(char **)extra_args)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void
+i40e_pf_switch_mode_link_update(const char *cfg_str,
+	struct rte_eth_dev **switch_ethdev)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
+	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret = -1;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else
+				*p_dst++ = *p_src++;
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			*switch_ethdev = NULL;
+		else
+			*switch_ethdev = &rte_eth_devices[port_id];
+	} else
+		*switch_ethdev = NULL;
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
@@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
 	int ret;
+	struct rte_devargs *devargs;
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_eth_dev *switch_ethdev;
+	char *switch_cfg_str = NULL;
 
 	memset(&link, 0, sizeof(link));
 
@@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	devargs = pci_dev->device.devargs;
+	if (devargs) {
+		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+		if (kvlist != NULL) {
+			if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG)
+				== 1) {
+				if (!rte_kvargs_process(kvlist,
+					ETH_I40E_SWITCH_MODE_ARG,
+					&i40e_pf_parse_switch_mode,
+					&switch_cfg_str)) {
+
+					i40e_pf_switch_mode_link_update(
+						switch_cfg_str,
+						&switch_ethdev);
+
+					if (switch_ethdev) {
+						rte_eth_linkstatus_get(
+							switch_ethdev,
+							&link);
+					} else {
+						link.link_duplex =
+							ETH_LINK_FULL_DUPLEX;
+						link.link_autoneg =
+							ETH_LINK_SPEED_FIXED;
+						link.link_speed =
+							ETH_SPEED_NUM_25G;
+						link.link_status = 0;
+					}
+				}
+			}
+			rte_kvargs_free(kvlist);
+		}
+	}
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Xu, Rosen]

Hi,

> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, May 23, 2019 17:15
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>
> Subject: [PATCH] net/i40e: i40e rework for ipn3ke

> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
> PF is connected to.
> i40e PF get link status update via the connected FPGA.
> 
> Fixes: c60869e2b742 ("net/i40e: fix link status update")
> Cc: roy.fan.zhang@intel.com
> Cc: qi.z.zhang@intel.com
> Cc: jingjing.wu@intel.com
> Cc: beilei.xing@intel.com
> Cc: ferruh.yigit@intel.com
> Cc: rosen.xu@intel.com

My understanding cc people should add in git send-email not in patch.

> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 128
> +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 122 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index cab440f..9873ea0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -39,11 +39,12 @@
>  #include "i40e_regs.h"
>  #include "rte_pmd_i40e.h"
> 
> -#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
> -#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
> -#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> -#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
> -#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
> +#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
> +#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
> +#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
> +#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
> +#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"
> +#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> 
>  #define I40E_CLEAR_PXE_WAIT_MS     200
> 
> @@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf
> *pf,
>  	ETH_I40E_SUPPORT_MULTI_DRIVER,
>  	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
>  	ETH_I40E_USE_LATEST_VEC,
> +	ETH_I40E_SWITCH_MODE_ARG,
>  	NULL};
> 
>  static const struct rte_pci_id pci_id_i40e_map[] = { @@ -2784,6 +2786,80
> @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
>  	}
>  }
> 
> +static int
> +i40e_pf_parse_switch_mode(const char *key __rte_unused,
> +	const char *value, void *extra_args)
> +{
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +	*(char **)extra_args = strdup(value);
> +
> +	if (!*(char **)extra_args)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void
> +i40e_pf_switch_mode_link_update(const char *cfg_str,
> +	struct rte_eth_dev **switch_ethdev)
> +{
> +	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	uint16_t port_id;
> +	const char *p_src;
> +	char *p_dst;
> +	int ret = -1;
> +
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;
> +
> +		/* move over the first underline */
> +		p_src++;
> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';
> +				p_src++;
> +			} else
> +				*p_dst++ = *p_src++;
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;
> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> +
> +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> +			"net_%s_representor_%s", switch_name,
> port_name);
> +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> +			switch_ethdev_name);
> +
> +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> +			&port_id);
> +		if (ret)
> +			*switch_ethdev = NULL;
> +		else
> +			*switch_ethdev = &rte_eth_devices[port_id];
> +	} else
> +		*switch_ethdev = NULL;
> +}
> +
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
> @@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
>  	int ret;
> +	struct rte_devargs *devargs;
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +	struct rte_eth_dev *switch_ethdev;
> +	char *switch_cfg_str = NULL;
> 
>  	memset(&link, 0, sizeof(link));
> 
> @@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	devargs = pci_dev->device.devargs;
> +	if (devargs) {
> +		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +		if (kvlist != NULL) {
> +			if (rte_kvargs_count(kvlist,
> ETH_I40E_SWITCH_MODE_ARG)
> +				== 1) {
> +				if (!rte_kvargs_process(kvlist,
> +					ETH_I40E_SWITCH_MODE_ARG,
> +					&i40e_pf_parse_switch_mode,
> +					&switch_cfg_str)) {
> +
> +					i40e_pf_switch_mode_link_update(
> +						switch_cfg_str,
> +						&switch_ethdev);
> +
> +					if (switch_ethdev) {
> +						rte_eth_linkstatus_get(
> +							switch_ethdev,
> +							&link);
> +					} else {
> +						link.link_duplex =
> +
> 	ETH_LINK_FULL_DUPLEX;
> +						link.link_autoneg =
> +
> 	ETH_LINK_SPEED_FIXED;
> +						link.link_speed =
> +
> 	ETH_SPEED_NUM_25G;
> +						link.link_status = 0;
> +					}
> +				}
> +			}
> +			rte_kvargs_free(kvlist);
> +		}
> +	}
> +
>  	ret = rte_eth_linkstatus_set(dev, &link);
>  	i40e_notify_all_vfs_link_status(dev);
> 
> @@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
>  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
>  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG
> "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Ferruh Yigit]

On 5/24/2019 2:05 AM, Xu, Rosen wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Pei, Andy
>> Sent: Thursday, May 23, 2019 17:15
>> To: dev@dpdk.org
>> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan
>> <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
>> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
>> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>
>> Subject: [PATCH] net/i40e: i40e rework for ipn3ke
> 
>> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
>> PF is connected to.
>> i40e PF get link status update via the connected FPGA.
>>
>> Fixes: c60869e2b742 ("net/i40e: fix link status update")
>> Cc: roy.fan.zhang@intel.com
>> Cc: qi.z.zhang@intel.com
>> Cc: jingjing.wu@intel.com
>> Cc: beilei.xing@intel.com
>> Cc: ferruh.yigit@intel.com
>> Cc: rosen.xu@intel.com
> 
> My understanding cc people should add in git send-email not in patch.

"git send-email" is picking the names from commit log, so putting the names in
the commit log is another way, but as you said at the end of the day we don't
want names be in the git history.
But what can be done is, putting the names after "---" works for both, those
names still will be picked by "git send-email" and cc'ed automatically, and this
information is part of the patch meanwhile they will be stripped automatically
when merged, so they won't be in the git history without causing manual cleanup
task to maintainers.

> 
>> Signed-off-by: Andy Pei <andy.pei@intel.com>
>> ---

<----- HERE

>>  drivers/net/i40e/i40e_ethdev.c | 128
>> +++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 122 insertions(+), 6 deletions(-)
>>

<...>

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Pei, Andy]

Hi, Ferruh

Thanks a lot. Your tips is really helpful, and I will follow this in the future.

-----Original Message-----
From: Yigit, Ferruh 
Sent: Friday, May 24, 2019 9:12 PM
To: Xu, Rosen <rosen.xu@intel.com>; Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
Subject: Re: [PATCH] net/i40e: i40e rework for ipn3ke

On 5/24/2019 2:05 AM, Xu, Rosen wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Pei, Andy
>> Sent: Thursday, May 23, 2019 17:15
>> To: dev@dpdk.org
>> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan 
>> <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, 
>> Jingjing <jingjing.wu@intel.com>; Xing, Beilei 
>> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, 
>> Rosen <rosen.xu@intel.com>
>> Subject: [PATCH] net/i40e: i40e rework for ipn3ke
> 
>> Add switch_mode argument for i40e PF to specify the specific FPGA 
>> that i40e PF is connected to.
>> i40e PF get link status update via the connected FPGA.
>>
>> Fixes: c60869e2b742 ("net/i40e: fix link status update")
>> Cc: roy.fan.zhang@intel.com
>> Cc: qi.z.zhang@intel.com
>> Cc: jingjing.wu@intel.com
>> Cc: beilei.xing@intel.com
>> Cc: ferruh.yigit@intel.com
>> Cc: rosen.xu@intel.com
> 
> My understanding cc people should add in git send-email not in patch.

"git send-email" is picking the names from commit log, so putting the names in the commit log is another way, but as you said at the end of the day we don't want names be in the git history.
But what can be done is, putting the names after "---" works for both, those names still will be picked by "git send-email" and cc'ed automatically, and this information is part of the patch meanwhile they will be stripped automatically when merged, so they won't be in the git history without causing manual cleanup task to maintainers.

> 
>> Signed-off-by: Andy Pei <andy.pei@intel.com>
>> ---

<----- HERE

>>  drivers/net/i40e/i40e_ethdev.c | 128
>> +++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 122 insertions(+), 6 deletions(-)
>>

<...>

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Pei, Andy]

Hi, Rosen


Your ACK is needed if there is no other issue except the CC list


-----Original Message-----
From: Xu, Rosen 
Sent: Friday, May 24, 2019 9:05 AM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke

Hi,

> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, May 23, 2019 17:15
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan 
> <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, 
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei 
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, 
> Rosen <rosen.xu@intel.com>
> Subject: [PATCH] net/i40e: i40e rework for ipn3ke

> Add switch_mode argument for i40e PF to specify the specific FPGA that 
> i40e PF is connected to.
> i40e PF get link status update via the connected FPGA.
> 
> Fixes: c60869e2b742 ("net/i40e: fix link status update")
> Cc: roy.fan.zhang@intel.com
> Cc: qi.z.zhang@intel.com
> Cc: jingjing.wu@intel.com
> Cc: beilei.xing@intel.com
> Cc: ferruh.yigit@intel.com
> Cc: rosen.xu@intel.com

My understanding cc people should add in git send-email not in patch.

> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 128
> +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 122 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c 
> b/drivers/net/i40e/i40e_ethdev.c index cab440f..9873ea0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -39,11 +39,12 @@
>  #include "i40e_regs.h"
>  #include "rte_pmd_i40e.h"
> 
> -#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
> -#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
> -#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> -#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
> -#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
> +#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
> +#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
> +#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
> +#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
> +#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"
> +#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> 
>  #define I40E_CLEAR_PXE_WAIT_MS     200
> 
> @@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct 
> i40e_pf *pf,
>  	ETH_I40E_SUPPORT_MULTI_DRIVER,
>  	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
>  	ETH_I40E_USE_LATEST_VEC,
> +	ETH_I40E_SWITCH_MODE_ARG,
>  	NULL};
> 
>  static const struct rte_pci_id pci_id_i40e_map[] = { @@ -2784,6 
> +2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
>  	}
>  }
> 
> +static int
> +i40e_pf_parse_switch_mode(const char *key __rte_unused,
> +	const char *value, void *extra_args) {
> +	if (!value || !extra_args)
> +		return -EINVAL;
> +
> +	*(char **)extra_args = strdup(value);
> +
> +	if (!*(char **)extra_args)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void
> +i40e_pf_switch_mode_link_update(const char *cfg_str,
> +	struct rte_eth_dev **switch_ethdev)
> +{
> +	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};
> +	uint16_t port_id;
> +	const char *p_src;
> +	char *p_dst;
> +	int ret = -1;
> +
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;
> +
> +		/* move over the first underline */
> +		p_src++;
> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';
> +				p_src++;
> +			} else
> +				*p_dst++ = *p_src++;
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;
> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> +
> +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> +			"net_%s_representor_%s", switch_name,
> port_name);
> +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> +			switch_ethdev_name);
> +
> +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> +			&port_id);
> +		if (ret)
> +			*switch_ethdev = NULL;
> +		else
> +			*switch_ethdev = &rte_eth_devices[port_id];
> +	} else
> +		*switch_ethdev = NULL;
> +}
> +
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
> @@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
>  	int ret;
> +	struct rte_devargs *devargs;
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +	struct rte_eth_dev *switch_ethdev;
> +	char *switch_cfg_str = NULL;
> 
>  	memset(&link, 0, sizeof(link));
> 
> @@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	devargs = pci_dev->device.devargs;
> +	if (devargs) {
> +		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +		if (kvlist != NULL) {
> +			if (rte_kvargs_count(kvlist,
> ETH_I40E_SWITCH_MODE_ARG)
> +				== 1) {
> +				if (!rte_kvargs_process(kvlist,
> +					ETH_I40E_SWITCH_MODE_ARG,
> +					&i40e_pf_parse_switch_mode,
> +					&switch_cfg_str)) {
> +
> +					i40e_pf_switch_mode_link_update(
> +						switch_cfg_str,
> +						&switch_ethdev);
> +
> +					if (switch_ethdev) {
> +						rte_eth_linkstatus_get(
> +							switch_ethdev,
> +							&link);
> +					} else {
> +						link.link_duplex =
> +
> 	ETH_LINK_FULL_DUPLEX;
> +						link.link_autoneg =
> +
> 	ETH_LINK_SPEED_FIXED;
> +						link.link_speed =
> +
> 	ETH_SPEED_NUM_25G;
> +						link.link_status = 0;
> +					}
> +				}
> +			}
> +			rte_kvargs_free(kvlist);
> +		}
> +	}
> +
>  	ret = rte_eth_linkstatus_set(dev, &link);
>  	i40e_notify_all_vfs_link_status(dev);
> 
> @@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
>  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
>  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Xu, Rosen]

Hi Andy,

> -----Original Message-----
> From: Pei, Andy
> Sent: Monday, June 10, 2019 14:15
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> Hi, Rosen
> 
> 
> Your ACK is needed if there is no other issue except the CC list
> 
> 
> -----Original Message-----
> From: Xu, Rosen
> Sent: Friday, May 24, 2019 9:05 AM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> Hi,
> 
> > -----Original Message-----
> > From: Pei, Andy
> > Sent: Thursday, May 23, 2019 17:15
> > To: dev@dpdk.org
> > Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> > Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> > <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu,
> > Rosen <rosen.xu@intel.com>
> > Subject: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> > Add switch_mode argument for i40e PF to specify the specific FPGA that
> > i40e PF is connected to.
> > i40e PF get link status update via the connected FPGA.
> >
> > Fixes: c60869e2b742 ("net/i40e: fix link status update")
> > Cc: roy.fan.zhang@intel.com
> > Cc: qi.z.zhang@intel.com
> > Cc: jingjing.wu@intel.com
> > Cc: beilei.xing@intel.com
> > Cc: ferruh.yigit@intel.com
> > Cc: rosen.xu@intel.com
> 
> My understanding cc people should add in git send-email not in patch.
> 
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 128
> > +++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 122 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c
> > b/drivers/net/i40e/i40e_ethdev.c index cab440f..9873ea0 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -39,11 +39,12 @@
> >  #include "i40e_regs.h"
> >  #include "rte_pmd_i40e.h"
> >
> > -#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
> > -#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
> > -#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> > -#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
> > -#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
> > +#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
> > +#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
> > +#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
> > +#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
> > +#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"
> > +#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> >
> >  #define I40E_CLEAR_PXE_WAIT_MS     200
> >
> > @@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct
> > i40e_pf *pf,
> >  	ETH_I40E_SUPPORT_MULTI_DRIVER,
> >  	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> >  	ETH_I40E_USE_LATEST_VEC,
> > +	ETH_I40E_SWITCH_MODE_ARG,
> >  	NULL};
> >
> >  static const struct rte_pci_id pci_id_i40e_map[] = { @@ -2784,6
> > +2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> >  	}
> >  }
> >
> > +static int
> > +i40e_pf_parse_switch_mode(const char *key __rte_unused,
> > +	const char *value, void *extra_args) {
> > +	if (!value || !extra_args)
> > +		return -EINVAL;
> > +
> > +	*(char **)extra_args = strdup(value);
> > +
> > +	if (!*(char **)extra_args)
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +static void
> > +i40e_pf_switch_mode_link_update(const char *cfg_str,
> > +	struct rte_eth_dev **switch_ethdev)
> > +{
> > +	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	uint16_t port_id;
> > +	const char *p_src;
> > +	char *p_dst;
> > +	int ret = -1;
> > +
> > +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> > +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> > +		p_src = cfg_str;
> > +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> > +
> > +		/* move over "IPN3KE" */
> > +		while ((*p_src != '_') && (*p_src))
> > +			p_src++;
> > +
> > +		/* move over the first underline */
> > +		p_src++;
> > +
> > +		p_dst = switch_name;
> > +		while ((*p_src != '_') && (*p_src)) {
> > +			if (*p_src == '@') {
> > +				*p_dst++ = '|';
> > +				p_src++;
> > +			} else
> > +				*p_dst++ = *p_src++;
> > +		}
> > +		*p_dst = 0;
> > +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> > +
> > +		/* move over the second underline */
> > +		p_src++;
> > +
> > +		p_dst = port_name;
> > +		while (*p_src)
> > +			*p_dst++ = *p_src++;
> > +		*p_dst = 0;
> > +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> > +
> > +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> > +			"net_%s_representor_%s", switch_name,
> > port_name);
> > +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> > +			switch_ethdev_name);
> > +
> > +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> > +			&port_id);
> > +		if (ret)
> > +			*switch_ethdev = NULL;
> > +		else
> > +			*switch_ethdev = &rte_eth_devices[port_id];
> > +	} else
> > +		*switch_ethdev = NULL;
> > +}
> > +
> >  int
> >  i40e_dev_link_update(struct rte_eth_dev *dev,
> >  		     int wait_to_complete)
> > @@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> >  	struct rte_eth_link link;
> >  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> >  	int ret;
> > +	struct rte_devargs *devargs;
> > +	struct rte_kvargs *kvlist = NULL;
> > +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > +	struct rte_eth_dev *switch_ethdev;
> > +	char *switch_cfg_str = NULL;
> >
> >  	memset(&link, 0, sizeof(link));
> >
> > @@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> >  	else
> >  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> >
> > +	devargs = pci_dev->device.devargs;
> > +	if (devargs) {
> > +		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> > +		if (kvlist != NULL) {
> > +			if (rte_kvargs_count(kvlist,
> > ETH_I40E_SWITCH_MODE_ARG)
> > +				== 1) {
> > +				if (!rte_kvargs_process(kvlist,
> > +					ETH_I40E_SWITCH_MODE_ARG,
> > +					&i40e_pf_parse_switch_mode,
> > +					&switch_cfg_str)) {
> > +
> > +					i40e_pf_switch_mode_link_update(
> > +						switch_cfg_str,
> > +						&switch_ethdev);
> > +
> > +					if (switch_ethdev) {
> > +						rte_eth_linkstatus_get(
> > +							switch_ethdev,
> > +							&link);
> > +					} else {
> > +						link.link_duplex =
> > +
> > 	ETH_LINK_FULL_DUPLEX;
> > +						link.link_autoneg =
> > +
> > 	ETH_LINK_SPEED_FIXED;
> > +						link.link_speed =
> > +
> > 	ETH_SPEED_NUM_25G;
> > +						link.link_status = 0;
> > +					}
> > +				}
> > +			}
> > +			rte_kvargs_free(kvlist);
> > +		}
> > +	}
> > +
> >  	ret = rte_eth_linkstatus_set(dev, &link);
> >  	i40e_notify_all_vfs_link_status(dev);
> >
> > @@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
> >  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> >  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG
> "=1|2|4|8|16"
> >  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> > -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> > +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> > +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> > --
> > 1.8.3.1

Acked-by: Rosen Xu <rosen.xu@intel.com>

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Pei, Andy]

+ Ye Xiaolong

-----Original Message-----
From: Xu, Rosen 
Sent: Tuesday, June 11, 2019 10:35 AM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke

Hi Andy,

> -----Original Message-----
> From: Pei, Andy
> Sent: Monday, June 10, 2019 14:15
> To: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> Hi, Rosen
> 
> 
> Your ACK is needed if there is no other issue except the CC list
> 
> 
> -----Original Message-----
> From: Xu, Rosen
> Sent: Friday, May 24, 2019 9:05 AM
> To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
> Cc: Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: RE: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> Hi,
> 
> > -----Original Message-----
> > From: Pei, Andy
> > Sent: Thursday, May 23, 2019 17:15
> > To: dev@dpdk.org
> > Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Roy Fan 
> > <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, 
> > Jingjing <jingjing.wu@intel.com>; Xing, Beilei 
> > <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, 
> > Rosen <rosen.xu@intel.com>
> > Subject: [PATCH] net/i40e: i40e rework for ipn3ke
> 
> > Add switch_mode argument for i40e PF to specify the specific FPGA 
> > that i40e PF is connected to.
> > i40e PF get link status update via the connected FPGA.
> >
> > Fixes: c60869e2b742 ("net/i40e: fix link status update")
> > Cc: roy.fan.zhang@intel.com
> > Cc: qi.z.zhang@intel.com
> > Cc: jingjing.wu@intel.com
> > Cc: beilei.xing@intel.com
> > Cc: ferruh.yigit@intel.com
> > Cc: rosen.xu@intel.com
> 
> My understanding cc people should add in git send-email not in patch.
> 
> > Signed-off-by: Andy Pei <andy.pei@intel.com>
> > ---
> >  drivers/net/i40e/i40e_ethdev.c | 128
> > +++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 122 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_ethdev.c 
> > b/drivers/net/i40e/i40e_ethdev.c index cab440f..9873ea0 100644
> > --- a/drivers/net/i40e/i40e_ethdev.c
> > +++ b/drivers/net/i40e/i40e_ethdev.c
> > @@ -39,11 +39,12 @@
> >  #include "i40e_regs.h"
> >  #include "rte_pmd_i40e.h"
> >
> > -#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
> > -#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
> > -#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
> > -#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
> > -#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
> > +#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
> > +#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
> > +#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
> > +#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
> > +#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"
> > +#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> >
> >  #define I40E_CLEAR_PXE_WAIT_MS     200
> >
> > @@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct
> > i40e_pf *pf,
> >  	ETH_I40E_SUPPORT_MULTI_DRIVER,
> >  	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> >  	ETH_I40E_USE_LATEST_VEC,
> > +	ETH_I40E_SWITCH_MODE_ARG,
> >  	NULL};
> >
> >  static const struct rte_pci_id pci_id_i40e_map[] = { @@ -2784,6
> > +2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw 
> > +*hw)
> >  	}
> >  }
> >
> > +static int
> > +i40e_pf_parse_switch_mode(const char *key __rte_unused,
> > +	const char *value, void *extra_args) {
> > +	if (!value || !extra_args)
> > +		return -EINVAL;
> > +
> > +	*(char **)extra_args = strdup(value);
> > +
> > +	if (!*(char **)extra_args)
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +static void
> > +i40e_pf_switch_mode_link_update(const char *cfg_str,
> > +	struct rte_eth_dev **switch_ethdev) {
> > +	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};
> > +	uint16_t port_id;
> > +	const char *p_src;
> > +	char *p_dst;
> > +	int ret = -1;
> > +
> > +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> > +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> > +		p_src = cfg_str;
> > +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> > +
> > +		/* move over "IPN3KE" */
> > +		while ((*p_src != '_') && (*p_src))
> > +			p_src++;
> > +
> > +		/* move over the first underline */
> > +		p_src++;
> > +
> > +		p_dst = switch_name;
> > +		while ((*p_src != '_') && (*p_src)) {
> > +			if (*p_src == '@') {
> > +				*p_dst++ = '|';
> > +				p_src++;
> > +			} else
> > +				*p_dst++ = *p_src++;
> > +		}
> > +		*p_dst = 0;
> > +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> > +
> > +		/* move over the second underline */
> > +		p_src++;
> > +
> > +		p_dst = port_name;
> > +		while (*p_src)
> > +			*p_dst++ = *p_src++;
> > +		*p_dst = 0;
> > +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> > +
> > +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> > +			"net_%s_representor_%s", switch_name,
> > port_name);
> > +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> > +			switch_ethdev_name);
> > +
> > +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> > +			&port_id);
> > +		if (ret)
> > +			*switch_ethdev = NULL;
> > +		else
> > +			*switch_ethdev = &rte_eth_devices[port_id];
> > +	} else
> > +		*switch_ethdev = NULL;
> > +}
> > +
> >  int
> >  i40e_dev_link_update(struct rte_eth_dev *dev,
> >  		     int wait_to_complete)
> > @@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> >  	struct rte_eth_link link;
> >  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> >  	int ret;
> > +	struct rte_devargs *devargs;
> > +	struct rte_kvargs *kvlist = NULL;
> > +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> > +	struct rte_eth_dev *switch_ethdev;
> > +	char *switch_cfg_str = NULL;
> >
> >  	memset(&link, 0, sizeof(link));
> >
> > @@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct
> > i40e_hw *hw)
> >  	else
> >  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> >
> > +	devargs = pci_dev->device.devargs;
> > +	if (devargs) {
> > +		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> > +		if (kvlist != NULL) {
> > +			if (rte_kvargs_count(kvlist,
> > ETH_I40E_SWITCH_MODE_ARG)
> > +				== 1) {
> > +				if (!rte_kvargs_process(kvlist,
> > +					ETH_I40E_SWITCH_MODE_ARG,
> > +					&i40e_pf_parse_switch_mode,
> > +					&switch_cfg_str)) {
> > +
> > +					i40e_pf_switch_mode_link_update(
> > +						switch_cfg_str,
> > +						&switch_ethdev);
> > +
> > +					if (switch_ethdev) {
> > +						rte_eth_linkstatus_get(
> > +							switch_ethdev,
> > +							&link);
> > +					} else {
> > +						link.link_duplex =
> > +
> > 	ETH_LINK_FULL_DUPLEX;
> > +						link.link_autoneg =
> > +
> > 	ETH_LINK_SPEED_FIXED;
> > +						link.link_speed =
> > +
> > 	ETH_SPEED_NUM_25G;
> > +						link.link_status = 0;
> > +					}
> > +				}
> > +			}
> > +			rte_kvargs_free(kvlist);
> > +		}
> > +	}
> > +
> >  	ret = rte_eth_linkstatus_set(dev, &link);
> >  	i40e_notify_all_vfs_link_status(dev);
> >
> > @@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
> >  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> >  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG
> "=1|2|4|8|16"
> >  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> > -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> > +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> > +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> > --
> > 1.8.3.1

Acked-by: Rosen Xu <rosen.xu@intel.com>

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Ye Xiaolong]

Hi, Andy

Better to use a more specific subject for this patch:

	net/i40e: get link status update for ipn3ke

or something similar.

On 05/23, Andy Pei wrote:
>Add switch_mode argument for i40e PF to specify the
>specific FPGA that i40e PF is connected to.
>i40e PF get link status update via the connected
>FPGA.
>
>Fixes: c60869e2b742 ("net/i40e: fix link status update")

For me, this patch is rather a new feature add than a fix.

>Cc: roy.fan.zhang@intel.com
>Cc: qi.z.zhang@intel.com
>Cc: jingjing.wu@intel.com
>Cc: beilei.xing@intel.com
>Cc: ferruh.yigit@intel.com
>Cc: rosen.xu@intel.com


As Ferruh suggested, currently we don't need this cc info in git log history,
you can move the cc block after `---` mark, while all those names will still be
picked-up and cc'ed by git send-email.

>
>Signed-off-by: Andy Pei <andy.pei@intel.com>
>---
> drivers/net/i40e/i40e_ethdev.c | 128 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 122 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
>index cab440f..9873ea0 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -39,11 +39,12 @@
> #include "i40e_regs.h"
> #include "rte_pmd_i40e.h"
> 
>-#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
>-#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
>-#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
>-#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
>-#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
>+#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
>+#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
>+#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
>+#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
>+#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"

Above changes are not relevant.

>+#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> 
> #define I40E_CLEAR_PXE_WAIT_MS     200
> 
>@@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
> 	ETH_I40E_SUPPORT_MULTI_DRIVER,
> 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> 	ETH_I40E_USE_LATEST_VEC,
>+	ETH_I40E_SWITCH_MODE_ARG,
> 	NULL};
> 
> static const struct rte_pci_id pci_id_i40e_map[] = {
>@@ -2784,6 +2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	}
> }
> 
>+static int
>+i40e_pf_parse_switch_mode(const char *key __rte_unused,
>+	const char *value, void *extra_args)
>+{
>+	if (!value || !extra_args)
>+		return -EINVAL;
>+
>+	*(char **)extra_args = strdup(value);
>+

When will you free the memory alloced by strdup.

>+	if (!*(char **)extra_args)
>+		return -ENOMEM;
>+
>+	return 0;
>+}
>+
>+static void
>+i40e_pf_switch_mode_link_update(const char *cfg_str,
>+	struct rte_eth_dev **switch_ethdev)
>+{
>+	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
>+	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
>+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};

Above initializations are not needed.

>+	uint16_t port_id;
>+	const char *p_src;
>+	char *p_dst;
>+	int ret = -1;

This initialization is not needed.

>+
>+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
>+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
>+		p_src = cfg_str;
>+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
>+
>+		/* move over "IPN3KE" */
>+		while ((*p_src != '_') && (*p_src))
>+			p_src++;
>+
>+		/* move over the first underline */
>+		p_src++;
>+
>+		p_dst = switch_name;
>+		while ((*p_src != '_') && (*p_src)) {
>+			if (*p_src == '@') {
>+				*p_dst++ = '|';
>+				p_src++;
>+			} else
>+				*p_dst++ = *p_src++;
>+		}
>+		*p_dst = 0;
>+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
>+
>+		/* move over the second underline */
>+		p_src++;
>+
>+		p_dst = port_name;
>+		while (*p_src)
>+			*p_dst++ = *p_src++;
>+		*p_dst = 0;
>+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
>+
>+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
>+			"net_%s_representor_%s", switch_name, port_name);
>+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
>+			switch_ethdev_name);
>+
>+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
>+			&port_id);
>+		if (ret)
>+			*switch_ethdev = NULL;
>+		else
>+			*switch_ethdev = &rte_eth_devices[port_id];
>+	} else
>+		*switch_ethdev = NULL;

After read the body of this function, it's about getting a switch_ethdev by a
name other than updating the link status, so the func name is confusing, better
to use a more precise name, and return value can be struct *rte_eth_dev, then
you wouldn't have to pass struct rte_eth_dev **.

>+}
>+
> int
> i40e_dev_link_update(struct rte_eth_dev *dev,
> 		     int wait_to_complete)
>@@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	struct rte_eth_link link;
> 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> 	int ret;
>+	struct rte_devargs *devargs;
>+	struct rte_kvargs *kvlist = NULL;
>+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>+	struct rte_eth_dev *switch_ethdev;
>+	char *switch_cfg_str = NULL;
> 
> 	memset(&link, 0, sizeof(link));
> 
>@@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	else
> 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
>+	devargs = pci_dev->device.devargs;
>+	if (devargs) {
>+		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
>+		if (kvlist != NULL) {
>+			if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG)
>+				== 1) {
>+				if (!rte_kvargs_process(kvlist,
>+					ETH_I40E_SWITCH_MODE_ARG,
>+					&i40e_pf_parse_switch_mode,
>+					&switch_cfg_str)) {
>+
>+					i40e_pf_switch_mode_link_update(
>+						switch_cfg_str,
>+						&switch_ethdev);
>+
>+					if (switch_ethdev) {
>+						rte_eth_linkstatus_get(
>+							switch_ethdev,
>+							&link);
>+					} else {
>+						link.link_duplex =
>+							ETH_LINK_FULL_DUPLEX;
>+						link.link_autoneg =
>+							ETH_LINK_SPEED_FIXED;
>+						link.link_speed =
>+							ETH_SPEED_NUM_25G;
>+						link.link_status = 0;
>+					}
>+				}
>+			}
>+			rte_kvargs_free(kvlist);
>+		}
>+	}
>+

Better to wrap above code to a function to avoid too many levels of block nesting.

Thanks,
Xiaolong
> 	ret = rte_eth_linkstatus_set(dev, &link);
> 	i40e_notify_all_vfs_link_status(dev);
> 
>@@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
> 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
> 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
>-			      ETH_I40E_USE_LATEST_VEC "=0|1");
>+			      ETH_I40E_USE_LATEST_VEC "=0|1"
>+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
>-- 
>1.8.3.1
>

Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

[Pei, Andy]

Hi, Xiaolong

OK, I will do this in V2.

Thanks.

-----Original Message-----
From: Ye, Xiaolong 
Sent: Wednesday, June 26, 2019 11:33 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dev@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>
Subject: Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke

Hi, Andy

Better to use a more specific subject for this patch:

	net/i40e: get link status update for ipn3ke

or something similar.

On 05/23, Andy Pei wrote:
>Add switch_mode argument for i40e PF to specify the specific FPGA that 
>i40e PF is connected to.
>i40e PF get link status update via the connected FPGA.
>
>Fixes: c60869e2b742 ("net/i40e: fix link status update")

For me, this patch is rather a new feature add than a fix.

>Cc: roy.fan.zhang@intel.com
>Cc: qi.z.zhang@intel.com
>Cc: jingjing.wu@intel.com
>Cc: beilei.xing@intel.com
>Cc: ferruh.yigit@intel.com
>Cc: rosen.xu@intel.com


As Ferruh suggested, currently we don't need this cc info in git log history, you can move the cc block after `---` mark, while all those names will still be picked-up and cc'ed by git send-email.

>
>Signed-off-by: Andy Pei <andy.pei@intel.com>
>---
> drivers/net/i40e/i40e_ethdev.c | 128 
>+++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 122 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/i40e/i40e_ethdev.c 
>b/drivers/net/i40e/i40e_ethdev.c index cab440f..9873ea0 100644
>--- a/drivers/net/i40e/i40e_ethdev.c
>+++ b/drivers/net/i40e/i40e_ethdev.c
>@@ -39,11 +39,12 @@
> #include "i40e_regs.h"
> #include "rte_pmd_i40e.h"
> 
>-#define ETH_I40E_FLOATING_VEB_ARG	"enable_floating_veb"
>-#define ETH_I40E_FLOATING_VEB_LIST_ARG	"floating_veb_list"
>-#define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
>-#define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
>-#define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
>+#define ETH_I40E_FLOATING_VEB_ARG         "enable_floating_veb"
>+#define ETH_I40E_FLOATING_VEB_LIST_ARG    "floating_veb_list"
>+#define ETH_I40E_SUPPORT_MULTI_DRIVER     "support-multi-driver"
>+#define ETH_I40E_QUEUE_NUM_PER_VF_ARG     "queue-num-per-vf"
>+#define ETH_I40E_USE_LATEST_VEC           "use-latest-supported-vec"

Above changes are not relevant.

>+#define ETH_I40E_SWITCH_MODE_ARG          "switch_mode"
> 
> #define I40E_CLEAR_PXE_WAIT_MS     200
> 
>@@ -410,6 +411,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
> 	ETH_I40E_SUPPORT_MULTI_DRIVER,
> 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
> 	ETH_I40E_USE_LATEST_VEC,
>+	ETH_I40E_SWITCH_MODE_ARG,
> 	NULL};
> 
> static const struct rte_pci_id pci_id_i40e_map[] = { @@ -2784,6 
>+2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	}
> }
> 
>+static int
>+i40e_pf_parse_switch_mode(const char *key __rte_unused,
>+	const char *value, void *extra_args)
>+{
>+	if (!value || !extra_args)
>+		return -EINVAL;
>+
>+	*(char **)extra_args = strdup(value);
>+

When will you free the memory alloced by strdup.

>+	if (!*(char **)extra_args)
>+		return -ENOMEM;
>+
>+	return 0;
>+}
>+
>+static void
>+i40e_pf_switch_mode_link_update(const char *cfg_str,
>+	struct rte_eth_dev **switch_ethdev)
>+{
>+	char switch_name[RTE_ETH_NAME_MAX_LEN] = {0};
>+	char port_name[RTE_ETH_NAME_MAX_LEN] = {0};
>+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] = {0};

Above initializations are not needed.

>+	uint16_t port_id;
>+	const char *p_src;
>+	char *p_dst;
>+	int ret = -1;

This initialization is not needed.

>+
>+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
>+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
>+		p_src = cfg_str;
>+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
>+
>+		/* move over "IPN3KE" */
>+		while ((*p_src != '_') && (*p_src))
>+			p_src++;
>+
>+		/* move over the first underline */
>+		p_src++;
>+
>+		p_dst = switch_name;
>+		while ((*p_src != '_') && (*p_src)) {
>+			if (*p_src == '@') {
>+				*p_dst++ = '|';
>+				p_src++;
>+			} else
>+				*p_dst++ = *p_src++;
>+		}
>+		*p_dst = 0;
>+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
>+
>+		/* move over the second underline */
>+		p_src++;
>+
>+		p_dst = port_name;
>+		while (*p_src)
>+			*p_dst++ = *p_src++;
>+		*p_dst = 0;
>+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
>+
>+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
>+			"net_%s_representor_%s", switch_name, port_name);
>+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
>+			switch_ethdev_name);
>+
>+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
>+			&port_id);
>+		if (ret)
>+			*switch_ethdev = NULL;
>+		else
>+			*switch_ethdev = &rte_eth_devices[port_id];
>+	} else
>+		*switch_ethdev = NULL;

After read the body of this function, it's about getting a switch_ethdev by a name other than updating the link status, so the func name is confusing, better to use a more precise name, and return value can be struct *rte_eth_dev, then you wouldn't have to pass struct rte_eth_dev **.

>+}
>+
> int
> i40e_dev_link_update(struct rte_eth_dev *dev,
> 		     int wait_to_complete)
>@@ -2792,6 +2868,11 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	struct rte_eth_link link;
> 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> 	int ret;
>+	struct rte_devargs *devargs;
>+	struct rte_kvargs *kvlist = NULL;
>+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>+	struct rte_eth_dev *switch_ethdev;
>+	char *switch_cfg_str = NULL;
> 
> 	memset(&link, 0, sizeof(link));
> 
>@@ -2805,6 +2886,40 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
> 	else
> 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
>+	devargs = pci_dev->device.devargs;
>+	if (devargs) {
>+		kvlist = rte_kvargs_parse(devargs->args, valid_keys);
>+		if (kvlist != NULL) {
>+			if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG)
>+				== 1) {
>+				if (!rte_kvargs_process(kvlist,
>+					ETH_I40E_SWITCH_MODE_ARG,
>+					&i40e_pf_parse_switch_mode,
>+					&switch_cfg_str)) {
>+
>+					i40e_pf_switch_mode_link_update(
>+						switch_cfg_str,
>+						&switch_ethdev);
>+
>+					if (switch_ethdev) {
>+						rte_eth_linkstatus_get(
>+							switch_ethdev,
>+							&link);
>+					} else {
>+						link.link_duplex =
>+							ETH_LINK_FULL_DUPLEX;
>+						link.link_autoneg =
>+							ETH_LINK_SPEED_FIXED;
>+						link.link_speed =
>+							ETH_SPEED_NUM_25G;
>+						link.link_status = 0;
>+					}
>+				}
>+			}
>+			rte_kvargs_free(kvlist);
>+		}
>+	}
>+

Better to wrap above code to a function to avoid too many levels of block nesting.

Thanks,
Xiaolong
> 	ret = rte_eth_linkstatus_set(dev, &link);
> 	i40e_notify_all_vfs_link_status(dev);
> 
>@@ -12790,4 +12905,5 @@ struct i40e_customized_pctype*
> 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
> 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
> 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
>-			      ETH_I40E_USE_LATEST_VEC "=0|1");
>+			      ETH_I40E_USE_LATEST_VEC "=0|1"
>+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
>--
>1.8.3.1
>

[dpdk-dev] [PATCH v2] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
  nesting.

 drivers/net/i40e/i40e_ethdev.c | 121 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 120 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 3364455..abf95aa 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -2778,6 +2780,116 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	*(char **)extra_args = strdup(value);
+
+	if (!*(char **)extra_args)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else
+				*p_dst++ = *p_src++;
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			return NULL;
+		else
+			return &rte_eth_devices[port_id];
+	} else
+		return NULL;
+}
+
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_devargs *devargs, struct rte_eth_link *link)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev;
+	char *switch_cfg_str = NULL;
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, &switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name(
+					switch_cfg_str);
+
+				rte_free(switch_cfg_str);
+
+				if (switch_ethdev) {
+					rte_eth_linkstatus_get(switch_ethdev,
+						link);
+				} else {
+					link->link_autoneg =
+						ETH_LINK_SPEED_FIXED;
+					link->link_duplex =
+						ETH_LINK_FULL_DUPLEX;
+					link->link_speed = ETH_SPEED_NUM_25G;
+					link->link_status = 0;
+				}
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
@@ -2785,6 +2897,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_devargs *devargs;
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2799,6 +2913,10 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	devargs = pci_dev->device.devargs;
+	if (devargs)
+		i40e_pf_linkstatus_get_from_switch_ethdev(devargs, &link);
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12772,4 +12890,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] net/i40e: i40e get link status update from ipn3ke

[Zhang, Qi Z]

> -----Original Message-----
> From: Pei, Andy
> Sent: Friday, June 28, 2019 4:33 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH v2] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
> PF is connected to. i40e PF get link status update via the connected FPGA.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
....

> @@ -2799,6 +2913,10 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	devargs = pci_dev->device.devargs;
> +	if (devargs)
> +		i40e_pf_linkstatus_get_from_switch_ethdev(devargs, &link);

It's not necessary to check devargs every time we do i40e_dev_link_update, 
its better initialize the ipn3ke mode during initialization (add some field in i40e_adapter maybe), so we can reuse the result at runtime

> +
......

[dpdk-dev] [PATCH v3] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.
Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
Try to bind i40e pf to switch device when i40e device is probed. If it
fail to find correct switch device, bind will occur again when update
i40e device link status.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v3:
* Add switch_ethdev to rte_eth_dev_data to track the bind switch device
* Try to bind i40e pf when it is probed.

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
  nesting.

 drivers/net/i40e/i40e_ethdev.c      | 136 +++++++++++++++++++++++++++++++++++-
 lib/librte_ethdev/rte_ethdev_core.h |   4 ++
 2 files changed, 138 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2b9fc45..8b4541d 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -625,14 +627,115 @@ struct rte_i40e_xstats_name_off {
 		sizeof(rte_i40e_txq_prio_strings[0]))
 
 static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	*(char **)extra_args = strdup(value);
+
+	if (!*(char **)extra_args)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else
+				*p_dst++ = *p_src++;
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			return NULL;
+		else
+			return &rte_eth_devices[port_id];
+	} else
+		return NULL;
+}
+
+static struct rte_eth_dev *
+i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev = NULL;
+	char *switch_cfg_str = NULL;
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, &switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name(
+					switch_cfg_str);
+
+				rte_free(switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+	return switch_ethdev;
+}
+
+static int
 eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
 	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
+	struct rte_eth_dev *switch_ethdev = NULL;
 	int i, retval;
 
 	if (pci_dev->device.devargs) {
+		switch_ethdev = i40e_get_switch_ethdev_from_devargs(
+			pci_dev->device.devargs);
 		retval = rte_eth_devargs_parse(pci_dev->device.devargs->args,
 				&eth_da);
 		if (retval)
@@ -654,6 +757,8 @@ struct rte_i40e_xstats_name_off {
 	if (pf_ethdev == NULL)
 		return -ENODEV;
 
+	pf_ethdev->data->switch_ethdev = switch_ethdev;
+
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct i40e_vf_representor representor = {
 			.vf_id = eth_da.representor_ports[i],
@@ -687,7 +792,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 	if (!ethdev)
 		return -ENODEV;
 
-
+	ethdev->data->switch_ethdev = NULL;
 	if (ethdev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR)
 		return rte_eth_dev_destroy(ethdev, i40e_vf_representor_uninit);
 	else
@@ -2779,6 +2884,20 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link)
+{
+	if (switch_ethdev) {
+		rte_eth_linkstatus_get(switch_ethdev, link);
+	} else {
+		link->link_autoneg = ETH_LINK_SPEED_FIXED;
+		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
+		link->link_speed   = ETH_SPEED_NUM_25G;
+		link->link_status  = 0;
+	}
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
@@ -2786,6 +2905,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_devargs *devargs;
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2800,6 +2921,16 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	if (!dev->data->switch_ethdev) {
+		devargs = pci_dev->device.devargs;
+		if (devargs)
+			dev->data->switch_ethdev =
+				i40e_get_switch_ethdev_from_devargs(
+					pci_dev->device.devargs);
+	}
+	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
+		&link);
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12773,4 +12904,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 2922d5b..62adc5c 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -635,6 +635,10 @@ struct rte_eth_dev_data {
 			/**< Switch-specific identifier.
 			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
 			 */
+	struct rte_eth_dev *switch_ethdev;
+			/* point to switch_ethdev specific by "switch_mode" in
+			 * devargs
+			 */
 } __rte_cache_aligned;
 
 /**
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2] net/i40e: i40e get link status update from ipn3ke

[Pei, Andy]

Hi Qi,

I will do it in v3.
Thanks

-----Original Message-----
From: Zhang, Qi Z 
Sent: Sunday, June 30, 2019 8:36 AM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; stable@dpdk.org
Subject: RE: [PATCH v2] net/i40e: i40e get link status update from ipn3ke



> -----Original Message-----
> From: Pei, Andy
> Sent: Friday, June 28, 2019 4:33 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh 
> <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong 
> <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; 
> stable@dpdk.org
> Subject: [PATCH v2] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that 
> i40e PF is connected to. i40e PF get link status update via the connected FPGA.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
....

> @@ -2799,6 +2913,10 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	devargs = pci_dev->device.devargs;
> +	if (devargs)
> +		i40e_pf_linkstatus_get_from_switch_ethdev(devargs, &link);

It's not necessary to check devargs every time we do i40e_dev_link_update, its better initialize the ipn3ke mode during initialization (add some field in i40e_adapter maybe), so we can reuse the result at runtime

> +
......

[dpdk-dev] [PATCH v4] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.
Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
Try to bind i40e pf to switch device when i40e device is probed. If it
fail to find correct switch device, bind will occur again when update
i40e device link status.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v4:
* use an array instead of a pointer to store switch device string to
* avoid memory free error.

v3:
* Add switch_ethdev to rte_eth_dev_data to track the bind switch device
* Try to bind i40e pf when it is probed.

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
* nesting.

 drivers/net/i40e/i40e_ethdev.c      | 139 +++++++++++++++++++++++++++++++++++-
 lib/librte_ethdev/rte_ethdev_core.h |   4 ++
 2 files changed, 141 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2b9fc45..85d7b18 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -625,14 +627,118 @@ struct rte_i40e_xstats_name_off {
 		sizeof(rte_i40e_txq_prio_strings[0]))
 
 static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	if (RTE_ETH_NAME_MAX_LEN > strlen(value)) {
+		rte_memcpy(extra_args, value, strlen(value) + 1);
+		return 0;
+	} else {
+		PMD_DRV_LOG(ERR,
+			"switch_mode args should be less than %d characters",
+			RTE_ETH_NAME_MAX_LEN);
+		return -EINVAL;
+	}
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else {
+				*p_dst++ = *p_src++;
+			}
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			return NULL;
+		else
+			return &rte_eth_devices[port_id];
+	} else {
+		return NULL;
+	}
+}
+
+static struct rte_eth_dev *
+i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev = NULL;
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name(
+					switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+	return switch_ethdev;
+}
+
+static int
 eth_i40e_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
 	char name[RTE_ETH_NAME_MAX_LEN];
 	struct rte_eth_devargs eth_da = { .nb_representor_ports = 0 };
+	struct rte_eth_dev *switch_ethdev = NULL;
 	int i, retval;
 
 	if (pci_dev->device.devargs) {
+		switch_ethdev = i40e_get_switch_ethdev_from_devargs(
+			pci_dev->device.devargs);
 		retval = rte_eth_devargs_parse(pci_dev->device.devargs->args,
 				&eth_da);
 		if (retval)
@@ -654,6 +760,8 @@ struct rte_i40e_xstats_name_off {
 	if (pf_ethdev == NULL)
 		return -ENODEV;
 
+	pf_ethdev->data->switch_ethdev = switch_ethdev;
+
 	for (i = 0; i < eth_da.nb_representor_ports; i++) {
 		struct i40e_vf_representor representor = {
 			.vf_id = eth_da.representor_ports[i],
@@ -687,7 +795,7 @@ static int eth_i40e_pci_remove(struct rte_pci_device *pci_dev)
 	if (!ethdev)
 		return -ENODEV;
 
-
+	ethdev->data->switch_ethdev = NULL;
 	if (ethdev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR)
 		return rte_eth_dev_destroy(ethdev, i40e_vf_representor_uninit);
 	else
@@ -2779,6 +2887,20 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link)
+{
+	if (switch_ethdev) {
+		rte_eth_linkstatus_get(switch_ethdev, link);
+	} else {
+		link->link_autoneg = ETH_LINK_SPEED_FIXED;
+		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
+		link->link_speed   = ETH_SPEED_NUM_25G;
+		link->link_status  = 0;
+	}
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
@@ -2786,6 +2908,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_devargs *devargs;
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2800,6 +2924,16 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	if (!dev->data->switch_ethdev) {
+		devargs = pci_dev->device.devargs;
+		if (devargs)
+			dev->data->switch_ethdev =
+				i40e_get_switch_ethdev_from_devargs(
+					pci_dev->device.devargs);
+	}
+	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
+		&link);
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12773,4 +12907,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
index 2922d5b..62adc5c 100644
--- a/lib/librte_ethdev/rte_ethdev_core.h
+++ b/lib/librte_ethdev/rte_ethdev_core.h
@@ -635,6 +635,10 @@ struct rte_eth_dev_data {
 			/**< Switch-specific identifier.
 			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
 			 */
+	struct rte_eth_dev *switch_ethdev;
+			/* point to switch_ethdev specific by "switch_mode" in
+			 * devargs
+			 */
 } __rte_cache_aligned;
 
 /**
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v3] net/i40e: i40e get link status update from ipn3ke

[Zhang, Qi Z]

Hi Andy:

> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 4, 2019 2:56 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH v3] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
> PF is connected to. i40e PF get link status update via the connected FPGA.
> Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
> Try to bind i40e pf to switch device when i40e device is probed. If it fail to find
> correct switch device, bind will occur again when update i40e device link
> status.
>

......
 
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
> @@ -2786,6 +2905,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +	struct rte_devargs *devargs;
>  	int ret;
> 
>  	memset(&link, 0, sizeof(link));
> @@ -2800,6 +2921,16 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	if (!dev->data->switch_ethdev) {
> +		devargs = pci_dev->device.devargs;
> +		if (devargs)
> +			dev->data->switch_ethdev =
> +				i40e_get_switch_ethdev_from_devargs(
> +					pci_dev->device.devargs);
> +	}

For regular mode, switch_ethdev is always null, seems above code did unnecessary devargs parsing for every link_update call
Can we add an intermediate flag (add a field in i40e_pf indicate if switch mode is required like other devargs ) so during probe this flag can be initialized, and it can be reused later.

So all switch mode related code can be braces with that flag. 
If (xxx_flag)
{
 .....
}


> +	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
> +		&link);


Why i40e_pf_linkstatus_get_from_switch_ethdev is always be called?, should we do

If (dev->data->switch_ethdev)
	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev, &link);

Re: [dpdk-dev] [PATCH v4] net/i40e: i40e get link status update from ipn3ke

[Zhang, Qi Z]

> -----Original Message-----
> From: Pei, Andy
> Sent: Monday, July 8, 2019 11:03 AM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH v4] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
> PF is connected to. i40e PF get link status update via the connected FPGA.
> Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
> Try to bind i40e pf to switch device when i40e device is probed. If it fail to find
> correct switch device, bind will occur again when update i40e device link
> status.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
.....
> diff --git a/lib/librte_ethdev/rte_ethdev_core.h
> b/lib/librte_ethdev/rte_ethdev_core.h
> index 2922d5b..62adc5c 100644
> --- a/lib/librte_ethdev/rte_ethdev_core.h
> +++ b/lib/librte_ethdev/rte_ethdev_core.h
> @@ -635,6 +635,10 @@ struct rte_eth_dev_data {
>  			/**< Switch-specific identifier.
>  			 *   Valid if RTE_ETH_DEV_REPRESENTOR in dev_flags.
>  			 */
> +	struct rte_eth_dev *switch_ethdev;
> +			/* point to switch_ethdev specific by "switch_mode" in
> +			 * devargs
> +			 */
>  } __rte_cache_aligned;

Missing below comment in previous review.

No need to add new field in rte_eth_dev_data to break ABI, you can just add to i40e specific structure: i40e_pf.

> 
>  /**
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v3] net/i40e: i40e get link status update from ipn3ke

[Pei, Andy]

Yes. I think you are right. I will try your suggestion.

-----Original Message-----
From: Zhang, Qi Z 
Sent: Monday, July 8, 2019 1:56 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; stable@dpdk.org
Subject: RE: [PATCH v3] net/i40e: i40e get link status update from ipn3ke

Hi Andy:

> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 4, 2019 2:56 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh 
> <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong 
> <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; 
> stable@dpdk.org
> Subject: [PATCH v3] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that 
> i40e PF is connected to. i40e PF get link status update via the connected FPGA.
> Add switch_ethdev to rte_eth_dev_data to track the bind switch device.
> Try to bind i40e pf to switch device when i40e device is probed. If it 
> fail to find correct switch device, bind will occur again when update 
> i40e device link status.
>

......
 
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
> @@ -2786,6 +2905,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +	struct rte_devargs *devargs;
>  	int ret;
> 
>  	memset(&link, 0, sizeof(link));
> @@ -2800,6 +2921,16 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	if (!dev->data->switch_ethdev) {
> +		devargs = pci_dev->device.devargs;
> +		if (devargs)
> +			dev->data->switch_ethdev =
> +				i40e_get_switch_ethdev_from_devargs(
> +					pci_dev->device.devargs);
> +	}

For regular mode, switch_ethdev is always null, seems above code did unnecessary devargs parsing for every link_update call Can we add an intermediate flag (add a field in i40e_pf indicate if switch mode is required like other devargs ) so during probe this flag can be initialized, and it can be reused later.

So all switch mode related code can be braces with that flag. 
If (xxx_flag)
{
 .....
}


> +	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev,
> +		&link);


Why i40e_pf_linkstatus_get_from_switch_ethdev is always be called?, should we do

If (dev->data->switch_ethdev)
	i40e_pf_linkstatus_get_from_switch_ethdev(dev->data->switch_ethdev, &link);

[dpdk-dev] [PATCH v5] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.
Add bool switch_ethdev_support_flag to struct i40e_pf to specify if
there are switch_mode argues in cmd.
Add switch_ethdev to struct i40e_pf to track the bind switch device.
Try to bind i40e pf to switch device when i40e device is initialized.
If it fail to find correct switch device, bind will occur again when
update i40e device link status.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v5:
* use a bool switch_ethdev_support_flag in struct i40e_pf to specify
* if i40e device needs to get link status update from ipn3ke device.

v4:
* use an array instead of a pointer to store switch device string to
* avoid memory free error.

v3:
* Add switch_ethdev to rte_eth_dev_data to track the bind switch
* device
* Try to bind i40e pf when it is probed.

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
* nesting.

 drivers/net/i40e/i40e_ethdev.c | 181 ++++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |   4 +
 2 files changed, 184 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2b9fc45..8733ffd 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -1260,6 +1262,125 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 #define I40E_ALARM_INTERVAL 50000 /* us */
 
 static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	if (RTE_ETH_NAME_MAX_LEN > strlen(value)) {
+		rte_memcpy(extra_args, value, strlen(value) + 1);
+	} else {
+		PMD_DRV_LOG(ERR,
+			"switch_mode args should be less than %d characters",
+			RTE_ETH_NAME_MAX_LEN);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else {
+				*p_dst++ = *p_src++;
+			}
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			return NULL;
+		else
+			return &rte_eth_devices[port_id];
+	} else {
+		return NULL;
+	}
+}
+
+static void
+eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev,
+struct i40e_pf *pf)
+{
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_devargs *devargs;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+
+	pf->switch_ethdev_support_flag = 0;
+	pf->switch_ethdev = NULL;
+
+	devargs = pci_dev->device.devargs;
+	if (!devargs)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			pf->switch_ethdev_support_flag = 1;
+
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				pf->switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+	rte_kvargs_free(kvlist);
+	}
+	return;
+}
+static void
+eth_i40e_pf_switch_dev_uninit(struct i40e_pf *pf)
+{
+	pf->switch_ethdev_support_flag = 0;
+	pf->switch_ethdev = NULL;
+	return;
+}
+
+static int
 eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 {
 	struct rte_pci_device *pci_dev;
@@ -1582,6 +1703,11 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 	memset(&pf->rss_info, 0,
 		sizeof(struct i40e_rte_flow_rss_conf));
 
+	/* parse the switch device from devargs, try to bind to the switch
+	 * device, if binding not succeed, bind again when i40e_dev_link_update
+	 */
+	eth_i40e_pf_switch_dev_init(dev, pf);
+
 	/* reset all stats of the device, including pf and main vsi */
 	i40e_dev_stats_reset(dev);
 
@@ -1762,6 +1888,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 		rte_free(p_flow);
 	}
 
+	eth_i40e_pf_switch_dev_uninit(pf);
+
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
@@ -2779,13 +2907,52 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link)
+{
+	if (switch_ethdev) {
+		rte_eth_linkstatus_get(switch_ethdev, link);
+	} else {
+		link->link_autoneg = ETH_LINK_SPEED_FIXED;
+		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
+		link->link_speed   = ETH_SPEED_NUM_25G;
+		link->link_status  = 0;
+	}
+}
+
+static struct rte_eth_dev *
+i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev = NULL;
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+	return switch_ethdev;
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
 {
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2800,6 +2967,17 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	if (pf->switch_ethdev_support_flag) {
+		if (!pf->switch_ethdev) {
+			if (pci_dev->device.devargs)
+				pf->switch_ethdev =
+					i40e_get_switch_ethdev_from_devargs
+						(pci_dev->device.devargs);
+		}
+		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
+			&link);
+	}
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12773,4 +12951,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 38ac3ea..cc56c8f 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -975,6 +975,10 @@ struct i40e_pf {
 	struct i40e_customized_pctype customized_pctype[I40E_CUSTOMIZED_MAX];
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
+	bool switch_ethdev_support_flag;
+	/* flag indicating if switch mode is required like other devargs */
+	struct rte_eth_dev *switch_ethdev;
+	/* point to switch_ethdev specific by "switch_mode" in devargs */
 };
 
 enum pending_msg {
-- 
1.8.3.1

[dpdk-dev] [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.
Add bool switch_ethdev_support_flag to struct i40e_pf to specify if
there are switch_mode argues in cmd.
Add switch_ethdev to struct i40e_pf to track the bind switch device.
Try to bind i40e pf to switch device when i40e device is initialized.
If it fail to find correct switch device, bind will occur again when
update i40e device link status.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v6:
* fix some coding style.

v5:
* use a bool switch_ethdev_support_flag in struct i40e_pf to specify
* if i40e device needs to get link status update from ipn3ke device.

v4:
* use an array instead of a pointer to store switch device string to
* avoid memory free error.

v3:
* Add switch_ethdev to rte_eth_dev_data to track the bind switch
* device
* Try to bind i40e pf when it is probed.

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
* nesting.

drivers/net/i40e/i40e_ethdev.c | 179 ++++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |   6 ++
 2 files changed, 184 insertions(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2b9fc45..deeca88 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -1260,6 +1262,123 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 #define I40E_ALARM_INTERVAL 50000 /* us */
 
 static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	if (strlen(value) < RTE_ETH_NAME_MAX_LEN) {
+		rte_memcpy(extra_args, value, strlen(value) + 1);
+	} else {
+		PMD_DRV_LOG(ERR,
+			"switch_mode args should be less than %d characters",
+			RTE_ETH_NAME_MAX_LEN);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else {
+				*p_dst++ = *p_src++;
+			}
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (ret)
+			return NULL;
+		else
+			return &rte_eth_devices[port_id];
+	} else {
+		return NULL;
+	}
+}
+
+static void
+eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev,
+struct i40e_pf *pf)
+{
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_devargs *devargs;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+
+	pf->switch_ethdev_support_flag = 0;
+	pf->switch_ethdev = NULL;
+
+	devargs = pci_dev->device.devargs;
+	if (!devargs)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			pf->switch_ethdev_support_flag = 1;
+
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				pf->switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+	rte_kvargs_free(kvlist);
+	}
+}
+static void
+eth_i40e_pf_switch_dev_uninit(struct i40e_pf *pf)
+{
+	pf->switch_ethdev_support_flag = 0;
+	pf->switch_ethdev = NULL;
+}
+
+static int
 eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 {
 	struct rte_pci_device *pci_dev;
@@ -1582,6 +1701,11 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 	memset(&pf->rss_info, 0,
 		sizeof(struct i40e_rte_flow_rss_conf));
 
+	/* parse the switch device from devargs, try to bind to the switch
+	 * device, if binding not succeed, bind again when i40e_dev_link_update
+	 */
+	eth_i40e_pf_switch_dev_init(dev, pf);
+
 	/* reset all stats of the device, including pf and main vsi */
 	i40e_dev_stats_reset(dev);
 
@@ -1762,6 +1886,8 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 		rte_free(p_flow);
 	}
 
+	eth_i40e_pf_switch_dev_uninit(pf);
+
 	/* Remove all Traffic Manager configuration */
 	i40e_tm_conf_uninit(dev);
 
@@ -2779,13 +2905,52 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link)
+{
+	if (switch_ethdev) {
+		rte_eth_linkstatus_get(switch_ethdev, link);
+	} else {
+		link->link_autoneg = ETH_LINK_SPEED_FIXED;
+		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
+		link->link_speed   = ETH_SPEED_NUM_25G;
+		link->link_status  = 0;
+	}
+}
+
+static struct rte_eth_dev *
+i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev = NULL;
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+	return switch_ethdev;
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
 {
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2800,6 +2965,17 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	if (pf->switch_ethdev_support_flag) {
+		if (!pf->switch_ethdev) {
+			if (pci_dev->device.devargs)
+				pf->switch_ethdev =
+					i40e_get_switch_ethdev_from_devargs
+						(pci_dev->device.devargs);
+		}
+		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
+			&link);
+	}
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12773,4 +12949,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 38ac3ea..cfdcecb 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -975,6 +975,12 @@ struct i40e_pf {
 	struct i40e_customized_pctype customized_pctype[I40E_CUSTOMIZED_MAX];
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
+
+	/* flag indicating if switch mode is required like other devargs */
+	bool switch_ethdev_support_flag;
+
+	/* point to switch_ethdev specific by "switch_mode" in devargs */
+	struct rte_eth_dev *switch_ethdev;
 };
 
 enum pending_msg {
-- 
1.8.3.1

[dpdk-dev] [PATCH v6 2/2] doc: add switch mode devargs

[Andy Pei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1151 bytes --]

add document for switch_mode devargs.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
 doc/guides/nics/i40e.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index a97484c..8d88f0d 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -513,6 +513,17 @@ details please refer to :doc:`../testpmd_app_ug/index`.
    testpmd> set port (port_id) queue-region flush (on|off)
    testpmd> show port (port_id) queue-region
 
+Switch Mode
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The Intel® Ethernet 700 Series can be component of IPN3KE device, in which the
+Intel® Ethernet 700 Series is connected to other pci device. This pci device
+works as switch device via which the Intel® Ethernet 700 Series get link status
+from. For example: the Intel® Ethernet 700 Series 0000:b1:00.0 is connected to
+an IPN3KE device whose BDF is b3:00.0. In this case 0000:b1:00.0 use representor
+0 on b3:00.0.
+
+   ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x3 -n 4 -w 0000:b1:00.0,switch_mode=IPN3KE_0@b3:00.0_0
+
 Limitations or Known issues
 ---------------------------
 
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke

[Zhang, Qi Z]

> -----Original Message-----
> From: Pei, Andy
> Sent: Wednesday, July 10, 2019 5:41 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke

You can merge patch 2/2 with this patch.

...

> 
> +static struct rte_eth_dev *
> +i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str) {
> +	char switch_name[RTE_ETH_NAME_MAX_LEN];
> +	char port_name[RTE_ETH_NAME_MAX_LEN];
> +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
> +	uint16_t port_id;
> +	const char *p_src;
> +	char *p_dst;
> +	int ret;
> +
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;
> +
> +		/* move over the first underline */
> +		p_src++;
> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';
> +				p_src++;
> +			} else {
> +				*p_dst++ = *p_src++;
> +			}
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;
> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> +
> +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> +			"net_%s_representor_%s", switch_name, port_name);
> +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> +			switch_ethdev_name);
> +
> +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> +			&port_id);
> +		if (ret)
> +			return NULL;
> +		else
> +			return &rte_eth_devices[port_id];
> +	} else {
> +		return NULL;
> +	}

This code can simplified to

If (... ) {
	...
	If (!ret)
		return &ret_eth_devices[port_id];
}

return NULL;


> +}
> +
> +static void
> +eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev, struct i40e_pf
> +*pf) {

Argument "pf" is redundant, please remove it and use
 pf =I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); 

> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_devargs *devargs;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +
> +	devargs = pci_dev->device.devargs;
> +	if (!devargs)
> +		return;
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			pf->switch_ethdev_support_flag = 1;
> +
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				pf->switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line.

> +			}
> +		}
> +	rte_kvargs_free(kvlist);
Wrong intend
> +	}
> +}
> +static void
> +eth_i40e_pf_switch_dev_uninit(struct i40e_pf *pf) {
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +}

This function is not necessary, after dev_init must be called after dev_unit, and above reset is performed in eth_i40e_pf_switch_dev_init already.
> +
> +static int
>  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
> {
>  	struct rte_pci_device *pci_dev;
> @@ -1582,6 +1701,11 @@ static inline void i40e_config_automask(struct
> i40e_pf *pf)
>  	memset(&pf->rss_info, 0,
>  		sizeof(struct i40e_rte_flow_rss_conf));
> 
> +	/* parse the switch device from devargs, try to bind to the switch
> +	 * device, if binding not succeed, bind again when i40e_dev_link_update
> +	 */
> +	eth_i40e_pf_switch_dev_init(dev, pf);
> +
>  	/* reset all stats of the device, including pf and main vsi */
>  	i40e_dev_stats_reset(dev);
> 
> @@ -1762,6 +1886,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  		rte_free(p_flow);
>  	}
> 
> +	eth_i40e_pf_switch_dev_uninit(pf);
> +
>  	/* Remove all Traffic Manager configuration */
>  	i40e_tm_conf_uninit(dev);
> 
> @@ -2779,13 +2905,52 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	}
>  }
> 
> +static void
> +i40e_pf_linkstatus_get_from_switch_ethdev
> +(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link) {
> +	if (switch_ethdev) {
> +		rte_eth_linkstatus_get(switch_ethdev, link);
> +	} else {
> +		link->link_autoneg = ETH_LINK_SPEED_FIXED;
> +		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
> +		link->link_speed   = ETH_SPEED_NUM_25G;
> +		link->link_status  = 0;
> +	}
> +}
> +
> +static struct rte_eth_dev *
> +i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs) {
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_eth_dev *switch_ethdev = NULL;
> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line

> +			}
> +		}
> +		rte_kvargs_free(kvlist);
> +	}
> +	return switch_ethdev;
> +}
> +
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
>  {
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	int ret;
> 
>  	memset(&link, 0, sizeof(link));
> @@ -2800,6 +2965,17 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	if (pf->switch_ethdev_support_flag) {
> +		if (!pf->switch_ethdev) {
> +			if (pci_dev->device.devargs)

No need to check here, 
If swtch_ethdev_support_flag is 1, pci_dev->device.devargs should not be null, right?

> +				pf->switch_ethdev =
> +					i40e_get_switch_ethdev_from_devargs
> +						(pci_dev->device.devargs);
> +		}
> +		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
> +			&link);
> +	}
> +
>  	ret = rte_eth_linkstatus_set(dev, &link);
>  	i40e_notify_all_vfs_link_status(dev);
> 
> @@ -12773,4 +12949,5 @@ struct i40e_customized_pctype*
>  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
>  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 38ac3ea..cfdcecb 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -975,6 +975,12 @@ struct i40e_pf {
>  	struct i40e_customized_pctype
> customized_pctype[I40E_CUSTOMIZED_MAX];
>  	/* Switch Domain Id */
>  	uint16_t switch_domain_id;
> +
> +	/* flag indicating if switch mode is required like other devargs */
> +	bool switch_ethdev_support_flag;
> +
> +	/* point to switch_ethdev specific by "switch_mode" in devargs */
> +	struct rte_eth_dev *switch_ethdev;
>  };
> 
>  enum pending_msg {
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke

[Pei, Andy]

Hi, Qi

Reply inline.

-----Original Message-----
From: Zhang, Qi Z 
Sent: Thursday, July 11, 2019 12:37 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; stable@dpdk.org
Subject: RE: [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke



> -----Original Message-----
> From: Pei, Andy
> Sent: Wednesday, July 10, 2019 5:41 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh 
> <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong 
> <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; 
> stable@dpdk.org
> Subject: [PATCH v6 1/2] net/i40e: i40e get link status update from 
> ipn3ke

You can merge patch 2/2 with this patch.
OK
...

> 
> +static struct rte_eth_dev *
> +i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str) {
> +	char switch_name[RTE_ETH_NAME_MAX_LEN];
> +	char port_name[RTE_ETH_NAME_MAX_LEN];
> +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
> +	uint16_t port_id;
> +	const char *p_src;
> +	char *p_dst;
> +	int ret;
> +
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;
> +
> +		/* move over the first underline */
> +		p_src++;
> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';
> +				p_src++;
> +			} else {
> +				*p_dst++ = *p_src++;
> +			}
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;
> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> +
> +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> +			"net_%s_representor_%s", switch_name, port_name);
> +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> +			switch_ethdev_name);
> +
> +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> +			&port_id);
> +		if (ret)
> +			return NULL;
> +		else
> +			return &rte_eth_devices[port_id];
> +	} else {
> +		return NULL;
> +	}

This code can simplified to

If (... ) {
	...
	If (!ret)
		return &ret_eth_devices[port_id];
}

return NULL;

OK

> +}
> +
> +static void
> +eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev, struct i40e_pf
> +*pf) {

Argument "pf" is redundant, please remove it and use  pf =I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); 

OK

> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_devargs *devargs;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +
> +	devargs = pci_dev->device.devargs;
> +	if (!devargs)
> +		return;
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			pf->switch_ethdev_support_flag = 1;
> +
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				pf->switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line.

This will cause a coding style issue.
CHECK:OPEN_ENDED_LINE: Lines should not end with a '('
#174: FILE: drivers/net/i40e/i40e_ethdev.c:721:
+					i40e_eth_dev_get_by_switch_mode_name(
> +			}
> +		}
> +	rte_kvargs_free(kvlist);
Wrong intend

OK
> +	}
> +}
> +static void
> +eth_i40e_pf_switch_dev_uninit(struct i40e_pf *pf) {
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +}

This function is not necessary, after dev_init must be called after dev_unit, and above reset is performed in eth_i40e_pf_switch_dev_init already.
OK
> +
> +static int
>  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params 
> __rte_unused) {
>  	struct rte_pci_device *pci_dev;
> @@ -1582,6 +1701,11 @@ static inline void i40e_config_automask(struct 
> i40e_pf *pf)
>  	memset(&pf->rss_info, 0,
>  		sizeof(struct i40e_rte_flow_rss_conf));
> 
> +	/* parse the switch device from devargs, try to bind to the switch
> +	 * device, if binding not succeed, bind again when i40e_dev_link_update
> +	 */
> +	eth_i40e_pf_switch_dev_init(dev, pf);
> +
>  	/* reset all stats of the device, including pf and main vsi */
>  	i40e_dev_stats_reset(dev);
> 
> @@ -1762,6 +1886,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  		rte_free(p_flow);
>  	}
> 
> +	eth_i40e_pf_switch_dev_uninit(pf);
> +
>  	/* Remove all Traffic Manager configuration */
>  	i40e_tm_conf_uninit(dev);
> 
> @@ -2779,13 +2905,52 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	}
>  }
> 
> +static void
> +i40e_pf_linkstatus_get_from_switch_ethdev
> +(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link) {
> +	if (switch_ethdev) {
> +		rte_eth_linkstatus_get(switch_ethdev, link);
> +	} else {
> +		link->link_autoneg = ETH_LINK_SPEED_FIXED;
> +		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
> +		link->link_speed   = ETH_SPEED_NUM_25G;
> +		link->link_status  = 0;
> +	}
> +}
> +
> +static struct rte_eth_dev *
> +i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs) {
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_eth_dev *switch_ethdev = NULL;
> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line

> +			}
> +		}
> +		rte_kvargs_free(kvlist);
> +	}
> +	return switch_ethdev;
> +}
> +
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
>  {
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	int ret;
> 
>  	memset(&link, 0, sizeof(link));
> @@ -2800,6 +2965,17 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	if (pf->switch_ethdev_support_flag) {
> +		if (!pf->switch_ethdev) {
> +			if (pci_dev->device.devargs)

No need to check here,
If swtch_ethdev_support_flag is 1, pci_dev->device.devargs should not be null, right?

Switch device may not be probed when i40e device is being probed. In this case, swtch_ethdev_support_flag is 1, , pci_dev->device.devargs is null.
This is explained here.
/* parse the switch device from devargs, try to bind to the switch
 * device, if binding not succeed, bind again when i40e_dev_link_update
 */
eth_i40e_pf_switch_dev_init(dev);

> +				pf->switch_ethdev =
> +					i40e_get_switch_ethdev_from_devargs
> +						(pci_dev->device.devargs);
> +		}
> +		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
> +			&link);
> +	}
> +
>  	ret = rte_eth_linkstatus_set(dev, &link);
>  	i40e_notify_all_vfs_link_status(dev);
> 
> @@ -12773,4 +12949,5 @@ struct i40e_customized_pctype*
>  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
>  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> diff --git a/drivers/net/i40e/i40e_ethdev.h 
> b/drivers/net/i40e/i40e_ethdev.h index 38ac3ea..cfdcecb 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -975,6 +975,12 @@ struct i40e_pf {
>  	struct i40e_customized_pctype
> customized_pctype[I40E_CUSTOMIZED_MAX];
>  	/* Switch Domain Id */
>  	uint16_t switch_domain_id;
> +
> +	/* flag indicating if switch mode is required like other devargs */
> +	bool switch_ethdev_support_flag;
> +
> +	/* point to switch_ethdev specific by "switch_mode" in devargs */
> +	struct rte_eth_dev *switch_ethdev;
>  };
> 
>  enum pending_msg {
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke

[Pei, Andy]

HI QI

-----Original Message-----
From: Zhang, Qi Z 
Sent: Thursday, July 11, 2019 12:37 PM
To: Pei, Andy <andy.pei@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; stable@dpdk.org
Subject: RE: [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke



> -----Original Message-----
> From: Pei, Andy
> Sent: Wednesday, July 10, 2019 5:41 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z 
> <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Xing, 
> Beilei <beilei.xing@intel.com>; Yigit, Ferruh 
> <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye, Xiaolong 
> <xiaolong.ye@intel.com>; Zhang, Roy Fan <roy.fan.zhang@intel.com>; 
> stable@dpdk.org
> Subject: [PATCH v6 1/2] net/i40e: i40e get link status update from 
> ipn3ke

You can merge patch 2/2 with this patch.

...

> 
> +static struct rte_eth_dev *
> +i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str) {
> +	char switch_name[RTE_ETH_NAME_MAX_LEN];
> +	char port_name[RTE_ETH_NAME_MAX_LEN];
> +	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
> +	uint16_t port_id;
> +	const char *p_src;
> +	char *p_dst;
> +	int ret;
> +
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;
> +
> +		/* move over the first underline */
> +		p_src++;
> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';
> +				p_src++;
> +			} else {
> +				*p_dst++ = *p_src++;
> +			}
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;
> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
> +
> +		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
> +			"net_%s_representor_%s", switch_name, port_name);
> +		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
> +			switch_ethdev_name);
> +
> +		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
> +			&port_id);
> +		if (ret)
> +			return NULL;
> +		else
> +			return &rte_eth_devices[port_id];
> +	} else {
> +		return NULL;
> +	}

This code can simplified to

If (... ) {
	...
	If (!ret)
		return &ret_eth_devices[port_id];
}

return NULL;


> +}
> +
> +static void
> +eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev, struct i40e_pf
> +*pf) {

Argument "pf" is redundant, please remove it and use  pf =I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); 

> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_devargs *devargs;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
> +
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +
> +	devargs = pci_dev->device.devargs;
> +	if (!devargs)
> +		return;
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			pf->switch_ethdev_support_flag = 1;
> +
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				pf->switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line.

> +			}
> +		}
> +	rte_kvargs_free(kvlist);
Wrong intend
> +	}
> +}
> +static void
> +eth_i40e_pf_switch_dev_uninit(struct i40e_pf *pf) {
> +	pf->switch_ethdev_support_flag = 0;
> +	pf->switch_ethdev = NULL;
> +}

This function is not necessary, after dev_init must be called after dev_unit, and above reset is performed in eth_i40e_pf_switch_dev_init already.
> +
> +static int
>  eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params 
> __rte_unused) {
>  	struct rte_pci_device *pci_dev;
> @@ -1582,6 +1701,11 @@ static inline void i40e_config_automask(struct 
> i40e_pf *pf)
>  	memset(&pf->rss_info, 0,
>  		sizeof(struct i40e_rte_flow_rss_conf));
> 
> +	/* parse the switch device from devargs, try to bind to the switch
> +	 * device, if binding not succeed, bind again when i40e_dev_link_update
> +	 */
> +	eth_i40e_pf_switch_dev_init(dev, pf);
> +
>  	/* reset all stats of the device, including pf and main vsi */
>  	i40e_dev_stats_reset(dev);
> 
> @@ -1762,6 +1886,8 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  		rte_free(p_flow);
>  	}
> 
> +	eth_i40e_pf_switch_dev_uninit(pf);
> +
>  	/* Remove all Traffic Manager configuration */
>  	i40e_tm_conf_uninit(dev);
> 
> @@ -2779,13 +2905,52 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	}
>  }
> 
> +static void
> +i40e_pf_linkstatus_get_from_switch_ethdev
> +(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link) {
> +	if (switch_ethdev) {
> +		rte_eth_linkstatus_get(switch_ethdev, link);
> +	} else {
> +		link->link_autoneg = ETH_LINK_SPEED_FIXED;
> +		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
> +		link->link_speed   = ETH_SPEED_NUM_25G;
> +		link->link_status  = 0;
> +	}
> +}
> +
> +static struct rte_eth_dev *
> +i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs) {
> +	struct rte_kvargs *kvlist = NULL;
> +	struct rte_eth_dev *switch_ethdev = NULL;
> +	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
> +
> +	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
> +	if (kvlist) {
> +		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
> +			if (!rte_kvargs_process(kvlist,
> +				ETH_I40E_SWITCH_MODE_ARG,
> +				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
> +				switch_ethdev =
> +					i40e_eth_dev_get_by_switch_mode_name
> +					(switch_cfg_str);
"(" should follow above line

> +			}
> +		}
> +		rte_kvargs_free(kvlist);
> +	}
> +	return switch_ethdev;
> +}
> +
>  int
>  i40e_dev_link_update(struct rte_eth_dev *dev,
>  		     int wait_to_complete)
>  {
>  	struct i40e_hw *hw =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
>  	struct rte_eth_link link;
>  	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
> +	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
>  	int ret;
> 
>  	memset(&link, 0, sizeof(link));
> @@ -2800,6 +2965,17 @@ void i40e_flex_payload_reg_set_default(struct
> i40e_hw *hw)
>  	else
>  		update_link_aq(hw, &link, enable_lse, wait_to_complete);
> 
> +	if (pf->switch_ethdev_support_flag) {
> +		if (!pf->switch_ethdev) {
> +			if (pci_dev->device.devargs)

No need to check here,
If swtch_ethdev_support_flag is 1, pci_dev->device.devargs should not be null, right?
Got your point

> +				pf->switch_ethdev =
> +					i40e_get_switch_ethdev_from_devargs
> +						(pci_dev->device.devargs);
> +		}
> +		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
> +			&link);
> +	}
> +
>  	ret = rte_eth_linkstatus_set(dev, &link);
>  	i40e_notify_all_vfs_link_status(dev);
> 
> @@ -12773,4 +12949,5 @@ struct i40e_customized_pctype*
>  			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
>  			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
>  			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
> -			      ETH_I40E_USE_LATEST_VEC "=0|1");
> +			      ETH_I40E_USE_LATEST_VEC "=0|1"
> +			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
> diff --git a/drivers/net/i40e/i40e_ethdev.h 
> b/drivers/net/i40e/i40e_ethdev.h index 38ac3ea..cfdcecb 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -975,6 +975,12 @@ struct i40e_pf {
>  	struct i40e_customized_pctype
> customized_pctype[I40E_CUSTOMIZED_MAX];
>  	/* Switch Domain Id */
>  	uint16_t switch_domain_id;
> +
> +	/* flag indicating if switch mode is required like other devargs */
> +	bool switch_ethdev_support_flag;
> +
> +	/* point to switch_ethdev specific by "switch_mode" in devargs */
> +	struct rte_eth_dev *switch_ethdev;
>  };
> 
>  enum pending_msg {
> --
> 1.8.3.1

[dpdk-dev] [PATCH v7] net/i40e: i40e get link status update from ipn3ke

[Andy Pei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 9729 bytes --]

Add switch_mode argument for i40e PF to specify the specific FPGA that
i40e PF is connected to. i40e PF get link status update via the
connected FPGA.
Add bool switch_ethdev_support_flag to struct i40e_pf to specify if
there are switch_mode argues in cmd.
Add switch_ethdev to struct i40e_pf to track the bind switch device.
Try to bind i40e pf to switch device when i40e device is initialized.
If it fail to find correct switch device, bind will occur again when
update i40e device link status.

Signed-off-by: Andy Pei <andy.pei@intel.com>
---
Cc: qi.z.zhang@intel.com
Cc: jingjing.wu@intel.com
Cc: beilei.xing@intel.com
Cc: ferruh.yigit@intel.com
Cc: rosen.xu@intel.com
Cc: xiaolong.ye@intel.com
Cc: roy.fan.zhang@intel.com
Cc: stable@dpdk.org

v7:
* simplify code and delete redundance.

v6:
* fix some coding style.

v5:
* use a bool switch_ethdev_support_flag in struct i40e_pf to specify
* if i40e device needs to get link status update from ipn3ke device.

v4:
* use an array instead of a pointer to store switch device string to
* avoid memory free error.

v3:
* Add switch_ethdev to rte_eth_dev_data to track the bind switch
* device
* Try to bind i40e pf when it is probed.

v2:
* use a more specific subject for this patch.
* delete modifications that are not relevant.
* free memory allocted by strdup.
* delete unnecessary initializations.
* name function more precisely.
* wrap relevant code to a function to avoid too many levels of block
* nesting.

 doc/guides/nics/i40e.rst       |  11 +++
 drivers/net/i40e/i40e_ethdev.c | 166 ++++++++++++++++++++++++++++++++++++++++-
 drivers/net/i40e/i40e_ethdev.h |   6 ++
 3 files changed, 182 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/i40e.rst b/doc/guides/nics/i40e.rst
index a97484c..8d88f0d 100644
--- a/doc/guides/nics/i40e.rst
+++ b/doc/guides/nics/i40e.rst
@@ -513,6 +513,17 @@ details please refer to :doc:`../testpmd_app_ug/index`.
    testpmd> set port (port_id) queue-region flush (on|off)
    testpmd> show port (port_id) queue-region
 
+Switch Mode
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The Intel® Ethernet 700 Series can be component of IPN3KE device, in which the
+Intel® Ethernet 700 Series is connected to other pci device. This pci device
+works as switch device via which the Intel® Ethernet 700 Series get link status
+from. For example: the Intel® Ethernet 700 Series 0000:b1:00.0 is connected to
+an IPN3KE device whose BDF is b3:00.0. In this case 0000:b1:00.0 use representor
+0 on b3:00.0.
+
+   ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x3 -n 4 -w 0000:b1:00.0,switch_mode=IPN3KE_0@b3:00.0_0
+
 Limitations or Known issues
 ---------------------------
 
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 2b9fc45..4b5f0bb 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -44,6 +44,7 @@
 #define ETH_I40E_SUPPORT_MULTI_DRIVER	"support-multi-driver"
 #define ETH_I40E_QUEUE_NUM_PER_VF_ARG	"queue-num-per-vf"
 #define ETH_I40E_USE_LATEST_VEC	"use-latest-supported-vec"
+#define ETH_I40E_SWITCH_MODE_ARG	"switch_mode"
 
 #define I40E_CLEAR_PXE_WAIT_MS     200
 
@@ -406,6 +407,7 @@ static int i40e_sw_tunnel_filter_insert(struct i40e_pf *pf,
 	ETH_I40E_SUPPORT_MULTI_DRIVER,
 	ETH_I40E_QUEUE_NUM_PER_VF_ARG,
 	ETH_I40E_USE_LATEST_VEC,
+	ETH_I40E_SWITCH_MODE_ARG,
 	NULL};
 
 static const struct rte_pci_id pci_id_i40e_map[] = {
@@ -1260,6 +1262,115 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 #define I40E_ALARM_INTERVAL 50000 /* us */
 
 static int
+i40e_pf_parse_switch_mode(const char *key __rte_unused,
+	const char *value, void *extra_args)
+{
+	if (!value || !extra_args)
+		return -EINVAL;
+
+	if (strlen(value) < RTE_ETH_NAME_MAX_LEN) {
+		rte_memcpy(extra_args, value, strlen(value) + 1);
+	} else {
+		PMD_DRV_LOG(ERR,
+			"switch_mode args should be less than %d characters",
+			RTE_ETH_NAME_MAX_LEN);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct rte_eth_dev *
+i40e_eth_dev_get_by_switch_mode_name(const char *cfg_str)
+{
+	char switch_name[RTE_ETH_NAME_MAX_LEN];
+	char port_name[RTE_ETH_NAME_MAX_LEN];
+	char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN];
+	uint16_t port_id;
+	const char *p_src;
+	char *p_dst;
+	int ret;
+
+	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */
+	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
+		p_src = cfg_str;
+		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
+
+		/* move over "IPN3KE" */
+		while ((*p_src != '_') && (*p_src))
+			p_src++;
+
+		/* move over the first underline */
+		p_src++;
+
+		p_dst = switch_name;
+		while ((*p_src != '_') && (*p_src)) {
+			if (*p_src == '@') {
+				*p_dst++ = '|';
+				p_src++;
+			} else {
+				*p_dst++ = *p_src++;
+			}
+		}
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
+
+		/* move over the second underline */
+		p_src++;
+
+		p_dst = port_name;
+		while (*p_src)
+			*p_dst++ = *p_src++;
+		*p_dst = 0;
+		PMD_DRV_LOG(DEBUG, "port_name is %s", port_name);
+
+		snprintf(switch_ethdev_name, sizeof(switch_ethdev_name),
+			"net_%s_representor_%s", switch_name, port_name);
+		PMD_DRV_LOG(DEBUG, "switch_ethdev_name is %s",
+			switch_ethdev_name);
+
+		ret = rte_eth_dev_get_port_by_name(switch_ethdev_name,
+			&port_id);
+		if (!ret)
+			return &rte_eth_devices[port_id];
+	}
+
+	return NULL;
+}
+
+static void
+eth_i40e_pf_switch_dev_init(struct rte_eth_dev *dev)
+{
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_devargs *devargs;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	pf->switch_ethdev_support_flag = 0;
+	pf->switch_ethdev = NULL;
+
+	devargs = pci_dev->device.devargs;
+	if (!devargs)
+		return;
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			pf->switch_ethdev_support_flag = 1;
+
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				pf->switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+}
+
+static int
 eth_i40e_dev_init(struct rte_eth_dev *dev, void *init_params __rte_unused)
 {
 	struct rte_pci_device *pci_dev;
@@ -1582,6 +1693,11 @@ static inline void i40e_config_automask(struct i40e_pf *pf)
 	memset(&pf->rss_info, 0,
 		sizeof(struct i40e_rte_flow_rss_conf));
 
+	/* parse the switch device from devargs, try to bind to the switch
+	 * device, if binding not succeed, bind again when i40e_dev_link_update
+	 */
+	eth_i40e_pf_switch_dev_init(dev);
+
 	/* reset all stats of the device, including pf and main vsi */
 	i40e_dev_stats_reset(dev);
 
@@ -2779,13 +2895,52 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	}
 }
 
+static void
+i40e_pf_linkstatus_get_from_switch_ethdev
+(struct rte_eth_dev *switch_ethdev, struct rte_eth_link *link)
+{
+	if (switch_ethdev) {
+		rte_eth_linkstatus_get(switch_ethdev, link);
+	} else {
+		link->link_autoneg = ETH_LINK_SPEED_FIXED;
+		link->link_duplex  = ETH_LINK_FULL_DUPLEX;
+		link->link_speed   = ETH_SPEED_NUM_25G;
+		link->link_status  = 0;
+	}
+}
+
+static struct rte_eth_dev *
+i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs)
+{
+	struct rte_kvargs *kvlist = NULL;
+	struct rte_eth_dev *switch_ethdev = NULL;
+	char switch_cfg_str[RTE_ETH_NAME_MAX_LEN];
+
+	kvlist = rte_kvargs_parse(devargs->args, valid_keys);
+	if (kvlist) {
+		if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) == 1) {
+			if (!rte_kvargs_process(kvlist,
+				ETH_I40E_SWITCH_MODE_ARG,
+				&i40e_pf_parse_switch_mode, switch_cfg_str)) {
+				switch_ethdev =
+					i40e_eth_dev_get_by_switch_mode_name
+					(switch_cfg_str);
+			}
+		}
+		rte_kvargs_free(kvlist);
+	}
+	return switch_ethdev;
+}
+
 int
 i40e_dev_link_update(struct rte_eth_dev *dev,
 		     int wait_to_complete)
 {
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
 	struct rte_eth_link link;
 	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	int ret;
 
 	memset(&link, 0, sizeof(link));
@@ -2800,6 +2955,14 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw)
 	else
 		update_link_aq(hw, &link, enable_lse, wait_to_complete);
 
+	if (pf->switch_ethdev_support_flag) {
+		if (!pf->switch_ethdev)
+			pf->switch_ethdev = i40e_get_switch_ethdev_from_devargs
+				(pci_dev->device.devargs);
+		i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev,
+			&link);
+	}
+
 	ret = rte_eth_linkstatus_set(dev, &link);
 	i40e_notify_all_vfs_link_status(dev);
 
@@ -12773,4 +12936,5 @@ struct i40e_customized_pctype*
 			      ETH_I40E_FLOATING_VEB_LIST_ARG "=<string>"
 			      ETH_I40E_QUEUE_NUM_PER_VF_ARG "=1|2|4|8|16"
 			      ETH_I40E_SUPPORT_MULTI_DRIVER "=1"
-			      ETH_I40E_USE_LATEST_VEC "=0|1");
+			      ETH_I40E_USE_LATEST_VEC "=0|1"
+			      ETH_I40E_SWITCH_MODE_ARG "=IPN3KE");
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 38ac3ea..cfdcecb 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -975,6 +975,12 @@ struct i40e_pf {
 	struct i40e_customized_pctype customized_pctype[I40E_CUSTOMIZED_MAX];
 	/* Switch Domain Id */
 	uint16_t switch_domain_id;
+
+	/* flag indicating if switch mode is required like other devargs */
+	bool switch_ethdev_support_flag;
+
+	/* point to switch_ethdev specific by "switch_mode" in devargs */
+	struct rte_eth_dev *switch_ethdev;
 };
 
 enum pending_msg {
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v7] net/i40e: i40e get link status update from ipn3ke

[Zhang, Qi Z]

> -----Original Message-----
> From: Pei, Andy
> Sent: Thursday, July 11, 2019 2:39 PM
> To: dev@dpdk.org
> Cc: Pei, Andy <andy.pei@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu,
> Jingjing <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yigit,
> Ferruh <ferruh.yigit@intel.com>; Xu, Rosen <rosen.xu@intel.com>; Ye,
> Xiaolong <xiaolong.ye@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; stable@dpdk.org
> Subject: [PATCH v7] net/i40e: i40e get link status update from ipn3ke
> 
> Add switch_mode argument for i40e PF to specify the specific FPGA that i40e
> PF is connected to. i40e PF get link status update via the connected FPGA.
> Add bool switch_ethdev_support_flag to struct i40e_pf to specify if there are
> switch_mode argues in cmd.
> Add switch_ethdev to struct i40e_pf to track the bind switch device.
> Try to bind i40e pf to switch device when i40e device is initialized.
> If it fail to find correct switch device, bind will occur again when update i40e
> device link status.
> 
> Signed-off-by: Andy Pei <andy.pei@intel.com>
....
> +	/* An example of cfg_str is "IPN3KE_0@b3:00.0_0" */

My understanding, in above example you want to have

Swtich_name = "0|b3:00.0" and port_name = "0", right?

> +	if (!strncmp(cfg_str, "IPN3KE", strlen("IPN3KE"))) {
> +		p_src = cfg_str;
> +		PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str);
> +
> +		/* move over "IPN3KE" */
> +		while ((*p_src != '_') && (*p_src))
> +			p_src++;

p_src = strchr(p_src, '_') should do the same thing?

> +
> +		/* move over the first underline */
> +		p_src++;

What if *p_src == null ?, error handle needed.

> +
> +		p_dst = switch_name;
> +		while ((*p_src != '_') && (*p_src)) {
> +			if (*p_src == '@') {
> +				*p_dst++ = '|';

Are you replace '@' to '|"? why not define the expected argument as "IPN3KE_0|b3:00.0_0", so can simply do strncpy here.

> +				p_src++;
> +			} else {
> +				*p_dst++ = *p_src++;
> +			}
> +		}
> +		*p_dst = 0;
> +		PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name);
> +
> +		/* move over the second underline */
> +		p_src++;

Same as above, error handle.

> +
> +		p_dst = port_name;
> +		while (*p_src)
> +			*p_dst++ = *p_src++;
> +		*p_dst = 0;

Use strncpy.

...