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 20084A00BE; Tue, 7 Jul 2020 19:52:22 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id EB7EB1DE43; Tue, 7 Jul 2020 19:52:21 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 9933D1DE40 for ; Tue, 7 Jul 2020 19:52:20 +0200 (CEST) IronPort-SDR: wDHqziYs00CGkdcYAC9l+etBJsGrs49oebaH8z2pLMDR8+MTK9M6bvco8RPsSju/8aIz002FCr RxAvfe8Let5w== X-IronPort-AV: E=McAfee;i="6000,8403,9675"; a="165738865" X-IronPort-AV: E=Sophos;i="5.75,324,1589266800"; d="scan'208";a="165738865" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Jul 2020 10:52:18 -0700 IronPort-SDR: wsISK/wLHwnAp0ChjnYA8um2mPOz7JByHHYJzLOOo6dMjfeYJJ3TB9INJ6FzMRZ/gzoqy5229M x6fHl8tTOr1w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,324,1589266800"; d="scan'208";a="279680607" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.213.248.101]) ([10.213.248.101]) by orsmga003.jf.intel.com with ESMTP; 07 Jul 2020 10:52:16 -0700 To: Ori Kam , Jerin Jacob , Andrey Vesnovaty Cc: Andrey Vesnovaty , Thomas Monjalon , Andrew Rybchenko , dpdk-dev References: <20200702120511.16315-1-andreyv@mellanox.com> <82a903f4-fb37-4c58-5449-31401b7dfd04@intel.com> From: Ferruh Yigit Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJsBBMBCgBWAhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEABQkKqZZ8FiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl6ha3sXGHZrczovL2tl eXMub3BlbnBncC5vcmcACgkQ+TPrQ98TYR8uLA//QwltuFliUWe60xwmu9sY38c1DXvX67wk UryQ1WijVdIoj4H8cf/s2KtyIBjc89R254KMEfJDao/LrXqJ69KyGKXFhFPlF3VmFLsN4XiT PSfxkx8s6kHVaB3O183p4xAqnnl/ql8nJ5ph9HuwdL8CyO5/7dC/MjZ/mc4NGq5O9zk3YRGO lvdZAp5HW9VKW4iynvy7rl3tKyEqaAE62MbGyfJDH3C/nV/4+mPc8Av5rRH2hV+DBQourwuC ci6noiDP6GCNQqTh1FHYvXaN4GPMHD9DX6LtT8Fc5mL/V9i9kEVikPohlI0WJqhE+vQHFzR2 1q5nznE+pweYsBi3LXIMYpmha9oJh03dJOdKAEhkfBr6n8BWkWQMMiwfdzg20JX0o7a/iF8H 4dshBs+dXdIKzPfJhMjHxLDFNPNH8zRQkB02JceY9ESEah3wAbzTwz+e/9qQ5OyDTQjKkVOo cxC2U7CqeNt0JZi0tmuzIWrfxjAUulVhBmnceqyMOzGpSCQIkvalb6+eXsC9V1DZ4zsHZ2Mx Hi+7pCksdraXUhKdg5bOVCt8XFmx1MX4AoV3GWy6mZ4eMMvJN2hjXcrreQgG25BdCdcxKgqp e9cMbCtF+RZax8U6LkAWueJJ1QXrav1Jk5SnG8/5xANQoBQKGz+yFiWcgEs9Tpxth15o2v59 gXK5Ag0EV9ZMvgEQAKc0Db17xNqtSwEvmfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ES YpV8QWj0xK4YM0dLxnDU2IYxjEshSB1TqAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4Ai bPtrHuIXWQOBECcVZTTOdZYGAzaYzxiAONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxD UQljeNvKYt1lZE/gAUUxNLWsYyTT+22/vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/ 3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35piVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVj sM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQI3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdc q9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYHfVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH7 1PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZqw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFB VOQOxCvwRG2QCgcJ/UTn5vlivul+cThi6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI 8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJlRr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYC GwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNhHwUCXqFrngUJCKxSYAAKCRD5M+tD3xNhH3YWD/9b cUiWaHJasX+OpiuZ1Li5GG3m9aw4lR/k2lET0UPRer2Jy1JsL+uqzdkxGvPqzFTBXgx/6Byz EMa2mt6R9BCyR286s3lxVS5Bgr5JGB3EkpPcoJT3A7QOYMV95jBiiJTy78Qdzi5LrIu4tW6H o0MWUjpjdbR01cnj6EagKrDx9kAsqQTfvz4ff5JIFyKSKEHQMaz1YGHyCWhsTwqONhs0G7V2 0taQS1bGiaWND0dIBJ/u0pU998XZhmMzn765H+/MqXsyDXwoHv1rcaX/kcZIcN3sLUVcbdxA WHXOktGTQemQfEpCNuf2jeeJlp8sHmAQmV3dLS1R49h0q7hH4qOPEIvXjQebJGs5W7s2vxbA 5u5nLujmMkkfg1XHsds0u7Zdp2n200VC4GQf8vsUp6CSMgjedHeF9zKv1W4lYXpHp576ZV7T GgsEsvveAE1xvHnpV9d7ZehPuZfYlP4qgo2iutA1c0AXZLn5LPcDBgZ+KQZTzm05RU1gkx7n gL9CdTzVrYFy7Y5R+TrE9HFUnsaXaGsJwOB/emByGPQEKrupz8CZFi9pkqPuAPwjN6Wonokv ChAewHXPUadcJmCTj78Oeg9uXR6yjpxyFjx3vdijQIYgi5TEGpeTQBymLANOYxYWYOjXk+ae dYuOYKR9nbPv+2zK9pwwQ2NXbUBystaGyQ== Message-ID: <20b57e45-bf2d-5d6f-e6e9-300ae865e5c9@intel.com> Date: Tue, 7 Jul 2020 18:52:16 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] add flow shared action 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 7/7/2020 6:24 PM, Ori Kam wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Ferruh Yigit >> >> On 7/7/2020 7:21 AM, Ori Kam wrote: >>> Hi Jerin, >>> Thanks you for your quick reply. >>> >>>> -----Original Message----- >>>> From: Jerin Jacob >>>> Subject: Re: [dpdk-dev] [PATCH] add flow shared action API >>>> >>>> On Mon, Jul 6, 2020 at 7:02 PM Andrey Vesnovaty >>>> wrote: >>>>> >>>>> Hi, Jerin. >>>> >>>> Hi Ori and Andrey, >>>> >>>> >>>>> >>>>> Please see below Ori's suggestion below to implement your >>>> rte_flow_action_update() idea >>>>> with some API changes of rte_flow_shared_action_xxx API changes. >>>>> >>>>> On Mon, Jul 6, 2020 at 3:28 PM Ori Kam wrote: >>>>>> >>>>>> Hi Jerin, >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Jerin Jacob >>>>>>> Sent: Monday, July 6, 2020 12:00 PM >>>>>>> Subject: Re: [dpdk-dev] [PATCH] add flow shared action API >>>>>>> >>>>>>> On Sun, Jul 5, 2020 at 3:56 PM Ori Kam wrote: >>>>>>>> >>>>>>>> Hi Jerin, >>>>>>>> PSB, >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Ori >>>>>>>> >>>>>>>>> -----Original Message----- >>>>>>>>> From: Jerin Jacob >>>>>>>>> Sent: Saturday, July 4, 2020 3:33 PM >>>>>>>>> dpdk-dev >>>>>>>>> Subject: Re: [dpdk-dev] [PATCH] add flow shared action API >>>>>>>>> >>>>>>>>> On Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty >>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> >>>>>>>>>> Andrey Vesnovaty >>>>>>>>>> (+972)526775512 | Skype: andrey775512 >>>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> [..Nip ..] >>>>>>>> >>>>>>>>>> I need to mention the locking issue once again. >>>>>>>>>> If there is a need to maintain "shared session" in the generic >>>> rte_flow >>>>>>> layer >>>>>>>>> all >>>>>>>>>> calls to flow_create() with shared action & all delete need to take >>>>>>>>> sharedsession >>>>>>>>>> management locks at least for verification. Lock partitioning is also >>>> bit >>>>>>>>> problematic >>>>>>>>>> since one flow may have more than one shared action. >>>>>>>>> >>>>>>>>> Then, I think better approach would be to introduce >>>>>>>>> rte_flow_action_update() public >>>>>>>>> API which can either take "const struct rte_flow_action []" OR shared >>>>>>>>> context ID, to cater to >>>>>>>>> both cases or something on similar lines. This would allow HW's >>>>>>>>> without have the shared context ID >>>>>>>>> to use the action update. >>>>>>>> >>>>>>>> Can you please explain your idea? >>>>>>> >>>>>>> I see two types of HW schemes supporting action updates without going >>>>>>> through call `rte_flow_destroy()` and call `rte_flow_create()` >>>>>>> - The shared HW action context feature >>>>>>> - The HW has "pattern" and "action" mapped to different HW objects >> and >>>>>>> action can be updated any time. >>>>>>> Other than above-mentioned RSS use case, another use case would be to >>>>>>> a) create rte_flow and set the action as DROP (Kind of reserving the HW >>>> object) >>>>>>> b) Update the action only when the rest of the requirements ready. >>>>>>> >>>>>>> Any API schematic that supports both notions of HW is fine with me. >>>>>>> >>>>>> I have an idea if the API will be changed to something like this, >>>>>> Rte_flow_shared_action_update(uint16_port port, rte_shared_ctx *ctx, >>>> rte_flow_action *action, error) >>>>>> This will enable the application to send a different action than the original >>>> one to be switched. >>>>>> Assuming the PMD supports this. >>>>>> Does it answer your concerns? >>>>> >>>>> >>>>> This allows both: >>>>> 1. Update action configuration >>>>> 2. Replace action by some other action >>>>> For 2 pure software implementation may carate shred action (that can be >>>> shared >>>>> with one flow only, depends on PMD) and later on >>>> rte_flow_shared_action_update may replace this >>>>> action with some other action by handle returned from >>>> rte_flow_shared_action_create >>>>> Doesign between 1 and 2 is per PMD. >>>> >>>> struct rte_flow * object holds the driver representation of the >>>> pattern + action. >>>> So in order to update the action, we would need struct rte_flow * in API. >>>> >>> Why is that? The idea is to change the action, the action itself is connected to >> flows. >>> The PMD can save in the shared_ctx all flows that are connected to this >> action. >>> >>>> I think, simple API change would be to accommodate "rte_shared_ctx >>>> *ctx, rte_flow_action *action" modes >>>> without introducing the emulation for one or other mode, will be. >>>> >>>> enum rte_flow_action_update_type { >>>> RTE_FLOW_ACTION_UPDATE_TYPE_SHARED_ACTION, >>>> RTE_FLOW_ACTION_UPDATE_TYPE_ACTION, >>>> }; >>>> >>>> struct rte_flow_action_update_type_param { >>>> enum rte_flow_action_update_type type; >>>> union { >>>> struct rte_flow_action_update_type_shared_action_param { >>>> rte_shared_ctx *ctx; >>>> } shared_action; >>>> struct rte_flow_action_update_type_shared_action_param { >>>> rte_flow *flow, >>>> rte_flow_action *action; >>>> } action; >>>> } >>>> } >>>> >>> Thank you for the idea but I fall to see how your suggested API is simpler than >> the one suggested by me? >>> In my suggestion the PMD simply needs to check if the new action and >> change the >>> context and to that action, or just change parameters in the action, if it is the >> same action. >>> >>> Let's go with the original patch API modified to support like you requested >> also changing the action, >>> based on my comments. >>> >>>> rte_flow_action_update(uint16_port port, struct >>>> rte_flow_action_update_type_param *param, error) >>>> >>>>> >>>>>> >>>>>>> >>>>>>>> As I can see if we use the flow_action array it may result in bugs. >>>>>>>> For example, the application created two flows with the same RSS (not >>>> using >>>>>>> the context) >>>>>>>> Then he wants to change one flow to use different RSS, but the result >> will >>>> that >>>>>>> both flows >>>>>>>> will be changed. >>>>>>> >>>>>>> Sorry. I don't quite follow this. >>>>>>> >>>>>> I was trying to show that there must be some context. But I don’t think this >> is >>>> relevant to >>>>>> your current ideas. >>>>>> >>>>>>>> Also this will enforce the PMD to keep track on all flows which will have >>>>>>> memory penalty for >>>>>>>> some PMDs. >> >> Hi Ori, Andrey, >> >> This is a set of new APIs and we are very close to the -rc1, so we have only a >> few days to close the feature to merge them for this release. >> >> Also accompanying PMD and testpmd implementation with the proposed API >> changes >> looks missing. >> >> We can either postpone the patchset to next release to give time for more >> PMD >> owners to participate, which can give better API for long term. >> Or try to to squeeze into this release taking into account that the APIs will be >> experimental. >> >> What do you think, what is you schedule for the feature, do you have room to >> postpone it? > Not so much it is an important API for Mellanox. Got it. > >> If not, first existing discussions needs to resolved, and it is good to have the >> PMD and testpmd implementations, do you think can this be done for next few >> days? >> > I think that this is the correct API to implement, I fully agree that this API is experimental > just like any other new API, and might change based on comments and use cases. > I know that Mellanox is committed to this feature and that Andrey is working around the clock > to complete the missing parts, and should have a version by tomorrow (July 8th ) evening. OK > (with update to flow filtering sample app, testpmd will not be ready by RC1, but it will be for RC2) > We would like very much to push it in this version. OK, please conclude the existing discussion before finalizing.