Skip to content

Commit 770aec6

Browse files
gthesswcawijngaards
gthess
and
wcawijngaards
committed
Update locking management for iter_fwd and iter_hints methods. (#1054)
fast reload, move most of the locking management to iter_fwd and iter_hints methods. The caller still has the ability to handle its own locking, if desired, for atomic operations on sets of different structs. Co-authored-by: Wouter Wijngaards <wcawijngaards@users.noreply.github.com>
1 parent 0637df2 commit 770aec6

File tree

9 files changed

+295
-151
lines changed

9 files changed

+295
-151
lines changed

‎daemon/cachedump.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,7 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
839839
char b[260];
840840
struct query_info qinfo;
841841
struct iter_hints_stub* stub;
842+
int nolock = 0;
842843
regional_free_all(region);
843844
qinfo.qname = nm;
844845
qinfo.qname_len = nmlen;
@@ -851,8 +852,7 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
851852
"of %s\n", b))
852853
return 0;
853854

854-
lock_rw_rdlock(&worker->env.fwds->lock);
855-
dp = forwards_lookup(worker->env.fwds, nm, qinfo.qclass);
855+
dp = forwards_lookup(worker->env.fwds, nm, qinfo.qclass, nolock);
856856
if(dp) {
857857
if(!ssl_printf(ssl, "forwarding request:\n")) {
858858
lock_rw_unlock(&worker->env.fwds->lock);
@@ -863,7 +863,6 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
863863
lock_rw_unlock(&worker->env.fwds->lock);
864864
return 1;
865865
}
866-
lock_rw_unlock(&worker->env.fwds->lock);
867866

868867
while(1) {
869868
dp = dns_cache_find_delegation(&worker->env, nm, nmlen,
@@ -898,9 +897,8 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
898897
continue;
899898
}
900899
}
901-
lock_rw_rdlock(&worker->env.hints->lock);
902900
stub = hints_lookup_stub(worker->env.hints, nm, qinfo.qclass,
903-
dp);
901+
dp, nolock);
904902
if(stub) {
905903
if(stub->noprime) {
906904
if(!ssl_printf(ssl, "The noprime stub servers "
@@ -919,7 +917,6 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
919917
print_dp_details(ssl, worker, stub->dp);
920918
lock_rw_unlock(&worker->env.hints->lock);
921919
} else {
922-
lock_rw_unlock(&worker->env.hints->lock);
923920
print_dp_main(ssl, dp, msg);
924921
print_dp_details(ssl, worker, dp);
925922
}

‎daemon/remote.c

+21-17
Original file line numberDiff line numberDiff line change
@@ -2070,10 +2070,9 @@ static int
20702070
print_root_fwds(RES* ssl, struct iter_forwards* fwds, uint8_t* root)
20712071
{
20722072
struct delegpt* dp;
2073-
lock_rw_rdlock(&fwds->lock);
2074-
dp = forwards_lookup(fwds, root, LDNS_RR_CLASS_IN);
2073+
int nolock = 0;
2074+
dp = forwards_lookup(fwds, root, LDNS_RR_CLASS_IN, nolock);
20752075
if(!dp) {
2076-
lock_rw_unlock(&fwds->lock);
20772076
return ssl_printf(ssl, "off (using root hints)\n");
20782077
}
20792078
/* if dp is returned it must be the root */
@@ -2155,6 +2154,7 @@ do_forward(RES* ssl, struct worker* worker, char* args)
21552154
{
21562155
struct iter_forwards* fwd = worker->env.fwds;
21572156
uint8_t* root = (uint8_t*)"\000";
2157+
int nolock = 0;
21582158
if(!fwd) {
21592159
(void)ssl_printf(ssl, "error: structure not allocated\n");
21602160
return;
@@ -2168,20 +2168,15 @@ do_forward(RES* ssl, struct worker* worker, char* args)
21682168
/* delete all the existing queries first */
21692169
mesh_delete_all(worker->env.mesh);
21702170
if(strcmp(args, "off") == 0) {
2171-
lock_rw_wrlock(&fwd->lock);
2172-
forwards_delete_zone(fwd, LDNS_RR_CLASS_IN, root);
2173-
lock_rw_unlock(&fwd->lock);
2171+
forwards_delete_zone(fwd, LDNS_RR_CLASS_IN, root, nolock);
21742172
} else {
21752173
struct delegpt* dp;
21762174
if(!(dp = parse_delegpt(ssl, args, root)))
21772175
return;
2178-
lock_rw_wrlock(&fwd->lock);
2179-
if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp)) {
2180-
lock_rw_unlock(&fwd->lock);
2176+
if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp, nolock)) {
21812177
(void)ssl_printf(ssl, "error out of memory\n");
21822178
return;
21832179
}
2184-
lock_rw_unlock(&fwd->lock);
21852180
}
21862181
send_ok(ssl);
21872182
}
@@ -2240,10 +2235,12 @@ do_forward_add(RES* ssl, struct worker* worker, char* args)
22402235
int insecure = 0, tls = 0;
22412236
uint8_t* nm = NULL;
22422237
struct delegpt* dp = NULL;
2238+
int nolock = 1;
22432239
if(!parse_fs_args(ssl, args, &nm, &dp, &insecure, NULL, &tls))
22442240
return;
22452241
if(tls)
22462242
dp->ssl_upstream = 1;
2243+
/* prelock forwarders for atomic operation with anchors */
22472244
lock_rw_wrlock(&fwd->lock);
22482245
if(insecure && worker->env.anchors) {
22492246
if(!anchors_add_insecure(worker->env.anchors, LDNS_RR_CLASS_IN,
@@ -2255,7 +2252,7 @@ do_forward_add(RES* ssl, struct worker* worker, char* args)
22552252
return;
22562253
}
22572254
}
2258-
if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp)) {
2255+
if(!forwards_add_zone(fwd, LDNS_RR_CLASS_IN, dp, nolock)) {
22592256
lock_rw_unlock(&fwd->lock);
22602257
(void)ssl_printf(ssl, "error out of memory\n");
22612258
free(nm);
@@ -2273,13 +2270,15 @@ do_forward_remove(RES* ssl, struct worker* worker, char* args)
22732270
struct iter_forwards* fwd = worker->env.fwds;
22742271
int insecure = 0;
22752272
uint8_t* nm = NULL;
2273+
int nolock = 1;
22762274
if(!parse_fs_args(ssl, args, &nm, NULL, &insecure, NULL, NULL))
22772275
return;
2276+
/* prelock forwarders for atomic operation with anchors */
22782277
lock_rw_wrlock(&fwd->lock);
22792278
if(insecure && worker->env.anchors)
22802279
anchors_delete_insecure(worker->env.anchors, LDNS_RR_CLASS_IN,
22812280
nm);
2282-
forwards_delete_zone(fwd, LDNS_RR_CLASS_IN, nm);
2281+
forwards_delete_zone(fwd, LDNS_RR_CLASS_IN, nm, nolock);
22832282
lock_rw_unlock(&fwd->lock);
22842283
free(nm);
22852284
send_ok(ssl);
@@ -2293,10 +2292,12 @@ do_stub_add(RES* ssl, struct worker* worker, char* args)
22932292
int insecure = 0, prime = 0, tls = 0;
22942293
uint8_t* nm = NULL;
22952294
struct delegpt* dp = NULL;
2295+
int nolock = 1;
22962296
if(!parse_fs_args(ssl, args, &nm, &dp, &insecure, &prime, &tls))
22972297
return;
22982298
if(tls)
22992299
dp->ssl_upstream = 1;
2300+
/* prelock forwarders and hints for atomic operation with anchors */
23002301
lock_rw_wrlock(&fwd->lock);
23012302
lock_rw_wrlock(&worker->env.hints->lock);
23022303
if(insecure && worker->env.anchors) {
@@ -2310,7 +2311,7 @@ do_stub_add(RES* ssl, struct worker* worker, char* args)
23102311
return;
23112312
}
23122313
}
2313-
if(!forwards_add_stub_hole(fwd, LDNS_RR_CLASS_IN, nm)) {
2314+
if(!forwards_add_stub_hole(fwd, LDNS_RR_CLASS_IN, nm, nolock)) {
23142315
if(insecure && worker->env.anchors)
23152316
anchors_delete_insecure(worker->env.anchors,
23162317
LDNS_RR_CLASS_IN, nm);
@@ -2321,9 +2322,10 @@ do_stub_add(RES* ssl, struct worker* worker, char* args)
23212322
free(nm);
23222323
return;
23232324
}
2324-
if(!hints_add_stub(worker->env.hints, LDNS_RR_CLASS_IN, dp, !prime)) {
2325+
if(!hints_add_stub(worker->env.hints, LDNS_RR_CLASS_IN, dp, !prime,
2326+
nolock)) {
23252327
(void)ssl_printf(ssl, "error out of memory\n");
2326-
forwards_delete_stub_hole(fwd, LDNS_RR_CLASS_IN, nm);
2328+
forwards_delete_stub_hole(fwd, LDNS_RR_CLASS_IN, nm, nolock);
23272329
if(insecure && worker->env.anchors)
23282330
anchors_delete_insecure(worker->env.anchors,
23292331
LDNS_RR_CLASS_IN, nm);
@@ -2345,15 +2347,17 @@ do_stub_remove(RES* ssl, struct worker* worker, char* args)
23452347
struct iter_forwards* fwd = worker->env.fwds;
23462348
int insecure = 0;
23472349
uint8_t* nm = NULL;
2350+
int nolock = 1;
23482351
if(!parse_fs_args(ssl, args, &nm, NULL, &insecure, NULL, NULL))
23492352
return;
2353+
/* prelock forwarders and hints for atomic operation with anchors */
23502354
lock_rw_wrlock(&fwd->lock);
23512355
lock_rw_wrlock(&worker->env.hints->lock);
23522356
if(insecure && worker->env.anchors)
23532357
anchors_delete_insecure(worker->env.anchors, LDNS_RR_CLASS_IN,
23542358
nm);
2355-
forwards_delete_stub_hole(fwd, LDNS_RR_CLASS_IN, nm);
2356-
hints_delete_stub(worker->env.hints, LDNS_RR_CLASS_IN, nm);
2359+
forwards_delete_stub_hole(fwd, LDNS_RR_CLASS_IN, nm, nolock);
2360+
hints_delete_stub(worker->env.hints, LDNS_RR_CLASS_IN, nm, nolock);
23572361
lock_rw_unlock(&fwd->lock);
23582362
lock_rw_unlock(&worker->env.hints->lock);
23592363
free(nm);

‎iterator/iter_fwd.c

+76-25
Original file line numberDiff line numberDiff line change
@@ -359,39 +359,50 @@ forwards_apply_cfg(struct iter_forwards* fwd, struct config_file* cfg)
359359
}
360360

361361
struct delegpt*
362-
forwards_find(struct iter_forwards* fwd, uint8_t* qname, uint16_t qclass)
362+
forwards_find(struct iter_forwards* fwd, uint8_t* qname, uint16_t qclass,
363+
int nolock)
363364
{
364-
rbnode_type* res = NULL;
365+
struct iter_forward_zone* res;
365366
struct iter_forward_zone key;
367+
int has_dp;
366368
key.node.key = &key;
367369
key.dclass = qclass;
368370
key.name = qname;
369371
key.namelabs = dname_count_size_labels(qname, &key.namelen);
370-
res = rbtree_search(fwd->tree, &key);
371-
if(res) return ((struct iter_forward_zone*)res)->dp;
372-
return NULL;
372+
/* lock_() calls are macros that could be nothing, surround in {} */
373+
if(!nolock) { lock_rw_rdlock(&fwd->lock); }
374+
res = (struct iter_forward_zone*)rbtree_search(fwd->tree, &key);
375+
has_dp = res && res->dp;
376+
if(!has_dp && !nolock) { lock_rw_unlock(&fwd->lock); }
377+
return has_dp?res->dp:NULL;
373378
}
374379

375380
struct delegpt*
376-
forwards_lookup(struct iter_forwards* fwd, uint8_t* qname, uint16_t qclass)
381+
forwards_lookup(struct iter_forwards* fwd, uint8_t* qname, uint16_t qclass,
382+
int nolock)
377383
{
378384
/* lookup the forward zone in the tree */
379385
rbnode_type* res = NULL;
380386
struct iter_forward_zone *result;
381387
struct iter_forward_zone key;
388+
int has_dp;
382389
key.node.key = &key;
383390
key.dclass = qclass;
384391
key.name = qname;
385392
key.namelabs = dname_count_size_labels(qname, &key.namelen);
393+
/* lock_() calls are macros that could be nothing, surround in {} */
394+
if(!nolock) { lock_rw_rdlock(&fwd->lock); }
386395
if(rbtree_find_less_equal(fwd->tree, &key, &res)) {
387396
/* exact */
388397
result = (struct iter_forward_zone*)res;
389398
} else {
390399
/* smaller element (or no element) */
391400
int m;
392401
result = (struct iter_forward_zone*)res;
393-
if(!result || result->dclass != qclass)
402+
if(!result || result->dclass != qclass) {
403+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
394404
return NULL;
405+
}
395406
/* count number of labels matched */
396407
(void)dname_lab_cmp(result->name, result->namelabs, key.name,
397408
key.namelabs, &m);
@@ -401,20 +412,22 @@ forwards_lookup(struct iter_forwards* fwd, uint8_t* qname, uint16_t qclass)
401412
result = result->parent;
402413
}
403414
}
404-
if(result)
405-
return result->dp;
406-
return NULL;
415+
has_dp = result && result->dp;
416+
if(!has_dp && !nolock) { lock_rw_unlock(&fwd->lock); }
417+
return has_dp?result->dp:NULL;
407418
}
408419

409420
struct delegpt*
410-
forwards_lookup_root(struct iter_forwards* fwd, uint16_t qclass)
421+
forwards_lookup_root(struct iter_forwards* fwd, uint16_t qclass, int nolock)
411422
{
412423
uint8_t root = 0;
413-
return forwards_lookup(fwd, &root, qclass);
424+
return forwards_lookup(fwd, &root, qclass, nolock);
414425
}
415426

416-
int
417-
forwards_next_root(struct iter_forwards* fwd, uint16_t* dclass)
427+
/* Finds next root item in forwards lookup tree.
428+
* Caller needs to handle locking of the forwards structure. */
429+
static int
430+
next_root_locked(struct iter_forwards* fwd, uint16_t* dclass)
418431
{
419432
struct iter_forward_zone key;
420433
rbnode_type* n;
@@ -431,7 +444,7 @@ forwards_next_root(struct iter_forwards* fwd, uint16_t* dclass)
431444
}
432445
/* root not first item? search for higher items */
433446
*dclass = p->dclass + 1;
434-
return forwards_next_root(fwd, dclass);
447+
return next_root_locked(fwd, dclass);
435448
}
436449
/* find class n in tree, we may get a direct hit, or if we don't
437450
* this is the last item of the previous class so rbtree_next() takes
@@ -459,10 +472,21 @@ forwards_next_root(struct iter_forwards* fwd, uint16_t* dclass)
459472
}
460473
/* not a root node, return next higher item */
461474
*dclass = p->dclass+1;
462-
return forwards_next_root(fwd, dclass);
475+
return next_root_locked(fwd, dclass);
463476
}
464477
}
465478

479+
int
480+
forwards_next_root(struct iter_forwards* fwd, uint16_t* dclass, int nolock)
481+
{
482+
int ret;
483+
/* lock_() calls are macros that could be nothing, surround in {} */
484+
if(!nolock) { lock_rw_rdlock(&fwd->lock); }
485+
ret = next_root_locked(fwd, dclass);
486+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
487+
return ret;
488+
}
489+
466490
size_t
467491
forwards_get_mem(struct iter_forwards* fwd)
468492
{
@@ -491,53 +515,80 @@ fwd_zone_find(struct iter_forwards* fwd, uint16_t c, uint8_t* nm)
491515
}
492516

493517
int
494-
forwards_add_zone(struct iter_forwards* fwd, uint16_t c, struct delegpt* dp)
518+
forwards_add_zone(struct iter_forwards* fwd, uint16_t c, struct delegpt* dp,
519+
int nolock)
495520
{
496521
struct iter_forward_zone *z;
522+
/* lock_() calls are macros that could be nothing, surround in {} */
523+
if(!nolock) { lock_rw_wrlock(&fwd->lock); }
497524
if((z=fwd_zone_find(fwd, c, dp->name)) != NULL) {
498525
(void)rbtree_delete(fwd->tree, &z->node);
499526
fwd_zone_free(z);
500527
}
501-
if(!forwards_insert(fwd, c, dp))
528+
if(!forwards_insert(fwd, c, dp)) {
529+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
502530
return 0;
531+
}
503532
fwd_init_parents(fwd);
533+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
504534
return 1;
505535
}
506536

507537
void
508-
forwards_delete_zone(struct iter_forwards* fwd, uint16_t c, uint8_t* nm)
538+
forwards_delete_zone(struct iter_forwards* fwd, uint16_t c, uint8_t* nm,
539+
int nolock)
509540
{
510541
struct iter_forward_zone *z;
511-
if(!(z=fwd_zone_find(fwd, c, nm)))
542+
/* lock_() calls are macros that could be nothing, surround in {} */
543+
if(!nolock) { lock_rw_wrlock(&fwd->lock); }
544+
if(!(z=fwd_zone_find(fwd, c, nm))) {
545+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
512546
return; /* nothing to do */
547+
}
513548
(void)rbtree_delete(fwd->tree, &z->node);
514549
fwd_zone_free(z);
515550
fwd_init_parents(fwd);
551+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
516552
}
517553

518554
int
519-
forwards_add_stub_hole(struct iter_forwards* fwd, uint16_t c, uint8_t* nm)
555+
forwards_add_stub_hole(struct iter_forwards* fwd, uint16_t c, uint8_t* nm,
556+
int nolock)
520557
{
521-
if(fwd_zone_find(fwd, c, nm) != NULL)
558+
/* lock_() calls are macros that could be nothing, surround in {} */
559+
if(!nolock) { lock_rw_wrlock(&fwd->lock); }
560+
if(fwd_zone_find(fwd, c, nm) != NULL) {
561+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
522562
return 1; /* already a stub zone there */
563+
}
523564
if(!fwd_add_stub_hole(fwd, c, nm)) {
565+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
524566
return 0;
525567
}
526568
fwd_init_parents(fwd);
569+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
527570
return 1;
528571
}
529572

530573
void
531-
forwards_delete_stub_hole(struct iter_forwards* fwd, uint16_t c, uint8_t* nm)
574+
forwards_delete_stub_hole(struct iter_forwards* fwd, uint16_t c,
575+
uint8_t* nm, int nolock)
532576
{
533577
struct iter_forward_zone *z;
534-
if(!(z=fwd_zone_find(fwd, c, nm)))
578+
/* lock_() calls are macros that could be nothing, surround in {} */
579+
if(!nolock) { lock_rw_wrlock(&fwd->lock); }
580+
if(!(z=fwd_zone_find(fwd, c, nm))) {
581+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
535582
return; /* nothing to do */
536-
if(z->dp != NULL)
583+
}
584+
if(z->dp != NULL) {
585+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
537586
return; /* not a stub hole */
587+
}
538588
(void)rbtree_delete(fwd->tree, &z->node);
539589
fwd_zone_free(z);
540590
fwd_init_parents(fwd);
591+
if(!nolock) { lock_rw_unlock(&fwd->lock); }
541592
}
542593

543594
void

0 commit comments

Comments
 (0)