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,
>>

Reply via email to