From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id DD47FA046B for ; Thu, 27 Jun 2019 03:20:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0ECC92BA5; Thu, 27 Jun 2019 03:20:58 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id A2C8E28EE for ; Thu, 27 Jun 2019 03:20:56 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jun 2019 18:20:55 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,421,1557212400"; d="scan'208";a="172944437" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga002.jf.intel.com with ESMTP; 26 Jun 2019 18:20:55 -0700 Received: from fmsmsx125.amr.corp.intel.com (10.18.125.40) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 26 Jun 2019 18:20:55 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX125.amr.corp.intel.com (10.18.125.40) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 26 Jun 2019 18:20:55 -0700 Received: from shsmsx105.ccr.corp.intel.com ([169.254.11.72]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.246]) with mapi id 14.03.0439.000; Thu, 27 Jun 2019 09:20:52 +0800 From: "Pei, Andy" To: "Ye, Xiaolong" CC: "dev@dpdk.org" , "Zhang, Roy Fan" , "Zhang, Qi Z" , "Wu, Jingjing" , "Xing, Beilei" , "Yigit, Ferruh" , "Xu, Rosen" Thread-Topic: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke Thread-Index: AQHVK/xbJymgP1zkkUmNW4oq4Ovu1KautP1Q Date: Thu, 27 Jun 2019 01:20:51 +0000 Message-ID: <5941F446C088714A85408FA3132CFCBB010388A7@SHSMSX105.ccr.corp.intel.com> References: <1558602875-429451-1-git-send-email-andy.pei@intel.com> <20190626153305.GB99403@intel.com> In-Reply-To: <20190626153305.GB99403@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/i40e: i40e rework for ipn3ke X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi, Xiaolong OK, I will do this in V2. Thanks. -----Original Message----- From: Ye, Xiaolong=20 Sent: Wednesday, June 26, 2019 11:33 PM To: Pei, Andy Cc: dev@dpdk.org; Zhang, Roy Fan ; Zhang, Qi Z ; Wu, Jingjing ; Xing, Beilei ; Yigit, Ferruh ; Xu, Rosen 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=20 >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 histor= y, you can move the cc block after `---` mark, while all those names will s= till be picked-up and cc'ed by git send-email. > >Signed-off-by: Andy Pei >--- > drivers/net/i40e/i40e_ethdev.c | 128=20 >+++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 122 insertions(+), 6 deletions(-) > >diff --git a/drivers/net/i40e/i40e_ethdev.c=20 >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" >=20 >-#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" >=20 > #define I40E_CLEAR_PXE_WAIT_MS 200 >=20 >@@ -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}; >=20 > static const struct rte_pci_id pci_id_i40e_map[] =3D { @@ -2784,6=20 >+2786,80 @@ void i40e_flex_payload_reg_set_default(struct i40e_hw *hw) > } > } >=20 >+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 =3D 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] =3D {0}; >+ char port_name[RTE_ETH_NAME_MAX_LEN] =3D {0}; >+ char switch_ethdev_name[RTE_ETH_NAME_MAX_LEN] =3D {0}; Above initializations are not needed. >+ uint16_t port_id; >+ const char *p_src; >+ char *p_dst; >+ int ret =3D -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 =3D cfg_str; >+ PMD_DRV_LOG(DEBUG, "cfg_str is %s", cfg_str); >+ >+ /* move over "IPN3KE" */ >+ while ((*p_src !=3D '_') && (*p_src)) >+ p_src++; >+ >+ /* move over the first underline */ >+ p_src++; >+ >+ p_dst =3D switch_name; >+ while ((*p_src !=3D '_') && (*p_src)) { >+ if (*p_src =3D=3D '@') { >+ *p_dst++ =3D '|'; >+ p_src++; >+ } else >+ *p_dst++ =3D *p_src++; >+ } >+ *p_dst =3D 0; >+ PMD_DRV_LOG(DEBUG, "switch_name is %s", switch_name); >+ >+ /* move over the second underline */ >+ p_src++; >+ >+ p_dst =3D port_name; >+ while (*p_src) >+ *p_dst++ =3D *p_src++; >+ *p_dst =3D 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 =3D rte_eth_dev_get_port_by_name(switch_ethdev_name, >+ &port_id); >+ if (ret) >+ *switch_ethdev =3D NULL; >+ else >+ *switch_ethdev =3D &rte_eth_devices[port_id]; >+ } else >+ *switch_ethdev =3D 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 =3D dev->data->dev_conf.intr_conf.lsc ? true : false; > int ret; >+ struct rte_devargs *devargs; >+ struct rte_kvargs *kvlist =3D NULL; >+ struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); >+ struct rte_eth_dev *switch_ethdev; >+ char *switch_cfg_str =3D NULL; >=20 > memset(&link, 0, sizeof(link)); >=20 >@@ -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); >=20 >+ devargs =3D pci_dev->device.devargs; >+ if (devargs) { >+ kvlist =3D rte_kvargs_parse(devargs->args, valid_keys); >+ if (kvlist !=3D NULL) { >+ if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) >+ =3D=3D 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 =3D >+ ETH_LINK_FULL_DUPLEX; >+ link.link_autoneg =3D >+ ETH_LINK_SPEED_FIXED; >+ link.link_speed =3D >+ ETH_SPEED_NUM_25G; >+ link.link_status =3D 0; >+ } >+ } >+ } >+ rte_kvargs_free(kvlist); >+ } >+ } >+ Better to wrap above code to a function to avoid too many levels of block n= esting. Thanks, Xiaolong > ret =3D rte_eth_linkstatus_set(dev, &link); > i40e_notify_all_vfs_link_status(dev); >=20 >@@ -12790,4 +12905,5 @@ struct i40e_customized_pctype* > ETH_I40E_FLOATING_VEB_LIST_ARG "=3D" > ETH_I40E_QUEUE_NUM_PER_VF_ARG "=3D1|2|4|8|16" > ETH_I40E_SUPPORT_MULTI_DRIVER "=3D1" >- ETH_I40E_USE_LATEST_VEC "=3D0|1"); >+ ETH_I40E_USE_LATEST_VEC "=3D0|1" >+ ETH_I40E_SWITCH_MODE_ARG "=3DIPN3KE"); >-- >1.8.3.1 >