From 939ff2674caa93d15b607bc514932533490ff2a0 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 15 Mar 2010 15:23:22 -0700 Subject: [PATCH] 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. --- 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 596040b4..11ec99d8 100644 --- 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 c148b6e1..7196ed61 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 f24311f5..24a2d9c1 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -420,14 +420,17 @@ + + If true, every packet arriving or departing on any port is + selected for mirroring. + + Ports on which departing packets are selected for mirroring. - Ports on which arriving packets are selected for mirroring. If this - column and are both empty, then all - packets on all ports are selected for mirroring. + Ports on which arriving packets are selected for mirroring. -- 2.30.2