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 62450A046B for ; Sun, 18 Aug 2019 07:57:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 454992A5D; Sun, 18 Aug 2019 07:57:26 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 0EF68B62 for ; Sun, 18 Aug 2019 07:57:25 +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-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 93176400056; Sun, 18 Aug 2019 05:57:23 +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:57:16 +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-4-pbhagavatula@marvell.com> <4ca361c2-c350-6888-e5c3-5d3454af35ab@solarflare.com> From: Andrew Rybchenko Message-ID: <3e4b3c49-842e-4d99-fdc0-7647a4897296@solarflare.com> Date: Sun, 18 Aug 2019 08:57:10 +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: 8bit 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-12.949900-8.000000-10 X-TMASE-MatchedRID: oCj5caaCQykeimh1YYHcKPZvT2zYoYOwC/ExpXrHizxigV5T7Tqql/gF luM+m+tyZnH9wlvrd9uYZfjORODtZTM9BBRuZZ1vboe6sMfg+k+7xmCZDXrutQ/vnJWZIdGAedR sVpejKZ6vEQNFRmBeZI/h+VzXVDnD5neCrx57xxp/OBWacv+iVadlL9piCOvOV8ukjx868O6MFi EZi1VquU/a96gHPa2eQVfk++ALZhc5NXlWWXBFjgCNFKULxGCZyeUl7aCTy8h5r69xGFEnQL2/x r1t+ygm5jr+I6ik278Y+GYXlqwk7V7w4pzklHQxSVHYMTQ1F1pdA4rYaKGKwvyQXCBzKijhyVdG RU34S3zrcZeS7mUy9xoAXSLeXC9wNmnQx++HUgUUmR8f4HPVzqDzzrtsjCZTKwrot6guaXOX0y6 nlwu2zNXGlFGnEwOETP+gXA7v436f2/wegNnQURlJRfzNw8afDxIIGsB6XY3fc2Xd6VJ+yj8nlI qeUd1tdsAYJ19+6cWuQ6I+8huFgtfySBN1FpNkKrDHzH6zmUWYTIcrNzjYvAdC9P1Le8dQyvFco BpD/5x8bO6hWfRWzo9CL1e45ag4ZU3/vuVLKG9qZ6OipRp3er/I3arxTrvi4bfJduou5JfIBOJi 1VI4qVUthA3szdE3Kqt2FG1/DdC/WXZS/HqJ2lZ0V5tYhzdWxEHRux+uk8h+ICquNi0WJNKmyaG bQY/tmB2Qnnf095StiZM0MdCcO4zciNALr4ijftwZ3X11IV0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--12.949900-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24854.003 X-MDID: 1566107844-fYISNhgRmSzP Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as an 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:59 AM, Shahaf Shuler wrote: > Friday, August 16, 2019 11:05 AM, Andrew Rybchenko: >> ; Thomas Monjalon >> Cc: dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 3/7] ethdev: add flow action type update as >> an offload >> >> On 8/16/19 8:55 AM, pbhagavatula@marvell.com wrote: >>> From: Pavan Nikhilesh >>> >>> Add new Rx offload flag `DEV_RX_OFFLOAD_FLOW_MARK` that can be >> used to >>> enable/disable PMDs write to `rte_mbuf::hash::fdir::hi`. >> Notes similar to RSS hash. >> >> It requires better motivation why. It lets Rx queue know that it will be used >> as flow action MARK target and the queue should be configured to deliver >> the mark from NIC to PMD and processed in the driver. > This one is even worse than the RSS (sorry). > > First - the API breakage exists also here and if we want to include such patch we should have proper doc on RN. Yes, there is a deprecation notice for it in v19.08 and I already mentioned in review notes for the patch [1/7] that release notes are required. > Second - the user explicitly inserted a rte_flow rule w/ action mark. Its expectation is to receive his mark on the mbuf (otherwise why would it set this action?). it is not expected from the user to set another offload flag just to enable the mark set on the mbuf. It makes the user experience very convoluted. > Third - so far we never reported rte_flow capabilities, and there is a good reason for it - the cap matrix is too big. For rte_flow the chosen approach was trail and error. If we start w/ the flow mark, the amount of cap bits the application will need to monitor will be huge. The feature differs a lot from other rte_flow features since it adds Rx meta information. And yes, rte_flow rule to set mark should fail with appropriate diagnostics if the offload is supported by not enabled or not supported at all. So, application will be informed and I think it is less worse than RSS hash. > My suggestion here, for PMD that wants to optimize their datapath is to check if flow w/ mark action was inserted on the queue. So long there is no such flow they can disable the set of the mark. Unfortunately the information is required on Rx queue setup stage and rte_rule insertion is too late. Anyway, the important point here is all Rx offloads consistency: want something - enable it. The only remaining exception is packet type which default behaviour is preserved since really flexible solution suggested by Konstantin. >> Also I think that flow API action MARK documentation should be updated to >> mentioned the offload. >> >>> Signed-off-by: Pavan Nikhilesh >>> --- >>> doc/guides/nics/features.rst | 12 ++++++++++++ >>> lib/librte_ethdev/rte_ethdev.h | 1 + >>> 2 files changed, 13 insertions(+) >>> >>> diff --git a/doc/guides/nics/features.rst >>> b/doc/guides/nics/features.rst index f79b69b38..d67430d90 100644 >>> --- a/doc/guides/nics/features.rst >>> +++ b/doc/guides/nics/features.rst >>> @@ -594,6 +594,18 @@ application to set ptypes it is interested in. >>> * **[provides] mbuf**: ``mbuf.packet_type``. >>> >>> >>> +.. _nic_features_flow_action_type_update: >> May be  _nic_features_flow_mark ? >> >>> + >>> +Flow type update >> May be "Flow mark delivery" ? >> >>> +---------------- >>> + >>> +Supports flow action type update to ``mbuf.ol_flags`` and >> ``mbuf.hash.fdir.hi``. >>> + >>> +* **[uses] rte_eth_rxconf,rte_eth_rxmode**: >> ``offloads:DEV_RX_OFFLOAD_FLOW_TYPE``. >> >> DEV_RX_OFFLOAD_FLOW_MARK, not TYPE. >> >> >> >>> +* **[provides] mbuf**: ``mbuf.ol_flags:PKT_RX_FDIR``, >>> +``mbuf.ol_flags:PKT_RX_FDIR_ID;``, >>> + ``mbuf.hash.fdir.hi`` >>> + >>> + >>> .. _nic_features_timesync: >>> >>> Timesync >>> diff --git a/lib/librte_ethdev/rte_ethdev.h >>> b/lib/librte_ethdev/rte_ethdev.h index 889486a11..4a0cff830 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> @@ -1014,6 +1014,7 @@ struct rte_eth_conf { >>> #define DEV_RX_OFFLOAD_SCTP_CKSUM 0x00020000 >>> #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM 0x00040000 >>> #define DEV_RX_OFFLOAD_RSS_HASH 0x00080000 >>> +#define DEV_RX_OFFLOAD_FLOW_MARK 0x00100000 >>> >>> #define DEV_RX_OFFLOAD_CHECKSUM >> (DEV_RX_OFFLOAD_IPV4_CKSUM | \ >>> DEV_RX_OFFLOAD_UDP_CKSUM | \ >> Add to rte_rx_offload_names