From patchwork Wed Sep 7 06:55:26 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gurucharan Shetty X-Patchwork-Id: 667099 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3sTqRP1QkXz9rxl for ; Thu, 8 Sep 2016 02:58:25 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 6A18510262; Wed, 7 Sep 2016 09:58:24 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e4.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id AAA851025F for ; Wed, 7 Sep 2016 09:58:22 -0700 (PDT) Received: from bar5.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id 3B2CF1E02C1 for ; Wed, 7 Sep 2016 10:58:22 -0600 (MDT) X-ASG-Debug-ID: 1473267501-09eadd335792d7b0001-byXFYA Received: from mx3-pf2.cudamail.com ([192.168.14.1]) by bar5.cudamail.com with ESMTP id YVt0goPgt6tPgCTN (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 07 Sep 2016 10:58:21 -0600 (MDT) X-Barracuda-Envelope-From: guru.ovn@gmail.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.1 Received: from unknown (HELO mail-pf0-f195.google.com) (209.85.192.195) by mx3-pf2.cudamail.com with ESMTPS (AES128-SHA encrypted); 7 Sep 2016 16:58:21 -0000 Received-SPF: pass (mx3-pf2.cudamail.com: SPF record at _netblocks.google.com designates 209.85.192.195 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.85.192.195 X-Barracuda-RBL-IP: 209.85.192.195 Received: by mail-pf0-f195.google.com with SMTP id 128so1207016pfb.0 for ; Wed, 07 Sep 2016 09:58:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=ItwaVosBmTIb4UrwYNIkcvO+Yq/4lKaEQofPIGiOmf8=; b=HHOK7EqpB8aVnrUuRjXNsikFA2cTNPy7ajpIOSp74j2nfW4ROT3Rk32Asciowm6CiK DGk+Gyy7Yv0Yqnc9u1xoAesyl4iYPIMKX24MbwAfPkYjVdg8XqKsKS/Xr/Q6jtmF2JT9 +Z60igA+StcDAFOnnnquawOs2VrR2mJAS3qGc3LtKC1nODNPv0fIBPMSMVCy3xLYcXre htD3nz+UCEs/uQeaiykI0aklssO0rNc4OqXmW45BLXuUalvdGJ4xhsDoRY96yfSow8xy zKAyYrjLk80ny7y7DW+SDcEwp8VbkXueKUdCxErrU+5Tj/UMSzwLmukzTXAqmopjd9Of UPUA== X-Gm-Message-State: AE9vXwPcndpovSQobhKfwN15fMEfosyC4xDCS0of6MAt/+qjsJZlYs6kCl5xPi8HvEK42w== X-Received: by 10.98.50.2 with SMTP id y2mr83557282pfy.138.1473267500479; Wed, 07 Sep 2016 09:58:20 -0700 (PDT) Received: from ubuntu.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id d72sm50225963pfj.15.2016.09.07.09.58.19 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 07 Sep 2016 09:58:19 -0700 (PDT) X-CudaMail-Envelope-Sender: guru.ovn@gmail.com From: Gurucharan Shetty To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V2-906031508 X-CudaMail-DTE: 090716 X-CudaMail-Originating-IP: 209.85.192.195 Date: Tue, 6 Sep 2016 23:55:26 -0700 X-ASG-Orig-Subj: [##CM-V2-906031508##][PATCH] ovn-controller: Fix group_id allocation. Message-Id: <1473231326-16611-1-git-send-email-guru@ovn.org> X-Mailer: git-send-email 1.9.1 X-Barracuda-Connect: UNKNOWN[192.168.14.1] X-Barracuda-Start-Time: 1473267501 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH] ovn-controller: Fix group_id allocation. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" 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 Acked-by: Ben Pfaff --- include/ovn/actions.h | 2 ++ ovn/controller/ofctrl.c | 6 +++++- ovn/lib/actions.c | 3 +++ 3 files changed, 10 insertions(+), 1 deletion(-) 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);