From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <arybchenko@solarflare.com>
Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com
 [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id 527FE7D0E
 for <dev@dpdk.org>; Tue,  8 May 2018 13:37:41 +0200 (CEST)
X-Virus-Scanned: Proofpoint Essentials engine
Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16])
 (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits))
 (No client certificate requested)
 by mx1-us3.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id
 831F7480059; Tue,  8 May 2018 11:37:36 +0000 (UTC)
Received: from [192.168.38.17] (84.52.114.114) by ukex01.SolarFlarecom.com
 (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1044.25; Tue, 8 May
 2018 12:37:11 +0100
To: Wei Dai <wei.dai@intel.com>, <ferruh.yigit@intel.com>,
 <thomas@monjalon.net>, <declan.doherty@intel.com>, <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>,
 <wenzhuo.lu@intel.com>, <konstantin.ananyev@intel.com>,
 <beilei.xing@intel.com>, <qi.z.zhang@intel.com>, <xiao.w.wang@intel.com>,
 <jingjing.wu@intel.com>, <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>, <tiwei.bie@intel.com>,
 <allain.legacy@windriver.com>, <matt.peters@windriver.com>,
 <pascal.mazon@6wind.com>, <bruce.richardson@intel.com>,
 <gaetan.rivet@6wind.com>, <jasvinder.singh@intel.com>,
 <cristian.dumitrescu@intel.com>
CC: <dev@dpdk.org>, Ivan Malov <Ivan.Malov@oktetlabs.ru>
References: <1525442529-12723-1-git-send-email-wei.dai@intel.com>
 <1525773917-62792-1-git-send-email-wei.dai@intel.com>
From: Andrew Rybchenko <arybchenko@solarflare.com>
Message-ID: <657fa573-d9b8-a898-89ea-3f631da6bdf5@solarflare.com>
Date: Tue, 8 May 2018 14:37:06 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.7.0
MIME-Version: 1.0
In-Reply-To: <1525773917-62792-1-git-send-email-wei.dai@intel.com>
Content-Language: en-GB
X-Originating-IP: [84.52.114.114]
X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To
 ukex01.SolarFlarecom.com (10.17.10.4)
X-TM-AS-Product-Ver: SMEX-11.0.0.1191-8.100.1062-23830.003
X-TM-AS-Result: No--21.665600-0.000000-31
X-TM-AS-User-Approved-Sender: Yes
X-TM-AS-User-Blocked-Sender: No
X-MDID: 1525779459-oj5dV0Y2UiDR
X-Mailman-Approved-At: Tue, 08 May 2018 14:13:42 +0200
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
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 <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 08 May 2018 11:37:41 -0000

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 <wei.dai@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

[...]

> 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.

>   	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.