diff mbox

[ovs-dev,v2] ofproto: Return the OFPC_BUNDLES bit in switch features reply.

Message ID 1478904455-50752-1-git-send-email-jarno@ovn.org
State Accepted
Headers show

Commit Message

Jarno Rajahalme Nov. 11, 2016, 10:47 p.m. UTC 
Add definitions for the OpenFlow 1.4.1/1.5 specific capabilities bits
OFPC14_BUNDLES and OFPC14_FLOW_MONITORING.  Return the bundles
capability bit in switch features reply.

Reported-by: Andrej Leitner <andrej.leitner@pantheon.tech>
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
v2: Add the capabilities bits to OpenFlow 1.4 instead of OpenFlow 1.5, as
    OpenFlow 1.4.1 added support for these.

 include/openflow/openflow-1.4.h |  6 ++++++
 include/openvswitch/ofp-util.h  | 11 ++++++++---
 lib/ofp-print.c                 |  2 ++
 lib/ofp-util.c                  |  8 ++++++--
 ofproto/ofproto.c               |  2 +-
 tests/ofp-print.at              | 12 ++++++++++++
 tests/ofproto.at                |  2 +-
 7 files changed, 36 insertions(+), 7 deletions(-)

Comments

Ben Pfaff Nov. 12, 2016, 5:55 p.m. UTC | #1 
On Fri, Nov 11, 2016 at 02:47:35PM -0800, Jarno Rajahalme wrote:
> Add definitions for the OpenFlow 1.4.1/1.5 specific capabilities bits
> OFPC14_BUNDLES and OFPC14_FLOW_MONITORING.  Return the bundles
> capability bit in switch features reply.
> 
> Reported-by: Andrej Leitner <andrej.leitner@pantheon.tech>
> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>

Maybe it would be worth an OF1.5 test (that is almost identical to the
OF1.4 one).

Acked-by: Ben Pfaff <blp@ovn.org>
Jarno Rajahalme Nov. 14, 2016, 10:03 p.m. UTC | #2 
> On Nov 12, 2016, at 9:55 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Fri, Nov 11, 2016 at 02:47:35PM -0800, Jarno Rajahalme wrote:
>> Add definitions for the OpenFlow 1.4.1/1.5 specific capabilities bits
>> OFPC14_BUNDLES and OFPC14_FLOW_MONITORING.  Return the bundles
>> capability bit in switch features reply.
>> 
>> Reported-by: Andrej Leitner <andrej.leitner@pantheon.tech>
>> Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
> 
> Maybe it would be worth an OF1.5 test (that is almost identical to the
> OF1.4 one).
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks for the review! Added OF1.5 test and pushed this to master, branch-2.6, branch-2.5, and branch-2.4 (where OF bundles were first introduced).

  Jarno
diff mbox

Patch

diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index 4599f95..fcebe4e 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -39,6 +39,12 @@ 
 
 #include <openflow/openflow-1.3.h>
 
+/* OpenFlow 1.4.1+ specific capabilities
+ * (struct ofp_switch_features, member capabilities). */
+enum ofp14_capabilities {
+    OFPC14_BUNDLES        = 1 << 9,    /* Switch supports bundles. */
+    OFPC14_FLOW_MONITORING = 1 << 10,  /* Switch supports flow monitoring. */
+};
 
 /* ## ---------- ## */
 /* ## ofp14_port ## */
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index e4dacbf..8703d2a 100644
--- a/include/openvswitch/ofp-util.h
+++ b/include/openvswitch/ofp-util.h
@@ -613,7 +613,7 @@  struct ofputil_phy_port {
 };
 
 enum ofputil_capabilities {
-    /* OpenFlow 1.0, 1.1, 1.2, and 1.3 share these capability values. */
+    /* All OpenFlow versions share these capability values. */
     OFPUTIL_C_FLOW_STATS     = 1 << 0,  /* Flow statistics. */
     OFPUTIL_C_TABLE_STATS    = 1 << 1,  /* Table statistics. */
     OFPUTIL_C_PORT_STATS     = 1 << 2,  /* Port statistics. */
@@ -626,11 +626,16 @@  enum ofputil_capabilities {
     /* OpenFlow 1.0 only. */
     OFPUTIL_C_STP            = 1 << 3,  /* 802.1d spanning tree. */
 
-    /* OpenFlow 1.1, 1.2, and 1.3 share this capability. */
+    /* OpenFlow 1.1+ only.  Note that this bit value does not match the one
+     * in the OpenFlow message. */
     OFPUTIL_C_GROUP_STATS    = 1 << 4,  /* Group statistics. */
 
-    /* OpenFlow 1.2 and 1.3 share this capability */
+    /* OpenFlow 1.2+ only. */
     OFPUTIL_C_PORT_BLOCKED   = 1 << 8,  /* Switch will block looping ports */
+
+    /* OpenFlow 1.4+ only. */
+    OFPUTIL_C_BUNDLES         = 1 << 9,  /* Switch supports bundles. */
+    OFPUTIL_C_FLOW_MONITORING = 1 << 10, /* Switch supports flow monitoring. */
 };
 
 /* Abstract ofp_switch_features. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 0a68551..b7b5290 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -506,6 +506,8 @@  ofputil_capabilities_to_name(uint32_t bit)
     case OFPUTIL_C_STP:          return "STP";
     case OFPUTIL_C_GROUP_STATS:  return "GROUP_STATS";
     case OFPUTIL_C_PORT_BLOCKED: return "PORT_BLOCKED";
+    case OFPUTIL_C_BUNDLES:      return "BUNDLES";
+    case OFPUTIL_C_FLOW_MONITORING: return "FLOW_MONITORING";
     }
 
     return NULL;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index c0a402f..899cfe3 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4644,6 +4644,9 @@  BUILD_ASSERT_DECL((int) OFPUTIL_C_PORT_STATS == OFPC_PORT_STATS);
 BUILD_ASSERT_DECL((int) OFPUTIL_C_IP_REASM == OFPC_IP_REASM);
 BUILD_ASSERT_DECL((int) OFPUTIL_C_QUEUE_STATS == OFPC_QUEUE_STATS);
 BUILD_ASSERT_DECL((int) OFPUTIL_C_ARP_MATCH_IP == OFPC_ARP_MATCH_IP);
+BUILD_ASSERT_DECL((int) OFPUTIL_C_PORT_BLOCKED == OFPC12_PORT_BLOCKED);
+BUILD_ASSERT_DECL((int) OFPUTIL_C_BUNDLES == OFPC14_BUNDLES);
+BUILD_ASSERT_DECL((int) OFPUTIL_C_FLOW_MONITORING == OFPC14_FLOW_MONITORING);
 
 static uint32_t
 ofputil_capabilities_mask(enum ofp_version ofp_version)
@@ -4655,10 +4658,12 @@  ofputil_capabilities_mask(enum ofp_version ofp_version)
         return OFPC_COMMON | OFPC_ARP_MATCH_IP;
     case OFP12_VERSION:
     case OFP13_VERSION:
+        return OFPC_COMMON | OFPC12_PORT_BLOCKED;
     case OFP14_VERSION:
     case OFP15_VERSION:
     case OFP16_VERSION:
-        return OFPC_COMMON | OFPC12_PORT_BLOCKED;
+        return OFPC_COMMON | OFPC12_PORT_BLOCKED | OFPC14_BUNDLES
+            | OFPC14_FLOW_MONITORING;
     default:
         /* Caller needs to check osf->header.version itself */
         return 0;
@@ -4784,7 +4789,6 @@  ofputil_encode_switch_features(const struct ofputil_switch_features *features,
     osf->n_buffers = htonl(features->n_buffers);
     osf->n_tables = features->n_tables;
 
-    osf->capabilities = htonl(features->capabilities & OFPC_COMMON);
     osf->capabilities = htonl(features->capabilities &
                               ofputil_capabilities_mask(version));
     switch (version) {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index de1c469..53b7226 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3295,7 +3295,7 @@  handle_features_request(struct ofconn *ofconn, const struct ofp_header *oh)
     features.n_tables = ofproto_get_n_visible_tables(ofproto);
     features.capabilities = (OFPUTIL_C_FLOW_STATS | OFPUTIL_C_TABLE_STATS |
                              OFPUTIL_C_PORT_STATS | OFPUTIL_C_QUEUE_STATS |
-                             OFPUTIL_C_GROUP_STATS);
+                             OFPUTIL_C_GROUP_STATS | OFPUTIL_C_BUNDLES);
     if (arp_match_ip) {
         features.capabilities |= OFPUTIL_C_ARP_MATCH_IP;
     }
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 6946438..5ae5df8 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -403,6 +403,18 @@  capabilities: FLOW_STATS TABLE_STATS PORT_STATS IP_REASM QUEUE_STATS PORT_BLOCKE
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_FEATURES_REPLY - OF1.4])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+05 06 00 20 00 00 00 01 00 00 50 54 00 00 00 01 \
+00 00 01 00 ff 00 00 00 00 00 07 6f 00 00 00 00 \
+"], [0], [dnl
+OFPT_FEATURES_REPLY (OF1.4) (xid=0x1): dpid:0000505400000001
+n_tables:255, n_buffers:256
+capabilities: FLOW_STATS TABLE_STATS PORT_STATS GROUP_STATS IP_REASM QUEUE_STATS PORT_BLOCKED BUNDLES FLOW_MONITORING
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_FEATURES_REPLY - with auxiliary_id - OF1.3])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 3964f1c..d360b7a 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1242,7 +1242,7 @@  do
     AT_CHECK_UNQUOTED([strip_xids < stdout], [0], [dnl
 OFPT_FEATURES_REPLY (OF1.4): dpid:fedcba9876543210
 n_tables:254, n_buffers:0
-capabilities: FLOW_STATS TABLE_STATS PORT_STATS GROUP_STATS QUEUE_STATS
+capabilities: FLOW_STATS TABLE_STATS PORT_STATS GROUP_STATS QUEUE_STATS BUNDLES
 OFPST_PORT_DESC reply (OF1.4):
  LOCAL(br0): addr:aa:55:aa:55:00:00
      config:     $config