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 <[email protected]> --- 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 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
