[ovs-dev] [PATCH] Fix packet-in reason for OpenFlow1.3 table-miss flow entries
YAMAMOTO Takashi
yamamoto at valinux.co.jp
Fri Oct 18 04:28:44 UTC 2013
As per spec, make packet-in reason for OpenFlow1.3 table-miss flow
entries no_match rather than action.
Signed-off-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
---
OPENFLOW-1.1+ | 3 --
include/openflow/openflow-common.h | 5 +-
lib/learn.c | 6 ++-
lib/learning-switch.c | 6 ++-
lib/ofp-actions.c | 64 +++++++++++++-----------
lib/ofp-actions.h | 1 +
lib/ofp-parse.c | 12 ++++-
lib/ofp-util.c | 100 +++++++++++++++++++++++++++++++++++++
lib/ofp-util.h | 1 +
ofproto/connmgr.c | 34 +++++++++++--
ofproto/ofproto-dpif-xlate.c | 18 ++++---
11 files changed, 201 insertions(+), 49 deletions(-)
diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index 4f30520..2405bb4 100644
--- a/OPENFLOW-1.1+
+++ b/OPENFLOW-1.1+
@@ -125,9 +125,6 @@ didn't compare the specs carefully yet.)
- Change the default table-miss action (in the absense of table-miss
entry) from packet_in to drop for OF1.3+. Decide what to do if
a switch is configured to support multiple OF versions.
- - Distinguish table-miss flow entry and make its packet_in reason
- OFPR_NO_MATCH. (OFPR_TABLE_MISS for OF1.4)
- Also, make it use the appropriate pin scheduler.
[required for OF1.3+]
* IPv6 extension header handling support. Fully implementing this
diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index 45d03ef..7f73be2 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -275,7 +275,10 @@ enum ofp_packet_in_reason {
OFPR_NO_MATCH, /* No matching flow. */
OFPR_ACTION, /* Action explicitly output to controller. */
OFPR_INVALID_TTL /* Packet has invalid TTL. */,
- OFPR_N_REASONS
+ OFPR_N_REASONS,
+
+ /* Action for OF1.3+ table-miss flow entry. Internal use only. */
+ OFPR_ACTION_TABLE_MISS = 999,
};
enum ofp_flow_mod_command {
diff --git a/lib/learn.c b/lib/learn.c
index 61799c9..d12d8ea 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -374,7 +374,11 @@ learn_execute(const struct ofpact_learn *learn, const struct flow *flow,
|| port == OFPP_FLOOD
|| port == OFPP_LOCAL
|| port == OFPP_ALL) {
- ofpact_put_OUTPUT(ofpacts)->port = port;
+ struct ofpact_output *output = ofpact_put_OUTPUT(ofpacts);
+
+ output->port = port;
+ output->max_len = 0;
+ output->reason = OFPR_ACTION;
}
}
break;
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index b133637..c0fb428 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -587,7 +587,11 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh)
/* No actions. */
} else if (queue_id == UINT32_MAX
|| ofp_to_u16(out_port) >= ofp_to_u16(OFPP_MAX)) {
- ofpact_put_OUTPUT(&ofpacts)->port = out_port;
+ struct ofpact_output *output = ofpact_put_OUTPUT(&ofpacts);
+
+ output->port = out_port;
+ output->max_len = 0;
+ output->reason = OFPR_ACTION;
} else {
struct ofpact_enqueue *enqueue = ofpact_put_ENQUEUE(&ofpacts);
enqueue->port = out_port;
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 79b0433..f07b98e 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -57,6 +57,7 @@ output_from_openflow10(const struct ofp10_action_output *oao,
output = ofpact_put_OUTPUT(out);
output->port = u16_to_ofp(ntohs(oao->port));
output->max_len = ntohs(oao->max_len);
+ output->reason = OFPR_ACTION;
return ofputil_check_output_port(output->port, OFPP_MAX);
}
@@ -742,7 +743,7 @@ output_from_openflow11(const struct ofp11_action_output *oao,
output = ofpact_put_OUTPUT(out);
output->max_len = ntohs(oao->max_len);
-
+ output->reason = OFPR_ACTION;
error = ofputil_port_from_ofp11(oao->port, &output->port);
if (error) {
return error;
@@ -2476,11 +2477,38 @@ print_fin_timeout(const struct ofpact_fin_timeout *fin_timeout,
}
static void
+ofpact_format_controller(uint16_t max_len, enum ofp_packet_in_reason reason,
+ uint16_t controller_id, struct ds *s)
+{
+ if (reason == OFPR_ACTION && controller_id == 0) {
+ ds_put_format(s, "CONTROLLER:%"PRIu16, max_len);
+ } else {
+ ds_put_cstr(s, "controller(");
+ if (reason != OFPR_ACTION) {
+ char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
+
+ ds_put_format(s, "reason=%s,",
+ ofputil_packet_in_reason_to_string(
+ reason, reasonbuf, sizeof reasonbuf));
+ }
+ if (max_len != UINT16_MAX) {
+ ds_put_format(s, "max_len=%"PRIu16",", max_len);
+ }
+ if (controller_id != 0) {
+ ds_put_format(s, "id=%"PRIu16",", controller_id);
+ }
+ ds_chomp(s, ',');
+ ds_put_char(s, ')');
+ }
+}
+
+static void
ofpact_format(const struct ofpact *a, struct ds *s)
{
const struct ofpact_enqueue *enqueue;
const struct ofpact_resubmit *resubmit;
const struct ofpact_controller *controller;
+ const struct ofpact_output *output;
const struct ofpact_metadata *metadata;
const struct ofpact_tunnel *tunnel;
const struct ofpact_sample *sample;
@@ -2488,43 +2516,21 @@ ofpact_format(const struct ofpact *a, struct ds *s)
switch (a->type) {
case OFPACT_OUTPUT:
- port = ofpact_get_OUTPUT(a)->port;
+ output = ofpact_get_OUTPUT(a);
+ port = output->port;
if (ofp_to_u16(port) < ofp_to_u16(OFPP_MAX)) {
ds_put_format(s, "output:%"PRIu16, port);
+ } else if (port == OFPP_CONTROLLER) {
+ ofpact_format_controller(output->max_len, output->reason, 0, s);
} else {
ofputil_format_port(port, s);
- if (port == OFPP_CONTROLLER) {
- ds_put_format(s, ":%"PRIu16, ofpact_get_OUTPUT(a)->max_len);
- }
}
break;
case OFPACT_CONTROLLER:
controller = ofpact_get_CONTROLLER(a);
- if (controller->reason == OFPR_ACTION &&
- controller->controller_id == 0) {
- ds_put_format(s, "CONTROLLER:%"PRIu16,
- ofpact_get_CONTROLLER(a)->max_len);
- } else {
- enum ofp_packet_in_reason reason = controller->reason;
-
- ds_put_cstr(s, "controller(");
- if (reason != OFPR_ACTION) {
- char reasonbuf[OFPUTIL_PACKET_IN_REASON_BUFSIZE];
-
- ds_put_format(s, "reason=%s,",
- ofputil_packet_in_reason_to_string(
- reason, reasonbuf, sizeof reasonbuf));
- }
- if (controller->max_len != UINT16_MAX) {
- ds_put_format(s, "max_len=%"PRIu16",", controller->max_len);
- }
- if (controller->controller_id != 0) {
- ds_put_format(s, "id=%"PRIu16",", controller->controller_id);
- }
- ds_chomp(s, ',');
- ds_put_char(s, ')');
- }
+ ofpact_format_controller(controller->max_len, controller->reason,
+ controller->controller_id, s);
break;
case OFPACT_ENQUEUE:
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 5996651..5d4aa9f 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -203,6 +203,7 @@ struct ofpact_output {
struct ofpact ofpact;
ofp_port_t port; /* Output port. */
uint16_t max_len; /* Max send len, for port OFPP_CONTROLLER. */
+ enum ofp_packet_in_reason reason; /* Reason to put in packet-in. */
};
/* OFPACT_CONTROLLER.
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 17bd7e2..6f3d797 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -213,6 +213,7 @@ parse_output(const char *arg, struct ofpbuf *ofpacts)
output = ofpact_put_OUTPUT(ofpacts);
output->max_len = output->port == OFPP_CONTROLLER ? UINT16_MAX : 0;
+ output->reason = OFPR_ACTION;
if (!ofputil_port_from_string(arg, &output->port)) {
return xasprintf("%s: output to unknown port", arg);
}
@@ -374,12 +375,13 @@ parse_controller(struct ofpbuf *b, char *arg)
}
}
- if (reason == OFPR_ACTION && controller_id == 0) {
+ if (controller_id == 0) {
struct ofpact_output *output;
output = ofpact_put_OUTPUT(b);
output->port = OFPP_CONTROLLER;
output->max_len = max_len;
+ output->reason = reason;
} else {
struct ofpact_controller *controller;
@@ -866,7 +868,11 @@ str_to_ofpact__(char *pos, char *act, char *arg,
} else {
ofp_port_t port;
if (ofputil_port_from_string(act, &port)) {
- ofpact_put_OUTPUT(ofpacts)->port = port;
+ struct ofpact_output *output = ofpact_put_OUTPUT(ofpacts);
+
+ output->port = port;
+ output->max_len = 0;
+ output->reason = OFPR_ACTION;
} else {
return xasprintf("Unknown action: %s", act);
}
@@ -1733,6 +1739,8 @@ parse_ofp_flow_mod_str(struct ofputil_flow_mod *fm, const char *string,
* that the switch can do whatever it likes with the flow. */
struct match match_copy = fm->match;
ofputil_normalize_match(&match_copy);
+
+ ofputil_mark_table_miss(fm);
}
return error;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 2bf595a..5c24b87 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1470,6 +1470,101 @@ ofputil_encode_flow_mod_flags(enum ofputil_flow_mod_flags flags,
return htons(raw_flags);
}
+static void
+mark_table_miss_actions(struct ofpact *ofpacts, size_t ofpacts_len)
+{
+ struct ofpact *a;
+
+ OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+ struct ofpact_output *output;
+ struct ofpact_nest *on;
+
+ switch (a->type) {
+ case OFPACT_OUTPUT:
+ output = ofpact_get_OUTPUT(a);
+
+ if (output->reason == OFPR_ACTION) {
+ output->reason = OFPR_ACTION_TABLE_MISS;
+ }
+ break;
+ case OFPACT_WRITE_ACTIONS:
+ on = ofpact_get_WRITE_ACTIONS(a);
+ mark_table_miss_actions(on->actions,
+ ofpact_nest_get_action_len(on));
+ break;
+ case OFPACT_BUNDLE:
+ case OFPACT_CLEAR_ACTIONS:
+ case OFPACT_CONTROLLER:
+ case OFPACT_DEC_MPLS_TTL:
+ case OFPACT_DEC_TTL:
+ case OFPACT_ENQUEUE:
+ case OFPACT_EXIT:
+ case OFPACT_FIN_TIMEOUT:
+ case OFPACT_GOTO_TABLE:
+ case OFPACT_GROUP:
+ case OFPACT_LEARN:
+ case OFPACT_METER:
+ case OFPACT_MULTIPATH:
+ case OFPACT_NOTE:
+ case OFPACT_OUTPUT_REG:
+ case OFPACT_POP_MPLS:
+ case OFPACT_POP_QUEUE:
+ case OFPACT_PUSH_MPLS:
+ case OFPACT_PUSH_VLAN:
+ case OFPACT_REG_LOAD:
+ case OFPACT_REG_MOVE:
+ case OFPACT_RESUBMIT:
+ case OFPACT_SAMPLE:
+ case OFPACT_SET_ETH_DST:
+ case OFPACT_SET_ETH_SRC:
+ case OFPACT_SET_IPV4_DSCP:
+ case OFPACT_SET_IPV4_DST:
+ case OFPACT_SET_IPV4_SRC:
+ case OFPACT_SET_L4_DST_PORT:
+ case OFPACT_SET_L4_SRC_PORT:
+ case OFPACT_SET_MPLS_TTL:
+ case OFPACT_SET_QUEUE:
+ case OFPACT_SET_TUNNEL:
+ case OFPACT_SET_VLAN_PCP:
+ case OFPACT_SET_VLAN_VID:
+ case OFPACT_STACK_POP:
+ case OFPACT_STACK_PUSH:
+ case OFPACT_STRIP_VLAN:
+ case OFPACT_WRITE_METADATA:
+ break;
+ }
+ }
+}
+
+/* Check if this looks like OpenFlow1.3+ table-miss flow entry.
+ * If so, mark output actions in fm->ofpacts so that connmgr can
+ * choose the appropriate packet-in reason for each OF channels.
+ *
+ * From OpenFlow 1.3.2 5.4 Table-miss (p.16):
+ * If the table-miss flow entry directly sends packets to
+ * the controller using the CONTROLLER reserved port (see 4.5),
+ * the packet-in reason must identify a table-miss (see 7.4.1).
+ *
+ * Our interpretation of "directly" is "not via groups".
+ *
+ * Don't check OF version here because what matters for our
+ * purpose would be the version of packet-in, not flow-mod. */
+void
+ofputil_mark_table_miss(struct ofputil_flow_mod *fm)
+{
+
+ if (fm->priority != 0) {
+ return;
+ }
+ if (fm->ofpacts == NULL) {
+ return;
+ }
+ if (!flow_wildcards_is_catchall(&fm->match.wc)) {
+ return;
+ }
+ mark_table_miss_actions(fm->ofpacts, fm->ofpacts_len);
+}
+
/* Converts an OFPT_FLOW_MOD or NXT_FLOW_MOD message 'oh' into an abstract
* flow_mod in 'fm'. Returns 0 if successful, otherwise an OpenFlow error
* code.
@@ -1648,6 +1743,7 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
: OFPERR_OFPFMFC_TABLE_FULL);
}
+ ofputil_mark_table_miss(fm);
return 0;
}
@@ -2941,6 +3037,8 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
size_t send_len = MIN(pin->send_len, pin->packet_len);
struct ofpbuf *packet;
+ ovs_assert(pin->reason != OFPR_ACTION_TABLE_MISS);
+
/* Add OFPT_PACKET_IN. */
if (protocol == OFPUTIL_P_OF13_OXM || protocol == OFPUTIL_P_OF12_OXM) {
struct ofp13_packet_in *opi;
@@ -3035,6 +3133,8 @@ ofputil_packet_in_reason_to_string(enum ofp_packet_in_reason reason,
return "action";
case OFPR_INVALID_TTL:
return "invalid_ttl";
+ case OFPR_ACTION_TABLE_MISS:
+ return "action_table_miss";
case OFPR_N_REASONS:
default:
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 937423e..4ad37fa 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -297,6 +297,7 @@ enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *,
struct ofpbuf *ofpacts);
struct ofpbuf *ofputil_encode_flow_mod(const struct ofputil_flow_mod *,
enum ofputil_protocol);
+void ofputil_mark_table_miss(struct ofputil_flow_mod *);
/* Flow stats or aggregate stats request, independent of protocol. */
struct ofputil_flow_stats_request {
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 8bb96f0..31cf582 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1435,7 +1435,8 @@ ofconn_send(const struct ofconn *ofconn, struct ofpbuf *msg,
/* Sending asynchronous messages. */
-static void schedule_packet_in(struct ofconn *, struct ofputil_packet_in);
+static void schedule_packet_in(struct ofconn *, struct ofputil_packet_in,
+ enum ofp_packet_in_reason reason);
/* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
* controllers managed by 'mgr'. */
@@ -1482,6 +1483,25 @@ connmgr_send_flow_removed(struct connmgr *mgr,
}
}
+/* Turn OFPR_ACTION_TABLE_MISS, which is OVS-internal, into
+ * an appropritate OpenFlow on-wire reason. */
+static enum ofp_packet_in_reason
+wire_reason(struct ofconn *ofconn, enum ofp_packet_in_reason reason)
+{
+ enum ofputil_protocol protocol;
+
+ if (reason != OFPR_ACTION_TABLE_MISS) {
+ return reason;
+ }
+ protocol = ofconn_get_protocol(ofconn);
+ if (protocol != OFPUTIL_P_NONE
+ && ofputil_protocol_to_ofp_version(protocol) < OFP13_VERSION) {
+ return OFPR_ACTION;
+ } else {
+ return OFPR_NO_MATCH;
+ }
+}
+
/* Given 'pin', sends an OFPT_PACKET_IN message to each OpenFlow controller as
* necessary according to their individual configurations.
*
@@ -1493,9 +1513,11 @@ connmgr_send_packet_in(struct connmgr *mgr,
struct ofconn *ofconn;
LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
- if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->reason)
+ enum ofp_packet_in_reason reason = wire_reason(ofconn, pin->reason);
+
+ if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, reason)
&& ofconn->controller_id == pin->controller_id) {
- schedule_packet_in(ofconn, *pin);
+ schedule_packet_in(ofconn, *pin, reason);
}
}
}
@@ -1513,18 +1535,20 @@ do_send_packet_in(struct ofpbuf *ofp_packet_in, void *ofconn_)
/* Takes 'pin', composes an OpenFlow packet-in message from it, and passes it
* to 'ofconn''s packet scheduler for sending. */
static void
-schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin)
+schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin,
+ enum ofp_packet_in_reason reason)
{
struct connmgr *mgr = ofconn->connmgr;
uint16_t controller_max_len;
pin.total_len = pin.packet_len;
- if (pin.reason == OFPR_ACTION) {
+ if (pin.reason == OFPR_ACTION || pin.reason == OFPR_ACTION_TABLE_MISS) {
controller_max_len = pin.send_len; /* max_len */
} else {
controller_max_len = ofconn->miss_send_len;
}
+ pin.reason = reason;
/* Get OpenFlow buffer_id.
* For OpenFlow 1.2+, OFPCML_NO_BUFFER (== UINT16_MAX) specifies
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7371750..2a9b4c8 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2030,7 +2030,9 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
static void
xlate_output_action(struct xlate_ctx *ctx,
- ofp_port_t port, uint16_t max_len, bool may_packet_in)
+ ofp_port_t port, uint16_t max_len,
+ enum ofp_packet_in_reason reason,
+ bool may_packet_in)
{
ofp_port_t prev_nf_output_iface = ctx->xout->nf_output_iface;
@@ -2054,7 +2056,7 @@ xlate_output_action(struct xlate_ctx *ctx,
flood_packets(ctx, true);
break;
case OFPP_CONTROLLER:
- execute_controller_action(ctx, max_len, OFPR_ACTION, 0);
+ execute_controller_action(ctx, max_len, reason, 0);
break;
case OFPP_NONE:
break;
@@ -2089,7 +2091,7 @@ xlate_output_reg_action(struct xlate_ctx *ctx,
memset(&value, 0xff, sizeof value);
mf_write_subfield_flow(&or->src, &value, &ctx->xout->wc.masks);
xlate_output_action(ctx, u16_to_ofp(port),
- or->max_len, false);
+ or->max_len, OFPR_ACTION, false);
}
}
@@ -2106,7 +2108,7 @@ xlate_enqueue_action(struct xlate_ctx *ctx,
error = dpif_queue_to_priority(ctx->xbridge->dpif, queue_id, &priority);
if (error) {
/* Fall back to ordinary output action. */
- xlate_output_action(ctx, enqueue->port, 0, false);
+ xlate_output_action(ctx, enqueue->port, 0, OFPR_ACTION, false);
return;
}
@@ -2179,7 +2181,7 @@ xlate_bundle_action(struct xlate_ctx *ctx,
nxm_reg_load(&bundle->dst, ofp_to_u16(port), &ctx->xin->flow,
&ctx->xout->wc);
} else {
- xlate_output_action(ctx, port, 0, false);
+ xlate_output_action(ctx, port, 0, OFPR_ACTION, false);
}
}
@@ -2287,6 +2289,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
struct ofpact_controller *controller;
+ struct ofpact_output *output;
const struct ofpact_metadata *metadata;
if (ctx->exit) {
@@ -2295,8 +2298,9 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
switch (a->type) {
case OFPACT_OUTPUT:
- xlate_output_action(ctx, ofpact_get_OUTPUT(a)->port,
- ofpact_get_OUTPUT(a)->max_len, true);
+ output = ofpact_get_OUTPUT(a);
+ xlate_output_action(ctx, output->port, output->max_len,
+ output->reason, true);
break;
case OFPACT_GROUP:
--
1.8.3.1
More information about the dev
mailing list