From 750638bbefd179b91f8fb3c9c1bde855af05cf45 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 10 Aug 2010 11:05:01 -0700 Subject: [PATCH] ofproto: Avoid ofpbuf_clone() for OFPAT_CONTROLLER common case. This additionally optimizes the common case of the first packet of a flow that consists only of an OFPAT_CONTROLLER action, by avoiding an ofpbuf_clone() call along that path. --- ofproto/ofproto.c | 63 ++++++++++++++++++++++++++++------------------- ofproto/pktbuf.c | 7 +++++- 2 files changed, 43 insertions(+), 27 deletions(-) diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index a9e270b9f..4b9ceebb7 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1927,41 +1927,44 @@ rule_has_out_port(const struct rule *rule, uint16_t out_port) return false; } +/* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on + * 'packet', which arrived on 'in_port'. + * + * Takes ownership of 'packet'. */ static bool execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, const union odp_action *actions, size_t n_actions, - const struct ofpbuf *packet) + struct ofpbuf *packet) { - if (n_actions > 0 && actions[0].type == ODPAT_CONTROLLER) { + if (n_actions == 1 && actions[0].type == ODPAT_CONTROLLER) { /* As an optimization, avoid a round-trip from userspace to kernel to * userspace. This also avoids possibly filling up kernel packet * buffers along the way. */ - struct ofpbuf *copy; struct odp_msg *msg; - copy = ofpbuf_new(DPIF_RECV_MSG_PADDING + sizeof(struct odp_msg) - + packet->size); - ofpbuf_reserve(copy, DPIF_RECV_MSG_PADDING); - msg = ofpbuf_put_uninit(copy, sizeof *msg); + msg = ofpbuf_push_uninit(packet, sizeof *msg); msg->type = _ODPL_ACTION_NR; msg->length = sizeof(struct odp_msg) + packet->size; msg->port = in_port; msg->reserved = 0; msg->arg = actions[0].controller.arg; - ofpbuf_put(copy, packet->data, packet->size); - send_packet_in(ofproto, copy); + send_packet_in(ofproto, packet); - actions++; - n_actions--; - } + return true; + } else { + int error; - return !n_actions || !dpif_execute(ofproto->dpif, in_port, - actions, n_actions, packet); + error = dpif_execute(ofproto->dpif, in_port, + actions, n_actions, packet); + ofpbuf_delete(packet); + return !error; + } } /* Executes the actions indicated by 'rule' on 'packet', which is in flow - * 'flow' and is considered to have arrived on ODP port 'in_port'. + * 'flow' and is considered to have arrived on ODP port 'in_port'. 'packet' + * must have at least sizeof(struct ofp_packet_in) bytes of headroom. * * The flow that 'packet' actually contains does not need to actually match * 'rule'; the actions in 'rule' will be applied to it either way. Likewise, @@ -1973,15 +1976,20 @@ execute_odp_actions(struct ofproto *ofproto, uint16_t in_port, * 'packet' using rule_make_actions(). If 'rule' is a wildcard rule, or if * 'rule' is an exact-match rule but 'flow' is not the rule's flow, then this * function will compose a set of ODP actions based on 'rule''s OpenFlow - * actions and apply them to 'packet'. */ + * actions and apply them to 'packet'. + * + * Takes ownership of 'packet'. */ static void rule_execute(struct ofproto *ofproto, struct rule *rule, struct ofpbuf *packet, const flow_t *flow) { const union odp_action *actions; + struct odp_flow_stats stats; size_t n_actions; struct odp_actions a; + assert(ofpbuf_headroom(packet) >= sizeof(struct ofp_packet_in)); + /* Grab or compose the ODP actions. * * The special case for an exact-match 'rule' where 'flow' is not the @@ -1992,6 +2000,7 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, struct rule *super = rule->super ? rule->super : rule; if (xlate_actions(super->actions, super->n_actions, flow, ofproto, packet, &a, NULL, 0, NULL)) { + ofpbuf_delete(packet); return; } actions = a.actions; @@ -2002,16 +2011,21 @@ rule_execute(struct ofproto *ofproto, struct rule *rule, } /* Execute the ODP actions. */ + flow_extract_stats(flow, packet, &stats); if (execute_odp_actions(ofproto, flow->in_port, actions, n_actions, packet)) { - struct odp_flow_stats stats; - flow_extract_stats(flow, packet, &stats); update_stats(ofproto, rule, &stats); rule->used = time_msec(); netflow_flow_update_time(ofproto->netflow, &rule->nf_flow, rule->used); } } +/* Inserts 'rule' into 'p''s flow table. + * + * If 'packet' is nonnull, takes ownership of 'packet', executes 'rule''s + * actions on it and credits the statistics for sending the packet to 'rule'. + * 'packet' must have at least sizeof(struct ofp_packet_in) bytes of + * headroom. */ static void rule_insert(struct ofproto *p, struct rule *rule, struct ofpbuf *packet, uint16_t in_port) @@ -3585,7 +3599,6 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, } rule_insert(p, rule, packet, in_port); - ofpbuf_delete(packet); return error; } @@ -3623,7 +3636,6 @@ send_buffered_packet(struct ofproto *ofproto, struct ofconn *ofconn, flow_extract(packet, 0, in_port, &flow); rule_execute(ofproto, rule, packet, &flow); - ofpbuf_delete(packet); return 0; } @@ -4131,9 +4143,6 @@ handle_odp_miss_msg(struct ofproto *p, struct ofpbuf *packet) } } - rule_execute(p, rule, &payload, &flow); - rule_reinstall(p, rule); - if (rule->super && rule->super->cr.priority == FAIL_OPEN_PRIORITY) { /* * Extra-special case for fail-open mode. @@ -4145,10 +4154,12 @@ handle_odp_miss_msg(struct ofproto *p, struct ofpbuf *packet) * * See the top-level comment in fail-open.c for more information. */ - send_packet_in(p, packet); - } else { - ofpbuf_delete(packet); + send_packet_in(p, ofpbuf_clone(packet)); } + + ofpbuf_pull(packet, sizeof *msg); + rule_execute(p, rule, packet, &flow); + rule_reinstall(p, rule); } static void diff --git a/ofproto/pktbuf.c b/ofproto/pktbuf.c index 49e5c4d63..67adb5606 100644 --- a/ofproto/pktbuf.c +++ b/ofproto/pktbuf.c @@ -112,7 +112,9 @@ pktbuf_save(struct pktbuf *pb, struct ofpbuf *buffer, uint16_t in_port) if (++p->cookie >= COOKIE_MAX) { p->cookie = 0; } - p->buffer = ofpbuf_clone(buffer); + p->buffer = ofpbuf_new(sizeof(struct ofp_packet_in) + buffer->size); + ofpbuf_reserve(p->buffer, sizeof(struct ofp_packet_in)); + ofpbuf_put(p->buffer, buffer->data, buffer->size); p->timeout = time_msec() + OVERWRITE_MSECS; p->in_port = in_port; return make_id(p - pb->packets, p->cookie); @@ -154,6 +156,9 @@ pktbuf_get_null(void) * identifies a "null" packet buffer (created with pktbuf_get_null()), stores * NULL in '*bufferp' and UINT16_max in '*in_port'. * + * A returned packet will have at least sizeof(struct ofp_packet_in) bytes of + * headroom. + * * On failure, stores NULL in in '*bufferp' and UINT16_MAX in '*in_port'. */ int pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp, -- 2.43.0