[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