On Fri, Nov 1, 2024 at 2:29 PM Dumitru Ceara <[email protected]> wrote:
> SB.MAC_Binding doesn't store a strong reference to SB.Port_Binding. > Instead it stores the port binding 'logical_port' name. As the > mac-binding cache in ovn-controller stored entries indexed by > [datapath-key, port-key, ip] it meant that when processing > SB.MAC_Binding changes (including deletion) it had to first lookup the > corresponding SB.Port_Binding. > > If the logical port associated with the MAC binding is deleted in the > same transaction then looking it up in the IDL index fails (deleted IDL > records are not indexed). This leads to stale entries staying in the > mac cache and in consequence to potential use after free (of the SB > MAC_Binding record) or to crashes if the statctrl module tries to update > timestamps of MAC_Bindings that happened to have been removed in the > current iteration (but still linger in the cache). > > In practice we don't need the port-key we can simply use the port > binding UUID as unique index. However, the same struct mac_binding type > is used by pinctrl to temporarily efficiently store buffered packets for > yet to be resolved MAC bindings. In those cases there's no SB > MAC_Binding record yet so the logical ingress port tunnel_key is used as > index. > > The two ways of using the MAC binding maps are orthogonal, that is, we > will never use a single map to store both types of bindings. > > In order to avoid issues we now include the SB MAC_Binding UUID into the > key of the struct mac_binding_data. > > Reported-at: https://issues.redhat.com/browse/FDP-906 > Fixes: b57f60a617b4 ("controller: Add MAC cache I-P node") > Signed-off-by: Dumitru Ceara <[email protected]> > --- > Hi Dumitru, thank you for the fix. There is just one small typo down below. We should probably think about a way of decoupling pinctrl and ovn-controller mac-cache in the future as it unfortunately doesn't work well together. controller/mac-cache.c | 77 ++++++++++++++++++------- > controller/mac-cache.h | 30 +++++++--- > controller/ovn-controller.c | 67 ++++++++++------------ > controller/pinctrl.c | 24 +++++--- > tests/ovn.at | 110 ++++++++++++++++++++++++++++++++++++ > 5 files changed, 236 insertions(+), 72 deletions(-) > > diff --git a/controller/mac-cache.c b/controller/mac-cache.c > index de45d7a6a5..d6ba9e20ee 100644 > --- a/controller/mac-cache.c > +++ b/controller/mac-cache.c > @@ -142,21 +142,18 @@ mac_cache_thresholds_clear(struct mac_cache_data > *data) > } > > /* MAC binding. */ > -struct mac_binding * > +void > mac_binding_add(struct hmap *map, struct mac_binding_data mb_data, > long long timestamp) { > > struct mac_binding *mb = mac_binding_find(map, &mb_data); > if (!mb) { > mb = xmalloc(sizeof *mb); > - mb->sbrec_mb = NULL; > hmap_insert(map, &mb->hmap_node, mac_binding_data_hash(&mb_data)); > } > > mb->data = mb_data; > mb->timestamp = timestamp; > - > - return mb; > } > > void > @@ -181,24 +178,37 @@ mac_binding_find(const struct hmap *map, > } > > bool > -mac_binding_data_from_sbrec(struct mac_binding_data *data, > - const struct sbrec_mac_binding *mb, > - struct ovsdb_idl_index *sbrec_pb_by_name) > +mac_binding_data_parse(struct mac_binding_data *data, > + uint32_t dp_key, uint32_t port_key, > + const char *ip_str, const char *mac_str) > { > - const struct sbrec_port_binding *pb = > - lport_lookup_by_name(sbrec_pb_by_name, mb->logical_port); > + struct eth_addr mac; > + struct in6_addr ip; > > - if (!pb || !pb->datapath || !ip46_parse(mb->ip, &data->ip) || > - !eth_addr_from_string(mb->mac, &data->mac)) { > + if (!ip46_parse(ip_str, &ip) || !eth_addr_from_string(mac_str, &mac)) > { > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "Couldn't parse MAC binding: ip=%s, mac=%s, " > - "logical_port=%s", mb->ip, mb->mac, > mb->logical_port); > + VLOG_WARN_RL(&rl, "Couldn't parse MAC binding: ip=%s, mac=%s", > + ip_str, mac_str); > return false; > } > > - data->dp_key = mb->datapath->tunnel_key; > - data->port_key = pb->tunnel_key; > + mac_binding_data_init(data, dp_key, port_key, ip, mac); > + return true; > +} > + > +bool > +mac_binding_data_from_sbrec(struct mac_binding_data *data, > + const struct sbrec_mac_binding *mb) > +{ > + /* This explicitly sets the port_key to 0 as port_binding tunnel_keys > + * can change. Instead use add the SB.MAC_Binding UUID as key; this > + * makes the mac_binding_data key unique. */ > + if (!mac_binding_data_parse(data, mb->datapath->tunnel_key, 0, > + mb->ip, mb->mac)) { > + return false; > + } > > + data->sbrec = mb; > return true; > } > > @@ -211,6 +221,30 @@ mac_bindings_clear(struct hmap *map) > } > } > > +void > +mac_bindings_to_string(const struct hmap *map, struct ds *out_data) > +{ > + struct mac_binding *mb; > + HMAP_FOR_EACH (mb, hmap_node, map) { > + char ip[INET6_ADDRSTRLEN]; > + > + if (!ipv6_string_mapped(ip, &mb->data.ip)) { > + continue; > + } > + if (mb->data.sbrec) { > + ds_put_format(out_data, "SB UUID: " UUID_FMT ", ", > + UUID_ARGS(&mb->data.sbrec->header_.uuid)); > + } else { > + ds_put_cstr(out_data, "SB UUID: <none>, "); > + } > + ds_put_format(out_data, "datapath-key: %"PRIu32", " > + "port-key: %"PRIu32", " > + "ip: %s, mac: " ETH_ADDR_FMT "\n", > + mb->data.dp_key, mb->data.port_key, > + ip, ETH_ADDR_ARGS(mb->data.mac)); > + } > +} > + > bool > sb_mac_binding_updated(const struct sbrec_mac_binding *mb) > { > @@ -382,9 +416,9 @@ mac_binding_stats_run(struct ovs_list *stats_list, > uint64_t *req_delay, > * used on this chassis. Also make sure that we don't update the > * timestamp more than once during the dump period. */ > if (stats->idle_age_ms < threshold->value && > - (timewall_now - mb->sbrec_mb->timestamp) >= > + (timewall_now - mb->data.sbrec->timestamp) >= > threshold->dump_period) { > - sbrec_mac_binding_set_timestamp(mb->sbrec_mb, timewall_now); > + sbrec_mac_binding_set_timestamp(mb->data.sbrec, timewall_now); > } > > free(stats); > @@ -608,7 +642,9 @@ mac_cache_stats_destroy(struct ovs_list *stats_list) > static uint32_t > mac_binding_data_hash(const struct mac_binding_data *mb_data) > { > - uint32_t hash = 0; > + uint32_t hash = mb_data->sbrec > + ? uuid_hash(&mb_data->sbrec->header_.uuid) > + : 0; > > hash = hash_add(hash, mb_data->port_key); > hash = hash_add(hash, mb_data->dp_key); > @@ -621,9 +657,10 @@ static inline bool > mac_binding_data_equals(const struct mac_binding_data *a, > const struct mac_binding_data *b) > { > - return a->port_key == b->port_key && > + return a->sbrec == b->sbrec && > + a->port_key == b->port_key && > a->dp_key == b->dp_key && > - ipv6_addr_equals(&a->ip, &b->ip); > + ipv6_addr_equals(&a->ip, &b->ip); > } > > static uint32_t > diff --git a/controller/mac-cache.h b/controller/mac-cache.h > index b94e7a1cf2..df84ef072c 100644 > --- a/controller/mac-cache.h > +++ b/controller/mac-cache.h > @@ -52,6 +52,7 @@ struct mac_cache_threshold { > > struct mac_binding_data { > /* Keys. */ > + const struct sbrec_mac_binding * sbrec; /* Only the UUID is used as > key.*/ > nit: Space after *. > uint32_t port_key; > uint32_t dp_key; > struct in6_addr ip; > @@ -63,8 +64,6 @@ struct mac_binding { > struct hmap_node hmap_node; > /* Common data to identify MAC binding. */ > struct mac_binding_data data; > - /* Reference to the SB MAC binding record (Might be NULL). */ > - const struct sbrec_mac_binding *sbrec_mb; > /* User specified timestamp (in ms) */ > long long timestamp; > }; > @@ -128,20 +127,37 @@ void mac_cache_thresholds_sync(struct mac_cache_data > *data, > void mac_cache_thresholds_clear(struct mac_cache_data *data); > > /* MAC binding. */ > -struct mac_binding *mac_binding_add(struct hmap *map, > - struct mac_binding_data mb_data, > - long long timestamp); > + > +static inline void > +mac_binding_data_init(struct mac_binding_data *data, > + uint32_t dp_key, uint32_t port_key, > + struct in6_addr ip, struct eth_addr mac) > +{ > + *data = (struct mac_binding_data) { > + .sbrec = NULL, > + .dp_key = (dp_key), > + .port_key = (port_key), > + .ip = (ip), > + .mac = (mac), > + }; > +} > + > +void mac_binding_add(struct hmap *map, struct mac_binding_data mb_data, > + long long timestamp); > > void mac_binding_remove(struct hmap *map, struct mac_binding *mb); > > struct mac_binding *mac_binding_find(const struct hmap *map, > const struct mac_binding_data > *mb_data); > > +bool mac_binding_data_parse(struct mac_binding_data *data, > + uint32_t dp_key, uint32_t port_key, > + const char *ip_str, const char *mac_str); > bool mac_binding_data_from_sbrec(struct mac_binding_data *data, > - const struct sbrec_mac_binding *mb, > - struct ovsdb_idl_index > *sbrec_pb_by_name); > + const struct sbrec_mac_binding *mb); > > void mac_bindings_clear(struct hmap *map); > +void mac_bindings_to_string(const struct hmap *map, struct ds *out_data); > > bool sb_mac_binding_updated(const struct sbrec_mac_binding *mb); > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index e87274121c..ab95938502 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -101,6 +101,7 @@ static unixctl_cb_func debug_status_execution; > static unixctl_cb_func debug_dump_local_bindings; > static unixctl_cb_func debug_dump_related_lports; > static unixctl_cb_func debug_dump_local_template_vars; > +static unixctl_cb_func debug_dump_local_mac_bindings; > static unixctl_cb_func debug_dump_lflow_conj_ids; > static unixctl_cb_func lflow_cache_flush_cmd; > static unixctl_cb_func lflow_cache_show_stats_cmd; > @@ -2932,26 +2933,22 @@ en_lb_data_cleanup(void *data) > > static void > mac_binding_add_sb(struct mac_cache_data *data, > - const struct sbrec_mac_binding *smb, > - struct ovsdb_idl_index *sbrec_pb_by_name) > + const struct sbrec_mac_binding *smb) > { > struct mac_binding_data mb_data; > - if (!mac_binding_data_from_sbrec(&mb_data, smb, sbrec_pb_by_name)) { > + if (!mac_binding_data_from_sbrec(&mb_data, smb)) { > return; > } > > - struct mac_binding *mb = mac_binding_add(&data->mac_bindings, > mb_data, 0); > - > - mb->sbrec_mb = smb; > + mac_binding_add(&data->mac_bindings, mb_data, 0); > } > > static void > mac_binding_remove_sb(struct mac_cache_data *data, > - const struct sbrec_mac_binding *smb, > - struct ovsdb_idl_index *sbrec_pb_by_name) > + const struct sbrec_mac_binding *smb) > { > struct mac_binding_data mb_data; > - if (!mac_binding_data_from_sbrec(&mb_data, smb, sbrec_pb_by_name)) { > + if (!mac_binding_data_from_sbrec(&mb_data, smb)) { > return; > } > > @@ -2995,8 +2992,7 @@ fdb_remove_sb(struct mac_cache_data *data, const > struct sbrec_fdb *sfdb) > static void > mac_cache_mb_handle_for_datapath(struct mac_cache_data *data, > const struct sbrec_datapath_binding *dp, > - struct ovsdb_idl_index *sbrec_mb_by_dp, > - struct ovsdb_idl_index *sbrec_pb_by_name) > + struct ovsdb_idl_index *sbrec_mb_by_dp) > { > bool has_threshold = mac_cache_threshold_find(data, dp->tunnel_key); > > @@ -3007,9 +3003,9 @@ mac_cache_mb_handle_for_datapath(struct > mac_cache_data *data, > const struct sbrec_mac_binding *mb; > SBREC_MAC_BINDING_FOR_EACH_EQUAL (mb, mb_index_row, sbrec_mb_by_dp) { > if (has_threshold) { > - mac_binding_add_sb(data, mb, sbrec_pb_by_name); > + mac_binding_add_sb(data, mb); > } else { > - mac_binding_remove_sb(data, mb, sbrec_pb_by_name); > + mac_binding_remove_sb(data, mb); > } > } > > @@ -3064,10 +3060,6 @@ en_mac_cache_run(struct engine_node *node, void > *data) > engine_ovsdb_node_get_index( > engine_get_input("SB_mac_binding", node), > "datapath"); > - struct ovsdb_idl_index *sbrec_pb_by_name = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_port_binding", node), > - "name"); > struct ovsdb_idl_index *sbrec_fdb_by_dp_key = > engine_ovsdb_node_get_index( > engine_get_input("SB_fdb", node), > @@ -3086,7 +3078,7 @@ en_mac_cache_run(struct engine_node *node, void > *data) > > mac_cache_threshold_add(cache_data, sbrec_dp); > mac_cache_mb_handle_for_datapath(cache_data, sbrec_dp, > - sbrec_mb_by_dp, > sbrec_pb_by_name); > + sbrec_mb_by_dp); > mac_cache_fdb_handle_for_datapath(cache_data, sbrec_dp, > sbrec_fdb_by_dp_key); > } > @@ -3102,11 +3094,6 @@ mac_cache_sb_mac_binding_handler(struct engine_node > *node, void *data) > engine_get_input_data("runtime_data", node); > const struct sbrec_mac_binding_table *mb_table = > EN_OVSDB_GET(engine_get_input("SB_mac_binding", node)); > - struct ovsdb_idl_index *sbrec_pb_by_name = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_port_binding", node), > - "name"); > - > size_t previous_size = hmap_count(&cache_data->mac_bindings); > > const struct sbrec_mac_binding *sbrec_mb; > @@ -3116,8 +3103,7 @@ mac_cache_sb_mac_binding_handler(struct engine_node > *node, void *data) > } > > if (!sbrec_mac_binding_is_new(sbrec_mb)) { > - mac_binding_remove_sb(cache_data, sbrec_mb, > - sbrec_pb_by_name); > + mac_binding_remove_sb(cache_data, sbrec_mb); > } > > if (sbrec_mac_binding_is_deleted(sbrec_mb) || > @@ -3128,7 +3114,7 @@ mac_cache_sb_mac_binding_handler(struct engine_node > *node, void *data) > > if (mac_cache_threshold_find(cache_data, > sbrec_mb->datapath->tunnel_key)) { > - mac_binding_add_sb(cache_data, sbrec_mb, sbrec_pb_by_name); > + mac_binding_add_sb(cache_data, sbrec_mb); > } > } > > @@ -3189,10 +3175,6 @@ mac_cache_runtime_data_handler(struct engine_node > *node, void *data OVS_UNUSED) > engine_ovsdb_node_get_index( > engine_get_input("SB_mac_binding", node), > "datapath"); > - struct ovsdb_idl_index *sbrec_pb_by_name = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_port_binding", node), > - "name"); > struct ovsdb_idl_index *sbrec_fdb_by_dp_key = > engine_ovsdb_node_get_index( > engine_get_input("SB_fdb", node), > @@ -3217,7 +3199,7 @@ mac_cache_runtime_data_handler(struct engine_node > *node, void *data OVS_UNUSED) > > HMAP_FOR_EACH (tdp, node, &rt_data->tracked_dp_bindings) { > mac_cache_mb_handle_for_datapath(cache_data, tdp->dp, > - sbrec_mb_by_dp, > sbrec_pb_by_name); > + sbrec_mb_by_dp); > > mac_cache_fdb_handle_for_datapath(cache_data, tdp->dp, > sbrec_fdb_by_dp_key); > @@ -3243,10 +3225,6 @@ mac_cache_sb_datapath_binding_handler(struct > engine_node *node, void *data) > engine_ovsdb_node_get_index( > engine_get_input("SB_mac_binding", node), > "datapath"); > - struct ovsdb_idl_index *sbrec_pb_by_name = > - engine_ovsdb_node_get_index( > - engine_get_input("SB_port_binding", node), > - "name"); > struct ovsdb_idl_index *sbrec_fdb_by_dp_key = > engine_ovsdb_node_get_index( > engine_get_input("SB_fdb", node), > @@ -3274,7 +3252,7 @@ mac_cache_sb_datapath_binding_handler(struct > engine_node *node, void *data) > > SBREC_DATAPATH_BINDING_TABLE_FOR_EACH_TRACKED (sbrec_dp, dp_table) { > mac_cache_mb_handle_for_datapath(cache_data, sbrec_dp, > - sbrec_mb_by_dp, > sbrec_pb_by_name); > + sbrec_mb_by_dp); > > mac_cache_fdb_handle_for_datapath(cache_data, sbrec_dp, > sbrec_fdb_by_dp_key); > @@ -5399,6 +5377,10 @@ main(int argc, char *argv[]) > debug_dump_local_template_vars, > &template_vars_data->local_templates); > > + unixctl_command_register("debug/dump-mac-bindings", "", 0, 0, > + debug_dump_local_mac_bindings, > + &mac_cache_data->mac_bindings); > + > unixctl_command_register("debug/ignore-startup-delay", "", 0, 0, > debug_ignore_startup_delay, NULL); > > @@ -6356,6 +6338,19 @@ debug_dump_local_template_vars(struct unixctl_conn > *conn, int argc OVS_UNUSED, > ds_destroy(&tv_str); > } > > +static void > +debug_dump_local_mac_bindings(struct unixctl_conn *conn, int argc > OVS_UNUSED, > + const char *argv[] OVS_UNUSED, > + void *mac_bindings) > +{ > + struct ds mb_str = DS_EMPTY_INITIALIZER; > + > + ds_put_cstr(&mb_str, "Local MAC bindings:\n"); > + mac_bindings_to_string(mac_bindings, &mb_str); > + unixctl_command_reply(conn, ds_cstr(&mb_str)); > + ds_destroy(&mb_str); > +} > + > static void > debug_ignore_startup_delay(struct unixctl_conn *conn, int argc OVS_UNUSED, > const char *argv[] OVS_UNUSED, void *arg > OVS_UNUSED) > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index dfb3130aa2..ca880a1da6 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -1539,19 +1539,20 @@ pinctrl_handle_buffered_packets(const struct > ofputil_packet_in *pin, > OVS_REQUIRES(pinctrl_mutex) > { > const struct match *md = &pin->flow_metadata; > - struct mac_binding_data mb_data = (struct mac_binding_data) { > - .dp_key = ntohll(md->flow.metadata), > - .port_key = md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0], > - .mac = eth_addr_zero, > - }; > + struct mac_binding_data mb_data; > + struct in6_addr ip; > > if (is_arp) { > - mb_data.ip = in6_addr_mapped_ipv4(htonl(md->flow.regs[0])); > + ip = in6_addr_mapped_ipv4(htonl(md->flow.regs[0])); > } else { > ovs_be128 ip6 = hton128(flow_get_xxreg(&md->flow, 0)); > - memcpy(&mb_data.ip, &ip6, sizeof mb_data.ip); > + memcpy(&ip, &ip6, sizeof ip); > } > > + mac_binding_data_init(&mb_data, ntohll(md->flow.metadata), > + md->flow.regs[MFF_LOG_OUTPORT - MFF_REG0], > + ip, eth_addr_zero); > + > struct buffered_packets *bp = > buffered_packets_add(&buffered_packets_ctx, > mb_data); > if (!bp) { > @@ -4959,9 +4960,14 @@ run_buffered_binding(const struct > sbrec_mac_binding_table *mac_binding_table, > const struct sbrec_port_binding *pb = lport_lookup_by_name( > sbrec_port_binding_by_name, smb->logical_port); > > + if (!pb || !pb->datapath) { > + continue; > + } > + > struct mac_binding_data mb_data; > - if (!mac_binding_data_from_sbrec(&mb_data, smb, > - sbrec_port_binding_by_name)) { > + > + if (!mac_binding_data_parse(&mb_data, smb->datapath->tunnel_key, > + pb->tunnel_key, smb->ip, smb->mac)) { > continue; > } > > diff --git a/tests/ovn.at b/tests/ovn.at > index 10cd7a79b9..44d58ca160 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -35168,6 +35168,116 @@ OVN_CLEANUP([hv1], [hv2]) > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([MAC binding aging - port deletion]) > +AT_SKIP_IF([test $HAVE_SCAPY = no]) > +ovn_start > + > +net_add n1 > +sim_add hv1 > +as hv1 > +check ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +check ovn-nbctl \ > + -- ls-add ls1 \ > + -- ls-add ls2 \ > + -- lr-add lr \ > + -- set logical_router lr options:mac_binding_age_threshold=3600 \ > + -- lrp-add lr lr-ls1 00:00:00:00:10:00 192.168.10.1/24 \ > + -- lrp-add lr lr-ls2 00:00:00:00:20:00 192.168.20.1/24 \ > + -- lsp-add ls1 ls1-lr \ > + -- lsp-set-type ls1-lr router \ > + -- lsp-set-addresses ls1-lr router \ > + -- lsp-set-options ls1-lr router-port=lr-ls1 \ > + -- lsp-add ls1 vif1 \ > + -- lsp-set-addresses vif1 "00:00:00:00:10:10 192.168.10.10" \ > + -- lsp-add ls2 ls2-lr \ > + -- lsp-set-type ls2-lr router \ > + -- lsp-set-addresses ls2-lr router \ > + -- lsp-set-options ls2-lr router-port=lr-ls2 \ > + -- lsp-add ls2 vif2 \ > + -- lsp-set-addresses vif2 "00:00:00:00:20:10 192.168.20.10" > + > +check ovs-vsctl \ > + -- add-port br-int vif1 \ > + -- set interface vif1 external-ids:iface-id=vif1 \ > + -- add-port br-int vif2 \ > + -- set interface vif2 external-ids:iface-id=vif2 > + > +OVN_POPULATE_ARP > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +dnl Wait for pinctrl thread to be connected. > +OVS_WAIT_UNTIL([grep pinctrl hv1/ovn-controller.log | grep -q connected]) > + > +send_garp() { > + hv=$1 > + dev=$2 > + mac=$3 > + ip=$4 > + > + packet=$(fmt_pkt "Ether(dst='ff:ff:ff:ff:ff:ff', src='${mac}')/ \ > + ARP(op=2, hwsrc='${mac}', hwdst='${mac}', \ > + psrc='${ip}', pdst='${ip}')") > + as $hv ovs-appctl netdev-dummy/receive $dev $packet > +} > + > +AS_BOX([Remove LRP with learnt MAC_Binding]) > +dnl Populate MAC Binding entry. > +send_garp hv1 vif1 00:00:00:00:10:10 192.168.10.10 > +wait_row_count mac_binding 1 ip="192.168.10.10" logical_port="lr-ls1" > + > +dnl Check local mac binding cache. > +lr_key=$(fetch_column datapath_binding tunnel_key external_ids:name=lr) > +mb_uuid=$(fetch_column mac_binding _uuid logical_port=lr-ls1) > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([as hv1 ovn-appctl -t ovn-controller > debug/dump-mac-bindings], [0], [dnl > +Local MAC bindings: > +SB UUID: $mb_uuid, datapath-key: $lr_key, port-key: 0, ip: 192.168.10.10, > mac: 00:00:00:00:10:10 > +]) > + > +dnl Remove router port. This deletes the mac binding in the same > transaction. > +check ovn-nbctl --wait=hv lrp-del lr-ls1 > + > +dnl Check local mac binding cache - it should be empty now. > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-mac-bindings], > [0], [dnl > +Local MAC bindings: > +]) > + > +AS_BOX([Change LRP tunnel-key with learnt MAC_Binding and remove LRP]) > +dnl Re-add port. > +check ovn-nbctl \ > + -- set logical_router_port lr-ls2 options:requested-tnl-key=42 > +check ovn-nbctl --wait=hv sync > +check_column 42 Port_Binding tunnel_key logical_port=lr-ls2 > + > +dnl Populate MAC Binding entry. > +send_garp hv1 vif2 00:00:00:00:20:10 192.168.20.10 > +wait_row_count mac_binding 1 ip="192.168.20.10" logical_port="lr-ls2" > + > +dnl Check local mac binding cache. > +mb_uuid=$(fetch_column mac_binding _uuid logical_port=lr-ls2) > +OVS_WAIT_FOR_OUTPUT_UNQUOTED([as hv1 ovn-appctl -t ovn-controller > debug/dump-mac-bindings], [0], [dnl > +Local MAC bindings: > +SB UUID: $mb_uuid, datapath-key: $lr_key, port-key: 0, ip: 192.168.20.10, > mac: 00:00:00:00:20:10 > +]) > + > +dnl Change LRP tunnel key. > +check ovn-nbctl --wait=hv set logical_router_port lr-ls2 > options:requested-tnl-key=43 > +check_column 43 Port_Binding tunnel_key logical_port=lr-ls2 > + > +dnl Remove router port. This deletes the mac binding in the same > transaction. > +check ovn-nbctl --wait=hv lrp-del lr-ls2 > +dnl Check local mac binding cache - it should be empty now. > +AT_CHECK([as hv1 ovn-appctl -t ovn-controller debug/dump-mac-bindings], > [0], [dnl > +Local MAC bindings: > +]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([router port type update and then remove]) > ovn_start > -- > 2.46.2 > > Other than that the fix looks good. Acked-by: Ales Musil <[email protected]> Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
