On 11/9/2018 4:10 AM, Jiri Pirko wrote: > Wed, Nov 07, 2018 at 10:22:42PM CET, amritha.namb...@intel.com wrote: >> Added support in tc flower 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 comments: >> 1. Added separate functions for dst and src comparisons. >> 2. Removed endpoint enum. >> 3. Added new bit TCA_FLOWER_FLAGS_RANGE to decide normal/range >> lookup. >> 4. Cleaned up fl_lookup function. >> >> Signed-off-by: Amritha Nambiar <amritha.namb...@intel.com> >> --- >> include/uapi/linux/pkt_cls.h | 7 ++ >> net/sched/cls_flower.c | 133 >> ++++++++++++++++++++++++++++++++++++++++-- >> 2 files changed, 134 insertions(+), 6 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 */ >> + > > Please put it at the end of the enum, as David mentioned.
Will fix in v3. > > >> 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/net/sched/cls_flower.c b/net/sched/cls_flower.c >> index 9aada2d..9d2582d 100644 >> --- a/net/sched/cls_flower.c >> +++ b/net/sched/cls_flower.c >> @@ -55,6 +55,9 @@ struct fl_flow_key { >> struct flow_dissector_key_ip ip; >> struct flow_dissector_key_ip enc_ip; >> struct flow_dissector_key_enc_opts enc_opts; >> + > > No need for an empty line. Will fix in v3. > > >> + struct flow_dissector_key_ports tp_min; >> + struct flow_dissector_key_ports tp_max; >> } __aligned(BITS_PER_LONG / 8); /* Ensure that we can do comparisons as >> longs. */ >> >> struct fl_flow_mask_range { >> @@ -65,6 +68,7 @@ struct fl_flow_mask_range { >> struct fl_flow_mask { >> struct fl_flow_key key; >> struct fl_flow_mask_range range; >> + u32 flags; >> struct rhash_head ht_node; >> struct rhashtable ht; >> struct rhashtable_params filter_ht_params; >> @@ -179,13 +183,89 @@ static void fl_clear_masked_range(struct fl_flow_key >> *key, >> memset(fl_key_get_start(key, mask), 0, fl_mask_range(mask)); >> } >> >> -static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask, >> - struct fl_flow_key *mkey) >> +static bool fl_range_port_dst_cmp(struct cls_fl_filter *filter, >> + struct fl_flow_key *key, >> + struct fl_flow_key *mkey) >> +{ >> + __be16 min_mask, max_mask, min_val, max_val; >> + >> + min_mask = htons(filter->mask->key.tp_min.dst); >> + max_mask = htons(filter->mask->key.tp_max.dst); >> + min_val = htons(filter->key.tp_min.dst); >> + max_val = htons(filter->key.tp_max.dst); >> + >> + if (min_mask && max_mask) { >> + if (htons(key->tp.dst) < min_val || >> + htons(key->tp.dst) > max_val) >> + return false; >> + >> + /* skb does not have min and max values */ >> + mkey->tp_min.dst = filter->mkey.tp_min.dst; >> + mkey->tp_max.dst = filter->mkey.tp_max.dst; >> + } >> + return true; >> +} >> + >> +static bool fl_range_port_src_cmp(struct cls_fl_filter *filter, >> + struct fl_flow_key *key, >> + struct fl_flow_key *mkey) >> +{ >> + __be16 min_mask, max_mask, min_val, max_val; >> + >> + min_mask = htons(filter->mask->key.tp_min.src); >> + max_mask = htons(filter->mask->key.tp_max.src); >> + min_val = htons(filter->key.tp_min.src); >> + max_val = htons(filter->key.tp_max.src); >> + >> + if (min_mask && max_mask) { >> + if (htons(key->tp.src) < min_val || >> + htons(key->tp.src) > max_val) >> + return false; >> + >> + /* skb does not have min and max values */ >> + mkey->tp_min.src = filter->mkey.tp_min.src; >> + mkey->tp_max.src = filter->mkey.tp_max.src; >> + } >> + return true; >> +} >> + >> +static struct cls_fl_filter *__fl_lookup(struct fl_flow_mask *mask, >> + struct fl_flow_key *mkey) >> { >> return rhashtable_lookup_fast(&mask->ht, fl_key_get_start(mkey, mask), >> mask->filter_ht_params); >> } >> >> +static struct cls_fl_filter *fl_lookup_range(struct fl_flow_mask *mask, >> + struct fl_flow_key *mkey, >> + struct fl_flow_key *key) >> +{ >> + struct cls_fl_filter *filter, *f; >> + >> + list_for_each_entry_rcu(filter, &mask->filters, list) { >> + if (!fl_range_port_dst_cmp(filter, key, mkey)) >> + continue; >> + >> + if (!fl_range_port_src_cmp(filter, key, mkey)) >> + continue; >> + >> + f = __fl_lookup(mask, mkey); >> + if (f) >> + return f; >> + } >> + return NULL; >> +} >> + >> +static struct cls_fl_filter *fl_lookup(struct fl_flow_mask *mask, >> + struct fl_flow_key *mkey, >> + struct fl_flow_key *key) >> +{ >> + if ((mask->flags & TCA_FLOWER_MASK_FLAGS_RANGE)) >> + return fl_lookup_range(mask, mkey, key); >> + >> + return __fl_lookup(mask, mkey); >> +} >> + >> static int fl_classify(struct sk_buff *skb, const struct tcf_proto *tp, >> struct tcf_result *res) >> { >> @@ -207,8 +287,8 @@ static int fl_classify(struct sk_buff *skb, const struct >> tcf_proto *tp, >> skb_flow_dissect(skb, &mask->dissector, &skb_key, 0); >> >> fl_set_masked_key(&skb_mkey, &skb_key, mask); >> + f = fl_lookup(mask, &skb_mkey, &skb_key); >> >> - f = fl_lookup(mask, &skb_mkey); > > Please leave the original ordering (empty line, fl_lookup call). Will fix in v3. > > >> if (f && !tc_skip_sw(f->flags)) { >> *res = f->res; >> return tcf_exts_exec(skb, &f->exts, res); >> @@ -909,6 +989,23 @@ static int fl_set_key(struct net *net, struct nlattr >> **tb, >> sizeof(key->arp.tha)); >> } >> >> + if (key->basic.ip_proto == IPPROTO_TCP || >> + key->basic.ip_proto == IPPROTO_UDP || >> + key->basic.ip_proto == IPPROTO_SCTP) { >> + fl_set_key_val(tb, &key->tp_min.dst, >> + TCA_FLOWER_KEY_PORT_DST_MIN, &mask->tp_min.dst, >> + TCA_FLOWER_UNSPEC, sizeof(key->tp_min.dst)); >> + fl_set_key_val(tb, &key->tp_max.dst, >> + TCA_FLOWER_KEY_PORT_DST_MAX, &mask->tp_max.dst, >> + TCA_FLOWER_UNSPEC, sizeof(key->tp_max.dst)); >> + fl_set_key_val(tb, &key->tp_min.src, >> + TCA_FLOWER_KEY_PORT_SRC_MIN, &mask->tp_min.src, >> + TCA_FLOWER_UNSPEC, sizeof(key->tp_min.src)); >> + fl_set_key_val(tb, &key->tp_max.src, >> + TCA_FLOWER_KEY_PORT_SRC_MAX, &mask->tp_max.src, >> + TCA_FLOWER_UNSPEC, sizeof(key->tp_max.src)); >> + } >> + >> if (tb[TCA_FLOWER_KEY_ENC_IPV4_SRC] || >> tb[TCA_FLOWER_KEY_ENC_IPV4_DST]) { >> key->enc_control.addr_type = FLOW_DISSECTOR_KEY_IPV4_ADDRS; >> @@ -1026,8 +1123,7 @@ static void fl_init_dissector(struct flow_dissector >> *dissector, >> FLOW_DISSECTOR_KEY_IPV4_ADDRS, ipv4); >> FL_KEY_SET_IF_MASKED(mask, keys, cnt, >> FLOW_DISSECTOR_KEY_IPV6_ADDRS, ipv6); >> - FL_KEY_SET_IF_MASKED(mask, keys, cnt, >> - FLOW_DISSECTOR_KEY_PORTS, tp); >> + FL_KEY_SET(keys, cnt, FLOW_DISSECTOR_KEY_PORTS, tp); >> FL_KEY_SET_IF_MASKED(mask, keys, cnt, >> FLOW_DISSECTOR_KEY_IP, ip); >> FL_KEY_SET_IF_MASKED(mask, keys, cnt, >> @@ -1074,6 +1170,10 @@ static struct fl_flow_mask *fl_create_new_mask(struct >> cls_fl_head *head, >> >> fl_mask_copy(newmask, mask); >> >> + if ((newmask->key.tp_min.dst && newmask->key.tp_max.dst) || >> + (newmask->key.tp_min.src && newmask->key.tp_max.src)) >> + newmask->flags |= TCA_FLOWER_MASK_FLAGS_RANGE; >> + >> err = fl_init_mask_hashtable(newmask); >> if (err) >> goto errout_free; >> @@ -1227,7 +1327,7 @@ static int fl_change(struct net *net, struct sk_buff >> *in_skb, >> goto errout_idr; >> >> if (!tc_skip_sw(fnew->flags)) { >> - if (!fold && fl_lookup(fnew->mask, &fnew->mkey)) { >> + if (!fold && __fl_lookup(fnew->mask, &fnew->mkey)) { >> err = -EEXIST; >> goto errout_mask; >> } >> @@ -1800,6 +1900,27 @@ static int fl_dump_key(struct sk_buff *skb, struct >> net *net, >> sizeof(key->arp.tha)))) >> goto nla_put_failure; >> >> + if ((key->basic.ip_proto == IPPROTO_TCP || >> + key->basic.ip_proto == IPPROTO_UDP || >> + key->basic.ip_proto == IPPROTO_SCTP) && >> + (fl_dump_key_val(skb, &key->tp_min.dst, >> + TCA_FLOWER_KEY_PORT_DST_MIN, >> + &mask->tp_min.dst, TCA_FLOWER_UNSPEC, >> + sizeof(key->tp_min.dst)) || >> + fl_dump_key_val(skb, &key->tp_max.dst, >> + TCA_FLOWER_KEY_PORT_DST_MAX, >> + &mask->tp_max.dst, TCA_FLOWER_UNSPEC, >> + sizeof(key->tp_max.dst)) || >> + fl_dump_key_val(skb, &key->tp_min.src, >> + TCA_FLOWER_KEY_PORT_SRC_MIN, >> + &mask->tp_min.src, TCA_FLOWER_UNSPEC, >> + sizeof(key->tp_min.src)) || >> + fl_dump_key_val(skb, &key->tp_max.src, >> + TCA_FLOWER_KEY_PORT_SRC_MAX, >> + &mask->tp_max.src, TCA_FLOWER_UNSPEC, >> + sizeof(key->tp_max.src)))) >> + goto nla_put_failure; >> + >> if (key->enc_control.addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS && >> (fl_dump_key_val(skb, &key->enc_ipv4.src, >> TCA_FLOWER_KEY_ENC_IPV4_SRC, &mask->enc_ipv4.src, >>