[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <424225aa-532e-8c5e-bece-510e9f648915@mellanox.com>
Date: Thu, 19 Jan 2017 14:19:23 +0200
From: Paul Blakey <paulb@...lanox.com>
To: Stephen Hemminger <stephen@...workplumber.org>,
<netdev@...r.kernel.org>
CC: <paulb@...lanox.com>, Jiri Pirko <jiri@...lanox.com>,
Or Gerlitz <ogerlitz@...lanox.com>,
Roi Dayan <roid@...lanox.com>, Jiri Benc <jbenc@...hat.com>,
Simon Horman <simon.horman@...ronome.com>
Subject: Re: [PATCH iproute2 net-next] tc: flower: Refactor matching flags to
be more user friendly
On 19/01/2017 13:32, Paul Blakey wrote:
> Instead of "magic numbers" we can now specify each flag
> by name. Prefix of "no_" (e.g no_frag) unsets the flag,
> otherwise it wil be set.
>
> Example:
> # add a flower filter that will drop fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags frag \
> action drop
>
> # add a flower filter that will drop non-fragmented packets
> tc filter add dev ens4f0 protocol ip parent ffff: \
> flower \
> src_mac e4:1d:2d:fd:8b:01 \
> dst_mac e4:1d:2d:fd:8b:02 \
> indev ens4f0 \
> ip_flags no_frag \
> action drop
>
> Fixes: 22a8f019891c ('tc: flower: support matching flags')
> Signed-off-by: Paul Blakey <paulb@...lanox.com>
> Reviewed-by: Roi Dayan <roid@...lanox.com>
> ---
>
> Hi,
> Added a framework to add new flags more easily, such
> as the upcoming tcp_flags (see kernel cls_flower), and other ip_flags.
>
> Thanks,
> Paul.
>
>
> man/man8/tc-flower.8 | 12 +++++-
> tc/f_flower.c | 117 ++++++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 102 insertions(+), 27 deletions(-)
>
> diff --git a/man/man8/tc-flower.8 b/man/man8/tc-flower.8
> index 2dd2c5e..77da10b 100644
> --- a/man/man8/tc-flower.8
> +++ b/man/man8/tc-flower.8
> @ -46,7 +46,9 @@ flower \- flow based traffic control filter
> .BR enc_dst_ip " | " enc_src_ip " } { "
> .IR ipv4_address " | " ipv6_address " } | "
> .B enc_dst_port
> -.IR port_number
> +.IR port_number " | "
> +.BR ip_flags
> +.IR IP_FLAGS
> .SH DESCRIPTION
> The
> .B flower
> @@ -183,13 +185,19 @@ prefix length. If the prefix is missing, \fBtc\fR assumes a full-length
> host match. Dst port
> .I NUMBER
> is a 16 bit UDP dst port.
> +.TP
> +.BI ip_flags " IP_FLAGS"
> +.I IP_FLAGS
> +may be either
> +.BR frag " or " no_frag
> +to match on fragmented packets or not respectively.
> .SH NOTES
> As stated above where applicable, matches of a certain layer implicitly depend
> on the matches of the next lower layer. Precisely, layer one and two matches
> (\fBindev\fR, \fBdst_mac\fR and \fBsrc_mac\fR)
> have no dependency, layer three matches
> (\fBip_proto\fR, \fBdst_ip\fR, \fBsrc_ip\fR, \fBarp_tip\fR, \fBarp_sip\fR,
> -\fBarp_op\fR, \fBarp_tha\fR and \fBarp_sha\fR)
> +\fBarp_op\fR, \fBarp_tha\fR, \fBarp_sha\fR and \fBip_flags\fR)
> depend on the
> .B protocol
> option of tc filter, layer four port matches
> diff --git a/tc/f_flower.c b/tc/f_flower.c
> index d301db3..3bafc75 100644
> --- a/tc/f_flower.c
> +++ b/tc/f_flower.c
> @@ -24,6 +24,10 @@
> #include "tc_util.h"
> #include "rt_names.h"
>
> +enum flower_matching_flags {
> + FLOWER_IP_FLAGS,
> +};
> +
> enum flower_endpoint {
> FLOWER_ENDPOINT_SRC,
> FLOWER_ENDPOINT_DST
> @@ -63,7 +67,7 @@ static void explain(void)
> " enc_dst_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_src_ip [ IPV4-ADDR | IPV6-ADDR ] |\n"
> " enc_key_id [ KEY-ID ] |\n"
> - " matching_flags MATCHING-FLAGS | \n"
> + " ip_flags IP-FLAGS | \n"
> " enc_dst_port [ port_number ] }\n"
> " FILTERID := X:Y:Z\n"
> " MASKED_LLADDR := { LLADDR | LLADDR/MASK | LLADDR/BITS }\n"
> @@ -136,28 +140,56 @@ static int flower_parse_vlan_eth_type(char *str, __be16 eth_type, int type,
> return 0;
> }
>
> -static int flower_parse_matching_flags(char *str, int type, int mask_type,
> - struct nlmsghdr *n)
> -{
> - __u32 mtf, mtf_mask;
> - char *c;
> +struct flag_to_string {
> + int flag;
> + enum flower_matching_flags type;
> + char *string;
> +};
>
> - c = strchr(str, '/');
> - if (c)
> - *c = '\0';
> +static struct flag_to_string flags_str[] = {
> + { TCA_FLOWER_KEY_FLAGS_IS_FRAGMENT, FLOWER_IP_FLAGS, "frag" },
> +};
>
> - if (get_u32(&mtf, str, 0))
> - return -1;
> +static int flower_parse_matching_flags(char *str,
> + enum flower_matching_flags type,
> + __u32 *mtf, __u32 *mtf_mask)
> +{
> + char *token;
> + bool no;
> + bool found;
> + int i;
>
> - if (c) {
> - if (get_u32(&mtf_mask, ++c, 0))
> + token = strtok(str, "|");
> +
> + while (token) {
> + if (!strncmp(token, "no_", 3)) {
> + no = true;
> + token = strchr(token, '_') + 1;
> + } else
> + no = false;
> +
> + found = false;
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> +
> + if (!strcmp(token, flags_str[i].string)) {
> + if (no)
> + *mtf &= ~flags_str[i].flag;
> + else
> + *mtf |= flags_str[i].flag;
> +
> + *mtf_mask |= flags_str[i].flag;
> + found = true;
> + break;
> + }
> + }
> + if (!found)
> return -1;
> - } else {
> - mtf_mask = 0xffffffff;
> +
> + token = strtok(NULL, "|");
> }
>
> - addattr32(n, MAX_MSG, type, htonl(mtf));
> - addattr32(n, MAX_MSG, mask_type, htonl(mtf_mask));
> return 0;
> }
>
> @@ -433,6 +465,8 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> __be16 vlan_ethtype = 0;
> __u8 ip_proto = 0xff;
> __u32 flags = 0;
> + __u32 mtf = 0;
> + __u32 mtf_mask = 0;
>
> if (handle) {
> ret = get_u32(&t->tcm_handle, handle, 0);
> @@ -462,14 +496,14 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> return -1;
> }
> addattr_l(n, MAX_MSG, TCA_FLOWER_CLASSID, &handle, 4);
> - } else if (matches(*argv, "matching_flags") == 0) {
> + } else if (matches(*argv, "ip_flags") == 0) {
> NEXT_ARG();
> ret = flower_parse_matching_flags(*argv,
> - TCA_FLOWER_KEY_FLAGS,
> - TCA_FLOWER_KEY_FLAGS_MASK,
> - n);
> + FLOWER_IP_FLAGS,
> + &mtf,
> + &mtf_mask);
> if (ret < 0) {
> - fprintf(stderr, "Illegal \"matching_flags\"\n");
> + fprintf(stderr, "Illegal \"ip_flags\"\n");
> return -1;
> }
> } else if (matches(*argv, "skip_hw") == 0) {
> @@ -723,6 +757,16 @@ static int flower_parse_opt(struct filter_util *qu, char *handle,
> parse_done:
> addattr32(n, MAX_MSG, TCA_FLOWER_FLAGS, flags);
>
> + if (mtf_mask) {
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS, htonl(mtf));
> + if (ret)
> + return ret;
> +
> + ret = addattr32(n, MAX_MSG, TCA_FLOWER_KEY_FLAGS_MASK, htonl(mtf_mask));
> + if (ret)
> + return ret;
> + }
> +
> ret = addattr16(n, MAX_MSG, TCA_FLOWER_KEY_ETH_TYPE, eth_type);
> if (ret) {
> fprintf(stderr, "Illegal \"eth_type\"(0x%x)\n",
> @@ -828,14 +872,36 @@ static void flower_print_ip_proto(FILE *f, __u8 *p_ip_proto,
> }
>
> static void flower_print_matching_flags(FILE *f, char *name,
> + enum flower_matching_flags type,
> struct rtattr *attr,
> struct rtattr *mask_attr)
> {
> + int i;
> + int count = 0;
> + __u32 mtf;
> + __u32 mtf_mask;
> +
> if (!mask_attr || RTA_PAYLOAD(mask_attr) != 4)
> return;
>
> - fprintf(f, "\n %s 0x%08x/0x%08x", name, ntohl(rta_getattr_u32(attr)),
> - mask_attr ? ntohl(rta_getattr_u32(mask_attr)) : 0xffffffff);
> + mtf = ntohl(rta_getattr_u32(attr));
> + mtf_mask = ntohl(rta_getattr_u32(mask_attr));
> +
> + for (i = 0; i < ARRAY_SIZE(flags_str); i++) {
> + if (type != flags_str[i].type)
> + continue;
> + if (mtf_mask & flags_str[i].flag) {
> + if (++count == 1)
> + fprintf(f, "\n %s ", name);
> + else
> + fprintf(f, "|");
> +
> + if (mtf & flags_str[i].flag)
> + fprintf(f, "%s", flags_str[i].string);
> + else
> + fprintf(f, "no_%s", flags_str[i].string);
> + }
> + }
> }
>
> static void flower_print_ip_addr(FILE *f, char *name, __be16 eth_type,
> @@ -1034,7 +1100,8 @@ static int flower_print_opt(struct filter_util *qu, FILE *f,
> flower_print_port(f, "enc_dst_port",
> tb[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
>
> - flower_print_matching_flags(f, "matching_flags",
> + flower_print_matching_flags(f, "ip_flags",
> + FLOWER_IP_FLAGS,
> tb[TCA_FLOWER_KEY_FLAGS],
> tb[TCA_FLOWER_KEY_FLAGS_MASK]);
>
>
Hi,
I'm going to change the delimiter to "/" since "|" might causes some errors.
Powered by blists - more mailing lists