| Message ID | 1473231326-16611-1-git-send-email-guru@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Sep 06, 2016 at 11:55:26PM -0700, Gurucharan Shetty wrote: > A bitmap in 'struct group_table' is used to track all the allocated > group_ids. For every run of logical flows action parsing, we > add 'group_info' structure to a hmap called 'desired_groups'. The > group_id assigned to this group_info either comes from an already > installed 'existing groups' or a new reservation done in the bitmap. > > In ofctrl_put(), if there is a backlog, we call ovn_group_table_clear(). > This could unreserve a group_id that comes from an already existing group. > This could result in re-use of group_id in the future causing errors while > installed new groups. > > This commit fixes the above scenario. > > Signed-off-by: Gurucharan Shetty <guru@ovn.org> Acked-by: Ben Pfaff <blp@ovn.org>
diff --git a/include/ovn/actions.h b/include/ovn/actions.h index d1942b3..fbf1f58 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -291,6 +291,8 @@ struct group_info { struct hmap_node hmap_node; struct ds group; uint32_t group_id; + bool new_group_id; /* 'True' if 'group_id' was reserved from + * group_table's 'group_ids' bitmap. */ }; enum action_opcode { diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c index 584e260..fe72d79 100644 --- a/ovn/controller/ofctrl.c +++ b/ovn/controller/ofctrl.c @@ -744,7 +744,11 @@ ovn_group_table_clear(struct group_table *group_table, bool existing) HMAP_FOR_EACH_SAFE (g, next, hmap_node, target_group) { hmap_remove(target_group, &g->hmap_node); - bitmap_set0(group_table->group_ids, g->group_id); + /* Don't unset bitmap for desired group_info if the group_id + * was not freshly reserved. */ + if (existing || g->new_group_id) { + bitmap_set0(group_table->group_ids, g->group_id); + } ds_destroy(&g->group); free(g); } diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index 28b66ed..8ed6ddc 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -974,10 +974,12 @@ encode_CT_LB(const struct ovnact_ct_lb *cl, } } + bool new_group_id = false; if (!group_id) { /* Reserve a new group_id. */ group_id = bitmap_scan(ep->group_table->group_ids, 0, 1, MAX_OVN_GROUPS + 1); + new_group_id = true; } if (group_id == MAX_OVN_GROUPS + 1) { @@ -993,6 +995,7 @@ encode_CT_LB(const struct ovnact_ct_lb *cl, group_info->group = ds; group_info->group_id = group_id; group_info->hmap_node.hash = hash; + group_info->new_group_id = new_group_id; hmap_insert(&ep->group_table->desired_groups, &group_info->hmap_node, group_info->hmap_node.hash);
A bitmap in 'struct group_table' is used to track all the allocated group_ids. For every run of logical flows action parsing, we add 'group_info' structure to a hmap called 'desired_groups'. The group_id assigned to this group_info either comes from an already installed 'existing groups' or a new reservation done in the bitmap. In ofctrl_put(), if there is a backlog, we call ovn_group_table_clear(). This could unreserve a group_id that comes from an already existing group. This could result in re-use of group_id in the future causing errors while installed new groups. This commit fixes the above scenario. Signed-off-by: Gurucharan Shetty <guru@ovn.org> --- include/ovn/actions.h | 2 ++ ovn/controller/ofctrl.c | 6 +++++- ovn/lib/actions.c | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-)