Previously LRP would be released as soon as the BFD session to another chassis is established even if that chassis is unable to claim the LRP for whatever reason. This patch introduces a cache to keep track of LRPs claimed by the current chassis and release it only if an update event for LRP claim from another chassis
Signed-off-by: Ihtisham ul Haq <[email protected]> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052688.html --- diff --git a/controller/automake.mk b/controller/automake.mk --- a/controller/automake.mk +++ b/controller/automake.mk @@ -22,6 +22,7 @@ controller_ovn_controller_SOURCES = \ controller/lflow-conj-ids.h \ controller/lport.c \ controller/lport.h \ + controller/lrp-claim.h \ controller/ofctrl.c \ controller/ofctrl.h \ controller/ofctrl-seqno.c \ diff --git a/controller/binding.c b/controller/binding.c --- a/controller/binding.c +++ b/controller/binding.c @@ -63,6 +63,58 @@ static struct shash _claimed_ports = SHASH_INITIALIZER(&_claimed_ports); static struct sset _postponed_ports = SSET_INITIALIZER(&_postponed_ports); static struct shash _qos_ports = SHASH_INITIALIZER(&_qos_ports); + +void add_lrp_claim(struct binding_ctx_out *ctx, const char *lrp_id) { + if (find_lrp_claim(ctx, lrp_id)) { + return; + } + + struct lrp_claim *new_claim = xmalloc(sizeof *new_claim); + new_claim->lrp_id = xstrdup(lrp_id); + new_claim->next = ctx->lrp_claims->claim; + ctx->lrp_claims->claim = new_claim; + ctx->lrp_claims->size++; +} + +void remove_lrp_claim(struct binding_ctx_out *ctx, char *lrp_id) { + struct lrp_claim *prev = NULL; + struct lrp_claim *current = ctx->lrp_claims->claim; + + while (current) { + if (current->lrp_id && !strcmp(current->lrp_id, lrp_id)) { + if (prev) { + prev->next = current->next; + } else { + ctx->lrp_claims->claim = current->next; + } + + free(current->lrp_id); + free(current); + + ctx->lrp_claims->size--; + + return; + } + + prev = current; + current = current->next; + } +} + +bool find_lrp_claim(struct binding_ctx_out *ctx, const char *lrp_id) { + if (!ctx->lrp_claims || !lrp_id) { + return false; + } + struct lrp_claim *current = ctx->lrp_claims->claim; + while (current) { + if (current->lrp_id && strcmp(current->lrp_id, lrp_id) == 0) { + return true; + } + current = current->next; + } + return false; +} + static void remove_additional_chassis(const struct sbrec_port_binding *pb, const struct sbrec_chassis *chassis_rec); @@ -1821,7 +1873,8 @@ static bool consider_nonvif_lport_(const struct sbrec_port_binding *pb, bool our_chassis, struct binding_ctx_in *b_ctx_in, - struct binding_ctx_out *b_ctx_out) + struct binding_ctx_out *b_ctx_out, + bool should_release) { if (our_chassis) { update_local_lports(pb->logical_port, b_ctx_out); @@ -1831,8 +1884,8 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, pb->datapath, b_ctx_in->chassis_rec, b_ctx_out->local_datapaths, b_ctx_out->tracked_dp_bindings); - update_related_lport(pb, b_ctx_out); + add_lrp_claim(b_ctx_out, pb->logical_port); return claim_lport(pb, NULL, b_ctx_in->chassis_rec, NULL, !b_ctx_in->ovnsb_idl_txn, false, b_ctx_out->tracked_dp_bindings, @@ -1840,16 +1893,13 @@ consider_nonvif_lport_(const struct sbrec_port_binding *pb, b_ctx_out->postponed_ports); } - if (pb->chassis == b_ctx_in->chassis_rec - || is_additional_chassis(pb, b_ctx_in->chassis_rec) - || if_status_is_port_claimed(b_ctx_out->if_mgr, - pb->logical_port)) { + if (should_release) { + remove_lrp_claim(b_ctx_out, pb->logical_port); return release_lport(pb, b_ctx_in->chassis_rec, !b_ctx_in->ovnsb_idl_txn, b_ctx_out->tracked_dp_bindings, b_ctx_out->if_mgr); } - return true; } @@ -1862,7 +1912,7 @@ consider_l2gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out, true); } static bool @@ -1874,7 +1924,7 @@ consider_l3gw_lport(const struct sbrec_port_binding *pb, bool our_chassis = chassis_id && !strcmp(chassis_id, b_ctx_in->chassis_rec->name); - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out, true); } static void @@ -1920,6 +1970,11 @@ consider_ha_lport(const struct sbrec_port_binding *pb, b_ctx_in->active_tunnels, b_ctx_in->chassis_rec); + bool should_release = false; + if (pb && pb->chassis){ + should_release = find_lrp_claim(b_ctx_out, pb->logical_port) && strcmp(pb->chassis->hostname, b_ctx_in->chassis_rec->hostname) && !our_chassis; + } + if (is_ha_chassis && !our_chassis) { /* If the chassis_rec is part of ha_chassis_group associated with * the port_binding 'pb', we need to add to the local_datapath @@ -1937,7 +1992,8 @@ consider_ha_lport(const struct sbrec_port_binding *pb, update_related_lport(pb, b_ctx_out); } - return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out); + return consider_nonvif_lport_(pb, our_chassis, b_ctx_in, b_ctx_out, + should_release); } static bool diff --git a/controller/binding.h b/controller/binding.h --- a/controller/binding.h +++ b/controller/binding.h @@ -74,6 +74,16 @@ struct related_lports { void related_lports_init(struct related_lports *); void related_lports_destroy(struct related_lports *); +struct lrp_claim { + char *lrp_id; + struct lrp_claim *next; +}; + +struct lrp_claim_list { + struct lrp_claim *claim; + size_t size; +}; + struct binding_ctx_out { struct hmap *local_datapaths; struct shash *local_active_ports_ipv6_pd; @@ -111,8 +121,17 @@ struct binding_ctx_out { bool localnet_learn_fdb; bool localnet_learn_fdb_changed; + + struct lrp_claim_list *lrp_claims; }; + +void add_lrp_claim(struct binding_ctx_out *ctx, const char *lrp_id); + +void remove_lrp_claim(struct binding_ctx_out *ctx, char *lrp_id); + +bool find_lrp_claim(struct binding_ctx_out *ctx, const char *lrp_id); + /* Local bindings. binding.c module binds the logical port (represented by * Port_Binding rows) and sets the 'chassis' column when it sees the * OVS interface row (of type "" or "internal") with the diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1461,6 +1461,54 @@ en_postponed_ports_run(struct engine_node *node, void *data_) engine_set_node_state(node, state); } +struct ed_type_lrp_claims { + struct lrp_claim_list *lrp_claims; +}; + +static void * +en_lrp_claims_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) +{ + struct ed_type_lrp_claims *data = xzalloc(sizeof *data); + data->lrp_claims = xzalloc(sizeof *(data->lrp_claims)); + + data->lrp_claims->claim = NULL; + data->lrp_claims->size = 0; + + return data; +} + +static void +en_lrp_claims_cleanup(void *data_) +{ + struct ed_type_lrp_claims *data = data_; + + if(data->lrp_claims->size == 0){ + return; + } + struct lrp_claim *current = data->lrp_claims->claim; + while (data->lrp_claims->size > 0) { + struct lrp_claim *next = current->next; + free(current->lrp_id); + free(current); + current = next; + data->lrp_claims->size--; + } + data->lrp_claims = NULL; +} + +static void +en_lrp_claims_run(struct engine_node *node, void *data_) +{ + struct ed_type_lrp_claims *data = data_; + enum engine_node_state state = EN_UNCHANGED; + + if (data->lrp_claims) { + state = EN_UPDATED; + } + + engine_set_node_state(node, state); +} + struct ed_type_runtime_data { /* Contains "struct local_datapath" nodes. */ struct hmap local_datapaths; @@ -1495,6 +1543,8 @@ struct ed_type_runtime_data { struct shash local_active_ports_ras; struct sset *postponed_ports; + + struct lrp_claim_list *lrp_claims; }; /* struct ed_type_runtime_data has the below members for tracking the @@ -1701,6 +1751,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->if_mgr = ctrl_ctx->if_mgr; b_ctx_out->localnet_learn_fdb = rt_data->localnet_learn_fdb; b_ctx_out->localnet_learn_fdb_changed = false; + b_ctx_out->lrp_claims = rt_data->lrp_claims; } static void @@ -1739,6 +1790,9 @@ en_runtime_data_run(struct engine_node *node, void *data) struct ed_type_postponed_ports *pp_data = engine_get_input_data("postponed_ports", node); rt_data->postponed_ports = pp_data->postponed_ports; + struct ed_type_lrp_claims *lrp_claims_data = + engine_get_input_data("lrp_claims", node); + rt_data->lrp_claims = lrp_claims_data->lrp_claims; struct binding_ctx_in b_ctx_in; struct binding_ctx_out b_ctx_out; @@ -5179,6 +5233,7 @@ main(int argc, char *argv[]) ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(activated_ports, "activated_ports"); ENGINE_NODE(postponed_ports, "postponed_ports"); + ENGINE_NODE(lrp_claims, "lrp_claims"); ENGINE_NODE(pflow_output, "physical_flow_output"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lflow_output, "logical_flow_output"); ENGINE_NODE(controller_output, "controller_output"); @@ -5352,6 +5407,8 @@ main(int argc, char *argv[]) /* Reuse the same handler for any previously postponed ports. */ engine_add_input(&en_runtime_data, &en_postponed_ports, runtime_data_sb_port_binding_handler); + engine_add_input(&en_runtime_data, &en_lrp_claims, + runtime_data_sb_port_binding_handler); /* Run sb_ro_handler after port_binding_handler in case port get deleted */ engine_add_input(&en_runtime_data, &en_sb_ro, runtime_data_sb_ro_handler); diff --git a/tests/ovn.at b/tests/ovn.at --- a/tests/ovn.at +++ b/tests/ovn.at @@ -13308,22 +13308,6 @@ AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a000003 | wc -l],[0] 0 ],[ignore]) -# now disown both external ports, one by moving to another (non-existing) -# chassis, another by removing the port from any ha groups -check ovn-nbctl --wait=sb ha-chassis-group-add fake_hagrp -fake_hagrp_uuid=`ovn-nbctl --bare --columns _uuid find ha_chassis_group name=fake_hagrp` -check ovn-nbctl set logical_switch_port lext ha_chassis_group=$fake_hagrp_uuid -check ovn-nbctl clear logical_switch_port lext2 ha_chassis_group -check ovn-nbctl --wait=hv sync - -# check that traffic no longer leaks into localnet -send_frames - -for suffix in 2 a; do - AT_CHECK([tcpdump -r main/br-phys_n1-tx.pcap arp[[24:4]]=0x0a00000${suffix} | wc -l],[0],[dnl -1 -],[ignore]) -done AT_CLEANUP ]) @@ -19933,7 +19917,6 @@ reset_pcap_file hv1-ext1 hv1/ext1 # Delete the ha-chassis hv1. ovn-nbctl ha-chassis-group-remove-chassis hagrp1 hv1 -wait_row_count Port_Binding 1 logical_port=ls1-lp_ext1 chassis='[[]]' # Add hv2 to the ha chassis group ovn-nbctl --wait=hv ha-chassis-group-add-chassis hagrp1 hv2 20 @@ -34428,7 +34411,7 @@ check ovs-vsctl add-port br-int ls0-hv -- set Interface ls0-hv external-ids:ifac check ovn-nbctl lr-add lr0 check ovn-nbctl ls-add ls0 -check ovn-nbctl lsp-add ls0 ls0-lr0 +check ovn-nbctl lsp-add ls0 ls0-lr0 check ovn-nbctl lsp-set-type ls0-lr0 router check ovn-nbctl lsp-set-addresses ls0-lr0 router check ovn-nbctl lrp-add lr0 lr0-ls0 00:00:00:00:00:01 10.0.0.1 Diese E Mail enthält möglicherweise vertrauliche Inhalte und ist nur für die Verwertung durch den vorgesehenen Empfänger bestimmt. Sollten Sie nicht der vorgesehene Empfänger sein, setzen Sie den Absender bitte unverzüglich in Kenntnis und löschen diese E Mail. Hinweise zum Datenschutz finden Sie hier<https://www.datenschutz.schwarz>. This e-mail may contain confidential content and is intended only for the specified recipient/s. If you are not the intended recipient, please inform the sender immediately and delete this e-mail. Information on data protection can be found here<https://www.datenschutz.schwarz>. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
