Subject: Re: [PATCH net-next v5 2/3] net: flower: add support for matching cfm fields

From: Ido Schimmel <[email-masked]>
Date: Mon, 5 Jun 2023 10:36:23 +0300

On Sun, Jun 04, 2023 at 01:58:24PM +0200, Zahari Doychev wrote:
> From: Zahari Doychev <[email-masked]>
> 
> Add support to the tc flower classifier to match based on fields in CFM
> information elements like level and opcode.
> 
> tc filter add dev ens6 ingress protocol 802.1q \
> 	flower vlan_id 698 vlan_ethtype 0x8902 cfm mdl 5 op 46 \
> 	action drop
> 
> Signed-off-by: Zahari Doychev <[email-masked]>
> Reviewed-by: Simon Horman <[email-masked]>
> ---
>  include/uapi/linux/pkt_cls.h |   9 ++
>  net/sched/cls_flower.c       | 195 ++++++++++++++++++++++++++---------
>  2 files changed, 158 insertions(+), 46 deletions(-)
> 
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 00933dda7b10..7865f5a9885b 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -596,6 +596,8 @@ enum {
>  
>  	TCA_FLOWER_L2_MISS,		/* u8 */
>  
> +	TCA_FLOWER_KEY_CFM,		/* nested */
> +
>  	__TCA_FLOWER_MAX,
>  };
>  
> @@ -704,6 +706,13 @@ enum {
>  	TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
>  };
>  
> +enum {
> +	TCA_FLOWER_KEY_CFM_OPT_UNSPEC,
> +	TCA_FLOWER_KEY_CFM_MD_LEVEL,
> +	TCA_FLOWER_KEY_CFM_OPCODE,
> +	TCA_FLOWER_KEY_CFM_OPT_MAX,
> +};
> +
>  #define TCA_FLOWER_MASK_FLAGS_RANGE	(1 << 0) /* Range-based match */
>  
>  /* Match-all classifier */
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index e02ecabbb75c..b32f5423721b 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -11,6 +11,7 @@
>  #include <linux/rhashtable.h>
>  #include <linux/workqueue.h>
>  #include <linux/refcount.h>
> +#include <linux/bitfield.h>
>  
>  #include <linux/if_ether.h>
>  #include <linux/in6.h>
> @@ -71,6 +72,7 @@ struct fl_flow_key {
>  	struct flow_dissector_key_num_of_vlans num_of_vlans;
>  	struct flow_dissector_key_pppoe pppoe;
>  	struct flow_dissector_key_l2tpv3 l2tpv3;
> +	struct flow_dissector_key_cfm cfm;
>  } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
>  
>  struct fl_flow_mask_range {
> @@ -617,6 +619,58 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
>  	return __fl_get(head, handle);
>  }
>  
> +static const struct nla_policy
> +enc_opts_policy[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1] = {
> +	[TCA_FLOWER_KEY_ENC_OPTS_UNSPEC]        = {
> +		.strict_start_type = TCA_FLOWER_KEY_ENC_OPTS_VXLAN },
> +	[TCA_FLOWER_KEY_ENC_OPTS_GENEVE]        = { .type = NLA_NESTED },
> +	[TCA_FLOWER_KEY_ENC_OPTS_VXLAN]         = { .type = NLA_NESTED },
> +	[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN]        = { .type = NLA_NESTED },
> +	[TCA_FLOWER_KEY_ENC_OPTS_GTP]		= { .type = NLA_NESTED },
> +};
> +
> +static const struct nla_policy
> +geneve_opt_policy[TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX + 1] = {
> +	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_CLASS]      = { .type = NLA_U16 },
> +	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_TYPE]       = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_DATA]       = { .type = NLA_BINARY,
> +						       .len = 128 },
> +};
> +
> +static const struct nla_policy
> +vxlan_opt_policy[TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX + 1] = {
> +	[TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP]         = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy
> +erspan_opt_policy[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX + 1] = {
> +	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_VER]        = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_INDEX]      = { .type = NLA_U32 },
> +	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_DIR]        = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_HWID]       = { .type = NLA_U8 },
> +};
> +
> +static const struct nla_policy
> +gtp_opt_policy[TCA_FLOWER_KEY_ENC_OPT_GTP_MAX + 1] = {
> +	[TCA_FLOWER_KEY_ENC_OPT_GTP_PDU_TYPE]	   = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_ENC_OPT_GTP_QFI]	   = { .type = NLA_U8 },
> +};
> +
> +static const struct nla_policy
> +mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
> +	[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH]    = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL]      = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS]      = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC]       = { .type = NLA_U8 },
> +	[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL]    = { .type = NLA_U32 },
> +};
> +
> +static const struct nla_policy cfm_opt_policy[TCA_FLOWER_KEY_CFM_OPT_MAX] = {
> +	[TCA_FLOWER_KEY_CFM_MD_LEVEL]	= NLA_POLICY_MAX(NLA_U8,
> +						FLOW_DIS_CFM_MDL_MAX),
> +	[TCA_FLOWER_KEY_CFM_OPCODE]	= { .type = NLA_U8 },
> +};
> +
>  static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
>  	[TCA_FLOWER_UNSPEC]		= { .strict_start_type =
>  						TCA_FLOWER_L2_MISS },
> @@ -725,52 +779,7 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
>  	[TCA_FLOWER_KEY_PPP_PROTO]	= { .type = NLA_U16 },
>  	[TCA_FLOWER_KEY_L2TPV3_SID]	= { .type = NLA_U32 },
>  	[TCA_FLOWER_L2_MISS]		= NLA_POLICY_MAX(NLA_U8, 1),
> -};
> -
> -static const struct nla_policy
> -enc_opts_policy[TCA_FLOWER_KEY_ENC_OPTS_MAX + 1] = {
> -	[TCA_FLOWER_KEY_ENC_OPTS_UNSPEC]        = {
> -		.strict_start_type = TCA_FLOWER_KEY_ENC_OPTS_VXLAN },
> -	[TCA_FLOWER_KEY_ENC_OPTS_GENEVE]        = { .type = NLA_NESTED },
> -	[TCA_FLOWER_KEY_ENC_OPTS_VXLAN]         = { .type = NLA_NESTED },
> -	[TCA_FLOWER_KEY_ENC_OPTS_ERSPAN]        = { .type = NLA_NESTED },
> -	[TCA_FLOWER_KEY_ENC_OPTS_GTP]		= { .type = NLA_NESTED },
> -};
> -
> -static const struct nla_policy
> -geneve_opt_policy[TCA_FLOWER_KEY_ENC_OPT_GENEVE_MAX + 1] = {
> -	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_CLASS]      = { .type = NLA_U16 },
> -	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_TYPE]       = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_ENC_OPT_GENEVE_DATA]       = { .type = NLA_BINARY,
> -						       .len = 128 },
> -};
> -
> -static const struct nla_policy
> -vxlan_opt_policy[TCA_FLOWER_KEY_ENC_OPT_VXLAN_MAX + 1] = {
> -	[TCA_FLOWER_KEY_ENC_OPT_VXLAN_GBP]         = { .type = NLA_U32 },
> -};
> -
> -static const struct nla_policy
> -erspan_opt_policy[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_MAX + 1] = {
> -	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_VER]        = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_INDEX]      = { .type = NLA_U32 },
> -	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_DIR]        = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_ENC_OPT_ERSPAN_HWID]       = { .type = NLA_U8 },
> -};
> -
> -static const struct nla_policy
> -gtp_opt_policy[TCA_FLOWER_KEY_ENC_OPT_GTP_MAX + 1] = {
> -	[TCA_FLOWER_KEY_ENC_OPT_GTP_PDU_TYPE]	   = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_ENC_OPT_GTP_QFI]	   = { .type = NLA_U8 },
> -};
> -
> -static const struct nla_policy
> -mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
> -	[TCA_FLOWER_KEY_MPLS_OPT_LSE_DEPTH]    = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TTL]      = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_MPLS_OPT_LSE_BOS]      = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_MPLS_OPT_LSE_TC]       = { .type = NLA_U8 },
> -	[TCA_FLOWER_KEY_MPLS_OPT_LSE_LABEL]    = { .type = NLA_U32 },
> +	[TCA_FLOWER_KEY_CFM]		= NLA_POLICY_NESTED(cfm_opt_policy),

I didn't suggest NLA_POLICY_NESTED() in previous versions because:

1. The code churn in this patch where different policies need to be
relocated.

2. AFAIK, rtnetlink does not support policy dump (unlike genl) which
makes this change quite meaningless.

No strong preference whether to keep it or drop it, but the purely
mechanical change of relocating policies need to be split into a patch
of its own.

And I'm sorry about the conflict with the "TCA_FLOWER_L2_MISS" stuff. I
assumed you would send v5 earlier.

>  };