DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Declan Doherty <declan.doherty@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 3/4] ethdev: Add group action type to rte_flow
Date: Fri, 6 Apr 2018 22:26:51 +0200	[thread overview]
Message-ID: <20180406202651.GT4957@6wind.com> (raw)
In-Reply-To: <1523017443-12414-4-git-send-email-declan.doherty@intel.com>

On Fri, Apr 06, 2018 at 01:24:02PM +0100, Declan Doherty wrote:
> Add group action type which defines a terminating action which
> allows a matched flow to be redirect to a group. This allows logical
> flow table hierarchies to be managed through rte_flow.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

OK, I'm wondering if perhaps with the addition of this action, we should
redefine groups as unlinked by default?

Currently traffic enters through the flow rule with the lowest priority of
the group with the lowest ID and iterates through subsequent flow rules and
groups until matched by a flow rule without PASSTHRU (according to latest
definition [1]).

This would make jumps between groups always explicit, not necessarily a bad
idea given no PMD implements groups as of yet. Thoughts?

Also as a rather fundamental API addition, I suggest to add it after
RTE_FLOW_ACTION_TYPE_PASSTHRU. It's OK because ABI is already broken. You
just need to mention it in the commit log [1].

Another suggestion would be to rename it "JUMP" (reasons below).

[1] "ethdev: alter behavior of flow API actions"
    http://dpdk.org/ml/archives/dev/2018-April/095779.html

> ---
>  doc/guides/prog_guide/rte_flow.rst | 23 +++++++++++++++++++++++
>  lib/librte_ether/rte_flow.h        | 15 +++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst
> index 106fb93..2f0a47a 100644
> --- a/doc/guides/prog_guide/rte_flow.rst
> +++ b/doc/guides/prog_guide/rte_flow.rst
> @@ -1557,6 +1557,29 @@ set of overlay header type.
>     | ``item type`` | Item type of tunnel end-point to decapsulate |
>     +---------------+----------------------------------------------+
>  
> +

Unnecessary empty line.

> +Action: ``GROUP``
> +^^^^^^^^^^^^^^^^^
> +
> +Redirects packets to a group on the current device.
> +
> +In a hierarchy of groups, which can be used to represent physical or logical
> +flow tables on the device, this action allows the terminating action to be a
> +group on that device.
> +
> +- Terminating by default.

Keep in mind there's no such thing as a terminating action anymore [1].

> +
> +.. _table_rte_flow_action_group:
> +
> +.. table:: GROUP
> +
> +   +--------------+---------------------------------+
> +   | Field        | Value                           |
> +   +==============+=================================+
> +   | ``id``       | Group ID to redirect packets to |
> +   +--------------+---------------------------------+

"Field" column can be shrunk somewhat.

> +
> +
>  Negative types
>  ~~~~~~~~~~~~~~
>  
> diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h
> index 6d94423..968a23b 100644
> --- a/lib/librte_ether/rte_flow.h
> +++ b/lib/librte_ether/rte_flow.h
> @@ -1251,6 +1251,21 @@ struct rte_flow_action_tunnel_decap {
>  };
>  
>  /**
> + * RTE_FLOW_ACTION_TYPE_GROUP

Its addition to enum rte_flow_action_type should be part of this commit.

> + *
> + * Redirects packets to a group on the current device.
> + *
> + * In a hierarchy of groups, which can be used to represent physical or logical
> + * flow tables on the device, this action allows the terminating action to be a
> + * group on that device.
> + *
> + * Terminating by default.

See [1].

> + */
> +struct rte_flow_action_group {
> +	uint32_t id;

Assuming this structure is named rte_flow_action_jump, naming this field
"group" would match the attribute of the same name.

> +};
> +
> +/**
>   * Definition of a single action.
>   *
>   * A list of actions is terminated by a END action.
> -- 
> 2.7.4
> 

Don't forget testpmd code and documentation update.

-- 
Adrien Mazarguil
6WIND

  reply	other threads:[~2018-04-06 20:33 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-06 12:23 [dpdk-dev] [PATCH v3 0/4] ethdev: Additions to support tunnel encap/decap offload Declan Doherty
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 1/4] ethdev: add group counter support to rte_flow Declan Doherty
2018-04-06 20:26   ` Adrien Mazarguil
2018-04-09 14:22     ` Mohammad Abdul Awal
2018-04-09 15:23       ` Adrien Mazarguil
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 2/4] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-06 20:26   ` Adrien Mazarguil
2018-04-09 16:10     ` Mohammad Abdul Awal
2018-04-10 10:19       ` Adrien Mazarguil
2018-04-10 11:06         ` Shahaf Shuler
2018-04-17 14:58     ` Doherty, Declan
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 3/4] ethdev: Add group action type to rte_flow Declan Doherty
2018-04-06 20:26   ` Adrien Mazarguil [this message]
2018-04-17 14:40     ` Doherty, Declan
2018-04-06 12:24 ` [dpdk-dev] [PATCH v3 4/4] ethdev: Add metadata flow and action items support Declan Doherty
2018-04-06 20:27   ` Adrien Mazarguil
2018-04-17 14:40     ` Doherty, Declan
2018-04-27  7:41   ` Xueming(Steven) Li
2018-04-18 21:04 ` [dpdk-dev] [PATCH v4 0/6] additions to support tunnel encap/decap Declan Doherty
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 1/6] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-23 11:00       ` Shahaf Shuler
2018-04-23 11:17         ` Doherty, Declan
2018-04-23 11:49           ` Adrien Mazarguil
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 2/6] ethdev: Add jump action type to rte_flow Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 3/6] testpmd: add jump action Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 4/6] ethdev: add mark flow item to flow item types Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-23 11:10       ` Shahaf Shuler
2018-04-23 11:49         ` Adrien Mazarguil
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 5/6] testpmd: add support for MARK flow item Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-18 21:04   ` [dpdk-dev] [PATCH v4 6/6] ethdev: add shared counter support to rte_flow Declan Doherty
2018-04-19 13:03     ` Adrien Mazarguil
2018-04-23 11:11   ` [dpdk-dev] [PATCH v4 0/6] additions to support tunnel encap/decap Shahaf Shuler
2018-04-23 11:13     ` Doherty, Declan
2018-04-23 15:56   ` [dpdk-dev] [PATCH v5 0/4] ethdev " Declan Doherty
2018-04-23 15:56     ` [dpdk-dev] [PATCH v5 1/4] ethdev: Add tunnel encap/decap actions Declan Doherty
2018-04-23 15:56     ` [dpdk-dev] [PATCH v5 2/4] ethdev: Add group JUMP action Declan Doherty
2018-04-23 15:56     ` [dpdk-dev] [PATCH v5 3/4] ethdev: add mark flow item to rte_flow_item_types Declan Doherty
2018-04-23 15:56     ` [dpdk-dev] [PATCH v5 4/4] ethdev: add shared counter support to rte_flow Declan Doherty
2018-04-24 16:26     ` [dpdk-dev] [PATCH v5 0/4] ethdev additions to support tunnel encap/decap Thomas Monjalon
2018-04-30 13:54       ` Thomas Monjalon
2018-04-25 22:05     ` Ferruh Yigit

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=20180406202651.GT4957@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    /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).