On Fri, 2022-06-17 at 11:07 +0200, Ales Musil wrote:
> The new chassis column in MAC_Binding should
> act as an "owner" order of that particular row.
> This can be utilized by MAC binding aging mechanism,
> where only single is responsible for that particular
> record. In order to keep it consistent the chassis
> that owns the MAC binding is the one that created it,
> with one exception, when the MAC of the binding has
> changed the ownership is transferred to the controller
> that made this update. MAC binding rows without
> chassis are removed by northd.
>
> Reported-at: https://bugzilla.redhat.com/2084668
> Signed-off-by: Ales Musil <[email protected]>
> ---
> controller/pinctrl.c | 35 ++++++++++++++++++++++++-----------
> northd/northd.c | 12 ++++++++++++
> northd/ovn-northd.c | 2 +-
> ovn-sb.ovsschema | 6 +++++-
> ovn-sb.xml | 5 +++++
> tests/ovn.at | 39 +++++++++++++++++++++++++--------------
> 6 files changed, 72 insertions(+), 27 deletions(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 9a1a0faa1..b95336ec0 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -192,7 +192,8 @@ static void run_put_mac_bindings(
> struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index *sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index *sbrec_port_binding_by_key,
> - struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip)
> + struct ovsdb_idl_index *sbrec_mac_binding_by_lport_ip,
> + const struct sbrec_chassis *chassis)
> OVS_REQUIRES(pinctrl_mutex);
> static void wait_put_mac_bindings(struct ovsdb_idl_txn
> *ovnsb_idl_txn);
> static void send_mac_binding_buffered_pkts(struct rconn *swconn)
> @@ -3508,7 +3509,7 @@ pinctrl_run(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> pinctrl_set_br_int_name_(br_int->name);
> run_put_mac_bindings(ovnsb_idl_txn,
> sbrec_datapath_binding_by_key,
> sbrec_port_binding_by_key,
> - sbrec_mac_binding_by_lport_ip);
> + sbrec_mac_binding_by_lport_ip, chassis);
> run_put_vport_bindings(ovnsb_idl_txn,
> sbrec_datapath_binding_by_key,
> sbrec_port_binding_by_key, chassis);
> send_garp_rarp_prepare(ovnsb_idl_txn,
> sbrec_port_binding_by_datapath,
> @@ -4163,6 +4164,7 @@ mac_binding_add_to_sb(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip,
> const char *logical_port,
> const struct sbrec_datapath_binding *dp,
> + const struct sbrec_chassis *chassis,
> struct eth_addr ea, const char *ip,
> bool update_only)
> {
> @@ -4181,8 +4183,11 @@ 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);
> + sbrec_mac_binding_set_chassis(b, chassis);
> } else if (strcmp(b->mac, mac_string)) {
> sbrec_mac_binding_set_mac(b, mac_string);
> + /* Transfer ownership to current chassis if we have seen
> never MAC */
> + sbrec_mac_binding_set_chassis(b, chassis);
> }
> }
>
> @@ -4194,6 +4199,7 @@ 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,
> + const struct sbrec_chassis *chassis,
> struct eth_addr ea, ovs_be32 ip)
> {
> if (!ovnsb_idl_txn) {
> @@ -4221,7 +4227,7 @@ send_garp_locally(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>
> 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,
> + remote->logical_port, remote-
> >datapath, chassis,
> ea, ds_cstr(&ip_s), update_only);
> ds_destroy(&ip_s);
> }
> @@ -4232,7 +4238,8 @@ run_put_mac_binding(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index
> *sbrec_port_binding_by_key,
> struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip,
> - const struct mac_binding *mb)
> + const struct mac_binding *mb,
> + const struct sbrec_chassis *chassis)
> {
> /* Convert logical datapath and logical port key into lport. */
> const struct sbrec_port_binding *pb = lport_lookup_by_key(
> @@ -4254,7 +4261,7 @@ run_put_mac_binding(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> struct ds ip_s = DS_EMPTY_INITIALIZER;
> 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,
> + pb->logical_port, pb->datapath, chassis,
> mb->mac,
> ds_cstr(&ip_s), false);
> ds_destroy(&ip_s);
> }
> @@ -4265,7 +4272,8 @@ static void
> run_put_mac_bindings(struct ovsdb_idl_txn *ovnsb_idl_txn,
> struct ovsdb_idl_index
> *sbrec_datapath_binding_by_key,
> struct ovsdb_idl_index
> *sbrec_port_binding_by_key,
> - struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip)
> + struct ovsdb_idl_index
> *sbrec_mac_binding_by_lport_ip,
> + const struct sbrec_chassis *chassis)
> OVS_REQUIRES(pinctrl_mutex)
> {
> if (!ovnsb_idl_txn) {
> @@ -4277,7 +4285,7 @@ run_put_mac_bindings(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> run_put_mac_binding(ovnsb_idl_txn,
> sbrec_datapath_binding_by_key,
> sbrec_port_binding_by_key,
> sbrec_mac_binding_by_lport_ip,
> - mb);
> + mb, chassis);
> }
> ovn_mac_bindings_flush(&put_mac_bindings);
> }
> @@ -4392,6 +4400,7 @@ 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,
> const struct sbrec_port_binding *binding_rec,
> + const struct sbrec_chassis *chassis,
> struct shash *nat_addresses)
> {
> volatile struct garp_rarp_data *garp_rarp = NULL;
> @@ -4425,7 +4434,8 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> binding_rec->tunnel_key);
> send_garp_locally(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, binding_rec,
> laddrs->ea,
> + local_datapaths, binding_rec,
> + chassis, laddrs->ea,
> laddrs->ipv4_addrs[i].addr);
>
> }
> @@ -4465,7 +4475,8 @@ send_garp_rarp_update(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> 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);
> + local_datapaths, binding_rec, chassis,
> + laddrs.ea, ip);
> }
>
> destroy_lport_addresses(&laddrs);
> @@ -5808,7 +5819,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> if (pb) {
> send_garp_rarp_update(ovnsb_idl_txn,
> sbrec_mac_binding_by_lport_ip,
> - local_datapaths, pb,
> &nat_addresses);
> + local_datapaths, pb, chassis,
> + &nat_addresses);
> }
> }
>
> @@ -5819,7 +5831,8 @@ send_garp_rarp_prepare(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
> = 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);
> + local_datapaths, pb, chassis,
> + &nat_addresses);
> }
> }
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 0d6ebccde..214fdddc9 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -15623,6 +15623,17 @@ handle_port_binding_changes(struct
> northd_input *input_data,
> }
> }
>
> +static void
> +remove_orphaned_mac_bindings(struct northd_input *input_data) {
> + const struct sbrec_mac_binding *mb;
> + SBREC_MAC_BINDING_TABLE_FOR_EACH_SAFE (mb,
> + input_data-
> >sbrec_mac_binding_table) {
> + if (!mb->chassis) {
> + sbrec_mac_binding_delete(mb);
> + }
> + }
> +}
> +
> /* Handle a fairly small set of changes in the southbound database.
> */
> static void
> ovnsb_db_run(struct northd_input *input_data,
> @@ -15641,6 +15652,7 @@ ovnsb_db_run(struct northd_input *input_data,
> if (ovnsb_txn) {
> update_sb_ha_group_ref_chassis(input_data,
> &ha_ref_chassis_map);
> + remove_orphaned_mac_bindings(input_data);
> }
> shash_destroy(&ha_ref_chassis_map);
> }
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e4e980720..a69b642b6 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -112,7 +112,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"};
>
> static const char *rbac_svc_monitor_auth[] =
> {""};
> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> index 3b78ea6f6..abb881761 100644
> --- a/ovn-sb.ovsschema
> +++ b/ovn-sb.ovsschema
> @@ -1,7 +1,7 @@
> {
> "name": "OVN_Southbound",
> "version": "20.23.0",
Need to bump the schema version so upgrades of existing DBs work
correctly, otherwise:
2022-06-22T16:54:33.291Z|00034|ovn_northd|INFO|ovn-northd lock acquired. This
ovn-northd instance is now active.
2022-06-22T16:54:33.293Z|00035|ovsdb_idl|WARN|MAC_Binding table in
OVN_Southbound database lacks chassis column (database needs upgrade?)
2022-06-22T16:54:33.293Z|00036|ovsdb_idl|WARN|Port_Binding table in
OVN_Southbound database lacks additional_chassis column (database needs
upgrade?)
2022-06-22T16:54:33.293Z|00037|ovsdb_idl|WARN|Port_Binding table in
OVN_Southbound database lacks additional_encap column (database needs upgrade?)
2022-06-22T16:54:33.293Z|00038|ovsdb_idl|WARN|Port_Binding table in
OVN_Southbound database lacks requested_additional_chassis column (database
needs upgrade?)
and ovn-controller will also be unhappy.
Dan
> - "cksum": "4045988377 28575",
> + "cksum": "2450182539 28828",
> "tables": {
> "SB_Global": {
> "columns": {
> @@ -260,6 +260,10 @@
> "logical_port": {"type": "string"},
> "ip": {"type": "string"},
> "mac": {"type": "string"},
> + "chassis": {"type": {"key": {"type": "uuid",
> + "refTable": "Chassis",
> + "refType": "weak"},
> + "min": 0, "max": 1}},
> "datapath": {"type": {"key": {"type": "uuid",
> "refTable":
> "Datapath_Binding"}}}},
> "indexes": [["logical_port", "ip"]],
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 898f3676a..842c24b70 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -3606,6 +3606,11 @@ tcp.flags = RST;
> <column name="mac">
> The Ethernet address to which the IP is bound.
> </column>
> +
> + <column name="chassis">
> + The chassis that "owns" the MAC binding.
> + </column>
> +
> <column name="datapath">
> The logical datapath to which the logical port belongs.
> </column>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 59d51f3e0..d71a08cdd 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -9038,9 +9038,13 @@ ovn-nbctl lsp-add ls0 lp1
> ovn-nbctl lsp-set-addresses lp0 "f0:00:00:00:00:01 192.168.0.1"
> ovn-nbctl lsp-set-addresses lp1 "f0:00:00:00:00:02 192.168.0.2"
> dp_uuid=$(fetch_column Datapath_Binding _uuid)
> -ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp0 mac="mac1"
> -ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp1 mac="mac2"
> -ovn-sbctl find MAC_Binding
> +chassis=$(fetch_column Chassis _uuid name=hv1)
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp0 mac="mac1" chassis=$chassis
> +ovn-sbctl create MAC_Binding ip=10.0.0.1 datapath=$dp_uuid
> logical_port=lp1 mac="mac2" chassis=$chassis
> +
> +wait_row_count MAC_Binding 1 logical_port=lp0
> +wait_row_count MAC_Binding 1 logical_port=lp1
> +
> # Delete port lp0 and check that its MAC_Binding is deleted.
> ovn-nbctl lsp-del lp0
> ovn-sbctl find MAC_Binding
> @@ -18916,8 +18920,9 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
> \
> ]])
> dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep
> _uuid | \
> awk '{print $3}')
> +chassis=$(fetch_column Chassis _uuid name=hv1)
> ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \
> -logical_port=lr0-public mac="00\:00\:00\:12\:af\:11"
> +logical_port=lr0-public mac="00\:00\:00\:12\:af\:11"
> chassis=$chassis
>
> # Try different gateway mtus and send a 142-byte packet
> (corresponding
> # to a 124-byte MTU). If the MTU is less than 124, ovn-controller
> @@ -18987,7 +18992,7 @@ ovn-nbctl lr-nat-add lr1 snat 2000::1
> 1000::/64
> dp_uuid=$(ovn-sbctl find datapath_binding | grep sw0 -B2 | grep
> _uuid | \
> awk '{print $3}')
> ovn-sbctl create MAC_Binding ip=172.168.0.3 datapath=$dp_uuid \
> -logical_port=lr1-public mac="00\:00\:00\:12\:af\:11"
> +logical_port=lr1-public mac="00\:00\:00\:12\:af\:11"
> chassis=$chassis
>
> # Try different gateway mtus and send a 142-byte packet
> (corresponding
> # to a 124-byte MTU). If the MTU is less than 124, ovn-controller
> @@ -19339,9 +19344,11 @@ check ovn-sbctl --all destroy mac_binding
>
> # Create a mac_binding entry on lr0-pub for 172.24.4.200 with a
> # wrong mac, expecting it to be updated with the real mac.
> +ovn-sbctl list chassis
> lr0_dp=$(fetch_column Datapath_Binding _uuid external_ids:name=lr0)
> +chassis=$(fetch_column Chassis _uuid name=hv1)
> ovn-sbctl create mac_binding datapath=$lr0_dp logical_port=lr0-pub \
> - ip=172.24.4.200 mac=\"aa:aa:aa:aa:aa:aa\"
> + ip=172.24.4.200 mac=\"aa:aa:aa:aa:aa:aa\" chassis=$chassis
>
> check ovn-nbctl lr-nat-add lr0 dnat_and_snat 172.24.4.100 10.0.0.10
> check ovn-nbctl lr-nat-add lr1 dnat_and_snat 172.24.4.200 20.0.0.10
> @@ -25851,18 +25858,19 @@ wait_for_ports_up
>
> wait_row_count datapath_binding 1 external-ids:name=lr0
> lr0_dp_uuid=$(ovn-sbctl --bare --columns _uuid list datapath_binding
> lr0)
> +chassis=$(fetch_column Chassis _uuid name=hv1)
>
> AT_CHECK(
> [ovn-sbctl create mac_binding datapath=$lr0_dp_uuid
> ip=172.168.0.120 \
> - logical_port=lr0-public mac="10\:54\:00\:00\:00\:03"
> + logical_port=lr0-public mac="10\:54\:00\:00\:00\:03"
> chassis=$chassis
> ovn-sbctl create mac_binding datapath=$lr0_dp_uuid
> ip=172.168.0.200 \
> - logical_port=lr0-public mac="10\:54\:00\:00\:00\:04"
> + logical_port=lr0-public mac="10\:54\:00\:00\:00\:04"
> chassis=$chassis
> ovn-sbctl create mac_binding datapath=$lr0_dp_uuid ip="bef0\:\:4"
> \
> - logical_port=lr0-public mac="10\:54\:00\:00\:00\:05"
> + logical_port=lr0-public mac="10\:54\:00\:00\:00\:05"
> chassis=$chassis
> ovn-sbctl create mac_binding datapath=$lr0_dp_uuid ip="bef0\:\:5"
> \
> - logical_port=lr0-public mac="10\:54\:00\:00\:00\:06"
> + logical_port=lr0-public mac="10\:54\:00\:00\:00\:06"
> chassis=$chassis
> ovn-sbctl create mac_binding datapath=$lr0_dp_uuid ip="bef0\:\:6"
> \
> - logical_port=lr0-public mac="10\:54\:00\:00\:00\:07"
> + logical_port=lr0-public mac="10\:54\:00\:00\:00\:07"
> chassis=$chassis
> ovn-nbctl -- --id=@lrt create Logical_Router_Static_Route \
> ip_prefix="\:\:/64" nexthop="bef0\:\:4" -- add Logical_Router
> lr0 \
> static_routes @lrt],
> @@ -29334,9 +29342,10 @@ AS_BOX([Test routing])
> # Before sending add mac_binding entry for 10.0.0.14
>
> lr0_dp_uuid=$(fetch_column datapath_binding _uuid
> external_ids:name=lr0)
> +chassis=$(fetch_column Chassis _uuid name=hv1)
>
> ovn-sbctl create mac_binding ip=10.0.0.14 logical_port=lr0-sw0 \
> -mac="50\:54\:00\:00\:00\:14" datapath=$lr0_dp_uuid
> +mac="50\:54\:00\:00\:00\:14" datapath=$lr0_dp_uuid chassis=$chassis
>
> # Wait till the mac_binding flows appear in hv1
> OVS_WAIT_UNTIL([test 1 = $(as hv1 ovs-ofctl dump-flows br-int
> table=66 \
> @@ -31496,8 +31505,9 @@ test_mac_binding_flows() {
> }
> # Create SB MAC_Binding entry on external gateway port
> lr0_dp_uuid=$(fetch_column datapath_binding _uuid
> external_ids:name=lr0)
> +chassis=$(fetch_column Chassis _uuid name=hv1)
>
> -ovn-sbctl create mac_binding ip=172.16.1.1 logical_port=lr0-ext-ls0
> mac="00\:00\:11\:22\:33\:44" datapath=$lr0_dp_uuid
> +ovn-sbctl create mac_binding ip=172.16.1.1 logical_port=lr0-ext-ls0
> mac="00\:00\:11\:22\:33\:44" datapath=$lr0_dp_uuid chassis=$chassis
> test_mac_binding_flows 100 00:00:11:22:33:44 1
>
> # Create Static_MAC_Binding entry on external gateway port. This
> should have
> @@ -31595,7 +31605,8 @@ test_mac_binding_flows() {
>
> # Create SB MAC_Binding entry on external gateway port
> gw_dp_uuid=$(fetch_column datapath_binding _uuid
> external_ids:name=gw)
> -ovn-sbctl create mac_binding ip=192.168.10.10 logical_port=gw-public
> mac="00\:00\:00\:00\:10\:10" datapath=$gw_dp_uuid
> +chassis=$(fetch_column Chassis _uuid name=hv1)
> +ovn-sbctl create mac_binding ip=192.168.10.10 logical_port=gw-public
> mac="00\:00\:00\:00\:10\:10" datapath=$gw_dp_uuid chassis=$chassis
>
> test_mac_binding_flows hv1 00\:00\:00\:00\:10\:10 1
> test_mac_binding_flows hv2 00\:00\:00\:00\:10\:10 0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
