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

[Ido Schimmel <idosch@idosch.org>]

On Tue, Jun 06, 2023 at 10:59:34PM +0200, Zahari Doychev wrote:
> From: Zahari Doychev <zdoychev@maxlinear.com>
> 
> 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

Looks good overall. One minor comment below.

[...]

> +static int fl_set_key_cfm_md_level(struct nlattr **tb,
> +				   struct fl_flow_key *key,
> +				   struct fl_flow_key *mask,
> +				   struct netlink_ext_ack *extack)
> +{
> +	u8 level;
> +
> +	if (!tb[TCA_FLOWER_KEY_CFM_MD_LEVEL])
> +		return 0;
> +
> +	level = nla_get_u8(tb[TCA_FLOWER_KEY_CFM_MD_LEVEL]);
> +	key->cfm.mdl_ver = FIELD_PREP(FLOW_DIS_CFM_MDL_MASK, level);
> +	mask->cfm.mdl_ver = FLOW_DIS_CFM_MDL_MASK;
> +
> +	return 0;
> +}

This function always returns '0' so it can be changed to return 'void'
instead of 'int' like fl_set_key_cfm_opcode().

> +
> +static void fl_set_key_cfm_opcode(struct nlattr **tb,
> +				  struct fl_flow_key *key,
> +				  struct fl_flow_key *mask,
> +				  struct netlink_ext_ack *extack)
> +{
> +	fl_set_key_val(tb, &key->cfm.opcode, TCA_FLOWER_KEY_CFM_OPCODE,
> +		       &mask->cfm.opcode, TCA_FLOWER_UNSPEC,
> +		       sizeof(key->cfm.opcode));
> +}
> +
> +static int fl_set_key_cfm(struct nlattr **tb,
> +			  struct fl_flow_key *key,
> +			  struct fl_flow_key *mask,
> +			  struct netlink_ext_ack *extack)
> +{
> +	struct nlattr *nla_cfm_opt[TCA_FLOWER_KEY_CFM_OPT_MAX];
> +	int err;
> +
> +	if (!tb[TCA_FLOWER_KEY_CFM])
> +		return 0;
> +
> +	err = nla_parse_nested(nla_cfm_opt, TCA_FLOWER_KEY_CFM_OPT_MAX,
> +			       tb[TCA_FLOWER_KEY_CFM], cfm_opt_policy, extack);
> +	if (err < 0)
> +		return err;
> +
> +	fl_set_key_cfm_opcode(nla_cfm_opt, key, mask, extack);
> +
> +	return fl_set_key_cfm_md_level(nla_cfm_opt, key, mask, extack);

And here simply do 'return 0;'.

> +}