* [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata
@ 2024-06-27 16:55 Davide Caratti
2024-06-27 17:05 ` Stephen Hemminger
2024-06-28 9:46 ` Asbjørn Sloth Tønnesen
0 siblings, 2 replies; 3+ messages in thread
From: Davide Caratti @ 2024-06-27 16:55 UTC (permalink / raw)
To: Asbjørn Sloth Tønnesen
Cc: David Ahern, aclaudi, Ilya Maximets, echaudro, netdev,
Jamal Hadi Salim
extend TC flower for matching on tunnel metadata.
smoke test:
# ip link add name myep type dummy
# ip link set dev myep up
# ip address add dev myep 192.0.2.1/24
# ip neigh add dev myep 192.0.2.2 lladdr 00:c1:a0:c1:a0:00 nud permanent
# ip link add name mytun type vxlan external
# ip link set dev mytun up
# tc qdisc add dev mytun clsact
# tc filter add dev mytun egress protocol ip pref 1 handle 1 matchall action tunnel_key \
> set src_ip 192.0.2.1 dst_ip 192.0.2.2 id 42 csum nofrag continue index 1
# tc filter add dev mytun egress protocol ip pref 2 handle 2 flower action continue index 30
# tc filter add dev mytun egress protocol ip pref 3 handle 3 flower enc_src_ip 192.0.2.1 action continue index 30
# tc filter add dev mytun egress protocol ip pref 4 handle 4 flower enc_flags tundf action pipe index 100
# mausezahn mytun -c 1 -p 100 -a 00:aa:bb:cc:dd:ee -b 00:ee:dd:cc:bb:aa -t icmp -q
# expect 2 packets below
# tc -s action get action gact index 30
# expect 1 packet below
# tc -s action get action gact index 100
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/uapi/linux/pkt_cls.h | 8 +++++++
tc/f_flower.c | 42 ++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 229fc925ec3a..24795aad7651 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,
};
@@ -674,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 */
+ 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 {
diff --git a/tc/f_flower.c b/tc/f_flower.c
index 08c1001af7b4..45fc31dc380f 100644
--- a/tc/f_flower.c
+++ b/tc/f_flower.c
@@ -17,6 +17,7 @@
#include <linux/tc_act/tc_vlan.h>
#include <linux/mpls.h>
#include <linux/ppp_defs.h>
+#include <linux/if_tunnel.h>
#include "utils.h"
#include "tc_util.h"
@@ -28,6 +29,7 @@
enum flower_matching_flags {
FLOWER_IP_FLAGS,
+ FLOWER_ENC_DST_FLAGS,
};
enum flower_endpoint {
@@ -92,6 +94,7 @@ static void explain(void)
" erspan_opts MASKED-OPTIONS |\n"
" gtp_opts MASKED-OPTIONS |\n"
" pfcp_opts MASKED-OPTIONS |\n"
+ " enc_flags ENC-FLAGS |\n"
" ip_flags IP-FLAGS |\n"
" l2_miss L2_MISS |\n"
" enc_dst_port [ port_number ] |\n"
@@ -205,6 +208,11 @@ struct flag_to_string {
static struct flag_to_string flags_str[] = {
{ TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
{ TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST, FLOWER_IP_FLAGS, "firstfrag" },
+ { TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM, FLOWER_ENC_DST_FLAGS, "csum" },
+ { TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT, FLOWER_ENC_DST_FLAGS, "tundf" },
+ { TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM, FLOWER_ENC_DST_FLAGS, "oam" },
+ { TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT, FLOWER_ENC_DST_FLAGS, "crit" },
+
};
static int flower_parse_matching_flags(char *str,
@@ -1461,6 +1469,29 @@ static int flower_parse_enc_opts_pfcp(char *str, struct nlmsghdr *n)
return 0;
}
+static int flower_parse_enc_dstflags(char *str, struct nlmsghdr *n)
+{
+
+ __u32 dst_flags, dst_flags_mask;
+ int err;
+
+ err = flower_parse_matching_flags(str,
+ FLOWER_ENC_DST_FLAGS,
+ &dst_flags,
+ &dst_flags_mask);
+
+ if (err < 0 || !dst_flags_mask)
+ return -1;
+ err = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_ENC_FLAGS, htonl(dst_flags));
+ if (err < 0)
+ return -1;
+ err = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_ENC_FLAGS_MASK, htonl(dst_flags_mask));
+ if (err < 0)
+ return -1;
+
+ return 0;
+}
+
static int flower_parse_mpls_lse(int *argc_p, char ***argv_p,
struct nlmsghdr *nlh)
{
@@ -2248,6 +2279,13 @@ static int flower_parse_opt(const struct filter_util *qu, char *handle,
fprintf(stderr, "Illegal \"pfcp_opts\"\n");
return -1;
}
+ } else if (matches(*argv, "enc_flags") == 0) {
+ NEXT_ARG();
+ ret = flower_parse_enc_dstflags(*argv, n);
+ if (ret < 0) {
+ fprintf(stderr, "Illegal \"enc_flags\"\n");
+ return -1;
+ }
} else if (matches(*argv, "action") == 0) {
NEXT_ARG();
ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
@@ -3262,6 +3300,10 @@ static int flower_print_opt(const struct filter_util *qu, FILE *f,
tb[TCA_FLOWER_KEY_FLAGS],
tb[TCA_FLOWER_KEY_FLAGS_MASK]);
+ flower_print_matching_flags("enc_flags", FLOWER_ENC_DST_FLAGS,
+ tb[TCA_FLOWER_KEY_ENC_FLAGS],
+ tb[TCA_FLOWER_KEY_ENC_FLAGS_MASK]);
+
if (tb[TCA_FLOWER_L2_MISS]) {
struct rtattr *attr = tb[TCA_FLOWER_L2_MISS];
--
2.45.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata
2024-06-27 16:55 [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata Davide Caratti
@ 2024-06-27 17:05 ` Stephen Hemminger
2024-06-28 9:46 ` Asbjørn Sloth Tønnesen
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Hemminger @ 2024-06-27 17:05 UTC (permalink / raw)
To: Davide Caratti
Cc: Asbjørn Sloth Tønnesen, David Ahern, aclaudi,
Ilya Maximets, echaudro, netdev, Jamal Hadi Salim
On Thu, 27 Jun 2024 18:55:47 +0200
Davide Caratti <dcaratti@redhat.com> wrote:
> + } else if (matches(*argv, "enc_flags") == 0) {
> + NEXT_ARG();
> + ret = flower_parse_enc_dstflags(*argv, n);
> + if (ret < 0) {
> + fprintf(stderr, "Illegal \"enc_flags\"\n");
> + return -1;
> + }
No new uses of matches since it leads to abbreviation conflicts.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata
2024-06-27 16:55 [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata Davide Caratti
2024-06-27 17:05 ` Stephen Hemminger
@ 2024-06-28 9:46 ` Asbjørn Sloth Tønnesen
1 sibling, 0 replies; 3+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2024-06-28 9:46 UTC (permalink / raw)
To: Davide Caratti
Cc: David Ahern, aclaudi, Ilya Maximets, echaudro, netdev,
Jamal Hadi Salim
Hi Davide,
Thank you for the patch.
Overall I think it looks good, I only have a few comments.
On 6/27/24 4:55 PM, Davide Caratti wrote:
> extend TC flower for matching on tunnel metadata.
> smoke test:
>
> # ip link add name myep type dummy
> # ip link set dev myep up
> # ip address add dev myep 192.0.2.1/24
> # ip neigh add dev myep 192.0.2.2 lladdr 00:c1:a0:c1:a0:00 nud permanent
> # ip link add name mytun type vxlan external
> # ip link set dev mytun up
> # tc qdisc add dev mytun clsact
> # tc filter add dev mytun egress protocol ip pref 1 handle 1 matchall action tunnel_key \
> > set src_ip 192.0.2.1 dst_ip 192.0.2.2 id 42 csum nofrag continue index 1
> # tc filter add dev mytun egress protocol ip pref 2 handle 2 flower action continue index 30
> # tc filter add dev mytun egress protocol ip pref 3 handle 3 flower enc_src_ip 192.0.2.1 action continue index 30
> # tc filter add dev mytun egress protocol ip pref 4 handle 4 flower enc_flags tundf action pipe index 100
> # mausezahn mytun -c 1 -p 100 -a 00:aa:bb:cc:dd:ee -b 00:ee:dd:cc:bb:aa -t icmp -q
> # expect 2 packets below
> # tc -s action get action gact index 30
> # expect 1 packet below
> # tc -s action get action gact index 100
>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> include/uapi/linux/pkt_cls.h | 8 +++++++
> tc/f_flower.c | 42 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+)
>
> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
> index 229fc925ec3a..24795aad7651 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 */
> +
FYI: I will do a v1 of the kernel side soon, where the comments
above will change to be32.
> __TCA_FLOWER_MAX,
> };
>
> @@ -674,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 */
> + 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 {
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index 08c1001af7b4..45fc31dc380f 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -17,6 +17,7 @@
> #include <linux/tc_act/tc_vlan.h>
> #include <linux/mpls.h>
> #include <linux/ppp_defs.h>
> +#include <linux/if_tunnel.h>
>
> #include "utils.h"
> #include "tc_util.h"
> @@ -28,6 +29,7 @@
>
> enum flower_matching_flags {
> FLOWER_IP_FLAGS,
> + FLOWER_ENC_DST_FLAGS,
> };
>
> enum flower_endpoint {
> @@ -92,6 +94,7 @@ static void explain(void)
> " erspan_opts MASKED-OPTIONS |\n"
> " gtp_opts MASKED-OPTIONS |\n"
> " pfcp_opts MASKED-OPTIONS |\n"
> + " enc_flags ENC-FLAGS |\n"
Maybe add a "ENC-FLAGS := foo,bar,..." below, could be generated from flag_to_string[],
properly best to do separately as it requires moving the flag_to_string[] definition.
> " ip_flags IP-FLAGS |\n"
IP-FLAGS is also not defined, properly a good time to fix that.
> " l2_miss L2_MISS |\n"
> " enc_dst_port [ port_number ] |\n"
> @@ -205,6 +208,11 @@ struct flag_to_string {
> static struct flag_to_string flags_str[] = {
> { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
> { TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST, FLOWER_IP_FLAGS, "firstfrag" },
> + { TCA_FLOWER_KEY_FLAGS_TUNNEL_CSUM, FLOWER_ENC_DST_FLAGS, "csum" },
> + { TCA_FLOWER_KEY_FLAGS_TUNNEL_DONT_FRAGMENT, FLOWER_ENC_DST_FLAGS, "tundf" },
> + { TCA_FLOWER_KEY_FLAGS_TUNNEL_OAM, FLOWER_ENC_DST_FLAGS, "oam" },
> + { TCA_FLOWER_KEY_FLAGS_TUNNEL_CRIT_OPT, FLOWER_ENC_DST_FLAGS, "crit" },
> +
Nit: I would prefix all of these with "tun_".
> };
>
> static int flower_parse_matching_flags(char *str,
> @@ -1461,6 +1469,29 @@ static int flower_parse_enc_opts_pfcp(char *str, struct nlmsghdr *n)
> return 0;
> }
>
> +static int flower_parse_enc_dstflags(char *str, struct nlmsghdr *n)
> +{
> +
> + __u32 dst_flags, dst_flags_mask;
> + int err;
> +
> + err = flower_parse_matching_flags(str,
> + FLOWER_ENC_DST_FLAGS,
> + &dst_flags,
> + &dst_flags_mask);
> +
> + if (err < 0 || !dst_flags_mask)
> + return -1;
> + err = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_ENC_FLAGS, htonl(dst_flags));
> + if (err < 0)
> + return -1;
> + err = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_ENC_FLAGS_MASK, htonl(dst_flags_mask));
> + if (err < 0)
> + return -1;
> +
> + return 0;
> +}
> +
> static int flower_parse_mpls_lse(int *argc_p, char ***argv_p,
> struct nlmsghdr *nlh)
> {
> @@ -2248,6 +2279,13 @@ static int flower_parse_opt(const struct filter_util *qu, char *handle,
> fprintf(stderr, "Illegal \"pfcp_opts\"\n");
> return -1;
> }
> + } else if (matches(*argv, "enc_flags") == 0) {
> + NEXT_ARG();
> + ret = flower_parse_enc_dstflags(*argv, n);
> + if (ret < 0) {
> + fprintf(stderr, "Illegal \"enc_flags\"\n");
> + return -1;
> + }
I agree that flower_parse_opt() is a bit crowded, but splitting it out to it's own function like this
implicitly also means that you can't specify enc_flags over multiple enc_flags argument, as addattr32()
is called once per enc_flags argument, instead of after all the arguments have been handled.
[..] flower ip_flags frag ip_flags firstfrag [..] is valid, and is equivalent to
[..] flower ip_flags frag/firstfrag [..].
IMHO I would completely mirror the way that ip_flags are handled, except the call to matches(),
so as to avoid adding any new quirks.
> } else if (matches(*argv, "action") == 0) {
> NEXT_ARG();
> ret = parse_action(&argc, &argv, TCA_FLOWER_ACT, n);
> @@ -3262,6 +3300,10 @@ static int flower_print_opt(const struct filter_util *qu, FILE *f,
> tb[TCA_FLOWER_KEY_FLAGS],
> tb[TCA_FLOWER_KEY_FLAGS_MASK]);
>
> + flower_print_matching_flags("enc_flags", FLOWER_ENC_DST_FLAGS,
> + tb[TCA_FLOWER_KEY_ENC_FLAGS],
> + tb[TCA_FLOWER_KEY_ENC_FLAGS_MASK]);
> +
> if (tb[TCA_FLOWER_L2_MISS]) {
> struct rtattr *attr = tb[TCA_FLOWER_L2_MISS];
>
--
Best regards
Asbjørn Sloth Tønnesen
Network Engineer
Fiberby - AS42541
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-06-28 9:46 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-27 16:55 [RFC iproute2-next] tc: f_flower: add support for matching on tunnel metadata Davide Caratti
2024-06-27 17:05 ` Stephen Hemminger
2024-06-28 9:46 ` Asbjørn Sloth Tønnesen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).