[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b501f628-695a-488e-8581-fa28f4c20923@intel.com>
Date: Wed, 3 Jul 2024 12:59:54 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Asbjørn Sloth Tønnesen <ast@...erby.net>
CC: <netdev@...r.kernel.org>, Davide Caratti <dcaratti@...hat.com>, "Ilya
Maximets" <i.maximets@....org>, Jamal Hadi Salim <jhs@...atatu.com>, Cong
Wang <xiyou.wangcong@...il.com>, Jiri Pirko <jiri@...nulli.us>, "David S.
Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, David Ahern
<dsahern@...nel.org>, Simon Horman <horms@...nel.org>, Ratheesh Kannoth
<rkannoth@...vell.com>, Florian Westphal <fw@...len.de>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 1/9] net/sched: flower: define new tunnel flags
From: Asbjørn Sloth Tønnesen <ast@...erby.net>
Date: Wed, 3 Jul 2024 10:45:50 +0000
> Define new TCA_FLOWER_KEY_FLAGS_* flags for use in struct
> flow_dissector_key_control, covering the same flags
> as currently exposed through TCA_FLOWER_KEY_ENC_FLAGS,
> but assign them new bit positions in so that they don't
> conflict with existing TCA_FLOWER_KEY_FLAGS_* flags.
>
> Synchronize FLOW_DIS_* flags, but put the new flags
> under FLOW_DIS_F_*. The idea is that we can later, move
> the existing flags under FLOW_DIS_F_* as well.
>
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@...erby.net>
> ---
> include/net/flow_dissector.h | 17 +++++++++++++----
> include/uapi/linux/pkt_cls.h | 5 +++++
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
> index 3e47e123934d..f560e2c8d0e7 100644
> --- a/include/net/flow_dissector.h
> +++ b/include/net/flow_dissector.h
> @@ -16,7 +16,8 @@ struct sk_buff;
> * struct flow_dissector_key_control:
> * @thoff: Transport header offset
> * @addr_type: Type of key. One of FLOW_DISSECTOR_KEY_*
> - * @flags: Key flags. Any of FLOW_DIS_(IS_FRAGMENT|FIRST_FRAGENCAPSULATION)
> + * @flags: Key flags.
> + * Any of FLOW_DIS_(IS_FRAGMENT|FIRST_FRAG|ENCAPSULATION|F_*)
> */
> struct flow_dissector_key_control {
> u16 thoff;
> @@ -24,9 +25,17 @@ struct flow_dissector_key_control {
> u32 flags;
> };
>
> -#define FLOW_DIS_IS_FRAGMENT BIT(0)
> -#define FLOW_DIS_FIRST_FRAG BIT(1)
> -#define FLOW_DIS_ENCAPSULATION BIT(2)
> +/* Please keep these flags in sync with TCA_FLOWER_KEY_FLAGS_*
> + * in include/uapi/linux/pkt_cls.h, as these bit flags are exposed
> + * to userspace in some error paths, ie. unsupported flags.
> + */
> +#define FLOW_DIS_IS_FRAGMENT BIT(0)
> +#define FLOW_DIS_FIRST_FRAG BIT(1)
> +#define FLOW_DIS_ENCAPSULATION BIT(2)
> +#define FLOW_DIS_F_TUNNEL_CSUM BIT(3)
> +#define FLOW_DIS_F_TUNNEL_DONT_FRAGMENT BIT(4)
> +#define FLOW_DIS_F_TUNNEL_OAM BIT(5)
> +#define FLOW_DIS_F_TUNNEL_CRIT_OPT BIT(6)
1. enum? Then they will be included in BTF info, maybe they might come
handy later in BPF...
2. Maybe "sync" them automatically?
FLOW_DIS_FIRST_FRAG = TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST,
...
(with the only exception for BIT(2))
>
> enum flow_dissect_ret {
> FLOW_DISSECT_RET_OUT_GOOD,
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index b6d38f5fd7c0..24795aad7651 100644
> --- a/include/uapi/linux/pkt_cls.h
> +++ b/include/uapi/linux/pkt_cls.h
> @@ -677,6 +677,11 @@ enum {
> enum {
> TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT = (1 << 0),
> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1),
> + /* FLOW_DIS_ENCAPSULATION (1 << 2) is not exposed to userspace */
Should uAPI header contain this comment? FLOW_DIS_ENCAPSULATION is an
internal kernel definition, so I believe its name shouldn't leak to the
userspace header.
> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM = (1 << 3),
> + TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT = (1 << 4),
> + TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM = (1 << 5),
> + TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT = (1 << 6),
> };
>
> enum {
Thanks,
Olek
Powered by blists - more mailing lists