[PATCH net-next v4 0/3] net: flower: add cfm support

[PATCH net-next v4 0/3] net: flower: add cfm support

[Zahari Doychev]

From: Zahari Doychev <zdoychev@maxlinear.com>

The first patch adds cfm support to the flow dissector.
The second adds the flower classifier support.
The third adds a selftest for the flower cfm functionality.

iproute2 changes will come in follow up patches.

---
v3->v4
 - use correct size in memchr_inv()
 - remove md level range check and use NLA_POLICY_MAX

v2->v3
 - split the flow dissector and flower changes in separate patches
 - use bit field macros
 - copy separately each cfm key field

v1->v2:
 - add missing comments
 - improve cfm packet dissection
 - move defines to header file
 - fix code formatting
 - remove unneeded attribute defines

rfc->v1:
 - add selftest to the makefile TEST_PROGS.

Zahari Doychev (3):
  net: flow_dissector: add support for cfm packets
  net: flower: add support for matching cfm fields
  selftests: net: add tc flower cfm test

 include/net/flow_dissector.h                  |  20 ++
 include/uapi/linux/pkt_cls.h                  |   9 +
 net/core/flow_dissector.c                     |  30 +++
 net/sched/cls_flower.c                        | 103 ++++++++++-
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/tc_flower_cfm.sh | 175 ++++++++++++++++++
 6 files changed, 337 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_flower_cfm.sh

-- 
2.40.0

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

[Zahari Doychev]

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
+ * @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;
+}
+
 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;
+	}
 	default:
 		fdret = FLOW_DISSECT_RET_OUT_BAD;
 		break;
-- 
2.40.0

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

[Zahari Doychev]

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

Signed-off-by: Zahari Doychev <zdoychev@maxlinear.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
---
 include/uapi/linux/pkt_cls.h |   9 +++
 net/sched/cls_flower.c       | 103 ++++++++++++++++++++++++++++++++++-
 2 files changed, 111 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 648a82f32666..8e3f809c9a03 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -594,6 +594,8 @@ enum {
 
 	TCA_FLOWER_KEY_L2TPV3_SID,	/* be32 */
 
+	TCA_FLOWER_KEY_CFM,	/* nested */
+
 	__TCA_FLOWER_MAX,
 };
 
@@ -702,6 +704,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 cc49256d5318..5d77da484a88 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 {
@@ -720,7 +722,7 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
 	[TCA_FLOWER_KEY_PPPOE_SID]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_PPP_PROTO]	= { .type = NLA_U16 },
 	[TCA_FLOWER_KEY_L2TPV3_SID]	= { .type = NLA_U32 },
-
+	[TCA_FLOWER_KEY_CFM]		= { .type = NLA_NESTED },
 };
 
 static const struct nla_policy
@@ -769,6 +771,11 @@ mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
 	[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, 7),
+	[TCA_FLOWER_KEY_CFM_OPCODE]	= { .type = NLA_U8 },
+};
+
 static void fl_set_key_val(struct nlattr **tb,
 			   void *val, int val_type,
 			   void *mask, int mask_type, int len)
@@ -1653,6 +1660,54 @@ static bool is_vlan_key(struct nlattr *tb, __be16 *ethertype,
 	return false;
 }
 
+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;
+}
+
+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);
+}
+
 static int fl_set_key(struct net *net, struct nlattr **tb,
 		      struct fl_flow_key *key, struct fl_flow_key *mask,
 		      struct netlink_ext_ack *extack)
@@ -1803,6 +1858,10 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
 			       TCA_FLOWER_KEY_L2TPV3_SID,
 			       &mask->l2tpv3.session_id, TCA_FLOWER_UNSPEC,
 			       sizeof(key->l2tpv3.session_id));
+	} else if (key->basic.n_proto  == htons(ETH_P_CFM)) {
+		ret = fl_set_key_cfm(tb, key, mask, extack);
+		if (ret)
+			return ret;
 	}
 
 	if (key->basic.ip_proto == IPPROTO_TCP ||
@@ -1985,6 +2044,8 @@ static void fl_init_dissector(struct flow_dissector *dissector,
 			     FLOW_DISSECTOR_KEY_PPPOE, pppoe);
 	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
 			     FLOW_DISSECTOR_KEY_L2TPV3, l2tpv3);
+	FL_KEY_SET_IF_MASKED(mask, keys, cnt,
+			     FLOW_DISSECTOR_KEY_CFM, cfm);
 
 	skb_flow_dissector_init(dissector, keys, cnt);
 }
@@ -3004,6 +3065,43 @@ static int fl_dump_key_ct(struct sk_buff *skb,
 	return -EMSGSIZE;
 }
 
+static int fl_dump_key_cfm(struct sk_buff *skb,
+			   struct flow_dissector_key_cfm *key,
+			   struct flow_dissector_key_cfm *mask)
+{
+	struct nlattr *opts;
+	int err;
+	u8 mdl;
+
+	if (!memchr_inv(mask, 0, sizeof(*mask)))
+		return 0;
+
+	opts = nla_nest_start(skb, TCA_FLOWER_KEY_CFM);
+	if (!opts)
+		return -EMSGSIZE;
+
+	if (FIELD_GET(FLOW_DIS_CFM_MDL_MASK, mask->mdl_ver)) {
+		mdl = FIELD_GET(FLOW_DIS_CFM_MDL_MASK, key->mdl_ver);
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_CFM_MD_LEVEL, mdl);
+		if (err)
+			goto err_cfm_opts;
+	}
+
+	if (mask->opcode) {
+		err = nla_put_u8(skb, TCA_FLOWER_KEY_CFM_OPCODE, key->opcode);
+		if (err)
+			goto err_cfm_opts;
+	}
+
+	nla_nest_end(skb, opts);
+
+	return 0;
+
+err_cfm_opts:
+	nla_nest_cancel(skb, opts);
+	return err;
+}
+
 static int fl_dump_key_options(struct sk_buff *skb, int enc_opt_type,
 			       struct flow_dissector_key_enc_opts *enc_opts)
 {
@@ -3286,6 +3384,9 @@ static int fl_dump_key(struct sk_buff *skb, struct net *net,
 			     sizeof(key->hash.hash)))
 		goto nla_put_failure;
 
+	if (fl_dump_key_cfm(skb, &key->cfm, &mask->cfm))
+		goto nla_put_failure;
+
 	return 0;
 
 nla_put_failure:
-- 
2.40.0

[PATCH net-next v4 3/3] selftests: net: add tc flower cfm test

[Zahari Doychev]

From: Zahari Doychev <zdoychev@maxlinear.com>

New cfm flower test case is added to the net forwarding selfttests.

Signed-off-by: Zahari Doychev <zdoychev@maxlinear.com>
---
 .../testing/selftests/net/forwarding/Makefile |   1 +
 .../selftests/net/forwarding/tc_flower_cfm.sh | 175 ++++++++++++++++++
 2 files changed, 176 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/tc_flower_cfm.sh

diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index a474c60fe348..11fb97a63646 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -83,6 +83,7 @@ TEST_PROGS = bridge_igmp.sh \
 	tc_chains.sh \
 	tc_flower_router.sh \
 	tc_flower.sh \
+	tc_flower_cfm.sh \
 	tc_mpls_l2vpn.sh \
 	tc_police.sh \
 	tc_shblocks.sh \
diff --git a/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh b/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh
new file mode 100755
index 000000000000..0509bc3c9f75
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh
@@ -0,0 +1,175 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="match_cfm_opcode match_cfm_level match_cfm_level_and_opcode"
+NUM_NETIFS=2
+source tc_common.sh
+source lib.sh
+
+tcflags="skip_hw"
+
+h1_create()
+{
+	simple_if_init $h1 192.0.2.1/24 198.51.100.1/24
+}
+
+h1_destroy()
+{
+	simple_if_fini $h1 192.0.2.1/24 198.51.100.1/24
+}
+
+h2_create()
+{
+	simple_if_init $h2 192.0.2.2/24 198.51.100.2/24
+	tc qdisc add dev $h2 clsact
+}
+
+h2_destroy()
+{
+	tc qdisc del dev $h2 clsact
+	simple_if_fini $h2 192.0.2.2/24 198.51.100.2/24
+}
+
+cfm_mdl_opcode()
+{
+	local mdl=$1
+	local op=$2
+	local flags=$3
+	local tlv_offset=$4
+
+	printf "%02x %02x %02x %02x"    \
+		   $((mdl << 5))             \
+		   $((op & 0xff))             \
+		   $((flags & 0xff)) \
+		   $tlv_offset
+}
+
+match_cfm_opcode()
+{
+	local ethtype="89 02"; readonly ethtype
+	RET=0
+
+	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
+	   flower cfm op 47 action drop
+	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
+	   flower cfm op 43 action drop
+
+	pkt="$ethtype $(cfm_mdl_opcode 7 47 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+	pkt="$ethtype $(cfm_mdl_opcode 6 5 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+
+	tc_check_packets "dev $h2 ingress" 101 1
+	check_err $? "Did not match on correct opcode"
+
+	tc_check_packets "dev $h2 ingress" 102 0
+	check_err $? "Matched on the wrong opcode"
+
+	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
+	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
+
+	log_test "CFM opcode match test"
+}
+
+match_cfm_level()
+{
+	local ethtype="89 02"; readonly ethtype
+	RET=0
+
+	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
+	   flower cfm mdl 5 action drop
+	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
+	   flower cfm mdl 3 action drop
+	tc filter add dev $h2 ingress protocol cfm pref 3 handle 103 \
+	   flower cfm mdl 0 action drop
+
+	pkt="$ethtype $(cfm_mdl_opcode 5 42 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+	pkt="$ethtype $(cfm_mdl_opcode 6 1 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+	pkt="$ethtype $(cfm_mdl_opcode 0 1 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+
+	tc_check_packets "dev $h2 ingress" 101 1
+	check_err $? "Did not match on correct level"
+
+	tc_check_packets "dev $h2 ingress" 102 0
+	check_err $? "Matched on the wrong level"
+
+	tc_check_packets "dev $h2 ingress" 103 1
+	check_err $? "Did not match on correct level"
+
+	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
+	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
+	tc filter del dev $h2 ingress protocol cfm pref 3 handle 103 flower
+
+	log_test "CFM level match test"
+}
+
+match_cfm_level_and_opcode()
+{
+	local ethtype="89 02"; readonly ethtype
+	RET=0
+
+	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
+	   flower cfm mdl 5 op 41 action drop
+	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
+	   flower cfm mdl 7 op 42 action drop
+
+	pkt="$ethtype $(cfm_mdl_opcode 5 41 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+	pkt="$ethtype $(cfm_mdl_opcode 7 3 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+	pkt="$ethtype $(cfm_mdl_opcode 3 42 0 4)"
+	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
+
+	tc_check_packets "dev $h2 ingress" 101 1
+	check_err $? "Did not match on correct level and opcode"
+	tc_check_packets "dev $h2 ingress" 102 0
+	check_err $? "Matched on the wrong level and opcode"
+
+	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
+	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
+
+	log_test "CFM opcode and level match test"
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	h2=${NETIFS[p2]}
+	h1mac=$(mac_get $h1)
+	h2mac=$(mac_get $h2)
+
+	vrf_prepare
+
+	h1_create
+	h2_create
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	h2_destroy
+	h1_destroy
+
+	vrf_cleanup
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+tc_offload_check
+if [[ $? -ne 0 ]]; then
+	log_info "Could not test offloaded functionality"
+else
+	tcflags="skip_sw"
+	tests_run
+fi
+
+exit $EXIT_STATUS
-- 
2.40.0

Re: [PATCH net-next v4 0/3] net: flower: add cfm support

[Leon Romanovsky]

On Tue, Apr 25, 2023 at 11:16:27PM +0200, Zahari Doychev wrote:
> From: Zahari Doychev <zdoychev@maxlinear.com>
> 
> The first patch adds cfm support to the flow dissector.
> The second adds the flower classifier support.
> The third adds a selftest for the flower cfm functionality.
> 
> iproute2 changes will come in follow up patches.

## Form letter - net-next-closed

The merge window for v6.3 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations.
We are currently accepting bug fixes only.

Please repost when net-next reopens after May 8th.

RFC patches sent for review only are obviously welcome at any time.

Thanks

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

[Ido Schimmel]

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
> 

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

[Ido Schimmel]

On Tue, Apr 25, 2023 at 11:16:29PM +0200, Zahari Doychev wrote:
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index cc49256d5318..5d77da484a88 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 {
> @@ -720,7 +722,7 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
>  	[TCA_FLOWER_KEY_PPPOE_SID]	= { .type = NLA_U16 },
>  	[TCA_FLOWER_KEY_PPP_PROTO]	= { .type = NLA_U16 },
>  	[TCA_FLOWER_KEY_L2TPV3_SID]	= { .type = NLA_U32 },
> -
> +	[TCA_FLOWER_KEY_CFM]		= { .type = NLA_NESTED },

"fl_policy" is used with nla_parse_nested_deprecated(). You can enable
strict validation for new attributes using the following diff:

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index fc9037685458..6bccfc1722ad 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -615,7 +615,8 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
 }
 
 static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
-       [TCA_FLOWER_UNSPEC]             = { .type = NLA_UNSPEC },
+       [TCA_FLOWER_UNSPEC]             = { .strict_start_type =
+                                               TCA_FLOWER_KEY_CFM },
        [TCA_FLOWER_CLASSID]            = { .type = NLA_U32 },
        [TCA_FLOWER_INDEV]              = { .type = NLA_STRING,
                                            .len = IFNAMSIZ },

>  };
>  
>  static const struct nla_policy
> @@ -769,6 +771,11 @@ mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
>  	[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, 7),

Instead of 7, can you use FIELD_MAX(FLOW_DIS_CFM_MDL_MASK) like you did
in the previous version?

> +	[TCA_FLOWER_KEY_CFM_OPCODE]	= { .type = NLA_U8 },
> +};

Re: [PATCH net-next v4 3/3] selftests: net: add tc flower cfm test

[Ido Schimmel]

On Tue, Apr 25, 2023 at 11:16:30PM +0200, Zahari Doychev wrote:
> From: Zahari Doychev <zdoychev@maxlinear.com>
> 
> New cfm flower test case is added to the net forwarding selfttests.
> 
> Signed-off-by: Zahari Doychev <zdoychev@maxlinear.com>
> ---
>  .../testing/selftests/net/forwarding/Makefile |   1 +
>  .../selftests/net/forwarding/tc_flower_cfm.sh | 175 ++++++++++++++++++
>  2 files changed, 176 insertions(+)
>  create mode 100755 tools/testing/selftests/net/forwarding/tc_flower_cfm.sh
> 
> diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
> index a474c60fe348..11fb97a63646 100644
> --- a/tools/testing/selftests/net/forwarding/Makefile
> +++ b/tools/testing/selftests/net/forwarding/Makefile
> @@ -83,6 +83,7 @@ TEST_PROGS = bridge_igmp.sh \
>  	tc_chains.sh \
>  	tc_flower_router.sh \
>  	tc_flower.sh \
> +	tc_flower_cfm.sh \
>  	tc_mpls_l2vpn.sh \
>  	tc_police.sh \
>  	tc_shblocks.sh \
> diff --git a/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh b/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh
> new file mode 100755
> index 000000000000..0509bc3c9f75
> --- /dev/null
> +++ b/tools/testing/selftests/net/forwarding/tc_flower_cfm.sh
> @@ -0,0 +1,175 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +ALL_TESTS="match_cfm_opcode match_cfm_level match_cfm_level_and_opcode"
> +NUM_NETIFS=2
> +source tc_common.sh
> +source lib.sh
> +
> +tcflags="skip_hw"
> +
> +h1_create()
> +{
> +	simple_if_init $h1 192.0.2.1/24 198.51.100.1/24

The IP address are not used in the test. Can be omitted.

> +}
> +
> +h1_destroy()
> +{
> +	simple_if_fini $h1 192.0.2.1/24 198.51.100.1/24
> +}
> +
> +h2_create()
> +{
> +	simple_if_init $h2 192.0.2.2/24 198.51.100.2/24
> +	tc qdisc add dev $h2 clsact
> +}
> +
> +h2_destroy()
> +{
> +	tc qdisc del dev $h2 clsact
> +	simple_if_fini $h2 192.0.2.2/24 198.51.100.2/24
> +}
> +
> +cfm_mdl_opcode()
> +{
> +	local mdl=$1
> +	local op=$2
> +	local flags=$3
> +	local tlv_offset=$4

If you use something like:

local mdl=$1; shift
local op=$1; shift

Then minimal changes are required if the order changes

> +
> +	printf "%02x %02x %02x %02x"    \
> +		   $((mdl << 5))             \
> +		   $((op & 0xff))             \
> +		   $((flags & 0xff)) \
> +		   $tlv_offset
> +}

See mldv2_is_in_get() in tools/testing/selftests/net/forwarding/lib.sh
and related functions for a more readable way to achieve the above.

> +
> +match_cfm_opcode()
> +{
> +	local ethtype="89 02"; readonly ethtype
> +	RET=0
> +
> +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> +	   flower cfm op 47 action drop
> +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> +	   flower cfm op 43 action drop

Both filters can use the same preference since the same mask is used.

> +
> +	pkt="$ethtype $(cfm_mdl_opcode 7 47 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +	pkt="$ethtype $(cfm_mdl_opcode 6 5 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +
> +	tc_check_packets "dev $h2 ingress" 101 1
> +	check_err $? "Did not match on correct opcode"
> +
> +	tc_check_packets "dev $h2 ingress" 102 0
> +	check_err $? "Matched on the wrong opcode"

For good measures you can send a packet with opcode 43 and check that
only 102 is hit.

> +
> +	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
> +	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
> +
> +	log_test "CFM opcode match test"
> +}
> +
> +match_cfm_level()
> +{
> +	local ethtype="89 02"; readonly ethtype
> +	RET=0
> +
> +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> +	   flower cfm mdl 5 action drop
> +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> +	   flower cfm mdl 3 action drop
> +	tc filter add dev $h2 ingress protocol cfm pref 3 handle 103 \
> +	   flower cfm mdl 0 action drop

Same comment about the preference.

> +
> +	pkt="$ethtype $(cfm_mdl_opcode 5 42 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +	pkt="$ethtype $(cfm_mdl_opcode 6 1 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +	pkt="$ethtype $(cfm_mdl_opcode 0 1 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +
> +	tc_check_packets "dev $h2 ingress" 101 1
> +	check_err $? "Did not match on correct level"
> +
> +	tc_check_packets "dev $h2 ingress" 102 0
> +	check_err $? "Matched on the wrong level"
> +
> +	tc_check_packets "dev $h2 ingress" 103 1
> +	check_err $? "Did not match on correct level"
> +
> +	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
> +	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
> +	tc filter del dev $h2 ingress protocol cfm pref 3 handle 103 flower
> +
> +	log_test "CFM level match test"
> +}
> +
> +match_cfm_level_and_opcode()
> +{
> +	local ethtype="89 02"; readonly ethtype
> +	RET=0
> +
> +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> +	   flower cfm mdl 5 op 41 action drop
> +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> +	   flower cfm mdl 7 op 42 action drop

Likewise

> +
> +	pkt="$ethtype $(cfm_mdl_opcode 5 41 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +	pkt="$ethtype $(cfm_mdl_opcode 7 3 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +	pkt="$ethtype $(cfm_mdl_opcode 3 42 0 4)"
> +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> +
> +	tc_check_packets "dev $h2 ingress" 101 1
> +	check_err $? "Did not match on correct level and opcode"
> +	tc_check_packets "dev $h2 ingress" 102 0
> +	check_err $? "Matched on the wrong level and opcode"
> +
> +	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
> +	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
> +
> +	log_test "CFM opcode and level match test"
> +}
> +
> +setup_prepare()
> +{
> +	h1=${NETIFS[p1]}
> +	h2=${NETIFS[p2]}
> +	h1mac=$(mac_get $h1)
> +	h2mac=$(mac_get $h2)
> +
> +	vrf_prepare
> +
> +	h1_create
> +	h2_create
> +}
> +
> +cleanup()
> +{
> +	pre_cleanup
> +
> +	h2_destroy
> +	h1_destroy
> +
> +	vrf_cleanup
> +}
> +
> +trap cleanup EXIT
> +
> +setup_prepare
> +setup_wait
> +
> +tests_run
> +
> +tc_offload_check
> +if [[ $? -ne 0 ]]; then
> +	log_info "Could not test offloaded functionality"
> +else
> +	tcflags="skip_sw"
> +	tests_run
> +fi
> +
> +exit $EXIT_STATUS
> -- 
> 2.40.0
> 

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

[Zahari Doychev]

[...] 
> >  
> > +/**
> > + * 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

[...]

 > +	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?
> 

thanks, I will fix them for the next series.

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

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

[Zahari Doychev]

On Sun, Apr 30, 2023 at 05:49:57PM +0300, Ido Schimmel wrote:
> On Tue, Apr 25, 2023 at 11:16:29PM +0200, Zahari Doychev wrote:
> > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> > index cc49256d5318..5d77da484a88 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 {
> > @@ -720,7 +722,7 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> >  	[TCA_FLOWER_KEY_PPPOE_SID]	= { .type = NLA_U16 },
> >  	[TCA_FLOWER_KEY_PPP_PROTO]	= { .type = NLA_U16 },
> >  	[TCA_FLOWER_KEY_L2TPV3_SID]	= { .type = NLA_U32 },
> > -
> > +	[TCA_FLOWER_KEY_CFM]		= { .type = NLA_NESTED },
> 
> "fl_policy" is used with nla_parse_nested_deprecated(). You can enable
> strict validation for new attributes using the following diff:
> 
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index fc9037685458..6bccfc1722ad 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -615,7 +615,8 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
>  }
>  
>  static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> -       [TCA_FLOWER_UNSPEC]             = { .type = NLA_UNSPEC },
> +       [TCA_FLOWER_UNSPEC]             = { .strict_start_type =
> +                                               TCA_FLOWER_KEY_CFM },
>         [TCA_FLOWER_CLASSID]            = { .type = NLA_U32 },
>         [TCA_FLOWER_INDEV]              = { .type = NLA_STRING,
>                                             .len = IFNAMSIZ },
> 
> >  };

thanks, noted.

> >  
> >  static const struct nla_policy
> > @@ -769,6 +771,11 @@ mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
> >  	[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, 7),
> 
> Instead of 7, can you use FIELD_MAX(FLOW_DIS_CFM_MDL_MASK) like you did
> in the previous version?
> 

It seems that the macro can be use only inside functions. I wanted to use it
but I was getting the following error:

linux/include/linux/bitfield.h:86:9: error: braced-group within expression allowed only inside a function

thanks
Zahari

> > +	[TCA_FLOWER_KEY_CFM_OPCODE]	= { .type = NLA_U8 },
> > +};

Re: [PATCH net-next v4 3/3] selftests: net: add tc flower cfm test

[Zahari Doychev]

> > +tcflags="skip_hw"
> > +

[...]

> > +h1_create()
> > +{
> > +	simple_if_init $h1 192.0.2.1/24 198.51.100.1/24
> 
> The IP address are not used in the test. Can be omitted.
> 
> > +}
> > +
> > +h1_destroy()
> > +{
> > +	simple_if_fini $h1 192.0.2.1/24 198.51.100.1/24
> > +}
> > +
> > +h2_create()
> > +{
> > +	simple_if_init $h2 192.0.2.2/24 198.51.100.2/24
> > +	tc qdisc add dev $h2 clsact
> > +}
> > +
> > +h2_destroy()
> > +{
> > +	tc qdisc del dev $h2 clsact
> > +	simple_if_fini $h2 192.0.2.2/24 198.51.100.2/24
> > +}
> > +
> > +cfm_mdl_opcode()
> > +{
> > +	local mdl=$1
> > +	local op=$2
> > +	local flags=$3
> > +	local tlv_offset=$4
> 
> If you use something like:
> 
> local mdl=$1; shift
> local op=$1; shift
> 
> Then minimal changes are required if the order changes
> 
> > +
> > +	printf "%02x %02x %02x %02x"    \
> > +		   $((mdl << 5))             \
> > +		   $((op & 0xff))             \
> > +		   $((flags & 0xff)) \
> > +		   $tlv_offset
> > +}
> 
> See mldv2_is_in_get() in tools/testing/selftests/net/forwarding/lib.sh
> and related functions for a more readable way to achieve the above.
> 
> > +
> > +match_cfm_opcode()
> > +{
> > +	local ethtype="89 02"; readonly ethtype
> > +	RET=0
> > +
> > +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> > +	   flower cfm op 47 action drop
> > +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> > +	   flower cfm op 43 action drop
> 
> Both filters can use the same preference since the same mask is used.
> 
> > +
> > +	pkt="$ethtype $(cfm_mdl_opcode 7 47 0 4)"
> > +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> > +	pkt="$ethtype $(cfm_mdl_opcode 6 5 0 4)"
> > +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> > +
> > +	tc_check_packets "dev $h2 ingress" 101 1
> > +	check_err $? "Did not match on correct opcode"
> > +
> > +	tc_check_packets "dev $h2 ingress" 102 0
> > +	check_err $? "Matched on the wrong opcode"
> 
> For good measures you can send a packet with opcode 43 and check that
> only 102 is hit.
> 
> > +
> > +	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
> > +	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
> > +
> > +	log_test "CFM opcode match test"
> > +}
> > +
> > +match_cfm_level()
> > +{
> > +	local ethtype="89 02"; readonly ethtype
> > +	RET=0
> > +
> > +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> > +	   flower cfm mdl 5 action drop
> > +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> > +	   flower cfm mdl 3 action drop
> > +	tc filter add dev $h2 ingress protocol cfm pref 3 handle 103 \
> > +	   flower cfm mdl 0 action drop
> 
> Same comment about the preference.
> 
> > +
> > +	pkt="$ethtype $(cfm_mdl_opcode 5 42 0 4)"
> > +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> > +	pkt="$ethtype $(cfm_mdl_opcode 6 1 0 4)"
> > +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> > +	pkt="$ethtype $(cfm_mdl_opcode 0 1 0 4)"
> > +	$MZ $h1 -c 1 -p 64 -a $h1mac -b $h2mac "$pkt" -q
> > +
> > +	tc_check_packets "dev $h2 ingress" 101 1
> > +	check_err $? "Did not match on correct level"
> > +
> > +	tc_check_packets "dev $h2 ingress" 102 0
> > +	check_err $? "Matched on the wrong level"
> > +
> > +	tc_check_packets "dev $h2 ingress" 103 1
> > +	check_err $? "Did not match on correct level"
> > +
> > +	tc filter del dev $h2 ingress protocol cfm pref 1 handle 101 flower
> > +	tc filter del dev $h2 ingress protocol cfm pref 2 handle 102 flower
> > +	tc filter del dev $h2 ingress protocol cfm pref 3 handle 103 flower
> > +
> > +	log_test "CFM level match test"
> > +}
> > +
> > +match_cfm_level_and_opcode()
> > +{
> > +	local ethtype="89 02"; readonly ethtype
> > +	RET=0
> > +
> > +	tc filter add dev $h2 ingress protocol cfm pref 1 handle 101 \
> > +	   flower cfm mdl 5 op 41 action drop
> > +	tc filter add dev $h2 ingress protocol cfm pref 2 handle 102 \
> > +	   flower cfm mdl 7 op 42 action drop
> 
> Likewise
> 
> > +

I will handle your comments in the next version.

thanks,
Zahari

[...]

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

[Ido Schimmel]

On Sun, Apr 30, 2023 at 06:35:13PM +0200, Zahari Doychev wrote:
> On Sun, Apr 30, 2023 at 05:49:57PM +0300, Ido Schimmel wrote:
> > On Tue, Apr 25, 2023 at 11:16:29PM +0200, Zahari Doychev wrote:
> > > +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, 7),
> > 
> > Instead of 7, can you use FIELD_MAX(FLOW_DIS_CFM_MDL_MASK) like you did
> > in the previous version?
> > 
> 
> It seems that the macro can be use only inside functions. I wanted to use it
> but I was getting the following error:
> 
> linux/include/linux/bitfield.h:86:9: error: braced-group within expression allowed only inside a function

I see. Another option that I personally find better than hard-coding 7
is the below:

diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index 479b66b11d2d..52f30906b210 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -317,6 +317,7 @@ struct flow_dissector_key_cfm {
 };
 
 #define FLOW_DIS_CFM_MDL_MASK GENMASK(7, 5)
+#define FLOW_DIS_CFM_MDL_MAX 7
 
 enum flow_dissector_key_id {
        FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 5d77da484a88..85fc77063866 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -772,7 +772,8 @@ mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
 };
 
 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, 7),
+       [TCA_FLOWER_KEY_CFM_MD_LEVEL]   = NLA_POLICY_MAX(NLA_U8,
+                                                        FLOW_DIS_CFM_MDL_MAX),
        [TCA_FLOWER_KEY_CFM_OPCODE]     = { .type = NLA_U8 },
 };

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

[Zahari Doychev]

On Mon, May 01, 2023 at 09:56:15AM +0300, Ido Schimmel wrote:
> On Sun, Apr 30, 2023 at 06:35:13PM +0200, Zahari Doychev wrote:
> > On Sun, Apr 30, 2023 at 05:49:57PM +0300, Ido Schimmel wrote:
> > > On Tue, Apr 25, 2023 at 11:16:29PM +0200, Zahari Doychev wrote:
> > > > +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, 7),
> > > 
> > > Instead of 7, can you use FIELD_MAX(FLOW_DIS_CFM_MDL_MASK) like you did
> > > in the previous version?
> > > 
> > 
> > It seems that the macro can be use only inside functions. I wanted to use it
> > but I was getting the following error:
> > 
> > linux/include/linux/bitfield.h:86:9: error: braced-group within expression allowed only inside a function
> 
> I see. Another option that I personally find better than hard-coding 7
> is the below:

I was thinking about the same. I will change it in the next version.

Thanks,
Zahari

> 
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index 479b66b11d2d..52f30906b210 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -317,6 +317,7 @@ struct flow_dissector_key_cfm {
>  };
>  
>  #define FLOW_DIS_CFM_MDL_MASK GENMASK(7, 5)
> +#define FLOW_DIS_CFM_MDL_MAX 7
>  
>  enum flow_dissector_key_id {
>         FLOW_DISSECTOR_KEY_CONTROL, /* struct flow_dissector_key_control */
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 5d77da484a88..85fc77063866 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -772,7 +772,8 @@ mpls_stack_entry_policy[TCA_FLOWER_KEY_MPLS_OPT_LSE_MAX + 1] = {
>  };
>  
>  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, 7),
> +       [TCA_FLOWER_KEY_CFM_MD_LEVEL]   = NLA_POLICY_MAX(NLA_U8,
> +                                                        FLOW_DIS_CFM_MDL_MAX),
>         [TCA_FLOWER_KEY_CFM_OPCODE]     = { .type = NLA_U8 },
>  };