From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <dev-bounces@dpdk.org> Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4025EA04B3; Fri, 8 Nov 2019 12:41:05 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 146D71C10E; Fri, 8 Nov 2019 12:41:05 +0100 (CET) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 9CE1A1C0D8 for <dev@dpdk.org>; Fri, 8 Nov 2019 12:41:03 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Nov 2019 03:41:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,281,1569308400"; d="scan'208";a="354075390" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga004.jf.intel.com with ESMTP; 08 Nov 2019 03:41:01 -0800 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 8 Nov 2019 03:41:01 -0800 Received: from shsmsx154.ccr.corp.intel.com (10.239.6.54) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 8 Nov 2019 03:41:01 -0800 Received: from shsmsx105.ccr.corp.intel.com ([169.254.11.225]) by SHSMSX154.ccr.corp.intel.com ([169.254.7.200]) with mapi id 14.03.0439.000; Fri, 8 Nov 2019 19:40:58 +0800 From: "Zhang, Qi Z" <qi.z.zhang@intel.com> To: Thomas Monjalon <thomas@monjalon.net>, Andrew Rybchenko <arybchenko@solarflare.com> CC: Ori Kam <orika@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>, "pbhagavatula@marvell.com" <pbhagavatula@marvell.com>, "Yigit, Ferruh" <ferruh.yigit@intel.com>, "jerinj@marvell.com" <jerinj@marvell.com>, "Mcnamara, John" <john.mcnamara@intel.com>, "Kovacevic, Marko" <marko.kovacevic@intel.com>, Adrien Mazarguil <adrien.mazarguil@6wind.com>, "david.marchand@redhat.com" <david.marchand@redhat.com>, "ktraynor@redhat.com" <ktraynor@redhat.com>, Olivier Matz <olivier.matz@6wind.com> Thread-Topic: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update as an offload Thread-Index: AQHVliRIKcszyu6pwkSe0f8x6P0oaqeBIASA Date: Fri, 8 Nov 2019 11:40:57 +0000 Message-ID: <039ED4275CED7440929022BC67E7061153DC6603@SHSMSX105.ccr.corp.intel.com> References: <20191025152142.12887-1-pbhagavatula@marvell.com> <1594828.JO7TyvRWtP@xps> <d38c77a7-22ef-7221-feed-69ea5db876a8@solarflare.com> <1784584.NQqjHnNvIa@xps> In-Reply-To: <1784584.NQqjHnNvIa@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTYwMjI3NGQtYThiMC00MGQ0LWI4ZTctM2M2NzhmNDBlMDU4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWG1EcGJIN1k4WG52c2tNemFtd09CSithQ0RYZnRCQ0lBWHFvUjZKdGpIOTc2N0RaY2pcL3d0UDRWS1JTY1lsUnQifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/2] 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 <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon > Sent: Friday, November 8, 2019 7:04 PM > To: Andrew Rybchenko <arybchenko@solarflare.com> > Cc: Ori Kam <orika@mellanox.com>; dev@dpdk.org; > pbhagavatula@marvell.com; Yigit, Ferruh <ferruh.yigit@intel.com>; > jerinj@marvell.com; Mcnamara, John <john.mcnamara@intel.com>; > Kovacevic, Marko <marko.kovacevic@intel.com>; Adrien Mazarguil > <adrien.mazarguil@6wind.com>; david.marchand@redhat.com; > ktraynor@redhat.com; Olivier Matz <olivier.matz@6wind.com> > Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: add flow action type update a= s an > offload >=20 > 08/11/2019 11:42, Andrew Rybchenko: > > On 11/8/19 1:28 PM, Thomas Monjalon wrote: > > > 08/11/2019 09:35, Andrew Rybchenko: > > >> The problem: > > >> ~~~~~~~~~~~~ > > >> PMD wants to know before port start if application wants to to use > > >> flow MARK/FLAG in the future. It is required because: > > >> > > >> 1. HW may be configured in a different way to reserve resources > > >> for MARK/FLAG delivery > > >> > > >> 2. Datapath implementation choice may depend on it (e.g. vPMD > > >> is faster, but does not support MARK) > > > > > > Thank you for the clear problem statement. > > > I agree with it. This is a real design issue. > > > > > > > > >> Discussed solutions: > > >> ~~~~~~~~~~~~~~~~~~~~ > > > > May be it is not 100% clear since below are alternatives. > > > > >> A. Explicit Rx offload suggested by the patch. > > >> > > >> B. Implicit by validation of a flow rule with MARK/FLAG actions used= . > > >> > > >> C. Use dynamic field/flag (i.e. application registers dynamic field > > >> and/or flag and PMD uses lookup to solve the problem) plus part > > >> of (B) to discover if a feature is supported. > > > > > > The dynamic field should be registered via a new API function named > > > '<feature>_init'. > > > It means the application must explicit request the feature. > > > I agree this is the way to go. > > > > If I understand your statement correctly, but (C) is not ideal since > > it looks global. If registered dynamic field of mbuf and is flag that > > the feature should be enabled, it is a flag to all ports/devices. > > > > >> All solutions require changes in applications which use these > > >> features. There is a deprecation notice in place which advertises > > >> DEV_RX_OFFLOAD_FLOW_MARK addition, but may be it is OK to > > >> substitute it with solution (B) or (C). Solution (C) requires > > >> changes since it should be combined with (B) in order to understand > > >> if the feature is supported. > > > > > > I don't understand. > > > Application request and PMD support are two different things. > > > PMD support must be via rte_flow validation on a case by case anyway. > > > > I mean that application wants to understand if the feature is > > supported. Then, it wants to enable it. In the case of (B), if I > > understand the solution correctly, there is no explicit way to enable, > > PMD just detects it because of discovery is done (that's what I mean > > by "implicit" and it is a drawback from my point of view, but still > > could be considered). (C) solves the problem of (B). > > > > >> Advantages and drawbacks of solutions: > > >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > >> 1. The main drawback of (A) is a "duplication" since we already > > >> have a way to request flow MARK using rte_flow API. > > >> I don't fully agree that it is a duplication, but I agree > > >> that it sounds like duplication and complicates a bit flow > > >> MARK usage by applications. (B) complicates it as well. > > >> > > >> 2. One more drawback of the solution (A) is the necessity of > > >> similar solution for META and it eats one more offload bit. > > >> Yes, that's true and I think it is not a problem. > > >> It would make it easier for applications to find out if > > >> either MARK or META is supported. > > >> > > >> 3. The main advantage of the solution (A) is simplicity. > > >> It is simple for application to understand if it supported. > > >> It is simple in PMD to understand that it is required. > > >> It is simple to disable it - just reconfigure. > > >> Also it is easier to document it - just mention that > > >> the offload should be supported and enabled. > > >> > > >> 4. The main advantage of the solution (B) is no "duplication". > > >> I agree that it is valid argument. Solving the problem > > >> without extra entities is always nice, but unfortunately > > >> it is too complex in this case. > > >> > > >> 5. The main drawback of the solution (B) is the complexity. > > >> It is necessary to choose a flow rule which should be used > > >> as a criteria. It could be hardware dependent. > > >> Complex logic is require in PMD if it wants to address the > > >> problem and control MARK delivery based on validated flow > > >> rules. It adds dependency between start/stop processing and > > >> flow rules validation code. > > >> It is pretty complicated to document it. > > >> > > >> 6. Useless enabling of the offload in the case of solution (A) > > >> if really used flow rules do not support MARK looks like > > >> drawback as well, but easily mitigated by a combination > > >> with solution (B) and only required if the application wants > > >> to dive in the level of optimization and complexity and > > >> makes sense if application knows required flow rules in > > >> advance. So, it is not a problem in this case. > > >> > > >> 7. Solution (C) has drawbacks of the solution (B) for > > >> applications to understand if these features are supported, > > >> but no drawbacks in PMD, since explicit criteria is used to > > >> enable/disable (dynamic field/flag lookup). > > >> > > >> 8. Solution (C) is nice since it avoids "duplication". > > >> > > >> 9. The main drawback of the solution (C) is asymmetry. > > >> As it was discussed in the case of RX_TIMESTAMP > > >> (if I remember it correctly): > > >> - PMD advertises RX_TIMESTAMP offload capability > > >> - application enables the offload > > >> - PMD registers dynamic field for timestamp > > >> Solution (C): > > >> - PMD advertises nothing > > >> - application uses solution (B) to understand if > > >> these features are supported > > >> - application registers dynamic field/flag > > >> - PMD does lookup and solve the problem > > >> The asymmetry could be partially mitigated if RX_TIMESTAMP > > >> solution is changed to require an application to register > > >> dynamic fields and PMD to do lookup if the offload is > > >> enabled. So, the only difference will be in no offload > > >> in the case of flow MARK/FLAG and usage of complex logic > > >> to understand if it is supported or no. > > >> May be it would be really good since it will allow to > > >> have dynamic fields registered before mempool population. > > >> > > >> 10. Common drawback of solutions (B) and (C) is no granularity. > > >> Solution (A) may be per queue while (B) and (C) cannot be > > >> per queue. Moreover (C) looks global - for all devices. > > >> It could be really painful. > > >> > > >> (C) is nice, but I still vote for simplicity and granularity of > > >> (A). > > > > > > I vote for clear separation of application needs and PMD support, by > > > using the method C (dynamic fields). > > > I agree timestamp must use the same path. > > > I agree it's complicate because we don't know in advance whether a > > > flow rule will be accepted, but that's the reality, config is complex= . > > > > Do you think that global nature of the (C) is acceptable? >=20 > That's a good question. > Maybe the feature request should be per port. > In this case, we are back to solution A with a flag per port? >=20 > Note that A and C will not guarantee that the offload will be possible. > We need B (flow rule validation) anyway. I may not understand how solution B can works well for all the cases. A rte_flow rule can be issued after dev_start, which means the rx_burst fun= ction is already selected at that time,=20 so does that mean the driver need to switch from a non- mark offload aware = path to a mark offload aware path without stop device? or it has to reject = the flow?=20 The question is if we have 2 data path, one support some offload , one not = but more fast, which one should be selected during dev_start? Isn't Offload= widely used to solve this problem? I think the option A solve all the problems, option C might also works, but= A is looks much straightforward for me. Regards Qi >=20 > It seems A, B, C are not alternatives but all required as pieces of a puz= zle... >=20