[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <08f4b4c2-eb3a-e92e-4b16-cc951e588935@intel.com>
Date: Fri, 9 Nov 2018 21:05:35 -0800
From: "Nambiar, Amritha" <amritha.nambiar@...el.com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: stephen@...workplumber.org, netdev@...r.kernel.org,
jakub.kicinski@...ronome.com, sridhar.samudrala@...el.com,
jhs@...atatu.com, xiyou.wangcong@...il.com
Subject: Re: [iproute2 PATCH v2] tc: flower: Classify packets based port
ranges
On 11/9/2018 12:51 AM, Jiri Pirko wrote:
> Wed, Nov 07, 2018 at 10:22:50PM CET, amritha.nambiar@...el.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.nambiar@...el.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.
>
Powered by blists - more mailing lists