Thread (8 messages) 8 messages, 4 authors, 2024-06-06

Re: [PATCH net-next v4 2/2] net/sched: cls_flower: add support for matching tunnel control flags

From: Asbjørn Sloth Tønnesen <hidden>
Date: 2024-06-05 07:48:11

Hi Davide and Ilva,

On 5/30/24 5:08 PM, Davide Caratti wrote:
quoted hunk ↗ jump to hunk
extend cls_flower to match TUNNEL_FLAGS_PRESENT bits in tunnel metadata.

Suggested-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Davide Caratti <redacted>
---
  include/uapi/linux/pkt_cls.h |  3 ++
  net/sched/cls_flower.c       | 56 +++++++++++++++++++++++++++++++++++-
  2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 229fc925ec3a..b6d38f5fd7c0 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -554,6 +554,9 @@ enum {
  	TCA_FLOWER_KEY_SPI,		/* be32 */
  	TCA_FLOWER_KEY_SPI_MASK,	/* be32 */
  
+	TCA_FLOWER_KEY_ENC_FLAGS,	/* u32 */
+	TCA_FLOWER_KEY_ENC_FLAGS_MASK,	/* u32 */
+
  	__TCA_FLOWER_MAX,
  };
  
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index fd9a6f20b60b..eef570c577ac 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -41,6 +41,12 @@
  #define TCA_FLOWER_KEY_CT_FLAGS_MASK \
  		(TCA_FLOWER_KEY_CT_FLAGS_MAX - 1)
  
+#define TUNNEL_FLAGS_PRESENT (\
+	_BITUL(IP_TUNNEL_CSUM_BIT) |		\
+	_BITUL(IP_TUNNEL_DONT_FRAGMENT_BIT) |	\
+	_BITUL(IP_TUNNEL_OAM_BIT) |		\
+	_BITUL(IP_TUNNEL_CRIT_OPT_BIT))
+
  struct fl_flow_key {
  	struct flow_dissector_key_meta meta;
  	struct flow_dissector_key_control control;
@@ -75,6 +81,7 @@ struct fl_flow_key {
  	struct flow_dissector_key_l2tpv3 l2tpv3;
  	struct flow_dissector_key_ipsec ipsec;
  	struct flow_dissector_key_cfm cfm;
+	struct flow_dissector_key_enc_flags enc_flags;
  } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as longs. */
  
  struct fl_flow_mask_range {
@@ -732,6 +739,10 @@ static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
  	[TCA_FLOWER_KEY_SPI_MASK]	= { .type = NLA_U32 },
  	[TCA_FLOWER_L2_MISS]		= NLA_POLICY_MAX(NLA_U8, 1),
  	[TCA_FLOWER_KEY_CFM]		= { .type = NLA_NESTED },
+	[TCA_FLOWER_KEY_ENC_FLAGS]	= NLA_POLICY_MASK(NLA_U32,
+							  TUNNEL_FLAGS_PRESENT),
+`	[TCA_FLOWER_KEY_ENC_FLAGS_MASK]	= NLA_POLICY_MASK(NLA_U32,
+							  TUNNEL_FLAGS_PRESENT),
  };
  
  static const struct nla_policy
@@ -1825,6 +1836,21 @@ static int fl_set_key_cfm(struct nlattr **tb,
  	return 0;
  }
  
+static int fl_set_key_enc_flags(struct nlattr **tb, u32 *flags_key,
+				u32 *flags_mask, struct netlink_ext_ack *extack)
+{
+	/* mask is mandatory for flags */
+	if (NL_REQ_ATTR_CHECK(extack, NULL, tb, TCA_FLOWER_KEY_ENC_FLAGS_MASK)) {
+		NL_SET_ERR_MSG(extack, "missing enc_flags mask");
+		return -EINVAL;
+	}
+
+	*flags_key = nla_get_u32(tb[TCA_FLOWER_KEY_ENC_FLAGS]);
+	*flags_mask = nla_get_u32(tb[TCA_FLOWER_KEY_ENC_FLAGS_MASK]);
+
+	return 0;
+}
+
  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)
@@ -2059,9 +2085,16 @@ static int fl_set_key(struct net *net, struct nlattr **tb,
  	if (ret)
  		return ret;
  
-	if (tb[TCA_FLOWER_KEY_FLAGS])
+	if (tb[TCA_FLOWER_KEY_FLAGS]) {
  		ret = fl_set_key_flags(tb, &key->control.flags,
  				       &mask->control.flags, extack);
+		if (ret)
+			return ret;
+	}
+
+	if (tb[TCA_FLOWER_KEY_ENC_FLAGS])
+		ret = fl_set_key_enc_flags(tb, &key->enc_flags.flags,
+					   &mask->enc_flags.flags, extack);
  
  	return ret;
Sorry, that I am late to the party, I have been away camping at
Electromagnetic Field in the UK.

Why not use the already existing key->enc_control.flags with dissector
key FLOW_DISSECTOR_KEY_ENC_CONTROL for storing the flags?

Currently key->enc_control.flags are unused.

I haven't fixed the drivers to validate that field yet, so currently
only sfc does so.

Look at include/uapi/linux/pkt_cls.h for netlink flags:

enum {
         TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
         TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
};

and at include/net/flow_dissector.h for the dissector flags:

#define FLOW_DIS_IS_FRAGMENT    BIT(0)
#define FLOW_DIS_FIRST_FRAG     BIT(1)
#define FLOW_DIS_ENCAPSULATION  BIT(2)

I would like to keep FLOW_DIS_ENCAPSULATION as the last flag, in order to
keep parity with the netlink flags, since the dissector flags field is
exposed to user space when some flags are not supported.

I realize that since this series is now merged, then fixing this up will
have to go in another series, are you up for that?

-- 
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help