| Message ID | 20220617090759.513730-2-amusil@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | MAC binding aging mechanism | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
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 <amusil@redhat.com> > --- > 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
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", - "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
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 <amusil@redhat.com> --- 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(-)