Hi Lucas, thanks for the patch.
I think searching by chassis name makes way more sense than searching by
the default name generated by ovn-nbctl.
In fact, I would go a step further than this patch and just remove the
search by "{router_port_name}-{chassis_name}" entirely. Searching by
chassis name on the logical router's existing gateway chassis is more
accurate, and potentially more efficient (if the Gateway_Chassis table
has a lot of rows).
On 7/22/25 9:44 AM, Lucas Vargas Dias via dev wrote:
Configuration of Gateway Chassis with different name than the default
format of "$lrp-$chassis" could lead to duplicates when priority was
changed for the GW Chassis with different name e.g.:
lrp0_chassis1 1 --> inserted by cms
lrp0-chassis1 2 --> change priority with ovn-nbctl lrp-set-gateway-chassis
ovn-controller would then create a log message every time it tried
recalculate priorities.
Search the GW Chassis linked to specified LRP by chassis_name first
before resorting to uuid or name search.
Signed-off-by: Lucas Vargas Dias <lucas.vd...@luizalabs.com>
---
tests/ovn-nbctl.at | 16 +++++++++++++++-
utilities/ovn-nbctl.c | 18 +++++++++++++++---
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index bdeb3aeee..fe4a984d0 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1958,7 +1958,21 @@ AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0],
[dnl
lrp0-chassis2 10
lrp0-chassis3 5
lrp0-chassis1 1
-])])
+])
+gc_uuid=$(ovn-nbctl --bare --colum _uuid find gateway_chassis
name='lrp0-chassis1')
+AT_CHECK([ovn-nbctl set gateway_chassis $gc_uuid name='lrp0_chassis1'])
+AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
+lrp0-chassis2 10
+lrp0-chassis3 5
+lrp0_chassis1 1
+])
+AT_CHECK([ovn-nbctl lrp-set-gateway-chassis lrp0 chassis1 2])
+AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
+lrp0-chassis2 10
+lrp0-chassis3 5
+lrp0_chassis1 2
+])
+])
dnl ---------------------------------------------------------------------
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 56a513217..34caf599b 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -5829,12 +5829,22 @@ lr_get_name(const struct nbrec_logical_router *lr, char
uuid_s[UUID_LEN + 1],
}
static char * OVS_WARN_UNUSED_RESULT
-gc_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
- const struct nbrec_gateway_chassis **gc_p)
+gc_by_chassis_name_or_name_or_uuid(struct ctl_context *ctx, const char *id,
+ const char *chassis_name, bool must_exist,
+ const struct nbrec_gateway_chassis **gc_p,
+ const struct nbrec_logical_router_port *lrp
+ )
{
const struct nbrec_gateway_chassis *gc = NULL;
*gc_p = NULL;
+ for (size_t i = 0; i < lrp->n_gateway_chassis; i++) {
+ if (!strcmp(lrp->gateway_chassis[i]->chassis_name, chassis_name)) {
+ *gc_p = lrp->gateway_chassis[i];
+ return NULL;
+ }
+ }
+
struct uuid gc_uuid;
bool is_uuid = uuid_from_string(&gc_uuid, id);
if (is_uuid) {
@@ -5867,6 +5877,7 @@ nbctl_pre_lrp_set_gateway_chassis(struct ctl_context *ctx)
ovsdb_idl_add_column(ctx->idl, &nbrec_gateway_chassis_col_name);
ovsdb_idl_add_column(ctx->idl, &nbrec_gateway_chassis_col_priority);
+ ovsdb_idl_add_column(ctx->idl, &nbrec_gateway_chassis_col_chassis_name);
}
static void
@@ -5897,7 +5908,8 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
const struct nbrec_gateway_chassis *gc;
- error = gc_by_name_or_uuid(ctx, gc_name, false, &gc);
+ error = gc_by_chassis_name_or_name_or_uuid(ctx, gc_name, chassis_name,
+ false, &gc, lrp);
if (error) {
ctx->error = error;
free(gc_name);
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
