DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ivan Malov <ivan.malov@arknetworks.am>
To: Tomer Shmilovich <tshmilovich@nvidia.com>
Cc: Ori Kam <orika@nvidia.com>,
	 "NBU-Contact-Thomas Monjalon (EXTERNAL)" <thomas@monjalon.net>,
	 Ferruh Yigit <ferruh.yigit@amd.com>,
	 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	 "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [RFC] ethdev: add group set miss actions API
Date: Tue, 8 Aug 2023 20:14:45 +0400 (+04)	[thread overview]
Message-ID: <42031422-f567-f376-195d-6aad89f56489@arknetworks.am> (raw)
In-Reply-To: <LV2PR12MB5749047F9B58804610515E06D40DA@LV2PR12MB5749.namprd12.prod.outlook.com>

Hi Tomer,

OK, let's see what others will say.
Minor comment below.

On Tue, 8 Aug 2023, Tomer Shmilovich wrote:

> Hi Ivan, please see inline comments.
>
> Thanks, Tomer
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@arknetworks.am>
>> Sent: Tuesday, 8 August 2023 2:03
>> To: Tomer Shmilovich <tshmilovich@nvidia.com>
>> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
>> (EXTERNAL) <thomas@monjalon.net>; Ferruh Yigit <ferruh.yigit@amd.com>;
>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; dev@dpdk.org
>> Subject: Re: [RFC] ethdev: add group set miss actions API
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Tomer,
>>
>> This is a good proposal overall, but it's a bit questionable with regard to the
>> transfer domain and precisely group 0.
>>
>> Say, the user starts a DPDK application and plugs a PF to it, also plugs a
>> representor for a VF. If I'm not mistaken, in this case, default behaviour is
>> hardly "undefined" for group 0. Packets sent by a VF are expected to reach the
>> representor (and vice versa). Also, packets arriving on the physical port are
>> expected to hit the PF port, ethdev 0, for instance.
>>
>
> True.
>
>> Does this new API intend to re-define this? I mean, if the application fails to set
>> the default action for group 0 (ENOTSUP), shall it assume that the behaviour
>> will be as described above? And if it succeeds, then assume that such implicit
>> interconnections cease functioning?
>>
>> So, this API is something like "isolated mode"
>> in the case of non-transfer API, but allowing to choose a "default" action
>> rather than DROP?
>
> You have a point. These are the default "miss actions" for all use cases as I understand them:
> Transfer - miss group 0 - goes to the other side of the connection: rep --> VF, VF --> rep.
> Transfer - miss group > 0 - goes to E-switch manager (proxy port).
> Ingress - miss group 0 - goes to application when expected (i.e. promiscuous mode); otherwise drop/go to kernel in case of bifurcated driver.
> Ingress - miss group > 0 - drop.
> Egress - miss any group - goes to wire.
>
> I suggest documenting these default "miss actions", and have the new function update the miss actions for a given group.
> If an application sets the group's miss actions as none (i.e. actions[0].type == RTE_FLOW_ACTION_TYPE_END), the miss actions should be restored to the aforementioned default miss actions.

There is also an action "PASSTHRU". It can potentially be used with this
meaning as "use implicit defaults", as "PASSTHRU" + "END".
Or just "END", as you suggest. Perhaps others will
take a look at this, too.

Thank you.

>
> Also, a different PMD may define other default miss actions and they should be documented in the NIC doc.
>
>>
>> Also, it is not quite clear how the new API is supposed to co-exist with the
>> transfer proxy concept. Has this been properly considered?
>
> All transfer groups are created on the proxy port, so I don't see any conflict when taking into consideration the above definition.
>
>>
>> Thank you.
>>
>> On Mon, 7 Aug 2023, Tomer Shmilovich wrote:
>>
>>> Introduce new group set miss actions API:
>>> rte_flow_group_set_miss_actions().
>>>
>>> A group's miss actions are a set of actions to be performed in case of
>>> a miss on a group, i.e. when a packet didn't hit any flow rules in the
>>> group.
>>>
>>> Currently, the expected behavior in this case is undefined.
>>> In order to achieve such functionality, a user can add a flow rule
>>> that matches on all traffic with the lowest priority in the group -
>>> this is not explicit however, and can be overridden by another flow
>>> rule with a lower priority.
>>>
>>> This new API function allows a user to set a group's miss actions in
>>> an explicit way.
>>>
>>> Signed-off-by: Tomer Shmilovich <tshmilovich@nvidia.com>
>>> ---
>>> doc/guides/prog_guide/rte_flow.rst | 30 +++++++++++++++++++++++++
>>> lib/ethdev/rte_flow.c              | 22 +++++++++++++++++++
>>> lib/ethdev/rte_flow.h              | 35 ++++++++++++++++++++++++++++++
>>> lib/ethdev/rte_flow_driver.h       |  7 ++++++
>>> lib/ethdev/version.map             |  3 +++
>>> 5 files changed, 97 insertions(+)
>>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 5bc998a433..590d2a770e 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -3758,6 +3758,36 @@ Information about the number of available
>> resources can be retrieved via
>>>                      struct rte_flow_queue_info *queue_info,
>>>                      struct rte_flow_error *error);
>>>
>>> +Group Miss Actions
>>> +~~~~~~~~~~~~~~~~~~
>>> +
>>> +In an application, many flow rules share common group attributes,
>>> +meaning they can be grouped and classified together. A user can
>>> +explicitly specify a set of actions performed on a packet when it did not
>> match any flows rules in a group using the following API:
>>> +
>>> +.. code-block:: c
>>> +
>>> +      int
>>> +      rte_flow_group_set_miss_actions(uint16_t port_id,
>>> +                                      uint32_t group_id,
>>> +                                      const struct rte_flow_group_attr *attr,
>>> +                                      const struct rte_flow_action actions[],
>>> +                                      struct rte_flow_error *error);
>>> +
>>> +For example, to configure a RTE_FLOW_TYPE_JUMP action as a miss action
>> for ingress group 1:
>>> +
>>> +.. code-block:: c
>>> +
>>> +      struct rte_flow_group_attr attr = {.ingress = 1};
>>> +      struct rte_flow_action act[] = {
>>> +      /* Setting miss actions to jump to group 3 */
>>> +          [0] = {.type = RTE_FLOW_ACTION_TYPE_JUMP,
>>> +                 .conf = &(struct rte_flow_action_jump){.group = 3}},
>>> +          [1] = {.type = RTE_FLOW_ACTION_TYPE_END},
>>> +      };
>>> +      struct rte_flow_error err;
>>> +      rte_flow_group_set_miss_actions(port, 1, &attr, act, &err);
>>> +
>>> Flow templates
>>> ~~~~~~~~~~~~~~
>>>
>>> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c index
>>> 271d854f78..a98d87265f 100644
>>> --- a/lib/ethdev/rte_flow.c
>>> +++ b/lib/ethdev/rte_flow.c
>>> @@ -1973,6 +1973,28 @@ rte_flow_template_table_destroy(uint16_t
>> port_id,
>>>                                 NULL, rte_strerror(ENOTSUP)); }
>>>
>>> +int
>>> +rte_flow_group_set_miss_actions(uint16_t port_id,
>>> +                             uint32_t group_id,
>>> +                             const struct rte_flow_group_attr *attr,
>>> +                             const struct rte_flow_action actions[],
>>> +                             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->group_set_miss_actions)) {
>>> +             return flow_err(port_id,
>>> +                             ops->group_set_miss_actions(dev, group_id, attr, actions,
>> error),
>>> +                             error);
>>> +     }
>>> +     return rte_flow_error_set(error, ENOTSUP,
>>> +                               RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
>>> +                               NULL, rte_strerror(ENOTSUP)); }
>>> +
>>> struct rte_flow *
>>> rte_flow_async_create(uint16_t port_id,
>>>                     uint32_t queue_id, diff --git
>>> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
>>> 86ed98c562..2d4fd49eb7 100644
>>> --- a/lib/ethdev/rte_flow.h
>>> +++ b/lib/ethdev/rte_flow.h
>>> @@ -129,6 +129,12 @@ struct rte_flow_attr {
>>>       uint32_t reserved:29; /**< Reserved, must be zero. */ };
>>>
>>> +struct rte_flow_group_attr {
>>> +     uint32_t ingress:1;
>>> +     uint32_t egress:1;
>>> +     uint32_t transfer:1;
>>> +};
>>> +
>>> /**
>>>  * Matching pattern item types.
>>>  *
>>> @@ -5839,6 +5845,35 @@ rte_flow_template_table_destroy(uint16_t
>> port_id,
>>>               struct rte_flow_template_table *template_table,
>>>               struct rte_flow_error *error);
>>>
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Set group miss actions.
>>> + *
>>> + * @param port_id
>>> + *   Port identifier of Ethernet device.
>>> + * @param group_id
>>> + *   Identifier of a group to set miss actions for.
>>> + * @param attr
>>> + *   Group attributes.
>>> + * @param actions
>>> + *   List of group miss actions.
>>> + * @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_group_set_miss_actions(uint16_t port_id,
>>> +                             uint32_t group_id,
>>> +                             const struct rte_flow_group_attr *attr,
>>> +                             const struct rte_flow_action actions[],
>>> +                             struct rte_flow_error *error);
>>> +
>>> /**
>>>  * @warning
>>>  * @b EXPERIMENTAL: this API may change without prior notice.
>>> diff --git a/lib/ethdev/rte_flow_driver.h
>>> b/lib/ethdev/rte_flow_driver.h index f9fb01b8a2..3ced086c47 100644
>>> --- a/lib/ethdev/rte_flow_driver.h
>>> +++ b/lib/ethdev/rte_flow_driver.h
>>> @@ -227,6 +227,13 @@ struct rte_flow_ops {
>>>               (struct rte_eth_dev *dev,
>>>                struct rte_flow_template_table *template_table,
>>>                struct rte_flow_error *err);
>>> +     /** See rte_flow_group_set_miss_actions() */
>>> +     int (*group_set_miss_actions)
>>> +             (struct rte_eth_dev *dev,
>>> +              uint32_t group_id,
>>> +              const struct rte_flow_group_attr *attr,
>>> +              const struct rte_flow_action actions[],
>>> +              struct rte_flow_error *err);
>>>       /** See rte_flow_async_create() */
>>>       struct rte_flow *(*async_create)
>>>               (struct rte_eth_dev *dev, diff --git
>>> a/lib/ethdev/version.map b/lib/ethdev/version.map index
>>> fc492ee839..bdd41ecb5e 100644
>>> --- a/lib/ethdev/version.map
>>> +++ b/lib/ethdev/version.map
>>> @@ -312,6 +312,9 @@ EXPERIMENTAL {
>>>       rte_flow_async_action_list_handle_query_update;
>>>       rte_flow_async_actions_update;
>>>       rte_flow_restore_info_dynflag;
>>> +
>>> +     # added in 23.11
>>> +     rte_flow_group_set_miss_actions;
>>> };
>>>
>>> INTERNAL {
>>> --
>>> 2.34.1
>>>
>>>
>

  reply	other threads:[~2023-08-08 16:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-07 13:36 Tomer Shmilovich
2023-08-07 23:03 ` Ivan Malov
2023-08-08 16:05   ` Tomer Shmilovich
2023-08-08 16:14     ` Ivan Malov [this message]
2023-08-08 16:53       ` Ori Kam

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=42031422-f567-f376-195d-6aad89f56489@arknetworks.am \
    --to=ivan.malov@arknetworks.am \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@amd.com \
    --cc=orika@nvidia.com \
    --cc=thomas@monjalon.net \
    --cc=tshmilovich@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).