From patchwork Tue May 8 12:34:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wei Dai X-Patchwork-Id: 39553 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 79A9C1B019; Wed, 9 May 2018 12:27:39 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id B05428E56 for ; Tue, 8 May 2018 14:34:50 +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 orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 May 2018 05:34:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,378,1520924400"; d="scan'208";a="38305192" Received: from kmsmsx151.gar.corp.intel.com ([172.21.73.86]) by fmsmga008.fm.intel.com with ESMTP; 08 May 2018 05:34:40 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.194]) by KMSMSX151.gar.corp.intel.com ([169.254.10.77]) with mapi id 14.03.0319.002; Tue, 8 May 2018 20:34:37 +0800 From: "Dai, Wei" To: Andrew Rybchenko , "Yigit, Ferruh" , "thomas@monjalon.net" , "Doherty, Declan" , "linville@tuxdriver.com" , "mw@semihalf.com" , "mk@semihalf.com" , "gtzalik@amazon.com" , "evgenys@amazon.com" , "ravi1.kumar@amd.com" , "shepard.siegel@atomicrules.com" , "ed.czeck@atomicrules.com" , "john.miller@atomicrules.com" , "ajit.khaparde@broadcom.com" , "somnath.kotur@broadcom.com" , "jerin.jacob@caviumnetworks.com" , "maciej.czekaj@caviumnetworks.com" , "shijith.thotton@cavium.com" , "ssrinivasan@cavium.com" , "santosh.shukla@caviumnetworks.com" , "rahul.lakkireddy@chelsio.com" , "ohndale@cisco.com" , "hyonkim@cisco.com" , "Lu, Wenzhuo" , "Ananyev, Konstantin" , "Xing, Beilei" , "Zhang, Qi Z" , "Wang, Xiao W" , "Wu, Jingjing" , "tdu@semihalf.com" , "dima@marvell.com" , "nsamsono@marvell.com" , "jianbo.liu@arm.com" , "adrien.mazarguil@6wind.com" , "nelio.laranjeiro@6wind.com" , "yskoh@mellanox.com" , "matan@mellanox.com" , "vido@cesnet.cz" , "alejandro.lucero@netronome.com" , "emant.agrawal@nxp.com" , "shreyansh.jain@nxp.com" , "hemant.agrawal@nxp.com" , "harish.patil@cavium.com" , "rasesh.mody@cavium.com" , "asesh.mody@cavium.com" , "shahed.shaikh@cavium.com" , "yongwang@vmware.com" , "maxime.coquelin@redhat.com" , "mtetsuyah@gmail.com" , "Bie, Tiwei" , "Legacy, Allain (Wind River)" , "Peters, Matt (Wind River)" , "pascal.mazon@6wind.com" , "Richardson, Bruce" , "gaetan.rivet@6wind.com" , "Singh, Jasvinder" , "Dumitrescu, Cristian" CC: "dev@dpdk.org" , Ivan Malov Thread-Topic: [PATCH v8] ethdev: check Rx/Tx offloads Thread-Index: AQHT5rbTkeIcWZY+wkmsdV6MzXhfJKQlLj4AgACO8JA= Date: Tue, 8 May 2018 12:34:36 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D66CF7EF32@PGSMSX111.gar.corp.intel.com> References: <1525442529-12723-1-git-send-email-wei.dai@intel.com> <1525773917-62792-1-git-send-email-wei.dai@intel.com> <657fa573-d9b8-a898-89ea-3f631da6bdf5@solarflare.com> In-Reply-To: <657fa573-d9b8-a898-89ea-3f631da6bdf5@solarflare.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzNiN2Y0MTctMjc5Zi00NTM5LTkwZDktNTY3ODEyZmFhMmZiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InJORDRZZmJSVUN5bW16RkxKU2E0RlNXMGZOMk5zOHRRSXZJK0M5R2hIejQ9In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.200.100 dlp-reaction: no-action x-originating-ip: [172.30.20.205] MIME-Version: 1.0 X-Mailman-Approved-At: Wed, 09 May 2018 12:27:38 +0200 Subject: Re: [dpdk-dev] [PATCH v8] ethdev: check Rx/Tx offloads 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, Andrew See my in-line comments as below. From: Andrew Rybchenko [mailto:arybchenko@solarflare.com] Sent: Tuesday, May 8, 2018 7:37 PM To: Dai, Wei ; Yigit, Ferruh ; thomas@monjalon.net; Doherty, Declan ; linville@tuxdriver.com; mw@semihalf.com; mk@semihalf.com; gtzalik@amazon.com; evgenys@amazon.com; ravi1.kumar@amd.com; shepard.siegel@atomicrules.com; ed.czeck@atomicrules.com; john.miller@atomicrules.com; ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com; jerin.jacob@caviumnetworks.com; maciej.czekaj@caviumnetworks.com; shijith.thotton@cavium.com; ssrinivasan@cavium.com; santosh.shukla@caviumnetworks.com; rahul.lakkireddy@chelsio.com; ohndale@cisco.com; hyonkim@cisco.com; Lu, Wenzhuo ; Ananyev, Konstantin ; Xing, Beilei ; Zhang, Qi Z ; Wang, Xiao W ; Wu, Jingjing ; tdu@semihalf.com; dima@marvell.com; nsamsono@marvell.com; jianbo.liu@arm.com; adrien.mazarguil@6wind.com; nelio.laranjeiro@6wind.com; yskoh@mellanox.com; matan@mellanox.com; vido@cesnet.cz; alejandro.lucero@netronome.com; emant.agrawal@nxp.com; shreyansh.jain@nxp.com; hemant.agrawal@nxp.com; harish.patil@cavium.com; rasesh.mody@cavium.com; asesh.mody@cavium.com; shahed.shaikh@cavium.com; yongwang@vmware.com; maxime.coquelin@redhat.com; mtetsuyah@gmail.com; Bie, Tiwei ; Legacy, Allain (Wind River) ; Peters, Matt (Wind River) ; pascal.mazon@6wind.com; Richardson, Bruce ; gaetan.rivet@6wind.com; Singh, Jasvinder ; Dumitrescu, Cristian Cc: dev@dpdk.org; Ivan Malov Subject: Re: [PATCH v8] ethdev: check Rx/Tx offloads On 05/08/2018 01:05 PM, Wei Dai wrote: This patch check if a input requested offloading is valid or not. Any reuqested offloading must be supported in the device capabilities. Any offloading is disabled by default if it is not set in the parameter dev_conf->[rt]xmode.offloads to rte_eth_dev_configure( ) and [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ). If any offloading is enabled in rte_eth_dev_configure( ) by application, it is enabled on all queues no matter whether it is per-queue or per-port type and no matter whether it is set or cleared in [rt]x_conf->offloads to rte_eth_[rt]x_queue_setup( ). If a per-queue offloading hasn't be enabled in rte_eth_dev_configure( ), it can be enabled or disabled for individual queue in ret_eth_[rt]x_queue_setup( ). A new added offloading is the one which hasn't been enabled in rte_eth_dev_configure( ) and is reuqested to be enabled in rte_eth_[rt]x_queue_setup( ), it must be per-queue type, otherwise return error. The underlying PMD must be aware that the requested offloadings to PMD specific queue_setup( ) function only carries those new added offloadings of per-queue type. This patch can make above such checking in a common way in rte_ethdev layer to avoid same checking in underlying PMD. This patch assumes that all PMDs in 18.05-rc2 have already converted to offload API defined in 17.11 . It also assumes that all PMDs can return correct offloading capabilities in rte_eth_dev_infos_get( ). In the beginning of [rt]x_queue_setup( ) of underlying PMD, add offloads = [rt]xconf->offloads | dev->data->dev_conf.[rt]xmode.offloads; to keep same as offload API defined in 17.11 to avoid upper application broken due to offload API change. PMD can use the info that input [rt]xconf->offloads only carry the new added per-queue offloads to do some optimization or some code change on base of this patch. Signed-off-by: Wei Dai Signed-off-by: Ferruh Yigit Signed-off-by: Qi Zhang [...] 1st argument sa . Andrew, if my code works well, can it be kept here, you can change it later in your separate patch, OK ? ] rc = sfc_rx_qinit(sa, rx_queue_id, nb_rx_desc, socket_id, - rx_conf, mb_pool); + rx_conf, mb_pool, offloads); if (rc != 0) goto fail_rx_qinit; @@ -469,13 +471,16 @@ sfc_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id, { struct sfc_adapter *sa = dev->data->dev_private; int rc; + uint64_t offloads; sfc_log_init(sa, "TxQ = %u, nb_tx_desc = %u, socket_id = %u", tx_queue_id, nb_tx_desc, socket_id); sfc_adapter_lock(sa); - rc = sfc_tx_qinit(sa, tx_queue_id, nb_tx_desc, socket_id, tx_conf); + offloads = tx_conf->offloads | dev->data->dev_conf.txmode.offloads; Same as above. + rc = sfc_tx_qinit(sa, tx_queue_id, nb_tx_desc, socket_id, + tx_conf, offloads); if (rc != 0) goto fail_tx_qinit; [...] It looks like device level offloads are checked on ethdev layer now. So, I think check in sfc_rx_check_mode () and sfc_tx_check_mode() may be removed as well (see offloads_rejected). I think it will make functions sfc_rx_log_offloads() and sfc_tx_log_offloads() unused and these functions should be removed. [Wei: sorry, I miss the sf_[rt]x_check_mode( ). Yes, offloads_rejected in this function checking can be removed. But as this patch is a big one, I'd like it keep as it is if it works well. And I'd like you remove such checking or do other changes in your own patch] [Wei: by the way, your mail is HTML format, I am afraid it is missed from mail archives.] diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index e42d553..fc2b254 100644 --- a/drivers/net/sfc/sfc_ethdev.c +++ b/drivers/net/sfc/sfc_ethdev.c @@ -413,14 +413,16 @@ sfc_rx_queue_setup(struct rte_eth_dev *dev, uint16_t rx_queue_id, { struct sfc_adapter *sa = dev->data->dev_private; int rc; + uint64_t offloads; sfc_log_init(sa, "RxQ=%u nb_rx_desc=%u socket_id=%u", rx_queue_id, nb_rx_desc, socket_id); sfc_adapter_lock(sa); + offloads = rx_conf->offloads | dev->data->dev_conf.rxmode.offloads; I'd prefer to see it inside sfc_rx_qinit() function. It would allow to avoid sfc_rx_qinit() function prototype changes. [Wei: As rx_conf is a const argument in sfc_rx_queue_setup( ), rx_conf->offloads can't be updated. If sfc_rx_qinit( ) function prototype keep unchanged, the dev->data can be deduced from the