As per spec, make packet-in reason for OpenFlow1.3 table-miss flow entries no_match rather than action.
Signed-off-by: YAMAMOTO Takashi <yamam...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev