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

Shahaf Shuler shahafs at mellanox.com
Tue Oct 9 14:50:30 CEST 2018


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 at 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



More information about the dev mailing list