matthieu/frr
1
0
Fork 0
forked from Mirror/frr
Code Pull requests Activity

lib: introduce a read-write lock for northbound configurations

Browse source 
The upcoming gRPC-based northbound plugin will run on a separate
pthread, and it will need to have access to the running configuration
global variable.  Introduce a rw-lock to control concurrent access
to the running configuration. Add the lock inside the "nb_config"
structure so that it can be used to protect candidate configurations
as well (this might be necessary depending on the threading scheme
of future northbound plugins).

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This commit is contained in:
Renato Westphal 2019-04-03 16:31:18 -03:00
parent 364ad673c8
commit 83981138fe
10 changed files with 305 additions and 183 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
isisd/isis_cli.c
View file

@ -188,10 +188,14 @@ DEFPY(ip_router_isis, ip_router_isis_cmd, "ip router isis WORD$tag",
}
/* check if the interface is a loopback and if so set it as passive */
pthread_rwlock_rdlock(&running_config->lock);
{
ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
if (ifp && if_is_loopback(ifp))
nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
NB_OP_MODIFY, "true");
}
pthread_rwlock_unlock(&running_config->lock);
return nb_cli_apply_changes(vty, NULL);
}
@ -258,10 +262,14 @@ DEFPY(ip6_router_isis, ip6_router_isis_cmd, "ipv6 router isis WORD$tag",
}
/* check if the interface is a loopback and if so set it as passive */
pthread_rwlock_rdlock(&running_config->lock);
{
ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
if (ifp && if_is_loopback(ifp))
nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive",
NB_OP_MODIFY, "true");
}
pthread_rwlock_unlock(&running_config->lock);
return nb_cli_apply_changes(vty, NULL);
}
@ -368,7 +376,10 @@ DEFPY(no_is_type, no_is_type_cmd,
"Act as both a station router and an area router\n"
"Act as an area router only\n")
{
const char *value = NULL;
const char *value;
pthread_rwlock_rdlock(&running_config->lock);
{
struct isis_area *area;
area = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
@ -382,6 +393,9 @@ DEFPY(no_is_type, no_is_type_cmd,
value = "level-1-2";
else
value = NULL;
}
pthread_rwlock_unlock(&running_config->lock);
nb_cli_enqueue_change(vty, "./is-type", NB_OP_MODIFY, value);
return nb_cli_apply_changes(vty, NULL);
@ -1769,30 +1783,27 @@ DEFPY(no_isis_circuit_type, no_isis_circuit_type_cmd,
"Level-1-2 adjacencies are formed\n"
"Level-2 only adjacencies are formed\n")
{
struct interface *ifp;
struct isis_circuit *circuit;
int is_type;
const char *circ_type;
const char *circ_type = NULL;
/*
* Default value depends on whether the circuit is part of an area,
* and the is-type of the area if there is one. So we need to do this
* here.
*/
pthread_rwlock_rdlock(&running_config->lock);
{
struct interface *ifp;
struct isis_circuit *circuit;
ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false);
if (!ifp)
goto def_val;
goto unlock;
circuit = circuit_scan_by_ifp(ifp);
if (!circuit)
goto def_val;
if (!circuit || circuit->state != C_STATE_UP)
goto unlock;
if (circuit->state == C_STATE_UP)
is_type = circuit->area->is_type;
else
goto def_val;
switch (is_type) {
switch (circuit->area->is_type) {
case IS_LEVEL_1:
circ_type = "level-1";
break;
@ -1802,19 +1813,15 @@ DEFPY(no_isis_circuit_type, no_isis_circuit_type_cmd,
case IS_LEVEL_1_AND_2:
circ_type = "level-1-2";
break;
default:
return CMD_ERR_NO_MATCH;
}
}
unlock:
pthread_rwlock_unlock(&running_config->lock);
nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
NB_OP_MODIFY, circ_type);
return nb_cli_apply_changes(vty, NULL);
def_val:
nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type",
NB_OP_MODIFY, NULL);
return nb_cli_apply_changes(vty, NULL);
}
void cli_show_ip_isis_circ_type(struct vty *vty, struct lyd_node *dnode,

4
lib/command.c
View file

@ -1705,12 +1705,16 @@ static int vty_write_config(struct vty *vty)
vty_out(vty, "frr defaults %s\n", DFLT_NAME);
vty_out(vty, "!\n");
pthread_rwlock_rdlock(&running_config->lock);
{
for (i = 0; i < vector_active(cmdvec); i++)
if ((node = vector_slot(cmdvec, i)) && node->func
&& (node->vtysh || vty->type != VTY_SHELL)) {
if ((*node->func)(vty))
vty_out(vty, "!\n");
}
}
pthread_rwlock_unlock(&running_config->lock);
if (vty->type == VTY_TERM) {
vty_out(vty, "end\n");

5
lib/if.c
View file

@ -187,6 +187,8 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
if (yang_module_find("frr-interface")) {
struct lyd_node *if_dnode;
pthread_rwlock_wrlock(&running_config->lock);
{
if_dnode = yang_dnode_get(
running_config->dnode,
"/frr-interface:lib/interface[name='%s'][vrf='%s']/vrf",
@ -196,8 +198,9 @@ void if_update_to_new_vrf(struct interface *ifp, vrf_id_t vrf_id)
running_config->version++;
}
}
pthread_rwlock_unlock(&running_config->lock);
}
}
/* Delete interface structure. */
void if_delete_retain(struct interface *ifp)

7
lib/libfrr.c
View file

@ -830,7 +830,12 @@ static int frr_config_read_in(struct thread *t)
/*
* Update the shared candidate after reading the startup configuration.
*/
nb_config_replace(vty_shared_candidate_config, running_config, true);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_replace(vty_shared_candidate_config, running_config,
true);
}
pthread_rwlock_unlock(&running_config->lock);
return 0;
}

74
lib/northbound.c
View file

@ -264,6 +264,7 @@ struct nb_config *nb_config_new(struct lyd_node *dnode)
else
config->dnode = yang_dnode_new(ly_native_ctx, true);
config->version = 0;
pthread_rwlock_init(&config->lock, NULL);
return config;
}
@ -272,6 +273,7 @@ void nb_config_free(struct nb_config *config)
{
if (config->dnode)
yang_dnode_free(config->dnode);
pthread_rwlock_destroy(&config->lock);
XFREE(MTYPE_NB_CONFIG, config);
}
@ -282,6 +284,7 @@ struct nb_config *nb_config_dup(const struct nb_config *config)
dup = XCALLOC(MTYPE_NB_CONFIG, sizeof(*dup));
dup->dnode = yang_dnode_dup(config->dnode);
dup->version = config->version;
pthread_rwlock_init(&dup->lock, NULL);
return dup;
}
@ -529,17 +532,28 @@ int nb_candidate_edit(struct nb_config *candidate,
bool nb_candidate_needs_update(const struct nb_config *candidate)
{
if (candidate->version < running_config->version)
return true;
bool ret = false;
return false;
pthread_rwlock_rdlock(&running_config->lock);
{
if (candidate->version < running_config->version)
ret = true;
}
pthread_rwlock_unlock(&running_config->lock);
return ret;
}
int nb_candidate_update(struct nb_config *candidate)
{
struct nb_config *updated_config;
pthread_rwlock_rdlock(&running_config->lock);
{
updated_config = nb_config_dup(running_config);
}
pthread_rwlock_unlock(&running_config->lock);
if (nb_config_merge(updated_config, candidate, true) != NB_OK)
return NB_ERR;
@ -591,9 +605,13 @@ int nb_candidate_validate(struct nb_config *candidate)
return NB_ERR_VALIDATION;
RB_INIT(nb_config_cbs, &changes);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_diff(running_config, candidate, &changes);
ret = nb_candidate_validate_changes(candidate, &changes);
nb_config_diff_del_changes(&changes);
}
pthread_rwlock_unlock(&running_config->lock);
return ret;
}
@ -613,26 +631,36 @@ int nb_candidate_commit_prepare(struct nb_config *candidate,
}
RB_INIT(nb_config_cbs, &changes);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_diff(running_config, candidate, &changes);
if (RB_EMPTY(nb_config_cbs, &changes))
if (RB_EMPTY(nb_config_cbs, &changes)) {
pthread_rwlock_unlock(&running_config->lock);
return NB_ERR_NO_CHANGES;
}
if (nb_candidate_validate_changes(candidate, &changes) != NB_OK) {
flog_warn(EC_LIB_NB_CANDIDATE_INVALID,
if (nb_candidate_validate_changes(candidate, &changes)
!= NB_OK) {
flog_warn(
EC_LIB_NB_CANDIDATE_INVALID,
"%s: failed to validate candidate configuration",
__func__);
nb_config_diff_del_changes(&changes);
pthread_rwlock_unlock(&running_config->lock);
return NB_ERR_VALIDATION;
}
*transaction =
nb_transaction_new(candidate, &changes, client, user, comment);
*transaction = nb_transaction_new(candidate, &changes, client,
user, comment);
if (*transaction == NULL) {
flog_warn(EC_LIB_NB_TRANSACTION_CREATION_FAILED,
"%s: failed to create transaction", __func__);
nb_config_diff_del_changes(&changes);
pthread_rwlock_unlock(&running_config->lock);
return NB_ERR_LOCKED;
}
}
pthread_rwlock_unlock(&running_config->lock);
return nb_transaction_process(NB_EV_PREPARE, *transaction);
}
@ -651,7 +679,11 @@ void nb_candidate_commit_apply(struct nb_transaction *transaction,
/* Replace running by candidate. */
transaction->config->version++;
pthread_rwlock_wrlock(&running_config->lock);
{
nb_config_replace(running_config, transaction->config, true);
}
pthread_rwlock_unlock(&running_config->lock);
/* Record transaction. */
if (save_transaction
@ -931,8 +963,15 @@ static int nb_transaction_process(enum nb_event event,
{
struct nb_config_cb *cb;
/*
* Need to lock the running configuration since transaction->changes
* can contain pointers to data nodes from the running configuration.
*/
pthread_rwlock_rdlock(&running_config->lock);
{
RB_FOREACH (cb, nb_config_cbs, &transaction->changes) {
struct nb_config_change *change = (struct nb_config_change *)cb;
struct nb_config_change *change =
(struct nb_config_change *)cb;
int ret;
/*
@ -946,25 +985,30 @@ static int nb_transaction_process(enum nb_event event,
ret = nb_callback_configuration(event, change);
switch (event) {
case NB_EV_PREPARE:
if (ret != NB_OK)
if (ret != NB_OK) {
pthread_rwlock_unlock(
&running_config->lock);
return ret;
}
change->prepare_ok = true;
break;
case NB_EV_ABORT:
case NB_EV_APPLY:
/*
* At this point it's not possible to reject the
* transaction anymore, so any failure here can lead to
* inconsistencies and should be treated as a bug.
* Operations prone to errors, like validations and
* resource allocations, should be performed during the
* 'prepare' phase.
* transaction anymore, so any failure here can
* lead to inconsistencies and should be treated
* as a bug. Operations prone to errors, like
* validations and resource allocations, should
* be performed during the 'prepare' phase.
*/
break;
default:
break;
}
}
}
pthread_rwlock_unlock(&running_config->lock);
return NB_OK;
}

11
lib/northbound.h
View file

@ -422,8 +422,19 @@ enum nb_client {
/* Northbound configuration. */
struct nb_config {
/* Configuration data. */
struct lyd_node *dnode;
/* Configuration version. */
uint32_t version;
/*
* Lock protecting this structure. The use of this lock is always
* necessary when reading or modifying the global running configuration.
* For candidate configurations, use of this lock is optional depending
* on the threading scheme of the northbound plugin.
*/
pthread_rwlock_t lock;
};
/* Northbound configuration callback. */

53
lib/northbound_cli.c
View file

@ -193,8 +193,13 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...)
"Please check the logs for more details.\n");
/* Regenerate candidate for consistency. */
nb_config_replace(vty->candidate_config, running_config,
true);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_replace(vty->candidate_config,
running_config, true);
}
pthread_rwlock_unlock(&running_config->lock);
return CMD_WARNING_CONFIG_FAILED;
}
}
@ -302,7 +307,12 @@ static int nb_cli_commit(struct vty *vty, bool force,
/* "confirm" parameter. */
if (confirmed_timeout) {
vty->confirmed_commit_rollback = nb_config_dup(running_config);
pthread_rwlock_rdlock(&running_config->lock);
{
vty->confirmed_commit_rollback =
nb_config_dup(running_config);
}
pthread_rwlock_unlock(&running_config->lock);
vty->t_confirmed_commit_timeout = NULL;
thread_add_timer(master, nb_cli_confirmed_commit_timeout, vty,
@ -316,8 +326,13 @@ static int nb_cli_commit(struct vty *vty, bool force,
/* Map northbound return code to CLI return code. */
switch (ret) {
case NB_OK:
nb_config_replace(vty->candidate_config_base, running_config,
true);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_replace(vty->candidate_config_base,
running_config, true);
}
pthread_rwlock_unlock(&running_config->lock);
vty_out(vty,
"%% Configuration committed successfully (Transaction ID #%u).\n\n",
transaction_id);
@ -682,7 +697,12 @@ DEFPY (config_update,
return CMD_WARNING;
}
nb_config_replace(vty->candidate_config_base, running_config, true);
pthread_rwlock_rdlock(&running_config->lock);
{
nb_config_replace(vty->candidate_config_base, running_config,
true);
}
pthread_rwlock_unlock(&running_config->lock);
vty_out(vty, "%% Candidate configuration updated successfully.\n\n");
@ -782,8 +802,12 @@ DEFPY (show_config_running,
}
}
pthread_rwlock_rdlock(&running_config->lock);
{
nb_cli_show_config(vty, running_config, format, translator,
!!with_defaults);
}
pthread_rwlock_unlock(&running_config->lock);
return CMD_SUCCESS;
}
@ -897,6 +921,12 @@ DEFPY (show_config_compare,
struct nb_config *config2, *config_transaction2 = NULL;
int ret = CMD_WARNING;
/*
* For simplicity, lock the running configuration regardless if it's
* going to be used or not.
*/
pthread_rwlock_rdlock(&running_config->lock);
{
if (c1_candidate)
config1 = vty->candidate_config;
else if (c1_running)
@ -904,7 +934,8 @@ DEFPY (show_config_compare,
else {
config_transaction1 = nb_db_transaction_load(c1_tid);
if (!config_transaction1) {
vty_out(vty, "%% Transaction %u does not exist\n\n",
vty_out(vty,
"%% Transaction %u does not exist\n\n",
(unsigned int)c1_tid);
goto exit;
}
@ -918,7 +949,8 @@ DEFPY (show_config_compare,
else {
config_transaction2 = nb_db_transaction_load(c2_tid);
if (!config_transaction2) {
vty_out(vty, "%% Transaction %u does not exist\n\n",
vty_out(vty,
"%% Transaction %u does not exist\n\n",
(unsigned int)c2_tid);
goto exit;
}
@ -935,7 +967,8 @@ DEFPY (show_config_compare,
if (translator_family) {
translator = yang_translator_find(translator_family);
if (!translator) {
vty_out(vty, "%% Module translator \"%s\" not found\n",
vty_out(vty,
"%% Module translator \"%s\" not found\n",
translator_family);
goto exit;
}
@ -948,6 +981,8 @@ exit:
nb_config_free(config_transaction1);
if (config_transaction2)
nb_config_free(config_transaction2);
}
pthread_rwlock_unlock(&running_config->lock);
return ret;
}

4
lib/northbound_confd.c
View file

@ -289,7 +289,11 @@ static int frr_confd_cdb_read_cb_prepare(int fd, int *subp, int reslen)
struct cdb_iter_args iter_args;
int ret;
pthread_rwlock_rdlock(&running_config->lock);
{
candidate = nb_config_dup(running_config);
}
pthread_rwlock_unlock(&running_config->lock);
/* Iterate over all configuration changes. */
iter_args.candidate = candidate;

4
lib/northbound_sysrepo.c
View file

@ -256,7 +256,11 @@ static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
return ret;
}
pthread_rwlock_rdlock(&running_config->lock);
{
candidate = nb_config_dup(running_config);
}
pthread_rwlock_unlock(&running_config->lock);
while ((ret = sr_get_change_next(session, it, &sr_op, &sr_old_val,
&sr_new_val))

7
lib/vty.c
View file

@ -2608,9 +2608,12 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
vty->private_config = private_config;
vty->xpath_index = 0;
pthread_rwlock_rdlock(&running_config->lock);
{
if (private_config) {
vty->candidate_config = nb_config_dup(running_config);
vty->candidate_config_base = nb_config_dup(running_config);
vty->candidate_config_base =
nb_config_dup(running_config);
vty_out(vty,
"Warning: uncommitted changes will be discarded on exit.\n\n");
} else {
@ -2619,6 +2622,8 @@ int vty_config_enter(struct vty *vty, bool private_config, bool exclusive)
vty->candidate_config_base =
nb_config_dup(running_config);
}
}
pthread_rwlock_unlock(&running_config->lock);
return CMD_SUCCESS;
}