On 11/9/2018 12:51 AM, Jiri Pirko wrote: > Wed, Nov 07, 2018 at 10:22:50PM CET, amritha.namb...@intel.com wrote: >> Added support for filtering based on port ranges. >> >> Example: >> 1. Match on a port range: >> ------------------------- >> $ tc filter add dev enp4s0 protocol ip parent ffff:\ >> prio 1 flower ip_proto tcp dst_port range 20-30 skip_hw\ >> action drop >> >> $ tc -s filter show dev enp4s0 parent ffff: >> filter protocol ip pref 1 flower chain 0 >> filter protocol ip pref 1 flower chain 0 handle 0x1 >> eth_type ipv4 >> ip_proto tcp >> dst_port range 20-30 >> skip_hw >> not_in_hw >> action order 1: gact action drop >> random type none pass val 0 >> index 1 ref 1 bind 1 installed 85 sec used 3 sec >> Action statistics: >> Sent 460 bytes 10 pkt (dropped 10, overlimits 0 requeues 0) >> backlog 0b 0p requeues 0 >> >> 2. Match on IP address and port range: >> -------------------------------------- >> $ tc filter add dev enp4s0 protocol ip parent ffff:\ >> prio 1 flower dst_ip 192.168.1.1 ip_proto tcp dst_port range 100-200\ >> skip_hw action drop >> >> $ tc -s filter show dev enp4s0 parent ffff: >> filter protocol ip pref 1 flower chain 0 handle 0x2 >> eth_type ipv4 >> ip_proto tcp >> dst_ip 192.168.1.1 >> dst_port range 100-200 >> skip_hw >> not_in_hw >> action order 1: gact action drop >> random type none pass val 0 >> index 2 ref 1 bind 1 installed 58 sec used 2 sec >> Action statistics: >> Sent 920 bytes 20 pkt (dropped 20, overlimits 0 requeues 0) >> backlog 0b 0p requeues 0 >> >> v2: >> Addressed Jiri's comment to sync output format with input >> >> Signed-off-by: Amritha Nambiar <amritha.namb...@intel.com> >> --- >> include/uapi/linux/pkt_cls.h | 7 ++ >> tc/f_flower.c | 145 >> +++++++++++++++++++++++++++++++++++++++--- >> 2 files changed, 142 insertions(+), 10 deletions(-) >> >> diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h >> index 401d0c1..b63c3cf 100644 >> --- a/include/uapi/linux/pkt_cls.h >> +++ b/include/uapi/linux/pkt_cls.h >> @@ -405,6 +405,11 @@ enum { >> TCA_FLOWER_KEY_UDP_SRC, /* be16 */ >> TCA_FLOWER_KEY_UDP_DST, /* be16 */ >> >> + TCA_FLOWER_KEY_PORT_SRC_MIN, /* be16 */ >> + TCA_FLOWER_KEY_PORT_SRC_MAX, /* be16 */ >> + TCA_FLOWER_KEY_PORT_DST_MIN, /* be16 */ >> + TCA_FLOWER_KEY_PORT_DST_MAX, /* be16 */ >> + >> TCA_FLOWER_FLAGS, >> TCA_FLOWER_KEY_VLAN_ID, /* be16 */ >> TCA_FLOWER_KEY_VLAN_PRIO, /* u8 */ >> @@ -518,6 +523,8 @@ enum { >> TCA_FLOWER_KEY_FLAGS_FRAG_IS_FIRST = (1 << 1), >> }; >> >> +#define TCA_FLOWER_MASK_FLAGS_RANGE (1 << 0) /* Range-based match */ >> + >> /* Match-all classifier */ >> >> enum { >> diff --git a/tc/f_flower.c b/tc/f_flower.c >> index 65fca04..7724a1d 100644 >> --- a/tc/f_flower.c >> +++ b/tc/f_flower.c >> @@ -494,6 +494,66 @@ static int flower_parse_port(char *str, __u8 ip_proto, >> return 0; >> } >> >> +static int flower_port_range_attr_type(__u8 ip_proto, enum flower_endpoint >> type, >> + __be16 *min_port_type, >> + __be16 *max_port_type) >> +{ >> + if (ip_proto == IPPROTO_TCP || ip_proto == IPPROTO_UDP || >> + ip_proto == IPPROTO_SCTP) { >> + if (type == FLOWER_ENDPOINT_SRC) { >> + *min_port_type = TCA_FLOWER_KEY_PORT_SRC_MIN; >> + *max_port_type = TCA_FLOWER_KEY_PORT_SRC_MAX; >> + } else { >> + *min_port_type = TCA_FLOWER_KEY_PORT_DST_MIN; >> + *max_port_type = TCA_FLOWER_KEY_PORT_DST_MAX; >> + } >> + } else { >> + return -1; >> + } >> + >> + return 0; >> +} >> + >> +static int flower_parse_port_range(__be16 *min, __be16 *max, __u8 ip_proto, >> + enum flower_endpoint endpoint, >> + struct nlmsghdr *n) >> +{ >> + __be16 min_port_type, max_port_type; >> + >> + flower_port_range_attr_type(ip_proto, endpoint, &min_port_type, >> + &max_port_type); >> + addattr16(n, MAX_MSG, min_port_type, *min); >> + addattr16(n, MAX_MSG, max_port_type, *max); >> + >> + return 0; >> +} >> + >> +static int get_range(__be16 *min, __be16 *max, char *argv) >> +{ >> + char *r; >> + >> + r = strchr(argv, '-'); >> + if (r) { >> + *r = '\0'; >> + if (get_be16(min, argv, 10)) { >> + fprintf(stderr, "invalid min range\n"); >> + return -1; >> + } >> + if (get_be16(max, r + 1, 10)) { >> + fprintf(stderr, "invalid max range\n"); >> + return -1; >> + } >> + if (htons(*max) <= htons(*min)) { >> + fprintf(stderr, "max value should be greater than min >> value\n"); >> + return -1; >> + } >> + } else { >> + fprintf(stderr, "Illegal range format\n"); >> + return -1; >> + } >> + return 0; >> +} >> + >> #define TCP_FLAGS_MAX_MASK 0xfff >> >> static int flower_parse_tcp_flags(char *str, int flags_type, int mask_type, >> @@ -1061,20 +1121,54 @@ static int flower_parse_opt(struct filter_util *qu, >> char *handle, >> return -1; >> } >> } else if (matches(*argv, "dst_port") == 0) { >> + __be16 min, max; >> + >> NEXT_ARG(); >> - ret = flower_parse_port(*argv, ip_proto, >> - FLOWER_ENDPOINT_DST, n); >> - if (ret < 0) { >> - fprintf(stderr, "Illegal \"dst_port\"\n"); >> - return -1; >> + if (matches(*argv, "range") == 0) { >> + NEXT_ARG(); >> + ret = get_range(&min, &max, *argv); >> + if (ret < 0) >> + return -1; >> + ret = flower_parse_port_range(&min, &max, >> + ip_proto, >> + >> FLOWER_ENDPOINT_DST, >> + n); >> + if (ret < 0) { >> + fprintf(stderr, "Illegal \"dst_port >> range\"\n"); >> + return -1; >> + } >> + } else { >> + ret = flower_parse_port(*argv, ip_proto, >> + FLOWER_ENDPOINT_DST, n); >> + if (ret < 0) { >> + fprintf(stderr, "Illegal >> \"dst_port\"\n"); >> + return -1; >> + } >> } >> } else if (matches(*argv, "src_port") == 0) { >> + __be16 min, max; >> + >> NEXT_ARG(); >> - ret = flower_parse_port(*argv, ip_proto, >> - FLOWER_ENDPOINT_SRC, n); >> - if (ret < 0) { >> - fprintf(stderr, "Illegal \"src_port\"\n"); >> - return -1; >> + if (matches(*argv, "range") == 0) { >> + NEXT_ARG(); >> + ret = get_range(&min, &max, *argv); >> + if (ret < 0) >> + return -1; >> + ret = flower_parse_port_range(&min, &max, >> + ip_proto, >> + >> FLOWER_ENDPOINT_SRC, >> + n); >> + if (ret < 0) { >> + fprintf(stderr, "Illegal \"src_port >> range\"\n"); >> + return -1; >> + } >> + } else { >> + ret = flower_parse_port(*argv, ip_proto, >> + FLOWER_ENDPOINT_SRC, n); >> + if (ret < 0) { >> + fprintf(stderr, "Illegal >> \"src_port\"\n"); >> + return -1; >> + } >> } >> } else if (matches(*argv, "tcp_flags") == 0) { >> NEXT_ARG(); >> @@ -1490,6 +1584,22 @@ static void flower_print_port(char *name, struct >> rtattr *attr) >> print_hu(PRINT_ANY, name, namefrm, rta_getattr_be16(attr)); >> } >> >> +static void flower_print_port_range(char *name, struct rtattr *min_attr, >> + struct rtattr *max_attr) >> +{ >> + SPRINT_BUF(namefrm); >> + SPRINT_BUF(out); >> + size_t done; >> + >> + if (!min_attr || !max_attr) >> + return; >> + >> + done = sprintf(out, "%u", rta_getattr_be16(min_attr)); >> + sprintf(out + done, "-%u", rta_getattr_be16(max_attr)); >> + sprintf(namefrm, "\n %s %%s", name); >> + print_string(PRINT_ANY, name, namefrm, out); >> +} >> + >> static void flower_print_tcp_flags(const char *name, struct rtattr >> *flags_attr, >> struct rtattr *mask_attr) >> { >> @@ -1678,6 +1788,7 @@ static int flower_print_opt(struct filter_util *qu, >> FILE *f, >> struct rtattr *opt, __u32 handle) >> { >> struct rtattr *tb[TCA_FLOWER_MAX + 1]; >> + __be16 min_port_type, max_port_type; >> int nl_type, nl_mask_type; >> __be16 eth_type = 0; >> __u8 ip_proto = 0xff; >> @@ -1796,6 +1907,20 @@ static int flower_print_opt(struct filter_util *qu, >> FILE *f, >> if (nl_type >= 0) >> flower_print_port("src_port", tb[nl_type]); >> >> + if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_DST, >> + &min_port_type, &max_port_type) >> + == 0) { > > This is odd. Please convert to !fower_port....
Will fix in v3 and resubmit after the UAPI changes are accepted into net-next. > > > >> + flower_print_port_range("dst_port range", >> + tb[min_port_type], tb[max_port_type]); >> + } >> + >> + if (flower_port_range_attr_type(ip_proto, FLOWER_ENDPOINT_SRC, >> + &min_port_type, &max_port_type) >> + == 0) { > > Same here. Will fix in v3. > > >> + flower_print_port_range("src_port range", >> + tb[min_port_type], tb[max_port_type]); >> + } >> + >> flower_print_tcp_flags("tcp_flags", tb[TCA_FLOWER_KEY_TCP_FLAGS], >> tb[TCA_FLOWER_KEY_TCP_FLAGS_MASK]); >> >> > > Otherwise this looks fine. >