vswitchd: Change port mirroring to match semantics expected by users.

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index db729fff7b3103060e051bb24b4b7ff2391a5c2b..c19e2cbfdf125532a05b196bdb7200e0496254d8 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -106,11 +106,9 @@ struct mirror {
     int *vlans;
     size_t n_vlans;
 
-    /* Transformations. */
-    int set_vlan;               /* New VLAN tag or -1 to keep original tag. */
-
     /* Output. */
     struct port *out_port;
+    int out_vlan;
 };
 
 #define FLOOD_PORT ((struct port *) 1) /* The 'flood' output port. */
@@ -136,6 +134,7 @@ struct port {
     /* Port mirroring info. */
     mirror_mask_t src_mirrors;  /* Mirrors triggered when packet received. */
     mirror_mask_t dst_mirrors;  /* Mirrors triggered when packet sent. */
+    bool is_mirror_output_port; /* Does port mirroring send frames here? */
 };
 
 #define DP_MAX_PORTS 255
@@ -1382,17 +1381,15 @@ compose_dsts(const struct bridge *br, const struct flow *flow, uint16_t vlan,
              struct ft_dst dsts[], tag_type *tags)
 {
     mirror_mask_t mirrors = in_port->src_mirrors;
-    struct ft_dst *dst;         /* First unused 'dsts' element. */
+    struct ft_dst *dst = dsts;
+    size_t i;
 
-    dst = dsts;
     if (out_port == FLOOD_PORT) {
-        /* Flood. */
         /* XXX use OFPP_FLOOD if no vlans or bonding. */
-        size_t i;
-
         for (i = 0; i < br->n_ports; i++) {
             struct port *port = br->ports[i];
             if (port != in_port && port_includes_vlan(port, vlan)
+                && !port->is_mirror_output_port
                 && set_dst(dst, flow, in_port, port, tags)) {
                 mirrors |= port->dst_mirrors;
                 dst++;
@@ -1405,13 +1402,32 @@ compose_dsts(const struct bridge *br, const struct flow *flow, uint16_t vlan,
 
     while (mirrors) {
         struct mirror *m = br->mirrors[mirror_mask_ffs(mirrors) - 1];
-        if ((!m->n_vlans || vlan_is_mirrored(m, vlan))
-            && set_dst(dst, flow, in_port, m->out_port, tags)) {
-            if (m->set_vlan >= 0) {
-                dst->vlan = m->set_vlan;
-            }
-            if (!dst_is_duplicate(dsts, dst - dsts, dst)) {
-                dst++;
+        if (!m->n_vlans || vlan_is_mirrored(m, vlan)) {
+            if (m->out_port) {
+                if (set_dst(dst, flow, in_port, m->out_port, tags)
+                    && !dst_is_duplicate(dsts, dst - dsts, dst)) {
+                    dst++;
+                }
+            } else {
+                for (i = 0; i < br->n_ports; i++) {
+                    struct port *port = br->ports[i];
+                    if (port_includes_vlan(port, m->out_vlan)
+                        && set_dst(dst, flow, in_port, port, tags)
+                        && !dst_is_duplicate(dsts, dst - dsts, dst))
+                    {
+                        if (!port->vlan) {
+                            dst->vlan = m->out_vlan;
+                        }
+                        if (dst->dp_ifidx == ntohs(flow->in_port)) {
+                            if (dst->vlan == vlan) {
+                                /* Don't send out input port on same VLAN. */
+                                continue;
+                            }
+                            dst->dp_ifidx = OFPP_IN_PORT;
+                        }
+                        dst++;
+                    }
+                }
             }
         }
         mirrors &= mirrors - 1;
@@ -1447,7 +1463,7 @@ send_packets(struct bridge *br, const struct flow *flow,
              const struct port *in_port, const struct port *out_port,
              tag_type tags, bool setup_flow)
 {
-    struct ft_dst dsts[DP_MAX_PORTS];
+    struct ft_dst dsts[DP_MAX_PORTS * (MAX_MIRRORS + 1)];
     size_t actions_len;         /* Estimated length of actions, in bytes. */
     size_t n_dsts;
 
@@ -1597,6 +1613,15 @@ process_flow(struct bridge *br, const struct flow *flow,
         }
     }
 
+    /* Drop frames on ports reserved for mirroring. */
+    if (in_port->is_mirror_output_port) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+        VLOG_WARN_RL(&rl, "bridge %s: dropping packet received on port %s, "
+                     "which is reserved exclusively for mirroring",
+                     br->name, in_port->name);
+        goto done;
+    }
+
     /* Drop multicast and broadcast packets on inactive bonded interfaces, to
      * avoid receiving duplicates. */
     if (in_port->n_ifaces > 1 && eth_addr_is_multicast(flow->dl_dst)) {
@@ -2474,7 +2499,7 @@ static void
 mirror_reconfigure(struct bridge *br)
 {
     struct svec old_mirrors, new_mirrors;
-    int i;
+    size_t i;
 
     /* Collect old and new mirrors. */
     svec_init(&old_mirrors);
@@ -2510,6 +2535,17 @@ mirror_reconfigure(struct bridge *br)
             mirror_reconfigure_one(br->mirrors[i]);
         }
     }
+
+    /* Update port reserved status. */
+    for (i = 0; i < br->n_ports; i++) {
+        br->ports[i]->is_mirror_output_port = false;
+    }
+    for (i = 0; i < MAX_MIRRORS; i++) {
+        struct mirror *m = br->mirrors[i];
+        if (m && m->out_port) {
+            m->out_port->is_mirror_output_port = true;
+        }
+    }
 }
 
 static void
@@ -2540,7 +2576,7 @@ mirror_create(struct bridge *br, const char *name)
     svec_init(&m->dst_ports);
     m->vlans = NULL;
     m->n_vlans = 0;
-    m->set_vlan = -1;
+    m->out_vlan = -1;
     m->out_port = NULL;
 }
 
@@ -2647,7 +2683,7 @@ mirror_reconfigure_one(struct mirror *m)
     mirror_mask_t mirror_bit;
     const char *out_port_name;
     struct port *out_port;
-    int set_vlan;
+    int out_vlan;
     size_t n_vlans;
     int *vlans;
     size_t i;
@@ -2655,16 +2691,28 @@ mirror_reconfigure_one(struct mirror *m)
     /* Get output port. */
     out_port_name = cfg_get_key(0, "mirror.%s.%s.output.port",
                                 m->bridge->name, m->name);
-    if (!out_port_name) {
-        VLOG_WARN("%s.output.port: missing output port", pfx);
-        mirror_destroy(m);
-        free(pfx);
-        return;
-    }
-    out_port = port_lookup(m->bridge, out_port_name);
-    if (!out_port) {
-        VLOG_WARN("%s.output.port: bridge %s does not have a port "
-                  "named %s", pfx, m->bridge->name, out_port_name);
+    if (out_port_name) {
+        out_port = port_lookup(m->bridge, out_port_name);
+        if (!out_port) {
+            VLOG_ERR("%s.output.port: bridge %s does not have a port "
+                      "named %s", pfx, m->bridge->name, out_port_name);
+            mirror_destroy(m);
+            free(pfx);
+            return;
+        }
+        out_vlan = -1;
+
+        if (cfg_has("%s.output.vlan", pfx)) {
+            VLOG_ERR("%s.output.port and %s.output.vlan both specified; "
+                     "ignoring %s.output.vlan", pfx, pfx, pfx);
+        }
+    } else if (cfg_has("%s.output.vlan", pfx)) {
+        out_port = NULL;
+        out_vlan = cfg_get_vlan(0, "%s.output.vlan", pfx);
+    } else {
+        VLOG_ERR("%s: neither %s.output.port nor %s.output.vlan specified, "
+                 "but exactly one is required; disabling port mirror %s",
+                 pfx, pfx, pfx, pfx);
         mirror_destroy(m);
         free(pfx);
         return;
@@ -2689,23 +2737,13 @@ mirror_reconfigure_one(struct mirror *m)
     n_vlans = prune_vlans(m, &vlan_strings, &vlans);
     svec_destroy(&vlan_strings);
 
-    /* Determine the VLAN transformation. */
-    if (cfg_has("%s.transform.set-vlan", pfx)) {
-        set_vlan = cfg_get_vlan(0, "%s.transform.set-vlan", pfx);
-        if (set_vlan == 0) {
-            set_vlan = OFP_VLAN_NONE;
-        }
-    } else {
-        set_vlan = -1;
-    }
-
     /* Update mirror data. */
     if (!svec_equal(&m->src_ports, &src_ports)
         || !svec_equal(&m->dst_ports, &dst_ports)
         || m->n_vlans != n_vlans
         || memcmp(m->vlans, vlans, sizeof *vlans * n_vlans)
-        || m->set_vlan != set_vlan
-        || m->out_port != out_port) {
+        || m->out_port != out_port
+        || m->out_vlan != out_vlan) {
         bridge_flush(m->bridge);
     }
     svec_swap(&m->src_ports, &src_ports);
@@ -2713,8 +2751,8 @@ mirror_reconfigure_one(struct mirror *m)
     free(m->vlans);
     m->vlans = vlans;
     m->n_vlans = n_vlans;
-    m->set_vlan = set_vlan;
     m->out_port = out_port;
+    m->out_vlan = out_vlan;
 
     /* Update ports. */
     mirror_bit = MIRROR_MASK_C(1) << m->idx;

diff --git a/vswitchd/vswitchd.conf.5 b/vswitchd/vswitchd.conf.5
index 8e0b5d4df807033e873b90f9467294765b75c69d..06721a464136267f0ec1bbe543b150d5b0848dc0 100644 (file)
--- a/vswitchd/vswitchd.conf.5
+++ b/vswitchd/vswitchd.conf.5
@@ -175,7 +175,7 @@ with physical network devices \fBeth2\fR and \fBeth3\fR:
 .fi
 .RE
 .SS "Port mirroring"
-\fBvswitchd\fR may be configured to send certain frames to special
+\fBvswitchd\fR may be configured to send selected frames to special
 "mirrored" ports, in addition to their normal destinations.
 .PP
 Up to 32 instances of port mirroring may be configured on a given
@@ -183,12 +183,11 @@ bridge.  Each must be given a name that is unique within the bridge.
 The keys associated with port mirroring instance \fIpmname\fR for
 bridge \fIbrname\fR begin with \fBmirror.\fIbrname\fB.\fIpmname\fR.
 .PP
-Each port mirroring instance requires three pieces of configuration:
-the frames to be mirrored, the types of transformations to be
-performed on the mirrored frames (if any), and the ports to which the
-frames should be sent.  Each of these pieces is configured through a
-subsection of \fBmirror.\fIbrname\fB.\fIpmname\fR, named \fBselect\fR,
-\fBtransform\fB, and \fBoutput\fR, respectively.
+The selection of the frames to mirror and the form in which they
+should be output is configured separately for each port mirroring
+instances, through a subsection of
+\fBmirror.\fIbrname\fB.\fIpmname\fR, named \fBselect\fR, and
+\fBoutput\fR, respectively.
 .PP
 .I "Selecting Frames to Mirror"
 .PP
@@ -218,26 +217,31 @@ tagging," above).  A \fIvid\fR of zero selects frames that do not
 belong to a VLAN, that is, frames that arrived on a trunk port without
 a VLAN tag or tagged with VLAN 0.
 .PP
-.I "Frame Transformations"
-.PP
-Only a single transformation is currently implemented.
-.TP
-\fBmirror.\fIbrname\fB.\fIpmname\fB.transform.set-vlan = \fIvid\fR
-.
-Changes the VLAN tag of the mirrored frames.  \fIvid\fR must be an
-integer between 0 and 4095, inclusive.  A nonzero \fIvid\fR sets the
-VLAN tag to \fIvid\fR, replacing any existing tag.  A \fIvid\fR of
-zero removes any VLAN tag from mirrored frames.
-.PP
 .I "Mirror Output"
 .PP
-Only a single form of mirror output is currently implemented.
+The values of the following keys determine how frames selected for
+mirroring are output.  Only one of the keys may be specified.
 .TP
 \fBmirror.\fIbrname\fB.\fIpmname\fB.output.port = \fIport\fR
 .
-Causes the selected and transformed frames to be sent out \fIport\fR,
-which must be part of \fIbridge\fR; that is, it must be listed on
+Causes the selected frames to be sent out \fIport\fR, which must be
+part of \fIbridge\fR; that is, it must be listed on
 \fBbridge.\fIbrname\fB.port\fR.
+.IP
+Specifying a \fIport\fR in this way reserves that port exclusively for
+mirroring.  No frames other than those selected for mirroring will be
+forwarded to \fIport\fR, and any frames received on \fIport\fR will be
+discarded.
+.TP
+\fBmirror.\fIbrname\fB.\fIpmname\fB.output.vlan = \fIvid\fR
+.
+Causes the selected frames to be sent on the VLAN numbered \fIvid\fR,
+which must be an integer between 0 and 4095, inclusive.  The frames
+will be sent out Iall ports that trunk VLAN \fIvid\fR, as well as any
+ports with implicit VLAN \fIvid\fR.  When a mirrored frame is sent out
+a trunk port, the frame's VLAN tag will be set to \fIvid\fR, replacing
+any existing tag; when it is sent out an implicit VLAN port, the frame
+will not be tagged.
 .PP
 .I "Example"
 .PP