vswitch: Use weak references in Mirror table.

A port mirror seems sufficiently disconnected from the ports that it
mirrors that it seems counterproductive to forbid removing a port if
it is mirrored.  This commit therefore changes the references from
Mirror to Port from strong references to weak references, so that
removing a port automatically removes references to it from the Mirror
table.

Since this could cause the port and VLAN selection for the Mirror to
become empty, which would make the mirror select all packets, at the same
time this commit adds a new column "select_all" to Mirror, to explicitly
allow selecting all packets.

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 596040b45482c161c6bd3808018e2db9d8fb2d48..11ec99d813202982e35c0cdf228db77b171e3e70 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3817,9 +3817,6 @@ mirror_reconfigure_one(struct mirror *m, struct ovsrec_mirror *cfg)
     size_t n_vlans;
     int *vlans;
     size_t i;
-    bool mirror_all_ports;
-    bool any_ports_specified;
-    bool any_vlans_specified;
 
     /* Get output port. */
     if (cfg->output_port) {
@@ -3847,30 +3844,25 @@ mirror_reconfigure_one(struct mirror *m, struct ovsrec_mirror *cfg)
         return;
     }
 
-    /* Get all the ports, and drop duplicates and ports that don't exist. */
     shash_init(&src_ports);
     shash_init(&dst_ports);
-    mirror_collect_ports(m, cfg->select_src_port, cfg->n_select_src_port,
-                         &src_ports);
-    mirror_collect_ports(m, cfg->select_dst_port, cfg->n_select_dst_port,
-                         &dst_ports);
-    any_ports_specified = cfg->n_select_dst_port || cfg->n_select_dst_port;
-    if (any_ports_specified
-        && shash_is_empty(&src_ports) && shash_is_empty(&dst_ports)) {
-        VLOG_ERR("bridge %s: disabling mirror %s since none of the specified "
-                 "selection ports exists", m->bridge->name, m->name);
-        mirror_destroy(m);
-        goto exit;
-    }
+    if (cfg->select_all) {
+        for (i = 0; i < m->bridge->n_ports; i++) {
+            const char *name = m->bridge->ports[i]->name;
+            shash_add_once(&src_ports, name, NULL);
+            shash_add_once(&dst_ports, name, NULL);
+        }
+        vlans = NULL;
+        n_vlans = 0;
+    } else {
+        /* Get ports, and drop duplicates and ports that don't exist. */
+        mirror_collect_ports(m, cfg->select_src_port, cfg->n_select_src_port,
+                             &src_ports);
+        mirror_collect_ports(m, cfg->select_dst_port, cfg->n_select_dst_port,
+                             &dst_ports);
 
-    /* Get all the vlans, and drop duplicate and invalid vlans. */
-    n_vlans = mirror_collect_vlans(m, cfg, &vlans);
-    any_vlans_specified = cfg->n_select_vlan > 0;
-    if (any_vlans_specified && !n_vlans) {
-        VLOG_ERR("bridge %s: disabling mirror %s since none of the specified "
-                 "VLANs exists", m->bridge->name, m->name);
-        mirror_destroy(m);
-        goto exit;
+        /* Get all the vlans, and drop duplicate and invalid vlans. */
+        n_vlans = mirror_collect_vlans(m, cfg, &vlans);
     }
 
     /* Update mirror data. */
@@ -3890,16 +3882,12 @@ mirror_reconfigure_one(struct mirror *m, struct ovsrec_mirror *cfg)
     m->out_port = out_port;
     m->out_vlan = out_vlan;
 
-    /* If no selection criteria have been given, mirror for all ports. */
-    mirror_all_ports = !any_ports_specified && !any_vlans_specified;
-
     /* Update ports. */
     mirror_bit = MIRROR_MASK_C(1) << m->idx;
     for (i = 0; i < m->bridge->n_ports; i++) {
         struct port *port = m->bridge->ports[i];
 
-        if (mirror_all_ports
-            || shash_find(&m->src_ports, port->name)
+        if (shash_find(&m->src_ports, port->name)
             || (m->n_vlans
                 && (!port->vlan
                     ? port_trunks_any_mirrored_vlan(m, port)
@@ -3909,7 +3897,7 @@ mirror_reconfigure_one(struct mirror *m, struct ovsrec_mirror *cfg)
             port->src_mirrors &= ~mirror_bit;
         }
 
-        if (mirror_all_ports || shash_find(&m->dst_ports, port->name)) {
+        if (shash_find(&m->dst_ports, port->name)) {
             port->dst_mirrors |= mirror_bit;
         } else {
             port->dst_mirrors &= ~mirror_bit;
@@ -3917,7 +3905,6 @@ mirror_reconfigure_one(struct mirror *m, struct ovsrec_mirror *cfg)
     }
 
     /* Clean up. */
-exit:
     shash_destroy(&src_ports);
     shash_destroy(&dst_ports);
 }

diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index c148b6e178866d9103e8d940d3520ff219db2a06..7196ed6192b78d3046c99d219343f408f8a81a60 100644 (file)
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -120,6 +120,9 @@
      "columns": {
        "name": {
          "type": "string"},
+       "select_all": {
+         "type": "boolean"
+       },
        "select_src_port": {
          "type": {"key": {"type": "uuid",
                           "refTable": "Port",

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index f24311f58cc6165da2698ff77bf2991339deb3c9..24a2d9c1af1b2dec267e9ba26ca97bd918d7c727 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -420,14 +420,17 @@
     </column>
 
     <group title="Selecting Packets for Mirroring">
+      <column name="select_all">
+        If true, every packet arriving or departing on any port is
+        selected for mirroring.
+      </column>
+
       <column name="select_dst_port">
         Ports on which departing packets are selected for mirroring.
       </column>
 
       <column name="select_src_port">
-        Ports on which arriving packets are selected for mirroring.  If this
-        column and <ref column="select_dst_port"/> are both empty, then all
-        packets on all ports are selected for mirroring.
+        Ports on which arriving packets are selected for mirroring.
       </column>
 
       <column name="select_vlan">