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 c0d0eabc6400e5adcd573115fa65724f17766b58..4934092bb5012fb57b581dabc0b84193195d32bf 100644 (file)
--- 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 df4f0cf2c6ba5be3b7fa21cda394135a7bcf92ba..450ae81e02dc6d54b891213c71a07b5f2c70e6ed 100644 (file)
--- 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 793e0852f3b63b46abda90c14aaa737ffafd6af4..b04f47160cef4cce72bd62b2fba670fa0dfa59a6 100644 (file)
--- 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;