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 <yamt@mwd.biglobe.ne.jp>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index ff9b2d5..2420865 100644 (file)
--- 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 0b7df80..4690215 100644 (file)
--- 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)