[ovs-dev] [bpdu 2/4] packets: Generalize reserved RSPAN protocols.

Ethan Jackson ethan at nicira.com
Thu Jun 7 00:44:39 UTC 2012


Open vSwitch refuses to mirror certain destination addresses in
addition to those classified by eth_addr_is_reserved().  Looking
through the uses of eth_addr_is_reserved(), one finds that no
callers should be using the additional addresses which mirroring
drops.  This patch folds the additional addresses dropped in the
mirroring code, into the more general eth_addr_is_reserverd()
function.

This patch also changes the implementation in a way that is
slightly less efficient, but much easier to read and extend int he
future.

Bug #11755.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 lib/packets.c          |   41 +++++++++++++++++++++++++++++++
 lib/packets.h          |   12 +--------
 ofproto/ofproto-dpif.c |   43 +--------------------------------
 vswitchd/vswitch.xml   |   63 ++++++++++++++++++++++++++----------------------
 4 files changed, 77 insertions(+), 82 deletions(-)

diff --git a/lib/packets.c b/lib/packets.c
index 631abf8..35829fc 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -43,6 +43,47 @@ dpid_from_string(const char *s, uint64_t *dpidp)
     return *dpidp != 0;
 }
 
+/* Returns true if 'ea' is a reserved multicast address, that a bridge must
+ * never forward, false otherwise.  Includes some proprietary vendor protocols
+ * that shouldn't be forwarded as well.
+ *
+ * If you change this function's behavior, please update corresponding
+ * documentation in vswitch.xml at the same time. */
+bool
+eth_addr_is_reserved(const uint8_t ea[ETH_ADDR_LEN])
+{
+    struct masked_eth_addr {
+        uint8_t ea[ETH_ADDR_LEN];
+        uint8_t mask[ETH_ADDR_LEN];
+    };
+
+    static struct masked_eth_addr mea[] = {
+        { /* STP, IEEE pause frames, and other reserved protocols. */
+            {0x01, 0x08, 0xc2, 0x00, 0x00, 0x00},
+            {0xff, 0xff, 0xff, 0xff, 0xff, 0xf0}},
+
+        { /* Cisco Inter Switch Link. */
+            {0x01, 0x00, 0x0c, 0x00, 0x00, 0x00},
+            {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}},
+
+        { /* Cisco protocols plus others following the same pattern:
+           *
+           * CDP, VTP, DTP, PAgP  (01-00-0c-cc-cc-cc)
+           * Spanning Tree PVSTP+ (01-00-0c-cc-cc-cd)
+           * STP Uplink Fast      (01-00-0c-cd-cd-cd) */
+            {0x01, 0x00, 0x0c, 0xcc, 0xcc, 0xcc},
+            {0xff, 0xff, 0xff, 0xfe, 0xfe, 0xfe}}};
+
+    size_t i;
+
+    for (i = 0; i < ARRAY_SIZE(mea); i++) {
+        if (eth_addr_equal_except(ea, mea[i].ea, mea[i].mask)) {
+            return true;
+        }
+    }
+    return false;
+}
+
 bool
 eth_addr_from_string(const char *s, uint8_t ea[ETH_ADDR_LEN])
 {
diff --git a/lib/packets.h b/lib/packets.h
index afe4b6b..4a0fcae 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -131,18 +131,8 @@ static inline void eth_addr_nicira_random(uint8_t ea[ETH_ADDR_LEN])
     /* Set the top bit to indicate random Nicira address. */
     ea[3] |= 0x80;
 }
-/* Returns true if 'ea' is a reserved multicast address, that a bridge must
- * never forward, false otherwise. */
-static inline bool eth_addr_is_reserved(const uint8_t ea[ETH_ADDR_LEN])
-{
-    return (ea[0] == 0x01
-            && ea[1] == 0x80
-            && ea[2] == 0xc2
-            && ea[3] == 0x00
-            && ea[4] == 0x00
-            && (ea[5] & 0xf0) == 0x00);
-}
 
+bool eth_addr_is_reserved(const uint8_t ea[ETH_ADDR_LEN]);
 bool eth_addr_from_string(const char *, uint8_t ea[ETH_ADDR_LEN]);
 
 void compose_benign_packet(struct ofpbuf *, const char *tag,
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e3efed7..e171b3c 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5856,47 +5856,6 @@ vlan_is_mirrored(const struct ofmirror *m, int vlan)
     return !m->vlans || bitmap_is_set(m->vlans, vlan);
 }
 
-/* Returns true if a packet with Ethernet destination MAC 'dst' may be mirrored
- * to a VLAN.  In general most packets may be mirrored but we want to drop
- * protocols that may confuse switches. */
-static bool
-eth_dst_may_rspan(const uint8_t dst[ETH_ADDR_LEN])
-{
-    /* If you change this function's behavior, please update corresponding
-     * documentation in vswitch.xml at the same time. */
-    if (dst[0] != 0x01) {
-        /* All the currently banned MACs happen to start with 01 currently, so
-         * this is a quick way to eliminate most of the good ones. */
-    } else {
-        if (eth_addr_is_reserved(dst)) {
-            /* Drop STP, IEEE pause frames, and other reserved protocols
-             * (01-80-c2-00-00-0x). */
-            return false;
-        }
-
-        if (dst[0] == 0x01 && dst[1] == 0x00 && dst[2] == 0x0c) {
-            /* Cisco OUI. */
-            if ((dst[3] & 0xfe) == 0xcc &&
-                (dst[4] & 0xfe) == 0xcc &&
-                (dst[5] & 0xfe) == 0xcc) {
-                /* Drop the following protocols plus others following the same
-                   pattern:
-
-                   CDP, VTP, DTP, PAgP  (01-00-0c-cc-cc-cc)
-                   Spanning Tree PVSTP+ (01-00-0c-cc-cc-cd)
-                   STP Uplink Fast      (01-00-0c-cd-cd-cd) */
-                return false;
-            }
-
-            if (!(dst[3] | dst[4] | dst[5])) {
-                /* Drop Inter Switch Link packets (01-00-0c-00-00-00). */
-                return false;
-            }
-        }
-    }
-    return true;
-}
-
 static void
 add_mirror_actions(struct action_xlate_ctx *ctx, const struct flow *orig_flow)
 {
@@ -5971,7 +5930,7 @@ add_mirror_actions(struct action_xlate_ctx *ctx, const struct flow *orig_flow)
         ctx->mirrors |= m->dup_mirrors;
         if (m->out) {
             output_normal(ctx, m->out, vlan);
-        } else if (eth_dst_may_rspan(orig_flow->dl_dst)
+        } else if (!eth_addr_is_reserved(orig_flow->dl_dst)
                    && vlan != m->out_vlan) {
             struct ofbundle *bundle;
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 0cd9b30..5be9a4f 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -556,6 +556,35 @@
         and if Open vSwitch node does not run STP, then this option
         should be enabled.  Default is disabled, set to
         <code>true</code> to enable.
+
+        The following destination MAC addresss will not be forwarded when this
+        option is enabled.
+        <dl>
+          <dt><code>01:80:c2:00:00:00</code></dt>
+          <dd>IEEE 802.1D Spanning Tree Protocol (STP).</dd>
+
+          <dt><code>01:80:c2:00:00:01</code></dt>
+          <dd>IEEE Pause frame.</dd>
+
+          <dt><code>01:80:c2:00:00:0<var>x</var></code></dt>
+          <dd>Other reserved protocols.</dd>
+
+          <dt><code>01:00:0c:cc:cc:cc</code></dt>
+          <dd>
+            Cisco Discovery Protocol (CDP), VLAN Trunking Protocol (VTP),
+            Dynamic Trunking Protocol (DTP), Port Aggregation Protocol (PAgP),
+            and others.
+          </dd>
+
+          <dt><code>01:00:0c:cc:cc:cd</code></dt>
+          <dd>Cisco Shared Spanning Tree Protocol PVSTP+.</dd>
+
+          <dt><code>01:00:0c:cd:cd:cd</code></dt>
+          <dd>Cisco STP Uplink Fast.</dd>
+
+          <dt><code>01:00:0c:00:00:00</code></dt>
+          <dd>Cisco Inter Switch Link.</dd>
+        </dl>
       </column>
 
       <column name="other_config" key="mac-aging-time"
@@ -2315,36 +2344,12 @@
         sent out an implicit VLAN port, the frame will not be tagged.  This
         type of mirroring is sometimes called RSPAN.</p>
         <p>
-          The following destination MAC addresses will not be mirrored to a
-          VLAN to avoid confusing switches that interpret the protocols that
-          they represent:
+          See the documentation for
+          <ref column="other_config" key="forward-bpdu"/> in the
+          <ref table="Interface"/> table for a list of destination MAC
+          addresses which will not be mirrored to a VLAN to avoid confusing
+          switches that interpret the protocols that they represent.
         </p>
-        <dl>
-          <dt><code>01:80:c2:00:00:00</code></dt>
-          <dd>IEEE 802.1D Spanning Tree Protocol (STP).</dd>
-
-          <dt><code>01:80:c2:00:00:01</code></dt>
-          <dd>IEEE Pause frame.</dd>
-
-          <dt><code>01:80:c2:00:00:0<var>x</var></code></dt>
-          <dd>Other reserved protocols.</dd>
-
-          <dt><code>01:00:0c:cc:cc:cc</code></dt>
-          <dd>
-            Cisco Discovery Protocol (CDP), VLAN Trunking Protocol (VTP),
-            Dynamic Trunking Protocol (DTP), Port Aggregation Protocol (PAgP),
-            and others.
-          </dd>
-
-          <dt><code>01:00:0c:cc:cc:cd</code></dt>
-          <dd>Cisco Shared Spanning Tree Protocol PVSTP+.</dd>
-
-          <dt><code>01:00:0c:cd:cd:cd</code></dt>
-          <dd>Cisco STP Uplink Fast.</dd>
-
-          <dt><code>01:00:0c:00:00:00</code></dt>
-          <dd>Cisco Inter Switch Link.</dd>
-        </dl>
         <p><em>Please note:</em> Mirroring to a VLAN can disrupt a network that
         contains unmanaged switches.  Consider an unmanaged physical switch
         with two ports: port 1, connected to an end host, and port 2,
-- 
1.7.10.2




More information about the dev mailing list