From patchwork Thu Oct 15 00:12:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 530450 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (unknown [IPv6:2600:3c00::f03c:91ff:fe6e:bdf7]) by ozlabs.org (Postfix) with ESMTP id 24545140E3B for ; Thu, 15 Oct 2015 11:13:02 +1100 (AEDT) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 30EFB109D8; Wed, 14 Oct 2015 17:13:01 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e3.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id 5CF19109D5 for ; Wed, 14 Oct 2015 17:12:59 -0700 (PDT) Received: from bar5.cudamail.com (localhost [127.0.0.1]) by mx1e3.cudamail.com (Postfix) with ESMTPS id A237842021C for ; Wed, 14 Oct 2015 18:12:58 -0600 (MDT) X-ASG-Debug-ID: 1444867978-09eadd76e001280001-byXFYA Received: from mx1-pf1.cudamail.com ([192.168.24.1]) by bar5.cudamail.com with ESMTP id eTy9DFPMGWmjHudN (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Wed, 14 Oct 2015 18:12:58 -0600 (MDT) X-Barracuda-Envelope-From: blp@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.1 Received: from unknown (HELO mail-pa0-f46.google.com) (209.85.220.46) by mx1-pf1.cudamail.com with ESMTPS (RC4-SHA encrypted); 15 Oct 2015 00:12:57 -0000 Received-SPF: unknown (mx1-pf1.cudamail.com: Multiple SPF records returned) X-Barracuda-Apparent-Source-IP: 209.85.220.46 X-Barracuda-RBL-IP: 209.85.220.46 Received: by pacao1 with SMTP id ao1so3711473pac.2 for ; Wed, 14 Oct 2015 17:12:56 -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:cc:subject:date:message-id; bh=hxSYE8By7xbxcGl3H4WQ9MdLaJ9gHJ23RB+m6WBN1Dw=; b=Gny52Pwuf5tYiuyYq07dAHCifmOjvUcScsmXDbrP9eujfOg8i3CGjzDndsBLcdl+M7 k3O0oUJ2AUb2eyqzwrPXRy7DjpOs1nYkBKaSq5MHg/jdJ3fjAv0qcpZbHS4rOiK4SdCQ gPhpZBYbaVriDCD2odzKp0ih/KXORepScmVQZPCxCTXKF09yAzgvt9iKz0g4H+Rt4rtL 4a1DGVfi3VNt9I77rJmO3+vYSv6cm3FFBmxWjlu956yC9iAObp4/xnSoBsVu2msJ+AnL 3rV1M/YeiS+ogwhwhtDk9qMdlvIhgvM0bHuNZm4TuZCj3xQb2VzkCxIOo8PurrjjJ4/a 63Vg== X-Gm-Message-State: ALoCoQnKORYDTtrE8oGx+4Ih0rDOi0L+Gj5lMndCtdBNILP03fHmnLHvYM9sNhtUK7sIy2TLaMow X-Received: by 10.68.184.195 with SMTP id ew3mr6718866pbc.28.1444867976596; Wed, 14 Oct 2015 17:12:56 -0700 (PDT) Received: from sigabrt.benpfaff.org ([208.91.2.4]) by smtp.gmail.com with ESMTPSA id ez2sm11747440pbb.5.2015.10.14.17.12.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 14 Oct 2015 17:12:55 -0700 (PDT) X-CudaMail-Envelope-Sender: blp@nicira.com From: Ben Pfaff To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E1-1013110939 X-CudaMail-DTE: 101415 X-CudaMail-Originating-IP: 209.85.220.46 Date: Wed, 14 Oct 2015 17:12:54 -0700 X-ASG-Orig-Subj: [##CM-E1-1013110939##][PATCH] ofproto: Fix inserting buckets at the end of an empty group. Message-Id: <1444867974-6175-1-git-send-email-blp@nicira.com> X-Mailer: git-send-email 2.1.3 X-Barracuda-Connect: UNKNOWN[192.168.24.1] X-Barracuda-Start-Time: 1444867978 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 Cc: Ben Pfaff , Ray Li Subject: [ovs-dev] [PATCH] ofproto: Fix inserting buckets at the end of an empty group. 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" This caused a segfault. Reported-by: Ray Li Reported-at: http://openvswitch.org/pipermail/discuss/2015-September/018746.html Signed-off-by: Ben Pfaff Reviewed-by: Simon Horman --- AUTHORS | 1 + ofproto/ofproto.c | 16 +++++++++------- tests/ofproto.at | 32 +++++++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/AUTHORS b/AUTHORS index f4e1ca9..04794fb 100644 --- a/AUTHORS +++ b/AUTHORS @@ -343,6 +343,7 @@ Pratap Reddy preddy@nicira.com Ralf Heiringhoff ralf@frosty-geek.net Ram Jothikumar rjothikumar@nicira.com Ramana Reddy gtvrreddy@gmail.com +Ray Li rayli1107@gmail.com RishiRaj Maulick rishi.raj2509@gmail.com Rob Sherwood rob.sherwood@bigswitch.com Robert Strickler anomalyst@gmail.com diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index c7dd8a2..389f853 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -6375,15 +6375,17 @@ copy_buckets_for_insert_bucket(const struct ofgroup *ofgroup, /* Rearrange list according to command_bucket_id */ if (command_bucket_id == OFPG15_BUCKET_LAST) { - struct ofputil_bucket *new_first; - const struct ofputil_bucket *first; + if (!list_is_empty(&ofgroup->buckets)) { + struct ofputil_bucket *new_first; + const struct ofputil_bucket *first; - first = ofputil_bucket_list_front(&ofgroup->buckets); - new_first = ofputil_bucket_find(&new_ofgroup->buckets, - first->bucket_id); + first = ofputil_bucket_list_front(&ofgroup->buckets); + new_first = ofputil_bucket_find(&new_ofgroup->buckets, + first->bucket_id); - list_splice(new_ofgroup->buckets.next, &new_first->list_node, - &new_ofgroup->buckets); + list_splice(new_ofgroup->buckets.next, &new_first->list_node, + &new_ofgroup->buckets); + } } else if (command_bucket_id <= OFPG15_BUCKET_MAX && last) { struct ofputil_bucket *after; diff --git a/tests/ofproto.at b/tests/ofproto.at index 5e4441c..cead710 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -417,33 +417,61 @@ dnl It at least checks request and reply serialization and deserialization. dnl Actions definition listed in both supported formats (w/ actions=) AT_SETUP([ofproto - insert buckets]) OVS_VSWITCHD_START +# Add group with no buckets. AT_DATA([groups.txt], [dnl -group_id=1234,type=all,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11 +group_id=1234,type=all ]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt]) AT_CHECK([ovs-ofctl -F OXM-OpenFlow15 -O OpenFlow15 -vwarn dump-groups br0 1234], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_DESC reply (OF1.5): + group_id=1234,type=all +]) + +# Add two buckets, using "last". +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=last,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11]) +AT_CHECK([ovs-ofctl -F OXM-OpenFlow15 -O OpenFlow15 -vwarn dump-groups br0 1234], [0], [stdout]) +AT_CHECK([STRIP_XIDS stdout], [0], [dnl +OFPST_GROUP_DESC reply (OF1.5): group_id=1234,type=all,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11 ]) + +# Start over again, then add two buckets using "first". +AT_CHECK([ovs-ofctl -O OpenFlow15 --strict del-groups br0 group_id=1234]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-group br0 group_id=1234,type=all]) +AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=first,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11]) +AT_CHECK([ovs-ofctl -F OXM-OpenFlow15 -O OpenFlow15 -vwarn dump-groups br0 1234], [0], [stdout]) +AT_CHECK([STRIP_XIDS stdout], [0], [dnl +OFPST_GROUP_DESC reply (OF1.5): + group_id=1234,type=all,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11 +]) + +# Add two more buckets before the existing ones. AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=first,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_DESC reply (OF1.5): group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11 ]) + +# Add another bucket at the end. AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=last,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_DESC reply (OF1.5): group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15 ]) + +# Add another bucket just before bucket 15. AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=15]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_DESC reply (OF1.5): group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15 ]) + +# Add two more buckets just before bucket 11, +# getting the command from a file. AT_DATA([buckets.txt], [dnl group_id=1234,command_bucket_id=11,bucket=bucket_id:12,actions=output:12,bucket=bucket_id:13,actions=output:13 ]) @@ -453,6 +481,8 @@ AT_CHECK([STRIP_XIDS stdout], [0], [dnl OFPST_GROUP_DESC reply (OF1.5): group_id=1234,type=all,bucket=bucket_id:0,actions=output:0,bucket=bucket_id:1,actions=output:1,bucket=bucket_id:10,actions=output:10,bucket=bucket_id:11,actions=output:11,bucket=bucket_id:12,actions=output:12,bucket=bucket_id:13,actions=output:13,bucket=bucket_id:14,actions=output:14,bucket=bucket_id:15,actions=output:15 ]) + +# Add yet two more buckets. AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 group_id=1234,command_bucket_id=15,bucket=bucket_id:20,actions=output:20,bucket=bucket_id:21,actions=output:21]) AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-groups br0], [0], [stdout]) AT_CHECK([STRIP_XIDS stdout], [0], [dnl