Re: [PATCH net-next v4 1/3] net: flow_dissector: add support for cfm packets

[Ido Schimmel <idosch@idosch.org>]

On Tue, Apr 25, 2023 at 11:16:28PM +0200, Zahari Doychev wrote:
> From: Zahari Doychev <zdoychev@maxlinear.com>
> 
> Add support for dissecting cfm packets. The cfm packet header
> fields maintenance domain level and opcode can be dissected.
> 
> Signed-off-by: Zahari Doychev <zdoychev@maxlinear.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> ---
>  include/net/flow_dissector.h | 20 ++++++++++++++++++++
>  net/core/flow_dissector.c    | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index 85b2281576ed..479b66b11d2d 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -299,6 +299,25 @@ struct flow_dissector_key_l2tpv3 {
>  	__be32 session_id;
>  };
>  
> +/**
> + * struct flow_dissector_key_cfm
> + * @mdl_ver: maintenance domain level(mdl) and cfm protocol version
                                        ^ missing space

> + * @opcode: code specifying a type of cfm protocol packet
> + *
> + * See 802.1ag, ITU-T G.8013/Y.1731
> + *         1               2
> + * |7 6 5 4 3 2 1 0|7 6 5 4 3 2 1 0|
> + * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + * | mdl | version |     opcode    |
> + * +-----+---------+-+-+-+-+-+-+-+-+
> + */
> +struct flow_dissector_key_cfm {
> +	u8	mdl_ver;
> +	u8	opcode;
> +};
> +
> +#define FLOW_DIS_CFM_MDL_MASK GENMASK(7, 5)
> +
>  enum flow_dissector_key_id {
>  	FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
>  	FLOW_DISSECTOR_KEY_BASIC, /* struct flow_dissector_key_basic */
> @@ -331,6 +350,7 @@ enum flow_dissector_key_id {
>  	FLOW_DISSECTOR_KEY_NUM_OF_VLANS, /* struct flow_dissector_key_num_of_vlans */
>  	FLOW_DISSECTOR_KEY_PPPOE, /* struct flow_dissector_key_pppoe */
>  	FLOW_DISSECTOR_KEY_L2TPV3, /* struct flow_dissector_key_l2tpv3 */
> +	FLOW_DISSECTOR_KEY_CFM, /* struct flow_dissector_key_cfm */
>  
>  	FLOW_DISSECTOR_KEY_MAX,
>  };
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 25fb0bbc310f..62cc1be693de 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -547,6 +547,30 @@ __skb_flow_dissect_arp(const struct sk_buff *skb,
>  	return FLOW_DISSECT_RET_OUT_GOOD;
>  }
>  
> +static enum flow_dissect_ret
> +__skb_flow_dissect_cfm(const struct sk_buff *skb,
> +		       struct flow_dissector *flow_dissector,
> +		       void *target_container, const void *data,
> +		       int nhoff, int hlen)
> +{
> +	struct flow_dissector_key_cfm *key, *hdr, _hdr;
> +
> +	if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_CFM))
> +		return FLOW_DISSECT_RET_OUT_GOOD;
> +
> +	hdr = __skb_header_pointer(skb, nhoff, sizeof(*key), data, hlen, &_hdr);
> +	if (!hdr)
> +		return FLOW_DISSECT_RET_OUT_BAD;
> +
> +	key = skb_flow_dissector_target(flow_dissector, FLOW_DISSECTOR_KEY_CFM,
> +					target_container);
> +
> +	key->mdl_ver = hdr->mdl_ver;
> +	key->opcode = hdr->opcode;
> +
> +	return  FLOW_DISSECT_RET_OUT_GOOD;
              ^ double space

> +}
> +
>  static enum flow_dissect_ret
>  __skb_flow_dissect_gre(const struct sk_buff *skb,
>  		       struct flow_dissector_key_control *key_control,
> @@ -1390,6 +1414,12 @@ bool __skb_flow_dissect(const struct net *net,
>  		break;
>  	}
>  
> +	case htons(ETH_P_CFM): {
> +		fdret = __skb_flow_dissect_cfm(skb, flow_dissector,
> +					       target_container, data,
> +					       nhoff, hlen);
> +		break;
> +	}

No variables are declared, drop the braces?

>  	default:
>  		fdret = FLOW_DISSECT_RET_OUT_BAD;
>  		break;
> -- 
> 2.40.0
>