[ovs-dev] [bridge 10/15] mac-learning: Refactor to increase generality.
Ethan Jackson
ethan at nicira.com
Mon Mar 21 22:42:15 UTC 2011
Thanks for writing this up. Looks like it will be nifty in the future.
> + * If the returned MAC entry is new (as may be determined by calling
> + * mac_entry_is_new()), then the caller must pass the new entry to
> + * mac_learning_touch() and mac_learning_change(), in that order. The caller
> + * must also initialize the new entry's 'port' member. Otherwise calling those
> + * functions is at the caller's discretion. */
> +struct mac_entry *
> +mac_learning_insert(struct mac_learning *ml,
> + const uint8_t src_mac[ETH_ADDR_LEN], uint16_t vlan)
> {
It may make sense to automatically call mac_learning_touch and
mac_learning_change for the caller if the returned mac_entry is new.
This may reduce the risk of errors. I think there is a slight risk of
either of these functions being called twice on new mac entries, but
that doesn't seem like a huge problem to me.
If we can't do this, it may make sense alternatively to have
mac_learning_lookup assert each returned mac_entries tag and expires
members to ensure that they were properly initialized.
> +mac_learning_change(struct mac_learning *ml, struct mac_entry *e)
> +{
> + tag_type old_tag = e->tag;
> +
> + COVERAGE_INC(mac_learning_learned);
It may make sense to rename this mac_learning_changed.
> + mac = mac_learning_insert(br->ml, flow->dl_src, vlan);
> + if (is_gratuitous_arp(flow)) {
> + /* We don't want to learn from gratuitous ARP packets that are
> + * reflected back over bond slaves so we lock the learning table. */
> + if (in_port->n_ifaces == 1) {
> + mac_entry_set_grat_arp_lock(mac);
> + } else if (mac_entry_is_grat_arp_locked(mac)) {
> + return;
If mac is new here it will never be touched or changed. I'm not sure
if that's a problem as it may get touched next time
update_learning_table is called, but it seems like we may end up with
entries with bogus lru status.
More information about the dev
mailing list