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 31CE9A00E6 for ; Thu, 11 Jul 2019 07:56:36 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DDD53322C; Thu, 11 Jul 2019 07:56:34 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B2BA631FC; Thu, 11 Jul 2019 07:56:32 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jul 2019 22:56:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,476,1557212400"; d="scan'208";a="166293029" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga008.fm.intel.com with ESMTP; 10 Jul 2019 22:56:31 -0700 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 10 Jul 2019 22:56:31 -0700 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 10 Jul 2019 22:56:28 -0700 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 10 Jul 2019 22:56:27 -0700 Received: from shsmsx105.ccr.corp.intel.com ([169.254.11.232]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.60]) with mapi id 14.03.0439.000; Thu, 11 Jul 2019 13:56:25 +0800 From: "Pei, Andy" To: "Zhang, Qi Z" , "dev@dpdk.org" CC: "Wu, Jingjing" , "Xing, Beilei" , "Yigit, Ferruh" , "Xu, Rosen" , "Ye, Xiaolong" , "Zhang, Roy Fan" , "stable@dpdk.org" Thread-Topic: [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3ke Thread-Index: AQHVNwSiyeLz6rGP0UisM8sPCCPzHabEzWxggAAbsSA= Date: Thu, 11 Jul 2019 05:56:25 +0000 Message-ID: <5941F446C088714A85408FA3132CFCBB010471D0@SHSMSX105.ccr.corp.intel.com> References: <1562654606-175783-1-git-send-email-andy.pei@intel.com> <1562751666-435953-1-git-send-email-andy.pei@intel.com> <039ED4275CED7440929022BC67E7061153D64E1C@SHSMSX105.ccr.corp.intel.com> In-Reply-To: <039ED4275CED7440929022BC67E7061153D64E1C@SHSMSX105.ccr.corp.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 v6 1/2] net/i40e: i40e get link status update from 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, Qi Reply inline. -----Original Message----- From: Zhang, Qi Z=20 Sent: Thursday, July 11, 2019 12:37 PM To: Pei, Andy ; dev@dpdk.org Cc: Wu, Jingjing ; Xing, Beilei ; Yigit, Ferruh ; Xu, Rosen = ; Ye, Xiaolong ; Zhang, Roy Fan ; stable@dpdk.org Subject: RE: [PATCH v6 1/2] net/i40e: i40e get link status update from ipn3= ke > -----Original Message----- > From: Pei, Andy > Sent: Wednesday, July 10, 2019 5:41 PM > To: dev@dpdk.org > Cc: Pei, Andy ; Zhang, Qi Z=20 > ; Wu, Jingjing ; Xing,=20 > Beilei ; Yigit, Ferruh=20 > ; Xu, Rosen ; Ye, Xiaolong=20 > ; Zhang, Roy Fan ;=20 > stable@dpdk.org > Subject: [PATCH v6 1/2] net/i40e: i40e get link status update from=20 > ipn3ke You can merge patch 2/2 with this patch. OK ... >=20 > +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 =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) > + 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 =3DI40E_DEV_PRIVAT= E_TO_PF(dev->data->dev_private);=20 OK > + char switch_cfg_str[RTE_ETH_NAME_MAX_LEN]; > + struct rte_kvargs *kvlist =3D NULL; > + struct rte_devargs *devargs; > + struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); > + > + pf->switch_ethdev_support_flag =3D 0; > + pf->switch_ethdev =3D NULL; > + > + devargs =3D pci_dev->device.devargs; > + if (!devargs) > + return; > + > + kvlist =3D rte_kvargs_parse(devargs->args, valid_keys); > + if (kvlist) { > + if (rte_kvargs_count(kvlist, ETH_I40E_SWITCH_MODE_ARG) =3D=3D 1) { > + pf->switch_ethdev_support_flag =3D 1; > + > + if (!rte_kvargs_process(kvlist, > + ETH_I40E_SWITCH_MODE_ARG, > + &i40e_pf_parse_switch_mode, switch_cfg_str)) { > + pf->switch_ethdev =3D > + 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 =3D 0; > + pf->switch_ethdev =3D NULL; > +} This function is not necessary, after dev_init must be called after dev_uni= t, 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=20 > __rte_unused) { > struct rte_pci_device *pci_dev; > @@ -1582,6 +1701,11 @@ static inline void i40e_config_automask(struct=20 > i40e_pf *pf) > memset(&pf->rss_info, 0, > sizeof(struct i40e_rte_flow_rss_conf)); >=20 > + /* 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); >=20 > @@ -1762,6 +1886,8 @@ void i40e_flex_payload_reg_set_default(struct > i40e_hw *hw) > rte_free(p_flow); > } >=20 > + eth_i40e_pf_switch_dev_uninit(pf); > + > /* Remove all Traffic Manager configuration */ > i40e_tm_conf_uninit(dev); >=20 > @@ -2779,13 +2905,52 @@ void i40e_flex_payload_reg_set_default(struct > i40e_hw *hw) > } > } >=20 > +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 =3D ETH_LINK_SPEED_FIXED; > + link->link_duplex =3D ETH_LINK_FULL_DUPLEX; > + link->link_speed =3D ETH_SPEED_NUM_25G; > + link->link_status =3D 0; > + } > +} > + > +static struct rte_eth_dev * > +i40e_get_switch_ethdev_from_devargs(struct rte_devargs *devargs) { > + struct rte_kvargs *kvlist =3D NULL; > + struct rte_eth_dev *switch_ethdev =3D NULL; > + char switch_cfg_str[RTE_ETH_NAME_MAX_LEN]; > + > + kvlist =3D rte_kvargs_parse(devargs->args, valid_keys); > + if (kvlist) { > + 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)) { > + switch_ethdev =3D > + 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 =3D > I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + struct i40e_pf *pf =3D I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private); > struct rte_eth_link link; > bool enable_lse =3D dev->data->dev_conf.intr_conf.lsc ? true : false; > + struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); > int ret; >=20 > 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); >=20 > + 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 nu= ll, right? Switch device may not be probed when i40e device is being probed. In this c= ase, 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 =3D > + i40e_get_switch_ethdev_from_devargs > + (pci_dev->device.devargs); > + } > + i40e_pf_linkstatus_get_from_switch_ethdev(pf->switch_ethdev, > + &link); > + } > + > ret =3D rte_eth_linkstatus_set(dev, &link); > i40e_notify_all_vfs_link_status(dev); >=20 > @@ -12773,4 +12949,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"); > diff --git a/drivers/net/i40e/i40e_ethdev.h=20 > 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; > }; >=20 > enum pending_msg { > -- > 1.8.3.1