Skip to content

Commit

Permalink
Update prefix len in Mtrie bucket as well
Browse files Browse the repository at this point in the history 
Right now, if mtrie already contains more specific entries and if a less
specific entry is added, only non-matching leaves are updated with less
specific entry's prefix len. The matching buckets also need to be
updated with prefix len. Not updating the prefix len in buckets, results
in, not collapsing the tree, if these less specific entries are deleted.

For example, in an empty tree, 8.1.1.10/32 -> nh1 is added. This results
in, at level 1 bucket 8, level 2 bucket 1, level 3 bucket 1 to contain
prefix len as 0 and level 4 bucket 10 to contain prefix len as 32. The
other leaves in level 4 (like 8.1.1.1 till 8.1.1.255 except 10) also
contain prefix len as 0. After this 8.1.1.0/24 -> nh2 is added.
Currently the prefix len of 24 is updated only in the leaves like, 8.1.1.1
till 8.1.1.255. Ideally the prefix len 24 should be updated in level 3
bucket 1 as well. Not updating this, leads to issues while collapsing
level 3 buckets.

Fixes:
1) Prefix len is updated in the matching buckets as well
2) In the label flags, if LABEL_VALID flag not set, the label stored
   does not have any meaning. But while collapsing the bucket the labels
   are also compared leading to non-collapse. The labels need to be
   compared only if the LABEL_VALID flag is set. To over come this, if
   the LABEL_VALID is not set, the label is stored as -1, so that they
   are comparable even if flag is not set
3) In 'rt' utility, the replacment len incase of delete is wrongly
   initialised to 100. This is rectified.

Change-Id: I6b10f934275999e2eb73a07ec4edf78eb5f155d7
closes-bug: #1605748
  • Loading branch information
@divakardhar
divakardhar committed Aug 30, 2016
1 parent 0031aa5 commit 398f3a8
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 58 deletions.
101 changes: 46 additions & 55 deletions dp-core/vr_ip_mtrie.c
View file
Expand Up @@ -316,28 +316,34 @@ add_to_tree(struct ip_bucket_entry *ent, int level, struct vr_route_req *rt)
struct ip_bucket *bkt;
struct mtrie_bkt_info *ip_bkt_info;

if (ent->entry_prefix_len > rt->rtr_req.rtr_prefix_len)
return;


ent->entry_prefix_len = rt->rtr_req.rtr_prefix_len;

if (ENTRY_IS_NEXTHOP(ent)) {
/* a less specific entry, which needs to be replaced */
set_entry_to_nh(ent, rt->rtr_nh);
ent->entry_label_flags = rt->rtr_req.rtr_label_flags;
ent->entry_label = rt->rtr_req.rtr_label;
ent->entry_bridge_index = rt->rtr_req.rtr_index;

return;
}

if (level >= (ip_bkt_get_max_level(rt->rtr_req.rtr_family) - 1))
/* assert here ? */
return;

ip_bkt_info = ip_bkt_info_get(rt->rtr_req.rtr_family);

/* assured that the first one is a bucket */
/* Assured that this is valid bucket now */
bkt = entry_to_bucket(ent);
level++;

for (i = 0; i < ip_bkt_info[level].bi_size; i++) {
ent = index_to_entry(bkt, i);
if (!ENTRY_IS_NEXTHOP(ent))
add_to_tree(ent, level, rt);
else if (ent->entry_prefix_len <= rt->rtr_req.rtr_prefix_len) {
/* a less specific entry, which needs to be replaced */
set_entry_to_nh(ent, rt->rtr_nh);
ent->entry_prefix_len = rt->rtr_req.rtr_prefix_len;
ent->entry_label_flags = rt->rtr_req.rtr_label_flags;
ent->entry_label = rt->rtr_req.rtr_label;
ent->entry_bridge_index = rt->rtr_req.rtr_index;
}
add_to_tree(ent, level, rt);
}

return;
Expand Down Expand Up @@ -384,11 +390,11 @@ mtrie_reset_entry(struct ip_bucket_entry *ent, int level,
static int
__mtrie_add(struct ip_mtrie *mtrie, struct vr_route_req *rt)
{
int ret, index = 0, level, err_level = 0;
unsigned char i, fin = 0;
struct ip_bucket *bkt;
struct ip_bucket_entry *ent, *err_ent = NULL;
struct vr_nexthop *nh, *err_nh = NULL;
int ret, index = 0, level, err_level = 0, fin;
unsigned char i;
struct ip_bucket *bkt;
struct ip_bucket_entry *ent, *err_ent = NULL;
struct vr_nexthop *nh, *err_nh = NULL;
struct mtrie_bkt_info *ip_bkt_info = ip_bkt_info_get(rt->rtr_req.rtr_family);

ent = &mtrie->root;
Expand Down Expand Up @@ -426,42 +432,20 @@ __mtrie_add(struct ip_mtrie *mtrie, struct vr_route_req *rt)
* cover all the indices for which this route is the best
* prefix match
*/

fin = ip_bkt_info[level].bi_size;

if ((rt->rtr_req.rtr_prefix_len >
(ip_bkt_info[level].bi_pfx_len - ip_bkt_info[level].bi_bits)) &&
(rt->rtr_req.rtr_prefix_len <= ip_bkt_info[level].bi_pfx_len)) {
fin = 1 << (ip_bkt_info[level].bi_pfx_len - rt->rtr_req.rtr_prefix_len);
}


/*
* Run through the loop 'fin' times only
* If fin is 0, it actually means 256 ('char' overflow), so run the
* loop 256 times
*/
for (i = index; i <= (ip_bkt_info[level].bi_size-1); i++) {
i = index;
for (; ((i <= (ip_bkt_info[level].bi_size-1)) && fin);
i++, fin--) {
ent = index_to_entry(bkt, i);
if (ENTRY_IS_BUCKET(ent))
add_to_tree(ent, level, rt);
else if (ent->entry_prefix_len <= rt->rtr_req.rtr_prefix_len) {
/* a less specific entry, which needs to be replaced */
set_entry_to_nh(ent, rt->rtr_nh);
ent->entry_prefix_len = rt->rtr_req.rtr_prefix_len;
ent->entry_label_flags = rt->rtr_req.rtr_label_flags;
ent->entry_label = rt->rtr_req.rtr_label;
ent->entry_bridge_index = rt->rtr_req.rtr_index;
}
if (fin) {
/* Repeat the loop 'fin' times only */
fin--;
if (fin == 0)
break;
}
/*
* Bailout at the last index,
* the below check takes care of overflow
*/
if (i == (ip_bkt_info[level].bi_size-1))
break;
add_to_tree(ent, level, rt);
}

break;
Expand Down Expand Up @@ -512,6 +496,7 @@ __mtrie_delete(struct vr_route_req *rt, struct ip_bucket_entry *ent,

for (i = index; i < fin; i++) {
tmp_ent = index_to_entry(bkt, i);

if (tmp_ent->entry_prefix_len == rt->rtr_req.rtr_prefix_len) {
tmp_ent->entry_label_flags = rt->rtr_req.rtr_label_flags;
tmp_ent->entry_label = rt->rtr_req.rtr_label;
Expand All @@ -529,15 +514,8 @@ __mtrie_delete(struct vr_route_req *rt, struct ip_bucket_entry *ent,

/* check if current bucket neds to be deleted */
for (i = 1; i < ip_bkt_info[level].bi_size; i++) {
if ((bkt->bkt_data[i].entry_long_i == bkt->bkt_data[0].entry_long_i) &&
(bkt->bkt_data[i].entry_label_flags ==
bkt->bkt_data[0].entry_label_flags) &&
(bkt->bkt_data[i].entry_label ==
bkt->bkt_data[0].entry_label) &&
(bkt->bkt_data[i].entry_prefix_len ==
bkt->bkt_data[0].entry_prefix_len)) {
continue;
} else
if (memcmp(bkt->bkt_data + i, bkt->bkt_data,
sizeof(struct ip_bucket_entry)))
return 0;
}

Expand Down Expand Up @@ -754,6 +732,12 @@ mtrie_delete(struct vr_rtable * _unused, struct vr_route_req *rt)
rt->rtr_req.rtr_index = lreq.rtr_req.rtr_index;
}

if (!(rt->rtr_req.rtr_label_flags & VR_RT_LABEL_VALID_FLAG)) {
rt->rtr_req.rtr_label = 0xFFFFF;
} else {
rt->rtr_req.rtr_label &= 0xFFFFF;
}

__mtrie_delete(rt, &rtable->root, 0);
vrouter_put_nexthop(rt->rtr_nh);

Expand Down Expand Up @@ -1003,6 +987,12 @@ mtrie_add(struct vr_rtable * _unused, struct vr_route_req *rt)
rt->rtr_req.rtr_index = tmp_req.rtr_req.rtr_index;
}

if (!(rt->rtr_req.rtr_label_flags & VR_RT_LABEL_VALID_FLAG)) {
rt->rtr_req.rtr_label = 0xFFFFF;
} else {
rt->rtr_req.rtr_label &= 0xFFFFF;
}

ret = __mtrie_add(mtrie, rt);
vrouter_put_nexthop(rt->rtr_nh);
return ret;
Expand Down Expand Up @@ -1041,6 +1031,7 @@ mtrie_alloc_vrf(unsigned int vrf_id, unsigned int family)
mtrie->root.entry_bridge_index = VR_BE_INVALID_INDEX;
mtrie_table = vn_rtable[index];
mtrie_table[vrf_id] = mtrie;
mtrie->root.entry_label = 0xFFFFF;
}

return mtrie;
Expand Down
9 changes: 6 additions & 3 deletions utils/rt.c
View file
Expand Up @@ -63,7 +63,7 @@ static int cmd_nh_id = -1;
static uint8_t cmd_prefix[16], cmd_src[16];
static uint32_t cmd_plen = 0;
static int32_t cmd_label;
static uint32_t cmd_replace_plen = 100;
static uint32_t cmd_replace_plen = 0xFFFFFFFF;
static char cmd_dst_mac[6];
static bool response_pending = true;
static void Usage(void);
Expand Down Expand Up @@ -520,8 +520,11 @@ validate_options(void)

case SANDESH_OP_DELETE:
if ((cmd_family_id == AF_INET) || (cmd_family_id == AF_INET6)) {
if ((cmd_replace_plen < 0 ||
( cmd_replace_plen > (RT_IP_ADDR_SIZE(cmd_family_id)*4)))) {

if (cmd_replace_plen == 0xFFFFFFFF)
goto usage_internal;

if (cmd_replace_plen > (RT_IP_ADDR_SIZE(cmd_family_id) * 8)) {
goto usage_internal;
}

Expand Down

0 comments on commit 398f3a8

Please sign in to comment.