[ovs-dev] [PATCH 2/6] ofp-protocol: Better abstract changing the protocol used for flow matches.
Ben Pfaff
blp at ovn.org
Fri Feb 16 22:54:41 UTC 2018
The previous interface here required the client to understand, to some
extent, the low-level NXFF_* values and the encoding format for the
NXT_SET_FLOW_FORMAT and NXT_SET_FLOW_MOD_TABLE_ID messages. This commit
changes the interface so that the client only has to understand the
ofputil_protocol type used elsewhere and none of the encoding otherwise.
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
include/openflow/nicira-ext.h | 60 +-----------------
include/openvswitch/ofp-flow.h | 9 ---
include/openvswitch/ofp-msgs.h | 4 +-
include/openvswitch/ofp-protocol.h | 11 ++++
lib/ofp-flow.c | 72 +--------------------
lib/ofp-print.c | 27 ++++----
lib/ofp-protocol.c | 126 +++++++++++++++++++++++++++++++++++--
ofproto/ofproto.c | 20 ++----
8 files changed, 153 insertions(+), 176 deletions(-)
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index 71a1de035d18..ddb68aa25e79 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -113,51 +113,6 @@ enum nx_hash_fields {
};
-/* This command enables or disables an Open vSwitch extension that allows a
- * controller to specify the OpenFlow table to which a flow should be added,
- * instead of having the switch decide which table is most appropriate as
- * required by OpenFlow 1.0. Because NXM was designed as an extension to
- * OpenFlow 1.0, the extension applies equally to ofp10_flow_mod and
- * nx_flow_mod. By default, the extension is disabled.
- *
- * When this feature is enabled, Open vSwitch treats struct ofp10_flow_mod's
- * and struct nx_flow_mod's 16-bit 'command' member as two separate fields.
- * The upper 8 bits are used as the table ID, the lower 8 bits specify the
- * command as usual. A table ID of 0xff is treated like a wildcarded table ID.
- *
- * The specific treatment of the table ID depends on the type of flow mod:
- *
- * - OFPFC_ADD: Given a specific table ID, the flow is always placed in that
- * table. If an identical flow already exists in that table only, then it
- * is replaced. If the flow cannot be placed in the specified table,
- * either because the table is full or because the table cannot support
- * flows of the given type, the switch replies with an OFPFMFC_TABLE_FULL
- * error. (A controller can distinguish these cases by comparing the
- * current and maximum number of entries reported in ofp_table_stats.)
- *
- * If the table ID is wildcarded, the switch picks an appropriate table
- * itself. If an identical flow already exist in the selected flow table,
- * then it is replaced. The choice of table might depend on the flows
- * that are already in the switch; for example, if one table fills up then
- * the switch might fall back to another one.
- *
- * - OFPFC_MODIFY, OFPFC_DELETE: Given a specific table ID, only flows
- * within that table are matched and modified or deleted. If the table ID
- * is wildcarded, flows within any table may be matched and modified or
- * deleted.
- *
- * - OFPFC_MODIFY_STRICT, OFPFC_DELETE_STRICT: Given a specific table ID,
- * only a flow within that table may be matched and modified or deleted.
- * If the table ID is wildcarded and exactly one flow within any table
- * matches, then it is modified or deleted; if flows in more than one
- * table match, then none is modified or deleted.
- */
-struct nx_flow_mod_table_id {
- uint8_t set; /* Nonzero to enable, zero to disable. */
- uint8_t pad[7];
-};
-OFP_ASSERT(sizeof(struct nx_flow_mod_table_id) == 8);
-
enum nx_packet_in_format {
NXPIF_STANDARD = 0, /* OFPT_PACKET_IN for this OpenFlow version. */
NXPIF_NXT_PACKET_IN = 1, /* NXT_PACKET_IN (since OVS v1.1). */
@@ -618,17 +573,6 @@ OFP_ASSERT(sizeof(struct nx_async_config) == 24);
/* ## Requests and replies. ## */
/* ## --------------------- ## */
-enum nx_flow_format {
- NXFF_OPENFLOW10 = 0, /* Standard OpenFlow 1.0 compatible. */
- NXFF_NXM = 2 /* Nicira extended match. */
-};
-
-/* NXT_SET_FLOW_FORMAT request. */
-struct nx_set_flow_format {
- ovs_be32 format; /* One of NXFF_*. */
-};
-OFP_ASSERT(sizeof(struct nx_set_flow_format) == 4);
-
/* NXT_FLOW_MOD (analogous to OFPT_FLOW_MOD).
*
* It is possible to limit flow deletions and modifications to certain
@@ -637,8 +581,8 @@ OFP_ASSERT(sizeof(struct nx_set_flow_format) == 4);
*/
struct nx_flow_mod {
ovs_be64 cookie; /* Opaque controller-issued identifier. */
- ovs_be16 command; /* OFPFC_* + possibly a table ID (see comment
- * on struct nx_flow_mod_table_id). */
+ ovs_be16 command; /* OFPFC_*, and table ID if flow_mod_table_id
+ * is enabled. */
ovs_be16 idle_timeout; /* Idle time before discarding (seconds). */
ovs_be16 hard_timeout; /* Max time before discarding (seconds). */
ovs_be16 priority; /* Priority level of flow entry. */
diff --git a/include/openvswitch/ofp-flow.h b/include/openvswitch/ofp-flow.h
index abaf4a87a400..28aa77bad820 100644
--- a/include/openvswitch/ofp-flow.h
+++ b/include/openvswitch/ofp-flow.h
@@ -31,15 +31,6 @@ struct ofputil_table_map;
extern "C" {
#endif
-/* nx_flow_format */
-struct ofpbuf *ofputil_encode_nx_set_flow_format(enum nx_flow_format);
-enum ofputil_protocol ofputil_nx_flow_format_to_protocol(enum nx_flow_format);
-bool ofputil_nx_flow_format_is_valid(enum nx_flow_format);
-const char *ofputil_nx_flow_format_to_string(enum nx_flow_format);
-
-/* NXT_FLOW_MOD_TABLE_ID extension. */
-struct ofpbuf *ofputil_make_flow_mod_table_id(bool flow_mod_table_id);
-
/* Protocol-independent flow_mod flags. */
enum ofputil_flow_mod_flags {
/* Flags that are maintained with a flow as part of its state.
diff --git a/include/openvswitch/ofp-msgs.h b/include/openvswitch/ofp-msgs.h
index 3f92f2a67751..23f3015aee20 100644
--- a/include/openvswitch/ofp-msgs.h
+++ b/include/openvswitch/ofp-msgs.h
@@ -443,10 +443,10 @@ enum ofpraw {
* Nicira extensions that correspond to standard OpenFlow messages are listed
* alongside the standard versions above. */
- /* NXT 1.0 (12): struct nx_set_flow_format. */
+ /* NXT 1.0 (12): ovs_be32. */
OFPRAW_NXT_SET_FLOW_FORMAT,
- /* NXT 1.0+ (15): struct nx_flow_mod_table_id. */
+ /* NXT 1.0+ (15): uint8_t[8]. */
OFPRAW_NXT_FLOW_MOD_TABLE_ID,
/* NXT 1.0+ (16): struct nx_set_packet_in_format. */
diff --git a/include/openvswitch/ofp-protocol.h b/include/openvswitch/ofp-protocol.h
index bd1bedf32e69..a351660ac216 100644
--- a/include/openvswitch/ofp-protocol.h
+++ b/include/openvswitch/ofp-protocol.h
@@ -162,11 +162,22 @@ enum ofputil_protocol ofputil_protocols_from_string(const char *s);
const char *ofputil_version_to_string(enum ofp_version ofp_version);
uint32_t ofputil_versions_from_string(const char *s);
uint32_t ofputil_versions_from_strings(char ** const s, size_t count);
+
+/* Messages for changing the protocol. */
+/* Changing the protocol at a high level. */
struct ofpbuf *ofputil_encode_set_protocol(enum ofputil_protocol current,
enum ofputil_protocol want,
enum ofputil_protocol *next);
+/* Changing the protocol at a low level. */
+struct ofpbuf *ofputil_encode_nx_set_flow_format(enum ofputil_protocol);
+enum ofputil_protocol ofputil_decode_nx_set_flow_format(
+ const struct ofp_header *);
+
+struct ofpbuf *ofputil_encode_nx_flow_mod_table_id(bool enable);
+bool ofputil_decode_nx_flow_mod_table_id(const struct ofp_header *);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/ofp-flow.c b/lib/ofp-flow.c
index c29f5b7cb05f..af6be74909e6 100644
--- a/lib/ofp-flow.c
+++ b/lib/ofp-flow.c
@@ -34,77 +34,7 @@
VLOG_DEFINE_THIS_MODULE(ofp_flow);
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-
-/* Returns an NXT_SET_FLOW_FORMAT message that can be used to set the flow
- * format to 'nxff'. */
-struct ofpbuf *
-ofputil_encode_nx_set_flow_format(enum nx_flow_format nxff)
-{
- struct nx_set_flow_format *sff;
- struct ofpbuf *msg;
-
- ovs_assert(ofputil_nx_flow_format_is_valid(nxff));
-
- msg = ofpraw_alloc(OFPRAW_NXT_SET_FLOW_FORMAT, OFP10_VERSION, 0);
- sff = ofpbuf_put_zeros(msg, sizeof *sff);
- sff->format = htonl(nxff);
-
- return msg;
-}
-
-/* Returns the base protocol if 'flow_format' is a valid NXFF_* value, false
- * otherwise. */
-enum ofputil_protocol
-ofputil_nx_flow_format_to_protocol(enum nx_flow_format flow_format)
-{
- switch (flow_format) {
- case NXFF_OPENFLOW10:
- return OFPUTIL_P_OF10_STD;
-
- case NXFF_NXM:
- return OFPUTIL_P_OF10_NXM;
-
- default:
- return 0;
- }
-}
-
-/* Returns true if 'flow_format' is a valid NXFF_* value, false otherwise. */
-bool
-ofputil_nx_flow_format_is_valid(enum nx_flow_format flow_format)
-{
- return ofputil_nx_flow_format_to_protocol(flow_format) != 0;
-}
-
-/* Returns a string version of 'flow_format', which must be a valid NXFF_*
- * value. */
-const char *
-ofputil_nx_flow_format_to_string(enum nx_flow_format flow_format)
-{
- switch (flow_format) {
- case NXFF_OPENFLOW10:
- return "openflow10";
- case NXFF_NXM:
- return "nxm";
- default:
- OVS_NOT_REACHED();
- }
-}
-
-/* Returns an OpenFlow message that can be used to turn the flow_mod_table_id
- * extension on or off (according to 'flow_mod_table_id'). */
-struct ofpbuf *
-ofputil_make_flow_mod_table_id(bool flow_mod_table_id)
-{
- struct nx_flow_mod_table_id *nfmti;
- struct ofpbuf *msg;
-
- msg = ofpraw_alloc(OFPRAW_NXT_FLOW_MOD_TABLE_ID, OFP10_VERSION, 0);
- nfmti = ofpbuf_put_zeros(msg, sizeof *nfmti);
- nfmti->set = flow_mod_table_id;
- return msg;
-}
-
+
struct ofputil_flow_mod_flag {
uint16_t raw_flag;
enum ofp_version min_version, max_version;
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 2f96b1863928..b13bc380386a 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2267,26 +2267,21 @@ ofp_print_role_status_message(struct ds *string, const struct ofp_header *oh)
}
static enum ofperr
-ofp_print_nxt_flow_mod_table_id(struct ds *string,
- const struct nx_flow_mod_table_id *nfmti)
+ofp_print_nxt_flow_mod_table_id(struct ds *string, const struct ofp_header *oh)
{
- ds_put_format(string, " %s", nfmti->set ? "enable" : "disable");
+ bool enable = ofputil_decode_nx_flow_mod_table_id(oh);
+ ds_put_format(string, " %s", enable ? "enable" : "disable");
return 0;
}
static enum ofperr
-ofp_print_nxt_set_flow_format(struct ds *string,
- const struct nx_set_flow_format *nsff)
+ofp_print_nxt_set_flow_format(struct ds *string, const struct ofp_header *oh)
{
- uint32_t format = ntohl(nsff->format);
-
- ds_put_cstr(string, " format=");
- if (ofputil_nx_flow_format_is_valid(format)) {
- ds_put_cstr(string, ofputil_nx_flow_format_to_string(format));
- } else {
- ds_put_format(string, "%"PRIu32, format);
- }
-
+ enum ofputil_protocol p = ofputil_decode_nx_set_flow_format(oh);
+ ds_put_format(string, " format=%s",
+ p == OFPUTIL_P_OF10_STD ? "openflow10"
+ : p == OFPUTIL_P_OF10_NXM ? "nxm"
+ : "(unknown)");
return 0;
}
@@ -3732,10 +3727,10 @@ ofp_to_string__(const struct ofp_header *oh,
return ofp_print_ofpst_port_desc_reply(string, oh);
case OFPTYPE_FLOW_MOD_TABLE_ID:
- return ofp_print_nxt_flow_mod_table_id(string, ofpmsg_body(oh));
+ return ofp_print_nxt_flow_mod_table_id(string, oh);
case OFPTYPE_SET_FLOW_FORMAT:
- return ofp_print_nxt_set_flow_format(string, ofpmsg_body(oh));
+ return ofp_print_nxt_set_flow_format(string, oh);
case OFPTYPE_SET_PACKET_IN_FORMAT:
return ofp_print_nxt_set_packet_in_format(string, ofpmsg_body(oh));
diff --git a/lib/ofp-protocol.c b/lib/ofp-protocol.c
index 175318380f3b..32cf1c3fca40 100644
--- a/lib/ofp-protocol.c
+++ b/lib/ofp-protocol.c
@@ -19,11 +19,15 @@
#include <ctype.h>
#include "openvswitch/dynamic-string.h"
#include "openvswitch/ofp-flow.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofpbuf.h"
#include "openvswitch/vlog.h"
#include "util.h"
VLOG_DEFINE_THIS_MODULE(ofp_protocol);
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+
/* Protocols. */
struct proto_abbrev {
@@ -555,7 +559,7 @@ ofputil_format_version_bitmap_names(struct ds *msg, uint32_t bitmap)
{
ofputil_format_version_bitmap__(msg, bitmap, ofputil_format_version_name);
}
-
+
/* Returns an OpenFlow message that, sent on an OpenFlow connection whose
* protocol is 'current', at least partly transitions the protocol to 'want'.
* Stores in '*next' the protocol that will be in effect on the OpenFlow
@@ -585,13 +589,10 @@ ofputil_encode_set_protocol(enum ofputil_protocol current,
want_base = ofputil_protocol_to_base(want);
if (cur_base != want_base) {
*next = ofputil_protocol_set_base(current, want_base);
-
switch (want_base) {
case OFPUTIL_P_OF10_NXM:
- return ofputil_encode_nx_set_flow_format(NXFF_NXM);
-
case OFPUTIL_P_OF10_STD:
- return ofputil_encode_nx_set_flow_format(NXFF_OPENFLOW10);
+ return ofputil_encode_nx_set_flow_format(want_base);
case OFPUTIL_P_OF11_STD:
case OFPUTIL_P_OF12_OXM:
@@ -613,7 +614,7 @@ ofputil_encode_set_protocol(enum ofputil_protocol current,
want_tid = (want & OFPUTIL_P_TID) != 0;
if (cur_tid != want_tid) {
*next = ofputil_protocol_set_tid(current, want_tid);
- return ofputil_make_flow_mod_table_id(want_tid);
+ return ofputil_encode_nx_flow_mod_table_id(want_tid);
}
ovs_assert(current == want);
@@ -621,3 +622,116 @@ ofputil_encode_set_protocol(enum ofputil_protocol current,
*next = current;
return NULL;
}
+
+enum nx_flow_format {
+ NXFF_OPENFLOW10 = 0, /* Standard OpenFlow 1.0 compatible. */
+ NXFF_NXM = 2 /* Nicira extended match. */
+};
+
+/* Returns an NXT_SET_FLOW_FORMAT message that can be used to set the flow
+ * format to 'nxff'. */
+struct ofpbuf *
+ofputil_encode_nx_set_flow_format(enum ofputil_protocol protocol)
+{
+ struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_SET_FLOW_FORMAT,
+ OFP10_VERSION, 0);
+ ovs_be32 *nxff = ofpbuf_put_uninit(msg, sizeof *nxff);
+ if (protocol == OFPUTIL_P_OF10_STD) {
+ *nxff = htonl(NXFF_OPENFLOW10);
+ } else if (protocol == OFPUTIL_P_OF10_NXM) {
+ *nxff = htonl(NXFF_NXM);
+ } else {
+ OVS_NOT_REACHED();
+ }
+
+ return msg;
+}
+
+/* Returns the protocol specified in the NXT_SET_FLOW_FORMAT message at 'oh'
+ * (either OFPUTIL_P_OF10_STD or OFPUTIL_P_OF10_NXM) or 0 if the message is
+ * invalid. */
+enum ofputil_protocol
+ofputil_decode_nx_set_flow_format(const struct ofp_header *oh)
+{
+ struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
+ ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_NXT_SET_FLOW_FORMAT);
+
+ ovs_be32 *flow_formatp = ofpbuf_pull(&b, sizeof *flow_formatp);
+ uint32_t flow_format = ntohl(*flow_formatp);
+ switch (flow_format) {
+ case NXFF_OPENFLOW10:
+ return OFPUTIL_P_OF10_STD;
+
+ case NXFF_NXM:
+ return OFPUTIL_P_OF10_NXM;
+
+ default:
+ VLOG_WARN_RL(&rl, "NXT_SET_FLOW_FORMAT message specified invalid "
+ "flow format %"PRIu32, flow_format);
+ return 0;
+ }
+}
+
+/* These functions work with the Open vSwitch extension feature called
+ * "flow_mod_table_id", which allows a controller to specify the OpenFlow table
+ * to which a flow should be added, instead of having the switch decide which
+ * table is most appropriate as required by OpenFlow 1.0. Because NXM was
+ * designed as an extension to OpenFlow 1.0, the extension applies equally to
+ * ofp10_flow_mod and nx_flow_mod. By default, the extension is disabled.
+ *
+ * When this feature is enabled, Open vSwitch treats struct ofp10_flow_mod's
+ * and struct nx_flow_mod's 16-bit 'command' member as two separate fields.
+ * The upper 8 bits are used as the table ID, the lower 8 bits specify the
+ * command as usual. A table ID of 0xff is treated like a wildcarded table ID.
+ *
+ * The specific treatment of the table ID depends on the type of flow mod:
+ *
+ * - OFPFC_ADD: Given a specific table ID, the flow is always placed in that
+ * table. If an identical flow already exists in that table only, then it
+ * is replaced. If the flow cannot be placed in the specified table,
+ * either because the table is full or because the table cannot support
+ * flows of the given type, the switch replies with an OFPFMFC_TABLE_FULL
+ * error. (A controller can distinguish these cases by comparing the
+ * current and maximum number of entries reported in ofp_table_stats.)
+ *
+ * If the table ID is wildcarded, the switch picks an appropriate table
+ * itself. If an identical flow already exist in the selected flow table,
+ * then it is replaced. The choice of table might depend on the flows
+ * that are already in the switch; for example, if one table fills up then
+ * the switch might fall back to another one.
+ *
+ * - OFPFC_MODIFY, OFPFC_DELETE: Given a specific table ID, only flows
+ * within that table are matched and modified or deleted. If the table ID
+ * is wildcarded, flows within any table may be matched and modified or
+ * deleted.
+ *
+ * - OFPFC_MODIFY_STRICT, OFPFC_DELETE_STRICT: Given a specific table ID,
+ * only a flow within that table may be matched and modified or deleted.
+ * If the table ID is wildcarded and exactly one flow within any table
+ * matches, then it is modified or deleted; if flows in more than one
+ * table match, then none is modified or deleted.
+ */
+
+/* Returns an OpenFlow message that can be used to turn the flow_mod_table_id
+ * extension on or off (according to 'enable'). */
+struct ofpbuf *
+ofputil_encode_nx_flow_mod_table_id(bool enable)
+{
+ struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_FLOW_MOD_TABLE_ID,
+ OFP10_VERSION, 0);
+ uint8_t *p = ofpbuf_put_zeros(msg, 8);
+ *p = enable;
+ return msg;
+}
+
+/* Decodes the NXT_SET_MOD_TABLE_ID message at 'oh'. Returns the message's
+ * argument, that is, whether the flow_mod_table_id feature should be
+ * enabled. */
+bool
+ofputil_decode_nx_flow_mod_table_id(const struct ofp_header *oh)
+{
+ struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length));
+ ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_NXT_FLOW_MOD_TABLE_ID);
+ uint8_t *enable = ofpbuf_pull(&b, 8);
+ return *enable != 0;
+}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f4604209e43f..09e5cac6a0c7 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5875,12 +5875,9 @@ static enum ofperr
handle_nxt_flow_mod_table_id(struct ofconn *ofconn,
const struct ofp_header *oh)
{
- const struct nx_flow_mod_table_id *msg = ofpmsg_body(oh);
- enum ofputil_protocol cur, next;
-
- cur = ofconn_get_protocol(ofconn);
- next = ofputil_protocol_set_tid(cur, msg->set != 0);
- ofconn_set_protocol(ofconn, next);
+ bool enable = ofputil_decode_nx_flow_mod_table_id(oh);
+ enum ofputil_protocol cur = ofconn_get_protocol(ofconn);
+ ofconn_set_protocol(ofconn, ofputil_protocol_set_tid(cur, enable));
return 0;
}
@@ -5888,18 +5885,13 @@ handle_nxt_flow_mod_table_id(struct ofconn *ofconn,
static enum ofperr
handle_nxt_set_flow_format(struct ofconn *ofconn, const struct ofp_header *oh)
{
- const struct nx_set_flow_format *msg = ofpmsg_body(oh);
- enum ofputil_protocol cur, next;
- enum ofputil_protocol next_base;
-
- next_base = ofputil_nx_flow_format_to_protocol(ntohl(msg->format));
+ enum ofputil_protocol next_base = ofputil_decode_nx_set_flow_format(oh);
if (!next_base) {
return OFPERR_OFPBRC_EPERM;
}
- cur = ofconn_get_protocol(ofconn);
- next = ofputil_protocol_set_base(cur, next_base);
- ofconn_set_protocol(ofconn, next);
+ enum ofputil_protocol cur = ofconn_get_protocol(ofconn);
+ ofconn_set_protocol(ofconn, ofputil_protocol_set_base(cur, next_base));
return 0;
}
--
2.16.1
More information about the dev
mailing list