[ovs-dev] [PATCH 2/2] ofproto: Correctly reject duplicate bucket ID for OFPGC_INSERT_BUCKET.

[Ben Pfaff]

Otherwise duplicate bucket IDs cause linked list loops and other nastiness
because the ofputil_bucket_find() in the OFPG15_BUCKET_LAST case later in
copy_buckets_for_insert_bucket() will find the new bucket instead of the
old one and the list_splice() call becomes nonsensical.

Reported-by: Ray Li <rayli1107 at gmail.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2015-September/018731.html
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 AUTHORS           | 1 +
 ofproto/ofproto.c | 2 +-
 tests/ofproto.at  | 8 ++++++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/AUTHORS b/AUTHORS
index a7f40bb..75ca32a 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -339,6 +339,7 @@ Pratap Reddy            preddy at nicira.com
 Ralf Heiringhoff        ralf at frosty-geek.net
 Ram Jothikumar          rjothikumar at nicira.com
 Ramana Reddy            gtvrreddy at gmail.com
+Ray Li                  rayli1107 at gmail.com
 RishiRaj Maulick        rishi.raj2509 at gmail.com
 Rob Sherwood            rob.sherwood at bigswitch.com
 Robert Strickler        anomalyst at gmail.com
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c7dd8a2..c884948 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6368,7 +6368,7 @@ copy_buckets_for_insert_bucket(const struct ofgroup *ofgroup,
 
     ofputil_bucket_clone_list(&new_ofgroup->buckets, &ofgroup->buckets, NULL);
 
-    if (ofputil_bucket_check_duplicate_id(&ofgroup->buckets)) {
+    if (ofputil_bucket_check_duplicate_id(&new_ofgroup->buckets)) {
             VLOG_INFO_RL(&rl, "Duplicate bucket id");
             return OFPERR_OFPGMFC_BUCKET_EXISTS;
     }
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 7e80293..0f3a55e 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -438,6 +438,14 @@ 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
 ])
+
+# Verify that duplicate bucket IDs are rejected.
+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], [1], [], [stderr])
+AT_CHECK([STRIP_XIDS stderr | sed '/truncated/,$d'], [0], [dnl
+OFPT_ERROR (OF1.5): OFPGMFC_BUCKET_EXISTS
+OFPT_GROUP_MOD (OF1.5):
+])
+
 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
-- 
2.1.3