From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id F31327D4E for ; Fri, 29 Sep 2017 06:54:49 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Sep 2017 21:54:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,451,1500966000"; d="scan'208";a="156855337" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 28 Sep 2017 21:54:49 -0700 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 28 Sep 2017 21:54:48 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx109.amr.corp.intel.com (10.18.116.9) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 28 Sep 2017 21:54:48 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.152]) with mapi id 14.03.0319.002; Fri, 29 Sep 2017 12:54:46 +0800 From: "Wu, Jingjing" To: "Zhao1, Wei" , "dev@dpdk.org" CC: "Zhao1, Wei" Thread-Topic: [dpdk-dev] [PATCH v5 1/3] net/i40e: queue region set and flush Thread-Index: AQHTONBNKrWmBElULk6iKEcSQ791YKLLOZMg Date: Fri, 29 Sep 2017 04:54:46 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F810E834FF@SHSMSX103.ccr.corp.intel.com> References: <1506589809-59533-1-git-send-email-wei.zhao1@intel.com> <1506653786-28970-1-git-send-email-wei.zhao1@intel.com> <1506653786-28970-2-git-send-email-wei.zhao1@intel.com> In-Reply-To: <1506653786-28970-2-git-send-email-wei.zhao1@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzNlM2I0MDktMTU0ZC00MjczLThmMWYtMDYzYThmZDZiOTEyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Im9iWThBQW1HMUJkcFQyNHhHMjZOZExsVUZKODBDK1YrRGtsYTV5VHBweVE9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 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 v5 1/3] net/i40e: queue region set and flush 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: , X-List-Received-Date: Fri, 29 Sep 2017 04:54:50 -0000 > +static int > +i40e_vsi_update_queue_region_mapping(struct i40e_hw *hw, > + struct i40e_pf *pf) > +{ > + uint16_t i; > + struct i40e_vsi *vsi =3D pf->main_vsi; > + uint16_t queue_offset, bsf, tc_index; > + struct i40e_vsi_context ctxt; > + struct i40e_aqc_vsi_properties_data *vsi_info; > + struct i40e_queue_region_info *region_info =3D > + &pf->queue_region; > + int32_t ret =3D -EINVAL; > + > + if (!region_info->queue_region_number) { > + PMD_INIT_LOG(ERR, "there is no that region id been set before"); > + return ret; > + } > + > + memset(&ctxt, 0, sizeof(struct i40e_vsi_context)); > + > + /* Update Queue Pairs Mapping for currently enabled UPs */ > + ctxt.seid =3D vsi->seid; > + ctxt.pf_num =3D hw->pf_id; > + ctxt.vf_num =3D 0; > + ctxt.uplink_seid =3D vsi->uplink_seid; > + ctxt.info =3D vsi->info; > + vsi_info =3D &ctxt.info; > + > + memset(vsi_info->tc_mapping, 0, sizeof(uint16_t) * 8); > + memset(vsi_info->queue_mapping, 0, sizeof(uint16_t) * 16); > + > + /** > + * Configure queue region and queue mapping parameters, > + * for enabled queue region, allocate queues to this region. > + */ > + > + for (i =3D 0; i < region_info->queue_region_number; i++) { > + tc_index =3D region_info->region[i].region_id; > + bsf =3D rte_bsf32(region_info->region[i].queue_num); > + queue_offset =3D region_info->region[i].queue_start_index; > + vsi_info->tc_mapping[tc_index] =3D rte_cpu_to_le_16( > + (queue_offset << I40E_AQ_VSI_TC_QUE_OFFSET_SHIFT) | > + (bsf << I40E_AQ_VSI_TC_QUE_NUMBER_SHIFT)); > + } > + > + /* Associate queue number with VSI, Keep vsi->nb_qps unchanged */ > + if (vsi->type =3D=3D I40E_VSI_SRIOV) { You already assign the vsi to main_vsi in the beginning of the function, so It is impossible here the type is SRIOV. I think I already commented it in = your v3 patch set? =20 [......] > +static int > +i40e_queue_region_set_region(struct i40e_pf *pf, > + struct rte_i40e_rss_region_conf *conf_ptr) > +{ > + uint16_t i; > + struct i40e_vsi *main_vsi =3D pf->main_vsi; > + struct i40e_queue_region_info *info =3D &pf->queue_region; > + int32_t ret =3D -EINVAL; > + > + if (!((rte_is_power_of_2(conf_ptr->queue_num)) && > + conf_ptr->queue_num <=3D 64)) { > + PMD_DRV_LOG(ERR, "The region sizes should be any of the following valu= es: 1, > 2, 4, 8, 16, 32, 64 as long as the " > + "total number of queues do not exceed the VSI allocation"); > + return ret; > + } > + > + if (conf_ptr->region_id > I40E_REGION_MAX_INDEX) { > + PMD_DRV_LOG(ERR, "the queue region max index is 7"); > + return ret; > + } > + > + if ((conf_ptr->queue_start_index + conf_ptr->queue_num) > + > main_vsi->nb_used_qps) { > + PMD_DRV_LOG(ERR, "the queue index exceeds the VSI range"); > + return ret; > + } > + You are using nb_used_qps for the comparison, is that to say the function Is supposed to be called after dev_start? > + for (i =3D 0; i < info->queue_region_number; i++) > + if (conf_ptr->region_id =3D=3D info->region[i].region_id) > + break; > + > + if (i =3D=3D info->queue_region_number && > + i <=3D I40E_REGION_MAX_INDEX) { > + info->region[i].region_id =3D conf_ptr->region_id; > + info->region[i].queue_num =3D conf_ptr->queue_num; > + info->region[i].queue_start_index =3D > + conf_ptr->queue_start_index; > + info->queue_region_number++; > + } else { > + PMD_DRV_LOG(ERR, "queue region number exceeds maxnum 8 or the queue > region id has been set before"); > + return ret; > + } > + > + return 0; > +} > + > +static int > +i40e_queue_region_set_flowtype(struct i40e_pf *pf, > + struct rte_i40e_rss_region_conf *rss_region_conf) > +{ > + int32_t ret =3D -EINVAL; > + struct i40e_queue_region_info *info =3D &pf->queue_region; > + uint16_t i, j, flowtype_set =3D 0; > + uint16_t region_index, flowtype_index; > + > + /** > + * For the pctype or hardware flowtype of packet, > + * the specific index for each type has been defined > + * in file i40e_type.h as enum i40e_filter_pctype. > + */ > + Are you meeing the type of hw_flowtype in rss_region_conf Is defined in i40e_type.h? > + if (rss_region_conf->region_id > I40E_PFQF_HREGION_MAX_INDEX) { > + PMD_DRV_LOG(ERR, "the queue region max index is 7"); > + return ret; > + } > + > + if (rss_region_conf->hw_flowtype >=3D I40E_FILTER_PCTYPE_MAX) { > + PMD_DRV_LOG(ERR, "the hw_flowtype or PCTYPE max index is 63"); > + return ret; > + } > + > + > + for (i =3D 0; i < info->queue_region_number; i++) > + if (rss_region_conf->region_id =3D=3D info->region[i].region_id) > + break; > + > + if (i =3D=3D info->queue_region_number) { > + PMD_DRV_LOG(ERR, "that region id has not been set before"); > + ret =3D -ENODATA; > + return ret; > + } > + region_index =3D i; > + > + for (i =3D 0; i < info->queue_region_number; i++) { > + for (j =3D 0; j < info->region[i].flowtype_num; j++) { > + if (rss_region_conf->hw_flowtype =3D=3D > + info->region[i].hw_flowtype[j]) { > + flowtype_set =3D 1; > + break; You can just return here? > + } > + } > + } > + [......] > +static int > +i40e_queue_region_set_user_priority(struct i40e_pf *pf, > + struct rte_i40e_rss_region_conf *rss_region_conf) > +{ > + struct i40e_queue_region_info *info =3D &pf->queue_region; > + int32_t ret =3D -EINVAL; > + uint16_t i, j, region_index, user_priority_set =3D 0; > + > + if (rss_region_conf->user_priority >=3D I40E_MAX_USER_PRIORITY) { > + PMD_DRV_LOG(ERR, "the queue region max index is 7"); > + return ret; > + } > + > + if (rss_region_conf->region_id >=3D I40E_REGION_MAX_INDEX) { > + PMD_DRV_LOG(ERR, "the region_id max index is 7"); > + return ret; > + } > + > + for (i =3D 0; i < info->queue_region_number; i++) > + if (rss_region_conf->region_id =3D=3D info->region[i].region_id) > + break; > + > + if (i =3D=3D info->queue_region_number) { > + PMD_DRV_LOG(ERR, "that region id has not been set before"); > + ret =3D -ENODATA; > + return ret; > + } > + region_index =3D i; > + > + for (i =3D 0; i < info->queue_region_number; i++) { > + for (j =3D 0; j < info->region[i].user_priority_num; j++) { > + if (info->region[i].user_priority[j] =3D=3D > + rss_region_conf->user_priority) { > + user_priority_set =3D 1; The same, you can return here. > + break; [......]=20 > + > +static int > +i40e_queue_region_dcb_configure(struct i40e_hw *hw, > + struct i40e_pf *pf) > +{ > + struct i40e_dcbx_config dcb_cfg_local; > + struct i40e_dcbx_config *dcb_cfg; > + struct i40e_queue_region_info *info =3D &pf->queue_region; > + struct i40e_dcbx_config *old_cfg =3D &hw->local_dcbx_config; > + int32_t ret =3D -EINVAL; > + uint16_t i, j, prio_index, region_index; > + uint8_t tc_map, tc_bw, bw_lf; > + > + if (!info->queue_region_number) { > + PMD_DRV_LOG(ERR, "there is no that region id been set before"); > + return ret; > + } > + > + dcb_cfg =3D &dcb_cfg_local; > + memset(dcb_cfg, 0, sizeof(struct i40e_dcbx_config)); > + > + /* assume each tc has the same bw */ > + tc_bw =3D I40E_MAX_PERCENT / info->queue_region_number; > + for (i =3D 0; i < info->queue_region_number; i++) > + dcb_cfg->etscfg.tcbwtable[i] =3D tc_bw; > + /* to ensure the sum of tcbw is equal to 100 */ > + bw_lf =3D I40E_MAX_PERCENT % info->queue_region_number; > + for (i =3D 0; i < bw_lf; i++) > + dcb_cfg->etscfg.tcbwtable[i]++; > + > + /* assume each tc has the same Transmission Selection Algorithm */ > + for (i =3D 0; i < info->queue_region_number; i++) > + dcb_cfg->etscfg.tsatable[i] =3D I40E_IEEE_TSA_ETS; > + > + for (i =3D 0; i < info->queue_region_number; i++) { > + for (j =3D 0; j < info->region[i].user_priority_num; j++) { > + prio_index =3D info->region[i].user_priority[j]; > + region_index =3D info->region[i].region_id; > + dcb_cfg->etscfg.prioritytable[prio_index] =3D > + region_index; > + } > + } > + > + /* FW needs one App to configure HW */ > + dcb_cfg->numapps =3D I40E_DEFAULT_DCB_APP_NUM; > + dcb_cfg->app[0].selector =3D I40E_APP_SEL_ETHTYPE; > + dcb_cfg->app[0].priority =3D I40E_DEFAULT_DCB_APP_PRIO; > + dcb_cfg->app[0].protocolid =3D I40E_APP_PROTOID_FCOE; > + > + tc_map =3D RTE_LEN2MASK(info->queue_region_number, uint8_t); > + > + dcb_cfg->pfc.willing =3D 0; > + dcb_cfg->pfc.pfccap =3D I40E_MAX_TRAFFIC_CLASS; > + dcb_cfg->pfc.pfcenable =3D tc_map; > + > + /* Copy the new config to the current config */ > + *old_cfg =3D *dcb_cfg; > + old_cfg->etsrec =3D old_cfg->etscfg; > + ret =3D i40e_set_dcb_config(hw); > + > + if (ret) { > + PMD_DRV_LOG(ERR, "Set queue region DCB Config failed, err %s aq_err %s= ", > + i40e_stat_str(hw, ret), > + i40e_aq_str(hw, hw->aq.asq_last_status)); > + return ret; > + } > + > + return 0; > +} > + > +int > +i40e_flush_queue_region_all_conf(struct rte_eth_dev *dev, > + struct i40e_hw *hw, struct i40e_pf *pf, uint16_t on) > +{ > + int32_t ret =3D -EINVAL; > + struct i40e_queue_region_info *info =3D &pf->queue_region; > + > + if (on) { > + i40e_queue_region_pf_flowtype_conf(hw, pf); > + > + ret =3D i40e_vsi_update_queue_region_mapping(hw, pf); > + if (ret !=3D I40E_SUCCESS) { > + PMD_DRV_LOG(INFO, "Failed to flush queue region mapping."); > + return ret; > + } > + > + ret =3D i40e_queue_region_dcb_configure(hw, pf); > + if (ret !=3D I40E_SUCCESS) { > + PMD_DRV_LOG(INFO, "Failed to flush dcb."); > + return ret; > + } > + > + return 0; > + } > + > + info->queue_region_number =3D 1; > + info->region[0].queue_num =3D 64; > + info->region[0].queue_start_index =3D 0; > + > + ret =3D i40e_vsi_update_queue_region_mapping(hw, pf); > + if (ret !=3D I40E_SUCCESS) > + PMD_DRV_LOG(INFO, "Failed to flush queue region mapping."); > + > + ret =3D i40e_dcb_init_configure(dev, TRUE); > + if (ret !=3D I40E_SUCCESS) { > + PMD_DRV_LOG(INFO, "Failed to flush dcb."); > + pf->flags &=3D ~I40E_FLAG_DCB; > + } > + > + i40e_init_queue_region_conf(dev); > + > + return 0; > +} > + > +static int > +i40e_queue_region_pf_check_rss(struct i40e_pf *pf) > +{ > + struct i40e_hw *hw =3D I40E_PF_TO_HW(pf); > + uint64_t hena; > + > + hena =3D (uint64_t)i40e_read_rx_ctl(hw, I40E_PFQF_HENA(0)); > + hena |=3D ((uint64_t)i40e_read_rx_ctl(hw, I40E_PFQF_HENA(1))) << 32; > + if (hw->mac.type =3D=3D I40E_MAC_X722) > + hena &=3D I40E_RSS_HENA_ALL_X722; > + else > + hena &=3D I40E_RSS_HENA_ALL; > + > + if (!hena) > + return -ENOTSUP; > + Why made such change? Will it be impacted by kiril's pctype patches and bei= lei's new pctype patches? > + return 0; > +} > + > +static void > +i40e_queue_region_get_all_info(struct i40e_pf *pf, uint16_t port_id) > +{ In this function, it looks like you print all the information about queue r= egion? If so, You can use "display" instead of "get". In my opinion, generally, we are getting info but not display info in API l= evel. [......] > + > +/** > + * Option types of queue region. > + */ > +enum rte_pmd_i40e_queue_region_op { > + RTE_PMD_I40E_REGION_UNDEFINED, > + RTE_PMD_I40E_QUEUE_REGION_SET, /**< add queue region set */ > + RTE_PMD_I40E_REGION_FLOWTYPE_SET, /**< add pf region pctype set */ > + /**< add queue region user priority set */ If the comments is added before the definition, you need to use /** xxx */ = instead of /**< XX */ for doxygen-likely format you can check http://www.dpdk.org/doc/guides/cont= ributing/documentation.html#doxygen-guidelines Thanks Jingjing