From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id CD9D31B1E6 for ; Thu, 10 May 2018 04:35:57 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5AC99225EB; Wed, 9 May 2018 22:35:57 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 09 May 2018 22:35:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=BSKO3Rzre90/S3u3NUDGFYMcUK IlavF6bsALOYxUE/0=; b=RbRZdUMKsdEVp4DiseveY23ZBzxrT8/HtC0AodvM7E R1PHlX9kYlVovJGkJyHtzqX/7Zc6W7fEdwTrXVYaOsnELcaqWg44vKw4lXv02XOV 1JuUQ7pWCKDJ+xVB7DEPSx9pmYlhtM6i0T7sPx/WZx1GA3779eorIu2GM+9cqSmk 0= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=BSKO3R zre90/S3u3NUDGFYMcUKIlavF6bsALOYxUE/0=; b=A1ZUozFLjUI4bJvVSG5qUk ouGopdrPITtRQGcA+MpewZyVXN5d7bGaTQQrggpo4ocDu6KC44FVML2k1JOJmiwk xqSUX1dfu4ygzBzqohKT/vEh+Ovjl3TJcD18Tp+xiLhDcb2w83MqmHMwpqX2RQ6F jKH2Oc+azXVAjvYyZ+m2azrYxRtU7LXghznYAcaeVZugqKJxrQDCvM43bBKW5aPz MbpuWC560acTm6yV6zQK7AfpQ3r+BDUfZJNjBK9Z4+gehct8MPB3lrbD3sH8AkRx 4l2yxPa7Y2mxuQ6Ah0IRRd1BUAofpRoRPJHn+8RfASwwPLngGcDL9iZ9SKlBBTqw == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 8BD3CE444A; Wed, 9 May 2018 22:35:56 -0400 (EDT) From: Thomas Monjalon To: Wei Dai Cc: dev@dpdk.org, ferruh.yigit@intel.com, Qi Zhang Date: Thu, 10 May 2018 04:35:55 +0200 Message-ID: <7647125.8C0XxgkuRT@xps> In-Reply-To: <1525913806-41723-1-git-send-email-wei.dai@intel.com> References: <1525913371-18178-1-git-send-email-wei.dai@intel.com> <1525913806-41723-1-git-send-email-wei.dai@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 02:35:58 -0000 Hi, I am checking if this patch comply with goals discussed in the survey: http://dpdk.org/ml/archives/dev/2018-March/094459.html - Allow "forgetting" port offloads in queue offloads setup. - An offload enabled at port level, cannot be disabled at queue level. - Every queue capabilities must be reported as port capabilities. - A capability should be reported at queue level only if it can be enabled on queue when it is disabled on port level. 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. Examples: - 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." - in port capabilities: "(include per-queue capabilities)" More comments below, thanks. 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. Good summary. Please forget the whitespace inside the parens. > This patch can make above such checking in a common way in rte_ethdev > layer to avoid same checking in underlying PMD. Good > --- 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-queue 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 at the same time. > +A per-port offloading can be enabled or disabled on all queues at the same time. What is the difference between pure per-port and per-port here? > +It is certain that both per-queue and pure per-port offloading are per-port type. I don't understand this sentence. > 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. Yes > +Any requested offloading by application must be within the device capabilities. Yes > +Any offloading is disabled by default if it is not set in the parameter Yes > +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. Yes > +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 requested to be enabled > +in ``rte_eth_[rt]x_queue_setup( )``, it must be per-queue type, otherwise return error. Yes > --- a/doc/guides/rel_notes/release_18_05.rst > +++ b/doc/guides/rel_notes/release_18_05.rst > +* **ethdev: changes to offload API** No need of bold formatting of title in API changes. > + > + 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 added offloading which has > + not been enabled in ``rte_eth_dev_configure( )`` and is requested to be enabled in > + ``rte_eth_[rt]x_queue_setup( )`` must be per-queue type, otherwise return error.