DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
@ 2018-12-24 10:50 Dekel Peled
  2018-12-25  3:25 ` Ori Kam
  2018-12-25  7:42 ` [dpdk-dev] [PATCH v2] " Dekel Peled
  0 siblings, 2 replies; 9+ messages in thread
From: Dekel Peled @ 2018-12-24 10:50 UTC (permalink / raw)
  To: orika; +Cc: dev, dekelp

Previous patch removed the VLAN item from example code.
This patch fixes the documentation accordingly.

Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
Cc: orika@mellanox.com

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
---
 doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++----------------------
 examples/flow_filtering/flow_blocks.c       | 18 ++-----
 2 files changed, 21 insertions(+), 71 deletions(-)

diff --git a/doc/guides/sample_app_ug/flow_filtering.rst b/doc/guides/sample_app_ug/flow_filtering.rst
index 840d557..9dba85a 100644
--- a/doc/guides/sample_app_ug/flow_filtering.rst
+++ b/doc/guides/sample_app_ug/flow_filtering.rst
@@ -53,7 +53,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.
 
@@ -380,13 +380,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;
 
@@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-           memset(&eth_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 = &eth_spec;
-           pattern[0].mask = &eth_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.
             */
@@ -444,12 +422,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)
@@ -464,14 +442,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;
 
@@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-   memset(&eth_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 = &eth_spec;
-   pattern[0].mask = &eth_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
 
@@ -527,15 +485,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 bae7116..1edf6f9 100644
--- a/examples/flow_filtering/flow_blocks.c
+++ b/examples/flow_filtering/flow_blocks.c
@@ -2,7 +2,8 @@
  * Copyright 2017 Mellanox Technologies, Ltd
  */
 
-#define MAX_PATTERN_NUM		4
+#define MAX_PATTERN_NUM		3
+#define MAX_ACTION_NUM		2
 
 struct rte_flow *
 generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
@@ -41,11 +42,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_ipv4 ip_spec;
 	struct rte_flow_item_ipv4 ip_mask;
 	int res;
@@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-	memset(&eth_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 = &eth_spec;
-	pattern[0].mask = &eth_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.
 	 */
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
  2018-12-24 10:50 [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation Dekel Peled
@ 2018-12-25  3:25 ` Ori Kam
  2018-12-25  6:30   ` Dekel Peled
  2018-12-25  7:42 ` [dpdk-dev] [PATCH v2] " Dekel Peled
  1 sibling, 1 reply; 9+ messages in thread
From: Ori Kam @ 2018-12-25  3:25 UTC (permalink / raw)
  To: Dekel Peled; +Cc: dev, Dekel Peled



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Monday, December 24, 2018 12:51 PM
> To: Ori Kam <orika@mellanox.com>
> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> documentation
> 
> Previous patch removed the VLAN item from example code.
> This patch fixes the documentation accordingly.

So why are you modifying the c file?

> 
> Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> ---
>  doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++----------------------
>  examples/flow_filtering/flow_blocks.c       | 18 ++-----
>  2 files changed, 21 insertions(+), 71 deletions(-)
> 
> diff --git a/doc/guides/sample_app_ug/flow_filtering.rst
> b/doc/guides/sample_app_ug/flow_filtering.rst
> index 840d557..9dba85a 100644
> --- a/doc/guides/sample_app_ug/flow_filtering.rst
> +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> @@ -53,7 +53,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.
> 
> @@ -380,13 +380,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;
> 
> @@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -           memset(&eth_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 = &eth_spec;
> -           pattern[0].mask = &eth_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.
>              */
> @@ -444,12 +422,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)
> @@ -464,14 +442,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;
> 
> @@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -   memset(&eth_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 = &eth_spec;
> -   pattern[0].mask = &eth_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
> 
> @@ -527,15 +485,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 bae7116..1edf6f9 100644
> --- a/examples/flow_filtering/flow_blocks.c
> +++ b/examples/flow_filtering/flow_blocks.c
> @@ -2,7 +2,8 @@
>   * Copyright 2017 Mellanox Technologies, Ltd
>   */
> 
> -#define MAX_PATTERN_NUM		4
> +#define MAX_PATTERN_NUM		3
> +#define MAX_ACTION_NUM		2
> 
>  struct rte_flow *
>  generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> @@ -41,11 +42,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_ipv4 ip_spec;
>  	struct rte_flow_item_ipv4 ip_mask;
>  	int res;
> @@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -	memset(&eth_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 = &eth_spec;
> -	pattern[0].mask = &eth_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.
>  	 */
> --
> 1.8.3.1


Thanks,
Ori

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
  2018-12-25  3:25 ` Ori Kam
@ 2018-12-25  6:30   ` Dekel Peled
  2018-12-25  6:37     ` Ori Kam
  2019-01-02 14:52     ` Ferruh Yigit
  0 siblings, 2 replies; 9+ messages in thread
From: Dekel Peled @ 2018-12-25  6:30 UTC (permalink / raw)
  To: Ori Kam; +Cc: dev

PSB.

> -----Original Message-----
> From: Ori Kam
> Sent: Tuesday, December 25, 2018 5:25 AM
> To: Dekel Peled <dekelp@mellanox.com>
> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> documentation
> 
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> > Sent: Monday, December 24, 2018 12:51 PM
> > To: Ori Kam <orika@mellanox.com>
> > Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> > documentation
> >
> > Previous patch removed the VLAN item from example code.
> > This patch fixes the documentation accordingly.
> 
> So why are you modifying the c file?
> 

The doc file quotes the c file, needed to modify both to align doc with code.
Code change includes fix of comments, and removing redundant variables and their initialization.

> >
> > Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
> > Cc: orika@mellanox.com
> >
> > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > ---
> >  doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++------------------
> ----
> >  examples/flow_filtering/flow_blocks.c       | 18 ++-----
> >  2 files changed, 21 insertions(+), 71 deletions(-)
> >
> > diff --git a/doc/guides/sample_app_ug/flow_filtering.rst
> > b/doc/guides/sample_app_ug/flow_filtering.rst
> > index 840d557..9dba85a 100644
> > --- a/doc/guides/sample_app_ug/flow_filtering.rst
> > +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> > @@ -53,7 +53,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.
> >
> > @@ -380,13 +380,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;
> >
> > @@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > -           memset(&eth_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 = &eth_spec;
> > -           pattern[0].mask = &eth_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.
> >              */
> > @@ -444,12 +422,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)
> > @@ -464,14 +442,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;
> >
> > @@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > -   memset(&eth_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 = &eth_spec;
> > -   pattern[0].mask = &eth_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
> >
> > @@ -527,15 +485,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 bae7116..1edf6f9 100644
> > --- a/examples/flow_filtering/flow_blocks.c
> > +++ b/examples/flow_filtering/flow_blocks.c
> > @@ -2,7 +2,8 @@
> >   * Copyright 2017 Mellanox Technologies, Ltd
> >   */
> >
> > -#define MAX_PATTERN_NUM		4
> > +#define MAX_PATTERN_NUM		3
> > +#define MAX_ACTION_NUM		2
> >
> >  struct rte_flow *
> >  generate_ipv4_flow(uint16_t port_id, uint16_t rx_q, @@ -41,11 +42,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_ipv4 ip_spec;
> >  	struct rte_flow_item_ipv4 ip_mask;
> >  	int res;
> > @@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > -	memset(&eth_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 = &eth_spec;
> > -	pattern[0].mask = &eth_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.
> >  	 */
> > --
> > 1.8.3.1
> 
> 
> Thanks,
> Ori

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
  2018-12-25  6:30   ` Dekel Peled
@ 2018-12-25  6:37     ` Ori Kam
  2019-01-02 14:52     ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Ori Kam @ 2018-12-25  6:37 UTC (permalink / raw)
  To: Dekel Peled; +Cc: dev

PSB

> -----Original Message-----
> From: Dekel Peled
> Sent: Tuesday, December 25, 2018 8:31 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> documentation
> 
> PSB.
> 
> > -----Original Message-----
> > From: Ori Kam
> > Sent: Tuesday, December 25, 2018 5:25 AM
> > To: Dekel Peled <dekelp@mellanox.com>
> > Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> > Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> > documentation
> >
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> > > Sent: Monday, December 24, 2018 12:51 PM
> > > To: Ori Kam <orika@mellanox.com>
> > > Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> > > Subject: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> > > documentation
> > >
> > > Previous patch removed the VLAN item from example code.
> > > This patch fixes the documentation accordingly.
> >
> > So why are you modifying the c file?
> >
> 
> The doc file quotes the c file, needed to modify both to align doc with code.
> Code change includes fix of comments, and removing redundant variables and
> their initialization.
> 

Modify the H to match the C I agree. 
Changing the C file is something else, since in order to make the H file match the C file you only need to change the H.

If you also wish to do some code cleanup feel free, but make sure you are adding this also 
to the commit log. 

> > >
> > > Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
> > > Cc: orika@mellanox.com
> > >
> > > Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> > > ---
> > >  doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++------------------
> > ----
> > >  examples/flow_filtering/flow_blocks.c       | 18 ++-----
> > >  2 files changed, 21 insertions(+), 71 deletions(-)
> > >
> > > diff --git a/doc/guides/sample_app_ug/flow_filtering.rst
> > > b/doc/guides/sample_app_ug/flow_filtering.rst
> > > index 840d557..9dba85a 100644
> > > --- a/doc/guides/sample_app_ug/flow_filtering.rst
> > > +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> > > @@ -53,7 +53,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.
> > >
> > > @@ -380,13 +380,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;
> > >
> > > @@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > > -           memset(&eth_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 = &eth_spec;
> > > -           pattern[0].mask = &eth_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.
> > >              */
> > > @@ -444,12 +422,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)
> > > @@ -464,14 +442,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;
> > >
> > > @@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > > -   memset(&eth_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 = &eth_spec;
> > > -   pattern[0].mask = &eth_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
> > >
> > > @@ -527,15 +485,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 bae7116..1edf6f9 100644
> > > --- a/examples/flow_filtering/flow_blocks.c
> > > +++ b/examples/flow_filtering/flow_blocks.c
> > > @@ -2,7 +2,8 @@
> > >   * Copyright 2017 Mellanox Technologies, Ltd
> > >   */
> > >
> > > -#define MAX_PATTERN_NUM		4
> > > +#define MAX_PATTERN_NUM		3
> > > +#define MAX_ACTION_NUM		2
> > >
> > >  struct rte_flow *
> > >  generate_ipv4_flow(uint16_t port_id, uint16_t rx_q, @@ -41,11 +42,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_ipv4 ip_spec;
> > >  	struct rte_flow_item_ipv4 ip_mask;
> > >  	int res;
> > > @@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> > > -	memset(&eth_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 = &eth_spec;
> > > -	pattern[0].mask = &eth_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.
> > >  	 */
> > > --
> > > 1.8.3.1
> >
> >
> > Thanks,
> > Ori

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v2] examples/flow_filtering: fix example documentation
  2018-12-24 10:50 [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation Dekel Peled
  2018-12-25  3:25 ` Ori Kam
@ 2018-12-25  7:42 ` Dekel Peled
  2018-12-25  9:54   ` Ori Kam
  1 sibling, 1 reply; 9+ messages in thread
From: Dekel Peled @ 2018-12-25  7:42 UTC (permalink / raw)
  To: orika; +Cc: dev, dekelp

Previous patch removed the VLAN item from example code.
This patch fixes the code and documentation accordingly.

Code update includes fix of comments, and removal of redundant
variables and their initialization.
Documentation update reflects the code changes done in previous
patch and in this patch.

Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
Cc: orika@mellanox.com

Signed-off-by: Dekel Peled <dekelp@mellanox.com>

---
v2: Update and elaborate patch log.
---

---
 doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++----------------------
 examples/flow_filtering/flow_blocks.c       | 18 ++-----
 2 files changed, 21 insertions(+), 71 deletions(-)

diff --git a/doc/guides/sample_app_ug/flow_filtering.rst b/doc/guides/sample_app_ug/flow_filtering.rst
index 840d557..9dba85a 100644
--- a/doc/guides/sample_app_ug/flow_filtering.rst
+++ b/doc/guides/sample_app_ug/flow_filtering.rst
@@ -53,7 +53,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.
 
@@ -380,13 +380,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;
 
@@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-           memset(&eth_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 = &eth_spec;
-           pattern[0].mask = &eth_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.
             */
@@ -444,12 +422,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)
@@ -464,14 +442,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;
 
@@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-   memset(&eth_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 = &eth_spec;
-   pattern[0].mask = &eth_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
 
@@ -527,15 +485,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 bae7116..1edf6f9 100644
--- a/examples/flow_filtering/flow_blocks.c
+++ b/examples/flow_filtering/flow_blocks.c
@@ -2,7 +2,8 @@
  * Copyright 2017 Mellanox Technologies, Ltd
  */
 
-#define MAX_PATTERN_NUM		4
+#define MAX_PATTERN_NUM		3
+#define MAX_ACTION_NUM		2
 
 struct rte_flow *
 generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
@@ -41,11 +42,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_ipv4 ip_spec;
 	struct rte_flow_item_ipv4 ip_mask;
 	int res;
@@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
-	memset(&eth_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 = &eth_spec;
-	pattern[0].mask = &eth_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.
 	 */
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v2] examples/flow_filtering: fix example documentation
  2018-12-25  7:42 ` [dpdk-dev] [PATCH v2] " Dekel Peled
@ 2018-12-25  9:54   ` Ori Kam
  2019-01-08 15:05     ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Ori Kam @ 2018-12-25  9:54 UTC (permalink / raw)
  To: Dekel Peled; +Cc: dev, Dekel Peled



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> Sent: Tuesday, December 25, 2018 9:42 AM
> To: Ori Kam <orika@mellanox.com>
> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> Subject: [dpdk-dev] [PATCH v2] examples/flow_filtering: fix example
> documentation
> 
> Previous patch removed the VLAN item from example code.
> This patch fixes the code and documentation accordingly.
> 
> Code update includes fix of comments, and removal of redundant
> variables and their initialization.
> Documentation update reflects the code changes done in previous
> patch and in this patch.
> 
> Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
> Cc: orika@mellanox.com
> 
> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> 
> ---
> v2: Update and elaborate patch log.
> ---
> 
> ---
>  doc/guides/sample_app_ug/flow_filtering.rst | 74 +++++++----------------------
>  examples/flow_filtering/flow_blocks.c       | 18 ++-----
>  2 files changed, 21 insertions(+), 71 deletions(-)
> 
> diff --git a/doc/guides/sample_app_ug/flow_filtering.rst
> b/doc/guides/sample_app_ug/flow_filtering.rst
> index 840d557..9dba85a 100644
> --- a/doc/guides/sample_app_ug/flow_filtering.rst
> +++ b/doc/guides/sample_app_ug/flow_filtering.rst
> @@ -53,7 +53,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.
> 
> @@ -380,13 +380,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;
> 
> @@ -404,37 +400,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -           memset(&eth_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 = &eth_spec;
> -           pattern[0].mask = &eth_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.
>              */
> @@ -444,12 +422,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)
> @@ -464,14 +442,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;
> 
> @@ -491,33 +465,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -   memset(&eth_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 = &eth_spec;
> -   pattern[0].mask = &eth_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
> 
> @@ -527,15 +485,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 bae7116..1edf6f9 100644
> --- a/examples/flow_filtering/flow_blocks.c
> +++ b/examples/flow_filtering/flow_blocks.c
> @@ -2,7 +2,8 @@
>   * Copyright 2017 Mellanox Technologies, Ltd
>   */
> 
> -#define MAX_PATTERN_NUM		4
> +#define MAX_PATTERN_NUM		3
> +#define MAX_ACTION_NUM		2
> 
>  struct rte_flow *
>  generate_ipv4_flow(uint16_t port_id, uint16_t rx_q,
> @@ -41,11 +42,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_ipv4 ip_spec;
>  	struct rte_flow_item_ipv4 ip_mask;
>  	int res;
> @@ -64,26 +63,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(&eth_spec, 0, sizeof(struct rte_flow_item_eth));
> -	memset(&eth_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 = &eth_spec;
> -	pattern[0].mask = &eth_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.
>  	 */
> --
> 1.8.3.1

Acked-by: Ori Kam <orika@mellanox.com>
Thanks,
Ori

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
  2018-12-25  6:30   ` Dekel Peled
  2018-12-25  6:37     ` Ori Kam
@ 2019-01-02 14:52     ` Ferruh Yigit
  2019-01-03  7:40       ` Ori Kam
  1 sibling, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2019-01-02 14:52 UTC (permalink / raw)
  To: Dekel Peled, Ori Kam; +Cc: dev

On 12/25/2018 6:30 AM, Dekel Peled wrote:
> PSB.
> 
>> -----Original Message-----
>> From: Ori Kam
>> Sent: Tuesday, December 25, 2018 5:25 AM
>> To: Dekel Peled <dekelp@mellanox.com>
>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
>> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
>> documentation
>>
>>
>>
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
>>> Sent: Monday, December 24, 2018 12:51 PM
>>> To: Ori Kam <orika@mellanox.com>
>>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
>>> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
>>> documentation
>>>
>>> Previous patch removed the VLAN item from example code.
>>> This patch fixes the documentation accordingly.
>>
>> So why are you modifying the c file?
>>
> 
> The doc file quotes the c file, needed to modify both to align doc with code.
> Code change includes fix of comments, and removing redundant variables and their initialization.

What about removing the code from doc file, it is guaranteed that it will be
outdated again? Or perhaps just add pseudo code if required.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation
  2019-01-02 14:52     ` Ferruh Yigit
@ 2019-01-03  7:40       ` Ori Kam
  0 siblings, 0 replies; 9+ messages in thread
From: Ori Kam @ 2019-01-03  7:40 UTC (permalink / raw)
  To: Ferruh Yigit, Dekel Peled; +Cc: dev

Hi Ferruh, PSB

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, January 2, 2019 4:53 PM
> To: Dekel Peled <dekelp@mellanox.com>; Ori Kam <orika@mellanox.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> documentation
> 
> On 12/25/2018 6:30 AM, Dekel Peled wrote:
> > PSB.
> >
> >> -----Original Message-----
> >> From: Ori Kam
> >> Sent: Tuesday, December 25, 2018 5:25 AM
> >> To: Dekel Peled <dekelp@mellanox.com>
> >> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> >> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> >> documentation
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
> >>> Sent: Monday, December 24, 2018 12:51 PM
> >>> To: Ori Kam <orika@mellanox.com>
> >>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
> >>> Subject: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> >>> documentation
> >>>
> >>> Previous patch removed the VLAN item from example code.
> >>> This patch fixes the documentation accordingly.
> >>
> >> So why are you modifying the c file?
> >>
> >
> > The doc file quotes the c file, needed to modify both to align doc with code.
> > Code change includes fix of comments, and removing redundant variables
> and their initialization.
> 
> What about removing the code from doc file, it is guaranteed that it will be
> outdated again? Or perhaps just add pseudo code if required.

I Agree with you, I think it will be better to change the doc to show only the 
key lines just as in the code while the rest of the lines as pseudo code.

Do to time limitation, I will update the doc hopefully only for the 19.05 version.

So please merge the current patch. 

Thanks,
Ori




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v2] examples/flow_filtering: fix example documentation
  2018-12-25  9:54   ` Ori Kam
@ 2019-01-08 15:05     ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2019-01-08 15:05 UTC (permalink / raw)
  To: Ori Kam, Dekel Peled; +Cc: dev

On 12/25/2018 9:54 AM, Ori Kam wrote:
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dekel Peled
>> Sent: Tuesday, December 25, 2018 9:42 AM
>> To: Ori Kam <orika@mellanox.com>
>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>
>> Subject: [dpdk-dev] [PATCH v2] examples/flow_filtering: fix example
>> documentation
>>
>> Previous patch removed the VLAN item from example code.
>> This patch fixes the code and documentation accordingly.
>>
>> Code update includes fix of comments, and removal of redundant
>> variables and their initialization.
>> Documentation update reflects the code changes done in previous
>> patch and in this patch.
>>
>> Fixes: 9af4eb565710 ("examples/flow_filtering: remove VLAN item")
>> Cc: orika@mellanox.com
>>
>> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
>
> Acked-by: Ori Kam <orika@mellanox.com>
>
Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-01-08 15:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-24 10:50 [dpdk-dev] [PATCH] examples/flow_filtering: fix example documentation Dekel Peled
2018-12-25  3:25 ` Ori Kam
2018-12-25  6:30   ` Dekel Peled
2018-12-25  6:37     ` Ori Kam
2019-01-02 14:52     ` Ferruh Yigit
2019-01-03  7:40       ` Ori Kam
2018-12-25  7:42 ` [dpdk-dev] [PATCH v2] " Dekel Peled
2018-12-25  9:54   ` Ori Kam
2019-01-08 15:05     ` Ferruh Yigit

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).