From: Ben Pfaff <blp@nicira.com>
Date: Fri, 16 Jan 2009 18:08:51 +0000 (-0800)
Subject: vswitchd: Delete flows on a deleted interface when revalidating.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f523312f3197bba9d17aeca57e1a9096c0d7baf4;p=openvswitch

vswitchd: Delete flows on a deleted interface when revalidating.

When an interface is deleted from a datapath by an entity other than
vswitchd (e.g. by a vif being deleted), we would revalidate all the
flows and change them to drop packets.  But that's a waste of flow
table space.  This commit changes the behavior in this case to delete
those flows entirely.

This commit is complicated by the need to deal gracefully with flows
on datapath interfaces that we don't know about, e.g. from the local port
if the local port is not part of the bridge or from interfaces added to
a datapath by an external mechanism (e.g. added with "dpctl addif"
manually).  We don't want to delete those flows, even though they resemble
the ones that we do want to delete, because they potentially save us from
processing a lot of packet-in messages that we don't care about.  So we
mark those flows with a new "need_drop" flag.
---

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index c0d0eabc..4934092b 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1571,6 +1571,7 @@ send_packets(struct bridge *br, const struct flow *flow,
             ft_insert(br->ft, f);
             queue = true;
         }
+        f->need_drop = false;
 
         if (queue) {
             struct ofpbuf *fbuf = make_add_flow(flow, pkt->buffer_id,
@@ -1631,25 +1632,47 @@ process_flow(struct bridge *br, const struct flow *flow,
     /* Find the interface and port structure for the received packet. */
     in_iface = iface_from_dp_ifidx(br, in_ifidx);
     if (!in_iface) {
-        struct ft_flow *f;
-
-        if (pkt->buf) {
+        /* No interface?  Something fishy... */
+        struct ft_flow *f = ft_lookup(br->ft, flow, flow_hash(flow, 0));
+        if (pkt->buf || (f && f->need_drop)) {
+            /* Odd.  A few possible reasons here:
+             *
+             * - We deleted an interface but there are still a few packets
+             *   queued up from it.
+             *
+             * - Someone externally added an interface (e.g. with "dpctl
+             *   addif") that we don't know about.
+             *
+             * - Packet arrived on the local port but the local port is not
+             *   one of our bridge ports.
+             */
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
             VLOG_WARN_RL(&rl, "bridge %s: received packet on unknown "
                          "interface %"PRIu16, br->name, in_ifidx); 
-        }
 
-        queue_tx(br, make_add_flow(flow, pkt->buffer_id,
-                                   br->flow_idle_time, 0));
+            queue_tx(br, make_add_flow(flow, pkt->buffer_id,
+                                       br->flow_idle_time, 0));
+            if (f) {
+                ftf_set_dsts(f, NULL, 0);
+                f->tags = tags;
+            } else {
+                ft_insert(br->ft, ftf_create(flow, NULL, 0, tags));
+            }
 
-        f = ft_lookup(br->ft, flow, flow_hash(flow, 0));
-        if (f) {
-            ftf_set_dsts(f, NULL, 0);
-            f->tags = tags;
+            /* Mark the flow as being needed to drop packets.  (If we don't do
+             * this, then the flow will be deleted as soon as we revalidate
+             * it.) */
+            f->need_drop = true;
         } else {
-            ft_insert(br->ft, ftf_create(flow, NULL, 0, tags));
+            /* We're revalidating, not receiving a fresh packet.  Most likely,
+             * this interface existed and has now been deleted from the
+             * datapath.  Drop the flow. */
+            assert(f);
+            queue_tx(br, make_del_flow(flow));
+            ft_remove(br->ft, f);
+            ftf_destroy(f);
+            /* 'flow' pointed to &f->flow, so don't access it anymore. */
         }
-
         return;
     }
     in_port = in_iface->port;
diff --git a/vswitchd/flowtrack.c b/vswitchd/flowtrack.c
index df4f0cf2..450ae81e 100644
--- a/vswitchd/flowtrack.c
+++ b/vswitchd/flowtrack.c
@@ -101,6 +101,7 @@ ftf_create(const struct flow *flow,
     f = xmalloc(sizeof *f);
     f->tags = tags;
     f->flow = *flow;
+    f->need_drop = false;
     f->node.hash = flow_hash(&f->flow, 0);
     f->dsts = &f->one_dst;
     ftf_set_dsts(f, dsts, n_dsts);
diff --git a/vswitchd/flowtrack.h b/vswitchd/flowtrack.h
index 793e0852..b04f4716 100644
--- a/vswitchd/flowtrack.h
+++ b/vswitchd/flowtrack.h
@@ -60,6 +60,7 @@ struct ft_flow {
     tag_type tags;
     struct hmap_node node;
     struct flow flow;
+    bool need_drop;
 
     /* Statistics. */
     uint64_t last_byte_count;