From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 4E16B1B763 for ; Thu, 10 May 2018 13:27:22 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 May 2018 04:27:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,385,1520924400"; d="scan'208";a="44791838" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by fmsmga002.fm.intel.com with ESMTP; 10 May 2018 04:27:19 -0700 Received: from pgsmsx111.gar.corp.intel.com ([169.254.2.194]) by PGSMSX102.gar.corp.intel.com ([169.254.6.245]) with mapi id 14.03.0319.002; Thu, 10 May 2018 19:27:18 +0800 From: "Dai, Wei" To: Thomas Monjalon CC: "dev@dpdk.org" , "Yigit, Ferruh" , "Zhang, Qi Z" Thread-Topic: [dpdk-dev] [PATCH v10] ethdev: new Rx/Tx offloads API Thread-Index: AQHT5/ns211BySEoVECznUHaSo3gVqQnuS2AgAC9SwA= Date: Thu, 10 May 2018 11:27:17 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D66CF7FE9F@PGSMSX111.gar.corp.intel.com> References: <1525913371-18178-1-git-send-email-wei.dai@intel.com> <1525913806-41723-1-git-send-email-wei.dai@intel.com> <7647125.8C0XxgkuRT@xps> In-Reply-To: <7647125.8C0XxgkuRT@xps> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYzgzNDk2MzUtZjVmMS00NTI0LTlmZDEtZGNhODY5ZjExZjBjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InFMaU5ldnJPKzFhWDlhOWI5R3NTTlBqSEFoMVkxNDZ5dDlcLzJvclo2VEs4PSJ9 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] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v10] ethdev: new Rx/Tx offloads API 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: Thu, 10 May 2018 11:27:22 -0000 Hi, Thomas Thanks for your feedback and guidance. > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Thursday, May 10, 2018 10:36 AM > To: Dai, Wei > Cc: dev@dpdk.org; Yigit, Ferruh ; Zhang, Qi Z > > Subject: Re: [dpdk-dev] [PATCH v10] ethdev: new Rx/Tx offloads API >=20 > Hi, >=20 > I am checking if this patch comply with goals discussed in the survey: > http://dpdk.org/ml/archives/dev/2018-March/094459.html >=20 > - Allow "forgetting" port offloads in queue offloads setup. >=20 > - An offload enabled at port level, cannot be disabled at queue level. >=20 > - Every queue capabilities must be reported as port capabilities. >=20 > - A capability should be reported at queue level > only if it can be enabled on queue when it is disabled on port level. >=20 > I think some items must be updated in doxygen comments of rte_ethdev.h. > Please could you try to do a v11 for doxygen? I will review it quickly. >=20 > Examples: >=20 > - in queue offloads: > "No need to repeat flags already enabled at port level. > A flag enabled at port level, cannot be disabled at queue level." >=20 > - in port capabilities: "(include per-queue capabilities)" >=20 > More comments below, thanks. >=20 >=20 > 10/05/2018 02:56, Wei Dai: > > 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 > > triger an error log. > > 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. >=20 > Good summary. > Please forget the whitespace inside the parens. I will remove whitespaces inside the parens. >=20 > > This patch can make above such checking in a common way in rte_ethdev > > layer to avoid same checking in underlying PMD. >=20 > Good >=20 > > --- a/doc/guides/prog_guide/poll_mode_drv.rst > > +++ b/doc/guides/prog_guide/poll_mode_drv.rst > > @@ -297,16 +297,30 @@ Per-Port and Per-Queue Offloads > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > In the DPDK offload API, offloads are divided into per-port and per-qu= eue > offloads. > > +A per-queue offloading can be enabled on a queue and disabled on > another queue at the same time. > > +A pure per-port offloading can't be enabled on a queue and disabled on > another queue at the same time. > > +A pure per-port offloading must be enabled or disabled on all queues a= t > the same time. > > +A per-port offloading can be enabled or disabled on all queues at the > same time. >=20 > What is the difference between pure per-port and per-port here? >=20 Can I add following words to explain more:=20 A pure per-port offloads are those supported by device but not per-queue ty= pe. A supported offload can be per-queue or pure per-port, but can't be both types at same device.=20 Any offloadings are per-port type as it can be enabled or disabled on all q= ueues at the same time. > > +It is certain that both per-queue and pure per-port offloading are per= -port > type. >=20 > I don't understand this sentence. >=20 Explained above, I just want to divide all offloading into two distinct typ= es:=20 Per-queue and pure per-port. And per-queue + pure per-port =3D per-port. > > The different offloads capabilities can be queried using > ``rte_eth_dev_info_get()``. > > +The dev_info->[rt]x_queue_offload_capa returned from > ``rte_eth_dev_info_get()`` includes all per-queue offloading capabilities= . > > +The dev_info->[rt]x_offload_capa returned from > ``rte_eth_dev_info_get()`` includes all per-port and per-queue offloading > capabilities. >=20 > Yes >=20 > > +Any requested offloading by application must be within the device > capabilities. >=20 > Yes >=20 > > +Any offloading is disabled by default if it is not set in the > > +parameter >=20 > Yes >=20 > > +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 been enabled in > > +``rte_eth_dev_configure( )``, it can be enabled or disabled in > ``rte_eth_[rt]x_queue_setup( )`` for individual queue. >=20 > Yes >=20 > > +A new added offloads in [rt]x_conf->offloads to > > +``rte_eth_[rt]x_queue_setup( )`` input by application is the one > > +which hasn't been enabled in ``rte_eth_dev_configure( )`` and is reque= sted > to be enabled in ``rte_eth_[rt]x_queue_setup( )``, it must be per-queue t= ype, > otherwise return error. >=20 > Yes >=20 >=20 > > --- a/doc/guides/rel_notes/release_18_05.rst > > +++ b/doc/guides/rel_notes/release_18_05.rst > > +* **ethdev: changes to offload API** >=20 > No need of bold formatting of title in API changes. >=20 Will revise it as you guide. > > + > > + A pure per-port offloading isn't requested to be repeated in > [rt]x_conf->offloads to > > + ``rte_eth_[rt]x_queue_setup( )``. Now any offloading enabled in > ``rte_eth_dev_configure( )`` > > + can't be disabled by ``rte_eth_[rt]x_queue_setup( )``. Any new adde= d > offloading which has > > + not been enabled in ``rte_eth_dev_configure( )`` and is requested t= o > be enabled in > > + ``rte_eth_[rt]x_queue_setup( )`` must be per-queue type, otherwise > return error. >=20 >=20 >=20