From: Yongseok Koh <yskoh@mellanox.com>
To: Dekel Peled <dekelp@mellanox.com>
Cc: dpdk stable <stable@dpdk.org>,
Shahaf Shuler <shahafs@mellanox.com>,
Ori Kam <orika@mellanox.com>
Subject: Re: [dpdk-stable] [PATCH 17.11] examples/flow_filtering: fix flow create function
Date: Fri, 28 Dec 2018 10:14:45 +0000 [thread overview]
Message-ID: <8B72F643-9C79-4FCE-BCD0-48E28A5BA259@mellanox.com> (raw)
In-Reply-To: <1545551899-56910-1-git-send-email-dekelp@mellanox.com>
> On Dec 22, 2018, at 11:58 PM, Dekel Peled <dekelp@mellanox.com> wrote:
>
> Flow filtering example implements a simple flow rule creation and use.
> The rule is created with ETH/VLAN/IPv4 pattern, but demonstrates only
> IPv4 filtering, hence the VLAN item is redundant.
> MLX5 PMD flow-rule validation was modified after the implementation of
> this example, and now the example fails when trying to create rule.
> Failure occurs due to the VLAN item.
>
> This patch simplifies the example, removing the VLAN item from rule.
> The documentation is modified accordingly.
>
> Fixes: 4a3ef59a10c8 ("examples/flow_filtering: add simple demo of flow API")
> Cc: orika@mellanox.com
>
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
Hi Dekel,
I can see the same patch in the dev mailing list. I will merge it once the
patch in dev list is acked and merged.
Thanks,
Yongseok
> doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++----------------------
> examples/flow_filtering/flow_blocks.c | 39 ++++-----------
> 2 files changed, 25 insertions(+), 88 deletions(-)
>
> diff --git a/doc/guides/sample_app_ug/flow_filtering.rst b/doc/guides/sample_app_ug/flow_filtering.rst
> index 725dcb4..6086632 100644
> --- a/doc/guides/sample_app_ug/flow_filtering.rst
> +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> @@ -81,7 +81,7 @@ applications and the Environment Abstraction Layer (EAL) options.
> Explanation
> -----------
>
> -The example is build from 2 main files,
> +The example is built from 2 files,
> ``main.c`` which holds the example logic and ``flow_blocks.c`` that holds the
> implementation for building the flow rule.
>
> @@ -378,13 +378,9 @@ This function is located in the ``flow_blocks.c`` file.
> {
> struct rte_flow_attr attr;
> struct rte_flow_item pattern[MAX_PATTERN_NUM];
> - struct rte_flow_action action[MAX_PATTERN_NUM];
> + struct rte_flow_action action[MAX_ACTION_NUM];
> struct rte_flow *flow = NULL;
> struct rte_flow_action_queue queue = { .index = rx_q };
> - struct rte_flow_item_eth eth_spec;
> - struct rte_flow_item_eth eth_mask;
> - struct rte_flow_item_vlan vlan_spec;
> - struct rte_flow_item_vlan vlan_mask;
> struct rte_flow_item_ipv4 ip_spec;
> struct rte_flow_item_ipv4 ip_mask;
>
> @@ -402,37 +398,19 @@ This function is located in the ``flow_blocks.c`` file.
> * create the action sequence.
> * one action only, move packet to queue
> */
> -
> action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> action[0].conf = &queue;
> action[1].type = RTE_FLOW_ACTION_TYPE_END;
>
> /*
> - * set the first level of the pattern (eth).
> + * set the first level of the pattern (ETH).
> * since in this example we just want to get the
> * ipv4 we set this level to allow all.
> */
> - memset(ð_spec, 0, sizeof(struct rte_flow_item_eth));
> - memset(ð_mask, 0, sizeof(struct rte_flow_item_eth));
> - eth_spec.type = 0;
> - eth_mask.type = 0;
> pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
> - pattern[0].spec = ð_spec;
> - pattern[0].mask = ð_mask;
> -
> - /*
> - * setting the second level of the pattern (vlan).
> - * since in this example we just want to get the
> - * ipv4 we also set this level to allow all.
> - */
> - memset(&vlan_spec, 0, sizeof(struct rte_flow_item_vlan));
> - memset(&vlan_mask, 0, sizeof(struct rte_flow_item_vlan));
> - pattern[1].type = RTE_FLOW_ITEM_TYPE_VLAN;
> - pattern[1].spec = &vlan_spec;
> - pattern[1].mask = &vlan_mask;
>
> /*
> - * setting the third level of the pattern (ip).
> + * setting the second level of the pattern (IP).
> * in this example this is the level we care about
> * so we set it according to the parameters.
> */
> @@ -442,12 +420,12 @@ This function is located in the ``flow_blocks.c`` file.
> ip_mask.hdr.dst_addr = dest_mask;
> ip_spec.hdr.src_addr = htonl(src_ip);
> ip_mask.hdr.src_addr = src_mask;
> - pattern[2].type = RTE_FLOW_ITEM_TYPE_IPV4;
> - pattern[2].spec = &ip_spec;
> - pattern[2].mask = &ip_mask;
> + pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
> + pattern[1].spec = &ip_spec;
> + pattern[1].mask = &ip_mask;
>
> /* the final level must be always type end */
> - pattern[3].type = RTE_FLOW_ITEM_TYPE_END;
> + pattern[2].type = RTE_FLOW_ITEM_TYPE_END;
>
> int res = rte_flow_validate(port_id, &attr, pattern, action, error);
> if(!res)
> @@ -462,14 +440,10 @@ The first part of the function is declaring the structures that will be used.
>
> struct rte_flow_attr attr;
> struct rte_flow_item pattern[MAX_PATTERN_NUM];
> - struct rte_flow_action action[MAX_PATTERN_NUM];
> + struct rte_flow_action action[MAX_ACTION_NUM];
> struct rte_flow *flow;
> struct rte_flow_error error;
> struct rte_flow_action_queue queue = { .index = rx_q };
> - struct rte_flow_item_eth eth_spec;
> - struct rte_flow_item_eth eth_mask;
> - struct rte_flow_item_vlan vlan_spec;
> - struct rte_flow_item_vlan vlan_mask;
> struct rte_flow_item_ipv4 ip_spec;
> struct rte_flow_item_ipv4 ip_mask;
>
> @@ -489,33 +463,17 @@ the rule. In this case send the packet to queue.
> action[0].conf = &queue;
> action[1].type = RTE_FLOW_ACTION_TYPE_END;
>
> -The forth part is responsible for creating the pattern and is build from
> -number of step. In each step we build one level of the pattern starting with
> +The fourth part is responsible for creating the pattern and is built from
> +number of steps. In each step we build one level of the pattern starting with
> the lowest one.
>
> Setting the first level of the pattern ETH:
>
> .. code-block:: c
>
> - memset(ð_spec, 0, sizeof(struct rte_flow_item_eth));
> - memset(ð_mask, 0, sizeof(struct rte_flow_item_eth));
> - eth_spec.type = 0;
> - eth_mask.type = 0;
> pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
> - pattern[0].spec = ð_spec;
> - pattern[0].mask = ð_mask;
> -
> -Setting the second level of the pattern VLAN:
> -
> -.. code-block:: c
> -
> - memset(&vlan_spec, 0, sizeof(struct rte_flow_item_vlan));
> - memset(&vlan_mask, 0, sizeof(struct rte_flow_item_vlan));
> - pattern[1].type = RTE_FLOW_ITEM_TYPE_VLAN;
> - pattern[1].spec = &vlan_spec;
> - pattern[1].mask = &vlan_mask;
>
> -Setting the third level ip:
> +Setting the second level of the pattern IP:
>
> .. code-block:: c
>
> @@ -525,15 +483,15 @@ Setting the third level ip:
> ip_mask.hdr.dst_addr = dest_mask;
> ip_spec.hdr.src_addr = htonl(src_ip);
> ip_mask.hdr.src_addr = src_mask;
> - pattern[2].type = RTE_FLOW_ITEM_TYPE_IPV4;
> - pattern[2].spec = &ip_spec;
> - pattern[2].mask = &ip_mask;
> + pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
> + pattern[1].spec = &ip_spec;
> + pattern[1].mask = &ip_mask;
>
> Closing the pattern part.
>
> .. code-block:: c
>
> - pattern[3].type = RTE_FLOW_ITEM_TYPE_END;
> + pattern[2].type = RTE_FLOW_ITEM_TYPE_END;
>
> The last part of the function is to validate the rule and create it.
>
> diff --git a/examples/flow_filtering/flow_blocks.c b/examples/flow_filtering/flow_blocks.c
> index f92df10..c1e482f 100644
> --- a/examples/flow_filtering/flow_blocks.c
> +++ b/examples/flow_filtering/flow_blocks.c
> @@ -30,7 +30,8 @@
> * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> */
>
> -#define MAX_PATTERN_NUM 4
> +#define MAX_PATTERN_NUM 3
> +#define MAX_ACTION_NUM 2
>
> struct rte_flow *
> generate_ipv4_flow(uint8_t port_id, uint16_t rx_q,
> @@ -69,13 +70,9 @@ struct rte_flow *
> {
> struct rte_flow_attr attr;
> struct rte_flow_item pattern[MAX_PATTERN_NUM];
> - struct rte_flow_action action[MAX_PATTERN_NUM];
> + struct rte_flow_action action[MAX_ACTION_NUM];
> struct rte_flow *flow = NULL;
> struct rte_flow_action_queue queue = { .index = rx_q };
> - struct rte_flow_item_eth eth_spec;
> - struct rte_flow_item_eth eth_mask;
> - struct rte_flow_item_vlan vlan_spec;
> - struct rte_flow_item_vlan vlan_mask;
> struct rte_flow_item_ipv4 ip_spec;
> struct rte_flow_item_ipv4 ip_mask;
> int res;
> @@ -94,37 +91,19 @@ struct rte_flow *
> * create the action sequence.
> * one action only, move packet to queue
> */
> -
> action[0].type = RTE_FLOW_ACTION_TYPE_QUEUE;
> action[0].conf = &queue;
> action[1].type = RTE_FLOW_ACTION_TYPE_END;
>
> /*
> - * set the first level of the pattern (eth).
> + * set the first level of the pattern (ETH).
> * since in this example we just want to get the
> * ipv4 we set this level to allow all.
> */
> - memset(ð_spec, 0, sizeof(struct rte_flow_item_eth));
> - memset(ð_mask, 0, sizeof(struct rte_flow_item_eth));
> - eth_spec.type = 0;
> - eth_mask.type = 0;
> pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
> - pattern[0].spec = ð_spec;
> - pattern[0].mask = ð_mask;
> -
> - /*
> - * setting the second level of the pattern (vlan).
> - * since in this example we just want to get the
> - * ipv4 we also set this level to allow all.
> - */
> - memset(&vlan_spec, 0, sizeof(struct rte_flow_item_vlan));
> - memset(&vlan_mask, 0, sizeof(struct rte_flow_item_vlan));
> - pattern[1].type = RTE_FLOW_ITEM_TYPE_VLAN;
> - pattern[1].spec = &vlan_spec;
> - pattern[1].mask = &vlan_mask;
>
> /*
> - * setting the third level of the pattern (ip).
> + * setting the second level of the pattern (IP).
> * in this example this is the level we care about
> * so we set it according to the parameters.
> */
> @@ -134,12 +113,12 @@ struct rte_flow *
> ip_mask.hdr.dst_addr = dest_mask;
> ip_spec.hdr.src_addr = htonl(src_ip);
> ip_mask.hdr.src_addr = src_mask;
> - pattern[2].type = RTE_FLOW_ITEM_TYPE_IPV4;
> - pattern[2].spec = &ip_spec;
> - pattern[2].mask = &ip_mask;
> + pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
> + pattern[1].spec = &ip_spec;
> + pattern[1].mask = &ip_mask;
>
> /* the final level must be always type end */
> - pattern[3].type = RTE_FLOW_ITEM_TYPE_END;
> + pattern[2].type = RTE_FLOW_ITEM_TYPE_END;
>
> res = rte_flow_validate(port_id, &attr, pattern, action, error);
> if (!res)
> --
> 1.8.3.1
>
prev parent reply other threads:[~2018-12-28 10:14 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-23 7:58 Dekel Peled
2018-12-28 10:14 ` Yongseok Koh [this message]
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=8B72F643-9C79-4FCE-BCD0-48E28A5BA259@mellanox.com \
--to=yskoh@mellanox.com \
--cc=dekelp@mellanox.com \
--cc=orika@mellanox.com \
--cc=shahafs@mellanox.com \
--cc=stable@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).