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 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 <dev@dpdk.org>; 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 <orika@mellanox.com>, Jerin Jacob <jerinjacobk@gmail.com>,
Andrey Vesnovaty <andrey.vesnovaty@gmail.com>
Cc: Andrey Vesnovaty <andreyv@mellanox.com>,
Thomas Monjalon <thomas@monjalon.net>,
Andrew Rybchenko <arybchenko@solarflare.com>, dpdk-dev <dev@dpdk.org>
References: <20200702120511.16315-1-andreyv@mellanox.com>
<CALBAE1O8mZS5q_hvSeJ91P_YzyYfXyb-VWyv5R=DQ4W8URdTFQ@mail.gmail.com>
<CAOwx9Su5t1XwFppqFgjNJuxj4JiE5O96nQXXiDQ=bFROV4nXGA@mail.gmail.com>
<CALBAE1PT-Wpr8b9AZO8wRqwh1YWTQwn9ddeFQBoo5w60s=iokw@mail.gmail.com>
<AM6PR05MB517660041EAF868EAFCF6F69DB680@AM6PR05MB5176.eurprd05.prod.outlook.com>
<CALBAE1MzSqSf=yWbGXr7=4ny6LKUnDgm5wieD8C4ev83pZfA0g@mail.gmail.com>
<AM6PR05MB51762BE0AD2C6D927F225B8BDB690@AM6PR05MB5176.eurprd05.prod.outlook.com>
<CAOwx9SsMToGpxkynQ4nsXECsNiCqo3K6UF=hTwXNmaZVa0Sbug@mail.gmail.com>
<CALBAE1OhmJUM+SoFw-xf2X6hPBwXzsCwJ1j5tZruFnv-9dtMpg@mail.gmail.com>
<AM6PR05MB5176010767C6BF5B68943E0BDB660@AM6PR05MB5176.eurprd05.prod.outlook.com>
<82a903f4-fb37-4c58-5449-31401b7dfd04@intel.com>
<AM6PR05MB517682B2453DA3B6F4E0614CDB660@AM6PR05MB5176.eurprd05.prod.outlook.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
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: <AM6PR05MB517682B2453DA3B6F4E0614CDB660@AM6PR05MB5176.eurprd05.prod.outlook.com>
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 <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>
On 7/7/2020 6:24 PM, Ori Kam wrote:
> Hi Ferruh,
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>>
>> On 7/7/2020 7:21 AM, Ori Kam wrote:
>>> Hi Jerin,
>>> Thanks you for your quick reply.
>>>
>>>> -----Original Message-----
>>>> From: Jerin Jacob <jerinjacobk@gmail.com>
>>>> Subject: Re: [dpdk-dev] [PATCH] add flow shared action API
>>>>
>>>> On Mon, Jul 6, 2020 at 7:02 PM Andrey Vesnovaty
>>>> <andrey.vesnovaty@gmail.com> 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 <orika@mellanox.com> wrote:
>>>>>>
>>>>>> Hi Jerin,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Jerin Jacob <jerinjacobk@gmail.com>
>>>>>>> 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 <orika@mellanox.com> wrote:
>>>>>>>>
>>>>>>>> Hi Jerin,
>>>>>>>> PSB,
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Ori
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Jerin Jacob <jerinjacobk@gmail.com>
>>>>>>>>> Sent: Saturday, July 4, 2020 3:33 PM
>>>>>>>>> dpdk-dev <dev@dpdk.org>
>>>>>>>>> Subject: Re: [dpdk-dev] [PATCH] add flow shared action API
>>>>>>>>>
>>>>>>>>> On Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty
>>>>>>>>> <andrey.vesnovaty@gmail.com> 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.