DPDK patches and discussions
 help / color / mirror / Atom feed
From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
To: Andrey Vesnovaty <andreyv@nvidia.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "jer@marvell.com" <jer@marvell.com>,
	"jerinjacobk@gmail.com" <jerinjacobk@gmail.com>,
	NBU-Contact-Thomas Monjalon <thomas@monjalon.net>,
	"ferruh.yigit@intel.com" <ferruh.yigit@intel.com>,
	"stephen@networkplumber.org" <stephen@networkplumber.org>,
	"bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	Ori Kam <orika@nvidia.com>,
	Slava Ovsiienko <viacheslavo@nvidia.com>,
	"andrey.vesnovaty@gmail.com" <andrey.vesnovaty@gmail.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>,
	"nhorman@tuxdriver.com" <nhorman@tuxdriver.com>,
	"ajit.khaparde@broadcom.com" <ajit.khaparde@broadcom.com>,
	"samik.gupta@broadcom.com" <samik.gupta@broadcom.com>
Subject: Re: [dpdk-dev] [PATCH v7 1/2] ethdev: add flow shared action API
Date: Wed, 14 Oct 2020 09:49:34 +0300	[thread overview]
Message-ID: <d3a63fbe-7373-bfd7-b166-12c56fe782c6@oktetlabs.ru> (raw)
In-Reply-To: <MWHPR1201MB2525B99A326205E8A34AABDCDB040@MWHPR1201MB2525.namprd12.prod.outlook.com>

Hi Andrey,

On 10/13/20 11:06 PM, Andrey Vesnovaty wrote:
> Hi Andrew.
> 
> Thanks for the input.
> All spelling & rephrases will be applied  PSB for the rest.
> Will publish v8 very soon.
> 
> Thanks,
> Andrey
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>
>> Sent: Monday, October 12, 2020 5:19 PM
>> To: Andrey Vesnovaty <andreyv@nvidia.com>; dev@dpdk.org
>> Cc: jer@marvell.com; jerinjacobk@gmail.com; NBU-Contact-Thomas Monjalon
>> <thomas@monjalon.net>; ferruh.yigit@intel.com;
>> stephen@networkplumber.org; bruce.richardson@intel.com; Ori Kam
>> <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
>> andrey.vesnovaty@gmail.com; mdr@ashroe.eu; nhorman@tuxdriver.com;
>> ajit.khaparde@broadcom.com; samik.gupta@broadcom.com
>> Subject: Re: [dpdk-dev] [PATCH v7 1/2] ethdev: add flow shared action API
>>
>> "add flow shared action API"
>>
>> Is flow shared? May be "add shared actions to flow API".
> 
> Accepted, will update commit message.
>>
>> On 10/8/20 2:51 PM, Andrey Vesnovaty wrote:
>>> This commit introduces extension of DPDK flow action API enabling
>>
>> "This commit" and "DPDK" are not necessary in description.
>> It is a commit description and the patch is to DPDK tree.
>> Consider just:
>> "Introduce extension of flow action API enabling..."
> 
> Accepted, will update commit message.
>>
>>> 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 affects 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 multiple
>>>   flows
>>>
>>> Change description
>>> ===
>>>
>>> Shared action
>>> ===
>>> In order to represent flow action shared by multiple flows new action
>>> type RTE_FLOW_ACTION_TYPE_SHARED is 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 across multiple flows.
>>>
>>> Shared action create/use/destroy
>>> ===
>>> Shared action may be reused by some or none flow rules at any given
>>> moment, i.e. shared action resides outside of the context of any flow.
>>> Shared action represent HW resources/objects used for action offloading
>>> implementation.
>>> API for shared action create (see `rte_flow_shared_action_create()`):
>>> - should allocate HW resources and make related initializations required
>>>   for shared action implementation.
>>> - make necessary preparations to maintain shared access to
>>>   the action resources, configuration and state.
>>> API for shared action destroy (see `rte_flow_shared_action_destroy()`)
>>> should release HW resources and make related cleanups required for shared
>>> action implementation.
>>>
>>> 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 any further i.e. result of the usage is
>>> undefined.
>>>
>>> Shared action re-configuration
>>> ===
>>> Shared action behavior defined by its configuration can be updated via
>>> rte_flow_shared_action_update() (see "example" section). The shared
>>> action update operation modifies HW related resources/objects allocated
>>> on the action creation. The number of operations performed by the update
>>> operation should not depend on the number of flows sharing the related
>>> action. On return of shared action update 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 state (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. This API doesn't query shared action configuration
>>> since it is controlled by rte_flow_shared_action_create() and
>>> rte_flow_shared_action_update() APIs and no supposed to change by other
>>> means.
>>>
>>> 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 (port) create {action_id (id)} (action) / end
>>> flow shared_action (port) update (id) (action) / end
>>> flow shared_action (port) destroy action_id (id) {action_id (id) [...]}
>>> flow shared_action (port) query (id)
>>>
>>> testpmd example
>>> ===
>>>
>>> configure rss to queues 1 & 2
>>>
>>>> flow shared_action 0 create action_id 100 rss queues 1 2 end / end
>>>
>>> create flow rule utilizing shared action
>>>
>>>> flow create 0 ingress \
>>>     pattern eth dst is 0c:42:a1:15:fd:ac / ipv6 / tcp / end \
>>>   actions shared 100 / end
>>>
>>> add 2 more queues
>>>
>>>> flow shared_action 0 modify 100 rss queues 1 2 3 4 end / end
>>
>> testpmd is out-of-scope of the patch and it is better to
>> remove the description to avoid unsync if testpmd
>> commands are changed.
> 
> There is still added value is testpmd example demonstrating usage of
> shared action with rte flows. I will update the example to reflect the current
> testpmd syntax for shared action.

Yes and no. IMHO It would be OK for series description etc,
but not OK for the changeset description which will be
kept in the history and will become misleading when
commands are changed. I think that no information is better
than potentially wrong information.

[snip]

>>> diff --git a/lib/librte_ethdev/rte_ethdev_version.map
>> b/lib/librte_ethdev/rte_ethdev_version.map
>>> index c95ef5157a..a8a4821dbb 100644
>>> --- a/lib/librte_ethdev/rte_ethdev_version.map
>>> +++ b/lib/librte_ethdev/rte_ethdev_version.map
>>> @@ -229,6 +229,10 @@ EXPERIMENTAL {
>>>  	# added in 20.11
>>>  	rte_eth_link_speed_to_str;
>>>  	rte_eth_link_to_str;
>>> +	rte_flow_shared_action_create;
>>> +	rte_flow_shared_action_destroy;
>>> +	rte_flow_shared_action_update;
>>> +	rte_flow_shared_action_query;
>>
>> Shouldn't it be alphabetically sorted?
> 
> Query before update?

yes

[snip]

>>> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
>>> index da8bfa5489..9050adec23 100644
>>> --- a/lib/librte_ethdev/rte_flow.h
>>> +++ b/lib/librte_ethdev/rte_flow.h

[snip]

>>> @@ -3357,6 +3380,150 @@ int
>>>  rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
>>>  			uint32_t nb_contexts, struct rte_flow_error *error);
>>>
>>> +/**
>>> + * Specify shared action configuration
>>> + */
>>> +struct rte_flow_shared_action_conf {
>>> +	/**
>>> +	 * Flow direction for shared action configuration.
>>> +	 *
>>> +	 * Shred action should be valid at least for one flow direction,
>>> +	 * otherwise it is invalid for both ingress and egress rules.
>>> +	 */
>>> +	uint32_t ingress:1;
>>> +	/**< Action valid for rules applied to ingress traffic. */
>>> +	uint32_t egress:1;
>>> +	/**< Action valid for rules applied to egress traffic. */
>>> +};
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Create shared action for reuse in multiple flow rules.
>>> + * The created shared action has single state and configuration
>>> + * across all flow rules using it.
>>> + *
>>> + * @param[in] port_id
>>> + *    The port identifier of the Ethernet device.
>>> + * @param[in] conf
>>> + *   Shared action configuration.
>>> + * @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.
>>
>> ENODEV ?
> 
> Can you elaborate?


ENODEV is returned if port_id is invalid and it should be
documented here if you try to list all return values.

  reply	other threads:[~2020-10-14  6:49 UTC|newest]

Thread overview: 106+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-02 12:05 [dpdk-dev] [PATCH] " Andrey Vesnovaty
2020-07-03 15:02 ` Jerin Jacob
2020-07-03 15:21   ` Thomas Monjalon
2020-07-04  9:54     ` Andrey Vesnovaty
2020-07-04 10:10   ` Andrey Vesnovaty
2020-07-04 12:33     ` Jerin Jacob
2020-07-05 10:26       ` Ori Kam
2020-07-06  9:00         ` Jerin Jacob
2020-07-06 12:28           ` Ori Kam
2020-07-06 13:32             ` Andrey Vesnovaty
2020-07-07  2:30               ` Jerin Jacob
2020-07-07  6:21                 ` Ori Kam
2020-07-07 15:21                   ` Ferruh Yigit
2020-07-07 17:24                     ` Ori Kam
2020-07-07 17:52                       ` Ferruh Yigit
2020-07-07 19:38                   ` Jerin Jacob
2020-07-07 21:03                     ` Ori Kam
2020-07-08  9:25                       ` Jerin Jacob
2020-07-08  9:47                         ` Ori Kam
2020-07-08 11:00                           ` Jerin Jacob
2020-07-08 11:50                             ` Thomas Monjalon
2020-07-08 12:18                             ` Ori Kam
     [not found]                               ` <20200708204015.24429-2-andreyv@mellanox.com>
2020-07-13  8:04                                 ` [dpdk-dev] [PATCH v2 1/6] ethdev: " Kinsella, Ray
2020-07-13 10:16                                   ` Andrew Rybchenko
2020-07-15  8:54                                   ` Andrew Rybchenko
2020-07-15  9:00                                     ` Andrew Rybchenko
2020-09-15 11:30                                     ` Andrey Vesnovaty
     [not found]                               ` <20200708204015.24429-3-andreyv@mellanox.com>
2020-07-13  8:06                                 ` [dpdk-dev] [PATCH v2 2/6] common/mlx5: modify advanced Rx object via DevX Kinsella, Ray
2020-07-08 21:39 ` [dpdk-dev] [PATCH v2 0/6] add flow shared action API + PMD Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 1/6] ethdev: add flow shared action API Andrey Vesnovaty
2020-09-12  2:18     ` Ajit Khaparde
2020-09-15 11:50       ` Andrey Vesnovaty
2020-09-15 15:49         ` Ajit Khaparde
2020-09-16 15:52           ` Andrey Vesnovaty
2020-09-16 19:20             ` Ajit Khaparde
2020-09-17 15:33               ` Andrew Rybchenko
2020-09-17 16:02                 ` Ori Kam
2020-09-24 19:25                   ` Ajit Khaparde
2020-09-26 11:09                     ` Andrey Vesnovaty
2020-10-03 22:06                       ` [dpdk-dev] [PATCH v3 00/10] RTE flow shared action Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 01/10] ethdev: add flow shared action API Andrey Vesnovaty
2020-10-04 11:10                           ` Ori Kam
2020-10-06 10:22                             ` Andrey Vesnovaty
2020-10-04 17:00                           ` Stephen Hemminger
2020-10-04 17:01                             ` Stephen Hemminger
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 02/10] ethdev: add conf arg to shared action icreate API Andrey Vesnovaty
2020-10-04 11:11                           ` Ori Kam
2020-10-06 10:28                             ` Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 03/10] common/mlx5: modify advanced Rx object via DevX Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 04/10] net/mlx5: modify hash Rx queue objects Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 05/10] net/mlx5: shared action PMD Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 06/10] net/mlx5: shared action PMD create conf arg Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 07/10] net/mlx5: driver support for shared action Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 08/10] net/mlx5: shared action create conf drv support Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 09/10] examples/flow_filtering: utilize shared RSS action Andrey Vesnovaty
2020-10-04 11:21                           ` Ori Kam
2020-10-06 10:34                             ` Andrey Vesnovaty
2020-10-03 22:06                         ` [dpdk-dev] [PATCH v3 10/10] app/testpmd: support shared action Andrey Vesnovaty
2020-10-04 11:28                           ` Ori Kam
2020-10-04 12:04                             ` Ori Kam
2020-10-06 10:36                               ` Andrey Vesnovaty
2020-10-04 11:14                         ` [dpdk-dev] [PATCH v3 00/10] RTE flow " Ori Kam
2020-10-06 10:28                           ` Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 2/6] common/mlx5: modify advanced Rx object via DevX Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 3/6] net/mlx5: modify hash Rx queue objects Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 4/6] net/mlx5: shared action PMD Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 5/6] net/mlx5: driver support for shared action Andrey Vesnovaty
2020-07-08 21:39   ` [dpdk-dev] [PATCH v2 6/6] examples/flow_filtering: utilize shared RSS action Andrey Vesnovaty
2020-07-09  4:44     ` Jerin Jacob
2020-07-09  6:08       ` Ori Kam
2020-07-09 12:25         ` Andrey Vesnovaty
2020-07-09 12:39           ` Thomas Monjalon
2020-07-09  4:39   ` [dpdk-dev] [PATCH v2 0/6] add flow shared action API + PMD Jerin Jacob
2020-10-06 20:08 ` [dpdk-dev] [PATCH v4 0/2] RTE flow shared action Andrey Vesnovaty
2020-10-06 20:08   ` [dpdk-dev] [PATCH v4 1/2] ethdev: add flow shared action API Andrey Vesnovaty
2020-10-07  6:27     ` Ori Kam
2020-10-06 20:08   ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: support shared action Andrey Vesnovaty
2020-10-07  6:30     ` Ori Kam
2020-10-07 12:56 ` [dpdk-dev] [PATCH v5 0/2] RTE flow " Andrey Vesnovaty
2020-10-07 12:56   ` [dpdk-dev] [PATCH v5 1/2] ethdev: add flow shared action API Andrey Vesnovaty
2020-10-07 13:01     ` Ori Kam
2020-10-07 21:23     ` Ajit Khaparde
2020-10-08  7:28       ` Andrey Vesnovaty
2020-10-07 12:56   ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: support shared action Andrey Vesnovaty
2020-10-07 18:36 ` [dpdk-dev] [PATCH v6 0/2] RTE flow " Andrey Vesnovaty
2020-10-07 18:36   ` [dpdk-dev] [PATCH v6 1/2] ethdev: add flow shared action API Andrey Vesnovaty
2020-10-07 18:36   ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: support shared action Andrey Vesnovaty
2020-10-07 20:01     ` Ajit Khaparde
2020-10-08 10:58       ` Andrey Vesnovaty
2020-10-08 11:51 ` [dpdk-dev] [PATCH v7 0/2] RTE flow " Andrey Vesnovaty
2020-10-08 11:51   ` [dpdk-dev] [PATCH v7 1/2] ethdev: add flow shared action API Andrey Vesnovaty
2020-10-08 22:30     ` Ajit Khaparde
2020-10-14 11:47       ` Andrey Vesnovaty
2020-10-12 14:19     ` Andrew Rybchenko
2020-10-13 20:06       ` Andrey Vesnovaty
2020-10-14  6:49         ` Andrew Rybchenko [this message]
2020-10-14  7:22           ` Thomas Monjalon
2020-10-14 11:43             ` Andrey Vesnovaty
2020-10-14 11:42           ` Andrey Vesnovaty
2020-10-08 11:51   ` [dpdk-dev] [PATCH v7 2/2] app/testpmd: support shared action Andrey Vesnovaty
2020-10-08 23:54     ` Ajit Khaparde
2020-10-14 11:40 ` [dpdk-dev] [PATCH v8 0/2] RTE flow " Andrey Vesnovaty
2020-10-14 11:40   ` [dpdk-dev] [PATCH v8 1/2] ethdev: add shared actions to flow API Andrey Vesnovaty
2020-10-14 11:44     ` Andrew Rybchenko
2020-10-14 16:17       ` Ferruh Yigit
2020-10-14 11:40   ` [dpdk-dev] [PATCH v8 2/2] app/testpmd: support shared action Andrey Vesnovaty

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d3a63fbe-7373-bfd7-b166-12c56fe782c6@oktetlabs.ru \
    --to=andrew.rybchenko@oktetlabs.ru \
    --cc=ajit.khaparde@broadcom.com \
    --cc=andrey.vesnovaty@gmail.com \
    --cc=andreyv@nvidia.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jer@marvell.com \
    --cc=jerinjacobk@gmail.com \
    --cc=mdr@ashroe.eu \
    --cc=nhorman@tuxdriver.com \
    --cc=orika@nvidia.com \
    --cc=samik.gupta@broadcom.com \
    --cc=stephen@networkplumber.org \
    --cc=thomas@monjalon.net \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).