PSB.
> -----Original Message-----
> From: Ori Kam
> Sent: Tuesday, December 25, 2018 5:25 AM
> To: Dekel Peled <dek...@mellanox.com>
> Cc: dev@dpdk.org; Dekel Peled <dek...@mellanox.com>
> Subject: RE: [dpdk-dev] [PATCH] examples/flow_filtering: fix example
> documentation
>
>
>
> > -----Original Message-----
> > From: dev <dev-boun...@dpdk.org> On Behalf Of Dekel Peled
> > Sent: Monday, December 24, 2018 12:51 PM
> > To: Ori Kam <or...@mellanox.com>
> > Cc: dev@dpdk.org; Dekel Peled <dek...@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: or...@mellanox.com
> >
> > Signed-off-by: Dekel Peled <dek...@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(ð_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.
> > */
> > @@ -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(ð_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
> >
> > @@ -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(ð_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 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
