Skip to content

avoid unnecessarily disconnecting ERs #862

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ekoby merged 1 commit into from
May 5, 2025
Merged

avoid unnecessarily disconnecting ERs #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion inc_internal/internal_model.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ XX(tls, model_string, none, tls, __VA_ARGS__)

#define ZITI_EDGE_ROUTER_MODEL(XX, ...)\
XX(name, model_string, none, name, __VA_ARGS__)\
XX(hostname, model_string, none, hostname, __VA_ARGS__) \
XX(protocols, ziti_er_protocols, none, supportedProtocols, __VA_ARGS__)

#define ZITI_SERVICE_EDGE_ROUTERS_MODEL(XX, ...) \
Expand Down
4 changes: 3 additions & 1 deletion inc_internal/zt_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,15 @@ bool ziti_channel_is_connected(ziti_channel_t *ch);

uint64_t ziti_channel_latency(ziti_channel_t *ch);

void ziti_channel_set_url(ziti_channel_t *ch, const char *url);

int ziti_channel_force_connect(ziti_channel_t *ch);

int ziti_channel_update_token(ziti_channel_t *ch, const char *token);

int ziti_channel_update_posture(ziti_channel_t *ch, const uint8_t *data, size_t len);

int ziti_channel_connect(ziti_context ztx, const char *name, const char *url);
int ziti_channel_connect(ziti_context ztx, const ziti_edge_router *er);

int ziti_channel_prepare(ziti_channel_t *ch);

Expand Down
54 changes: 37 additions & 17 deletions library/channel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#define POOLED_MESSAGE_SIZE (32 * 1024)
#define INBOUND_POOL_SIZE (32)

#define CH_LOG(lvl, fmt, ...) ZITI_LOG(lvl, "ch[%d] " fmt, ch->id, ##__VA_ARGS__)

Check warning on line 42 in library/channel.c

View workflow job for this annotation

GitHub Actions / Linux ARM 

format '%ld' expects argument of type 'long int', but argument 10 has type 'size_t' ***aka 'unsigned int'*** [-Wformat=]

enum ChannelState {
Initial,
Expand Down Expand Up @@ -190,17 +190,17 @@
}

int ziti_close_channels(struct ziti_ctx *ztx, int err) {
const char *url;
const char *er_id;
model_list ch_ids = {0};
MODEL_MAP_FOR(it, ztx->channels) {
model_list_append(&ch_ids, model_map_it_key(it));
}

MODEL_LIST_FOR(it, ch_ids) {
url = model_list_it_element(it);
ziti_channel_t *ch = model_map_get(&ztx->channels, url);
er_id = model_list_it_element(it);
ziti_channel_t *ch = model_map_get(&ztx->channels, er_id);
if (ch != NULL) {
ZTX_LOG(DEBUG, "closing channel[%s]: %s", url, ziti_errorstr(err));
ZTX_LOG(DEBUG, "closing channel[%s]: %s", er_id, ziti_errorstr(err));
ziti_channel_close(ch, err);
}
}
Expand Down Expand Up @@ -257,21 +257,15 @@
return ch->latency;
}

static ziti_channel_t *new_ziti_channel(ziti_context ztx, const char *ch_name, const char *url) {
static ziti_channel_t *new_ziti_channel(ziti_context ztx, const ziti_edge_router *er) {
ziti_channel_t *ch = calloc(1, sizeof(ziti_channel_t));
ziti_channel_init(ztx, ch, channel_counter++);
const ziti_identity *identity = ziti_get_identity(ztx);
ch->name = strdup(ch_name);
ch->url = strdup(url);
ch->name = strdup(er->name);
CH_LOG(INFO, "(%s) new channel for ztx[%d] identity[%s]", ch->name, ztx->id, identity->name);

struct tlsuv_url_s ingress;
tlsuv_parse_url(&ingress, url);

ch->host = calloc(1, ingress.hostname_len + 1);
snprintf(ch->host, ingress.hostname_len + 1, "%.*s", (int) ingress.hostname_len, ingress.hostname);
ch->port = ingress.port;
model_map_set(&ztx->channels, url, ch);
ziti_channel_set_url(ch, er->protocols.tls);
model_map_set(&ztx->channels, er->name, ch);
return ch;
}

Expand Down Expand Up @@ -312,6 +306,26 @@
}
}

void ziti_channel_set_url(ziti_channel_t *ch, const char *url) {
assert(ch != NULL);
assert(url != NULL);

if (ch->url && strcmp(ch->url, url) == 0) {
return;
}
CH_LOG(DEBUG, "setting channel[%s] url[%s]", ch->name, url);

FREE(ch->url);
FREE(ch->host);
ch->url = strdup(url);

struct tlsuv_url_s ingress;
tlsuv_parse_url(&ingress, ch->url);
ch->host = calloc(1, ingress.hostname_len + 1);
snprintf(ch->host, ingress.hostname_len + 1, "%.*s", (int) ingress.hostname_len, ingress.hostname);
ch->port = ingress.port;
}

int ziti_channel_update_token(ziti_channel_t *ch, const char *token) {
if (ch == NULL) {
return ZITI_INVALID_STATE;
Expand Down Expand Up @@ -348,14 +362,20 @@
return ZITI_OK;
}

int ziti_channel_connect(ziti_context ztx, const char *ch_name, const char *url) {
ziti_channel_t *ch = model_map_get(&ztx->channels, url);
int ziti_channel_connect(ziti_context ztx, const ziti_edge_router* er) {
const char *url = er->protocols.tls;
if (url == NULL) {
ZTX_LOG(ERROR, "er[%s] does not have TLS edge listener", er->name);
return ZITI_INVALID_CONFIG;
}

ziti_channel_t *ch = model_map_get(&ztx->channels, er->name);

if (ch != NULL) {
ZTX_LOG(DEBUG, "existing ch[%d](%s) found for ingress[%s]", ch->id, ch_state_str(ch), url);
}
else {
ch = new_ziti_channel(ztx, ch_name, url);
ch = new_ziti_channel(ztx, er);
ch->notify_cb(ch, EdgeRouterAdded, ch->notify_ctx);
}

Expand Down Expand Up @@ -743,7 +763,7 @@
},
};
ch->latency = uv_now(ch->loop);
ziti_channel_send_for_reply(ch, ContentTypeHelloType, headers, 2, ch->token, strlen(ch->token), hello_reply_cb, ch);

Check warning on line 766 in library/channel.c

View workflow job for this annotation

GitHub Actions / MacOS arm64 

passing 'char[37]' to parameter of type 'const uint8_t *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]

Check warning on line 766 in library/channel.c

View workflow job for this annotation

GitHub Actions / MacOS x86_64 

passing 'char[37]' to parameter of type 'const uint8_t *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
}


Expand Down Expand Up @@ -919,7 +939,7 @@

CH_LOG(TRACE, "on_data [len=%zd]", len);
ch->last_read = uv_now(ch->loop);
buffer_append(ch->incoming, buf->base, (uint32_t) len);

Check warning on line 942 in library/channel.c

View workflow job for this annotation

GitHub Actions / MacOS arm64 

passing 'char *const' to parameter of type 'uint8_t *' (aka 'unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]

Check warning on line 942 in library/channel.c

View workflow job for this annotation

GitHub Actions / MacOS x86_64 

passing 'char *const' to parameter of type 'uint8_t *' (aka 'unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
process_inbound(ch);
}

Expand Down
24 changes: 10 additions & 14 deletions library/connect.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@
if (r->cb != NULL) {
r->cb(conn, status ? status : (ssize_t) r->len, r->ctx);
}
r = model_list_it_element(it);

Check warning on line 255 in library/connect.c

View workflow job for this annotation

GitHub Actions / Linux ARM64 

assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]

Check warning on line 255 in library/connect.c

View workflow job for this annotation

GitHub Actions / Linux x86_64 

assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
it = model_list_it_next(it);
} while(r);
model_list_clear(&req->chain, free);
Expand Down Expand Up @@ -388,23 +388,19 @@


MODEL_LIST_FOREACH(er, session->edge_routers) {
const char *tls = er->protocols.tls;

if (tls) {
ch = model_map_get(&ztx->channels, tls);
if (ch == NULL) continue;

if (ch->state == Connected) {
uint64_t latency = ziti_channel_latency(ch);
if (latency < best_latency) {
best_ch = ch;
best_latency = latency;
}
ch = model_map_get(&ztx->channels, er->name);
if (ch == NULL) continue;

if (ch->state == Connected) {
uint64_t latency = ziti_channel_latency(ch);
if (latency < best_latency) {
best_ch = ch;
best_latency = latency;
}
}

if (ch->state == Disconnected) {
if (ch->state == Disconnected) {
model_list_append(&disconnected, ch);
}
}
}

Expand Down Expand Up @@ -667,7 +663,7 @@
count++;
tot += r->len;

r = model_list_it_element(it);

Check warning on line 666 in library/connect.c

View workflow job for this annotation

GitHub Actions / Linux ARM64 

assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]

Check warning on line 666 in library/connect.c

View workflow job for this annotation

GitHub Actions / Linux x86_64 

assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
it = model_list_it_next(it);
} while(r != NULL);
CONN_LOG(DEBUG, "consolidated %d payloads total_len[%zd]", count, tot);
Expand Down
49 changes: 24 additions & 25 deletions library/ziti.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@
ziti_ctrl_set_token(ztx_get_controller(ztx), session_token);
ziti_ctrl_list_controllers(ctrl, ctrl_list_cb, ztx);

const char* url;
const char* er_name;
ziti_channel_t *ch;
MODEL_MAP_FOREACH(url, ch, &ztx->channels) {
MODEL_MAP_FOREACH(er_name, ch, &ztx->channels) {
ziti_channel_update_token(ch, session_token);
}
}
Expand Down Expand Up @@ -907,11 +907,11 @@

printer(ctx, "\n==================\nChannels:\n");
ziti_channel_t *ch;
const char *url;
MODEL_MAP_FOREACH(url, ch, &ztx->channels) {
const char *er_id;
MODEL_MAP_FOREACH(er_id, ch, &ztx->channels) {
printer(ctx, "ch[%d] %s\n", ch->id, ch->name);
printer(ctx, "\tconnected[%c] version[%s] address[%s]",
ziti_channel_is_connected(ch) ? 'Y' : 'N', ch->version, url);
ziti_channel_is_connected(ch) ? 'Y' : 'N', ch->version, ch->url);
if (ziti_channel_is_connected(ch)) {
printer(ctx, " latency[%" PRIu64 "]\n", ziti_channel_latency(ch));
} else {
Expand Down Expand Up @@ -1450,26 +1450,28 @@
}

model_map curr_routers = {0};
const char *er_url;
const char *er_name;
ziti_channel_t *ch;
MODEL_MAP_FOREACH(er_url, ch, &ztx->channels) {
model_map_set(&curr_routers, er_url, (void *) er_url);
MODEL_MAP_FOREACH(er_name, ch, &ztx->channels) {
model_map_set(&curr_routers, er_name, (void *) er_name);
}

ziti_edge_router **erp = ers;
while (*erp) {
ziti_edge_router *er = *erp;
const char *tls = er->protocols.tls;

if (tls) {
// check if it is already in the list
if (model_map_remove(&curr_routers, tls) == NULL) {
ZTX_LOG(TRACE, "connecting to %s(%s)", er->name, tls);
ziti_channel_connect(ztx, er->name, tls);
// check if it is already in the list
if (model_map_remove(&curr_routers, er->name) == NULL) {
if (ziti_channel_connect(ztx, er) == ZITI_OK) {
ers_changed = true;
ZTX_LOG(TRACE, "connecting to %s(%s)", er->name, er->protocols.tls);
}
} else {
ZTX_LOG(DEBUG, "edge router %s does not have TLS edge listener", er->name);
} else if(er->protocols.tls != NULL) {
// N.B.: if protocols.tls is NULL,
// controller may not have refreshed the ER model leave the channel as is
// otherwise update the url
ch = model_map_get(&ztx->channels, er->name);
ziti_channel_set_url(ch, er->protocols.tls);
}

free_ziti_edge_router(er);
Expand All @@ -1480,9 +1482,9 @@

model_map_iter it = model_map_iterator(&curr_routers);
while (it != NULL) {
er_url = model_map_it_key(it);
ch = model_map_remove(&ztx->channels, er_url);
ZTX_LOG(INFO, "removing channel[%s@%s]: no longer available", ch->name, er_url);
er_name = model_map_it_key(it);
ch = model_map_remove(&ztx->channels, er_name);
ZTX_LOG(INFO, "removing channel[%s@%s]: no longer available", ch->name, ch->url);
ziti_channel_close(ch, ZITI_GATEWAY_UNAVAILABLE);
it = model_map_it_remove(it);
ers_changed = true;
Expand Down Expand Up @@ -1734,7 +1736,7 @@
ziti_send_event(ztx, &ev);

if (status == EdgeRouterRemoved) {
model_map_remove(&ztx->channels, ch->url);
model_map_remove(&ztx->channels, ch->name);
if (ztx->closing) {
shutdown_and_free(ztx);
}
Expand All @@ -1761,7 +1763,7 @@
id_it = model_list_it_remove(id_it);
}

MODEL_MAP_FOREACH(conn_id, conn, &ztx->connections) {

Check warning on line 1766 in library/ziti.c

View workflow job for this annotation

GitHub Actions / MacOS arm64 

comparison between pointer and integer ('uint32_t' (aka 'unsigned int') and 'void *') [-Wpointer-integer-compare]

Check warning on line 1766 in library/ziti.c

View workflow job for this annotation

GitHub Actions / MacOS x86_64 

comparison between pointer and integer ('uint32_t' (aka 'unsigned int') and 'void *') [-Wpointer-integer-compare]
if (conn->type == Server) {
update_bindings(conn);
}
Expand Down Expand Up @@ -2034,12 +2036,9 @@
assert(ztx);
assert(er);

model_string url = er->protocols.tls;
if (url == NULL) return NULL;

ziti_channel_t *ch = (ziti_channel_t *) model_map_get(&ztx->channels, url);
ziti_channel_t *ch = (ziti_channel_t *) model_map_get(&ztx->channels, er->name);
if (ch == NULL) {
ziti_channel_connect(ztx, er->name, url);
ziti_channel_connect(ztx, er);
}
return ch;
}
Expand Down
Loading