[dpdk-dev] [PATCH 0/2] net/mlx5: support multiple groups and jump action

[dpdk-dev] [PATCH 0/2] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

In accordance with
        "[RFC] net/mlx5: support multiple groups and jump action" [1].

[1] https://mails.dpdk.org/archives/dev/2018-September/111140.html

Yongseok Koh (2):
  ethdev: add jump action to description table
  net/mlx5: support multiple groups and jump action

 app/test-pmd/config.c            |  2 +-
 drivers/net/mlx5/Makefile        | 10 ++++++
 drivers/net/mlx5/meson.build     |  4 +++
 drivers/net/mlx5/mlx5_flow.h     |  5 +++
 drivers/net/mlx5/mlx5_flow_tcf.c | 78 +++++++++++++++++++++++++++++++++++-----
 lib/librte_ethdev/rte_flow.c     |  1 +
 6 files changed, 90 insertions(+), 10 deletions(-)

-- 
2.11.0

[dpdk-dev] [PATCH 1/2] ethdev: add jump action to description table

[Yongseok Koh]

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 app/test-pmd/config.c        | 2 +-
 lib/librte_ethdev/rte_flow.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 794aa5268..641ac5e17 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1141,7 +1141,7 @@ static const struct {
 	MK_FLOW_ACTION(END, 0),
 	MK_FLOW_ACTION(VOID, 0),
 	MK_FLOW_ACTION(PASSTHRU, 0),
-	MK_FLOW_ACTION(JUMP, 0),
+	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
 	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
 	MK_FLOW_ACTION(FLAG, 0),
 	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),
diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
index cff4b5209..00ed67b5a 100644
--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c
@@ -80,6 +80,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
 	MK_FLOW_ACTION(END, 0),
 	MK_FLOW_ACTION(VOID, 0),
 	MK_FLOW_ACTION(PASSTHRU, 0),
+	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
 	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
 	MK_FLOW_ACTION(FLAG, 0),
 	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),
-- 
2.11.0

Re: [dpdk-dev] [PATCH 1/2] ethdev: add jump action to description table

[Andrew Rybchenko]

On 10/3/18 4:56 AM, Yongseok Koh wrote:
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>   app/test-pmd/config.c        | 2 +-
>   lib/librte_ethdev/rte_flow.c | 1 +
>   2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 794aa5268..641ac5e17 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1141,7 +1141,7 @@ static const struct {
>   	MK_FLOW_ACTION(END, 0),
>   	MK_FLOW_ACTION(VOID, 0),
>   	MK_FLOW_ACTION(PASSTHRU, 0),
> -	MK_FLOW_ACTION(JUMP, 0),
> +	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
>   	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
>   	MK_FLOW_ACTION(FLAG, 0),
>   	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),
> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
> index cff4b5209..00ed67b5a 100644
> --- a/lib/librte_ethdev/rte_flow.c
> +++ b/lib/librte_ethdev/rte_flow.c
> @@ -80,6 +80,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
>   	MK_FLOW_ACTION(END, 0),
>   	MK_FLOW_ACTION(VOID, 0),
>   	MK_FLOW_ACTION(PASSTHRU, 0),
> +	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
>   	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
>   	MK_FLOW_ACTION(FLAG, 0),
>   	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),

I think it should have Fixes tag and Cc to stable.

Re: [dpdk-dev] [PATCH 1/2] ethdev: add jump action to description table

[Yongseok Koh]

> On Oct 3, 2018, at 12:12 AM, Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> On 10/3/18 4:56 AM, Yongseok Koh wrote:
>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>> 
>> ---
>>  app/test-pmd/config.c        | 2 +-
>>  lib/librte_ethdev/rte_flow.c | 1 +
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 794aa5268..641ac5e17 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -1141,7 +1141,7 @@ static const struct {
>>  	MK_FLOW_ACTION(END, 0),
>>  	MK_FLOW_ACTION(VOID, 0),
>>  	MK_FLOW_ACTION(PASSTHRU, 0),
>> -	MK_FLOW_ACTION(JUMP, 0),
>> +	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
>>  	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
>>  	MK_FLOW_ACTION(FLAG, 0),
>>  	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),
>> diff --git a/lib/librte_ethdev/rte_flow.c b/lib/librte_ethdev/rte_flow.c
>> index cff4b5209..00ed67b5a 100644
>> --- a/lib/librte_ethdev/rte_flow.c
>> +++ b/lib/librte_ethdev/rte_flow.c
>> @@ -80,6 +80,7 @@ static const struct rte_flow_desc_data rte_flow_desc_action[] = {
>>  	MK_FLOW_ACTION(END, 0),
>>  	MK_FLOW_ACTION(VOID, 0),
>>  	MK_FLOW_ACTION(PASSTHRU, 0),
>> +	MK_FLOW_ACTION(JUMP, sizeof(struct rte_flow_action_jump)),
>>  	MK_FLOW_ACTION(MARK, sizeof(struct rte_flow_action_mark)),
>>  	MK_FLOW_ACTION(FLAG, 0),
>>  	MK_FLOW_ACTION(QUEUE, sizeof(struct rte_flow_action_queue)),
>> 
> 
> I think it should have Fixes tag and Cc to stable.

Agree.

Thanks,
Yongseok

[dpdk-dev] [PATCH 2/2] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

rte_flow has 'group' attribute and 'jump' action in order to support
multiple groups. This feature is known as multi-table support ('chain' in
linux TC flower) in general because a group means a table of flows. Example
commands are:

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 100 / end
	     actions jump group 1 / end

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 200 / end
	     actions jump group 2 / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern eth / vlan vid is 100 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions drop / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern end
	     actions of_pop_vlan / port_id id 1 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern eth / vlan vid is 200 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions of_pop_vlan / port_id id 2 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern end
	     actions port_id id 2 / end

With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1,
this packet will firstly hit the 1st flow. Then it will hit the 5th flow
because of the 'jump' action. As a result, the packet will be forwarded to
port 2 (VF representor) with vlan tag being stripped off. If the packet had
vlan 100 instead, it would be dropped by the 3rd flow.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 drivers/net/mlx5/Makefile        | 10 ++++++
 drivers/net/mlx5/meson.build     |  4 +++
 drivers/net/mlx5/mlx5_flow.h     |  5 +++
 drivers/net/mlx5/mlx5_flow_tcf.c | 78 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index ca1de9f21..92bae9dfc 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TCA_CHAIN \
+		linux/rtnetlink.h \
+		enum TCA_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
+		HAVE_TC_ACT_GOTO_CHAIN \
+		linux/pkt_cls.h \
+		define TC_ACT_GOTO_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_SUPPORTED_40000baseKR4_Full \
 		/usr/include/linux/ethtool.h \
 		define SUPPORTED_40000baseKR4_Full \
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index fd93ac162..696624838 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -182,6 +182,10 @@ if build
 		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
 		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
 		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
+		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
+		'TCA_CHAIN' ],
+		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
+		'TC_ACT_GOTO_CHAIN' ],
 		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
 		'RDMA_NL_NLDEV' ],
 		[ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h',
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7117f1471..d4253110c 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -78,6 +78,7 @@
 #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
+#define MLX5_FLOW_ACTION_JUMP (1u << 11)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)
@@ -125,6 +126,10 @@
 /* Max number of actions per DV flow. */
 #define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8
 
+/* Due to a limitation on driver/FW. */
+#define MLX5_FLOW_GROUP_ID_MAX 3
+#define MLX5_FLOW_GROUP_PRIORITY_MAX 14
+
 enum mlx5_flow_drv_type {
 	MLX5_FLOW_TYPE_MIN,
 	MLX5_FLOW_TYPE_DV,
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index b78998ec0..4adc6bd6e 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -148,6 +148,12 @@ struct tc_vlan {
 #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE
 #define TCA_FLOWER_KEY_VLAN_ETH_TYPE 25
 #endif
+#ifndef HAVE_TCA_CHAIN
+#define TCA_CHAIN 11
+#endif
+#ifndef HAVE_TC_ACT_GOTO_CHAIN
+#define TC_ACT_GOTO_CHAIN 0x20000000
+#endif
 
 #ifndef IPV6_ADDR_LEN
 #define IPV6_ADDR_LEN 16
@@ -225,7 +231,9 @@ struct flow_tcf_ptoi {
 	unsigned int ifindex; /**< Network interface index. */
 };
 
-#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
+#define MLX5_TCF_FATE_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
+	 MLX5_FLOW_ACTION_JUMP)
 #define MLX5_TCF_VLAN_ACTIONS \
 	(MLX5_FLOW_ACTION_OF_POP_VLAN | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
 	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
@@ -370,14 +378,25 @@ flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
 			     struct rte_flow_error *error)
 {
 	/*
-	 * Supported attributes: no groups, some priorities and ingress only.
-	 * Don't care about transfer as it is the caller's problem.
+	 * Supported attributes: groups, some priorities and ingress only.
+	 * group is supported only if kernel supports chain. Don't care about
+	 * transfer as it is the caller's problem.
 	 */
-	if (attr->group)
+	if (attr->group > MLX5_FLOW_GROUP_ID_MAX)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
-					  "groups are not supported");
-	if (attr->priority > 0xfffe)
+					  "group ID larger than "
+					  RTE_STR(MLX5_FLOW_GROUP_ID_MAX)
+					  " isn't supported");
+	else if (attr->group > 0 &&
+		 attr->priority > MLX5_FLOW_GROUP_PRIORITY_MAX)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+					  attr,
+					  "lowest priority level is "
+					  RTE_STR(MLX5_FLOW_GROUP_PRIORITY_MAX)
+					  " when group is configured");
+	else if (attr->priority > 0xfffe)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
 					  attr,
@@ -428,6 +447,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -675,6 +695,16 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
 			port_id_dev = &rte_eth_devices[conf.port_id->id];
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			if (attr->group >= conf.jump->group)
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION,
+					 actions,
+					 "can't jump to a group backward");
+			action_flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
 				return rte_flow_error_set
@@ -755,7 +785,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
  *   Maximum size of memory for items.
  */
 static int
-flow_tcf_get_items_and_size(const struct rte_flow_item items[],
+flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item items[],
 			    uint64_t *item_flags)
 {
 	int size = 0;
@@ -764,6 +795,8 @@ flow_tcf_get_items_and_size(const struct rte_flow_item items[],
 	size += SZ_NLATTR_STRZ_OF("flower") +
 		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
 		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW. */
+	if (attr->group > 0)
+		size += SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CHAIN. */
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
@@ -853,6 +886,13 @@ flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
 			flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			size += SZ_NLATTR_NEST + /* na_act_index. */
+				SZ_NLATTR_STRZ_OF("gact") +
+				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
+				SZ_NLATTR_TYPE_OF(struct tc_gact);
+			flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
 				SZ_NLATTR_STRZ_OF("gact") +
@@ -938,7 +978,7 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t handle)
  *   otherwise NULL and rte_ernno is set.
  */
 static struct mlx5_flow *
-flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
+flow_tcf_prepare(const struct rte_flow_attr *attr,
 		 const struct rte_flow_item items[],
 		 const struct rte_flow_action actions[],
 		 uint64_t *item_flags, uint64_t *action_flags,
@@ -951,7 +991,7 @@ flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
 	struct nlmsghdr *nlh;
 	struct tcmsg *tcm;
 
-	size += flow_tcf_get_items_and_size(items, item_flags);
+	size += flow_tcf_get_items_and_size(attr, items, item_flags);
 	size += flow_tcf_get_actions_and_size(actions, action_flags);
 	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
 	if (!dev_flow) {
@@ -1022,6 +1062,7 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -1056,6 +1097,8 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	 */
 	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
 				  RTE_BE16(ETH_P_ALL));
+	if (attr->group > 0)
+		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
 	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
 	na_flower = mnl_attr_nest_start(nlh, TCA_OPTIONS);
 	mnl_attr_put_u32(nlh, TCA_FLOWER_FLAGS, TCA_CLS_FLAGS_SKIP_SW);
@@ -1330,6 +1373,23 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 			mnl_attr_nest_end(nlh, na_act);
 			mnl_attr_nest_end(nlh, na_act_index);
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			na_act_index =
+				mnl_attr_nest_start(nlh, na_act_index_cur++);
+			assert(na_act_index);
+			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
+			na_act = mnl_attr_nest_start(nlh, TCA_ACT_OPTIONS);
+			assert(na_act);
+			mnl_attr_put(nlh, TCA_GACT_PARMS,
+				     sizeof(struct tc_gact),
+				     &(struct tc_gact){
+					.action = TC_ACT_GOTO_CHAIN |
+						  conf.jump->group,
+				     });
+			mnl_attr_nest_end(nlh, na_act);
+			mnl_attr_nest_end(nlh, na_act_index);
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			na_act_index =
 				mnl_attr_nest_start(nlh, na_act_index_cur++);
-- 
2.11.0

[dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

rte_flow has 'group' attribute and 'jump' action in order to support
multiple groups. This feature is known as multi-table support ('chain' in
linux TC flower) in general because a group means a table of flows. Example
commands are:

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 100 / end
	     actions jump group 1 / end

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 200 / end
	     actions jump group 2 / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern eth / vlan vid is 100 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions drop / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern end
	     actions of_pop_vlan / port_id id 1 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern eth / vlan vid is 200 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions of_pop_vlan / port_id id 2 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern end
	     actions port_id id 2 / end

With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1,
this packet will firstly hit the 1st flow. Then it will hit the 5th flow
because of the 'jump' action. As a result, the packet will be forwarded to
port 2 (VF representor) with vlan tag being stripped off. If the packet had
vlan 100 instead, it would be dropped by the 3rd flow.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v2:
* drop ethdev patch as it had already been fixed by Adrien's patch
* move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c

 drivers/net/mlx5/Makefile        | 10 +++++
 drivers/net/mlx5/meson.build     |  4 ++
 drivers/net/mlx5/mlx5_flow.h     |  1 +
 drivers/net/mlx5/mlx5_flow_tcf.c | 82 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index ca1de9f21..92bae9dfc 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TCA_CHAIN \
+		linux/rtnetlink.h \
+		enum TCA_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
+		HAVE_TC_ACT_GOTO_CHAIN \
+		linux/pkt_cls.h \
+		define TC_ACT_GOTO_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_SUPPORTED_40000baseKR4_Full \
 		/usr/include/linux/ethtool.h \
 		define SUPPORTED_40000baseKR4_Full \
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index fd93ac162..696624838 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -182,6 +182,10 @@ if build
 		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
 		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
 		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
+		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
+		'TCA_CHAIN' ],
+		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
+		'TC_ACT_GOTO_CHAIN' ],
 		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
 		'RDMA_NL_NLDEV' ],
 		[ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h',
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 12de841e8..3ed0ddc58 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -78,6 +78,7 @@
 #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
+#define MLX5_FLOW_ACTION_JUMP (1u << 11)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 91f6ef678..fbc4c2bb7 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -148,6 +148,12 @@ struct tc_vlan {
 #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE
 #define TCA_FLOWER_KEY_VLAN_ETH_TYPE 25
 #endif
+#ifndef HAVE_TCA_CHAIN
+#define TCA_CHAIN 11
+#endif
+#ifndef HAVE_TC_ACT_GOTO_CHAIN
+#define TC_ACT_GOTO_CHAIN 0x20000000
+#endif
 
 #ifndef IPV6_ADDR_LEN
 #define IPV6_ADDR_LEN 16
@@ -225,7 +231,13 @@ struct flow_tcf_ptoi {
 	unsigned int ifindex; /**< Network interface index. */
 };
 
-#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
+/* Due to a limitation on driver/FW. */
+#define MLX5_TCF_GROUP_ID_MAX 3
+#define MLX5_TCF_GROUP_PRIORITY_MAX 14
+
+#define MLX5_TCF_FATE_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
+	 MLX5_FLOW_ACTION_JUMP)
 #define MLX5_TCF_VLAN_ACTIONS \
 	(MLX5_FLOW_ACTION_OF_POP_VLAN | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
 	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
@@ -370,14 +382,25 @@ flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
 			     struct rte_flow_error *error)
 {
 	/*
-	 * Supported attributes: no groups, some priorities and ingress only.
-	 * Don't care about transfer as it is the caller's problem.
+	 * Supported attributes: groups, some priorities and ingress only.
+	 * group is supported only if kernel supports chain. Don't care about
+	 * transfer as it is the caller's problem.
 	 */
-	if (attr->group)
+	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
-					  "groups are not supported");
-	if (attr->priority > 0xfffe)
+					  "group ID larger than "
+					  RTE_STR(MLX5_TCF_GROUP_ID_MAX)
+					  " isn't supported");
+	else if (attr->group > 0 &&
+		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+					  attr,
+					  "lowest priority level is "
+					  RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
+					  " when group is configured");
+	else if (attr->priority > 0xfffe)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
 					  attr,
@@ -428,6 +451,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -675,6 +699,16 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
 			port_id_dev = &rte_eth_devices[conf.port_id->id];
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			if (attr->group >= conf.jump->group)
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION,
+					 actions,
+					 "can't jump to a group backward");
+			action_flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
 				return rte_flow_error_set
@@ -757,7 +791,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
  *   Maximum size of memory for items.
  */
 static int
-flow_tcf_get_items_and_size(const struct rte_flow_item items[],
+flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item items[],
 			    uint64_t *item_flags)
 {
 	int size = 0;
@@ -766,6 +801,8 @@ flow_tcf_get_items_and_size(const struct rte_flow_item items[],
 	size += SZ_NLATTR_STRZ_OF("flower") +
 		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
 		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW. */
+	if (attr->group > 0)
+		size += SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CHAIN. */
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
@@ -855,6 +892,13 @@ flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
 			flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			size += SZ_NLATTR_NEST + /* na_act_index. */
+				SZ_NLATTR_STRZ_OF("gact") +
+				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
+				SZ_NLATTR_TYPE_OF(struct tc_gact);
+			flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
 				SZ_NLATTR_STRZ_OF("gact") +
@@ -940,7 +984,7 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t handle)
  *   otherwise NULL and rte_ernno is set.
  */
 static struct mlx5_flow *
-flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
+flow_tcf_prepare(const struct rte_flow_attr *attr,
 		 const struct rte_flow_item items[],
 		 const struct rte_flow_action actions[],
 		 uint64_t *item_flags, uint64_t *action_flags,
@@ -953,7 +997,7 @@ flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
 	struct nlmsghdr *nlh;
 	struct tcmsg *tcm;
 
-	size += flow_tcf_get_items_and_size(items, item_flags);
+	size += flow_tcf_get_items_and_size(attr, items, item_flags);
 	size += flow_tcf_get_actions_and_size(actions, action_flags);
 	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
 	if (!dev_flow) {
@@ -1024,6 +1068,7 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -1058,6 +1103,8 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	 */
 	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
 				  RTE_BE16(ETH_P_ALL));
+	if (attr->group > 0)
+		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
 	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
 	na_flower = mnl_attr_nest_start(nlh, TCA_OPTIONS);
 	mnl_attr_put_u32(nlh, TCA_FLOWER_FLAGS, TCA_CLS_FLAGS_SKIP_SW);
@@ -1332,6 +1379,23 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 			mnl_attr_nest_end(nlh, na_act);
 			mnl_attr_nest_end(nlh, na_act_index);
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			na_act_index =
+				mnl_attr_nest_start(nlh, na_act_index_cur++);
+			assert(na_act_index);
+			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
+			na_act = mnl_attr_nest_start(nlh, TCA_ACT_OPTIONS);
+			assert(na_act);
+			mnl_attr_put(nlh, TCA_GACT_PARMS,
+				     sizeof(struct tc_gact),
+				     &(struct tc_gact){
+					.action = TC_ACT_GOTO_CHAIN |
+						  conf.jump->group,
+				     });
+			mnl_attr_nest_end(nlh, na_act);
+			mnl_attr_nest_end(nlh, na_act_index);
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			na_act_index =
 				mnl_attr_nest_start(nlh, na_act_index_cur++);
-- 
2.11.0

Re: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action

[Ori Kam]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Yongseok Koh
> Sent: Monday, October 8, 2018 9:06 PM
> To: Shahaf Shuler <shahafs@mellanox.com>
> Cc: dev@dpdk.org; Yongseok Koh <yskoh@mellanox.com>
> Subject: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump
> action
> 
> rte_flow has 'group' attribute and 'jump' action in order to support
> multiple groups. This feature is known as multi-table support ('chain' in
> linux TC flower) in general because a group means a table of flows. Example
> commands are:
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 100 / end
> 	     actions jump group 1 / end
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 200 / end
> 	     actions jump group 2 / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern eth / vlan vid is 100 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions drop / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern end
> 	     actions of_pop_vlan / port_id id 1 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern eth / vlan vid is 200 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions of_pop_vlan / port_id id 2 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern end
> 	     actions port_id id 2 / end
> 
> With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1,
> this packet will firstly hit the 1st flow. Then it will hit the 5th flow
> because of the 'jump' action. As a result, the packet will be forwarded to
> port 2 (VF representor) with vlan tag being stripped off. If the packet had
> vlan 100 instead, it would be dropped by the 3rd flow.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v2:
> * drop ethdev patch as it had already been fixed by Adrien's patch
> * move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c
> 
>  drivers/net/mlx5/Makefile        | 10 +++++
>  drivers/net/mlx5/meson.build     |  4 ++
>  drivers/net/mlx5/mlx5_flow.h     |  1 +
>  drivers/net/mlx5/mlx5_flow_tcf.c | 82
> +++++++++++++++++++++++++++++++++++-----
>  4 files changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
> index ca1de9f21..92bae9dfc 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-
> config-h.sh
>  		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
> +		HAVE_TCA_CHAIN \
> +		linux/rtnetlink.h \
> +		enum TCA_CHAIN \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
> +		HAVE_TC_ACT_GOTO_CHAIN \
> +		linux/pkt_cls.h \
> +		define TC_ACT_GOTO_CHAIN \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_SUPPORTED_40000baseKR4_Full \
>  		/usr/include/linux/ethtool.h \
>  		define SUPPORTED_40000baseKR4_Full \
> diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> index fd93ac162..696624838 100644
> --- a/drivers/net/mlx5/meson.build
> +++ b/drivers/net/mlx5/meson.build
> @@ -182,6 +182,10 @@ if build
>  		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
>  		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
>  		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
> +		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
> +		'TCA_CHAIN' ],
> +		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
> +		'TC_ACT_GOTO_CHAIN' ],
>  		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
>  		'RDMA_NL_NLDEV' ],
>  		[ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h',
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 12de841e8..3ed0ddc58 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -78,6 +78,7 @@
>  #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
>  #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> +#define MLX5_FLOW_ACTION_JUMP (1u << 11)
> 
>  #define MLX5_FLOW_FATE_ACTIONS \
>  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> MLX5_FLOW_ACTION_RSS)
> diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 91f6ef678..fbc4c2bb7 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -148,6 +148,12 @@ struct tc_vlan {
>  #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE
>  #define TCA_FLOWER_KEY_VLAN_ETH_TYPE 25
>  #endif
> +#ifndef HAVE_TCA_CHAIN
> +#define TCA_CHAIN 11
> +#endif
> +#ifndef HAVE_TC_ACT_GOTO_CHAIN
> +#define TC_ACT_GOTO_CHAIN 0x20000000
> +#endif
> 
>  #ifndef IPV6_ADDR_LEN
>  #define IPV6_ADDR_LEN 16
> @@ -225,7 +231,13 @@ struct flow_tcf_ptoi {
>  	unsigned int ifindex; /**< Network interface index. */
>  };
> 
> -#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
> MLX5_FLOW_ACTION_PORT_ID)
> +/* Due to a limitation on driver/FW. */
> +#define MLX5_TCF_GROUP_ID_MAX 3
> +#define MLX5_TCF_GROUP_PRIORITY_MAX 14
> +
> +#define MLX5_TCF_FATE_ACTIONS \
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
> +	 MLX5_FLOW_ACTION_JUMP)
>  #define MLX5_TCF_VLAN_ACTIONS \
>  	(MLX5_FLOW_ACTION_OF_POP_VLAN |
> MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
>  	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID |
> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
> @@ -370,14 +382,25 @@ flow_tcf_validate_attributes(const struct
> rte_flow_attr *attr,
>  			     struct rte_flow_error *error)
>  {
>  	/*
> -	 * Supported attributes: no groups, some priorities and ingress only.
> -	 * Don't care about transfer as it is the caller's problem.
> +	 * Supported attributes: groups, some priorities and ingress only.
> +	 * group is supported only if kernel supports chain. Don't care about
> +	 * transfer as it is the caller's problem.
>  	 */
> -	if (attr->group)
> +	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
> -					  "groups are not supported");
> -	if (attr->priority > 0xfffe)
> +					  "group ID larger than "
> +
> RTE_STR(MLX5_TCF_GROUP_ID_MAX)
> +					  " isn't supported");
> +	else if (attr->group > 0 &&
> +		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> +					  attr,
> +					  "lowest priority level is "
> +
> RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
> +					  " when group is configured");
> +	else if (attr->priority > 0xfffe)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
>  					  attr,
> @@ -428,6 +451,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  	} spec, mask;
>  	union {
>  		const struct rte_flow_action_port_id *port_id;
> +		const struct rte_flow_action_jump *jump;
>  		const struct rte_flow_action_of_push_vlan *of_push_vlan;
>  		const struct rte_flow_action_of_set_vlan_vid *
>  			of_set_vlan_vid;
> @@ -675,6 +699,16 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>  			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
>  			port_id_dev = &rte_eth_devices[conf.port_id->id];
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			conf.jump = actions->conf;
> +			if (attr->group >= conf.jump->group)
> +				return rte_flow_error_set
> +					(error, ENOTSUP,
> +					 RTE_FLOW_ERROR_TYPE_ACTION,
> +					 actions,
> +					 "can't jump to a group backward");
> +			action_flags |= MLX5_FLOW_ACTION_JUMP;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			if (action_flags & MLX5_TCF_FATE_ACTIONS)
>  				return rte_flow_error_set
> @@ -757,7 +791,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
>   *   Maximum size of memory for items.
>   */
>  static int
> -flow_tcf_get_items_and_size(const struct rte_flow_item items[],
> +flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
> +			    const struct rte_flow_item items[],
>  			    uint64_t *item_flags)
>  {
>  	int size = 0;
> @@ -766,6 +801,8 @@ flow_tcf_get_items_and_size(const struct
> rte_flow_item items[],
>  	size += SZ_NLATTR_STRZ_OF("flower") +
>  		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
>  		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW.
> */
> +	if (attr->group > 0)
> +		size += SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CHAIN. */
>  	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
>  		switch (items->type) {
>  		case RTE_FLOW_ITEM_TYPE_VOID:
> @@ -855,6 +892,13 @@ flow_tcf_get_actions_and_size(const struct
> rte_flow_action actions[],
>  				SZ_NLATTR_TYPE_OF(struct tc_mirred);
>  			flags |= MLX5_FLOW_ACTION_PORT_ID;
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			size += SZ_NLATTR_NEST + /* na_act_index. */
> +				SZ_NLATTR_STRZ_OF("gact") +
> +				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
> +				SZ_NLATTR_TYPE_OF(struct tc_gact);
> +			flags |= MLX5_FLOW_ACTION_JUMP;
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			size += SZ_NLATTR_NEST + /* na_act_index. */
>  				SZ_NLATTR_STRZ_OF("gact") +
> @@ -940,7 +984,7 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t
> handle)
>   *   otherwise NULL and rte_ernno is set.
>   */
>  static struct mlx5_flow *
> -flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
> +flow_tcf_prepare(const struct rte_flow_attr *attr,
>  		 const struct rte_flow_item items[],
>  		 const struct rte_flow_action actions[],
>  		 uint64_t *item_flags, uint64_t *action_flags,
> @@ -953,7 +997,7 @@ flow_tcf_prepare(const struct rte_flow_attr *attr
> __rte_unused,
>  	struct nlmsghdr *nlh;
>  	struct tcmsg *tcm;
> 
> -	size += flow_tcf_get_items_and_size(items, item_flags);
> +	size += flow_tcf_get_items_and_size(attr, items, item_flags);
>  	size += flow_tcf_get_actions_and_size(actions, action_flags);
>  	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
>  	if (!dev_flow) {
> @@ -1024,6 +1068,7 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct
> mlx5_flow *dev_flow,
>  	} spec, mask;
>  	union {
>  		const struct rte_flow_action_port_id *port_id;
> +		const struct rte_flow_action_jump *jump;
>  		const struct rte_flow_action_of_push_vlan *of_push_vlan;
>  		const struct rte_flow_action_of_set_vlan_vid *
>  			of_set_vlan_vid;
> @@ -1058,6 +1103,8 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct
> mlx5_flow *dev_flow,
>  	 */
>  	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
>  				  RTE_BE16(ETH_P_ALL));
> +	if (attr->group > 0)
> +		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
>  	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
>  	na_flower = mnl_attr_nest_start(nlh, TCA_OPTIONS);
>  	mnl_attr_put_u32(nlh, TCA_FLOWER_FLAGS,
> TCA_CLS_FLAGS_SKIP_SW);
> @@ -1332,6 +1379,23 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct
> mlx5_flow *dev_flow,
>  			mnl_attr_nest_end(nlh, na_act);
>  			mnl_attr_nest_end(nlh, na_act_index);
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			conf.jump = actions->conf;
> +			na_act_index =
> +				mnl_attr_nest_start(nlh, na_act_index_cur++);
> +			assert(na_act_index);
> +			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
> +			na_act = mnl_attr_nest_start(nlh, TCA_ACT_OPTIONS);
> +			assert(na_act);
> +			mnl_attr_put(nlh, TCA_GACT_PARMS,
> +				     sizeof(struct tc_gact),
> +				     &(struct tc_gact){
> +					.action = TC_ACT_GOTO_CHAIN |
> +						  conf.jump->group,
> +				     });
> +			mnl_attr_nest_end(nlh, na_act);
> +			mnl_attr_nest_end(nlh, na_act_index);
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			na_act_index =
>  				mnl_attr_nest_start(nlh, na_act_index_cur++);
> --
> 2.11.0

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

Thanks,
Ori KAm

Re: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action

[Shahaf Shuler]

Hi Koh,
Few comments. 

Monday, October 8, 2018 9:06 PM¸ Yongseok Koh:
> Subject: [PATCH v2] net/mlx5: support multiple groups and jump action
> 
> rte_flow has 'group' attribute and 'jump' action in order to support multiple
> groups. This feature is known as multi-table support ('chain' in linux TC
> flower) in general because a group means a table of flows. Example
> commands are:
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 100 / end
> 	     actions jump group 1 / end
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 200 / end
> 	     actions jump group 2 / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern eth / vlan vid is 100 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions drop / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern end
> 	     actions of_pop_vlan / port_id id 1 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern eth / vlan vid is 200 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions of_pop_vlan / port_id id 2 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern end
> 	     actions port_id id 2 / end
> 
> With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1, this
> packet will firstly hit the 1st flow. Then it will hit the 5th flow because of the
> 'jump' action. As a result, the packet will be forwarded to port 2 (VF
> representor) with vlan tag being stripped off. If the packet had vlan 100
> instead, it would be dropped by the 3rd flow.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
> 
> v2:
> * drop ethdev patch as it had already been fixed by Adrien's patch
> * move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c
> 
>  drivers/net/mlx5/Makefile        | 10 +++++
>  drivers/net/mlx5/meson.build     |  4 ++
>  drivers/net/mlx5/mlx5_flow.h     |  1 +
>  drivers/net/mlx5/mlx5_flow_tcf.c | 82
> +++++++++++++++++++++++++++++++++++-----
>  4 files changed, 88 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile index
> ca1de9f21..92bae9dfc 100644
> --- a/drivers/net/mlx5/Makefile
> +++ b/drivers/net/mlx5/Makefile
> @@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-
> config-h.sh
>  		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
>  		$(AUTOCONF_OUTPUT)
>  	$Q sh -- '$<' '$@' \
> +		HAVE_TCA_CHAIN \
> +		linux/rtnetlink.h \
> +		enum TCA_CHAIN \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
> +		HAVE_TC_ACT_GOTO_CHAIN \
> +		linux/pkt_cls.h \
> +		define TC_ACT_GOTO_CHAIN \
> +		$(AUTOCONF_OUTPUT)
> +	$Q sh -- '$<' '$@' \
>  		HAVE_SUPPORTED_40000baseKR4_Full \
>  		/usr/include/linux/ethtool.h \
>  		define SUPPORTED_40000baseKR4_Full \
> diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> index fd93ac162..696624838 100644
> --- a/drivers/net/mlx5/meson.build
> +++ b/drivers/net/mlx5/meson.build
> @@ -182,6 +182,10 @@ if build
>  		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
>  		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
>  		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
> +		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
> +		'TCA_CHAIN' ],
> +		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
> +		'TC_ACT_GOTO_CHAIN' ],

Please keep the dictionary order according to the linux header for the search. 

>  		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
>  		'RDMA_NL_NLDEV' ],
>  		[ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h',
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 12de841e8..3ed0ddc58 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -78,6 +78,7 @@
>  #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)  #define
> MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)  #define
> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> +#define MLX5_FLOW_ACTION_JUMP (1u << 11)
> 
>  #define MLX5_FLOW_FATE_ACTIONS \
>  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> MLX5_FLOW_ACTION_RSS) diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> b/drivers/net/mlx5/mlx5_flow_tcf.c
> index 91f6ef678..fbc4c2bb7 100644
> --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> @@ -148,6 +148,12 @@ struct tc_vlan {
>  #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE  #define
> TCA_FLOWER_KEY_VLAN_ETH_TYPE 25  #endif
> +#ifndef HAVE_TCA_CHAIN
> +#define TCA_CHAIN 11
> +#endif
> +#ifndef HAVE_TC_ACT_GOTO_CHAIN
> +#define TC_ACT_GOTO_CHAIN 0x20000000
> +#endif
> 
>  #ifndef IPV6_ADDR_LEN
>  #define IPV6_ADDR_LEN 16
> @@ -225,7 +231,13 @@ struct flow_tcf_ptoi {
>  	unsigned int ifindex; /**< Network interface index. */  };
> 
> -#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
> MLX5_FLOW_ACTION_PORT_ID)
> +/* Due to a limitation on driver/FW. */ #define
> MLX5_TCF_GROUP_ID_MAX 3
> +#define MLX5_TCF_GROUP_PRIORITY_MAX 14

I guess there is no way to query those and trial and error is overkill for this first implementation. 

> +
> +#define MLX5_TCF_FATE_ACTIONS \
> +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
> +	 MLX5_FLOW_ACTION_JUMP)
>  #define MLX5_TCF_VLAN_ACTIONS \
>  	(MLX5_FLOW_ACTION_OF_POP_VLAN |
> MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
>  	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID |
> MLX5_FLOW_ACTION_OF_SET_VLAN_PCP) @@ -370,14 +382,25 @@
> flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
>  			     struct rte_flow_error *error)
>  {
>  	/*
> -	 * Supported attributes: no groups, some priorities and ingress only.
> -	 * Don't care about transfer as it is the caller's problem.
> +	 * Supported attributes: groups, some priorities and ingress only.
> +	 * group is supported only if kernel supports chain. Don't care about
> +	 * transfer as it is the caller's problem.
>  	 */
> -	if (attr->group)
> +	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
> -					  "groups are not supported");
> -	if (attr->priority > 0xfffe)
> +					  "group ID larger than "
> +
> RTE_STR(MLX5_TCF_GROUP_ID_MAX)
> +					  " isn't supported");
> +	else if (attr->group > 0 &&
> +		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
> +		return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> +					  attr,
> +					  "lowest priority level is "

Lowest or highest?

> +
> RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
> +					  " when group is configured");
> +	else if (attr->priority > 0xfffe)
>  		return rte_flow_error_set(error, ENOTSUP,
> 
> RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
>  					  attr,

[... ]

> flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
>  			mnl_attr_nest_end(nlh, na_act);
>  			mnl_attr_nest_end(nlh, na_act_index);
>  			break;
> +		case RTE_FLOW_ACTION_TYPE_JUMP:
> +			conf.jump = actions->conf;
> +			na_act_index =
> +				mnl_attr_nest_start(nlh,
> na_act_index_cur++);
> +			assert(na_act_index);
> +			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
> +			na_act = mnl_attr_nest_start(nlh,
> TCA_ACT_OPTIONS);
> +			assert(na_act);
> +			mnl_attr_put(nlh, TCA_GACT_PARMS,
> +				     sizeof(struct tc_gact),
> +				     &(struct tc_gact){
> +					.action = TC_ACT_GOTO_CHAIN |
> +						  conf.jump->group,
> +				     });
> +			mnl_attr_nest_end(nlh, na_act);
> +			mnl_attr_nest_end(nlh, na_act_index);
> +			break;
>  		case RTE_FLOW_ACTION_TYPE_DROP:
>  			na_act_index =
>  				mnl_attr_nest_start(nlh,
> na_act_index_cur++);


We also spoke about that for group > 0 the flow items can start from the middle. E.g. on table 0 we have flow rule that redirect all eth/ip to group 1, and on group 1 the rules can start with udp or tcp. 
Is this possible today w/o any code change? 

> --
> 2.11.0

Re: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

On Tue, Oct 09, 2018 at 05:50:30AM -0700, Shahaf Shuler wrote:
> Hi Koh,
> Few comments. 
> 
> Monday, October 8, 2018 9:06 PM¸ Yongseok Koh:
> > Subject: [PATCH v2] net/mlx5: support multiple groups and jump action
> > 
> > rte_flow has 'group' attribute and 'jump' action in order to support multiple
> > groups. This feature is known as multi-table support ('chain' in linux TC
> > flower) in general because a group means a table of flows. Example
> > commands are:
> > 
> > 	flow create 0 transfer priority 1 ingress
> > 	     pattern eth / vlan vid is 100 / end
> > 	     actions jump group 1 / end
> > 
> > 	flow create 0 transfer priority 1 ingress
> > 	     pattern eth / vlan vid is 200 / end
> > 	     actions jump group 2 / end
> > 
> > 	flow create 0 transfer group 1 priority 2 ingress
> > 	     pattern eth / vlan vid is 100 /
> > 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> > 	     actions drop / end
> > 
> > 	flow create 0 transfer group 1 priority 2 ingress
> > 	     pattern end
> > 	     actions of_pop_vlan / port_id id 1 / end
> > 
> > 	flow create 0 transfer group 2 priority 2 ingress
> > 	     pattern eth / vlan vid is 200 /
> > 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> > 	     actions of_pop_vlan / port_id id 2 / end
> > 
> > 	flow create 0 transfer group 2 priority 2 ingress
> > 	     pattern end
> > 	     actions port_id id 2 / end
> > 
> > With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1, this
> > packet will firstly hit the 1st flow. Then it will hit the 5th flow because of the
> > 'jump' action. As a result, the packet will be forwarded to port 2 (VF
> > representor) with vlan tag being stripped off. If the packet had vlan 100
> > instead, it would be dropped by the 3rd flow.
> > 
> > Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> > 
> > v2:
> > * drop ethdev patch as it had already been fixed by Adrien's patch
> > * move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c
> > 
> >  drivers/net/mlx5/Makefile        | 10 +++++
> >  drivers/net/mlx5/meson.build     |  4 ++
> >  drivers/net/mlx5/mlx5_flow.h     |  1 +
> >  drivers/net/mlx5/mlx5_flow_tcf.c | 82
> > +++++++++++++++++++++++++++++++++++-----
> >  4 files changed, 88 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile index
> > ca1de9f21..92bae9dfc 100644
> > --- a/drivers/net/mlx5/Makefile
> > +++ b/drivers/net/mlx5/Makefile
> > @@ -347,6 +347,16 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-
> > config-h.sh
> >  		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
> >  		$(AUTOCONF_OUTPUT)
> >  	$Q sh -- '$<' '$@' \
> > +		HAVE_TCA_CHAIN \
> > +		linux/rtnetlink.h \
> > +		enum TCA_CHAIN \
> > +		$(AUTOCONF_OUTPUT)
> > +	$Q sh -- '$<' '$@' \
> > +		HAVE_TC_ACT_GOTO_CHAIN \
> > +		linux/pkt_cls.h \
> > +		define TC_ACT_GOTO_CHAIN \
> > +		$(AUTOCONF_OUTPUT)
> > +	$Q sh -- '$<' '$@' \
> >  		HAVE_SUPPORTED_40000baseKR4_Full \
> >  		/usr/include/linux/ethtool.h \
> >  		define SUPPORTED_40000baseKR4_Full \
> > diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
> > index fd93ac162..696624838 100644
> > --- a/drivers/net/mlx5/meson.build
> > +++ b/drivers/net/mlx5/meson.build
> > @@ -182,6 +182,10 @@ if build
> >  		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
> >  		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
> >  		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
> > +		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
> > +		'TCA_CHAIN' ],
> > +		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
> > +		'TC_ACT_GOTO_CHAIN' ],
> 
> Please keep the dictionary order according to the linux header for the search. 

Okay.

> >  		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
> >  		'RDMA_NL_NLDEV' ],
> >  		[ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h',
> > diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> > index 12de841e8..3ed0ddc58 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -78,6 +78,7 @@
> >  #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)  #define
> > MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)  #define
> > MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
> > +#define MLX5_FLOW_ACTION_JUMP (1u << 11)
> > 
> >  #define MLX5_FLOW_FATE_ACTIONS \
> >  	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE |
> > MLX5_FLOW_ACTION_RSS) diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c
> > b/drivers/net/mlx5/mlx5_flow_tcf.c
> > index 91f6ef678..fbc4c2bb7 100644
> > --- a/drivers/net/mlx5/mlx5_flow_tcf.c
> > +++ b/drivers/net/mlx5/mlx5_flow_tcf.c
> > @@ -148,6 +148,12 @@ struct tc_vlan {
> >  #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE  #define
> > TCA_FLOWER_KEY_VLAN_ETH_TYPE 25  #endif
> > +#ifndef HAVE_TCA_CHAIN
> > +#define TCA_CHAIN 11
> > +#endif
> > +#ifndef HAVE_TC_ACT_GOTO_CHAIN
> > +#define TC_ACT_GOTO_CHAIN 0x20000000
> > +#endif
> > 
> >  #ifndef IPV6_ADDR_LEN
> >  #define IPV6_ADDR_LEN 16
> > @@ -225,7 +231,13 @@ struct flow_tcf_ptoi {
> >  	unsigned int ifindex; /**< Network interface index. */  };
> > 
> > -#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
> > MLX5_FLOW_ACTION_PORT_ID)
> > +/* Due to a limitation on driver/FW. */ #define
> > MLX5_TCF_GROUP_ID_MAX 3
> > +#define MLX5_TCF_GROUP_PRIORITY_MAX 14
> 
> I guess there is no way to query those and trial and error is overkill for this first implementation. 

Well, not a huge task. If you (or FW/drv team) think this max value is likely
increased in the near future (before the next LTS version), then it isn't a bad
idea to add such code now. Let me know.

> > +
> > +#define MLX5_TCF_FATE_ACTIONS \
> > +	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
> > +	 MLX5_FLOW_ACTION_JUMP)
> >  #define MLX5_TCF_VLAN_ACTIONS \
> >  	(MLX5_FLOW_ACTION_OF_POP_VLAN |
> > MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
> >  	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID |
> > MLX5_FLOW_ACTION_OF_SET_VLAN_PCP) @@ -370,14 +382,25 @@
> > flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
> >  			     struct rte_flow_error *error)
> >  {
> >  	/*
> > -	 * Supported attributes: no groups, some priorities and ingress only.
> > -	 * Don't care about transfer as it is the caller's problem.
> > +	 * Supported attributes: groups, some priorities and ingress only.
> > +	 * group is supported only if kernel supports chain. Don't care about
> > +	 * transfer as it is the caller's problem.
> >  	 */
> > -	if (attr->group)
> > +	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
> >  		return rte_flow_error_set(error, ENOTSUP,
> > 
> > RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
> > -					  "groups are not supported");
> > -	if (attr->priority > 0xfffe)
> > +					  "group ID larger than "
> > +
> > RTE_STR(MLX5_TCF_GROUP_ID_MAX)
> > +					  " isn't supported");
> > +	else if (attr->group > 0 &&
> > +		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +
> > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> > +					  attr,
> > +					  "lowest priority level is "
> 
> Lowest or highest?

Lowest it is. Consistent with the error message of no-multi-group case below.

> > +
> > RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
> > +					  " when group is configured");
> > +	else if (attr->priority > 0xfffe)
> >  		return rte_flow_error_set(error, ENOTSUP,
> > 
> > RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
> >  					  attr,
> 
> [... ]
> 
> > flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
> >  			mnl_attr_nest_end(nlh, na_act);
> >  			mnl_attr_nest_end(nlh, na_act_index);
> >  			break;
> > +		case RTE_FLOW_ACTION_TYPE_JUMP:
> > +			conf.jump = actions->conf;
> > +			na_act_index =
> > +				mnl_attr_nest_start(nlh,
> > na_act_index_cur++);
> > +			assert(na_act_index);
> > +			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
> > +			na_act = mnl_attr_nest_start(nlh,
> > TCA_ACT_OPTIONS);
> > +			assert(na_act);
> > +			mnl_attr_put(nlh, TCA_GACT_PARMS,
> > +				     sizeof(struct tc_gact),
> > +				     &(struct tc_gact){
> > +					.action = TC_ACT_GOTO_CHAIN |
> > +						  conf.jump->group,
> > +				     });
> > +			mnl_attr_nest_end(nlh, na_act);
> > +			mnl_attr_nest_end(nlh, na_act_index);
> > +			break;
> >  		case RTE_FLOW_ACTION_TYPE_DROP:
> >  			na_act_index =
> >  				mnl_attr_nest_start(nlh,
> > na_act_index_cur++);
> 
> 
> We also spoke about that for group > 0 the flow items can start from the middle. E.g. on table 0 we have flow rule that redirect all eth/ip to group 1, and on group 1 the rules can start with udp or tcp. 
> Is this possible today w/o any code change? 

Not possible. It needs more changes. Complexity of the additional code depends
on a set of limitations we make. In the simplest way, we can unconditionally
allow such flows if TCF allows it. But if we need smarter way, we would have to
add much more validation code. For example, in a case where grp 0 has "item
eth/ip proto is udp / aciton goto grp 1" and grp 1 has "item tcp ...", we should
decide whether this is a violation or not. IIRC, that's why we decided to not
allow such flows during the design review. Users have to specify full items
starting from 'eth' with the current implementation.

Will submit v3 with the change in Makefile and meson.build. But if you think
there's need to add additional features like I answered above, let me know so
that I can submit v4.


Thanks,
Yongseok

Re: [dpdk-dev] [PATCH v2] net/mlx5: support multiple groups and jump action

[Shahaf Shuler]

Wednesday, October 10, 2018 5:48 AM, Yongseok Koh:
> Subject: Re: [PATCH v2] net/mlx5: support multiple groups and jump action
> On Tue, Oct 09, 2018 at 05:50:30AM -0700, Shahaf Shuler wrote:
> > Hi Koh,
> > Few comments.
> >
> > Monday, October 8, 2018 9:06 PM¸ Yongseok Koh:
> > > Subject: [PATCH v2] net/mlx5: support multiple groups and jump
> > > action
> > >

[...]

> > > -#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP |
> > > MLX5_FLOW_ACTION_PORT_ID)
> > > +/* Due to a limitation on driver/FW. */ #define
> > > MLX5_TCF_GROUP_ID_MAX 3
> > > +#define MLX5_TCF_GROUP_PRIORITY_MAX 14
> >
> > I guess there is no way to query those and trial and error is overkill for this
> first implementation.
> 
> Well, not a huge task. If you (or FW/drv team) think this max value is likely
> increased in the near future (before the next LTS version), then it isn't a bad
> idea to add such code now. Let me know.

I don't think it is needed now. Even if it is increased we can stay with 3 tables till we will have a use case for more. 

> 
> > > +
> > > +#define MLX5_TCF_FATE_ACTIONS \

[...]

> >
> >
> > We also spoke about that for group > 0 the flow items can start from the
> middle. E.g. on table 0 we have flow rule that redirect all eth/ip to group 1,
> and on group 1 the rules can start with udp or tcp.
> > Is this possible today w/o any code change?
> 
> Not possible. It needs more changes. Complexity of the additional code
> depends on a set of limitations we make. In the simplest way, we can
> unconditionally allow such flows if TCF allows it. But if we need smarter way,
> we would have to add much more validation code. For example, in a case
> where grp 0 has "item eth/ip proto is udp / aciton goto grp 1" and grp 1 has
> "item tcp ...", we should decide whether this is a violation or not. 

This is an application error. Obviously the TCP rule will never hit.  

IIRC, that's
> why we decided to not allow such flows during the design review. Users
> have to specify full items starting from 'eth' with the current implementation.
> 
> Will submit v3 with the change in Makefile and meson.build. But if you think
> there's need to add additional features like I answered above, let me know
> so that I can submit v4.

I think we need to allow items in the groups to start from arbitrary header and not always eth. It can be in the most simple way like you mentioned. 
It is needed because:
1. to save the user the overhead of creating the same pattern prefix for the rules in the groups
2. it fits better with OVS and openflow model. For example you will have one table for the outer header and one table for the inner header each contains only the outer/inner headers. 
3. not sure how kernel inserts the rule but it is an overhead to create the full steering entry from the outer eth in case it was already being matched by previous rules. It is bigger steering entries with more cache misses and comparators. 

> 
> 
> Thanks,
> Yongseok

[dpdk-dev] [PATCH v3] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

rte_flow has 'group' attribute and 'jump' action in order to support
multiple groups. This feature is known as multi-table support ('chain' in
linux TC flower) in general because a group means a table of flows. Example
commands are:

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 100 / end
	     actions jump group 1 / end

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 200 / end
	     actions jump group 2 / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern eth / vlan vid is 100 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions drop / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern end
	     actions of_pop_vlan / port_id id 1 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern eth / vlan vid is 200 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions of_pop_vlan / port_id id 2 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern end
	     actions port_id id 2 / end

With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1,
this packet will firstly hit the 1st flow. Then it will hit the 5th flow
because of the 'jump' action. As a result, the packet will be forwarded to
port 2 (VF representor) with vlan tag being stripped off. If the packet had
vlan 100 instead, it would be dropped by the 3rd flow.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
Acked-by: Ori Kam <orika@mellanox.com>
---

v3:
* reorder build macros

v2:
* drop ethdev patch as it had already been fixed by Adrien's patch
* move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c

 drivers/net/mlx5/Makefile        | 10 +++++
 drivers/net/mlx5/meson.build     |  4 ++
 drivers/net/mlx5/mlx5_flow.h     |  1 +
 drivers/net/mlx5/mlx5_flow_tcf.c | 82 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 88 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index ca1de9f210..928837bb85 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -207,6 +207,11 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum IFLA_PHYS_PORT_NAME \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TCA_CHAIN \
+		linux/rtnetlink.h \
+		enum TCA_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_TCA_FLOWER_ACT \
 		linux/pkt_cls.h \
 		enum TCA_FLOWER_ACT \
@@ -342,6 +347,11 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum TCA_FLOWER_KEY_VLAN_ETH_TYPE \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TC_ACT_GOTO_CHAIN \
+		linux/pkt_cls.h \
+		define TC_ACT_GOTO_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_TC_ACT_VLAN \
 		linux/tc_act/tc_vlan.h \
 		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index fd93ac1625..025dcc9e96 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -126,6 +126,8 @@ if build
 		'IFLA_PHYS_SWITCH_ID' ],
 		[ 'HAVE_IFLA_PHYS_PORT_NAME', 'linux/if_link.h',
 		'IFLA_PHYS_PORT_NAME' ],
+		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
+		'TCA_CHAIN' ],
 		[ 'HAVE_TCA_FLOWER_ACT', 'linux/pkt_cls.h',
 		'TCA_FLOWER_ACT' ],
 		[ 'HAVE_TCA_FLOWER_FLAGS', 'linux/pkt_cls.h',
@@ -180,6 +182,8 @@ if build
 		'TCA_FLOWER_KEY_VLAN_PRIO' ],
 		[ 'HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE', 'linux/pkt_cls.h',
 		'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ],
+		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
+		'TC_ACT_GOTO_CHAIN' ],
 		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
 		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
 		[ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h',
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 12de841e86..3ed0ddc585 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -78,6 +78,7 @@
 #define MLX5_FLOW_ACTION_OF_PUSH_VLAN (1u << 8)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_VID (1u << 9)
 #define MLX5_FLOW_ACTION_OF_SET_VLAN_PCP (1u << 10)
+#define MLX5_FLOW_ACTION_JUMP (1u << 11)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index 91f6ef6786..fbc4c2bb7c 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -148,6 +148,12 @@ struct tc_vlan {
 #ifndef HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE
 #define TCA_FLOWER_KEY_VLAN_ETH_TYPE 25
 #endif
+#ifndef HAVE_TCA_CHAIN
+#define TCA_CHAIN 11
+#endif
+#ifndef HAVE_TC_ACT_GOTO_CHAIN
+#define TC_ACT_GOTO_CHAIN 0x20000000
+#endif
 
 #ifndef IPV6_ADDR_LEN
 #define IPV6_ADDR_LEN 16
@@ -225,7 +231,13 @@ struct flow_tcf_ptoi {
 	unsigned int ifindex; /**< Network interface index. */
 };
 
-#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
+/* Due to a limitation on driver/FW. */
+#define MLX5_TCF_GROUP_ID_MAX 3
+#define MLX5_TCF_GROUP_PRIORITY_MAX 14
+
+#define MLX5_TCF_FATE_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
+	 MLX5_FLOW_ACTION_JUMP)
 #define MLX5_TCF_VLAN_ACTIONS \
 	(MLX5_FLOW_ACTION_OF_POP_VLAN | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
 	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
@@ -370,14 +382,25 @@ flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
 			     struct rte_flow_error *error)
 {
 	/*
-	 * Supported attributes: no groups, some priorities and ingress only.
-	 * Don't care about transfer as it is the caller's problem.
+	 * Supported attributes: groups, some priorities and ingress only.
+	 * group is supported only if kernel supports chain. Don't care about
+	 * transfer as it is the caller's problem.
 	 */
-	if (attr->group)
+	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
-					  "groups are not supported");
-	if (attr->priority > 0xfffe)
+					  "group ID larger than "
+					  RTE_STR(MLX5_TCF_GROUP_ID_MAX)
+					  " isn't supported");
+	else if (attr->group > 0 &&
+		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+					  attr,
+					  "lowest priority level is "
+					  RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
+					  " when group is configured");
+	else if (attr->priority > 0xfffe)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
 					  attr,
@@ -428,6 +451,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -675,6 +699,16 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
 			port_id_dev = &rte_eth_devices[conf.port_id->id];
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			if (attr->group >= conf.jump->group)
+				return rte_flow_error_set
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION,
+					 actions,
+					 "can't jump to a group backward");
+			action_flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			if (action_flags & MLX5_TCF_FATE_ACTIONS)
 				return rte_flow_error_set
@@ -757,7 +791,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
  *   Maximum size of memory for items.
  */
 static int
-flow_tcf_get_items_and_size(const struct rte_flow_item items[],
+flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item items[],
 			    uint64_t *item_flags)
 {
 	int size = 0;
@@ -766,6 +801,8 @@ flow_tcf_get_items_and_size(const struct rte_flow_item items[],
 	size += SZ_NLATTR_STRZ_OF("flower") +
 		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
 		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW. */
+	if (attr->group > 0)
+		size += SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CHAIN. */
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
@@ -855,6 +892,13 @@ flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
 			flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			size += SZ_NLATTR_NEST + /* na_act_index. */
+				SZ_NLATTR_STRZ_OF("gact") +
+				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
+				SZ_NLATTR_TYPE_OF(struct tc_gact);
+			flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
 				SZ_NLATTR_STRZ_OF("gact") +
@@ -940,7 +984,7 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t handle)
  *   otherwise NULL and rte_ernno is set.
  */
 static struct mlx5_flow *
-flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
+flow_tcf_prepare(const struct rte_flow_attr *attr,
 		 const struct rte_flow_item items[],
 		 const struct rte_flow_action actions[],
 		 uint64_t *item_flags, uint64_t *action_flags,
@@ -953,7 +997,7 @@ flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
 	struct nlmsghdr *nlh;
 	struct tcmsg *tcm;
 
-	size += flow_tcf_get_items_and_size(items, item_flags);
+	size += flow_tcf_get_items_and_size(attr, items, item_flags);
 	size += flow_tcf_get_actions_and_size(actions, action_flags);
 	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
 	if (!dev_flow) {
@@ -1024,6 +1068,7 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -1058,6 +1103,8 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	 */
 	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
 				  RTE_BE16(ETH_P_ALL));
+	if (attr->group > 0)
+		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
 	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
 	na_flower = mnl_attr_nest_start(nlh, TCA_OPTIONS);
 	mnl_attr_put_u32(nlh, TCA_FLOWER_FLAGS, TCA_CLS_FLAGS_SKIP_SW);
@@ -1332,6 +1379,23 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 			mnl_attr_nest_end(nlh, na_act);
 			mnl_attr_nest_end(nlh, na_act_index);
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			na_act_index =
+				mnl_attr_nest_start(nlh, na_act_index_cur++);
+			assert(na_act_index);
+			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
+			na_act = mnl_attr_nest_start(nlh, TCA_ACT_OPTIONS);
+			assert(na_act);
+			mnl_attr_put(nlh, TCA_GACT_PARMS,
+				     sizeof(struct tc_gact),
+				     &(struct tc_gact){
+					.action = TC_ACT_GOTO_CHAIN |
+						  conf.jump->group,
+				     });
+			mnl_attr_nest_end(nlh, na_act);
+			mnl_attr_nest_end(nlh, na_act_index);
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			na_act_index =
 				mnl_attr_nest_start(nlh, na_act_index_cur++);
-- 
2.11.0

[dpdk-dev] [PATCH v4] net/mlx5: support multiple groups and jump action

[Yongseok Koh]

rte_flow has 'group' attribute and 'jump' action in order to support
multiple groups. This feature is known as multi-table support ('chain' in
linux TC flower) in general because a group means a table of flows. Example
commands are:

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 100 / end
	     actions jump group 1 / end

	flow create 0 transfer priority 1 ingress
	     pattern eth / vlan vid is 200 / end
	     actions jump group 2 / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern eth / vlan vid is 100 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions drop / end

	flow create 0 transfer group 1 priority 2 ingress
	     pattern end
	     actions of_pop_vlan / port_id id 1 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern eth / vlan vid is 200 /
	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
	     actions of_pop_vlan / port_id id 2 / end

	flow create 0 transfer group 2 priority 2 ingress
	     pattern end
	     actions port_id id 2 / end

With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1,
this packet will firstly hit the 1st flow. Then it will hit the 5th flow
because of the 'jump' action. As a result, the packet will be forwarded to
port 2 (VF representor) with vlan tag being stripped off. If the packet had
vlan 100 instead, it would be dropped by the 3rd flow.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---

v4:
* rebase on "net/mlx5: rewrite IP address UDP/TCP port by E-Switch"

v3:
* reorder build macros

v2:
* drop ethdev patch as it had already been fixed by Adrien's patch
* move TCF macros from mlx5_flow.h to mlx5_flow_tcf.c

 drivers/net/mlx5/Makefile        |  10 ++++
 drivers/net/mlx5/meson.build     |   4 ++
 drivers/net/mlx5/mlx5_flow.h     |   1 +
 drivers/net/mlx5/mlx5_flow_tcf.c | 110 +++++++++++++++++++++++++++++++--------
 4 files changed, 102 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/Makefile b/drivers/net/mlx5/Makefile
index 1c0b848d5a..1e9c0b42ac 100644
--- a/drivers/net/mlx5/Makefile
+++ b/drivers/net/mlx5/Makefile
@@ -207,6 +207,11 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum IFLA_PHYS_PORT_NAME \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TCA_CHAIN \
+		linux/rtnetlink.h \
+		enum TCA_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_TCA_FLOWER_ACT \
 		linux/pkt_cls.h \
 		enum TCA_FLOWER_ACT \
@@ -352,6 +357,11 @@ mlx5_autoconf.h.new: $(RTE_SDK)/buildtools/auto-config-h.sh
 		enum TCA_FLOWER_KEY_TCP_FLAGS_MASK \
 		$(AUTOCONF_OUTPUT)
 	$Q sh -- '$<' '$@' \
+		HAVE_TC_ACT_GOTO_CHAIN \
+		linux/pkt_cls.h \
+		define TC_ACT_GOTO_CHAIN \
+		$(AUTOCONF_OUTPUT)
+	$Q sh -- '$<' '$@' \
 		HAVE_TC_ACT_VLAN \
 		linux/tc_act/tc_vlan.h \
 		enum TCA_VLAN_PUSH_VLAN_PRIORITY \
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index 801f02871f..c192d44c10 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -126,6 +126,8 @@ if build
 		'IFLA_PHYS_SWITCH_ID' ],
 		[ 'HAVE_IFLA_PHYS_PORT_NAME', 'linux/if_link.h',
 		'IFLA_PHYS_PORT_NAME' ],
+		[ 'HAVE_TCA_CHAIN', 'linux/rtnetlink.h',
+		'TCA_CHAIN' ],
 		[ 'HAVE_TCA_FLOWER_ACT', 'linux/pkt_cls.h',
 		'TCA_FLOWER_ACT' ],
 		[ 'HAVE_TCA_FLOWER_FLAGS', 'linux/pkt_cls.h',
@@ -184,6 +186,8 @@ if build
 		'TCA_FLOWER_KEY_TCP_FLAGS' ],
 		[ 'HAVE_TCA_FLOWER_KEY_TCP_FLAGS_MASK', 'linux/pkt_cls.h',
 		'TCA_FLOWER_KEY_TCP_FLAGS_MASK' ],
+		[ 'HAVE_TC_ACT_GOTO_CHAIN', 'linux/pkt_cls.h',
+		'TC_ACT_GOTO_CHAIN' ],
 		[ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h',
 		'TCA_VLAN_PUSH_VLAN_PRIORITY' ],
 		[ 'HAVE_TC_ACT_PEDIT', 'linux/tc_act/tc_pedit.h',
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 860fb1f6e8..094f6668fa 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -84,6 +84,7 @@
 #define MLX5_FLOW_ACTION_SET_IPV6_DST (1u << 14)
 #define MLX5_FLOW_ACTION_SET_TP_SRC (1u << 15)
 #define MLX5_FLOW_ACTION_SET_TP_DST (1u << 16)
+#define MLX5_FLOW_ACTION_JUMP (1u << 17)
 
 #define MLX5_FLOW_FATE_ACTIONS \
 	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)
diff --git a/drivers/net/mlx5/mlx5_flow_tcf.c b/drivers/net/mlx5/mlx5_flow_tcf.c
index aaafaf52d6..1e1102e079 100644
--- a/drivers/net/mlx5/mlx5_flow_tcf.c
+++ b/drivers/net/mlx5/mlx5_flow_tcf.c
@@ -124,6 +124,9 @@ struct tc_pedit_sel {
 #ifndef TCA_CLS_FLAGS_SKIP_SW
 #define TCA_CLS_FLAGS_SKIP_SW (1 << 1)
 #endif
+#ifndef HAVE_TCA_CHAIN
+#define TCA_CHAIN 11
+#endif
 #ifndef HAVE_TCA_FLOWER_ACT
 #define TCA_FLOWER_ACT 3
 #endif
@@ -211,6 +214,9 @@ struct tc_pedit_sel {
 #ifndef HAVE_TCA_FLOWER_KEY_TCP_FLAGS_MASK
 #define TCA_FLOWER_KEY_TCP_FLAGS_MASK 72
 #endif
+#ifndef HAVE_TC_ACT_GOTO_CHAIN
+#define TC_ACT_GOTO_CHAIN 0x20000000
+#endif
 
 #ifndef IPV6_ADDR_LEN
 #define IPV6_ADDR_LEN 16
@@ -297,7 +303,14 @@ struct flow_tcf_ptoi {
 	unsigned int ifindex; /**< Network interface index. */
 };
 
-#define MLX5_TCF_FATE_ACTIONS (MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID)
+/* Due to a limitation on driver/FW. */
+#define MLX5_TCF_GROUP_ID_MAX 3
+#define MLX5_TCF_GROUP_PRIORITY_MAX 14
+
+#define MLX5_TCF_FATE_ACTIONS \
+	(MLX5_FLOW_ACTION_DROP | MLX5_FLOW_ACTION_PORT_ID | \
+	 MLX5_FLOW_ACTION_JUMP)
+
 #define MLX5_TCF_VLAN_ACTIONS \
 	(MLX5_FLOW_ACTION_OF_POP_VLAN | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
 	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP)
@@ -308,9 +321,9 @@ struct flow_tcf_ptoi {
 	 MLX5_FLOW_ACTION_SET_TP_SRC | MLX5_FLOW_ACTION_SET_TP_DST)
 
 #define MLX5_TCF_CONFIG_ACTIONS \
-	(MLX5_FLOW_ACTION_PORT_ID | MLX5_FLOW_ACTION_OF_PUSH_VLAN | \
-	 MLX5_FLOW_ACTION_OF_SET_VLAN_VID | MLX5_FLOW_ACTION_OF_SET_VLAN_PCP | \
-	 MLX5_TCF_PEDIT_ACTIONS)
+	(MLX5_FLOW_ACTION_PORT_ID | MLX5_FLOW_ACTION_JUMP | \
+	 MLX5_FLOW_ACTION_OF_PUSH_VLAN | MLX5_FLOW_ACTION_OF_SET_VLAN_VID | \
+	 MLX5_FLOW_ACTION_OF_SET_VLAN_PCP | MLX5_TCF_PEDIT_ACTIONS)
 
 #define MAX_PEDIT_KEYS 128
 #define SZ_PEDIT_KEY_VAL 4
@@ -704,14 +717,25 @@ flow_tcf_validate_attributes(const struct rte_flow_attr *attr,
 			     struct rte_flow_error *error)
 {
 	/*
-	 * Supported attributes: no groups, some priorities and ingress only.
-	 * Don't care about transfer as it is the caller's problem.
+	 * Supported attributes: groups, some priorities and ingress only.
+	 * group is supported only if kernel supports chain. Don't care about
+	 * transfer as it is the caller's problem.
 	 */
-	if (attr->group)
+	if (attr->group > MLX5_TCF_GROUP_ID_MAX)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, attr,
-					  "groups are not supported");
-	if (attr->priority > 0xfffe)
+					  "group ID larger than "
+					  RTE_STR(MLX5_TCF_GROUP_ID_MAX)
+					  " isn't supported");
+	else if (attr->group > 0 &&
+		 attr->priority > MLX5_TCF_GROUP_PRIORITY_MAX)
+		return rte_flow_error_set(error, ENOTSUP,
+					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+					  attr,
+					  "lowest priority level is "
+					  RTE_STR(MLX5_TCF_GROUP_PRIORITY_MAX)
+					  " when group is configured");
+	else if (attr->priority > 0xfffe)
 		return rte_flow_error_set(error, ENOTSUP,
 					  RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
 					  attr,
@@ -762,6 +786,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -995,11 +1020,6 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 			break;
 		case RTE_FLOW_ACTION_TYPE_PORT_ID:
 			current_action_flag = MLX5_FLOW_ACTION_PORT_ID;
-			if (action_flags & MLX5_TCF_FATE_ACTIONS)
-				return rte_flow_error_set
-					(error, EINVAL,
-					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
-					 "can't have multiple fate actions");
 			if (!actions->conf)
 				break;
 			conf.port_id = actions->conf;
@@ -1018,12 +1038,19 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 					 " ifindex");
 			port_id_dev = &rte_eth_devices[conf.port_id->id];
 			break;
-		case RTE_FLOW_ACTION_TYPE_DROP:
-			if (action_flags & MLX5_TCF_FATE_ACTIONS)
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			current_action_flag = MLX5_FLOW_ACTION_JUMP;
+			if (!actions->conf)
+				break;
+			conf.jump = actions->conf;
+			if (attr->group >= conf.jump->group)
 				return rte_flow_error_set
-					(error, EINVAL,
-					 RTE_FLOW_ERROR_TYPE_ACTION, actions,
-					 "can't have multiple fate actions");
+					(error, ENOTSUP,
+					 RTE_FLOW_ERROR_TYPE_ACTION,
+					 actions,
+					 "can jump only to a group forward");
+			break;
+		case RTE_FLOW_ACTION_TYPE_DROP:
 			current_action_flag = MLX5_FLOW_ACTION_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_OF_POP_VLAN:
@@ -1082,7 +1109,7 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 						"action configuration not set");
 		}
 		if ((current_action_flag & MLX5_TCF_PEDIT_ACTIONS) &&
-				pedit_validated)
+		    pedit_validated)
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  actions,
@@ -1091,6 +1118,13 @@ flow_tcf_validate(struct rte_eth_dev *dev,
 		if ((current_action_flag & ~MLX5_TCF_PEDIT_ACTIONS) &&
 		    (action_flags & MLX5_TCF_PEDIT_ACTIONS))
 			pedit_validated = 1;
+		if ((current_action_flag & MLX5_TCF_FATE_ACTIONS) &&
+		    (action_flags & MLX5_TCF_FATE_ACTIONS))
+			return rte_flow_error_set(error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  actions,
+						  "can't have multiple fate"
+						  " actions");
 		action_flags |= current_action_flag;
 	}
 	if ((action_flags & MLX5_TCF_PEDIT_ACTIONS) &&
@@ -1179,7 +1213,8 @@ flow_tcf_validate(struct rte_eth_dev *dev,
  *   Maximum size of memory for items.
  */
 static int
-flow_tcf_get_items_and_size(const struct rte_flow_item items[],
+flow_tcf_get_items_and_size(const struct rte_flow_attr *attr,
+			    const struct rte_flow_item items[],
 			    uint64_t *item_flags)
 {
 	int size = 0;
@@ -1188,6 +1223,8 @@ flow_tcf_get_items_and_size(const struct rte_flow_item items[],
 	size += SZ_NLATTR_STRZ_OF("flower") +
 		SZ_NLATTR_NEST + /* TCA_OPTIONS. */
 		SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CLS_FLAGS_SKIP_SW. */
+	if (attr->group > 0)
+		size += SZ_NLATTR_TYPE_OF(uint32_t); /* TCA_CHAIN. */
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		switch (items->type) {
 		case RTE_FLOW_ITEM_TYPE_VOID:
@@ -1277,6 +1314,13 @@ flow_tcf_get_actions_and_size(const struct rte_flow_action actions[],
 				SZ_NLATTR_TYPE_OF(struct tc_mirred);
 			flags |= MLX5_FLOW_ACTION_PORT_ID;
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			size += SZ_NLATTR_NEST + /* na_act_index. */
+				SZ_NLATTR_STRZ_OF("gact") +
+				SZ_NLATTR_NEST + /* TCA_ACT_OPTIONS. */
+				SZ_NLATTR_TYPE_OF(struct tc_gact);
+			flags |= MLX5_FLOW_ACTION_JUMP;
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			size += SZ_NLATTR_NEST + /* na_act_index. */
 				SZ_NLATTR_STRZ_OF("gact") +
@@ -1371,7 +1415,7 @@ flow_tcf_nl_brand(struct nlmsghdr *nlh, uint32_t handle)
  *   otherwise NULL and rte_ernno is set.
  */
 static struct mlx5_flow *
-flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
+flow_tcf_prepare(const struct rte_flow_attr *attr,
 		 const struct rte_flow_item items[],
 		 const struct rte_flow_action actions[],
 		 uint64_t *item_flags, uint64_t *action_flags,
@@ -1384,7 +1428,7 @@ flow_tcf_prepare(const struct rte_flow_attr *attr __rte_unused,
 	struct nlmsghdr *nlh;
 	struct tcmsg *tcm;
 
-	size += flow_tcf_get_items_and_size(items, item_flags);
+	size += flow_tcf_get_items_and_size(attr, items, item_flags);
 	size += flow_tcf_get_actions_and_size(actions, action_flags);
 	dev_flow = rte_zmalloc(__func__, size, MNL_ALIGNTO);
 	if (!dev_flow) {
@@ -1455,6 +1499,7 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	} spec, mask;
 	union {
 		const struct rte_flow_action_port_id *port_id;
+		const struct rte_flow_action_jump *jump;
 		const struct rte_flow_action_of_push_vlan *of_push_vlan;
 		const struct rte_flow_action_of_set_vlan_vid *
 			of_set_vlan_vid;
@@ -1490,6 +1535,8 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 	 */
 	tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16,
 				  RTE_BE16(ETH_P_ALL));
+	if (attr->group > 0)
+		mnl_attr_put_u32(nlh, TCA_CHAIN, attr->group);
 	mnl_attr_put_strz(nlh, TCA_KIND, "flower");
 	na_flower = mnl_attr_nest_start(nlh, TCA_OPTIONS);
 	mnl_attr_put_u32(nlh, TCA_FLOWER_FLAGS, TCA_CLS_FLAGS_SKIP_SW);
@@ -1782,6 +1829,23 @@ flow_tcf_translate(struct rte_eth_dev *dev, struct mlx5_flow *dev_flow,
 			mnl_attr_nest_end(nlh, na_act);
 			mnl_attr_nest_end(nlh, na_act_index);
 			break;
+		case RTE_FLOW_ACTION_TYPE_JUMP:
+			conf.jump = actions->conf;
+			na_act_index =
+				mnl_attr_nest_start(nlh, na_act_index_cur++);
+			assert(na_act_index);
+			mnl_attr_put_strz(nlh, TCA_ACT_KIND, "gact");
+			na_act = mnl_attr_nest_start(nlh, TCA_ACT_OPTIONS);
+			assert(na_act);
+			mnl_attr_put(nlh, TCA_GACT_PARMS,
+				     sizeof(struct tc_gact),
+				     &(struct tc_gact){
+					.action = TC_ACT_GOTO_CHAIN |
+						  conf.jump->group,
+				     });
+			mnl_attr_nest_end(nlh, na_act);
+			mnl_attr_nest_end(nlh, na_act_index);
+			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			na_act_index =
 				mnl_attr_nest_start(nlh, na_act_index_cur++);
-- 
2.11.0

Re: [dpdk-dev] [PATCH v4] net/mlx5: support multiple groups and jump action

[Shahaf Shuler]

Friday, October 12, 2018 11:43 AM, Yongseok Koh:
> Subject: [PATCH v4] net/mlx5: support multiple groups and jump action
> 
> rte_flow has 'group' attribute and 'jump' action in order to support multiple
> groups. This feature is known as multi-table support ('chain' in linux TC
> flower) in general because a group means a table of flows. Example
> commands are:
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 100 / end
> 	     actions jump group 1 / end
> 
> 	flow create 0 transfer priority 1 ingress
> 	     pattern eth / vlan vid is 200 / end
> 	     actions jump group 2 / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern eth / vlan vid is 100 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions drop / end
> 
> 	flow create 0 transfer group 1 priority 2 ingress
> 	     pattern end
> 	     actions of_pop_vlan / port_id id 1 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern eth / vlan vid is 200 /
> 	     	     ipv4 dst spec 192.168.40.0 dst prefix 24 / end
> 	     actions of_pop_vlan / port_id id 2 / end
> 
> 	flow create 0 transfer group 2 priority 2 ingress
> 	     pattern end
> 	     actions port_id id 2 / end
> 
> With theses flows, if a packet having vlan 200 and src_ip as 192.168.40.1, this
> packet will firstly hit the 1st flow. Then it will hit the 5th flow because of the
> 'jump' action. As a result, the packet will be forwarded to port 2 (VF
> representor) with vlan tag being stripped off. If the packet had vlan 100
> instead, it would be dropped by the 3rd flow.
> 
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---

Applied to next-net-mlx, thanks.