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 D6F36A0524; Sat, 4 Jul 2020 12:10:16 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0A78A1C201; Sat, 4 Jul 2020 12:10:16 +0200 (CEST) Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) by dpdk.org (Postfix) with ESMTP id 820501C01F for ; Sat, 4 Jul 2020 12:10:13 +0200 (CEST) Received: by mail-lj1-f193.google.com with SMTP id 9so39711229ljv.5 for ; Sat, 04 Jul 2020 03:10:13 -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=U8djqO8JaQPM5DBc1dl+PSQiGhFhO1rAwA8TeHrqBT0=; b=T64xAdM5hFCgQBDJeUmIk/+dqNlwF8sj0UEPWibpEcysFzXGe1hP+suwO4njhkMPfu QML1Er3Fzd2RYbM4TD3+XyGMZhmq+C1tS0WC79m0LkRn7/Ory5aVQVw2gBpB6q8+KOlF XdN2GJ/1wZfYj633zkpxRgBSA1YmkJe1QOV0J9ePWSNZYMKs3p1RrmJX97mTSOvvQFq+ qzRrP7u4/iQ+BWRDgMYk01MB0kmNjUSkXbyeybaow8kmDRWIQ7KIDfLqRJQo0ug1fgSC 43UR+pnig2R0tcIRoKT/16UlNxiccwvn1pMnZ+dFxO0RqhUjdblGYoQsCWNwKvvvJj1m Q7kw== 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=U8djqO8JaQPM5DBc1dl+PSQiGhFhO1rAwA8TeHrqBT0=; b=PrVIRCR6+7Y4Frx9DnrOp9ini0r/hNF2nKkB3VAG2pPFrKJsgJdP9o/Ps7LbAVDVW8 LLRU+zx61ziaghHx12nnNCYJqlZOEwhTcgoriB0zTF11z4r7vngWljHYF+tbwOM7TZwZ UyooTSn40KuFtHBXGQrUzAJ5HFvpIqDnpZh3dZ6knMoAHv5f7wYXNDAHaVsoTSP6yBbV 6xhz41cHhvfVfJLYhDRkfC7cSi6ZKV65qLf7MqrE+n7WgW5R3VQzlKojNVbM4g6KOcct HP1fVAqp1fNwMrS6v2lKHYrYnsPSCRaY3yeGppWEayKeBhweuV8BzALo+ZC7mDQ5nDbe B3IA== X-Gm-Message-State: AOAM531t11i4HTMr8IhTKjTnS0gavt2sS+5Vph2KTwYzVAA0dXTOL+KK jgCYLUwQTleY9BTZSCZ0o5wyq5ddg2WmooJPS00= X-Google-Smtp-Source: ABdhPJyhdcn7tI/cq6SJBBWZiWU1ZXmkUZUH07uKI6PmOlLZyckW1w/MccS6fx/kJq05QlopvqJnaD7LGXdUw+80uFg= X-Received: by 2002:a05:651c:1aa:: with SMTP id c10mr17732159ljn.260.1593857412575; Sat, 04 Jul 2020 03:10:12 -0700 (PDT) MIME-Version: 1.0 References: <20200702120511.16315-1-andreyv@mellanox.com> In-Reply-To: From: Andrey Vesnovaty Date: Sat, 4 Jul 2020 13:10:01 +0300 Message-ID: To: Jerin Jacob Cc: Andrey Vesnovaty , Thomas Monjalon , Ferruh Yigit , Andrew Rybchenko , Ori Kam , dpdk-dev Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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" 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. > > > > > --- > > This patch based on RFC: https://patches.dpdk.org/patch/71820/ > > > > --- > > lib/librte_ethdev/rte_ethdev_version.map | 6 + > > lib/librte_ethdev/rte_flow.c | 81 +++++++++++++ > > lib/librte_ethdev/rte_flow.h | 148 ++++++++++++++++++++++- > > lib/librte_ethdev/rte_flow_driver.h | 22 ++++ > > 4 files changed, 256 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_ethdev/rte_ethdev_version.map > b/lib/librte_ethdev/rte_ethdev_version.map > > index 3f32fdecf..e291c2bd9 100644 > > --- a/lib/librte_ethdev/rte_ethdev_version.map > > +++ b/lib/librte_ethdev/rte_ethdev_version.map > > @@ -230,4 +230,10 @@ EXPERIMENTAL { > > > > # added in 20.02 > > rte_flow_dev_dump; > > + > > + # added in 20.08 > > + rte_flow_shared_action_create; > > + rte_flow_shared_action_destoy; > > + rte_flow_shared_action_update; > > + rte_flow_shared_action_query; > > }; > > diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c > > index 885a7ff9a..7728057c3 100644 > > --- a/lib/librte_ethdev/rte_flow.c > > +++ b/lib/librte_ethdev/rte_flow.c > > @@ -1231,3 +1231,84 @@ rte_flow_dev_dump(uint16_t port_id, FILE *file, > struct rte_flow_error *error) > > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > NULL, rte_strerror(ENOSYS)); > > } > > + > > +struct rte_flow_shared_action * > > +rte_flow_shared_action_create(uint16_t port_id, > > + const struct rte_flow_action *action, > > + struct rte_flow_error *error) > > +{ > > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > > + struct rte_flow_shared_action *shared_action; > > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, > error); > > + > > + if (unlikely(!ops)) > > + return NULL; > > + if (likely(!!ops->shared_action_create)) { > > + shared_action = ops->shared_action_create(dev, action, > error); > > + if (shared_action == NULL) > > + flow_err(port_id, -rte_errno, error); > > + return shared_action; > > + } > > + rte_flow_error_set(error, ENOSYS, > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > + NULL, rte_strerror(ENOSYS)); > > + return NULL; > > +} > > + > > +int > > +rte_flow_shared_action_destoy(uint16_t port_id, > > + struct rte_flow_shared_action *action, > > + struct rte_flow_error *error) > > +{ > > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, > error); > > + > > + if (unlikely(!ops)) > > + return -rte_errno; > > + if (likely(!!ops->shared_action_destroy)) > > + return flow_err(port_id, > > + ops->shared_action_destroy(dev, action, > error), > > + error); > > + return rte_flow_error_set(error, ENOSYS, > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > + NULL, rte_strerror(ENOSYS)); > > +} > > + > > +int > > +rte_flow_shared_action_update(uint16_t port_id, > > + struct rte_flow_shared_action *action, > > + const void *action_conf, > > + struct rte_flow_error *error) > > +{ > > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, > error); > > + > > + if (unlikely(!ops)) > > + return -rte_errno; > > + if (likely(!!ops->shared_action_update)) > > + return flow_err(port_id, ops->shared_action_update(dev, > action, > > + action_conf, error), > > + error); > > + return rte_flow_error_set(error, ENOSYS, > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > + NULL, rte_strerror(ENOSYS)); > > +} > > + > > +int > > +rte_flow_shared_action_query(uint16_t port_id, > > + const struct rte_flow_shared_action *action, > > + void *data, > > + struct rte_flow_error *error) > > +{ > > + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > > + const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, > error); > > + > > + if (unlikely(!ops)) > > + return -rte_errno; > > + if (likely(!!ops->shared_action_query)) > > + return flow_err(port_id, ops->shared_action_query(dev, > action, > > + data, error), > > + error); > > + return rte_flow_error_set(error, ENOSYS, > > + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > > + NULL, rte_strerror(ENOSYS)); > > +} > > diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h > > index 5625dc491..98140ebb1 100644 > > --- a/lib/librte_ethdev/rte_flow.h > > +++ b/lib/librte_ethdev/rte_flow.h > > @@ -1643,7 +1643,8 @@ enum rte_flow_action_type { > > /** > > * Enables counters for this flow rule. > > * > > - * These counters can be retrieved and reset through > rte_flow_query(), > > + * These counters can be retrieved and reset through > rte_flow_query() or > > + * rte_flow_shared_action_query() if the action provided via > handle, > > * see struct rte_flow_query_count. > > * > > * See struct rte_flow_action_count. > > @@ -2051,6 +2052,14 @@ enum rte_flow_action_type { > > * See struct rte_flow_action_set_dscp. > > */ > > RTE_FLOW_ACTION_TYPE_SET_IPV6_DSCP, > > + > > + /** > > + * Describes action shared a cross multiple flow rules. > > + * > > + * Enables multiple rules reference the same action by handle > (see > > + * struct rte_flow_shared_action). > > + */ > > + RTE_FLOW_ACTION_TYPE_SHARED, > > }; > > > > /** > > @@ -2593,6 +2602,20 @@ struct rte_flow_action_set_dscp { > > uint8_t dscp; > > }; > > > > + > > +/** > > + * RTE_FLOW_ACTION_TYPE_SHARED > > + * > > + * Opaque type returned after successfully creating a shared action. > > + * > > + * This handle can be used to manage and query the related action: > > + * - share it a cross multiple flow rules > > + * - update action configuration > > + * - query action data > > + * - destroy action > > + */ > > +struct rte_flow_shared_action; > > + > > /* Mbuf dynamic field offset for metadata. */ > > extern int rte_flow_dynf_metadata_offs; > > > > @@ -3224,6 +3247,129 @@ rte_flow_conv(enum rte_flow_conv_op op, > > const void *src, > > struct rte_flow_error *error); > > > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice. > > + * > > + * Create shared action for reuse in multiple flow rules. > > + * > > + * @param[in] port_id > > + * The port identifier of the Ethernet device. > > + * @param[in] action > > + * Action configuration for shared action creation. > > + * @param[out] error > > + * Perform verbose error reporting if not NULL. PMDs initialize this > > + * structure in case of error only. > > + * @return > > + * A valid handle in case of success, NULL otherwise and rte_errno is > set > > + * to one of the error codes defined: > > + * - (ENOSYS) if underlying device does not support this > functionality. > > + * - (EIO) if underlying device is removed. > > + * - (EINVAL) if *action* invalid. > > + * - (ENOTSUP) if *action* valid but unsupported. > > + */ > > +__rte_experimental > > +struct rte_flow_shared_action * > > +rte_flow_shared_action_create(uint16_t port_id, > > + const struct rte_flow_action *action, > > + struct rte_flow_error *error); > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice. > > + * > > + * Destroys the shared action by handle. > > + * > > + * @param[in] port_id > > + * The port identifier of the Ethernet device. > > + * @param[in] action > > + * Handle for the shared action to be destroyed. > > + * @param[out] error > > + * Perform verbose error reporting if not NULL. PMDs initialize this > > + * structure in case of error only. > > + * @return > > + * - (0) if success. > > + * - (-ENOSYS) if underlying device does not support this > functionality. > > + * - (-EIO) if underlying device is removed. > > + * - (-ENOENT) if action pointed by *action* handle was not found. > > + * - (-ETOOMANYREFS) if action pointed by *action* handle still used > by one or > > + * more rules > > + * rte_errno is also set. > > + */ > > +__rte_experimental > > +int > > +rte_flow_shared_action_destoy(uint16_t port_id, > > + struct rte_flow_shared_action *action, > > + struct rte_flow_error *error); > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice. > > + * > > + * Updates inplace the shared action configuration pointed by *action* > handle > > + * with the configuration provided as *action_conf* argument. > > + * The update of the shared action configuration effects all flow rules > reusing > > + * the action via handle. > > + * > > + * @param[in] port_id > > + * The port identifier of the Ethernet device. > > + * @param[in] action > > + * Handle for the shared action to be updated. > > + * @param[in] action_conf > > + * Action specification used to modify the action pointed by handle. > > + * action_conf should be of same type with the action pointed by the > *action* > > + * handle argument, otherwise function behavior undefined. > > + * @param[out] error > > + * Perform verbose error reporting if not NULL. PMDs initialize this > > + * structure in case of error only. > > + * @return > > + * - (0) if success. > > + * - (-ENOSYS) if underlying device does not support this > functionality. > > + * - (-EIO) if underlying device is removed. > > + * - (-EINVAL) if *action_conf* invalid. > > + * - (-ENOTSUP) if *action_conf* valid but unsupported. > > + * - (-ENOENT) if action pointed by *ctx* was not found. > > + * rte_errno is also set. > > + */ > > +__rte_experimental > > +int > > +rte_flow_shared_action_update(uint16_t port_id, > > + struct rte_flow_shared_action *action, > > + const void *action_conf, > > + struct rte_flow_error *error); > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice. > > + * > > + * Query the shared action by handle. > > + * > > + * This function allows retrieving action-specific data such as > counters. > > + * Data is gathered by special action which may be present/referenced in > > + * more than one flow rule definition. > > + * > > + * \see RTE_FLOW_ACTION_TYPE_COUNT > > + * > > + * @param port_id > > + * Port identifier of Ethernet device. > > + * @param[in] action > > + * Handle for the shared action to query. > > + * @param[in, out] data > > + * Pointer to storage for the associated query data type. > > + * @param[out] error > > + * Perform verbose error reporting if not NULL. PMDs initialize this > > + * structure in case of error only. > > + * > > + * @return > > + * 0 on success, a negative errno value otherwise and rte_errno is > set. > > + */ > > +__rte_experimental > > +int > > +rte_flow_shared_action_query(uint16_t port_id, > > + const struct rte_flow_shared_action *action, > > + void *data, > > + struct rte_flow_error *error); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/lib/librte_ethdev/rte_flow_driver.h > b/lib/librte_ethdev/rte_flow_driver.h > > index 51a9a57a0..c103d159e 100644 > > --- a/lib/librte_ethdev/rte_flow_driver.h > > +++ b/lib/librte_ethdev/rte_flow_driver.h > > @@ -101,6 +101,28 @@ struct rte_flow_ops { > > (struct rte_eth_dev *dev, > > FILE *file, > > struct rte_flow_error *error); > > + /** See rte_flow_shared_action_create() */ > > + struct rte_flow_shared_action *(*shared_action_create) > > + (struct rte_eth_dev *dev, > > + const struct rte_flow_action *action, > > + struct rte_flow_error *error); > > + /** See rte_flow_shared_action_destroy() */ > > + int (*shared_action_destroy) > > + (struct rte_eth_dev *dev, > > + struct rte_flow_shared_action *shared_action, > > + struct rte_flow_error *error); > > + /** See rte_flow_shared_action_update() */ > > + int (*shared_action_update) > > + (struct rte_eth_dev *dev, > > + struct rte_flow_shared_action *shared_action, > > + const void *action_conf, > > + struct rte_flow_error *error); > > + /** See rte_flow_shared_action_query() */ > > + int (*shared_action_query) > > + (struct rte_eth_dev *dev, > > + const struct rte_flow_shared_action *shared_action, > > + void *data, > > + struct rte_flow_error *error); > > }; > > > > /** > > -- > > 2.26.2 > > >