[dpdk-dev] [PATCH v4 05/16] ethdev: alter behavior of flow API actions

[Andrew Rybchenko]

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.mazarguil at 6wind.com>
> Reviewed-by: Andrew Rybchenko <arybchenko at oktetlabs.ru>
> Cc: Ajit Khaparde <ajit.khaparde at broadcom.com>
> Cc: Wenzhuo Lu <wenzhuo.lu at intel.com>
> Cc: John Daley <johndale at cisco.com>
> Cc: Gaetan Rivet <gaetan.rivet at 6wind.com>
> Cc: Beilei Xing <beilei.xing at intel.com>
> Cc: Konstantin Ananyev <konstantin.ananyev at intel.com>
> Cc: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
> Cc: Andrew Rybchenko <arybchenko at solarflare.com>
> Cc: Pascal Mazon <pascal.mazon at 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;
>   }

[...]