From 0fb7792ab3428a28044e53b443388cbc1231035a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 22 Oct 2013 16:16:31 -0700 Subject: [PATCH] ofproto, ofp-util: Begin disentangling packet-in wire format and handling. struct ofputil_packet_in mixes data included in OpenFlow packet_in messages with data that used internally by ofproto and connmgr to queue and route packet_ins. This commit begins disentangling these purposes by adding a new struct ofproto_packet_in that wraps struct ofputil_packet_in. Adding this new level of indirection causes a lot of code churn, so this commit mainly takes care of that to make the remaining changes easier to read. This commit does move the list node used for queuing packet_ins into the new wrapper structure. Signed-off-by: Ben Pfaff --- lib/ofp-util.h | 2 -- ofproto/connmgr.c | 40 ++++++++++++++++++----------------- ofproto/connmgr.h | 12 +++++++---- ofproto/fail-open.c | 12 +++++------ ofproto/ofproto-dpif-upcall.c | 19 +++++++++-------- ofproto/ofproto-dpif-xlate.c | 20 +++++++++--------- ofproto/ofproto-dpif.c | 12 +++++------ ofproto/ofproto-dpif.h | 3 ++- 8 files changed, 63 insertions(+), 57 deletions(-) diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 73555599c..2de6e769f 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -376,8 +376,6 @@ struct ofpbuf *ofputil_encode_flow_removed(const struct ofputil_flow_removed *, /* Abstract packet-in message. */ struct ofputil_packet_in { - struct list list_node; /* For queueing packet_ins. */ - const void *packet; size_t packet_len; diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 8bb96f028..2c325256b 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -1435,7 +1435,7 @@ 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 ofproto_packet_in); /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate * controllers managed by 'mgr'. */ @@ -1488,13 +1488,13 @@ connmgr_send_flow_removed(struct connmgr *mgr, * The caller doesn't need to fill in pin->buffer_id or pin->total_len. */ void connmgr_send_packet_in(struct connmgr *mgr, - const struct ofputil_packet_in *pin) + const struct ofproto_packet_in *pin) { struct ofconn *ofconn; LIST_FOR_EACH (ofconn, node, &mgr->all_conns) { - if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->reason) - && ofconn->controller_id == pin->controller_id) { + if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason) + && ofconn->controller_id == pin->up.controller_id) { schedule_packet_in(ofconn, *pin); } } @@ -1513,15 +1513,15 @@ 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 ofproto_packet_in pin) { struct connmgr *mgr = ofconn->connmgr; uint16_t controller_max_len; - pin.total_len = pin.packet_len; + pin.up.total_len = pin.up.packet_len; - if (pin.reason == OFPR_ACTION) { - controller_max_len = pin.send_len; /* max_len */ + if (pin.up.reason == OFPR_ACTION) { + controller_max_len = pin.up.send_len; /* max_len */ } else { controller_max_len = ofconn->miss_send_len; } @@ -1530,31 +1530,33 @@ schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin) * For OpenFlow 1.2+, OFPCML_NO_BUFFER (== UINT16_MAX) specifies * unbuffered. This behaviour doesn't violate prior versions, too. */ if (controller_max_len == UINT16_MAX) { - pin.buffer_id = UINT32_MAX; + pin.up.buffer_id = UINT32_MAX; } else if (mgr->fail_open && fail_open_is_active(mgr->fail_open)) { - pin.buffer_id = pktbuf_get_null(); + pin.up.buffer_id = pktbuf_get_null(); } else if (!ofconn->pktbuf) { - pin.buffer_id = UINT32_MAX; + pin.up.buffer_id = UINT32_MAX; } else { - pin.buffer_id = pktbuf_save(ofconn->pktbuf, pin.packet, pin.packet_len, - pin.fmd.in_port); + pin.up.buffer_id = pktbuf_save(ofconn->pktbuf, + pin.up.packet, pin.up.packet_len, + pin.up.fmd.in_port); } /* Figure out how much of the packet to send. * If not buffered, send the entire packet. Otherwise, depending on * the reason of packet-in, send what requested by the controller. */ - if (pin.buffer_id == UINT32_MAX) { - pin.send_len = pin.packet_len; + if (pin.up.buffer_id == UINT32_MAX) { + pin.up.send_len = pin.up.packet_len; } else { - pin.send_len = MIN(pin.packet_len, controller_max_len); + pin.up.send_len = MIN(pin.up.packet_len, controller_max_len); } /* Make OFPT_PACKET_IN and hand over to packet scheduler. It might * immediately call into do_send_packet_in() or it might buffer it for a * while (until a later call to pinsched_run()). */ - pinsched_send(ofconn->schedulers[pin.reason == OFPR_NO_MATCH ? 0 : 1], - pin.fmd.in_port, - ofputil_encode_packet_in(&pin, ofconn_get_protocol(ofconn), + pinsched_send(ofconn->schedulers[pin.up.reason == OFPR_NO_MATCH ? 0 : 1], + pin.up.fmd.in_port, + ofputil_encode_packet_in(&pin.up, + ofconn_get_protocol(ofconn), ofconn->packet_in_format), do_send_packet_in, ofconn); } diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 505a757ee..4846da9c8 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -22,6 +22,7 @@ #include "list.h" #include "match.h" #include "ofp-errors.h" +#include "ofp-util.h" #include "ofproto.h" #include "openflow/nicira-ext.h" #include "openvswitch/types.h" @@ -29,9 +30,6 @@ struct nlattr; struct ofconn; struct ofopgroup; -struct ofputil_flow_removed; -struct ofputil_packet_in; -struct ofputil_phy_port; struct rule; struct simap; struct sset; @@ -64,6 +62,12 @@ enum ofconn_async_msg_type { OAM_N_TYPES }; +/* A packet_in, with extra members to assist in queuing and routing it. */ +struct ofproto_packet_in { + struct ofputil_packet_in up; + struct list list_node; /* For queuing. */ +}; + /* Basics. */ struct connmgr *connmgr_create(struct ofproto *ofproto, const char *dpif_name, const char *local_name); @@ -141,7 +145,7 @@ void connmgr_send_port_status(struct connmgr *, void connmgr_send_flow_removed(struct connmgr *, const struct ofputil_flow_removed *); void connmgr_send_packet_in(struct connmgr *, - const struct ofputil_packet_in *); + const struct ofproto_packet_in *); /* Fail-open settings. */ enum ofproto_fail_mode connmgr_get_fail_mode(const struct connmgr *); diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c index 4900c0507..40859a4e8 100644 --- a/ofproto/fail-open.c +++ b/ofproto/fail-open.c @@ -116,7 +116,7 @@ fail_open_is_active(const struct fail_open *fo) static void send_bogus_packet_ins(struct fail_open *fo) { - struct ofputil_packet_in pin; + struct ofproto_packet_in pin; uint8_t mac[ETH_ADDR_LEN]; struct ofpbuf b; @@ -125,11 +125,11 @@ send_bogus_packet_ins(struct fail_open *fo) compose_rarp(&b, mac); memset(&pin, 0, sizeof pin); - pin.packet = b.data; - pin.packet_len = b.size; - pin.reason = OFPR_NO_MATCH; - pin.send_len = b.size; - pin.fmd.in_port = OFPP_LOCAL; + pin.up.packet = b.data; + pin.up.packet_len = b.size; + pin.up.reason = OFPR_NO_MATCH; + pin.up.send_len = b.size; + pin.up.fmd.in_port = OFPP_LOCAL; connmgr_send_packet_in(fo->connmgr, &pin); ofpbuf_uninit(&b); diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 954e92fac..41d8ee4aa 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -19,6 +19,7 @@ #include #include +#include "connmgr.h" #include "coverage.h" #include "dynamic-string.h" #include "dpif.h" @@ -838,17 +839,17 @@ handle_upcalls(struct udpif *udpif, struct list *upcalls) LIST_FOR_EACH (upcall, list_node, upcalls) { struct flow_miss *miss = upcall->flow_miss; struct ofpbuf *packet = upcall->dpif_upcall.packet; - struct ofputil_packet_in *pin; + struct ofproto_packet_in *pin; pin = xmalloc(sizeof *pin); - pin->packet = xmemdup(packet->data, packet->size); - pin->packet_len = packet->size; - pin->reason = OFPR_NO_MATCH; - pin->controller_id = 0; - pin->table_id = 0; - pin->cookie = 0; - pin->send_len = 0; /* Not used for flow table misses. */ - flow_get_metadata(&miss->flow, &pin->fmd); + pin->up.packet = xmemdup(packet->data, packet->size); + pin->up.packet_len = packet->size; + pin->up.reason = OFPR_NO_MATCH; + pin->up.controller_id = 0; + pin->up.table_id = 0; + pin->up.cookie = 0; + pin->up.send_len = 0; /* Not used for flow table misses. */ + flow_get_metadata(&miss->flow, &pin->up.fmd); ofproto_dpif_send_packet_in(miss->ofproto, pin); } } diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 737175033..a7026ba63 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1820,7 +1820,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len, enum ofp_packet_in_reason reason, uint16_t controller_id) { - struct ofputil_packet_in *pin; + struct ofproto_packet_in *pin; struct ofpbuf *packet; struct flow key; @@ -1844,15 +1844,15 @@ execute_controller_action(struct xlate_ctx *ctx, int len, ctx->xout->odp_actions.size, NULL, NULL); pin = xmalloc(sizeof *pin); - pin->packet_len = packet->size; - pin->packet = ofpbuf_steal_data(packet); - pin->reason = reason; - pin->controller_id = controller_id; - pin->table_id = ctx->table_id; - pin->cookie = ctx->rule ? rule_dpif_get_flow_cookie(ctx->rule) : 0; - - pin->send_len = len; - flow_get_metadata(&ctx->xin->flow, &pin->fmd); + pin->up.packet_len = packet->size; + pin->up.packet = ofpbuf_steal_data(packet); + pin->up.reason = reason; + pin->up.controller_id = controller_id; + pin->up.table_id = ctx->table_id; + pin->up.cookie = ctx->rule ? rule_dpif_get_flow_cookie(ctx->rule) : 0; + + pin->up.send_len = len; + flow_get_metadata(&ctx->xin->flow, &pin->up.fmd); ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin); ofpbuf_delete(packet); diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 90ce71559..f53384d62 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -545,11 +545,11 @@ ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto, * Takes ownership of 'pin' and pin->packet. */ void ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto, - struct ofputil_packet_in *pin) + struct ofproto_packet_in *pin) { if (!guarded_list_push_back(&ofproto->pins, &pin->list_node, 1024)) { COVERAGE_INC(packet_in_overflow); - free(CONST_CAST(void *, pin->packet)); + free(CONST_CAST(void *, pin->up.packet)); free(pin); } } @@ -1359,7 +1359,7 @@ destruct(struct ofproto *ofproto_) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); struct rule_dpif *rule, *next_rule; - struct ofputil_packet_in *pin, *next_pin; + struct ofproto_packet_in *pin, *next_pin; struct facet *facet, *next_facet; struct cls_cursor cursor; struct oftable *table; @@ -1397,7 +1397,7 @@ destruct(struct ofproto *ofproto_) guarded_list_pop_all(&ofproto->pins, &pins); LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) { list_remove(&pin->list_node); - free(CONST_CAST(void *, pin->packet)); + free(CONST_CAST(void *, pin->up.packet)); free(pin); } guarded_list_destroy(&ofproto->pins); @@ -1428,7 +1428,7 @@ static int run_fast(struct ofproto *ofproto_) { struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_); - struct ofputil_packet_in *pin, *next_pin; + struct ofproto_packet_in *pin, *next_pin; struct list pins; /* Do not perform any periodic activity required by 'ofproto' while @@ -1441,7 +1441,7 @@ run_fast(struct ofproto *ofproto_) LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) { connmgr_send_packet_in(ofproto->up.connmgr, pin); list_remove(&pin->list_node); - free(CONST_CAST(void *, pin->packet)); + free(CONST_CAST(void *, pin->up.packet)); free(pin); } diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 51b197922..0f9603189 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -28,6 +28,7 @@ union user_action_cookie; struct dpif_flow_stats; struct ofproto_dpif; +struct ofproto_packet_in; struct ofport_dpif; struct dpif_backer; struct OVS_LOCKABLE rule_dpif; @@ -97,7 +98,7 @@ int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *, struct rule_dpif *, const struct ofpact *, size_t ofpacts_len, struct ofpbuf *); void ofproto_dpif_send_packet_in(struct ofproto_dpif *, - struct ofputil_packet_in *pin); + struct ofproto_packet_in *); int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *); void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *); -- 2.43.0