From f0e6c4fa2cd3dfdfafecdb2be2f7d0a464f14a89 Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Fri, 6 Dec 2024 16:05:59 +0100 Subject: [PATCH 1/2] - Draft the idea of unjostable queries if upstreams are explicitly configured. --- services/mesh.c | 73 +++++++++++++------ services/mesh.h | 6 ++ .../mesh_jostle_priority.conf | 27 +++++++ .../mesh_jostle_priority.dsc | 16 ++++ .../mesh_jostle_priority.post | 12 +++ .../mesh_jostle_priority.pre | 36 +++++++++ .../mesh_jostle_priority.test | 31 ++++++++ .../mesh_jostle_priority.testns | 35 +++++++++ 8 files changed, 212 insertions(+), 24 deletions(-) create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.conf create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.dsc create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.post create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.pre create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.test create mode 100644 testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.testns diff --git a/services/mesh.c b/services/mesh.c index a25094d12..74b72eea8 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -43,6 +43,8 @@ * send back to clients. */ #include "config.h" +#include "iterator/iter_hints.h" +#include "iterator/iter_fwd.h" #include "services/mesh.h" #include "services/outbound_list.h" #include "services/cache/dns.h" @@ -275,37 +277,60 @@ mesh_delete_all(struct mesh_area* mesh) int mesh_make_new_space(struct mesh_area* mesh, sldns_buffer* qbuf) { struct mesh_state* m = mesh->jostle_first; - /* free space is available */ - if(mesh->num_reply_states < mesh->max_reply_states) + if(mesh->num_reply_states < mesh->max_reply_states) { + /* free space is available */ return 1; + } /* try to kick out a jostle-list item */ if(m && m->reply_list && m->list_select == mesh_jostle_list) { - /* how old is it? */ struct timeval age; + struct iter_hints_stub* stub = NULL; + struct delegpt* fwd = NULL; + /* how old is it? */ timeval_subtract(&age, mesh->env->now_tv, &m->reply_list->start_time); - if(timeval_smaller(&mesh->jostle_max, &age)) { - /* its a goner */ - log_nametypeclass(VERB_ALGO, "query jostled out to " - "make space for a new one", - m->s.qinfo.qname, m->s.qinfo.qtype, - m->s.qinfo.qclass); - /* backup the query */ - if(qbuf) sldns_buffer_copy(mesh->qbuf_bak, qbuf); - /* notify supers */ - if(m->super_set.count > 0) { - verbose(VERB_ALGO, "notify supers of failure"); - m->s.return_msg = NULL; - m->s.return_rcode = LDNS_RCODE_SERVFAIL; - mesh_walk_supers(mesh, m); + if(!timeval_smaller(&mesh->jostle_max, &age) + || m->has_configured_upstream) { + /* can't jostle out; still fresh or has configured + * upstreams */ + return 0; + } + if((stub=hints_lookup_stub(mesh->env->hints, + m->s.qinfo.qname, m->s.qinfo.qclass, NULL, 0)) + || (fwd=forwards_find(mesh->env->fwds, + m->s.qinfo.qname, m->s.qinfo.qclass, 0))) { + /* can't jostle out; there is a configured upstream + * for this */ + m->has_configured_upstream = 1; + if(stub) { + lock_rw_unlock(&mesh->env->hints->lock); } - mesh->stats_jostled ++; - mesh_state_delete(&m->s); - /* restore the query - note that the qinfo ptr to - * the querybuffer is then correct again. */ - if(qbuf) sldns_buffer_copy(qbuf, mesh->qbuf_bak); - return 1; + if(fwd) { + lock_rw_unlock(&mesh->env->fwds->lock); + } + return 0; } + /* its a goner; older than jostle-timeout and there are no + * configured upstreams that could give it 'persistence' */ + log_nametypeclass(VERB_ALGO, "query jostled out to " + "make space for a new one", + m->s.qinfo.qname, m->s.qinfo.qtype, + m->s.qinfo.qclass); + /* backup the query */ + if(qbuf) sldns_buffer_copy(mesh->qbuf_bak, qbuf); + /* notify supers */ + if(m->super_set.count > 0) { + verbose(VERB_ALGO, "notify supers of failure"); + m->s.return_msg = NULL; + m->s.return_rcode = LDNS_RCODE_SERVFAIL; + mesh_walk_supers(mesh, m); + } + mesh->stats_jostled ++; + mesh_state_delete(&m->s); + /* restore the query - note that the qinfo ptr to + * the querybuffer is then correct again. */ + if(qbuf) sldns_buffer_copy(qbuf, mesh->qbuf_bak); + return 1; } /* no space for new item */ return 0; @@ -1441,7 +1466,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, * Need to explicitly check for rep->security otherwise failed * validation paths may attach to a secure answer. */ if(m->s.env->cfg->ede && rep && - (rep->security <= sec_status_bogus || + (rep->security <= sec_status_bogus || rep->security == sec_status_insecure || rep->security == sec_status_secure_sentinel_fail)) { mesh_find_and_attach_ede_and_reason(m, rep, r); } diff --git a/services/mesh.h b/services/mesh.h index 0906ed9cf..7cbd4c62a 100644 --- a/services/mesh.h +++ b/services/mesh.h @@ -206,6 +206,12 @@ struct mesh_state { /** pointer to this state for uniqueness or NULL */ struct mesh_state* unique; + /** if the qname of this mesh state will go out to one of the + * configured upstreams (stub-zone or forward-zone). + * Only used when trying to make space (jostle) for new client + * queries. */ + int has_configured_upstream; + /** true if replies have been sent out (at end for alignment) */ uint8_t replies_sent; }; diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.conf b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.conf new file mode 100644 index 000000000..cca59ec07 --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.conf @@ -0,0 +1,27 @@ +server: + num-threads: 1 + verbosity: 5 + use-syslog: no + directory: "" + pidfile: "unbound.pid" + chroot: "" + username: "" + do-not-query-localhost: no + use-caps-for-id: no + port: @SERVER_PORT@ + interface: 127.0.0.1 + + num-queries-per-thread: 2 + infra-cache-min-rtt: 1000 + +stub-zone: + name: "a.slow" + stub-addr: 127.0.0.1@@SLOW_STUB_PORT@ + +stub-zone: + name: "b.slow" + stub-addr: 127.0.0.1@@SLOW_STUB_PORT@ + +stub-zone: + name: "c.fast" + stub-addr: 127.0.0.1@@FAST_STUB_PORT@ diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.dsc b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.dsc new file mode 100644 index 000000000..1517cf1b7 --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.dsc @@ -0,0 +1,16 @@ +BaseName: mesh_jostle_priority +Version: 1.0 +Description: Check that mesh items will not be jostled if there is a explicitly configured upstream +CreationDate: Tue 3 Dec 11:00:00 CEST 2024 +Maintainer: +Category: +Component: +CmdDepends: +Depends: +Help: +Pre: mesh_jostle_priority.pre +Post: mesh_jostle_priority.post +Test: mesh_jostle_priority.test +AuxFiles: +Passed: +Failure: diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.post b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.post new file mode 100644 index 000000000..759610920 --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.post @@ -0,0 +1,12 @@ +# #-- mesh_jostle_priority.post --# +# source the master var file when it's there +[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master +# source the test var file when it's there +[ -f .tpkg.var.test ] && source .tpkg.var.test +# +# do your teardown here +. ../common.sh +kill_from_pidfile "unbound.pid" +kill_pid $SLOW_STUB_PID +kill_pid $FAST_STUB_PID +cat unbound.log diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.pre b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.pre new file mode 100644 index 000000000..f6830468d --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.pre @@ -0,0 +1,36 @@ +# #-- mesh_jostle_priority.pre--# +PRE="../.." +. ../common.sh + +get_random_port 3 +SERVER_PORT=$RND_PORT +SLOW_STUB_PORT=$(($RND_PORT + 1)) +FAST_STUB_PORT=$(($RND_PORT + 2)) +echo "SERVER_PORT=$SERVER_PORT" >> .tpkg.var.test +echo "SLOW_STUB_PORT=$SLOW_STUB_PORT" >> .tpkg.var.test +echo "FAST_STUB_PORT=$FAST_STUB_PORT" >> .tpkg.var.test + +# start stub +get_ldns_testns +$LDNS_TESTNS -v -p $SLOW_STUB_PORT mesh_jostle_priority.testns >stub_slow.log 2>&1 & +SLOW_STUB_PID=$! +echo "SLOW_STUB_PID=$SLOW_STUB_PID" >> .tpkg.var.test + +$LDNS_TESTNS -v -p $FAST_STUB_PORT mesh_jostle_priority.testns >stub_fast.log 2>&1 & +FAST_STUB_PID=$! +echo "FAST_STUB_PID=$FAST_STUB_PID" >> .tpkg.var.test + +# make config file +sed \ + -e 's/@SERVER_PORT\@/'$SERVER_PORT'/' \ + -e 's/@SLOW_STUB_PORT\@/'$SLOW_STUB_PORT'/' \ + -e 's/@FAST_STUB_PORT\@/'$FAST_STUB_PORT'/' \ + < mesh_jostle_priority.conf > ub.conf + +# start unbound in the background +$PRE/unbound -d -c ub.conf > unbound.log 2>&1 & + +cat .tpkg.var.test +wait_ldns_testns_up stub_slow.log +wait_ldns_testns_up stub_fast.log +wait_unbound_up unbound.log diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.test b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.test new file mode 100644 index 000000000..c78a10e71 --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.test @@ -0,0 +1,31 @@ +# #-- mesh_jostle_priority.test --# +# source the master var file when it's there +[ -f ../.tpkg.var.master ] && source ../.tpkg.var.master +# use .tpkg.var.test for in test variable passing +[ -f .tpkg.var.test ] && source .tpkg.var.test +PRE="../.." +. ../common.sh + +get_make +(cd $PRE; $MAKE streamtcp) + +teststep "Fire up and forget the two slow queries" +$PRE/streamtcp -f 127.0.0.1@$SERVER_PORT -u -n a.slow. A IN b.slow. A IN +if test "$?" -ne 0 +then + echo "error with streamtcp" + exit 1 +fi + +teststep "Let time pass so that queries can be jostled based on time" +sleep 1 + +teststep "Check that the fast query is dropped because the slow ones have explicitly configured upstreams" +dig @127.0.0.1 -p $SERVER_PORT +retry=0 +timeout=1 c.fast. > outfile +if grep "10.20.30.40" outfile +then + cat outfile + echo "Did not get a drop" + exit 1 +fi +exit 0 diff --git a/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.testns b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.testns new file mode 100644 index 000000000..8b76de19f --- /dev/null +++ b/testdata/mesh_jostle_priority.tdir/mesh_jostle_priority.testns @@ -0,0 +1,35 @@ +$ORIGIN slow. +$TTL 3600 + +ENTRY_BEGIN +MATCH opcode qtype qname +REPLY QR AA NOERROR +ADJUST copy_id sleep=5 +SECTION QUESTION +a IN A +SECTION ANSWER +a IN A 1.1.1.1 +ENTRY_END + +ENTRY_BEGIN +MATCH opcode qtype qname +REPLY QR AA NOERROR +ADJUST copy_id sleep=5 +SECTION QUESTION +b IN A +SECTION ANSWER +b IN A 2.2.2.2 +ENTRY_END + +$ORIGIN fast. + +ENTRY_BEGIN +MATCH opcode qtype qname +REPLY QR AA NOERROR +ADJUST copy_id +SECTION QUESTION +c IN A +SECTION ANSWER +c IN A 10.20.30.40 +ENTRY_END + From 34d0b93c7123c3843c9546b98fa8e616e4505ade Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Fri, 6 Dec 2024 16:16:58 +0100 Subject: [PATCH 2/2] - This sleplt in the precious commit. --- services/mesh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/mesh.c b/services/mesh.c index 74b72eea8..25e5ea399 100644 --- a/services/mesh.c +++ b/services/mesh.c @@ -1466,7 +1466,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep, * Need to explicitly check for rep->security otherwise failed * validation paths may attach to a secure answer. */ if(m->s.env->cfg->ede && rep && - (rep->security <= sec_status_bogus || rep->security == sec_status_insecure || + (rep->security <= sec_status_bogus || rep->security == sec_status_secure_sentinel_fail)) { mesh_find_and_attach_ede_and_reason(m, rep, r); }