[ovs-dev] [PATCH] ofproto: Only allow indirect groups with one bucket

[Simon Horman]

OpenFlow 1.1 - 1.4 specify that indirect groups should
opperate on the "one defined bucket in the group". OpenFlow
1.2 - 1.4 also state "This group only supports a single bucket."

This patch enforces the single bucket limitation for indirect groups
when decoding group mod messages. A test is also added to exercise
this change.

Signed-off-by: Simon Horman <simon.horman at netronome.com>
---
 lib/ofp-util.c   | 14 ++++++++++++++
 tests/ofproto.at | 16 ++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d765d03..573f38a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -7420,6 +7420,20 @@ ofputil_decode_group_mod(const struct ofp_header *oh,
         return err;
     }
 
+    switch (gm->type) {
+    case OFPGT11_INDIRECT:
+        if (!list_is_singleton(&gm->buckets)) {
+            return OFPERR_OFPGMFC_INVALID_GROUP;
+        }
+        break;
+    case OFPGT11_ALL:
+    case OFPGT11_SELECT:
+    case OFPGT11_FF:
+        break;
+    default:
+        OVS_NOT_REACHED();
+    }
+
     LIST_FOR_EACH (bucket, list_node, &gm->buckets) {
         switch (gm->type) {
         case OFPGT11_ALL:
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 3278287..69f8dc9 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -298,6 +298,22 @@ AT_CLEANUP
 
 dnl This is really bare-bones.
 dnl It at least checks request and reply serialization and deserialization.
+dnl Actions definition listed in both supported formats (w/ actions=)
+AT_SETUP([ofproto - add indirect group])
+OVS_VSWITCHD_START
+dnl indirect group must have exactly one bucket
+AT_DATA([stderr], [dnl
+OFPT_ERROR (OF1.1) (xid=0x2): OFPGMFC_INVALID_GROUP
+OFPT_GROUP_MOD (OF1.1) (xid=0x2): ***decode error: OFPGMFC_INVALID_GROUP***
+])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-group br0 'group_id=1234,type=indirect'], [1], , [stderr])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-group br0 'group_id=1235,type=indirect,bucket=output:10'])
+AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-group br0 'group_id=1236,type=indirect,bucket=output:10,bucket=output:11'], [1], , [stderr])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+dnl This is really bare-bones.
+dnl It at least checks request and reply serialization and deserialization.
 AT_SETUP([ofproto - del group deletes flows])
 OVS_VSWITCHD_START
 AT_DATA([groups.txt], [dnl
-- 
2.1.1