net/i40e: i40e rework for ipn3ke
Checks
Commit Message
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(-)
Comments
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
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(-)
>>
<...>
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(-)
>>
<...>
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
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>
+ 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>
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
>
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
>
@@ -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");