Skip to content
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

Allow zone-reload/reload to fail due to module configuration errors #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow zone-reload/reload to fail due to module configuration errors #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f4ce894
feat: allow reload fail caused by module
Dec 12, 2024
cf58751
fix: zone hang if last zone-reload oom
Dec 12, 2024
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
20 changes: 11 additions & 9 deletions src/knot/conf/module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ void conf_mod_unload_shared(
else \
log_##level(LOG_ARGS(mod_id, msg), ##__VA_ARGS__);

void conf_activate_modules(
int conf_activate_modules(
conf_t *conf,
struct server *server,
const knot_dname_t *zone_name,
Expand All @@ -368,13 +368,14 @@ void conf_activate_modules(
} else {
val = conf_default_get(conf, C_GLOBAL_MODULE);
}

switch (val.code) {
case KNOT_EOK:
break;
case KNOT_ENOENT: // Check if a module is configured at all.
return KNOT_EOK;
case KNOT_YP_EINVAL_ID:
return;
return KNOT_EZONEINVAL;
default:
ret = val.code;
goto activate_error;
Expand Down Expand Up @@ -404,22 +405,22 @@ void conf_activate_modules(
if (mod == NULL) {
MOD_ID_LOG(zone_name, error, mod_id, "failed to open");
conf_free_mod_id(mod_id);
goto skip_module;
goto activate_error;
}

// Check the module scope.
if ((zone_name == NULL && !(mod->api->flags & KNOTD_MOD_FLAG_SCOPE_GLOBAL)) ||
(zone_name != NULL && !(mod->api->flags & KNOTD_MOD_FLAG_SCOPE_ZONE))) {
MOD_ID_LOG(zone_name, error, mod_id, "out of scope");
query_module_close(mod);
goto skip_module;
goto activate_error;
}

// Check if the module is loadable.
if (mod->api->load == NULL) {
MOD_ID_LOG(zone_name, debug, mod_id, "empty module, not loaded");
query_module_close(mod);
goto skip_module;
goto activate_error;
}

// Load the module.
Expand All @@ -428,18 +429,19 @@ void conf_activate_modules(
MOD_ID_LOG(zone_name, error, mod_id, "failed to load (%s)",
knot_strerror(ret));
query_module_close(mod);
goto skip_module;
goto activate_error;
}
mod->config = NULL; // Invalidate the current config.

add_tail(query_modules, &mod->node);
skip_module:
conf_val_next(&val);
}

return;
return KNOT_EOK;
activate_error:
CONF_LOG(LOG_ERR, "failed to activate modules (%s)", knot_strerror(ret));
conf_deactivate_modules(query_modules, query_plan);
return KNOT_EZONEINVAL;
}

void conf_deactivate_modules(
Expand Down
4 changes: 3 additions & 1 deletion src/knot/conf/module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,10 @@ void conf_mod_unload_shared(
* \param[in] zone_name Zone name, NULL for all zones.
* \param[in] query_modules Destination query modules list.
* \param[in] query_plan Destination query plan.
*
* \return Error code, KNOT_EOK if success.
*/
void conf_activate_modules(
int conf_activate_modules(
conf_t *conf,
struct server *server,
const knot_dname_t *zone_name,
Expand Down
7 changes: 6 additions & 1 deletion src/knot/server/server.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1324,8 +1324,13 @@ int server_reload(server_t *server, reload_t mode)
return ret;
}

conf_activate_modules(new_conf, server, NULL, new_conf->query_modules,
ret = conf_activate_modules(new_conf, server, NULL, new_conf->query_modules,
&new_conf->query_plan);
if (ret!= KNOT_EOK) {
conf_free(new_conf);
systemd_ready_notify();
return ret;
}
}

conf_update_flag_t upd_flags = CONF_UPD_FNOFREE;
Expand Down
73 changes: 64 additions & 9 deletions src/knot/zone/zonedb-load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,15 @@ static zone_t *reuse_member_zone(zone_t *zone, server_t *server, conf_t *conf,
log_zone_error(zone->name, "zone cannot be created");
} else {
assert(zone_get_flag(newzone, ZONE_IS_CAT_MEMBER, false));
conf_activate_modules(conf, server, newzone->name, &newzone->query_modules,
int ret = conf_activate_modules(conf, server, newzone->name, &newzone->query_modules,
&newzone->query_plan);
if (ret != KNOT_EOK) {
log_zone_error(newzone->name, "zone cannot be activated(%s)", knot_strerror(ret));
newzone->contents = NULL;
zone_free(&newzone);
return NULL; // remove the member zone
}

}

return newzone;
Expand All @@ -336,8 +343,14 @@ static zone_t *reuse_cold_zone(const knot_dname_t *zname, server_t *server, conf
log_zone_error(zname, "zone cannot be created");
} else {
zone_set_flag(zone, ZONE_IS_CAT_MEMBER);
conf_activate_modules(conf, server, zone->name, &zone->query_modules,
int ret = conf_activate_modules(conf, server, zone->name, &zone->query_modules,
&zone->query_plan);
if (ret != KNOT_EOK) {
log_zone_error(zone->name, "zone cannot be activated(%s)", knot_strerror(ret));
zone->contents = NULL;
zone_free(&zone);
return NULL; // remove the member zone
}
}
return zone;
}
Expand Down Expand Up @@ -384,8 +397,14 @@ static zone_t *add_member_zone(catalog_upd_val_t *val, knot_zonedb_t *check,
log_zone_error(val->member, "zone cannot be created");
} else {
zone_set_flag(zone, ZONE_IS_CAT_MEMBER);
conf_activate_modules(conf, server, zone->name, &zone->query_modules,
int ret = conf_activate_modules(conf, server, zone->name, &zone->query_modules,
&zone->query_plan);
if (ret != KNOT_EOK) {
log_zone_error(zone->name, "zone cannot be activated(%s)", knot_strerror(ret));
zone->contents = NULL;
zone_free(&zone);
return NULL; // remove the member zone
}
log_zone_info(val->member, "zone added from catalog");
}
return zone;
Expand Down Expand Up @@ -420,7 +439,10 @@ static knot_zonedb_t *create_zonedb(conf_t *conf, server_t *server, reload_t mod
if (mode == RELOAD_COMMIT) {
mark_changed_zones(db_old, conf->io.zones);
}

list_t new_zone_set, old_zone_set;
init_list(&new_zone_set);
init_list(&old_zone_set);
int ret = KNOT_EOK;
/* Process regular zones from the configuration. */
for (conf_iter_t iter = conf_iter(conf, C_ZONE); iter.code == KNOT_EOK;
conf_iter_next(conf, &iter)) {
Expand All @@ -431,7 +453,7 @@ static knot_zonedb_t *create_zonedb(conf_t *conf, server_t *server, reload_t mod
if (old_zone != NULL && (mode & (RELOAD_COMMIT | RELOAD_CATALOG))) {
/* Reuse unchanged zone. */
if (!(old_zone->change_type & CONF_IO_TRELOAD)) {
knot_zonedb_insert(db_new, old_zone);
ptrlist_add(&old_zone_set, old_zone, NULL);
continue;
}
}
Expand All @@ -442,10 +464,34 @@ static knot_zonedb_t *create_zonedb(conf_t *conf, server_t *server, reload_t mod
continue;
}

conf_activate_modules(conf, server, zone->name, &zone->query_modules,
ret = conf_activate_modules(conf, server, zone->name, &zone->query_modules,
&zone->query_plan);
if (ret != KNOT_EOK) {
log_zone_error(zone->name, "zone cannot be activated(%s)", knot_strerror(ret));
zone->contents = NULL;
zone_free(&zone);
break;
}
ptrlist_add(&new_zone_set, zone, NULL);
}

ptrnode_t *n, *nxt;
if (ret!= KNOT_EOK) {
WALK_LIST_DELSAFE(n, nxt, new_zone_set) {
zone_free((zone_t**)&n->d);
ptrlist_rem(n, NULL);
}
knot_zonedb_deep_free(&db_new, false);
return NULL;
}

knot_zonedb_insert(db_new, zone);
WALK_LIST_DELSAFE(n, nxt, new_zone_set) {
knot_zonedb_insert(db_new, (zone_t*)n->d);
ptrlist_rem(n, NULL);
}
WALK_LIST_DELSAFE(n, nxt, old_zone_set) {
knot_zonedb_insert(db_new, (zone_t*)n->d);
ptrlist_rem(n, NULL);
}

/* Purge decataloged zones before catalog removals are commited. */
Expand All @@ -463,7 +509,7 @@ static knot_zonedb_t *create_zonedb(conf_t *conf, server_t *server, reload_t mod
}
catalog_it_free(cat_it);

int ret = catalog_update_commit(&server->catalog_upd, &server->catalog);
ret = catalog_update_commit(&server->catalog_upd, &server->catalog);
if (ret != KNOT_EOK) {
log_error("catalog, failed to apply changes (%s)", knot_strerror(ret));
return db_new;
Expand Down Expand Up @@ -652,11 +698,20 @@ int zone_reload_modules(conf_t *conf, server_t *server, const knot_dname_t *zone

zone_t *newzone = create_zone(conf, zone_name, server, *zone);
if (newzone == NULL) {
knot_sem_post(&(*zone)->cow_lock);
return KNOT_ENOMEM;
}
knot_sem_wait(&newzone->cow_lock);
conf_activate_modules(conf, server, newzone->name, &newzone->query_modules,
int ret = conf_activate_modules(conf, server, newzone->name, &newzone->query_modules,
&newzone->query_plan);
if (ret != KNOT_EOK) {
log_zone_error(newzone->name, "zone cannot be activated(%s)", knot_strerror(ret));
knot_sem_post(&newzone->cow_lock);
knot_sem_post(&(*zone)->cow_lock);
newzone->contents = NULL;
zone_free(&newzone);
return ret;
}

zone_t *oldzone = rcu_xchg_pointer(zone, newzone);
synchronize_rcu();
Expand Down
12 changes: 11 additions & 1 deletion src/utils/knotd/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,8 +768,18 @@ int main(int argc, char **argv)
drop_capabilities();

/* Activate global query modules. */
conf_activate_modules(conf(), &server, NULL, conf()->query_modules,
ret = conf_activate_modules(conf(), &server, NULL, conf()->query_modules,
&conf()->query_plan);
if (ret != KNOT_EOK) {
log_fatal("failed to activate query modules");
server_wait(&server);
server_deinit(&server);
conf_free(conf());
dbus_close();
log_close();
dnssec_crypto_cleanup();
return EXIT_FAILURE;
}

/* Check and create PID file. */
unsigned long pid = pid_check_and_create();
Expand Down