From d5ff77e22c97dfe93df9b98beefdcc8182dd94f1 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Fri, 16 Aug 2013 14:17:23 +0900 Subject: [PATCH] ofproto-dpif-upcall: ofproto_dpif_send_packet_in() needs object on heap. fix a bug introduced by commit e1ec7dd46 (ofproto-dpif: Implement multi-threaded miss handling.), in which execute_flow_miss() passes a stack-allocated object to ofproto_dpif_send_packet_in() whereas that function requires a heap-allocated object. Also fixes two related bugs: the 'packet' previously used in the packet-in was invalid and its data was not copied with xmemdup(). Previously, when there were multiple packets in a single flow miss, execute_flow_miss() would only send the first one to the controller. This was intentional (the goal is to find out whether the controller is operational, and sending a single packet is sufficient for that) but possibly confusing to the reader. This commit switches to sending all of the packets (the common case is a single packet anyhow). Signed-off-by: YAMAMOTO Takashi Signed-off-by: Ben Pfaff --- ofproto/ofproto-dpif-upcall.c | 41 +++++++++++++++++++---------------- ofproto/ofproto-dpif.c | 2 ++ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index ff9b2d5f2..242086584 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -639,25 +639,28 @@ execute_flow_miss(struct flow_miss *miss, struct dpif_op *ops, size_t *n_ops) flow_wildcards_or(&miss->xout.wc, &miss->xout.wc, &wc); if (rule->up.cr.priority == FAIL_OPEN_PRIORITY) { - struct ofputil_packet_in pin; - - /* Extra-special case for fail-open mode. - * - * We are in fail-open mode and the packet matched the fail-open - * rule, but we are connected to a controller too. We should send - * the packet up to the controller in the hope that it will try to - * set up a flow and thereby allow us to exit fail-open. - * - * See the top-level comment in fail-open.c for more information. */ - pin.packet = packet->data; - 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); - ofproto_dpif_send_packet_in(ofproto, &pin); + LIST_FOR_EACH (packet, list_node, &miss->packets) { + struct ofputil_packet_in *pin; + + /* Extra-special case for fail-open mode. + * + * We are in fail-open mode and the packet matched the fail-open + * rule, but we are connected to a controller too. We should send + * the packet up to the controller in the hope that it will try to + * set up a flow and thereby allow us to exit fail-open. + * + * See the top-level comment in fail-open.c for more information. */ + 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); + ofproto_dpif_send_packet_in(ofproto, pin); + } } if (miss->xout.slow) { diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 0b7df80a6..46902159b 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -560,6 +560,8 @@ ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto, ovs_mutex_unlock(&ofproto->flow_mod_mutex); } +/* Appends 'pin' to the queue of "packet ins" to be sent to the controller. + * Takes ownership of 'pin' and pin->packet. */ void ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto, struct ofputil_packet_in *pin) -- 2.43.0