[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d1e494b5-c537-4faa-8226-892718b736aa@fiberby.net>
Date: Wed, 5 Jun 2024 07:37:13 +0000
From: Asbjørn Sloth Tønnesen <ast@...erby.net>
To: Davide Caratti <dcaratti@...hat.com>, i.maximets@....org
Cc: davem@...emloft.net, edumazet@...gle.com, jhs@...atatu.com,
jiri@...nulli.us, kuba@...nel.org, lucien.xin@...il.com,
marcelo.leitner@...il.com, netdev@...r.kernel.org, pabeni@...hat.com,
xiyou.wangcong@...il.com, echaudro@...hat.com,
Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net-next v4 2/2] net/sched: cls_flower: add support for
matching tunnel control flags
Hi Davide and Ilva,
On 5/30/24 5:08 PM, Davide Caratti wrote:
> extend cls_flower to match TUNNEL_FLAGS_PRESENT bits in tunnel metadata.
>
> Suggested-by: Ilya Maximets <i.maximets@....org>
> Acked-by: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Davide Caratti <dcaratti@...hat.com>
> ---
> 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
Powered by blists - more mailing lists