From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 64C38A046B for ; Sun, 18 Aug 2019 07:38:58 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 05B562A5D; Sun, 18 Aug 2019 07:38:57 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id E61FD1DBD for ; Sun, 18 Aug 2019 07:38:54 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us5.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id CB7EC80005E; Sun, 18 Aug 2019 05:38:52 +0000 (UTC) Received: from [192.168.1.11] (85.187.13.152) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Sun, 18 Aug 2019 06:38:45 +0100 To: Shahaf Shuler , "pbhagavatula@marvell.com" , "jerinj@marvell.com" , "ferruh.yigit@intel.com" , John McNamara , Marko Kovacevic , Thomas Monjalon CC: "dev@dpdk.org" References: <20190816055511.2322-1-pbhagavatula@marvell.com> <20190816055511.2322-3-pbhagavatula@marvell.com> <672afa31-6f88-9099-f4c7-e85a24959c0e@solarflare.com> From: Andrew Rybchenko Message-ID: Date: Sun, 18 Aug 2019 08:38:39 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [85.187.13.152] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24854.003 X-TM-AS-Result: No-11.060000-8.000000-10 X-TMASE-MatchedRID: Rp71wniPtoMeimh1YYHcKPZvT2zYoYOwC/ExpXrHizxigV5T7Tqqlxlg ra56oHr6sOgv3K25H2kqIo4myHvjB3gz4aOScPlwnMQdNQ64xfdKgIbix5+XxMAkyHiYDAQbQlf mKk5XlH3gYsxc0OenavZBnprW/qxWFy3KwcvXoX1xzSc1O7BXgVucRt3PQ/Wr33Nl3elSfsqFiI wYSrmUJvXd0yvRmLVf7gjPN5QSs+fCfVIFjo47qao2fOuRT7aaM56LeR0qzjeNTrsIz649BNbXw yjvXpaTjcpy2UCQvnEBGFkSkEoEtWM2XgNitpqtJEFMYGmfGMc5iooXtStiHlXWVD8efRM6FpNQ WPoSqU2ZLrn1N8axxtxhSspBtrcyR/oM2cgUsZr0VCHd+VQiHn316REeU5CRsS0sZEB7c8bCmXn 3d0JAl1r3Vbggd0Wgr8cClirK9kmykZg9uxRW6gCNFKULxGCZccy0Gdkm5+QPpGoR1th3nqeguO hTIJ7RNhqXgy2O1nwHFegM4rZUqFGEd5OSBmbmPwKTD1v8YV5MkOX0UoduuSEhXUjPhv6ZcRFtL s5mkQM7xYjavBtzOYV1EryRKlU7R37774wLVvZxoP7A9oFi1jVPM/rRSR0dRZzmX/N2e+ujxYyR Ba/qJQPTK4qtAgwIAYt5KiTiutlt1O49r1VEa8RB0bsfrpPIfiAqrjYtFiSVa1MA0kfXpTLj1vd 8vAKhutkG/hTD1kCw/w+66EYC437cGd19dSFd X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.060000-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24854.003 X-MDID: 1566106734-74v-y2GSlOMO Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a offload 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" On 8/18/19 7:52 AM, Shahaf Shuler wrote: > Friday, August 16, 2019 10:48 AM, Andrew Rybchenko: >> Subject: Re: [dpdk-dev] [PATCH 2/7] ethdev: add mbuf RSS update as a >> offload >> >> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote: >>> From: Pavan Nikhilesh >>> >>> Add new Rx offload flag `DEV_RX_OFFLOAD_RSS_HASH` which can be used >> to >>> enable/disable PMDs write to `rte_mbuf::hash::rss`. >> It should be highlighted that presence of the RSS hash is indicated by >> PKT_RX_RSS_HASH flag in mbuf anyway. Now applications have a way to >> check that RSS hash delivery is supported and should enable the offload if >> RSS hash is used. PMD may still provide the hash even if the offload is not >> enabled. > I don't understand how PMDs should act w/ this addition when considering the API breakage to application. There is a deprecation notice for it. I mentioned in my review notes for one of patches in the series that the change should be highlighted in release notes. Yes, it is absolutely required if these patches are accepted. > Currently application don't set this flag, and expect to get the RSS hash result on mbuf. > If PMDs will not set the RSS hash result when flag is not present then applications might break. > If they will always set, then there is no meaning for it. > > as I understand the motivation to save few cycles on the PMD receive path, if we want to include it we should treat it as API breakage and documents it on the release notes. > My option is that some offload should just be usable (OOB) by the fact user enabled them (e.g. RSS). no need to complicate the user by checking and set this field. What I don't understand is why some offloads should just work but another requires action to enable it. Just because it is the current state of things - I don't think it is a good motivation. Sorry. I think more applications use checksum offloads than RSS hash, but it is still required to enable it. It looks like no single DPDK example uses RSS hash. So, I guess it not widely used by applications as well. Anyway these 2 patches for flow action and RSS hash make all Rx offloads consistent - if you need something, enable it. And the question is not to save few cycles in the PMD receive path. It makes is possible to not deliver both from NIC to host. 8 bytes (4 RSS hash and 4 flow mark) are more than 10% for the smallest packets. >>> Signed-off-by: Pavan Nikhilesh >> Reviewed-by: Andrew Rybchenko >> >> with above and one note below fixed. >> >>> --- >>> doc/guides/nics/features.rst | 2 ++ >>> lib/librte_ethdev/rte_ethdev.h | 1 + >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/doc/guides/nics/features.rst >>> b/doc/guides/nics/features.rst index d4d55f721..f79b69b38 100644 >>> --- a/doc/guides/nics/features.rst >>> +++ b/doc/guides/nics/features.rst >>> @@ -274,6 +274,7 @@ Supports RSS hashing on RX. >>> >>> * **[uses] user config**: ``dev_conf.rxmode.mq_mode`` = >> ``ETH_MQ_RX_RSS_FLAG``. >>> * **[uses] user config**: ``dev_conf.rx_adv_conf.rss_conf``. >>> +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: >> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``. >>> * **[provides] rte_eth_dev_info**: ``flow_type_rss_offloads``. >>> * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``. >>> >>> @@ -286,6 +287,7 @@ Inner RSS >>> Supports RX RSS hashing on Inner headers. >>> >>> * **[uses] rte_flow_action_rss**: ``level``. >>> +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: >> ``offloads:DEV_RX_OFFLOAD_RSS_HASH``. >>> * **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_RSS_HASH``, ``mbuf.rss``. >>> >>> >>> diff --git a/lib/librte_ethdev/rte_ethdev.h >>> b/lib/librte_ethdev/rte_ethdev.h index f97f0a6e5..889486a11 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> @@ -1013,6 +1013,7 @@ struct rte_eth_conf { >>> #define DEV_RX_OFFLOAD_KEEP_CRC 0x00010000 >>> #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 >>> #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 >>> +#define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 >> Should be added to rte_rx_offload_names in >> lib/librte_ethdev/rte_ethdev.c.