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 F1710A0524; Sat, 4 Jul 2020 14:33:41 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 510FC1DB5B; Sat, 4 Jul 2020 14:33:41 +0200 (CEST) Received: from mail-io1-f65.google.com (mail-io1-f65.google.com [209.85.166.65]) by dpdk.org (Postfix) with ESMTP id 0A4861DB5A for ; Sat, 4 Jul 2020 14:33:39 +0200 (CEST) Received: by mail-io1-f65.google.com with SMTP id v6so21207908iob.4 for ; Sat, 04 Jul 2020 05:33:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=L4jnrNW2L30Xycd2PTNsCPzqet8a0mC480hsk6F6lBY=; b=sH2v1U7/4tYjyXH6OlKFqjXS6qeZ1ujHy4SfbH89EusyK4qNWgUV6/vvCNLpLw5TaB KhMx8gUoirhnA/Ve4wygezrd6b8izrwuKMUHRXWbcJ18dNLRfWZenvs6sdDsLG8ZFv1i FvcyGFO5kOnJU0EW0xTC5PzyTQ/f9EurrFkCod6+jiwb1RljMK+anaUlZoEK0+SybiNZ 4oaglNftNQV1n2fmLnxd4C5Cn9oiXAiNxepuOocdm6dXroEeYHh05ObNE00qJqXEQCa9 upSDsFjbqDFRhC33e8SsHYUKqtTMFAlbV9P/45Sfy3mk1RLLQTvO1ohxrrvQoX+eMDhd gNmg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=L4jnrNW2L30Xycd2PTNsCPzqet8a0mC480hsk6F6lBY=; b=bKu3L7b3pkscc9glcfjG3RJ9i6rVmMmy/ApnGZwBUv6rHsHRI6E4SBFiLI3/OU+ZyA ZR0b/BWDrkIsiaE/42EnuFsrr6kkk51JfWNddPdDJhgtV4Z3qRcvvtRpBmIWUznl9NKW I3z5W4wIRODup+j9d3eYVtOLXmsKM6E6efpNkHddVrq3mBjuYztnSugVnZ9lQoAY2Iqy UgOmnx0TEIVdr/2N4lgQFyW6V/U0NblF87mW54Dk8cxkSnxrlypfVQWLw1KQZeB51a/n 6ULHJ2meTlV/RDcgcUUXk/LVCtV0/zAZNcxByvsz5JZYJ4erLKu/gLormhu9R1qaVpgg 9oFQ== X-Gm-Message-State: AOAM5338RwHuddc+kLXWiLykQaX08BguWKoqjYK4lUuzmeIRj4JH6A2d 6IPM1Y7U/C6MjC6fNCghBHiYxfPttWLEB2b3Lds= X-Google-Smtp-Source: ABdhPJwn2Ne2kyWd1Cpp/6vdtvNS8SGabdhHr/j7qlYKiX79JxRCL/l41hJ8wpEy/1LL8UGJtyj0FrZioYPJaetHyME= X-Received: by 2002:a05:6638:1511:: with SMTP id b17mr36702973jat.113.1593866018161; Sat, 04 Jul 2020 05:33:38 -0700 (PDT) MIME-Version: 1.0 References: <20200702120511.16315-1-andreyv@mellanox.com> In-Reply-To: From: Jerin Jacob Date: Sat, 4 Jul 2020 18:03:22 +0530 Message-ID: To: Andrey Vesnovaty Cc: Andrey Vesnovaty , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ori Kam , dpdk-dev Content-Type: text/plain; charset="UTF-8" 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 Sat, Jul 4, 2020 at 3:40 PM Andrey Vesnovaty wrote: > > > Thanks, > > Andrey Vesnovaty > (+972)526775512 | Skype: andrey775512 > > > On Fri, Jul 3, 2020 at 6:02 PM Jerin Jacob wrote: >> >> On Fri, Jul 3, 2020 at 8:07 PM Andrey Vesnovaty wrote: >> > >> > From: Andrey Vesnovaty >> > >> > This commit introduces extension of DPDK flow action API enabling >> > sharing of single rte_flow_action in multiple flows. The API intended for >> > PMDs where multiple HW offloaded flows can reuse the same HW >> > essence/object representing flow action and modification of such an >> > essence/object effects all the rules using it. >> > >> > Motivation and example >> > === >> > Adding or removing one or more queues to RSS used by multiple flow rules >> > imposes per rule toll for current DPDK flow API; the scenario requires >> > for each flow sharing cloned RSS action: >> > - call `rte_flow_destroy()` >> > - call `rte_flow_create()` with modified RSS action >> > >> > API for sharing action and its in-place update benefits: >> > - reduce the overhead of multiple RSS flow rules reconfiguration . >> > - optimize resource utilization by sharing action across of of multiple >> > flows >> > >> > Change description >> > === >> > >> > Shared action >> > === >> > In order to represent flow action shared by multiple flows new action >> > type RTE_FLOW_ACTION_TYPE_SHARED introduced (see `enum >> > rte_flow_action_type`). >> > Actually the introduced API decouples action from any specific flow and >> > enables sharing of single action by its handle for multiple flows. >> > >> > Shared action create/use/destroy >> > === >> > Shared action may be reused by some or none flow rules at any given >> > moment, IOW shared action reside outside of the context of any flow. >> > Shared action represent HW resources/objects used for action offloading >> > implementation. For allocation/release of all HW resources and all >> > related initializations/cleanups in PMD space required for shared action >> > implementation added new API >> > rte_flow_shared_action_create()/rte_flow_shared_action_destroy(). >> > In addition to the above all preparations needed to maintain shared >> > access to the action resources, configuration and state should be done in >> > rte_flow_shared_action_create(). >> > >> > In order to share some flow action reuse the handle of type >> > `struct rte_flow_shared_action` returned by >> > rte_flow_shared_action_create() as a `conf` field of >> > `struct rte_flow_action` (see "example" section). >> > >> > If some shared action not used by any flow rule all resources allocated >> > by the shared action can be released by rte_flow_shared_action_destroy() >> > (see "example" section). The shared action handle passed as argument to >> > destroy API should not be used i.e. result of the usage is undefined. >> > >> > Shared action re-configuration >> > === >> > Shared action behavior defined by its configuration & and can be updated >> > via rte_flow_shared_action_update() (see "example" section). The shared >> > action update operation modifies HW related resources/objects allocated >> > by the action. The number of operations performed by the update operation >> > should not be dependent on number of flows sharing the related action. >> > On return of shared action updated API action behavior should be >> > according to updated configuration for all flows sharing the action. >> > >> > Shared action query >> > === >> > Provide separate API to query shared action sate (see >> > rte_flow_shared_action_update()). Taking a counter as an example: query >> > returns value aggregating all counter increments across all flow rules >> > sharing the counter. >> > >> > PMD support >> > === >> > The support of introduced API is pure PMD specific design and >> > responsibility for each action type (see struct rte_flow_ops). >> > >> > testpmd >> > === >> > In order to utilize introduced API testpmd cli may implement following >> > extension >> > create/update/destroy/query shared action accordingly >> > >> > flow shared_action create {port_id} [index] {action} >> > flow shared_action update {port_id} {index} {action} >> > flow shared_action destroy {port_id} {index} >> > flow shared_action query {port_id} {index} >> > >> > testpmd example >> > === >> > >> > configure rss to queues 1 & 2 >> > >> > testpmd> flow shared_action create 0 100 rss 1 2 >> > >> > create flow rule utilizing shared action >> > >> > testpmd> flow create 0 ingress \ >> > pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \ >> > actions shared 100 end / end >> > >> > add 2 more queues >> > >> > testpmd> flow shared_action modify 0 100 rss 1 2 3 4 >> > >> > example >> > === >> > >> > struct rte_flow_action actions[2]; >> > struct rte_flow_action action; >> > /* skipped: initialize action */ >> > struct rte_flow_shared_action *handle = rte_flow_shared_action_create( >> > port_id, &action, &error); >> > actions[0].type = RTE_FLOW_ACTION_TYPE_SHARED; >> > actions[0].conf = handle; >> > actions[1].type = RTE_FLOW_ACTION_TYPE_END; >> > /* skipped: init attr0 & pattern0 args */ >> > struct rte_flow *flow0 = rte_flow_create(port_id, &attr0, pattern0, >> > actions, error); >> > /* create more rules reusing shared action */ >> > struct rte_flow *flow1 = rte_flow_create(port_id, &attr1, pattern1, >> > actions, error); >> > /* skipped: for flows 2 till N */ >> > struct rte_flow *flowN = rte_flow_create(port_id, &attrN, patternN, >> > actions, error); >> > /* update shared action */ >> > struct rte_flow_action updated_action; >> > /* >> > * skipped: initialize updated_action according to desired action >> > * configuration change >> > */ >> > rte_flow_shared_action_update(port_id, handle, updated_action.conf, >> > error); >> > /* >> > * from now on all flows 1 till N will act according to configuration of >> > * updated_action >> > */ >> > /* skipped: destroy all flows 1 till N */ >> > rte_flow_shared_action_destroy(port_id, handle, error); >> > >> > Signed-off-by: Andrey Vesnovaty >> > Signed-off-by: Andrey Vesnovaty >> > Signed-off-by: Andrey Vesnovaty >> >> Duplicate Signoffs. >> >> # Request to CC all the people who are already giving comments to this patch. >> # The Following comment is not addressed in this patch. >> http://mails.dpdk.org/archives/dev/2020-July/172408.html > > > 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.