[ovs-dev] [PATCH 8/8] vswitch: Use weak references in Mirror table.

Ben Pfaff blp at nicira.com
Mon Mar 15 22:42:56 UTC 2010


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.
---
 vswitchd/bridge.c          |   49 ++++++++++++++++---------------------------
 vswitchd/vswitch.ovsschema |    3 ++
 vswitchd/vswitch.xml       |    9 +++++--
 3 files changed, 27 insertions(+), 34 deletions(-)

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 6440bb9..5adab4a 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -3819,9 +3819,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) {
@@ -3849,30 +3846,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. */
@@ -3892,16 +3884,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)
@@ -3911,7 +3899,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;
@@ -3919,7 +3907,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 c148b6e..7196ed6 100644
--- 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 94b5972..86da682 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -415,14 +415,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">
-- 
1.6.6.1





More information about the dev mailing list