Skip to content

Commit

Permalink
encaps: Support backward compatibility for tunnel chassis id change.
Browse files Browse the repository at this point in the history
In commit 41eefcb, the format of external_ids:ovn-chassis-id for
tunnels was modified to include the local encapsulation IP. This change
can lead to the recreation of tunnels during an upgrade, potentially
disrupting the dataplane temporarily, especially in large-scale
environments.

This patch resolves the issue by recognizing the previous format. Thus,
if the only modification is to the ID format, the tunnel will not be
recreated. Instead, the external_ids will be updated directly. This
approach ensures that the upgrade process is non-disruptive.

Fixes: 41eefcb ("encaps: Create separate tunnels for multiple local encap IPs.")
Signed-off-by: Han Zhou <hzhou@ovn.org>
Acked-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit f0f31fd)
  • Loading branch information
hzhou8 committed Mar 1, 2024
1 parent 24ea5b2 commit 4aa0755
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 17 deletions.
83 changes: 66 additions & 17 deletions controller/encaps.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,47 +104,73 @@ encaps_tunnel_id_create(const char *chassis_id, const char *remote_encap_ip,
'%', local_encap_ip);
}

/*
* The older version of encaps_tunnel_id_create, which doesn't include
* local_encap_ip in the ID. This is used for backward compatibility support.
*/
static char *
encaps_tunnel_id_create_legacy(const char *chassis_id,
const char *remote_encap_ip)
{
return xasprintf("%s%c%s", chassis_id, '@', remote_encap_ip);
}

/*
* Parses a 'tunnel_id' of the form <chassis_name>@<remote IP>%<local IP>.
* If the 'chassis_id' argument is not NULL the function will allocate memory
* and store the chassis_name part of the tunnel-id at '*chassis_id'.
* Same for remote_encap_ip and local_encap_ip.
*
* The old form <chassis_name>@<remote IP> is also supported for backward
* compatibility during upgrade.
*/
bool
encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
char **remote_encap_ip, char **local_encap_ip)
{
/* Find the @. Fail if there is no @ or if any part is empty. */
const char *d = strchr(tunnel_id, '@');
if (d == tunnel_id || !d || !d[1]) {
return false;
char *tokstr = xstrdup(tunnel_id);
char *saveptr = NULL;
bool ret = false;

char *token_chassis = strtok_r(tokstr, "@", &saveptr);
if (!token_chassis) {
goto out;
}

/* Find the %. Fail if there is no % or if any part is empty. */
const char *d2 = strchr(d + 1, '%');
if (d2 == d + 1 || !d2 || !d2[1]) {
return false;
char *token_remote_ip = strtok_r(NULL, "%", &saveptr);
if (!token_remote_ip) {
goto out;
}

char *token_local_ip = strtok_r(NULL, "", &saveptr);

if (chassis_id) {
*chassis_id = xmemdup0(tunnel_id, d - tunnel_id);
*chassis_id = xstrdup(token_chassis);
}

if (remote_encap_ip) {
*remote_encap_ip = xmemdup0(d + 1, d2 - (d + 1));
*remote_encap_ip = xstrdup(token_remote_ip);
}

if (local_encap_ip) {
*local_encap_ip = xstrdup(d2 + 1);
/* To support backward compatibility during upgrade, ignore local ip if
* it is not encoded in the tunnel id yet. */
*local_encap_ip = nullable_xstrdup(token_local_ip);
}
return true;

ret = true;
out:
free(tokstr);
return ret;
}

/*
* Returns true if 'tunnel_id' in the format
* <chassis_id>@<remote_encap_ip>%<local_encap_ip>
* contains 'chassis_id' and, if specified, the given 'remote_encap_ip' and
* 'local_encap_ip'. Returns false otherwise.
*
* The old format <chassis_id>@<remote_encap_ip> is also supported for backward
* compatibility during upgrade, and the local_encap_ip matching is ignored in
* that case.
*/
bool
encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
Expand All @@ -166,8 +192,10 @@ encaps_tunnel_id_match(const char *tunnel_id, const char *chassis_id,
}

char *token_local_ip = strtok_r(NULL, "", &saveptr);
if (local_encap_ip &&
(!token_local_ip || strcmp(token_local_ip, local_encap_ip))) {
if (!token_local_ip) {
/* It is old format. To support backward compatibility during upgrade,
* just ignore local_ip. */
} else if (local_encap_ip && strcmp(token_local_ip, local_encap_ip)) {
goto out;
}

Expand All @@ -189,6 +217,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
const char *dst_port = smap_get(&encap->options, "dst_port");
const char *csum = smap_get(&encap->options, "csum");
char *tunnel_entry_id = NULL;
char *tunnel_entry_id_old = NULL;

/*
* Since a chassis may have multiple encap-ip, we can't just add the
Expand All @@ -198,6 +227,8 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
*/
tunnel_entry_id = encaps_tunnel_id_create(new_chassis_id, encap->ip,
local_ip);
tunnel_entry_id_old = encaps_tunnel_id_create_legacy(new_chassis_id,
encap->ip);
if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) {
smap_add(&options, "csum", csum);
}
Expand Down Expand Up @@ -269,11 +300,28 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
* it). */
struct tunnel_node *tunnel = shash_find_data(&tc->tunnel,
tunnel_entry_id);
bool old_id_format = false;
if (!tunnel) {
tunnel = shash_find_data(&tc->tunnel, tunnel_entry_id_old);
old_id_format = true;
}
if (tunnel
&& tunnel->port->n_interfaces == 1
&& !strcmp(tunnel->port->interfaces[0]->type, encap->type)
&& smap_equal(&tunnel->port->interfaces[0]->options, &options)) {
shash_find_and_delete(&tc->tunnel, tunnel_entry_id);
if (old_id_format) {
/* We must be upgrading from an older version. We can reuse the
* existing tunnel, but needs to update the tunnel's ID to the new
* format. */
ovsrec_port_update_external_ids_setkey(tunnel->port, OVN_TUNNEL_ID,
tunnel_entry_id);
ovsrec_interface_update_external_ids_setkey(
tunnel->port->interfaces[0], OVN_TUNNEL_ID, tunnel_entry_id);

shash_find_and_delete(&tc->tunnel, tunnel_entry_id_old);
} else {
shash_find_and_delete(&tc->tunnel, tunnel_entry_id);
}
free(tunnel);
goto exit;
}
Expand Down Expand Up @@ -306,6 +354,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,

exit:
free(tunnel_entry_id);
free(tunnel_entry_id_old);
smap_destroy(&options);
}

Expand Down
44 changes: 44 additions & 0 deletions tests/ovn-controller.at
Original file line number Diff line number Diff line change
Expand Up @@ -2790,3 +2790,47 @@ OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status], [0], [conn

OVN_CLEANUP([hv1])
AT_CLEANUP

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-controller - tunnel chassis id backward compatibility])

ovn_start

net_add n1
sim_add hv1
as hv1
ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.1

check ovn-nbctl --wait=sb sync
wait_row_count Chassis 1 name=hv1

ovn-sbctl chassis-add fakechassis geneve 192.168.0.2
fakech_tunnel=ovn-fakech-0
OVS_WAIT_UNTIL([ovs-vsctl list port $fakech_tunnel])
port_uuid=$(ovs-vsctl get port $fakech_tunnel _uuid)

AT_CHECK([ovs-vsctl get port $fakech_tunnel external_ids:ovn-chassis-id], [0], [dnl
"fakechassis@192.168.0.2%192.168.0.1"
])

# Stop ovn-controller without deleting tunnel
check ovn-appctl -t ovn-controller exit --restart

# Change the tunnel external id to the old format, and then start
# ovn-controller, pretending we are upgrading from an older version.
ovs-vsctl set port $fakech_tunnel external_ids:ovn-chassis-id=fakechassis@192.168.0.2
AT_CHECK([ovs-vsctl get port $fakech_tunnel external_ids:ovn-chassis-id], [0], [dnl
"fakechassis@192.168.0.2"
])

start_daemon ovn-controller

# The tunnel id should be updated to the new format but the tunnel's uuid
# should kept the same (no recreation).
OVS_WAIT_UNTIL([test x$(ovs-vsctl get port $fakech_tunnel external_ids:ovn-chassis-id) = x\"fakechassis@192.168.0.2%192.168.0.1\"])
AT_CHECK([test x"$port_uuid"=$(ovs-vsctl get port $fakech_tunnel _uuid)])

OVN_CLEANUP([hv1])
AT_CLEANUP
])

0 comments on commit 4aa0755

Please sign in to comment.