Skip to content

Commit dfe7d94

Browse files
committed
session UPDATE minor lock refactoring
Also added another lock to be held during server hello message creation.
1 parent cddf61b commit dfe7d94

File tree

4 files changed

+72
-22
lines changed

4 files changed

+72
-22
lines changed

src/session.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,8 +1117,6 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
11171117
cpblts[1] = strdup("urn:ietf:params:netconf:base:1.1");
11181118
count = 2;
11191119

1120-
/* capabilities */
1121-
11221120
mod = ly_ctx_get_module_implemented(ctx, "ietf-netconf");
11231121
if (mod) {
11241122
if (lys_feature_value(mod, "writable-running") == LY_SUCCESS) {
@@ -1153,11 +1151,15 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
11531151
}
11541152
}
11551153

1154+
/* HELLO LOCK */
1155+
pthread_rwlock_rdlock(&server_opts.hello_lock);
1156+
11561157
mod = ly_ctx_get_module_implemented(ctx, "ietf-netconf-with-defaults");
11571158
if (mod) {
1158-
wd_basic_mode = ATOMIC_LOAD_RELAXED(server_opts.wd_basic_mode);
1159+
wd_basic_mode = server_opts.wd_basic_mode;
11591160
if (!wd_basic_mode) {
1160-
VRB(NULL, "with-defaults capability will not be advertised even though \"ietf-netconf-with-defaults\" model is present, unknown basic-mode.");
1161+
VRB(NULL, "with-defaults capability will not be advertised even though \"ietf-netconf-with-defaults\" "
1162+
"model is present, unknown basic-mode.");
11611163
} else {
11621164
strcpy(str, "urn:ietf:params:netconf:capability:with-defaults:1.0");
11631165
switch (wd_basic_mode) {
@@ -1172,7 +1174,7 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
11721174
break;
11731175
default:
11741176
ERRINT;
1175-
break;
1177+
goto unlock_error;
11761178
}
11771179

11781180
wd_also_supported = ATOMIC_LOAD_RELAXED(server_opts.wd_also_supported);
@@ -1213,7 +1215,7 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
12131215
if (!strcmp(mod->name, "ietf-yang-library")) {
12141216
if (!mod->revision || (strcmp(mod->revision, "2016-06-21") && strcmp(mod->revision, "2019-01-04"))) {
12151217
ERR(NULL, "Unknown \"ietf-yang-library\" revision, only 2016-06-21 and 2019-01-04 are supported.");
1216-
goto error;
1218+
goto unlock_error;
12171219
}
12181220

12191221
/* get content-id */
@@ -1298,11 +1300,18 @@ nc_server_get_cpblts_version(const struct ly_ctx *ctx, LYS_VERSION version)
12981300
add_cpblt(str, &cpblts, &size, &count);
12991301
}
13001302

1303+
/* HELLO UNLOCK */
1304+
pthread_rwlock_unlock(&server_opts.hello_lock);
1305+
13011306
/* ending NULL capability */
13021307
add_cpblt(NULL, &cpblts, &size, &count);
13031308

13041309
return cpblts;
13051310

1311+
unlock_error:
1312+
/* HELLO UNLOCK */
1313+
pthread_rwlock_unlock(&server_opts.hello_lock);
1314+
13061315
error:
13071316
free(cpblts);
13081317
return NULL;

src/session_p.h

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -498,19 +498,27 @@ struct nc_ch_client_thread_arg {
498498
};
499499

500500
struct nc_server_opts {
501-
/* ACCESS unlocked */
502-
ATOMIC_T wd_basic_mode;
503-
ATOMIC_T wd_also_supported;
504-
uint32_t capabilities_count;
501+
/* ACCESS locked - hello lock - separate lock to not always hold config_lock */
502+
char **ignored_modules; /**< Names of YANG modules that are not reported in the server <hello> message. */
503+
uint16_t ignored_mod_count;
504+
NC_WD_MODE wd_basic_mode; /**< With-defaults basic mode of the server. */
505+
int wd_also_supported; /**< Bitmap of with-defaults modes that are also supported by the server. */
505506
char **capabilities;
507+
uint32_t capabilities_count;
506508

507-
char *(*content_id_clb)(void *user_data);
509+
char *(*content_id_clb)(void *user_data); /**< Callback for generating content_id for ietf-yang-library data. */
508510
void *content_id_data;
509-
510511
void (*content_id_data_free)(void *data);
511512

513+
pthread_rwlock_t hello_lock; /**< Needs to be held while the server <hello> message is being generated. */
514+
515+
/* ACCESS unlocked */
512516
uint16_t idle_timeout;
513517

518+
/* ACCESS locked - options modified by YANG data/API - WRITE lock
519+
* - options read when accepting sessions - READ lock */
520+
pthread_rwlock_t config_lock;
521+
514522
#ifdef NC_ENABLED_SSH_TLS
515523
char *authkey_path_fmt; /**< Path to users' public keys that may contain tokens with special meaning. */
516524
char *pam_config_name; /**< PAM configuration file name. */
@@ -521,13 +529,12 @@ struct nc_server_opts {
521529
int (*user_verify_clb)(const struct nc_session *session);
522530
#endif /* NC_ENABLED_SSH_TLS */
523531

524-
pthread_rwlock_t config_lock;
525-
526532
#ifdef NC_ENABLED_SSH_TLS
527-
struct nc_keystore keystore; /**< store for server's keys/certificates */
528-
struct nc_truststore truststore; /**< store for server client's keys/certificates */
533+
struct nc_keystore keystore; /**< Server's keys/certificates. */
534+
struct nc_truststore truststore; /**< Server client's keys/certificates. */
529535
#endif /* NC_ENABLED_SSH_TLS */
530536

537+
/* ACCESS locked */
531538
struct nc_bind *binds;
532539
pthread_mutex_t bind_lock; /**< To avoid concurrent calls of poll and accept on the bound sockets **/
533540
struct nc_endpt {
@@ -607,11 +614,12 @@ struct nc_server_opts {
607614
} ch_dispatch_data;
608615
#endif /* NC_ENABLED_SSH_TLS */
609616

610-
/* Atomic IDs */
617+
/* ACCESS unlocked */
611618
ATOMIC_T new_session_id;
612619
ATOMIC_T new_client_id;
613620

614621
#ifdef NC_ENABLED_SSH_TLS
622+
/* ACCESS locked */
615623
struct {
616624
pthread_t tid; /**< Thread ID of the certificate expiration notification thread. */
617625
int thread_running; /**< Flag representing the runningness of the cert exp notification thread. */
@@ -628,7 +636,8 @@ struct nc_server_opts {
628636
int interval_count; /**< Number of intervals. */
629637
} cert_exp_notif;
630638

631-
FILE *tls_keylog_file; /**< File to log TLS secrets to. */
639+
/* ACCESS unlocked */
640+
FILE *tls_keylog_file; /**< File to log TLS secrets to. */
632641
#endif
633642
};
634643

src/session_server.c

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
#endif
5353

5454
struct nc_server_opts server_opts = {
55+
.hello_lock = PTHREAD_RWLOCK_INITIALIZER,
5556
.config_lock = PTHREAD_RWLOCK_INITIALIZER,
5657
.ch_client_lock = PTHREAD_RWLOCK_INITIALIZER,
5758
.idle_timeout = 180, /**< default idle timeout (not in config for UNIX socket) */
@@ -962,8 +963,15 @@ nc_server_set_capab_withdefaults(NC_WD_MODE basic_mode, int also_supported)
962963
return -1;
963964
}
964965

965-
ATOMIC_STORE_RELAXED(server_opts.wd_basic_mode, basic_mode);
966-
ATOMIC_STORE_RELAXED(server_opts.wd_also_supported, also_supported);
966+
/* HELLO LOCK */
967+
pthread_rwlock_wrlock(&server_opts.hello_lock);
968+
969+
server_opts.wd_basic_mode = basic_mode;
970+
server_opts.wd_also_supported = also_supported;
971+
972+
/* HELLO UNLOCK */
973+
pthread_rwlock_unlock(&server_opts.hello_lock);
974+
967975
return 0;
968976
}
969977

@@ -975,12 +983,18 @@ nc_server_get_capab_withdefaults(NC_WD_MODE *basic_mode, int *also_supported)
975983
return;
976984
}
977985

986+
/* HELLO LOCK */
987+
pthread_rwlock_wrlock(&server_opts.hello_lock);
988+
978989
if (basic_mode) {
979-
*basic_mode = ATOMIC_LOAD_RELAXED(server_opts.wd_basic_mode);
990+
*basic_mode = server_opts.wd_basic_mode;
980991
}
981992
if (also_supported) {
982-
*also_supported = ATOMIC_LOAD_RELAXED(server_opts.wd_also_supported);
993+
*also_supported = server_opts.wd_also_supported;
983994
}
995+
996+
/* HELLO UNLOCK */
997+
pthread_rwlock_unlock(&server_opts.hello_lock);
984998
}
985999

9861000
API int
@@ -993,23 +1007,35 @@ nc_server_set_capability(const char *value)
9931007
return EXIT_FAILURE;
9941008
}
9951009

1010+
/* HELLO LOCK */
1011+
pthread_rwlock_wrlock(&server_opts.hello_lock);
1012+
9961013
mem = realloc(server_opts.capabilities, (server_opts.capabilities_count + 1) * sizeof *server_opts.capabilities);
9971014
NC_CHECK_ERRMEM_RET(!mem, EXIT_FAILURE);
9981015
server_opts.capabilities = mem;
9991016

10001017
server_opts.capabilities[server_opts.capabilities_count] = strdup(value);
10011018
server_opts.capabilities_count++;
10021019

1020+
/* HELLO UNLOCK */
1021+
pthread_rwlock_unlock(&server_opts.hello_lock);
1022+
10031023
return EXIT_SUCCESS;
10041024
}
10051025

10061026
API void
10071027
nc_server_set_content_id_clb(char *(*content_id_clb)(void *user_data), void *user_data,
10081028
void (*free_user_data)(void *user_data))
10091029
{
1030+
/* HELLO LOCK */
1031+
pthread_rwlock_wrlock(&server_opts.hello_lock);
1032+
10101033
server_opts.content_id_clb = content_id_clb;
10111034
server_opts.content_id_data = user_data;
10121035
server_opts.content_id_data_free = free_user_data;
1036+
1037+
/* HELLO UNLOCK */
1038+
pthread_rwlock_unlock(&server_opts.hello_lock);
10131039
}
10141040

10151041
API NC_MSG_TYPE

src/session_server_tls.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,13 @@ nc_session_get_client_cert(const struct nc_session *session)
658658
API void
659659
nc_server_tls_set_verify_clb(int (*verify_clb)(const struct nc_session *session))
660660
{
661+
/* CONFIG LOCK */
662+
pthread_rwlock_wrlock(&server_opts.config_lock);
663+
661664
server_opts.user_verify_clb = verify_clb;
665+
666+
/* CONFIG UNLOCK */
667+
pthread_rwlock_unlock(&server_opts.config_lock);
662668
}
663669

664670
int

0 commit comments

Comments
 (0)