From 6240624b3b536a8b7fd6f7036a7028fd1c34a030 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 13 Sep 2013 08:42:55 -0700 Subject: [PATCH] ofp-util: Announce OpenFlow 1.3 table features only in OpenFlow 1.3. 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 Tested-by: Torbjorn Tornkvist Signed-off-by: Ben Pfaff --- 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 d1e42a4db..541b1439f 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 23c7136bc..9edfe9ef0 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -4010,6 +4010,19 @@ ofputil_put_ofp11_table_stats(const struct ofp12_table_stats *in, out->matched_count = in->matched_count; } +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 02769fe8f..112790040 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2917,12 +2917,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 91ee85ab8..0e1d41ba0 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 -- 2.47.0