On 04/16/2018 07:22 PM, Adrien Mazarguil wrote:
This patch makes the following changes to flow rule actions:

- List order now matters, they are redefined as performed first to last
   instead of "all simultaneously".

- Repeated actions are now supported (e.g. specifying QUEUE multiple times
   now duplicates traffic among them). Previously only the last action of
   any given kind was taken into account.

- No more distinction between terminating/non-terminating/meta actions.
   Flow rules themselves are now defined as always terminating unless a
   PASSTHRU action is specified.

These changes alter the behavior of flow rules in corner cases in order to
prepare the flow API for actions that modify traffic contents or properties
(e.g. encapsulation, compression) and for which order matter when combined.

Previously one would have to do so through multiple flow rules by combining
PASSTRHU with priority levels, however this proved overly complex to
implement at the PMD level, hence this simpler approach.

This breaks ABI compatibility for the following public functions:

- rte_flow_create()
- rte_flow_validate()

PMDs with rte_flow support are modified accordingly:

- bnxt: no change, implementation already forbids multiple actions and does
   not support PASSTHRU.

- e1000: no change, same as bnxt.

- enic: modified to forbid redundant actions, no support for default drop.

- failsafe: no change needed.

- i40e: no change, implementation already forbids multiple actions.

- ixgbe: same as i40e.

- mlx4: modified to forbid multiple fate-deciding actions and drop when
   unspecified.

- mlx5: same as mlx4, with other redundant actions also forbidden.

- sfc: same as mlx4.

- tap: implementation already complies with the new behavior except for
   the default pass-through modified as a default drop.

Signed-off-by: Adrien Mazarguil <adrien.mazarg...@6wind.com>
Reviewed-by: Andrew Rybchenko <arybche...@oktetlabs.ru>
Cc: Ajit Khaparde <ajit.khapa...@broadcom.com>
Cc: Wenzhuo Lu <wenzhuo...@intel.com>
Cc: John Daley <johnd...@cisco.com>
Cc: Gaetan Rivet <gaetan.ri...@6wind.com>
Cc: Beilei Xing <beilei.x...@intel.com>
Cc: Konstantin Ananyev <konstantin.anan...@intel.com>
Cc: Nelio Laranjeiro <nelio.laranje...@6wind.com>
Cc: Andrew Rybchenko <arybche...@solarflare.com>
Cc: Pascal Mazon <pascal.ma...@6wind.com>
---
  doc/guides/prog_guide/rte_flow.rst | 67 +++++++++++++-------------------
  drivers/net/enic/enic_flow.c       | 25 ++++++++++++
  drivers/net/mlx4/mlx4_flow.c       | 21 +++++++---
  drivers/net/mlx5/mlx5_flow.c       | 69 ++++++++++++++-------------------
  drivers/net/sfc/sfc_flow.c         | 22 +++++++----
  drivers/net/tap/tap_flow.c         | 11 ++++++
  lib/librte_ether/rte_flow.h        | 54 +++++++-------------------
  7 files changed, 138 insertions(+), 131 deletions(-)

[...]

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index b9f36587c..a5c6a1670 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -975,6 +979,10 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
                        const struct rte_flow_action_queue *queue =
                                (const struct rte_flow_action_queue *)
                                actions->conf;
+
+                       if (overlap & FATE)
+                               return ENOTSUP;
+                       overlap |= FATE;
                        enic_action->rq_idx =
                                enic_rte_rq_idx_to_sop_idx(queue->index);
                        break;
@@ -984,6 +992,8 @@ enic_copy_action_v1(const struct rte_flow_action actions[],
                        break;
                }
        }
+       if (!overlap & FATE)

Build using clang on Ubuntu 17.10 fails:

/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: fatal error: logical not is only applied to       the left hand side of this bitwise operator [-Wlogical-not-parentheses]
        if (!overlap & FATE)
            ^        ~
/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add parentheses after the '!' to
      evaluate the bitwise operator first
        if (!overlap & FATE)
            ^
             (             )
/var/tmp/dpdk-next-net/drivers/net/enic/enic_flow.c:1000:6: note: add parentheses around left hand side
      expression to silence this warning
        if (!overlap & FATE)
            ^
            (       )
1 error generated.
/var/tmp/dpdk-next-net/mk/internal/rte.compile-pre.mk:114: recipe for target 'enic_flow.o' failed
make[4]: *** [enic_flow.o] Error 1
/var/tmp/dpdk-next-net/mk/rte.subdir.mk:35: recipe for target 'enic' failed
make[3]: *** [enic] Error 2
  CC nfp_cpp_pcie_ops.o
make[3]: *** Waiting for unfinished jobs....

$ clang --version
clang version 4.0.1-6 (tags/RELEASE_401/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin


+               return ENOTSUP;
        enic_action->type = FILTER_ACTION_RQ_STEERING;
        return 0;
  }
@@ -1001,6 +1011,9 @@ static int
  enic_copy_action_v2(const struct rte_flow_action actions[],
                    struct filter_action_v2 *enic_action)
  {
+       enum { FATE = 1, MARK = 2, };
+       uint32_t overlap = 0;
+
        FLOW_TRACE();
for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) {
@@ -1009,6 +1022,10 @@ enic_copy_action_v2(const struct rte_flow_action 
actions[],
                        const struct rte_flow_action_queue *queue =
                                (const struct rte_flow_action_queue *)
                                actions->conf;
+
+                       if (overlap & FATE)
+                               return ENOTSUP;
+                       overlap |= FATE;
                        enic_action->rq_idx =
                                enic_rte_rq_idx_to_sop_idx(queue->index);
                        enic_action->flags |= FILTER_ACTION_RQ_STEERING_FLAG;
@@ -1019,6 +1036,9 @@ enic_copy_action_v2(const struct rte_flow_action 
actions[],
                                (const struct rte_flow_action_mark *)
                                actions->conf;
+ if (overlap & MARK)

Same

+                               return ENOTSUP;
+                       overlap |= MARK;
                        /* ENIC_MAGIC_FILTER_ID is reserved and is the highest
                         * in the range of allows mark ids.
                         */
@@ -1029,6 +1049,9 @@ enic_copy_action_v2(const struct rte_flow_action 
actions[],
                        break;
                }
                case RTE_FLOW_ACTION_TYPE_FLAG: {
+                       if (overlap & MARK)
+                               return ENOTSUP;
+                       overlap |= MARK;
                        enic_action->filter_id = ENIC_MAGIC_FILTER_ID;
                        enic_action->flags |= FILTER_ACTION_FILTER_ID_FLAG;
                        break;
@@ -1044,6 +1067,8 @@ enic_copy_action_v2(const struct rte_flow_action 
actions[],
                        break;
                }
        }
+       if (!overlap & FATE)

Same

+               return ENOTSUP;
        enic_action->type = FILTER_ACTION_V2;
        return 0;
  }

[...]

Reply via email to