[ovs-dev] [PATCH] ofp-util: Announce OpenFlow 1.3 table features only in OpenFlow 1.3.

Ben Pfaff blp at nicira.com
Thu Sep 12 17:33:09 UTC 2013


The translation into OpenFlow 1.2 didn't trim off the OpenFlow 1.3 specific
bits.  This fixes the problem.

It would probably be wise to introduce an ofputil_table_stats structure.
Using ofp12_table_stats is somewhat confusing.

Reported-by: Torbjorn Tornkvist <kruskakli at gmail.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 include/openflow/openflow-1.2.h |   26 +++++++++++-----------
 lib/ofp-util.c                  |   45 +++++++++++++++++++++++----------------
 ofproto/ofproto.c               |    8 +++----
 tests/ofproto.at                |   12 +++++------
 4 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/include/openflow/openflow-1.2.h b/include/openflow/openflow-1.2.h
index d1e42a4..541b143 100644
--- a/include/openflow/openflow-1.2.h
+++ b/include/openflow/openflow-1.2.h
@@ -109,18 +109,16 @@ enum oxm12_ofb_match_fields {
     OFPXMT12_OFB_IPV6_ND_TLL,    /* Target link-layer for ND. */
     OFPXMT12_OFB_MPLS_LABEL,     /* MPLS label. */
     OFPXMT12_OFB_MPLS_TC,        /* MPLS TC. */
-    /* Following added in OpenFlow 1.3 */
-    OFPXMT12_OFB_MPLS_BOS,       /* MPLS BoS bit. */
-    OFPXMT12_OFB_PBB_ISID,       /* PBB I-SID. */
-    OFPXMT12_OFB_TUNNEL_ID,      /* Logical Port Metadata */
-    OFPXMT12_OFB_IPV6_EXTHDR,    /* IPv6 Extension Header pseudo-field */
+#define OFPXMT12_MASK ((1ULL << (OFPXMT12_OFB_MPLS_TC + 1)) - 1)
 
-    /* End Marker */
-    OFPXMT12_OFB_MAX,
+    /* Following added in OpenFlow 1.3 */
+    OFPXMT13_OFB_MPLS_BOS,       /* MPLS BoS bit. */
+    OFPXMT13_OFB_PBB_ISID,       /* PBB I-SID. */
+    OFPXMT13_OFB_TUNNEL_ID,      /* Logical Port Metadata */
+    OFPXMT13_OFB_IPV6_EXTHDR,    /* IPv6 Extension Header pseudo-field */
+#define OFPXMT13_MASK ((1ULL << (OFPXMT13_OFB_IPV6_EXTHDR + 1)) - 1)
 };
 
-#define OFPXMT12_MASK ((1ULL << OFPXMT12_OFB_MAX) - 1)
-
 /* OXM implementation makes use of NXM as they are the same format
  * with different field definitions
  */
@@ -180,13 +178,13 @@ enum oxm12_ofb_match_fields {
 #define OXM_OF_IPV6_ND_TLL    OXM_HEADER   (OFPXMT12_OFB_IPV6_ND_TLL, 6)
 #define OXM_OF_MPLS_LABEL     OXM_HEADER   (OFPXMT12_OFB_MPLS_LABEL, 4)
 #define OXM_OF_MPLS_TC        OXM_HEADER   (OFPXMT12_OFB_MPLS_TC, 1)
-#define OXM_OF_MPLS_BOS       OXM_HEADER   (OFPXMT12_OFB_MPLS_BOS, 1)
+#define OXM_OF_MPLS_BOS       OXM_HEADER   (OFPXMT13_OFB_MPLS_BOS, 1)
 #define OXM_OF_PBB_ISID       OXM_HEADER   (OFPXMT12_OFB_PBB_ISID, 4)
 #define OXM_OF_PBB_ISID_W     OXM_HEADER_W (OFPXMT12_OFB_PBB_ISID, 4)
-#define OXM_OF_TUNNEL_ID      OXM_HEADER   (OFPXMT12_OFB_TUNNEL_ID, 8)
-#define OXM_OF_TUNNEL_ID_W    OXM_HEADER_W (OFPXMT12_OFB_TUNNEL_ID, 8)
-#define OXM_OF_IPV6_EXTHDR    OXM_HEADER   (OFPXMT12_OFB_IPV6_EXTHDR, 2)
-#define OXM_OF_IPV6_EXTHDR_W  OXM_HEADER_W (OFPXMT12_OFB_IPV6_EXTHDR, 2)
+#define OXM_OF_TUNNEL_ID      OXM_HEADER   (OFPXMT13_OFB_TUNNEL_ID, 8)
+#define OXM_OF_TUNNEL_ID_W    OXM_HEADER_W (OFPXMT13_OFB_TUNNEL_ID, 8)
+#define OXM_OF_IPV6_EXTHDR    OXM_HEADER   (OFPXMT13_OFB_IPV6_EXTHDR, 2)
+#define OXM_OF_IPV6_EXTHDR_W  OXM_HEADER_W (OFPXMT13_OFB_IPV6_EXTHDR, 2)
 
 /* The VLAN id is 12-bits, so we can use the entire 16 bits to indicate
  * special conditions.
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 23c7136..9edfe9e 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4011,6 +4011,19 @@ ofputil_put_ofp11_table_stats(const struct ofp12_table_stats *in,
 }
 
 static void
+ofputil_put_ofp12_table_stats(const struct ofp12_table_stats *in,
+                              struct ofpbuf *buf)
+{
+    struct ofp12_table_stats *out = ofpbuf_put(buf, in, sizeof *in);
+
+    /* Trim off OF1.3-only capabilities. */
+    out->match &= htonll(OFPXMT12_MASK);
+    out->wildcards &= htonll(OFPXMT12_MASK);
+    out->write_setfields &= htonll(OFPXMT12_MASK);
+    out->apply_setfields &= htonll(OFPXMT12_MASK);
+}
+
+static void
 ofputil_put_ofp13_table_stats(const struct ofp12_table_stats *in,
                               struct ofpbuf *buf)
 {
@@ -4035,31 +4048,27 @@ ofputil_encode_table_stats_reply(const struct ofp12_table_stats stats[], int n,
 
     reply = ofpraw_alloc_stats_reply(request, n * sizeof *stats);
 
-    switch ((enum ofp_version) request->version) {
-    case OFP10_VERSION:
-        for (i = 0; i < n; i++) {
+    for (i = 0; i < n; i++) {
+        switch ((enum ofp_version) request->version) {
+        case OFP10_VERSION:
             ofputil_put_ofp10_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    case OFP11_VERSION:
-        for (i = 0; i < n; i++) {
+        case OFP11_VERSION:
             ofputil_put_ofp11_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    case OFP12_VERSION:
-        ofpbuf_put(reply, stats, n * sizeof *stats);
-        break;
+        case OFP12_VERSION:
+            ofputil_put_ofp12_table_stats(&stats[i], reply);
+            break;
 
-    case OFP13_VERSION:
-        for (i = 0; i < n; i++) {
+        case OFP13_VERSION:
             ofputil_put_ofp13_table_stats(&stats[i], reply);
-        }
-        break;
+            break;
 
-    default:
-        NOT_REACHED();
+        default:
+            NOT_REACHED();
+        }
     }
 
     return reply;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9605baa..aa946e9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2711,12 +2711,12 @@ handle_table_stats_request(struct ofconn *ofconn,
     for (i = 0; i < p->n_tables; i++) {
         ots[i].table_id = i;
         sprintf(ots[i].name, "table%zu", i);
-        ots[i].match = htonll(OFPXMT12_MASK);
-        ots[i].wildcards = htonll(OFPXMT12_MASK);
+        ots[i].match = htonll(OFPXMT13_MASK);
+        ots[i].wildcards = htonll(OFPXMT13_MASK);
         ots[i].write_actions = htonl(OFPAT11_OUTPUT);
         ots[i].apply_actions = htonl(OFPAT11_OUTPUT);
-        ots[i].write_setfields = htonll(OFPXMT12_MASK);
-        ots[i].apply_setfields = htonll(OFPXMT12_MASK);
+        ots[i].write_setfields = htonll(OFPXMT13_MASK);
+        ots[i].apply_setfields = htonll(OFPXMT13_MASK);
         ots[i].metadata_match = htonll(UINT64_MAX);
         ots[i].metadata_write = htonll(UINT64_MAX);
         ots[i].instructions = htonl(OFPIT11_ALL);
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 91ee85a..0e1d41b 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -732,13 +732,13 @@ AT_CLEANUP
 AT_SETUP([ofproto - flow table configuration (OpenFlow 1.2)])
 OVS_VSWITCHD_START
 # Check the default configuration.
-(mid="wild=0xffffffffff, max=1000000,"
+(mid="wild=0xfffffffff, max=1000000,"
  tail="
                lookup=0, matched=0
-               match=0xffffffffff, instructions=0x00000007, config=0x00000003
+               match=0xfffffffff, instructions=0x00000007, config=0x00000003
                write_actions=0x00000000, apply_actions=0x00000000
-               write_setfields=0x000000ffffffffff
-               apply_setfields=0x000000ffffffffff
+               write_setfields=0x0000000fffffffff
+               apply_setfields=0x0000000fffffffff
                metadata_match=0xffffffffffffffff
                metadata_write=0xffffffffffffffff"
  echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables
@@ -763,9 +763,9 @@ AT_CHECK(
 # Check that the configuration was updated.
 mv expout orig-expout
 (echo "OFPST_TABLE reply (OF1.2) (xid=0x2): 254 tables
-  0: main    : wild=0xffffffffff, max=1000000, active=0"
+  0: main    : wild=0xfffffffff, max=1000000, active=0"
  tail -n +3 orig-expout | head -7
- echo "  1: table1  : wild=0xffffffffff, max=  1024, active=0"
+ echo "  1: table1  : wild=0xfffffffff, max=  1024, active=0"
  tail -n +11 orig-expout) > expout
 AT_CHECK([ovs-ofctl -O OpenFlow12 dump-tables br0], [0], [expout])
 OVS_VSWITCHD_STOP
-- 
1.7.10.4




More information about the dev mailing list