From: Jesse Gross Date: Wed, 2 Jun 2010 23:26:46 +0000 (-0700) Subject: bridge: Filter some gratuitous ARPs on bond slaves. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7febb9100b6a63aab46f8850de6de335f8a47345;p=openvswitch bridge: Filter some gratuitous ARPs on bond slaves. Normally we filter out packets received on a bond if we have learned the source MAC as belonging to another port to avoid packets sent on one slave and reflected back on another. The exception to this is gratuitous ARPs because they indicate that the host has moved to another port. However, this can result in an additional problem on the switch that the host moved to if the gratuitous ARP is reflected back on a bond slave. In this case, we incorrectly relearn the slave as the source of the MAC address. To solve this, we lock the learning entry for 5 seconds after receiving a gratuitous ARP against further updates caused by gratuitous ARPs on bond slaves. Bug #2516 Reported-by: Ian Campbell --- diff --git a/lib/learning-switch.c b/lib/learning-switch.c index 64639f2c..2c4172e1 100644 --- a/lib/learning-switch.c +++ b/lib/learning-switch.c @@ -401,7 +401,8 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn, void *opi_) flow_extract(&pkt, 0, in_port, &flow); if (may_learn(sw, in_port) && sw->ml) { - if (mac_learning_learn(sw->ml, flow.dl_src, 0, in_port)) { + if (mac_learning_learn(sw->ml, flow.dl_src, 0, in_port, + GRAT_ARP_LOCK_NONE)) { VLOG_DBG_RL(&rl, "%016llx: learned that "ETH_ADDR_FMT" is on " "port %"PRIu16, sw->datapath_id, ETH_ADDR_ARGS(flow.dl_src), in_port); @@ -418,7 +419,7 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn, void *opi_) } if (sw->ml) { - int learned_port = mac_learning_lookup(sw->ml, flow.dl_dst, 0); + int learned_port = mac_learning_lookup(sw->ml, flow.dl_dst, 0, NULL); if (learned_port >= 0 && may_send(sw, learned_port)) { out_port = learned_port; } diff --git a/lib/mac-learning.c b/lib/mac-learning.c index f9859b6b..5d64f543 100644 --- a/lib/mac-learning.c +++ b/lib/mac-learning.c @@ -175,11 +175,14 @@ is_learning_vlan(const struct mac_learning *ml, uint16_t vlan) * that now need revalidation. * * The 'vlan' parameter is used to maintain separate per-VLAN learning tables. - * Specify 0 if this behavior is undesirable. */ + * Specify 0 if this behavior is undesirable. + * + * 'lock_type' specifies whether the entry should be locked or existing locks + * are check. */ tag_type mac_learning_learn(struct mac_learning *ml, const uint8_t src_mac[ETH_ADDR_LEN], uint16_t vlan, - uint16_t src_port) + uint16_t src_port, enum grat_arp_lock_type lock_type) { struct mac_entry *e; struct list *bucket; @@ -209,32 +212,42 @@ mac_learning_learn(struct mac_learning *ml, e->port = -1; e->vlan = vlan; e->tag = make_unknown_mac_tag(ml, src_mac, vlan); + e->grat_arp_lock = TIME_MIN; } - /* Make the entry most-recently-used. */ - list_remove(&e->lru_node); - list_push_back(&ml->lrus, &e->lru_node); - e->expires = time_now() + MAC_ENTRY_IDLE_TIME; - - /* Did we learn something? */ - if (e->port != src_port) { - tag_type old_tag = e->tag; - e->port = src_port; - e->tag = tag_create_random(); - COVERAGE_INC(mac_learning_learned); - return old_tag; + if (lock_type != GRAT_ARP_LOCK_CHECK || time_now() >= e->grat_arp_lock) { + /* Make the entry most-recently-used. */ + list_remove(&e->lru_node); + list_push_back(&ml->lrus, &e->lru_node); + e->expires = time_now() + MAC_ENTRY_IDLE_TIME; + if (lock_type == GRAT_ARP_LOCK_SET) { + e->grat_arp_lock = time_now() + MAC_GRAT_ARP_LOCK_TIME; + } + + /* Did we learn something? */ + if (e->port != src_port) { + tag_type old_tag = e->tag; + e->port = src_port; + e->tag = tag_create_random(); + COVERAGE_INC(mac_learning_learned); + return old_tag; + } } + return 0; } /* Looks up MAC 'dst' for VLAN 'vlan' in 'ml'. Returns the port on which a - * frame destined for 'dst' should be sent, -1 if unknown. */ + * frame destined for 'dst' should be sent, -1 if unknown. 'is_grat_arp_locked' + * is an optional parameter that returns whether the entry is currently + * locked. */ int mac_learning_lookup(const struct mac_learning *ml, - const uint8_t dst[ETH_ADDR_LEN], uint16_t vlan) + const uint8_t dst[ETH_ADDR_LEN], uint16_t vlan, + bool *is_grat_arp_locked) { tag_type tag = 0; - return mac_learning_lookup_tag(ml, dst, vlan, &tag); + return mac_learning_lookup_tag(ml, dst, vlan, &tag, is_grat_arp_locked); } /* Looks up MAC 'dst' for VLAN 'vlan' in 'ml'. Returns the port on which a @@ -242,11 +255,14 @@ mac_learning_lookup(const struct mac_learning *ml, * * Adds to '*tag' (which the caller must have initialized) the tag that should * be attached to any flow created based on the return value, if any, to allow - * those flows to be revalidated when the MAC learning entry changes. */ + * those flows to be revalidated when the MAC learning entry changes. + * + * 'is_grat_arp_locked' is an optional parameter that returns whether the entry + * is currently locked.*/ int mac_learning_lookup_tag(const struct mac_learning *ml, const uint8_t dst[ETH_ADDR_LEN], uint16_t vlan, - tag_type *tag) + tag_type *tag, bool *is_grat_arp_locked) { if (eth_addr_is_multicast(dst) || !is_learning_vlan(ml, vlan)) { return -1; @@ -255,6 +271,11 @@ mac_learning_lookup_tag(const struct mac_learning *ml, dst, vlan); if (e) { *tag |= e->tag; + + if (is_grat_arp_locked) { + *is_grat_arp_locked = time_now() < e->grat_arp_lock; + } + return e->port; } else { *tag |= make_unknown_mac_tag(ml, dst, vlan); diff --git a/lib/mac-learning.h b/lib/mac-learning.h index c4a0e28b..89a4e909 100644 --- a/lib/mac-learning.h +++ b/lib/mac-learning.h @@ -31,11 +31,22 @@ /* Time, in seconds, before expiring a mac_entry due to inactivity. */ #define MAC_ENTRY_IDLE_TIME 60 +/* Time, in seconds, to lock an entry updated by a gratuitous ARP to avoid + * relearning based on a reflection from a bond slave. */ +#define MAC_GRAT_ARP_LOCK_TIME 5 + +enum grat_arp_lock_type { + GRAT_ARP_LOCK_NONE, + GRAT_ARP_LOCK_SET, + GRAT_ARP_LOCK_CHECK +}; + /* A MAC learning table entry. */ struct mac_entry { struct list hash_node; /* Element in a mac_learning 'table' list. */ struct list lru_node; /* Element in 'lrus' or 'free' list. */ time_t expires; /* Expiration time. */ + time_t grat_arp_lock; /* Gratuitous ARP lock expiration time. */ uint8_t mac[ETH_ADDR_LEN]; /* Known MAC address. */ uint16_t vlan; /* VLAN tag. */ int port; /* Port on which MAC was most recently seen. */ @@ -61,12 +72,15 @@ bool mac_learning_set_flood_vlans(struct mac_learning *, unsigned long *bitmap); tag_type mac_learning_learn(struct mac_learning *, const uint8_t src[ETH_ADDR_LEN], uint16_t vlan, - uint16_t src_port); + uint16_t src_port, enum grat_arp_lock_type + lock_type); int mac_learning_lookup(const struct mac_learning *, - const uint8_t dst[ETH_ADDR_LEN], uint16_t vlan); + const uint8_t dst[ETH_ADDR_LEN], uint16_t vlan, + bool *is_grat_arp_locked); int mac_learning_lookup_tag(const struct mac_learning *, const uint8_t dst[ETH_ADDR_LEN], - uint16_t vlan, tag_type *tag); + uint16_t vlan, tag_type *tag, + bool *is_grat_arp_locked); void mac_learning_flush(struct mac_learning *); void mac_learning_run(struct mac_learning *, struct tag_set *); void mac_learning_wait(struct mac_learning *); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 9a91c1ce..0fd2fdfb 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4230,7 +4230,8 @@ default_normal_ofhook_cb(const flow_t *flow, const struct ofpbuf *packet, /* Learn source MAC (but don't try to learn from revalidation). */ if (packet != NULL) { tag_type rev_tag = mac_learning_learn(ofproto->ml, flow->dl_src, - 0, flow->in_port); + 0, flow->in_port, + GRAT_ARP_LOCK_NONE); if (rev_tag) { /* The log messages here could actually be useful in debugging, * so keep the rate limit relatively high. */ @@ -4242,7 +4243,8 @@ default_normal_ofhook_cb(const flow_t *flow, const struct ofpbuf *packet, } /* Determine output port. */ - out_port = mac_learning_lookup_tag(ofproto->ml, flow->dl_dst, 0, tags); + out_port = mac_learning_lookup_tag(ofproto->ml, flow->dl_dst, 0, tags, + NULL); if (out_port < 0) { add_output_group_action(actions, DP_GROUP_FLOOD, nf_output_iface); } else if (out_port != flow->in_port) { diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 931f987c..b43a9bf6 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -2246,12 +2246,34 @@ static int flow_get_vlan(struct bridge *br, const flow_t *flow, return vlan; } +/* A VM broadcasts a gratuitous ARP to indicate that it has resumed after + * migration. Older Citrix-patched Linux DomU used gratuitous ARP replies to + * indicate this; newer upstream kernels use gratuitous ARP requests. */ +static bool +is_gratuitous_arp(const flow_t *flow) +{ + return (flow->dl_type == htons(ETH_TYPE_ARP) + && eth_addr_is_broadcast(flow->dl_dst) + && (flow->nw_proto == ARP_OP_REPLY + || (flow->nw_proto == ARP_OP_REQUEST + && flow->nw_src == flow->nw_dst))); +} + static void update_learning_table(struct bridge *br, const flow_t *flow, int vlan, struct port *in_port) { - tag_type rev_tag = mac_learning_learn(br->ml, flow->dl_src, - vlan, in_port->port_idx); + enum grat_arp_lock_type lock_type; + tag_type rev_tag; + + /* We don't want to learn from gratuitous ARP packets that are reflected + * back over bond slaves so we lock the learning table. */ + lock_type = !is_gratuitous_arp(flow) ? GRAT_ARP_LOCK_NONE : + (in_port->n_ifaces == 1) ? GRAT_ARP_LOCK_SET : + GRAT_ARP_LOCK_CHECK; + + rev_tag = mac_learning_learn(br->ml, flow->dl_src, vlan, in_port->port_idx, + lock_type); if (rev_tag) { /* The log messages here could actually be useful in debugging, * so keep the rate limit relatively high. */ @@ -2265,19 +2287,6 @@ update_learning_table(struct bridge *br, const flow_t *flow, int vlan, } } -/* A VM broadcasts a gratuitous ARP to indicate that it has resumed after - * migration. Older Citrix-patched Linux DomU used gratuitous ARP replies to - * indicate this; newer upstream kernels use gratuitous ARP requests. */ -static bool -is_gratuitous_arp(const flow_t *flow) -{ - return (flow->dl_type == htons(ETH_TYPE_ARP) - && eth_addr_is_broadcast(flow->dl_dst) - && (flow->nw_proto == ARP_OP_REPLY - || (flow->nw_proto == ARP_OP_REQUEST - && flow->nw_src == flow->nw_dst))); -} - /* Determines whether packets in 'flow' within 'br' should be forwarded or * dropped. Returns true if they may be forwarded, false if they should be * dropped. @@ -2356,6 +2365,7 @@ is_admissible(struct bridge *br, const flow_t *flow, bool have_packet, /* Packets received on bonds need special attention to avoid duplicates. */ if (in_port->n_ifaces > 1) { int src_idx; + bool is_grat_arp_locked; if (eth_addr_is_multicast(flow->dl_dst)) { *tags |= in_port->active_iface_tag; @@ -2368,10 +2378,14 @@ is_admissible(struct bridge *br, const flow_t *flow, bool have_packet, /* Drop all packets for which we have learned a different input * port, because we probably sent the packet on one slave and got * it back on the other. Gratuitous ARP packets are an exception - * to this rule: the host has moved to another switch. */ - src_idx = mac_learning_lookup(br->ml, flow->dl_src, vlan); + * to this rule: the host has moved to another switch. The exception + * to the exception is if we locked the learning table to avoid + * reflections on bond slaves. If this is the case, just drop the + * packet now. */ + src_idx = mac_learning_lookup(br->ml, flow->dl_src, vlan, + &is_grat_arp_locked); if (src_idx != -1 && src_idx != in_port->port_idx && - !is_gratuitous_arp(flow)) { + (!is_gratuitous_arp(flow) || is_grat_arp_locked)) { return false; } } @@ -2404,7 +2418,8 @@ process_flow(struct bridge *br, const flow_t *flow, } /* Determine output port. */ - out_port_idx = mac_learning_lookup_tag(br->ml, flow->dl_dst, vlan, tags); + out_port_idx = mac_learning_lookup_tag(br->ml, flow->dl_dst, vlan, tags, + NULL); if (out_port_idx >= 0 && out_port_idx < br->n_ports) { out_port = br->ports[out_port_idx]; } else if (!packet && !eth_addr_is_multicast(flow->dl_dst)) {