diff mbox

[ovs-dev,V10] ovn-controller-vtep: Extend vtep module to install Ucast_Macs_Remote.

Message ID 1442180349-23049-1-git-send-email-ee07b291@gmail.com
State Accepted
Headers show

Commit Message

alex wang Sept. 13, 2015, 9:39 p.m. UTC
This commit extends the vtep module to support creating the
'Ucast_Macs_Remote' table entries in the vtep database for
MAC addresses on the ovn logical ports.

Signed-off-by: Alex Wang <ee07b291@gmail.com>
Acked-by: Russell Bryant <rbryant@redhat.com>
---
V9->V10:
- Refine comments as suggested by Justin.
- Add check for 'vtep_ls->n_tunnel_key' being 0.
- Allow overlapping MAC in different logical switches.
- Fix not free'ed 'vtep_pswitches'.

V8->V9:
- Add Ack from Russell.
- Minor adjustment based on Russell's review.

V7->V8:
- rebase.

V6->V7:
- rebase.
- adopt suggestions from Russell.

V5->V6:
- rebase.

V4->V5:
- rebase on top of master.
- rewrite the feature since a lot have changed.

V3->V4:
- add logic to remove Ucast_Macs_Remote for non-existent MACs.

V2->V3:
- rebase to master.

PATCH->V2:
- split into separate commit.
- few optimizations.
---
 ovn/controller-vtep/vtep.c   | 316 ++++++++++++++++++++++++++++++++++++++-----
 tests/ovn-controller-vtep.at | 136 +++++++++++++++++++
 2 files changed, 418 insertions(+), 34 deletions(-)

Comments

Justin Pettit Sept. 14, 2015, 11:46 p.m. UTC | #1
Looks good.  Thanks!

Acked-by: Justin Pettit <jpettit@nicira.com>

--Justin


> On Sep 13, 2015, at 2:39 PM, Alex Wang <ee07b291@gmail.com> wrote:
> 
> This commit extends the vtep module to support creating the
> 'Ucast_Macs_Remote' table entries in the vtep database for
> MAC addresses on the ovn logical ports.
> 
> Signed-off-by: Alex Wang <ee07b291@gmail.com>
> Acked-by: Russell Bryant <rbryant@redhat.com>
> ---
> V9->V10:
> - Refine comments as suggested by Justin.
> - Add check for 'vtep_ls->n_tunnel_key' being 0.
> - Allow overlapping MAC in different logical switches.
> - Fix not free'ed 'vtep_pswitches'.
> 
> V8->V9:
> - Add Ack from Russell.
> - Minor adjustment based on Russell's review.
> 
> V7->V8:
> - rebase.
> 
> V6->V7:
> - rebase.
> - adopt suggestions from Russell.
> 
> V5->V6:
> - rebase.
> 
> V4->V5:
> - rebase on top of master.
> - rewrite the feature since a lot have changed.
> 
> V3->V4:
> - add logic to remove Ucast_Macs_Remote for non-existent MACs.
> 
> V2->V3:
> - rebase to master.
> 
> PATCH->V2:
> - split into separate commit.
> - few optimizations.
> ---
> ovn/controller-vtep/vtep.c   | 316 ++++++++++++++++++++++++++++++++++++++-----
> tests/ovn-controller-vtep.at | 136 +++++++++++++++++++
> 2 files changed, 418 insertions(+), 34 deletions(-)
> 
> diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
> index bae8bd5..016c2e0 100644
> --- a/ovn/controller-vtep/vtep.c
> +++ b/ovn/controller-vtep/vtep.c
> @@ -19,6 +19,7 @@
> 
> #include "lib/hash.h"
> #include "lib/hmap.h"
> +#include "lib/shash.h"
> #include "lib/smap.h"
> #include "lib/sset.h"
> #include "lib/util.h"
> @@ -30,46 +31,79 @@
> VLOG_DEFINE_THIS_MODULE(vtep);
> 
> /*
> - * Scans through the Binding table in ovnsb and updates the vtep logical
> - * switch tunnel keys.
> + * Scans through the Binding table in ovnsb, and updates the vtep logical
> + * switch tunnel keys and the 'Ucast_Macs_Remote' table in the VTEP
> + * database.
>  *
>  */
> 
> +/* Searches the 'chassis_rec->encaps' for the first vtep tunnel
> + * configuration, returns the 'ip'.  Unless duplicated, the returned
> + * pointer cannot live past current vtep_run() execution. */
> +static const char *
> +get_chassis_vtep_ip(const struct sbrec_chassis *chassis_rec)
> +{
> +    if (chassis_rec) {
> +        size_t i;
> +
> +        for (i = 0; i < chassis_rec->n_encaps; i++) {
> +            if (!strcmp(chassis_rec->encaps[i]->type, "vxlan")) {
> +                return chassis_rec->encaps[i]->ip;
> +            }
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +/* Creates a new 'Ucast_Macs_Remote'. */
> +static struct vteprec_ucast_macs_remote *
> +create_umr(struct ovsdb_idl_txn *vtep_idl_txn, const char *mac,
> +           const struct vteprec_logical_switch *vtep_ls)
> +{
> +    struct vteprec_ucast_macs_remote *new_umr;
> +
> +    new_umr = vteprec_ucast_macs_remote_insert(vtep_idl_txn);
> +    vteprec_ucast_macs_remote_set_MAC(new_umr, mac);
> +    vteprec_ucast_macs_remote_set_logical_switch(new_umr, vtep_ls);
> +
> +    return new_umr;
> +}
> +
> +/* Creates a new 'Physical_Locator'. */
> +static struct vteprec_physical_locator *
> +create_pl(struct ovsdb_idl_txn *vtep_idl_txn, const char *chassis_ip)
> +{
> +    struct vteprec_physical_locator *new_pl;
> +
> +    new_pl = vteprec_physical_locator_insert(vtep_idl_txn);
> +    vteprec_physical_locator_set_dst_ip(new_pl, chassis_ip);
> +    vteprec_physical_locator_set_encapsulation_type(new_pl, VTEP_ENCAP_TYPE);
> +
> +    return new_pl;
> +}
> +
> +
> /* Updates the vtep Logical_Switch table entries' tunnel keys based
>  * on the port bindings. */
> static void
> -vtep_lswitch_run(struct controller_vtep_ctx *ctx)
> +vtep_lswitch_run(struct shash *vtep_pbs, struct sset *vtep_pswitches,
> +                 struct shash *vtep_lswitches)
> {
> -    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
> -    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
>     struct sset used_ls = SSET_INITIALIZER(&used_ls);
> -    const struct vteprec_physical_switch *pswitch;
> -    const struct sbrec_port_binding *port_binding_rec;
> -    const struct vteprec_logical_switch *vtep_ls;
> -
> -    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (pswitch, ctx->vtep_idl) {
> -        sset_add(&vtep_pswitches, pswitch->name);
> -    }
> -    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
> -        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
> -    }
> +    struct shash_node *node;
> 
> -    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
> -                              "ovn-controller-vtep: update logical switch "
> -                              "tunnel keys");
>     /* Collects the logical switch bindings from port binding entries.
>      * Since the binding module has already guaranteed that each vtep
>      * logical switch is bound only to one ovn-sb logical datapath,
>      * we can just iterate and assign tunnel key to vtep logical switch. */
> -    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
> -        if (strcmp(port_binding_rec->type, "vtep")
> -            || !port_binding_rec->chassis) {
> -            continue;
> -        }
> +    SHASH_FOR_EACH (node, vtep_pbs) {
> +        const struct sbrec_port_binding *port_binding_rec = node->data;
>         const char *pswitch_name = smap_get(&port_binding_rec->options,
>                                             "vtep-physical-switch");
>         const char *lswitch_name = smap_get(&port_binding_rec->options,
>                                             "vtep-logical-switch");
> +        const struct vteprec_logical_switch *vtep_ls;
> 
>         /* If 'port_binding_rec->chassis' exists then 'pswitch_name'
>          * and 'lswitch_name' must also exist. */
> @@ -83,11 +117,11 @@ vtep_lswitch_run(struct controller_vtep_ctx *ctx)
>                      port_binding_rec->chassis->name);
>             continue;
>         }
> -        vtep_ls = shash_find_data(&vtep_lswitches, lswitch_name);
> +        vtep_ls = shash_find_data(vtep_lswitches, lswitch_name);
>         /* Also checks 'pswitch_name' since the same 'lswitch_name' could
>          * exist in multiple vtep database instances and be bound to different
>          * ovn logical networks. */
> -        if (vtep_ls && sset_find(&vtep_pswitches, pswitch_name)) {
> +        if (vtep_ls && sset_find(vtep_pswitches, pswitch_name)) {
>             int64_t tnl_key;
> 
>             if (sset_find(&used_ls, lswitch_name)) {
> @@ -105,21 +139,155 @@ vtep_lswitch_run(struct controller_vtep_ctx *ctx)
>             sset_add(&used_ls, lswitch_name);
>         }
>     }
> -    struct shash_node *node;
> -    /* Resets the tunnel keys for the rest of vtep logical switches. */
> -    SHASH_FOR_EACH (node, &vtep_lswitches) {
> +    /* Resets the tunnel keys for unused vtep logical switches. */
> +    SHASH_FOR_EACH (node, vtep_lswitches) {
>         if (!sset_find(&used_ls, node->name)) {
>             int64_t tnl_key = 0;
> -
>             vteprec_logical_switch_set_tunnel_key(node->data, &tnl_key, 1);
>         }
>     }
> -
> -    shash_destroy(&vtep_lswitches);
> -    sset_destroy(&vtep_pswitches);
>     sset_destroy(&used_ls);
> }
> 
> +/* Updates the vtep 'Ucast_Macs_Remote' table based on non-vtep port
> + * bindings. */
> +static void
> +vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct shash *ucast_macs_rmts,
> +              struct shash *physical_locators, struct shash *vtep_lswitches,
> +              struct shash *non_vtep_pbs)
> +{
> +    struct shash_node *node;
> +    struct hmap ls_map;
> +
> +    /* Maps from ovn logical datapath tunnel key (which is also the vtep
> +     * logical switch tunnel key) to the corresponding vtep logical switch
> +     * instance.  Also, the shash map 'added_macs' is used for checking
> +     * duplicated MAC addresses in the same ovn logical datapath. */
> +    struct ls_hash_node {
> +        struct hmap_node hmap_node;
> +
> +        const struct vteprec_logical_switch *vtep_ls;
> +        struct shash added_macs;
> +    };
> +
> +    hmap_init(&ls_map);
> +    SHASH_FOR_EACH (node, vtep_lswitches) {
> +        const struct vteprec_logical_switch *vtep_ls = node->data;
> +        struct ls_hash_node *ls_node;
> +
> +        if (!vtep_ls->n_tunnel_key) {
> +            continue;
> +        }
> +        ls_node = xmalloc(sizeof *ls_node);
> +        ls_node->vtep_ls = vtep_ls;
> +        shash_init(&ls_node->added_macs);
> +        hmap_insert(&ls_map, &ls_node->hmap_node,
> +                    hash_uint64((uint64_t) vtep_ls->tunnel_key[0]));
> +    }
> +
> +    SHASH_FOR_EACH (node, non_vtep_pbs) {
> +        const struct sbrec_port_binding *port_binding_rec = node->data;
> +        const struct sbrec_chassis *chassis_rec;
> +        struct ls_hash_node *ls_node;
> +        const char *chassis_ip;
> +        int64_t tnl_key;
> +        size_t i;
> +
> +        chassis_rec = port_binding_rec->chassis;
> +        if (!chassis_rec) {
> +            continue;
> +        }
> +
> +        tnl_key = port_binding_rec->datapath->tunnel_key;
> +        HMAP_FOR_EACH_WITH_HASH (ls_node, hmap_node,
> +                                 hash_uint64((uint64_t) tnl_key),
> +                                 &ls_map) {
> +            if (ls_node->vtep_ls->tunnel_key[0] == tnl_key) {
> +                break;
> +            }
> +        }
> +        /* If 'ls_node' is NULL, that means no vtep logical switch is
> +         * attached to the corresponding ovn logical datapath, so pass.
> +         */
> +        if (!ls_node) {
> +            continue;
> +        }
> +
> +        chassis_ip = get_chassis_vtep_ip(chassis_rec);
> +        /* Unreachable chassis, continue. */
> +        if (!chassis_ip) {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +            VLOG_INFO_RL(&rl, "VTEP tunnel encap on chassis (%s) not found",
> +                         chassis_rec->name);
> +            continue;
> +        }
> +
> +        for (i = 0; i < port_binding_rec->n_mac; i++) {
> +            const struct vteprec_ucast_macs_remote *umr;
> +            const struct vteprec_physical_locator *pl;
> +            const struct sbrec_port_binding *conflict;
> +            char *mac = port_binding_rec->mac[i];
> +
> +            /* xxx Need to address this later when we support
> +             * update of 'Mcast_Macs_Remote' table in VTEP. */
> +            if (!strcmp(mac, "unknown")) {
> +                continue;
> +            }
> +
> +            /* Checks for duplicate MAC in the same vtep logical switch. */
> +            conflict = shash_find_data(&ls_node->added_macs, mac);
> +            if (conflict) {
> +                VLOG_WARN("MAC address (%s) has already been known to be "
> +                          "on logical port (%s) in the same logical "
> +                          "datapath, so just ignore this logical port (%s)",
> +                          mac, conflict->logical_port,
> +                          port_binding_rec->logical_port);
> +                continue;
> +            }
> +            shash_add(&ls_node->added_macs, mac, port_binding_rec);
> +
> +            char *mac_ip_tnlkey = xasprintf("%s_%s_%"PRId64, mac, chassis_ip,
> +                                            tnl_key);
> +            umr = shash_find_data(ucast_macs_rmts, mac_ip_tnlkey);
> +            /* If finds the 'umr' entry for the mac, ip, and tnl_key, deletes
> +             * the entry from shash so that it is not gargage collected.
> +             *
> +             * If not found, creates a new 'umr' entry. */
> +            if (umr && umr->logical_switch == ls_node->vtep_ls) {
> +                shash_find_and_delete(ucast_macs_rmts, mac_ip_tnlkey);
> +            } else {
> +                const struct vteprec_ucast_macs_remote *new_umr;
> +
> +                new_umr = create_umr(vtep_idl_txn, mac, ls_node->vtep_ls);
> +                pl = shash_find_data(physical_locators, chassis_ip);
> +                if (pl) {
> +                    vteprec_ucast_macs_remote_set_locator(new_umr, pl);
> +                } else {
> +                    const struct vteprec_physical_locator *new_pl;
> +
> +                    new_pl = create_pl(vtep_idl_txn, chassis_ip);
> +                    vteprec_ucast_macs_remote_set_locator(new_umr, new_pl);
> +                    /* Updates the 'physical_locators'. */
> +                    shash_add(physical_locators, chassis_ip, new_pl);
> +                }
> +            }
> +            free(mac_ip_tnlkey);
> +        }
> +    }
> +
> +    /* Removes all remaining 'umr's, since they do not exist anymore. */
> +    SHASH_FOR_EACH (node, ucast_macs_rmts) {
> +        vteprec_ucast_macs_remote_delete(node->data);
> +    }
> +    struct ls_hash_node *iter, *next;
> +    HMAP_FOR_EACH_SAFE (iter, next, hmap_node, &ls_map) {
> +        hmap_remove(&ls_map, &iter->hmap_node);
> +        shash_destroy(&iter->added_macs);
> +        free(iter);
> +    }
> +    hmap_destroy(&ls_map);
> +}
> +
> /* Resets all logical switches' 'tunnel_key' to NULL */
> static bool
> vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
> @@ -137,6 +305,19 @@ vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
>     return done;
> }
> 
> +/* Removes all entries in the 'Ucast_Macs_Remote' table in vtep database.
> + * Returns true when all done (no entry to remove). */
> +static bool
> +vtep_macs_cleanup(struct ovsdb_idl *vtep_idl)
> +{
> +    const struct vteprec_ucast_macs_remote *umr;
> +
> +    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, vtep_idl) {
> +        vteprec_ucast_macs_remote_delete(umr);
> +        return false;
> +    }
> +    return true;
> +}
> 
> /* Updates vtep logical switch tunnel keys. */
> void
> @@ -145,7 +326,69 @@ vtep_run(struct controller_vtep_ctx *ctx)
>     if (!ctx->vtep_idl_txn) {
>         return;
>     }
> -    vtep_lswitch_run(ctx);
> +
> +    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
> +    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
> +    struct shash ucast_macs_rmts = SHASH_INITIALIZER(&ucast_macs_rmts);
> +    struct shash physical_locators = SHASH_INITIALIZER(&physical_locators);
> +    struct shash vtep_pbs = SHASH_INITIALIZER(&vtep_pbs);
> +    struct shash non_vtep_pbs = SHASH_INITIALIZER(&non_vtep_pbs);
> +    const struct vteprec_physical_switch *vtep_ps;
> +    const struct vteprec_logical_switch *vtep_ls;
> +    const struct vteprec_ucast_macs_remote *umr;
> +    const struct vteprec_physical_locator *pl;
> +    const struct sbrec_port_binding *port_binding_rec;
> +
> +    /* Collects 'Physical_Switch's. */
> +    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (vtep_ps, ctx->vtep_idl) {
> +        sset_add(&vtep_pswitches, vtep_ps->name);
> +    }
> +
> +    /* Collects 'Logical_Switch's. */
> +    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
> +        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
> +    }
> +
> +    /* Collects 'Ucast_Macs_Remote's. */
> +    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, ctx->vtep_idl) {
> +        char *mac_ip_tnlkey =
> +            xasprintf("%s_%s_%"PRId64, umr->MAC,
> +                      umr->locator ? umr->locator->dst_ip : "",
> +                      umr->logical_switch && umr->logical_switch->n_tunnel_key
> +                          ? umr->logical_switch->tunnel_key[0] : INT64_MAX);
> +
> +        shash_add(&ucast_macs_rmts, mac_ip_tnlkey, umr);
> +        free(mac_ip_tnlkey);
> +    }
> +    /* Collects 'Physical_Locator's. */
> +    VTEPREC_PHYSICAL_LOCATOR_FOR_EACH (pl, ctx->vtep_idl) {
> +        shash_add(&physical_locators, pl->dst_ip, pl);
> +    }
> +    /* Collects and classifies 'Port_Binding's. */
> +    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
> +        struct shash *target =
> +            !strcmp(port_binding_rec->type, "vtep") ? &vtep_pbs : &non_vtep_pbs;
> +
> +        if (!port_binding_rec->chassis) {
> +            continue;
> +        }
> +        shash_add(target, port_binding_rec->logical_port, port_binding_rec);
> +    }
> +
> +    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
> +                              "ovn-controller-vtep: update logical switch "
> +                              "tunnel keys and 'ucast_macs_remote's");
> +
> +    vtep_lswitch_run(&vtep_pbs, &vtep_pswitches, &vtep_lswitches);
> +    vtep_macs_run(ctx->vtep_idl_txn, &ucast_macs_rmts, &physical_locators,
> +                  &vtep_lswitches, &non_vtep_pbs);
> +
> +    sset_destroy(&vtep_pswitches);
> +    shash_destroy(&vtep_lswitches);
> +    shash_destroy(&ucast_macs_rmts);
> +    shash_destroy(&physical_locators);
> +    shash_destroy(&vtep_pbs);
> +    shash_destroy(&non_vtep_pbs);
> }
> 
> /* Cleans up all related entries in vtep.  Returns true when done (i.e.
> @@ -157,8 +400,13 @@ vtep_cleanup(struct controller_vtep_ctx *ctx)
>         return false;
>     }
> 
> +    bool all_done;
> +
>     ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
>                               "ovn-controller-vtep: cleaning up vtep "
>                               "configuration");
> -    return vtep_lswitch_cleanup(ctx->vtep_idl);
> +    all_done = vtep_lswitch_cleanup(ctx->vtep_idl);
> +    all_done = vtep_macs_cleanup(ctx->vtep_idl) && all_done;
> +
> +    return all_done;
> }
> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> index 0aea936..1fbedf5 100644
> --- a/tests/ovn-controller-vtep.at
> +++ b/tests/ovn-controller-vtep.at
> @@ -327,3 +327,139 @@ AT_CHECK([vtep-ctl --columns=tunnel_key list Logical_Switch | cut -d ':' -f2 | t
> 
> OVN_CONTROLLER_VTEP_STOP
> AT_CLEANUP
> +
> +
> +# Tests vtep module 'Ucast_Macs_Remote's.
> +AT_SETUP([ovn-controller-vtep - test vtep-macs 1])
> +OVN_CONTROLLER_VTEP_START
> +
> +# creates a simple logical network with the vtep device and a fake hv chassis
> +# 'ch0'.
> +AT_CHECK([ovn-nbctl lport-add br-test vif0])
> +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
> +AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> +AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> +
> +# creates the logical switch in vtep and adds the corresponding logical
> +# port to 'br-test'.
> +AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
> +OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep], [lswitch0])
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-vtep_lswitch0`"])
> +
> +# adds another lswitch 'br-void' in ovn-nb database.
> +AT_CHECK([ovn-nbctl lswitch-add br-void])
> +# adds fake hv chassis 'ch1'.
> +AT_CHECK([ovn-nbctl lport-add br-void vif1])
> +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> +AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> +AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> +
> +# checks Ucast_Macs_Remote creation.
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep _uuid`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' '], [0], [dnl
> +"f0:ab:cd:ef:01:02"
> +])
> +
> +# checks physical locator creation.
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Physical_Locator | grep _uuid`"])
> +AT_CHECK([vtep-ctl --columns=dst_ip list Physical_Locator | cut -d ':' -f2 | tr -d ' ' | grep -v 1.2.3.4 | sed '/^$/d'], [0], [dnl
> +"1.2.3.5"
> +])
> +
> +# checks tunnel creation by ovs-vtep.
> +OVS_WAIT_UNTIL([test -n "`ovs-vsctl list Interface bfd1.2.3.5`"])
> +AT_CHECK([ovs-vsctl --columns=options list Interface bfd1.2.3.5 | cut -d ':' -f2 | tr -d ' '], [0], [dnl
> +{remote_ip="1.2.3.5"}
> +])
> +
> +# adds another mac to lport.
> +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02 f0:ab:cd:ef:01:03])
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 03`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
> +
> +"f0:ab:cd:ef:01:02"
> +"f0:ab:cd:ef:01:03"
> +])
> +
> +# removes one mac to lport.
> +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:03])
> +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 02`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
> +"f0:ab:cd:ef:01:03"
> +])
> +
> +# migrates mac to lport vif1 on 'br-void'.
> +AT_CHECK([ovn-nbctl lport-set-macs vif0])
> +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:03])
> +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 03`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
> +])
> +
> +OVN_CONTROLLER_VTEP_STOP
> +AT_CLEANUP
> +
> +
> +# Tests vtep module 'Ucast_Macs_Remote's (corner cases).
> +AT_SETUP([ovn-controller-vtep - test vtep-macs 2])
> +OVN_CONTROLLER_VTEP_START
> +
> +# creates a simple logical network with the vtep device and a fake hv chassis
> +# 'ch0'.
> +AT_CHECK([ovn-nbctl lport-add br-test vif0])
> +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
> +AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> +AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> +
> +# creates another vif in the same logical switch with duplicate mac.
> +AT_CHECK([ovn-nbctl lport-add br-test vif1])
> +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> +AT_CHECK([ovn-sbctl lport-bind vif1 ch0])
> +
> +# creates the logical switch in vtep and adds the corresponding logical
> +# port to 'br-test'.
> +AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
> +OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep], [lswitch0])
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-vtep_lswitch0`"])
> +
> +# checks Ucast_Macs_Remote creation.  Should still only be one entry, since duplicate
> +# mac in the same logical switch is not allowed.
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep _uuid`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' '], [0], [dnl
> +"f0:ab:cd:ef:01:02"
> +])
> +# confirms the warning log.
> +OVS_WAIT_UNTIL([test -n "`grep WARN ovn-controller-vtep.log`"])
> +AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' ovn-controller-vtep.log | sed 's/([[-_:0-9a-z]][[-_:0-9a-z]]*)/()/g' | uniq], [0], [dnl
> +|WARN|MAC address () has already been known to be on logical port () in the same logical datapath, so just ignore this logical port ()
> +])
> +
> +# deletes vif1.
> +AT_CHECK([ovn-nbctl lport-del vif1])
> +
> +# adds another lswitch 'br-void' in ovn-nb database.
> +AT_CHECK([ovn-nbctl lswitch-add br-void])
> +# adds fake hv chassis 'ch1' and vif1 with same mac address as vif0.
> +AT_CHECK([ovn-nbctl lport-add br-void vif1])
> +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> +AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> +AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> +
> +# creates another logical switch in vtep and adds the corresponding logical
> +# port to 'br-void'.
> +AT_CHECK([vtep-ctl add-ls lswitch1 -- bind-ls br-vtep p0 200 lswitch1])
> +OVN_NB_ADD_VTEP_PORT([br-void], [br-void_lswitch1], [br-vtep], [lswitch1])
> +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-void_lswitch1`"])
> +
> +# checks Ucast_Macs_Remote creation.  Should see two entries since it is allowed
> +# to have duplicate macs in different logical switches.
> +OVS_WAIT_UNTIL([test `vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 02 | wc -l` -gt 1])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
> +
> +"f0:ab:cd:ef:01:02"
> +"f0:ab:cd:ef:01:02"
> +])
> +
> +OVN_CONTROLLER_VTEP_STOP([/has already been known to be on logical port/d])
> +AT_CLEANUP
> -- 
> 1.9.1
>
alex wang Sept. 15, 2015, 6:40 a.m. UTC | #2
Thx for the review,

Applied to master~,

On 14 September 2015 at 16:46, Justin Pettit <jpettit@nicira.com> wrote:

> Looks good.  Thanks!
>
> Acked-by: Justin Pettit <jpettit@nicira.com>
>
> --Justin
>
>
> > On Sep 13, 2015, at 2:39 PM, Alex Wang <ee07b291@gmail.com> wrote:
> >
> > This commit extends the vtep module to support creating the
> > 'Ucast_Macs_Remote' table entries in the vtep database for
> > MAC addresses on the ovn logical ports.
> >
> > Signed-off-by: Alex Wang <ee07b291@gmail.com>
> > Acked-by: Russell Bryant <rbryant@redhat.com>
> > ---
> > V9->V10:
> > - Refine comments as suggested by Justin.
> > - Add check for 'vtep_ls->n_tunnel_key' being 0.
> > - Allow overlapping MAC in different logical switches.
> > - Fix not free'ed 'vtep_pswitches'.
> >
> > V8->V9:
> > - Add Ack from Russell.
> > - Minor adjustment based on Russell's review.
> >
> > V7->V8:
> > - rebase.
> >
> > V6->V7:
> > - rebase.
> > - adopt suggestions from Russell.
> >
> > V5->V6:
> > - rebase.
> >
> > V4->V5:
> > - rebase on top of master.
> > - rewrite the feature since a lot have changed.
> >
> > V3->V4:
> > - add logic to remove Ucast_Macs_Remote for non-existent MACs.
> >
> > V2->V3:
> > - rebase to master.
> >
> > PATCH->V2:
> > - split into separate commit.
> > - few optimizations.
> > ---
> > ovn/controller-vtep/vtep.c   | 316
> ++++++++++++++++++++++++++++++++++++++-----
> > tests/ovn-controller-vtep.at | 136 +++++++++++++++++++
> > 2 files changed, 418 insertions(+), 34 deletions(-)
> >
> > diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
> > index bae8bd5..016c2e0 100644
> > --- a/ovn/controller-vtep/vtep.c
> > +++ b/ovn/controller-vtep/vtep.c
> > @@ -19,6 +19,7 @@
> >
> > #include "lib/hash.h"
> > #include "lib/hmap.h"
> > +#include "lib/shash.h"
> > #include "lib/smap.h"
> > #include "lib/sset.h"
> > #include "lib/util.h"
> > @@ -30,46 +31,79 @@
> > VLOG_DEFINE_THIS_MODULE(vtep);
> >
> > /*
> > - * Scans through the Binding table in ovnsb and updates the vtep logical
> > - * switch tunnel keys.
> > + * Scans through the Binding table in ovnsb, and updates the vtep
> logical
> > + * switch tunnel keys and the 'Ucast_Macs_Remote' table in the VTEP
> > + * database.
> >  *
> >  */
> >
> > +/* Searches the 'chassis_rec->encaps' for the first vtep tunnel
> > + * configuration, returns the 'ip'.  Unless duplicated, the returned
> > + * pointer cannot live past current vtep_run() execution. */
> > +static const char *
> > +get_chassis_vtep_ip(const struct sbrec_chassis *chassis_rec)
> > +{
> > +    if (chassis_rec) {
> > +        size_t i;
> > +
> > +        for (i = 0; i < chassis_rec->n_encaps; i++) {
> > +            if (!strcmp(chassis_rec->encaps[i]->type, "vxlan")) {
> > +                return chassis_rec->encaps[i]->ip;
> > +            }
> > +        }
> > +    }
> > +
> > +    return NULL;
> > +}
> > +
> > +/* Creates a new 'Ucast_Macs_Remote'. */
> > +static struct vteprec_ucast_macs_remote *
> > +create_umr(struct ovsdb_idl_txn *vtep_idl_txn, const char *mac,
> > +           const struct vteprec_logical_switch *vtep_ls)
> > +{
> > +    struct vteprec_ucast_macs_remote *new_umr;
> > +
> > +    new_umr = vteprec_ucast_macs_remote_insert(vtep_idl_txn);
> > +    vteprec_ucast_macs_remote_set_MAC(new_umr, mac);
> > +    vteprec_ucast_macs_remote_set_logical_switch(new_umr, vtep_ls);
> > +
> > +    return new_umr;
> > +}
> > +
> > +/* Creates a new 'Physical_Locator'. */
> > +static struct vteprec_physical_locator *
> > +create_pl(struct ovsdb_idl_txn *vtep_idl_txn, const char *chassis_ip)
> > +{
> > +    struct vteprec_physical_locator *new_pl;
> > +
> > +    new_pl = vteprec_physical_locator_insert(vtep_idl_txn);
> > +    vteprec_physical_locator_set_dst_ip(new_pl, chassis_ip);
> > +    vteprec_physical_locator_set_encapsulation_type(new_pl,
> VTEP_ENCAP_TYPE);
> > +
> > +    return new_pl;
> > +}
> > +
> > +
> > /* Updates the vtep Logical_Switch table entries' tunnel keys based
> >  * on the port bindings. */
> > static void
> > -vtep_lswitch_run(struct controller_vtep_ctx *ctx)
> > +vtep_lswitch_run(struct shash *vtep_pbs, struct sset *vtep_pswitches,
> > +                 struct shash *vtep_lswitches)
> > {
> > -    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
> > -    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
> >     struct sset used_ls = SSET_INITIALIZER(&used_ls);
> > -    const struct vteprec_physical_switch *pswitch;
> > -    const struct sbrec_port_binding *port_binding_rec;
> > -    const struct vteprec_logical_switch *vtep_ls;
> > -
> > -    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (pswitch, ctx->vtep_idl) {
> > -        sset_add(&vtep_pswitches, pswitch->name);
> > -    }
> > -    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
> > -        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
> > -    }
> > +    struct shash_node *node;
> >
> > -    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
> > -                              "ovn-controller-vtep: update logical
> switch "
> > -                              "tunnel keys");
> >     /* Collects the logical switch bindings from port binding entries.
> >      * Since the binding module has already guaranteed that each vtep
> >      * logical switch is bound only to one ovn-sb logical datapath,
> >      * we can just iterate and assign tunnel key to vtep logical switch.
> */
> > -    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
> > -        if (strcmp(port_binding_rec->type, "vtep")
> > -            || !port_binding_rec->chassis) {
> > -            continue;
> > -        }
> > +    SHASH_FOR_EACH (node, vtep_pbs) {
> > +        const struct sbrec_port_binding *port_binding_rec = node->data;
> >         const char *pswitch_name = smap_get(&port_binding_rec->options,
> >                                             "vtep-physical-switch");
> >         const char *lswitch_name = smap_get(&port_binding_rec->options,
> >                                             "vtep-logical-switch");
> > +        const struct vteprec_logical_switch *vtep_ls;
> >
> >         /* If 'port_binding_rec->chassis' exists then 'pswitch_name'
> >          * and 'lswitch_name' must also exist. */
> > @@ -83,11 +117,11 @@ vtep_lswitch_run(struct controller_vtep_ctx *ctx)
> >                      port_binding_rec->chassis->name);
> >             continue;
> >         }
> > -        vtep_ls = shash_find_data(&vtep_lswitches, lswitch_name);
> > +        vtep_ls = shash_find_data(vtep_lswitches, lswitch_name);
> >         /* Also checks 'pswitch_name' since the same 'lswitch_name' could
> >          * exist in multiple vtep database instances and be bound to
> different
> >          * ovn logical networks. */
> > -        if (vtep_ls && sset_find(&vtep_pswitches, pswitch_name)) {
> > +        if (vtep_ls && sset_find(vtep_pswitches, pswitch_name)) {
> >             int64_t tnl_key;
> >
> >             if (sset_find(&used_ls, lswitch_name)) {
> > @@ -105,21 +139,155 @@ vtep_lswitch_run(struct controller_vtep_ctx *ctx)
> >             sset_add(&used_ls, lswitch_name);
> >         }
> >     }
> > -    struct shash_node *node;
> > -    /* Resets the tunnel keys for the rest of vtep logical switches. */
> > -    SHASH_FOR_EACH (node, &vtep_lswitches) {
> > +    /* Resets the tunnel keys for unused vtep logical switches. */
> > +    SHASH_FOR_EACH (node, vtep_lswitches) {
> >         if (!sset_find(&used_ls, node->name)) {
> >             int64_t tnl_key = 0;
> > -
> >             vteprec_logical_switch_set_tunnel_key(node->data, &tnl_key,
> 1);
> >         }
> >     }
> > -
> > -    shash_destroy(&vtep_lswitches);
> > -    sset_destroy(&vtep_pswitches);
> >     sset_destroy(&used_ls);
> > }
> >
> > +/* Updates the vtep 'Ucast_Macs_Remote' table based on non-vtep port
> > + * bindings. */
> > +static void
> > +vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct shash
> *ucast_macs_rmts,
> > +              struct shash *physical_locators, struct shash
> *vtep_lswitches,
> > +              struct shash *non_vtep_pbs)
> > +{
> > +    struct shash_node *node;
> > +    struct hmap ls_map;
> > +
> > +    /* Maps from ovn logical datapath tunnel key (which is also the vtep
> > +     * logical switch tunnel key) to the corresponding vtep logical
> switch
> > +     * instance.  Also, the shash map 'added_macs' is used for checking
> > +     * duplicated MAC addresses in the same ovn logical datapath. */
> > +    struct ls_hash_node {
> > +        struct hmap_node hmap_node;
> > +
> > +        const struct vteprec_logical_switch *vtep_ls;
> > +        struct shash added_macs;
> > +    };
> > +
> > +    hmap_init(&ls_map);
> > +    SHASH_FOR_EACH (node, vtep_lswitches) {
> > +        const struct vteprec_logical_switch *vtep_ls = node->data;
> > +        struct ls_hash_node *ls_node;
> > +
> > +        if (!vtep_ls->n_tunnel_key) {
> > +            continue;
> > +        }
> > +        ls_node = xmalloc(sizeof *ls_node);
> > +        ls_node->vtep_ls = vtep_ls;
> > +        shash_init(&ls_node->added_macs);
> > +        hmap_insert(&ls_map, &ls_node->hmap_node,
> > +                    hash_uint64((uint64_t) vtep_ls->tunnel_key[0]));
> > +    }
> > +
> > +    SHASH_FOR_EACH (node, non_vtep_pbs) {
> > +        const struct sbrec_port_binding *port_binding_rec = node->data;
> > +        const struct sbrec_chassis *chassis_rec;
> > +        struct ls_hash_node *ls_node;
> > +        const char *chassis_ip;
> > +        int64_t tnl_key;
> > +        size_t i;
> > +
> > +        chassis_rec = port_binding_rec->chassis;
> > +        if (!chassis_rec) {
> > +            continue;
> > +        }
> > +
> > +        tnl_key = port_binding_rec->datapath->tunnel_key;
> > +        HMAP_FOR_EACH_WITH_HASH (ls_node, hmap_node,
> > +                                 hash_uint64((uint64_t) tnl_key),
> > +                                 &ls_map) {
> > +            if (ls_node->vtep_ls->tunnel_key[0] == tnl_key) {
> > +                break;
> > +            }
> > +        }
> > +        /* If 'ls_node' is NULL, that means no vtep logical switch is
> > +         * attached to the corresponding ovn logical datapath, so pass.
> > +         */
> > +        if (!ls_node) {
> > +            continue;
> > +        }
> > +
> > +        chassis_ip = get_chassis_vtep_ip(chassis_rec);
> > +        /* Unreachable chassis, continue. */
> > +        if (!chassis_ip) {
> > +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
> 5);
> > +            VLOG_INFO_RL(&rl, "VTEP tunnel encap on chassis (%s) not
> found",
> > +                         chassis_rec->name);
> > +            continue;
> > +        }
> > +
> > +        for (i = 0; i < port_binding_rec->n_mac; i++) {
> > +            const struct vteprec_ucast_macs_remote *umr;
> > +            const struct vteprec_physical_locator *pl;
> > +            const struct sbrec_port_binding *conflict;
> > +            char *mac = port_binding_rec->mac[i];
> > +
> > +            /* xxx Need to address this later when we support
> > +             * update of 'Mcast_Macs_Remote' table in VTEP. */
> > +            if (!strcmp(mac, "unknown")) {
> > +                continue;
> > +            }
> > +
> > +            /* Checks for duplicate MAC in the same vtep logical
> switch. */
> > +            conflict = shash_find_data(&ls_node->added_macs, mac);
> > +            if (conflict) {
> > +                VLOG_WARN("MAC address (%s) has already been known to
> be "
> > +                          "on logical port (%s) in the same logical "
> > +                          "datapath, so just ignore this logical port
> (%s)",
> > +                          mac, conflict->logical_port,
> > +                          port_binding_rec->logical_port);
> > +                continue;
> > +            }
> > +            shash_add(&ls_node->added_macs, mac, port_binding_rec);
> > +
> > +            char *mac_ip_tnlkey = xasprintf("%s_%s_%"PRId64, mac,
> chassis_ip,
> > +                                            tnl_key);
> > +            umr = shash_find_data(ucast_macs_rmts, mac_ip_tnlkey);
> > +            /* If finds the 'umr' entry for the mac, ip, and tnl_key,
> deletes
> > +             * the entry from shash so that it is not gargage collected.
> > +             *
> > +             * If not found, creates a new 'umr' entry. */
> > +            if (umr && umr->logical_switch == ls_node->vtep_ls) {
> > +                shash_find_and_delete(ucast_macs_rmts, mac_ip_tnlkey);
> > +            } else {
> > +                const struct vteprec_ucast_macs_remote *new_umr;
> > +
> > +                new_umr = create_umr(vtep_idl_txn, mac,
> ls_node->vtep_ls);
> > +                pl = shash_find_data(physical_locators, chassis_ip);
> > +                if (pl) {
> > +                    vteprec_ucast_macs_remote_set_locator(new_umr, pl);
> > +                } else {
> > +                    const struct vteprec_physical_locator *new_pl;
> > +
> > +                    new_pl = create_pl(vtep_idl_txn, chassis_ip);
> > +                    vteprec_ucast_macs_remote_set_locator(new_umr,
> new_pl);
> > +                    /* Updates the 'physical_locators'. */
> > +                    shash_add(physical_locators, chassis_ip, new_pl);
> > +                }
> > +            }
> > +            free(mac_ip_tnlkey);
> > +        }
> > +    }
> > +
> > +    /* Removes all remaining 'umr's, since they do not exist anymore. */
> > +    SHASH_FOR_EACH (node, ucast_macs_rmts) {
> > +        vteprec_ucast_macs_remote_delete(node->data);
> > +    }
> > +    struct ls_hash_node *iter, *next;
> > +    HMAP_FOR_EACH_SAFE (iter, next, hmap_node, &ls_map) {
> > +        hmap_remove(&ls_map, &iter->hmap_node);
> > +        shash_destroy(&iter->added_macs);
> > +        free(iter);
> > +    }
> > +    hmap_destroy(&ls_map);
> > +}
> > +
> > /* Resets all logical switches' 'tunnel_key' to NULL */
> > static bool
> > vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
> > @@ -137,6 +305,19 @@ vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
> >     return done;
> > }
> >
> > +/* Removes all entries in the 'Ucast_Macs_Remote' table in vtep
> database.
> > + * Returns true when all done (no entry to remove). */
> > +static bool
> > +vtep_macs_cleanup(struct ovsdb_idl *vtep_idl)
> > +{
> > +    const struct vteprec_ucast_macs_remote *umr;
> > +
> > +    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, vtep_idl) {
> > +        vteprec_ucast_macs_remote_delete(umr);
> > +        return false;
> > +    }
> > +    return true;
> > +}
> >
> > /* Updates vtep logical switch tunnel keys. */
> > void
> > @@ -145,7 +326,69 @@ vtep_run(struct controller_vtep_ctx *ctx)
> >     if (!ctx->vtep_idl_txn) {
> >         return;
> >     }
> > -    vtep_lswitch_run(ctx);
> > +
> > +    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
> > +    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
> > +    struct shash ucast_macs_rmts = SHASH_INITIALIZER(&ucast_macs_rmts);
> > +    struct shash physical_locators =
> SHASH_INITIALIZER(&physical_locators);
> > +    struct shash vtep_pbs = SHASH_INITIALIZER(&vtep_pbs);
> > +    struct shash non_vtep_pbs = SHASH_INITIALIZER(&non_vtep_pbs);
> > +    const struct vteprec_physical_switch *vtep_ps;
> > +    const struct vteprec_logical_switch *vtep_ls;
> > +    const struct vteprec_ucast_macs_remote *umr;
> > +    const struct vteprec_physical_locator *pl;
> > +    const struct sbrec_port_binding *port_binding_rec;
> > +
> > +    /* Collects 'Physical_Switch's. */
> > +    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (vtep_ps, ctx->vtep_idl) {
> > +        sset_add(&vtep_pswitches, vtep_ps->name);
> > +    }
> > +
> > +    /* Collects 'Logical_Switch's. */
> > +    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
> > +        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
> > +    }
> > +
> > +    /* Collects 'Ucast_Macs_Remote's. */
> > +    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, ctx->vtep_idl) {
> > +        char *mac_ip_tnlkey =
> > +            xasprintf("%s_%s_%"PRId64, umr->MAC,
> > +                      umr->locator ? umr->locator->dst_ip : "",
> > +                      umr->logical_switch &&
> umr->logical_switch->n_tunnel_key
> > +                          ? umr->logical_switch->tunnel_key[0] :
> INT64_MAX);
> > +
> > +        shash_add(&ucast_macs_rmts, mac_ip_tnlkey, umr);
> > +        free(mac_ip_tnlkey);
> > +    }
> > +    /* Collects 'Physical_Locator's. */
> > +    VTEPREC_PHYSICAL_LOCATOR_FOR_EACH (pl, ctx->vtep_idl) {
> > +        shash_add(&physical_locators, pl->dst_ip, pl);
> > +    }
> > +    /* Collects and classifies 'Port_Binding's. */
> > +    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
> > +        struct shash *target =
> > +            !strcmp(port_binding_rec->type, "vtep") ? &vtep_pbs :
> &non_vtep_pbs;
> > +
> > +        if (!port_binding_rec->chassis) {
> > +            continue;
> > +        }
> > +        shash_add(target, port_binding_rec->logical_port,
> port_binding_rec);
> > +    }
> > +
> > +    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
> > +                              "ovn-controller-vtep: update logical
> switch "
> > +                              "tunnel keys and 'ucast_macs_remote's");
> > +
> > +    vtep_lswitch_run(&vtep_pbs, &vtep_pswitches, &vtep_lswitches);
> > +    vtep_macs_run(ctx->vtep_idl_txn, &ucast_macs_rmts,
> &physical_locators,
> > +                  &vtep_lswitches, &non_vtep_pbs);
> > +
> > +    sset_destroy(&vtep_pswitches);
> > +    shash_destroy(&vtep_lswitches);
> > +    shash_destroy(&ucast_macs_rmts);
> > +    shash_destroy(&physical_locators);
> > +    shash_destroy(&vtep_pbs);
> > +    shash_destroy(&non_vtep_pbs);
> > }
> >
> > /* Cleans up all related entries in vtep.  Returns true when done (i.e.
> > @@ -157,8 +400,13 @@ vtep_cleanup(struct controller_vtep_ctx *ctx)
> >         return false;
> >     }
> >
> > +    bool all_done;
> > +
> >     ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
> >                               "ovn-controller-vtep: cleaning up vtep "
> >                               "configuration");
> > -    return vtep_lswitch_cleanup(ctx->vtep_idl);
> > +    all_done = vtep_lswitch_cleanup(ctx->vtep_idl);
> > +    all_done = vtep_macs_cleanup(ctx->vtep_idl) && all_done;
> > +
> > +    return all_done;
> > }
> > diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> > index 0aea936..1fbedf5 100644
> > --- a/tests/ovn-controller-vtep.at
> > +++ b/tests/ovn-controller-vtep.at
> > @@ -327,3 +327,139 @@ AT_CHECK([vtep-ctl --columns=tunnel_key list
> Logical_Switch | cut -d ':' -f2 | t
> >
> > OVN_CONTROLLER_VTEP_STOP
> > AT_CLEANUP
> > +
> > +
> > +# Tests vtep module 'Ucast_Macs_Remote's.
> > +AT_SETUP([ovn-controller-vtep - test vtep-macs 1])
> > +OVN_CONTROLLER_VTEP_START
> > +
> > +# creates a simple logical network with the vtep device and a fake hv
> chassis
> > +# 'ch0'.
> > +AT_CHECK([ovn-nbctl lport-add br-test vif0])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
> > +AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> > +AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> > +
> > +# creates the logical switch in vtep and adds the corresponding logical
> > +# port to 'br-test'.
> > +AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
> > +OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep],
> [lswitch0])
> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep
> br-vtep_lswitch0`"])
> > +
> > +# adds another lswitch 'br-void' in ovn-nb database.
> > +AT_CHECK([ovn-nbctl lswitch-add br-void])
> > +# adds fake hv chassis 'ch1'.
> > +AT_CHECK([ovn-nbctl lport-add br-void vif1])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> > +AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> > +AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> > +
> > +# checks Ucast_Macs_Remote creation.
> > +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep
> _uuid`"])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' '], [0], [dnl
> > +"f0:ab:cd:ef:01:02"
> > +])
> > +
> > +# checks physical locator creation.
> > +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Physical_Locator | grep
> _uuid`"])
> > +AT_CHECK([vtep-ctl --columns=dst_ip list Physical_Locator | cut -d ':'
> -f2 | tr -d ' ' | grep -v 1.2.3.4 | sed '/^$/d'], [0], [dnl
> > +"1.2.3.5"
> > +])
> > +
> > +# checks tunnel creation by ovs-vtep.
> > +OVS_WAIT_UNTIL([test -n "`ovs-vsctl list Interface bfd1.2.3.5`"])
> > +AT_CHECK([ovs-vsctl --columns=options list Interface bfd1.2.3.5 | cut
> -d ':' -f2 | tr -d ' '], [0], [dnl
> > +{remote_ip="1.2.3.5"}
> > +])
> > +
> > +# adds another mac to lport.
> > +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02
> f0:ab:cd:ef:01:03])
> > +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 03`"])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' ' | sort], [0], [dnl
> > +
> > +"f0:ab:cd:ef:01:02"
> > +"f0:ab:cd:ef:01:03"
> > +])
> > +
> > +# removes one mac to lport.
> > +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:03])
> > +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote
> | grep 02`"])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' ' | sort], [0], [dnl
> > +"f0:ab:cd:ef:01:03"
> > +])
> > +
> > +# migrates mac to lport vif1 on 'br-void'.
> > +AT_CHECK([ovn-nbctl lport-set-macs vif0])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:03])
> > +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote
> | grep 03`"])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' ' | sort], [0], [dnl
> > +])
> > +
> > +OVN_CONTROLLER_VTEP_STOP
> > +AT_CLEANUP
> > +
> > +
> > +# Tests vtep module 'Ucast_Macs_Remote's (corner cases).
> > +AT_SETUP([ovn-controller-vtep - test vtep-macs 2])
> > +OVN_CONTROLLER_VTEP_START
> > +
> > +# creates a simple logical network with the vtep device and a fake hv
> chassis
> > +# 'ch0'.
> > +AT_CHECK([ovn-nbctl lport-add br-test vif0])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
> > +AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
> > +AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
> > +
> > +# creates another vif in the same logical switch with duplicate mac.
> > +AT_CHECK([ovn-nbctl lport-add br-test vif1])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> > +AT_CHECK([ovn-sbctl lport-bind vif1 ch0])
> > +
> > +# creates the logical switch in vtep and adds the corresponding logical
> > +# port to 'br-test'.
> > +AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
> > +OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep],
> [lswitch0])
> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep
> br-vtep_lswitch0`"])
> > +
> > +# checks Ucast_Macs_Remote creation.  Should still only be one entry,
> since duplicate
> > +# mac in the same logical switch is not allowed.
> > +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep
> _uuid`"])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' '], [0], [dnl
> > +"f0:ab:cd:ef:01:02"
> > +])
> > +# confirms the warning log.
> > +OVS_WAIT_UNTIL([test -n "`grep WARN ovn-controller-vtep.log`"])
> > +AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' ovn-controller-vtep.log |
> sed 's/([[-_:0-9a-z]][[-_:0-9a-z]]*)/()/g' | uniq], [0], [dnl
> > +|WARN|MAC address () has already been known to be on logical port () in
> the same logical datapath, so just ignore this logical port ()
> > +])
> > +
> > +# deletes vif1.
> > +AT_CHECK([ovn-nbctl lport-del vif1])
> > +
> > +# adds another lswitch 'br-void' in ovn-nb database.
> > +AT_CHECK([ovn-nbctl lswitch-add br-void])
> > +# adds fake hv chassis 'ch1' and vif1 with same mac address as vif0.
> > +AT_CHECK([ovn-nbctl lport-add br-void vif1])
> > +AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
> > +AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
> > +AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
> > +
> > +# creates another logical switch in vtep and adds the corresponding
> logical
> > +# port to 'br-void'.
> > +AT_CHECK([vtep-ctl add-ls lswitch1 -- bind-ls br-vtep p0 200 lswitch1])
> > +OVN_NB_ADD_VTEP_PORT([br-void], [br-void_lswitch1], [br-vtep],
> [lswitch1])
> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep
> br-void_lswitch1`"])
> > +
> > +# checks Ucast_Macs_Remote creation.  Should see two entries since it
> is allowed
> > +# to have duplicate macs in different logical switches.
> > +OVS_WAIT_UNTIL([test `vtep-ctl --columns=MAC list Ucast_Macs_Remote |
> grep 02 | wc -l` -gt 1])
> > +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':'
> -f2- | tr -d ' ' | sort], [0], [dnl
> > +
> > +"f0:ab:cd:ef:01:02"
> > +"f0:ab:cd:ef:01:02"
> > +])
> > +
> > +OVN_CONTROLLER_VTEP_STOP([/has already been known to be on logical
> port/d])
> > +AT_CLEANUP
> > --
> > 1.9.1
> >
>
>
diff mbox

Patch

diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
index bae8bd5..016c2e0 100644
--- a/ovn/controller-vtep/vtep.c
+++ b/ovn/controller-vtep/vtep.c
@@ -19,6 +19,7 @@ 
 
 #include "lib/hash.h"
 #include "lib/hmap.h"
+#include "lib/shash.h"
 #include "lib/smap.h"
 #include "lib/sset.h"
 #include "lib/util.h"
@@ -30,46 +31,79 @@ 
 VLOG_DEFINE_THIS_MODULE(vtep);
 
 /*
- * Scans through the Binding table in ovnsb and updates the vtep logical
- * switch tunnel keys.
+ * Scans through the Binding table in ovnsb, and updates the vtep logical
+ * switch tunnel keys and the 'Ucast_Macs_Remote' table in the VTEP
+ * database.
  *
  */
 
+/* Searches the 'chassis_rec->encaps' for the first vtep tunnel
+ * configuration, returns the 'ip'.  Unless duplicated, the returned
+ * pointer cannot live past current vtep_run() execution. */
+static const char *
+get_chassis_vtep_ip(const struct sbrec_chassis *chassis_rec)
+{
+    if (chassis_rec) {
+        size_t i;
+
+        for (i = 0; i < chassis_rec->n_encaps; i++) {
+            if (!strcmp(chassis_rec->encaps[i]->type, "vxlan")) {
+                return chassis_rec->encaps[i]->ip;
+            }
+        }
+    }
+
+    return NULL;
+}
+
+/* Creates a new 'Ucast_Macs_Remote'. */
+static struct vteprec_ucast_macs_remote *
+create_umr(struct ovsdb_idl_txn *vtep_idl_txn, const char *mac,
+           const struct vteprec_logical_switch *vtep_ls)
+{
+    struct vteprec_ucast_macs_remote *new_umr;
+
+    new_umr = vteprec_ucast_macs_remote_insert(vtep_idl_txn);
+    vteprec_ucast_macs_remote_set_MAC(new_umr, mac);
+    vteprec_ucast_macs_remote_set_logical_switch(new_umr, vtep_ls);
+
+    return new_umr;
+}
+
+/* Creates a new 'Physical_Locator'. */
+static struct vteprec_physical_locator *
+create_pl(struct ovsdb_idl_txn *vtep_idl_txn, const char *chassis_ip)
+{
+    struct vteprec_physical_locator *new_pl;
+
+    new_pl = vteprec_physical_locator_insert(vtep_idl_txn);
+    vteprec_physical_locator_set_dst_ip(new_pl, chassis_ip);
+    vteprec_physical_locator_set_encapsulation_type(new_pl, VTEP_ENCAP_TYPE);
+
+    return new_pl;
+}
+
+
 /* Updates the vtep Logical_Switch table entries' tunnel keys based
  * on the port bindings. */
 static void
-vtep_lswitch_run(struct controller_vtep_ctx *ctx)
+vtep_lswitch_run(struct shash *vtep_pbs, struct sset *vtep_pswitches,
+                 struct shash *vtep_lswitches)
 {
-    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
-    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
     struct sset used_ls = SSET_INITIALIZER(&used_ls);
-    const struct vteprec_physical_switch *pswitch;
-    const struct sbrec_port_binding *port_binding_rec;
-    const struct vteprec_logical_switch *vtep_ls;
-
-    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (pswitch, ctx->vtep_idl) {
-        sset_add(&vtep_pswitches, pswitch->name);
-    }
-    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
-        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
-    }
+    struct shash_node *node;
 
-    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
-                              "ovn-controller-vtep: update logical switch "
-                              "tunnel keys");
     /* Collects the logical switch bindings from port binding entries.
      * Since the binding module has already guaranteed that each vtep
      * logical switch is bound only to one ovn-sb logical datapath,
      * we can just iterate and assign tunnel key to vtep logical switch. */
-    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
-        if (strcmp(port_binding_rec->type, "vtep")
-            || !port_binding_rec->chassis) {
-            continue;
-        }
+    SHASH_FOR_EACH (node, vtep_pbs) {
+        const struct sbrec_port_binding *port_binding_rec = node->data;
         const char *pswitch_name = smap_get(&port_binding_rec->options,
                                             "vtep-physical-switch");
         const char *lswitch_name = smap_get(&port_binding_rec->options,
                                             "vtep-logical-switch");
+        const struct vteprec_logical_switch *vtep_ls;
 
         /* If 'port_binding_rec->chassis' exists then 'pswitch_name'
          * and 'lswitch_name' must also exist. */
@@ -83,11 +117,11 @@  vtep_lswitch_run(struct controller_vtep_ctx *ctx)
                      port_binding_rec->chassis->name);
             continue;
         }
-        vtep_ls = shash_find_data(&vtep_lswitches, lswitch_name);
+        vtep_ls = shash_find_data(vtep_lswitches, lswitch_name);
         /* Also checks 'pswitch_name' since the same 'lswitch_name' could
          * exist in multiple vtep database instances and be bound to different
          * ovn logical networks. */
-        if (vtep_ls && sset_find(&vtep_pswitches, pswitch_name)) {
+        if (vtep_ls && sset_find(vtep_pswitches, pswitch_name)) {
             int64_t tnl_key;
 
             if (sset_find(&used_ls, lswitch_name)) {
@@ -105,21 +139,155 @@  vtep_lswitch_run(struct controller_vtep_ctx *ctx)
             sset_add(&used_ls, lswitch_name);
         }
     }
-    struct shash_node *node;
-    /* Resets the tunnel keys for the rest of vtep logical switches. */
-    SHASH_FOR_EACH (node, &vtep_lswitches) {
+    /* Resets the tunnel keys for unused vtep logical switches. */
+    SHASH_FOR_EACH (node, vtep_lswitches) {
         if (!sset_find(&used_ls, node->name)) {
             int64_t tnl_key = 0;
-
             vteprec_logical_switch_set_tunnel_key(node->data, &tnl_key, 1);
         }
     }
-
-    shash_destroy(&vtep_lswitches);
-    sset_destroy(&vtep_pswitches);
     sset_destroy(&used_ls);
 }
 
+/* Updates the vtep 'Ucast_Macs_Remote' table based on non-vtep port
+ * bindings. */
+static void
+vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct shash *ucast_macs_rmts,
+              struct shash *physical_locators, struct shash *vtep_lswitches,
+              struct shash *non_vtep_pbs)
+{
+    struct shash_node *node;
+    struct hmap ls_map;
+
+    /* Maps from ovn logical datapath tunnel key (which is also the vtep
+     * logical switch tunnel key) to the corresponding vtep logical switch
+     * instance.  Also, the shash map 'added_macs' is used for checking
+     * duplicated MAC addresses in the same ovn logical datapath. */
+    struct ls_hash_node {
+        struct hmap_node hmap_node;
+
+        const struct vteprec_logical_switch *vtep_ls;
+        struct shash added_macs;
+    };
+
+    hmap_init(&ls_map);
+    SHASH_FOR_EACH (node, vtep_lswitches) {
+        const struct vteprec_logical_switch *vtep_ls = node->data;
+        struct ls_hash_node *ls_node;
+
+        if (!vtep_ls->n_tunnel_key) {
+            continue;
+        }
+        ls_node = xmalloc(sizeof *ls_node);
+        ls_node->vtep_ls = vtep_ls;
+        shash_init(&ls_node->added_macs);
+        hmap_insert(&ls_map, &ls_node->hmap_node,
+                    hash_uint64((uint64_t) vtep_ls->tunnel_key[0]));
+    }
+
+    SHASH_FOR_EACH (node, non_vtep_pbs) {
+        const struct sbrec_port_binding *port_binding_rec = node->data;
+        const struct sbrec_chassis *chassis_rec;
+        struct ls_hash_node *ls_node;
+        const char *chassis_ip;
+        int64_t tnl_key;
+        size_t i;
+
+        chassis_rec = port_binding_rec->chassis;
+        if (!chassis_rec) {
+            continue;
+        }
+
+        tnl_key = port_binding_rec->datapath->tunnel_key;
+        HMAP_FOR_EACH_WITH_HASH (ls_node, hmap_node,
+                                 hash_uint64((uint64_t) tnl_key),
+                                 &ls_map) {
+            if (ls_node->vtep_ls->tunnel_key[0] == tnl_key) {
+                break;
+            }
+        }
+        /* If 'ls_node' is NULL, that means no vtep logical switch is
+         * attached to the corresponding ovn logical datapath, so pass.
+         */
+        if (!ls_node) {
+            continue;
+        }
+
+        chassis_ip = get_chassis_vtep_ip(chassis_rec);
+        /* Unreachable chassis, continue. */
+        if (!chassis_ip) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            VLOG_INFO_RL(&rl, "VTEP tunnel encap on chassis (%s) not found",
+                         chassis_rec->name);
+            continue;
+        }
+
+        for (i = 0; i < port_binding_rec->n_mac; i++) {
+            const struct vteprec_ucast_macs_remote *umr;
+            const struct vteprec_physical_locator *pl;
+            const struct sbrec_port_binding *conflict;
+            char *mac = port_binding_rec->mac[i];
+
+            /* xxx Need to address this later when we support
+             * update of 'Mcast_Macs_Remote' table in VTEP. */
+            if (!strcmp(mac, "unknown")) {
+                continue;
+            }
+
+            /* Checks for duplicate MAC in the same vtep logical switch. */
+            conflict = shash_find_data(&ls_node->added_macs, mac);
+            if (conflict) {
+                VLOG_WARN("MAC address (%s) has already been known to be "
+                          "on logical port (%s) in the same logical "
+                          "datapath, so just ignore this logical port (%s)",
+                          mac, conflict->logical_port,
+                          port_binding_rec->logical_port);
+                continue;
+            }
+            shash_add(&ls_node->added_macs, mac, port_binding_rec);
+
+            char *mac_ip_tnlkey = xasprintf("%s_%s_%"PRId64, mac, chassis_ip,
+                                            tnl_key);
+            umr = shash_find_data(ucast_macs_rmts, mac_ip_tnlkey);
+            /* If finds the 'umr' entry for the mac, ip, and tnl_key, deletes
+             * the entry from shash so that it is not gargage collected.
+             *
+             * If not found, creates a new 'umr' entry. */
+            if (umr && umr->logical_switch == ls_node->vtep_ls) {
+                shash_find_and_delete(ucast_macs_rmts, mac_ip_tnlkey);
+            } else {
+                const struct vteprec_ucast_macs_remote *new_umr;
+
+                new_umr = create_umr(vtep_idl_txn, mac, ls_node->vtep_ls);
+                pl = shash_find_data(physical_locators, chassis_ip);
+                if (pl) {
+                    vteprec_ucast_macs_remote_set_locator(new_umr, pl);
+                } else {
+                    const struct vteprec_physical_locator *new_pl;
+
+                    new_pl = create_pl(vtep_idl_txn, chassis_ip);
+                    vteprec_ucast_macs_remote_set_locator(new_umr, new_pl);
+                    /* Updates the 'physical_locators'. */
+                    shash_add(physical_locators, chassis_ip, new_pl);
+                }
+            }
+            free(mac_ip_tnlkey);
+        }
+    }
+
+    /* Removes all remaining 'umr's, since they do not exist anymore. */
+    SHASH_FOR_EACH (node, ucast_macs_rmts) {
+        vteprec_ucast_macs_remote_delete(node->data);
+    }
+    struct ls_hash_node *iter, *next;
+    HMAP_FOR_EACH_SAFE (iter, next, hmap_node, &ls_map) {
+        hmap_remove(&ls_map, &iter->hmap_node);
+        shash_destroy(&iter->added_macs);
+        free(iter);
+    }
+    hmap_destroy(&ls_map);
+}
+
 /* Resets all logical switches' 'tunnel_key' to NULL */
 static bool
 vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
@@ -137,6 +305,19 @@  vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
     return done;
 }
 
+/* Removes all entries in the 'Ucast_Macs_Remote' table in vtep database.
+ * Returns true when all done (no entry to remove). */
+static bool
+vtep_macs_cleanup(struct ovsdb_idl *vtep_idl)
+{
+    const struct vteprec_ucast_macs_remote *umr;
+
+    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, vtep_idl) {
+        vteprec_ucast_macs_remote_delete(umr);
+        return false;
+    }
+    return true;
+}
 
 /* Updates vtep logical switch tunnel keys. */
 void
@@ -145,7 +326,69 @@  vtep_run(struct controller_vtep_ctx *ctx)
     if (!ctx->vtep_idl_txn) {
         return;
     }
-    vtep_lswitch_run(ctx);
+
+    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
+    struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches);
+    struct shash ucast_macs_rmts = SHASH_INITIALIZER(&ucast_macs_rmts);
+    struct shash physical_locators = SHASH_INITIALIZER(&physical_locators);
+    struct shash vtep_pbs = SHASH_INITIALIZER(&vtep_pbs);
+    struct shash non_vtep_pbs = SHASH_INITIALIZER(&non_vtep_pbs);
+    const struct vteprec_physical_switch *vtep_ps;
+    const struct vteprec_logical_switch *vtep_ls;
+    const struct vteprec_ucast_macs_remote *umr;
+    const struct vteprec_physical_locator *pl;
+    const struct sbrec_port_binding *port_binding_rec;
+
+    /* Collects 'Physical_Switch's. */
+    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (vtep_ps, ctx->vtep_idl) {
+        sset_add(&vtep_pswitches, vtep_ps->name);
+    }
+
+    /* Collects 'Logical_Switch's. */
+    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) {
+        shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls);
+    }
+
+    /* Collects 'Ucast_Macs_Remote's. */
+    VTEPREC_UCAST_MACS_REMOTE_FOR_EACH (umr, ctx->vtep_idl) {
+        char *mac_ip_tnlkey =
+            xasprintf("%s_%s_%"PRId64, umr->MAC,
+                      umr->locator ? umr->locator->dst_ip : "",
+                      umr->logical_switch && umr->logical_switch->n_tunnel_key
+                          ? umr->logical_switch->tunnel_key[0] : INT64_MAX);
+
+        shash_add(&ucast_macs_rmts, mac_ip_tnlkey, umr);
+        free(mac_ip_tnlkey);
+    }
+    /* Collects 'Physical_Locator's. */
+    VTEPREC_PHYSICAL_LOCATOR_FOR_EACH (pl, ctx->vtep_idl) {
+        shash_add(&physical_locators, pl->dst_ip, pl);
+    }
+    /* Collects and classifies 'Port_Binding's. */
+    SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) {
+        struct shash *target =
+            !strcmp(port_binding_rec->type, "vtep") ? &vtep_pbs : &non_vtep_pbs;
+
+        if (!port_binding_rec->chassis) {
+            continue;
+        }
+        shash_add(target, port_binding_rec->logical_port, port_binding_rec);
+    }
+
+    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
+                              "ovn-controller-vtep: update logical switch "
+                              "tunnel keys and 'ucast_macs_remote's");
+
+    vtep_lswitch_run(&vtep_pbs, &vtep_pswitches, &vtep_lswitches);
+    vtep_macs_run(ctx->vtep_idl_txn, &ucast_macs_rmts, &physical_locators,
+                  &vtep_lswitches, &non_vtep_pbs);
+
+    sset_destroy(&vtep_pswitches);
+    shash_destroy(&vtep_lswitches);
+    shash_destroy(&ucast_macs_rmts);
+    shash_destroy(&physical_locators);
+    shash_destroy(&vtep_pbs);
+    shash_destroy(&non_vtep_pbs);
 }
 
 /* Cleans up all related entries in vtep.  Returns true when done (i.e.
@@ -157,8 +400,13 @@  vtep_cleanup(struct controller_vtep_ctx *ctx)
         return false;
     }
 
+    bool all_done;
+
     ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
                               "ovn-controller-vtep: cleaning up vtep "
                               "configuration");
-    return vtep_lswitch_cleanup(ctx->vtep_idl);
+    all_done = vtep_lswitch_cleanup(ctx->vtep_idl);
+    all_done = vtep_macs_cleanup(ctx->vtep_idl) && all_done;
+
+    return all_done;
 }
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 0aea936..1fbedf5 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -327,3 +327,139 @@  AT_CHECK([vtep-ctl --columns=tunnel_key list Logical_Switch | cut -d ':' -f2 | t
 
 OVN_CONTROLLER_VTEP_STOP
 AT_CLEANUP
+
+
+# Tests vtep module 'Ucast_Macs_Remote's.
+AT_SETUP([ovn-controller-vtep - test vtep-macs 1])
+OVN_CONTROLLER_VTEP_START
+
+# creates a simple logical network with the vtep device and a fake hv chassis
+# 'ch0'.
+AT_CHECK([ovn-nbctl lport-add br-test vif0])
+AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
+AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
+AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
+
+# creates the logical switch in vtep and adds the corresponding logical
+# port to 'br-test'.
+AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
+OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep], [lswitch0])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-vtep_lswitch0`"])
+
+# adds another lswitch 'br-void' in ovn-nb database.
+AT_CHECK([ovn-nbctl lswitch-add br-void])
+# adds fake hv chassis 'ch1'.
+AT_CHECK([ovn-nbctl lport-add br-void vif1])
+AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
+AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
+AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
+
+# checks Ucast_Macs_Remote creation.
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep _uuid`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' '], [0], [dnl
+"f0:ab:cd:ef:01:02"
+])
+
+# checks physical locator creation.
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Physical_Locator | grep _uuid`"])
+AT_CHECK([vtep-ctl --columns=dst_ip list Physical_Locator | cut -d ':' -f2 | tr -d ' ' | grep -v 1.2.3.4 | sed '/^$/d'], [0], [dnl
+"1.2.3.5"
+])
+
+# checks tunnel creation by ovs-vtep.
+OVS_WAIT_UNTIL([test -n "`ovs-vsctl list Interface bfd1.2.3.5`"])
+AT_CHECK([ovs-vsctl --columns=options list Interface bfd1.2.3.5 | cut -d ':' -f2 | tr -d ' '], [0], [dnl
+{remote_ip="1.2.3.5"}
+])
+
+# adds another mac to lport.
+AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02 f0:ab:cd:ef:01:03])
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 03`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
+
+"f0:ab:cd:ef:01:02"
+"f0:ab:cd:ef:01:03"
+])
+
+# removes one mac to lport.
+AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:03])
+OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 02`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
+"f0:ab:cd:ef:01:03"
+])
+
+# migrates mac to lport vif1 on 'br-void'.
+AT_CHECK([ovn-nbctl lport-set-macs vif0])
+AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:03])
+OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 03`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
+])
+
+OVN_CONTROLLER_VTEP_STOP
+AT_CLEANUP
+
+
+# Tests vtep module 'Ucast_Macs_Remote's (corner cases).
+AT_SETUP([ovn-controller-vtep - test vtep-macs 2])
+OVN_CONTROLLER_VTEP_START
+
+# creates a simple logical network with the vtep device and a fake hv chassis
+# 'ch0'.
+AT_CHECK([ovn-nbctl lport-add br-test vif0])
+AT_CHECK([ovn-nbctl lport-set-macs vif0 f0:ab:cd:ef:01:02])
+AT_CHECK([ovn-sbctl chassis-add ch0 vxlan 1.2.3.5])
+AT_CHECK([ovn-sbctl lport-bind vif0 ch0])
+
+# creates another vif in the same logical switch with duplicate mac.
+AT_CHECK([ovn-nbctl lport-add br-test vif1])
+AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
+AT_CHECK([ovn-sbctl lport-bind vif1 ch0])
+
+# creates the logical switch in vtep and adds the corresponding logical
+# port to 'br-test'.
+AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
+OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep], [lswitch0])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-vtep_lswitch0`"])
+
+# checks Ucast_Macs_Remote creation.  Should still only be one entry, since duplicate
+# mac in the same logical switch is not allowed.
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep _uuid`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' '], [0], [dnl
+"f0:ab:cd:ef:01:02"
+])
+# confirms the warning log.
+OVS_WAIT_UNTIL([test -n "`grep WARN ovn-controller-vtep.log`"])
+AT_CHECK([sed -n 's/^.*\(|WARN|.*\)$/\1/p' ovn-controller-vtep.log | sed 's/([[-_:0-9a-z]][[-_:0-9a-z]]*)/()/g' | uniq], [0], [dnl
+|WARN|MAC address () has already been known to be on logical port () in the same logical datapath, so just ignore this logical port ()
+])
+
+# deletes vif1.
+AT_CHECK([ovn-nbctl lport-del vif1])
+
+# adds another lswitch 'br-void' in ovn-nb database.
+AT_CHECK([ovn-nbctl lswitch-add br-void])
+# adds fake hv chassis 'ch1' and vif1 with same mac address as vif0.
+AT_CHECK([ovn-nbctl lport-add br-void vif1])
+AT_CHECK([ovn-nbctl lport-set-macs vif1 f0:ab:cd:ef:01:02])
+AT_CHECK([ovn-sbctl chassis-add ch1 vxlan 1.2.3.6])
+AT_CHECK([ovn-sbctl lport-bind vif1 ch1])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding | grep vif1`"])
+
+# creates another logical switch in vtep and adds the corresponding logical
+# port to 'br-void'.
+AT_CHECK([vtep-ctl add-ls lswitch1 -- bind-ls br-vtep p0 200 lswitch1])
+OVN_NB_ADD_VTEP_PORT([br-void], [br-void_lswitch1], [br-vtep], [lswitch1])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep br-void_lswitch1`"])
+
+# checks Ucast_Macs_Remote creation.  Should see two entries since it is allowed
+# to have duplicate macs in different logical switches.
+OVS_WAIT_UNTIL([test `vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 02 | wc -l` -gt 1])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr -d ' ' | sort], [0], [dnl
+
+"f0:ab:cd:ef:01:02"
+"f0:ab:cd:ef:01:02"
+])
+
+OVN_CONTROLLER_VTEP_STOP([/has already been known to be on logical port/d])
+AT_CLEANUP