This change will be useful for migration cases, where it can be used to sync ct-entries before port is up on new chassis, resulting in reduced network package drops. It also fulfills the need of any other service which might need advance ct-zone reservation in future.
Restarting this patch again which was reviewed here originally: https://mail.openvswitch.org/pipermail/ovs-dev/2024-September/417457.html Signed-off-by: Mansi Sharma <mansi.sha...@nutanix.com> --- v8->v9 1. Restructured code to have common function which gives current list of reserved ports. 2. Rebased to latest code 3. Details about syncing conntrack entries between chassis can be found in this repo - https://github.com/nutanix/conntrack-migrator v9->v10 1. Fixed memory leaks. --- controller/ct-zone.c | 54 ++++++++++++++-- controller/ct-zone.h | 9 ++- controller/ovn-controller.c | 18 ++++-- tests/ovn-controller.at | 119 ++++++++++++++++++++++++++++++++---- 4 files changed, 178 insertions(+), 22 deletions(-) diff --git a/controller/ct-zone.c b/controller/ct-zone.c index 469a8fc54..ba7dbf4e6 100644 --- a/controller/ct-zone.c +++ b/controller/ct-zone.c @@ -118,6 +118,37 @@ ct_zones_restore(struct ct_zone_ctx *ctx, } } +void +ct_zones_reserved_lports(const struct ovsrec_open_vswitch_table *ovs_table, + struct sset * reserved_lports_set) +{ + const struct ovsrec_open_vswitch *cfg; + cfg = ovsrec_open_vswitch_table_first(ovs_table); + + if (cfg == NULL) { + return; + } + + const char *reserve_ct_zone_request_list = smap_get( + &cfg->external_ids, "reserve_ct_zones"); + + if (reserve_ct_zone_request_list == NULL) { + return; + } + + char *dup_reserve_list = xstrdup(reserve_ct_zone_request_list); + char *reserve_port; + char *save_ptr = NULL; + + for (reserve_port = strtok_r(dup_reserve_list, ",", &save_ptr); + reserve_port != NULL; + reserve_port = strtok_r(NULL, ",", &save_ptr)) { + sset_add(reserved_lports_set, reserve_port); + free(reserve_port); + } + free(dup_reserve_list); +} + void ct_zones_parse_range(const struct ovsrec_open_vswitch_table *ovs_table, int *min_ct_zone, int *max_ct_zone) @@ -169,7 +200,8 @@ out: void ct_zones_update(const struct sset *local_lports, const struct ovsrec_open_vswitch_table *ovs_table, - const struct hmap *local_datapaths, struct ct_zone_ctx *ctx) + const struct hmap *local_datapaths, struct ct_zone_ctx *ctx, + struct sset *reserved_lports) { int min_ct_zone, max_ct_zone; const char *user; @@ -183,6 +215,13 @@ ct_zones_update(const struct sset *local_lports, sset_add(&all_users, local_lport); } + if (!sset_is_empty(reserved_lports)) { + const char *reserved_lport; + SSET_FOR_EACH (reserved_lport, reserved_lports) { + sset_add(&all_users, reserved_lport); + } + } + /* Local patched datapath (gateway routers) need zones assigned. */ const struct local_datapath *ld; HMAP_FOR_EACH (ld, hmap_node, local_datapaths) { @@ -397,12 +436,15 @@ ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, /* Returns "true" if there was an update to the context. */ bool -ct_zone_handle_port_update(struct ct_zone_ctx *ctx, - const struct sbrec_port_binding *pb, - bool updated, int *scan_start, - int min_ct_zone, int max_ct_zone) +ct_zone_handle_port_update( + struct ct_zone_ctx *ctx, + const struct sbrec_port_binding *pb, + bool updated, int *scan_start, + int min_ct_zone, int max_ct_zone, + struct sset *reserved_lports) { struct shash_node *node = shash_find(&ctx->current, pb->logical_port); + bool is_reserved = node && sset_contains(reserved_lports, node->name); if (node) { struct ct_zone *ct_zone = node->data; @@ -419,6 +461,8 @@ ct_zone_handle_port_update(struct ct_zone_ctx *ctx, } ct_zone_limit_update(ctx, pb->logical_port, ct_zone_get_pb_limit(pb)); return true; + } else if (node && is_reserved) { + return true; } else if (node && ct_zone_remove(ctx, node->name)) { return true; } diff --git a/controller/ct-zone.h b/controller/ct-zone.h index 6df03975c..85314d91d 100644 --- a/controller/ct-zone.h +++ b/controller/ct-zone.h @@ -64,6 +64,9 @@ struct ct_zone_pending_entry { void ct_zone_ctx_init(struct ct_zone_ctx *ctx); void ct_zone_ctx_destroy(struct ct_zone_ctx *ctx); +void ct_zones_reserved_lports( + const struct ovsrec_open_vswitch_table *ovs_table, + struct sset * reserved_lports_set); void ct_zones_parse_range(const struct ovsrec_open_vswitch_table *ovs_table, int *min_ct_zone, int *max_ct_zone); void ct_zones_restore(struct ct_zone_ctx *ctx, @@ -73,7 +76,8 @@ void ct_zones_restore(struct ct_zone_ctx *ctx, void ct_zones_update(const struct sset *local_lports, const struct ovsrec_open_vswitch_table *ovs_table, const struct hmap *local_datapaths, - struct ct_zone_ctx *ctx); + struct ct_zone_ctx *ctx, + struct sset *reserved_lports); void ct_zones_commit(const struct ovsrec_bridge *br_int, const struct ovsrec_datapath *ovs_dp, struct ovsdb_idl_txn *ovs_idl_txn, @@ -85,7 +89,8 @@ bool ct_zone_handle_dp_update(struct ct_zone_ctx *ctx, bool ct_zone_handle_port_update(struct ct_zone_ctx *ctx, const struct sbrec_port_binding *pb, bool updated, int *scan_start, - int min_ct_zone, int max_ct_zone); + int min_ct_zone, int max_ct_zone, + struct sset *reserved_lports); uint16_t ct_zone_find_zone(const struct shash *ct_zones, const char *name); void ct_zones_limits_sync(struct ct_zone_ctx *ctx, const struct hmap *local_datapaths, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 910963e25..ec4740744 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -2317,10 +2317,13 @@ en_ct_zones_run(struct engine_node *node, void *data) EN_OVSDB_GET(engine_get_input("SB_datapath_binding", node)); const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table); - + struct sset reserved_lports = SSET_INITIALIZER(&reserved_lports); + ct_zones_reserved_lports(ovs_table, &reserved_lports); ct_zones_restore(&ct_zones_data->ctx, ovs_table, dp_table, br_int); ct_zones_update(&rt_data->local_lports, ovs_table, - &rt_data->local_datapaths, &ct_zones_data->ctx); + &rt_data->local_datapaths, &ct_zones_data->ctx, + &reserved_lports); + sset_destroy(&reserved_lports); ct_zones_limits_sync(&ct_zones_data->ctx, &rt_data->local_datapaths, &rt_data->lbinding_data.lports); @@ -2387,6 +2390,9 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) ct_zones_parse_range(ovs_table, &min_ct_zone, &max_ct_zone); scan_start = min_ct_zone; + struct sset reserved_lports = SSET_INITIALIZER(&reserved_lports); + ct_zones_reserved_lports(ovs_table, &reserved_lports); + HMAP_FOR_EACH (tdp, node, tracked_dp_bindings) { if (tdp->tracked_type == TRACKED_RESOURCE_NEW) { /* A new datapath has been added. Fall back to full recompute. */ @@ -2407,7 +2413,8 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, t_lport->pb, false, &scan_start, - min_ct_zone, max_ct_zone); + min_ct_zone, max_ct_zone, + &reserved_lports); } continue; @@ -2419,10 +2426,13 @@ ct_zones_runtime_data_handler(struct engine_node *node, void *data) updated |= ct_zone_handle_port_update(&ct_zones_data->ctx, t_lport->pb, port_updated, &scan_start, - min_ct_zone, max_ct_zone); + min_ct_zone, max_ct_zone, + &reserved_lports); } } + sset_destroy(&reserved_lports); + if (updated) { engine_set_node_state(node, EN_UPDATED); } diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 73bd50358..29fd91886 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -2579,7 +2579,7 @@ ovn_attach n1 br-phys 192.168.0.1 get_zone_num () { output=$1 name=$2 - printf "$output" | grep $name | cut -d ' ' -f 2 + echo "$output" | grep $name | cut -d ' ' -f 2 } check_ovsdb_zone() { @@ -2589,8 +2589,14 @@ check_ovsdb_zone() { test $ct_zone -eq $db_zone } +check_duplicates() { + output=$1 + AT_CHECK([echo "$output" | tr ' ' '\n' | sort | uniq -d | grep .], [1]) +} + check ovs-vsctl add-port br-int ls0-hv1 -- set Interface ls0-hv1 external-ids:iface-id=ls0-hv1 check ovs-vsctl add-port br-int ls0-hv2 -- set Interface ls0-hv2 external-ids:iface-id=ls0-hv2 +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv3,ls0-req-hv4 check ovn-nbctl lr-add lr0 @@ -2612,21 +2618,23 @@ check ovn-nbctl lrp-set-gateway-chassis lrp-gw hv1 check ovn-nbctl --wait=hv sync ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) -echo "$ct_zones" + port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) echo "snat_zone is $snat_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" - OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +check_duplicates "$ct_zones" # Now purposely request an SNAT zone for lr0 that conflicts with a zone # currently assigned to a logical port @@ -2636,20 +2644,110 @@ check ovn-nbctl set Logical_Router lr0 options:snat-ct-zone=$snat_req_zone check ovn-nbctl --wait=hv sync ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) -echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) snat_zone=$(get_zone_num "$ct_zones" lr0_snat) +req_port3_zone=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone=$(get_zone_num "$ct_zones" ls0-req-hv4) + check test "$snat_zone" -eq "$snat_req_zone" -check test "$port1_zone" -ne "$port2_zone" -check test "$port2_zone" -ne "$snat_zone" -check test "$port1_zone" -ne "$snat_zone" +check_duplicates "$ct_zones" + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +# Add port named ls0-req-hv3 and check if same zone assigned +# previously get assigned to it this time as well. + +check ovn-nbctl lsp-add ls0 ls0-req-hv3 +check ovs-vsctl -- add-port br-int hv3-vif3 -- \ + set interface hv3-vif3 external-ids:iface-id=ls0-req-hv3 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) + +check ovn-nbctl --wait=hv sync + +req_port3_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv3) +check test "$req_port3_zone" -eq "$req_port3_zone_new" +check_duplicates "$ct_zones" OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv4 $req_port4_zone]) + +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv3], +[0], [ignore]) + +# Checks for two cases after removing entry from ovs_vswitch table - +# 1. If port is already up, ct-zone should be reserved. +# 2. If port is not up yet, ct-zone should not be reserved. + +check ovs-vsctl remove Open_vSwitch . external_ids reserve_ct_zones + +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) + +req_port3_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv3) +req_port4_zone_after_delete=$(get_zone_num "$ct_zones" ls0-req-hv4) + +check test "$req_port3_zone_new" -eq "$req_port3_zone_after_delete" + +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv3], +[0], [ignore]) + +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv4], +[1], [ignore], [ignore]) + +# check test "$req_port4_zone_after_delete" == "" + +# Checks for case when a ct-zone is reserved it comes up on that chassis, and +# gets deleted, but its persisted in ovs_vswitch table, it should persist the +# same zone throughout. + +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv5 +check ovn-nbctl lsp-add ls0 ls0-req-hv5 +check ovs-vsctl -- add-port br-int hv5-vif5 -- \ + set interface hv5-vif5 external-ids:iface-id=ls0-req-hv5 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +req_port5_zone=$(get_zone_num "$ct_zones" ls0-req-hv5) +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv5], +[0], [ignore]) + +check ovs-vsctl remove interface hv5-vif5 external_ids iface-id +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +req_port5_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv5) +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv5], +[0], [ignore]) +check test "$req_port5_zone" -eq "$req_port5_zone_new" +check_duplicates "$ct_zones" + +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv1 $port1_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-hv2 $port2_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone lr0_snat $snat_zone]) +OVS_WAIT_UNTIL([check_ovsdb_zone ls0-req-hv3 $req_port3_zone]) + +check ovs-vsctl set Open_vSwitch . external_ids:reserve_ct_zones=ls0-req-hv6 +check ovn-nbctl lsp-add ls0 ls0-req-hv6 +check ovs-vsctl -- add-port br-int hv6-vif6 -- \ + set interface hv6-vif6 external-ids:iface-id=ls0-req-hv6 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +req_port6_zone=$(get_zone_num "$ct_zones" ls0-req-hv6) + +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv6], +[0], [ignore]) +check ovn-nbctl --wait=hv lsp-del ls0-req-hv6 +ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) +AT_CHECK([ovs-vsctl get bridge br-int external_ids:ct-zone-ls0-req-hv6], +[0], [ignore]) +req_port6_zone_new=$(get_zone_num "$ct_zones" ls0-req-hv6) +check_duplicates "$ct_zones" +check test "$req_port6_zone" -eq "$req_port6_zone_new" +check_duplicates "$ct_zones" # Now create a conflict in the OVSDB and restart ovn-controller. @@ -2659,7 +2757,6 @@ ovs-vsctl set bridge br-int external_ids:ct-zone-ls0-hv2="$snat_req_zone" ovn-appctl -t ovn-controller inc-engine/recompute ct_zones=$(ovn-appctl -t ovn-controller ct-zone-list) -echo "$ct_zones" port1_zone=$(get_zone_num "$ct_zones" ls0-hv1) port2_zone=$(get_zone_num "$ct_zones" ls0-hv2) -- 2.39.3 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
