vswitchd: Do not choose generated MAC address for local port.
authorBen Pfaff <blp@nicira.com>
Mon, 14 Dec 2009 21:09:47 +0000 (13:09 -0800)
committerBen Pfaff <blp@nicira.com>
Mon, 14 Dec 2009 21:09:47 +0000 (13:09 -0800)
ovs-vswitchd needs to choose a sensible MAC address for the local port of
a bridge.  Until now, the algorithm has ignored certain interfaces, in
particular internal interfaces and those with the MAC addresses that
indicate that they are probably Xen VIFs.  The goal is to choose a physical
interface's MAC address because this is more stable and more likely to
be meaningful to the outside world.  Stability, in turn, is important
because the MAC address of the local port is used as the default datapath
ID for OpenFlow connections.

This existing algorithm was too specialized to work well with the new
kinds of ports that we have been introducing in OVS.  In particular,
GRE ports could be chosen as the MAC address.  This commit changes the
algorithm for choosing the local port MAC address.  Now it ignores any
interface that has the "local" bit set in its MAC address, which
catches GRE ports.  The new rule also catches the VIF and internal
port cases, so this commit also deletes those special cases.

This commit deleted the only user of eth_addr_is_vif(), so it deletes
that function also.

Jesse Gross suggested this revised heuristic.

CC: Jeremy Stribling <strib@nicira.com>
lib/packets.h
vswitchd/bridge.c

index 83bdb6b57c2427701f0c48c80f4a030c2108a68a..f61c20ca5896e3ddc2200f38c90c3b7fc7197ccf 100644 (file)
@@ -38,14 +38,6 @@ static inline bool eth_addr_is_broadcast(const uint8_t ea[6])
     return (ea[0] & ea[1] & ea[2] & ea[3] & ea[4] & ea[5]) == 0xff;
 }
 
-/* Returns true if 'ea' is an Ethernet address used for virtual interfaces
- * under XenServer.  Generally the actual Ethernet address is FE:FF:FF:FF:FF:FF
- * but it can be FE:FE:FE:FE:FE:FE in some cases. */
-static inline bool eth_addr_is_vif(const uint8_t ea[6])
-{
-    return ea[0] == 0xfe && (ea[1] & ea[2] & ea[3] & ea[4] & ea[5]) >= 0xfe;
-}
-
 static inline bool eth_addr_is_multicast(const uint8_t ea[6])
 {
     return ea[0] & 1;
index c7aff74a8132e264c42fb0b5798ec1a02c748aa9..2b4a3f0cd94e3df10adcbddf54116c97c7764feb 100644 (file)
@@ -794,9 +794,8 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
         }
     }
 
-    /* Otherwise choose the minimum MAC address among all of the interfaces.
-     * (Xen uses FE:FF:FF:FF:FF:FF for virtual interfaces so this will get the
-     * MAC of the physical interface in such an environment.) */
+    /* Otherwise choose the minimum non-local MAC address among all of the
+     * interfaces. */
     memset(ea, 0xff, sizeof ea);
     for (i = 0; i < br->n_ports; i++) {
         struct port *port = br->ports[i];
@@ -837,11 +836,8 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
             }
 
             /* The local port doesn't count (since we're trying to choose its
-             * MAC address anyway).  Other internal ports don't count because
-             * we really want a physical MAC if we can get it, and internal
-             * ports typically have randomly generated MACs. */
-            if (iface->dp_ifidx == ODPP_LOCAL
-                || !strcmp(iface->cfg->type, "internal")) {
+             * MAC address anyway). */
+            if (iface->dp_ifidx == ODPP_LOCAL) {
                 continue;
             }
 
@@ -857,6 +853,7 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
 
         /* Compare against our current choice. */
         if (!eth_addr_is_multicast(iface_ea) &&
+            !eth_addr_is_local(iface_ea) &&
             !eth_addr_is_reserved(iface_ea) &&
             !eth_addr_is_zero(iface_ea) &&
             memcmp(iface_ea, ea, ETH_ADDR_LEN) < 0)
@@ -865,7 +862,7 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
             *hw_addr_iface = iface;
         }
     }
-    if (eth_addr_is_multicast(ea) || eth_addr_is_vif(ea)) {
+    if (eth_addr_is_multicast(ea)) {
         memcpy(ea, br->default_ea, ETH_ADDR_LEN);
         *hw_addr_iface = NULL;
         VLOG_WARN("bridge %s: using default bridge Ethernet "