On Tue, Apr 20, 2021 at 11:57 AM Mark Michelson <[email protected]> wrote:
>
> Previous behavior of ovn-controller was to only create MAC bindings for
> the same addresses for which we would send gARPs. That is:
>
> * A logical switch has its "nat_addresses" configured.
> * That logical switch has a localnet port.
>
> This makes sense for gARPs: we only want to send gARPs for addresses
> accessible from outside of OVN, and we only want to send gARPs for
> addresses explicitly enabled by the administrator.
>
> For directly adding MAC bindings, though, this makes less sense. In the
> case where a VM or pod on the underlay network wishes to reach another
> VM or pod via a DNAT or load balancer address, an ARP was required to
> learn the MAC binding. When the information is all available in the
> database, this seems like an unnecessary step.
>
> This commit makes the following changes:
> * MAC_Binding now has a "chassis_name" column. This is used to identify
> the chassis that added the MAC Binding IF the MAC Binding was auto-
> generated by ovn-controller. This is also used to ensure that stale
> bindings can be cleaned up.
> * The term "local garp" has been replaced with "chassis MAC binding".
> Removing the word "garp" helps to remove any implication of packets
> being transmitted. The word "chassis" precedes "MAC binding" to
> indicate it is a MAC binding added by a chassis and with a
> chassis_name in its records. MAC bindings with no chassis_name were
> added dynamically due to reception of ARP/gARP packets.
> * Chassis MAC Binding code has been separated from gARP code since they
> do not operate using the same sets of addresses.
> * Chassis MAC Bindings are added for all router_addresses on a port
> binding. This means all NAT, Load Balancer, and router interface
> addresses are added for routers that are residents of the chassis.
>
> Signed-off-by: Mark Michelson <[email protected]>
Hi Mark,
Please see below for just a comment.
Thanks
Numan
> ---
> controller/ovn-controller.c | 4 +
> controller/pinctrl.c | 298 +++++++++++++++++++++++++++---------
> controller/pinctrl.h | 1 +
> northd/ovn-northd.c | 2 +-
> northd/ovn_northd.dl | 5 +-
> ovn-sb.ovsschema | 5 +-
> ovn-sb.xml | 7 +
> tests/ovn-controller.at | 179 ++++++++++++++++++++++
> 8 files changed, 427 insertions(+), 74 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 16c8ecb21..872d68799 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2507,6 +2507,9 @@ main(int argc, char *argv[])
> = ovsdb_idl_index_create2(ovnsb_idl_loop.idl,
> &sbrec_fdb_col_mac,
> &sbrec_fdb_col_dp_key);
> + struct ovsdb_idl_index *sbrec_mac_binding_by_chassis
> + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> + &sbrec_mac_binding_col_chassis_name);
>
> ovsdb_idl_track_add_all(ovnsb_idl_loop.idl);
> ovsdb_idl_omit_alert(ovnsb_idl_loop.idl,
> @@ -2931,6 +2934,7 @@ main(int argc, char *argv[])
> sbrec_port_binding_by_key,
> sbrec_port_binding_by_name,
> sbrec_mac_binding_by_lport_ip,
> + sbrec_mac_binding_by_chassis,
> sbrec_igmp_group,
> sbrec_ip_multicast,
> sbrec_fdb_by_dp_key_mac,
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 523a45b9a..51327f370 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -205,6 +205,7 @@ static void send_garp_rarp_prepare(
> struct ovsdb_idl_index *sbrec_port_binding_by_datapath,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> + struct ovsdb_idl_index *sbrec_mac_binding_by_chassis,
> const struct ovsrec_bridge *,
> const struct sbrec_chassis *,
> const struct hmap *local_datapaths,
> @@ -3318,6 +3319,7 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> + struct ovsdb_idl_index *sbrec_mac_binding_by_chassis,
> struct ovsdb_idl_index *sbrec_igmp_groups,
> struct ovsdb_idl_index *sbrec_ip_multicast_opts,
> struct ovsdb_idl_index *sbrec_fdb_by_dp_key_mac,
> @@ -3339,7 +3341,8 @@ pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> sbrec_port_binding_by_key, chassis);
> send_garp_rarp_prepare(ovnsb_idl_txn, sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_name,
> - sbrec_mac_binding_by_lport_ip, br_int, chassis,
> + sbrec_mac_binding_by_lport_ip,
> + sbrec_mac_binding_by_chassis, br_int, chassis,
> local_datapaths, active_tunnels);
> prepare_ipv6_ras(local_datapaths);
> prepare_ipv6_prefixd(ovnsb_idl_txn, sbrec_port_binding_by_name,
> @@ -4008,7 +4011,8 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> const char *logical_port,
> const struct sbrec_datapath_binding *dp,
> struct eth_addr ea, const char *ip,
> - bool update_only)
> + bool update_only,
> + const struct sbrec_chassis *chassis)
> {
> /* Convert ethernet argument to string form for database. */
> char mac_string[ETH_ADDR_STRLEN + 1];
> @@ -4025,49 +4029,20 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> sbrec_mac_binding_set_ip(b, ip);
> sbrec_mac_binding_set_mac(b, mac_string);
> sbrec_mac_binding_set_datapath(b, dp);
> - } else if (strcmp(b->mac, mac_string)) {
> - sbrec_mac_binding_set_mac(b, mac_string);
> - }
> -}
> -
> -/* Simulate the effect of a GARP on local datapaths, i.e., create
> MAC_Bindings
> - * on peer router datapaths.
> - */
> -static void
> -send_garp_locally(struct ovsdb_idl_txn *ovnsb_idl_txn,
> - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> - const struct hmap *local_datapaths,
> - const struct sbrec_port_binding *in_pb,
> - struct eth_addr ea, ovs_be32 ip)
> -{
> - if (!ovnsb_idl_txn) {
> - return;
> - }
> -
> - const struct local_datapath *ldp =
> - get_local_datapath(local_datapaths, in_pb->datapath->tunnel_key);
> -
> - ovs_assert(ldp);
> - for (size_t i = 0; i < ldp->n_peer_ports; i++) {
> - const struct sbrec_port_binding *local = ldp->peer_ports[i].local;
> - const struct sbrec_port_binding *remote = ldp->peer_ports[i].remote;
> -
> - /* Skip "ingress" port. */
> - if (local == in_pb) {
> - continue;
> + if (chassis) {
> + sbrec_mac_binding_set_chassis_name(b, chassis->name);
> + }
> + } else {
> + if (strcmp(b->mac, mac_string)) {
> + sbrec_mac_binding_set_mac(b, mac_string);
> + }
> + if (b->chassis_name[0]) {
> + if (chassis && strcmp(b->chassis_name, chassis->name)) {
> + sbrec_mac_binding_set_chassis_name(b, chassis->name);
> + } else if (!chassis) {
> + sbrec_mac_binding_set_chassis_name(b, "");
> + }
> }
> -
> - bool update_only = !smap_get_bool(&remote->datapath->external_ids,
> - "always_learn_from_arp_request",
> - true);
> -
> - struct ds ip_s = DS_EMPTY_INITIALIZER;
> -
> - ip_format_masked(ip, OVS_BE32_MAX, &ip_s);
> - mac_binding_add_to_sb(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> - remote->logical_port, remote->datapath,
> - ea, ds_cstr(&ip_s), update_only);
> - ds_destroy(&ip_s);
> }
> }
>
> @@ -4099,7 +4074,7 @@ run_put_mac_binding(struct ovsdb_idl_txn *ovnsb_idl_txn,
> ipv6_format_mapped(&mb->ip, &ip_s);
> mac_binding_add_to_sb(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> pb->logical_port, pb->datapath, mb->mac,
> - ds_cstr(&ip_s), false);
> + ds_cstr(&ip_s), false, NULL);
> ds_destroy(&ip_s);
> }
>
> @@ -4233,9 +4208,7 @@ add_garp_rarp(const char *name, const struct eth_addr
> ea, ovs_be32 ip,
>
> /* Add or update a vif for which GARPs need to be announced. */
> static void
> -send_garp_rarp_update(struct ovsdb_idl_txn *ovnsb_idl_txn,
> - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> - const struct hmap *local_datapaths,
> +send_garp_rarp_update(const struct hmap *local_datapaths,
> const struct sbrec_port_binding *binding_rec,
> struct shash *nat_addresses)
> {
> @@ -4246,6 +4219,13 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> return;
> }
>
> + struct local_datapath *ldp = get_local_datapath(
> + local_datapaths, binding_rec->datapath->tunnel_key);
> + ovs_assert(ldp);
> + if (!ldp->localnet_port) {
> + return;
> + }
> +
> /* Update GARP for NAT IP if it exists. Consider port bindings with type
> * "l3gateway" for logical switch ports attached to gateway routers, and
> * port bindings with type "patch" for logical switch ports attached to
> @@ -4268,11 +4248,6 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> laddrs->ipv4_addrs[i].addr,
> binding_rec->datapath->tunnel_key,
> binding_rec->tunnel_key);
> - send_garp_locally(ovnsb_idl_txn,
> - sbrec_mac_binding_by_lport_ip,
> - local_datapaths, binding_rec,
> laddrs->ea,
> - laddrs->ipv4_addrs[i].addr);
> -
> }
> free(name);
> }
> @@ -4308,10 +4283,6 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> laddrs.ea, ip,
> binding_rec->datapath->tunnel_key,
> binding_rec->tunnel_key);
> - if (ip) {
> - send_garp_locally(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> - local_datapaths, binding_rec, laddrs.ea, ip);
> - }
>
> destroy_lport_addresses(&laddrs);
> break;
> @@ -5424,10 +5395,6 @@ get_localnet_vifs_l3gwports(
> HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
> const struct sbrec_port_binding *pb;
>
> - if (!ld->localnet_port) {
> - continue;
> - }
> -
> /* Get l3gw ports. Consider port bindings with type "l3gateway"
> * that connect to gateway routers (if local), and consider port
> * bindings of type "patch" since they might connect to
> @@ -5551,7 +5518,8 @@ get_nat_addresses_and_keys(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> struct sset *local_l3gw_ports,
> const struct sbrec_chassis *chassis,
> const struct sset *active_tunnels,
> - struct shash *nat_addresses)
> + struct shash *garp_addresses,
> + struct shash *router_addresses)
> {
> const char *gw_port;
> SSET_FOR_EACH(gw_port, local_l3gw_ports) {
> @@ -5568,7 +5536,7 @@ get_nat_addresses_and_keys(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> pb->nat_addresses[i], pb,
> nat_address_keys, chassis,
> active_tunnels,
> - nat_addresses);
> + garp_addresses);
> }
> } else {
> /* Continue to support options:nat-addresses for version
> @@ -5580,9 +5548,16 @@ get_nat_addresses_and_keys(struct ovsdb_idl_index
> *sbrec_port_binding_by_name,
> nat_addresses_options, pb,
> nat_address_keys, chassis,
> active_tunnels,
> - nat_addresses);
> + garp_addresses);
> }
> }
> +
> + for (int i = 0; i < pb->n_router_addresses; i++) {
s/int/size_t
> + consider_nat_address(sbrec_port_binding_by_name,
> + pb->router_addresses[i], pb,
> + nat_address_keys, chassis,
> + active_tunnels, router_addresses);
> + }
> }
> }
>
> @@ -5618,6 +5593,165 @@ send_garp_rarp_run(struct rconn *swconn, long long
> int *send_garp_rarp_time)
> }
> }
>
> +struct chassis_mac_binding {
> + struct hmap_node hmap_node;
> + const struct sbrec_mac_binding *binding;
> + uint32_t hash;
> +};
> +
> +static uint32_t
> +chassis_mac_binding_hash_(const char *ip, const char *logical_port)
> +{
> + return hash_string(ip, hash_string(logical_port, 0));
> +}
> +
> +static uint32_t
> +chassis_mac_binding_hash(const struct sbrec_mac_binding *binding)
> +{
> + return chassis_mac_binding_hash_(binding->ip, binding->logical_port);
> +}
> +
> +static struct chassis_mac_binding *
> +chassis_mac_binding_alloc(const struct sbrec_mac_binding *binding)
> +{
> + struct chassis_mac_binding *cmb = xmalloc(sizeof *cmb);
> + cmb->binding = binding;
> + cmb->hash = chassis_mac_binding_hash(binding);
> + return cmb;
> +}
> +
> +static void
> +chassis_mac_binding_remove(struct hmap *chassis_mac_bindings,
> + const char *ip, const char *logical_port)
> +{
> + uint32_t hash = chassis_mac_binding_hash_(ip, logical_port);
> + struct chassis_mac_binding *cmb;
> + HMAP_FOR_EACH_WITH_HASH (cmb, hmap_node, hash, chassis_mac_bindings) {
> + if (!strcmp(cmb->binding->ip, ip)
> + && !strcmp(cmb->binding->logical_port, logical_port)) {
> + hmap_remove(chassis_mac_bindings, &cmb->hmap_node);
> + free(cmb);
> + break;
> + }
> + }
> +}
> +
> +static void
> +delete_old_chassis_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct hmap *chassis_mac_bindings)
> +{
> + if (!ovnsb_idl_txn) {
> + return;
> + }
> + struct chassis_mac_binding *cmb;
> + HMAP_FOR_EACH_POP (cmb, hmap_node, chassis_mac_bindings) {
> + sbrec_mac_binding_delete(cmb->binding);
> + free(cmb);
> + }
> +}
> +
> +static void
> +get_chassis_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct ovsdb_idl_index
> *sbrec_mac_binding_by_chassis,
> + const struct sbrec_chassis *chassis,
> + struct hmap *chassis_mac_bindings)
> +{
> + if (!ovnsb_idl_txn) {
> + return;
> + }
> + const struct sbrec_mac_binding *mac_binding;
> +
> + struct sbrec_mac_binding *mb_row = sbrec_mac_binding_index_init_row(
> + sbrec_mac_binding_by_chassis);
> + sbrec_mac_binding_index_set_chassis_name(mb_row, chassis->name);
> +
> + SBREC_MAC_BINDING_FOR_EACH_EQUAL (mac_binding, mb_row,
> + sbrec_mac_binding_by_chassis) {
> + struct chassis_mac_binding *ch_binding =
> + chassis_mac_binding_alloc(mac_binding);
> + hmap_insert(chassis_mac_bindings, &ch_binding->hmap_node,
> ch_binding->hash);
> + }
> + sbrec_mac_binding_index_destroy_row(mb_row);
> +}
> +
> +/* Simulate the effect of a GARP on local datapaths, i.e., create
> MAC_Bindings
> + * on peer router datapaths.
> + */
> +static void
> +install_chassis_mac_binding(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip,
> + const struct local_datapath *ldp,
> + const struct sbrec_port_binding *in_pb,
> + struct eth_addr ea, ovs_be32 ip,
> + const struct sbrec_chassis *chassis,
> + struct hmap *old_chassis_mac_bindings)
> +{
> + ovs_assert(ovnsb_idl_txn && ldp);
> +
> + struct ds ip_s = DS_EMPTY_INITIALIZER;
> +
> + ip_format_masked(ip, OVS_BE32_MAX, &ip_s);
> + for (size_t i = 0; i < ldp->n_peer_ports; i++) {
> + const struct sbrec_port_binding *local = ldp->peer_ports[i].local;
> + const struct sbrec_port_binding *remote = ldp->peer_ports[i].remote;
> +
> + /* Skip "ingress" port. */
> + if (local == in_pb) {
> + continue;
> + }
> +
> + bool update_only = !smap_get_bool(&remote->datapath->external_ids,
> + "always_learn_from_arp_request",
> + true);
> +
> + mac_binding_add_to_sb(ovnsb_idl_txn, sbrec_mac_binding_by_lport_ip,
> + remote->logical_port, remote->datapath,
> + ea, ds_cstr(&ip_s), update_only, chassis);
> +
> + chassis_mac_binding_remove(old_chassis_mac_bindings, ds_cstr(&ip_s),
> + remote->logical_port);
> +
> + }
> + ds_destroy(&ip_s);
> +}
> +
> +static void
> +install_chassis_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
> + struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip,
> + const struct hmap *local_datapaths,
> + const struct sbrec_port_binding *in_pb,
> + struct shash *router_addresses,
> + const struct sbrec_chassis *chassis,
> + struct hmap *old_chassis_mac_bindings)
> +{
> + if (!ovnsb_idl_txn) {
> + return;
> + }
> +
> + if (strcmp(in_pb->type, "l3gateway")
> + && strcmp(in_pb->type, "patch")) {
> + return;
> + }
> +
> + const struct local_datapath *ldp =
> + get_local_datapath(local_datapaths, in_pb->datapath->tunnel_key);
> +
> + struct lport_addresses *laddrs = NULL;
> + while ((laddrs = shash_find_and_delete(router_addresses,
> + in_pb->logical_port))) {
> + int i;
> + for (i = 0; i < laddrs->n_ipv4_addrs; i++) {
> + install_chassis_mac_binding(ovnsb_idl_txn,
> + sbrec_mac_binding_by_lport_ip,
> + ldp, in_pb, laddrs->ea,
> + laddrs->ipv4_addrs[i].addr,
> + chassis, old_chassis_mac_bindings);
> + }
> + destroy_lport_addresses(laddrs);
> + free(laddrs);
> + }
> +}
> +
> /* Called by pinctrl_run(). Runs with in the main ovn-controller
> * thread context. */
> static void
> @@ -5625,6 +5759,7 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct ovsdb_idl_index
> *sbrec_port_binding_by_datapath,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> + struct ovsdb_idl_index *sbrec_mac_binding_by_chassis,
> const struct ovsrec_bridge *br_int,
> const struct sbrec_chassis *chassis,
> const struct hmap *local_datapaths,
> @@ -5635,8 +5770,15 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct sset local_l3gw_ports = SSET_INITIALIZER(&local_l3gw_ports);
> struct sset nat_ip_keys = SSET_INITIALIZER(&nat_ip_keys);
> struct shash nat_addresses;
> + struct shash router_addresses;
> + struct hmap old_chassis_mac_bindings;
>
> shash_init(&nat_addresses);
> + shash_init(&router_addresses);
> + hmap_init(&old_chassis_mac_bindings);
> +
> + get_chassis_mac_bindings(ovnsb_idl_txn, sbrec_mac_binding_by_chassis,
> + chassis, &old_chassis_mac_bindings);
>
> get_localnet_vifs_l3gwports(sbrec_port_binding_by_datapath,
> sbrec_port_binding_by_name,
> @@ -5646,7 +5788,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> get_nat_addresses_and_keys(sbrec_port_binding_by_name,
> &nat_ip_keys, &local_l3gw_ports,
> chassis, active_tunnels,
> - &nat_addresses);
> + &nat_addresses,
> + &router_addresses);
> /* For deleted ports and deleted nat ips, remove from
> * send_garp_rarp_data. */
> struct shash_node *iter, *next;
> @@ -5663,8 +5806,11 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> const struct sbrec_port_binding *pb = lport_lookup_by_name(
> sbrec_port_binding_by_name, iface_id);
> if (pb) {
> - send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, pb, &nat_addresses);
> + send_garp_rarp_update(local_datapaths, pb, &nat_addresses);
> + install_chassis_mac_bindings(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> + local_datapaths, pb,
> + &router_addresses, chassis,
> + &old_chassis_mac_bindings);
> }
> }
>
> @@ -5674,11 +5820,17 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> const struct sbrec_port_binding *pb
> = lport_lookup_by_name(sbrec_port_binding_by_name, gw_port);
> if (pb) {
> - send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, pb, &nat_addresses);
> + send_garp_rarp_update(local_datapaths, pb, &nat_addresses);
> + install_chassis_mac_bindings(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> + local_datapaths, pb,
> + &router_addresses, chassis,
> + &old_chassis_mac_bindings);
> }
> }
>
> + delete_old_chassis_mac_bindings(ovnsb_idl_txn,
> &old_chassis_mac_bindings);
> + hmap_destroy(&old_chassis_mac_bindings);
> +
> /* pinctrl_handler thread will send the GARPs. */
>
> sset_destroy(&localnet_vifs);
> @@ -5692,6 +5844,14 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> }
> shash_destroy(&nat_addresses);
>
> + SHASH_FOR_EACH_SAFE (iter, next, &router_addresses) {
> + struct lport_addresses *laddrs = iter->data;
> + destroy_lport_addresses(laddrs);
> + shash_delete(&router_addresses, iter);
> + free(laddrs);
> + }
> + shash_destroy(&router_addresses);
> +
> sset_destroy(&nat_ip_keys);
> }
>
> diff --git a/controller/pinctrl.h b/controller/pinctrl.h
> index cc0a51984..5d4f32b4a 100644
> --- a/controller/pinctrl.h
> +++ b/controller/pinctrl.h
> @@ -40,6 +40,7 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_name,
> struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> + struct ovsdb_idl_index *sbrec_mac_binding_by_chassis,
> struct ovsdb_idl_index *sbrec_igmp_groups,
> struct ovsdb_idl_index *sbrec_ip_multicast_opts,
> struct ovsdb_idl_index *sbrec_fdb_by_dp_key_mac,
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 78ac696a5..d36a3c29b 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -13696,7 +13696,7 @@ static const char *rbac_port_binding_update[] =
> static const char *rbac_mac_binding_auth[] =
> {""};
> static const char *rbac_mac_binding_update[] =
> - {"logical_port", "ip", "mac", "datapath"};
> + {"logical_port", "ip", "mac", "datapath", "chassis_name"};
>
> static const char *rbac_svc_monitor_auth[] =
> {""};
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 2be6591e9..089947516 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -955,7 +955,8 @@ sb::Out_MAC_Binding (._uuid = mb._uuid,
> .logical_port = mb.logical_port,
> .ip = mb.ip,
> .mac = mb.mac,
> - .datapath = mb.datapath) :-
> + .datapath = mb.datapath,
> + .chassis_name = mb.chassis_name) :-
> sb::MAC_Binding[mb],
> sb::Out_Port_Binding(.logical_port = mb.logical_port),
> sb::Out_Datapath_Binding(._uuid = mb.datapath).
> @@ -1312,7 +1313,7 @@ sb::Out_RBAC_Permission (
> .table = "MAC_Binding",
> .authorization = set_singleton(""),
> .insert_delete = true,
> - .update = ["logical_port", "ip", "mac", "datapath"].to_set()
> + .update = ["logical_port", "ip", "mac", "datapath",
> "chassis_name"].to_set()
> ).
>
> sb::Out_RBAC_Permission (
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index b879e6228..182a95528 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> "version": "20.17.0",
> - "cksum": "367711378 26723",
> + "cksum": "394417570 26775",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -241,7 +241,8 @@
> "ip": {"type": "string"},
> "mac": {"type": "string"},
> "datapath": {"type": {"key": {"type": "uuid",
> - "refTable":
> "Datapath_Binding"}}}},
> + "refTable":
> "Datapath_Binding"}}},
> + "chassis_name": {"type": "string"}},
> "indexes": [["logical_port", "ip"]],
> "isRoot": true},
> "DHCP_Options": {
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 610dca17c..7648a9e78 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3345,6 +3345,13 @@ tcp.flags = RST;
> <column name="datapath">
> The logical datapath to which the logical port belongs.
> </column>
> + <column name="chassis_name">
> + This column indicates the chassis on which the binding was added. This
> + column is only filled in when ovn-controller generates the MAC binding
> + internally. MAC bindings learned due to ARP requests will not have this
> + column set. This is used internally by ovn-controller to ensure that
> + defunct bindings are removed.
> + </column>
> </table>
>
> <table name="DHCP_Options" title="DHCP Options supported by native OVN
> DHCP">
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index d1d758c49..b37e135f9 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -549,3 +549,182 @@ done
>
> OVN_CLEANUP([hv1])
> AT_CLEANUP
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn -- MAC bindings from router addresses])
> +ovn_start
> +
> +net_add n1
> +for i in 1 2; do
> + sim_add hv$i
> + as hv$i
> + ovs-vsctl add-br br-phys
> + ovn_attach n1 br-phys 192.168.$i.1
> +done
> +
> +AS_BOX([Setting up the logical network])
> +# This test ties in heavily with the ovn-northd "Router Address
> +# Propagation" test. That test ensures that Port_Binding
> +# router_addresses are filled in as expected based on the logical
> +# network configuration.
> +#
> +# This test focuses on ensuring that router_addresses get
> +# converted to the expected MAC_Bindings, specifically with regards
> +# to ensuring they are created based on is_chassis_resident
> +# restrictions. Therefore, it is not necessary to ensure that an
> +# exhaustive set of logical network configurations is attempted for
> +# this test.
> +
> +check ovn-nbctl ls-add sw
> +
> +check ovn-nbctl lr-add ro1
> +check ovn-nbctl lrp-add ro1 ro1-sw 00:00:00:00:00:01 10.0.0.1/24
> +check ovn-nbctl lsp-add sw sw-ro1
> +
> +check ovn-nbctl lr-add ro2
> +check ovn-nbctl lrp-add ro2 ro2-sw 00:00:00:00:00:02 20.0.0.1/24
> +check ovn-nbctl --wait=sb lsp-add sw sw-ro2
> +
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 vm1
> +check ovn-nbctl lsp-set-addresses vm1 "00:00:00:00:01:02 172.16.1.2"
> +check ovn-nbctl lrp-add ro1 ro1-ls1 00:00:00:00:01:01 172.16.1.1/24
> +check ovn-nbctl lsp-add ls1 ls1-ro1
> +check ovn-nbctl lsp-set-type ls1-ro1 router
> +check ovn-nbctl lsp-set-addresses ls1-ro1 router
> +check ovn-nbctl lsp-set-options ls1-ro1 router-port=ro1-ls1
> +
> +check ovn-nbctl ls-add ls2
> +check ovn-nbctl lsp-add ls2 vm2
> +check ovn-nbctl lsp-set-addresses vm2 "00:00:00:00:02:02 172.16.2.2"
> +check ovn-nbctl lrp-add ro2 ro2-ls2 00:00:00:00:02:01 172.16.2.1/24
> +check ovn-nbctl lsp-add ls2 ls2-ro2
> +check ovn-nbctl lsp-set-type ls2-ro2 router
> +check ovn-nbctl lsp-set-addresses ls2-ro2 router
> +check ovn-nbctl --wait=hv lsp-set-options ls2-ro2 router-port=ro2-ls2
> +
> +as hv1
> +check ovs-vsctl add-port br-int vm1 -- set Interface vm1
> external_ids:iface-id=vm1
> +as hv2
> +check ovs-vsctl add-port br-int vm2 -- set Interface vm2
> external_ids:iface-id=vm2
> +
> +AS_BOX([Checking that no router addresses results in no chassis-specific MAC
> Bindings])
> +
> +check_row_count MAC_Binding 0 chassis_name=hv1
> +check_row_count MAC_Binding 0 chassis_name=hv2
> +
> +AS_BOX([Checking that connecting routers results in no chassis-specific MAC
> Bindings])
> +
> +check ovn-nbctl lsp-set-type sw-ro1 router
> +check ovn-nbctl lsp-set-addresses sw-ro1 router
> +check ovn-nbctl lsp-set-options sw-ro1 router-port=ro1-sw
> +
> +check ovn-nbctl lsp-set-type sw-ro2 router
> +check ovn-nbctl lsp-set-addresses sw-ro2 router
> +check ovn-nbctl --wait=hv lsp-set-options sw-ro2 router-port=ro2-sw
> +
> +check_row_count MAC_Binding 0 chassis_name=hv1
> +check_row_count MAC_Binding 0 chassis_name=hv2
> +
> +AS_BOX([Checking that l3dgw routers have chassis-specific MAC Bindings])
> +
> +check ovn-nbctl lrp-set-gateway-chassis ro1-sw hv1 100
> +check ovn-nbctl --wait=hv lrp-set-gateway-chassis ro2-sw hv2 100
> +
> +check_row_count MAC_Binding 1 chassis_name=hv1
> +check_row_count MAC_Binding 1 chassis_name=hv2
> +
> +check_column "10.0.0.1" MAC_Binding ip chassis_name=hv1
> +check_column "00:00:00:00:00:01" MAC_Binding mac chassis_name=hv1
> +check_column "ro2-sw" MAC_Binding logical_port chassis_name=hv1
> +
> +check_column "20.0.0.1" MAC_Binding ip chassis_name=hv2
> +check_column "00:00:00:00:00:02" MAC_Binding mac chassis_name=hv2
> +check_column "ro1-sw" MAC_Binding logical_port chassis_name=hv2
> +
> +AS_BOX([Checking that NAT addresses result in chassis-specific MAC Bindings])
> +
> +check ovn-nbctl lr-nat-add ro1 dnat 10.0.0.100 172.16.1.100
> +check ovn-nbctl --wait=hv lr-nat-add ro1 snat 10.0.0.200 172.16.1.200/30
> +
> +# hv1 installs MAC bindings for the ro1's router address and two NAT
> addresses
> +check_row_count MAC_Binding 3 chassis_name=hv1
> +# hv2 only installs MAC bindings for ro2's router address
> +check_row_count MAC_Binding 1 chassis_name=hv2
> +
> +check_column "10.0.0.1 10.0.0.100 10.0.0.200" MAC_Binding ip chassis_name=hv1
> +check_column "00:00:00:00:00:01 00:00:00:00:00:01 00:00:00:00:00:01"
> MAC_Binding mac chassis_name=hv1
> +check_column "ro2-sw ro2-sw ro2-sw" MAC_Binding logical_port chassis_name=hv1
> +
> +check_column "20.0.0.1" MAC_Binding ip chassis_name=hv2
> +check_column "00:00:00:00:00:02" MAC_Binding mac chassis_name=hv2
> +check_column "ro1-sw" MAC_Binding logical_port chassis_name=hv2
> +
> +AS_BOX([Checking that changing which router has NAT addresses results in
> correct MAC Bindings])
> +
> +check ovn-nbctl lr-nat-del ro1
> +
> +check ovn-nbctl lr-nat-add ro2 dnat 20.0.0.100 172.16.2.100
> +check ovn-nbctl --wait=hv lr-nat-add ro2 snat 20.0.0.200 172.16.2.200/30
> +
> +# hv1 installs MAC bindings for ro1's router address
> +check_row_count MAC_Binding 1 chassis_name=hv1
> +# hv2 installs MAC bindings for ro2's router address and two NAT addresses
> +check_row_count MAC_Binding 3 chassis_name=hv2
> +
> +check_column "10.0.0.1" MAC_Binding ip chassis_name=hv1
> +check_column "00:00:00:00:00:01" MAC_Binding mac chassis_name=hv1
> +check_column "ro2-sw" MAC_Binding logical_port chassis_name=hv1
> +
> +check_column "20.0.0.1 20.0.0.100 20.0.0.200" MAC_Binding ip chassis_name=hv2
> +check_column "00:00:00:00:00:02 00:00:00:00:00:02 00:00:00:00:00:02"
> MAC_Binding mac chassis_name=hv2
> +check_column "ro1-sw ro1-sw ro1-sw" MAC_Binding logical_port chassis_name=hv2
> +
> +AS_BOX([Checking that Floating IP NAT address configured on router results
> in MAC Binding])
> +
> +check ovn-nbctl lr-nat-del ro2
> +
> +check ovn-nbctl --wait=hv lr-nat-add ro1 dnat_and_snat 10.0.0.100
> 172.16.1.100 vm1 00:00:00:00:00:01
> +
> +# HV1 will have MAC bindings for the router address and the Floating IP NAT
> since vm1 is bound on hv1
> +check_row_count MAC_Binding 2 chassis_name=hv1
> +check_row_count MAC_Binding 1 chassis_name=hv2
> +
> +check_column "10.0.0.1 10.0.0.100" MAC_Binding ip chassis_name=hv1
> +check_column "00:00:00:00:00:01 00:00:00:00:00:01" MAC_Binding mac
> chassis_name=hv1
> +check_column "ro2-sw ro2-sw" MAC_Binding logical_port chassis_name=hv1
> +
> +check_column "20.0.0.1" MAC_Binding ip chassis_name=hv2
> +check_column "00:00:00:00:00:02" MAC_Binding mac chassis_name=hv2
> +check_column "ro1-sw" MAC_Binding logical_port chassis_name=hv2
> +
> +AS_BOX([Checking that un-binding vm results in removal of Floating IP MAC
> Binding])
> +
> +as hv1
> +check ovs-vsctl del-port br-int vm1
> +
> +# This will be exactly as if no NATs were configured.
> +# Use wait_row_count instead of check_row_count here since
> +# we don't have an equivalent to --wait=hv in ovs-vsctl.
> +wait_row_count MAC_Binding 1 chassis_name=hv1
> +check_row_count MAC_Binding 1 chassis_name=hv2
> +
> +check_column "10.0.0.1" MAC_Binding ip chassis_name=hv1
> +check_column "00:00:00:00:00:01" MAC_Binding mac chassis_name=hv1
> +check_column "ro2-sw" MAC_Binding logical_port chassis_name=hv1
> +
> +check_column "20.0.0.1" MAC_Binding ip chassis_name=hv2
> +check_column "00:00:00:00:00:02" MAC_Binding mac chassis_name=hv2
> +check_column "ro1-sw" MAC_Binding logical_port chassis_name=hv2
> +
> +AS_BOX([Ensure that disconnecting routers results in removal of all MAC
> Bindings])
> +
> +check ovn-nbctl clear logical_switch_port sw-ro1 options
> +check ovn-nbctl --wait=hv clear logical_switch_port sw-ro2 options
> +
> +check_row_count MAC_Binding 0 chassis_name=hv1
> +check_row_count MAC_Binding 0 chassis_name=hv2
> +
> +OVN_CLEANUP([hv1],[hv2])
> +AT_CLEANUP
> +])
> --
> 2.29.2
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
