diff mbox

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

Message ID 1476314065-48789-1-git-send-email-jarno@ovn.org
State Superseded
Headers show

Commit Message

Jarno Rajahalme Oct. 12, 2016, 11:14 p.m. UTC
Add definitions for the OpenFlow 1.5 specific capabilities bits
OFPC15_BUNDLES and OFPC15_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>
---
 include/openflow/openflow-1.5.h |  7 +++++++
 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 ++++++++++++
 6 files changed, 36 insertions(+), 6 deletions(-)

Comments

Jarno Rajahalme Nov. 11, 2016, 1:17 a.m. UTC | #1
> On Nov 10, 2016, at 4:42 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Wed, Oct 12, 2016 at 04:14:25PM -0700, Jarno Rajahalme wrote:
>> Add definitions for the OpenFlow 1.5 specific capabilities bits
>> OFPC15_BUNDLES and OFPC15_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>
> 
> Thank you!
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks for the review!

As Andrej noted in an email, these capabilities bits were added also in OpenFlow 1.4.1 (which was published after OpenFlow 1.5). I’ll rework the patch to add these in 1.4 instead of 1.5 and re-post, unless you think otherwise.

Thanks,

  Jarno
Jarno Rajahalme Nov. 11, 2016, 10:54 p.m. UTC | #2
I just posted a v2 for your reviewing enjoyment.

  Jarno

> On Nov 10, 2016, at 5:17 PM, Jarno Rajahalme <jarno@ovn.org> wrote:
> 
> 
>> On Nov 10, 2016, at 4:42 PM, Ben Pfaff <blp@ovn.org> wrote:
>> 
>> On Wed, Oct 12, 2016 at 04:14:25PM -0700, Jarno Rajahalme wrote:
>>> Add definitions for the OpenFlow 1.5 specific capabilities bits
>>> OFPC15_BUNDLES and OFPC15_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>
>> 
>> Thank you!
>> 
>> Acked-by: Ben Pfaff <blp@ovn.org>
> 
> Thanks for the review!
> 
> As Andrej noted in an email, these capabilities bits were added also in OpenFlow 1.4.1 (which was published after OpenFlow 1.5). I’ll rework the patch to add these in 1.4 instead of 1.5 and re-post, unless you think otherwise.
> 
> Thanks,
> 
>  Jarno
>
diff mbox

Patch

diff --git a/include/openflow/openflow-1.5.h b/include/openflow/openflow-1.5.h
index 3649e6c..e3c7c6f 100644
--- a/include/openflow/openflow-1.5.h
+++ b/include/openflow/openflow-1.5.h
@@ -39,6 +39,13 @@ 
 
 #include <openflow/openflow-common.h>
 
+/* OpenFlow 1.5 specific capabilities
+ * (struct ofp_switch_features, member capabilities). */
+enum ofp15_capabilities {
+    OFPC15_BUNDLES        = 1 << 9,    /* Switch supports bundles. */
+    OFPC15_FLOW_MONITORING = 1 << 10,  /* Switch supports flow monitoring. */
+};
+
 /* Body for ofp15_multipart_request of type OFPMP_PORT_DESC. */
 struct ofp15_port_desc_request {
     ovs_be32 port_no;         /* All ports if OFPP_ANY. */
diff --git a/include/openvswitch/ofp-util.h b/include/openvswitch/ofp-util.h
index f3cb624..e55d7b5 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.5+ 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 0445968..84109d0 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 == OFPC15_BUNDLES);
+BUILD_ASSERT_DECL((int) OFPUTIL_C_FLOW_MONITORING == OFPC15_FLOW_MONITORING);
 
 static uint32_t
 ofputil_capabilities_mask(enum ofp_version ofp_version)
@@ -4656,9 +4659,11 @@  ofputil_capabilities_mask(enum ofp_version ofp_version)
     case OFP12_VERSION:
     case OFP13_VERSION:
     case OFP14_VERSION:
+        return OFPC_COMMON | OFPC12_PORT_BLOCKED;
     case OFP15_VERSION:
     case OFP16_VERSION:
-        return OFPC_COMMON | OFPC12_PORT_BLOCKED;
+        return OFPC_COMMON | OFPC12_PORT_BLOCKED | OFPC15_BUNDLES
+            | OFPC15_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..5fb2880 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.5])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "\
+06 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.5) (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 "\