From 4c194d0ce1eaaf0cbd792a89b3020b61a7915e6b Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 21 May 2024 20:41:17 +1000 Subject: [PATCH 01/18] Move variable definition closer to use, and reduce globals --- apps/n3n-supernode.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/n3n-supernode.c b/apps/n3n-supernode.c index 49471cf8..94409b6c 100644 --- a/apps/n3n-supernode.c +++ b/apps/n3n-supernode.c @@ -54,8 +54,6 @@ #define HASH_FIND_COMMUNITY(head, name, out) HASH_FIND_STR(head, name, out) -static struct n3n_runtime_data sss_node; - /* *************************************************** */ #define GETOPTS "O:Vdhv" @@ -383,6 +381,7 @@ static void term_handler (int sig) /** Main program entry point from kernel. */ int main (int argc, char * argv[]) { + static struct n3n_runtime_data sss_node; // Do this early to register all internals n3n_initfuncs(); From 9121f44cc21ba306de0653aca8859a7fc1389214 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 21 May 2024 20:43:45 +1000 Subject: [PATCH 02/18] Since no caller uses any result, make the function return void --- src/edge_utils.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/edge_utils.c b/src/edge_utils.c index 2d6a8268..3db8e67b 100644 --- a/src/edge_utils.c +++ b/src/edge_utils.c @@ -316,7 +316,7 @@ static int detect_local_ip_address (n2n_sock_t* out_sock, const struct n3n_runti // open socket, close it before if TCP // in case of TCP, 'connect()' is required -int supernode_connect (struct n3n_runtime_data *eee) { +void supernode_connect (struct n3n_runtime_data *eee) { int sockopt; struct sockaddr_in sn_sock; @@ -338,7 +338,7 @@ int supernode_connect (struct n3n_runtime_data *eee) { if(eee->sock < 0) { traceEvent(TRACE_ERROR, "failed to bind main UDP port"); - return -1; + return; } fill_sockaddr((struct sockaddr*)&sn_sock, sizeof(sn_sock), &eee->curr_sn->sock); @@ -354,8 +354,9 @@ int supernode_connect (struct n3n_runtime_data *eee) { #endif if((connect(eee->sock, (struct sockaddr*)&(sn_sock), sizeof(struct sockaddr)) < 0) && (errno != EINPROGRESS)) { + traceEvent(TRACE_INFO, "Error connecting TCP: %i", errno); eee->sock = -1; - return -1; + return; } } @@ -426,7 +427,7 @@ int supernode_connect (struct n3n_runtime_data *eee) { // REVISIT: add mgmt port notification to listener for better mgmt port // subscription support - return 0; + return; } From 762a6782c55825ab573118a925360fcbddeaa7e0 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 21 May 2024 20:44:58 +1000 Subject: [PATCH 03/18] Add some functional notes --- src/edge_utils.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/edge_utils.c b/src/edge_utils.c index 3db8e67b..8775bc0e 100644 --- a/src/edge_utils.c +++ b/src/edge_utils.c @@ -1027,6 +1027,7 @@ static void check_known_peer_sock_change (struct n3n_runtime_data *eee, * Confirm that we can send to this edge. * TODO: for the TCP case, this could cause a stall in the packet * send path, so this probably should be reworked to use a queue + * (and non blocking IO) */ static bool check_sock_ready (struct n3n_runtime_data *eee) { if(!eee->conf.connect_tcp) { @@ -1103,6 +1104,7 @@ static ssize_t sendto_fd (struct n3n_runtime_data *eee, const void *buf, if(eee->conf.connect_tcp) { supernode_disconnect(eee); eee->sn_wait = 1; + // Not true if eee->sock == -1 traceEvent(TRACE_DEBUG, "error in sendto_fd"); } @@ -2887,6 +2889,9 @@ int fetch_and_eventually_process_data (struct n3n_runtime_data *eee, SOCKET sock return -1; } + // TODO: if bread > 64K, something is wrong + // but this case should not happen + // we have a datagram to process... if(bread > 0) { // ...and the datagram has data (not just a header) From 0a3f2ad4a65bd2abcded6b937aaffc8c1ac0e15e Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 21 May 2024 20:45:47 +1000 Subject: [PATCH 04/18] Dont leak memory --- src/sn_utils.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/sn_utils.c b/src/sn_utils.c index 9a8cc95e..54453719 100644 --- a/src/sn_utils.c +++ b/src/sn_utils.c @@ -308,13 +308,11 @@ int load_allowed_sn_community (struct n3n_runtime_data *sss) { // remove community HASH_DEL(sss->communities, comm); - if(NULL != comm->header_encryption_ctx_static) { - // remove header encryption keys - free(comm->header_encryption_ctx_static); - free(comm->header_iv_ctx_static); - free(comm->header_encryption_ctx_dynamic); - free(comm->header_iv_ctx_dynamic); - } + // remove header encryption keys + free(comm->header_encryption_ctx_static); + free(comm->header_iv_ctx_static); + free(comm->header_encryption_ctx_dynamic); + free(comm->header_iv_ctx_dynamic); free(comm); } @@ -527,7 +525,7 @@ static ssize_t sendto_fd (struct n3n_runtime_data *sss, return -1; } } else { - traceEvent(TRACE_DEBUG, "sendto sent=%d to ", (signed int)sent); + traceEvent(TRACE_DEBUG, "sendto_fd sent=%d", (signed int)sent); } return sent; @@ -1680,7 +1678,9 @@ static int process_udp (struct n3n_runtime_data * sss, "unencrypted headers", comm->community); /* set 'no encryption' in case it is not set yet */ comm->header_encryption = HEADER_ENCRYPTION_NONE; + free(comm->header_encryption_ctx_static); comm->header_encryption_ctx_static = NULL; + free(comm->header_encryption_ctx_dynamic); comm->header_encryption_ctx_dynamic = NULL; } } @@ -2028,7 +2028,9 @@ static int process_udp (struct n3n_runtime_data * sss, comm_init(comm, (char *)cmn.community); /* new communities introduced by REGISTERs could not have had encrypted header... */ comm->header_encryption = HEADER_ENCRYPTION_NONE; + free(comm->header_encryption_ctx_static); comm->header_encryption_ctx_static = NULL; + free(comm->header_encryption_ctx_dynamic); comm->header_encryption_ctx_dynamic = NULL; /* ... and also are purgeable during periodic purge */ comm->purgeable = true; From 6eddebc9d49f0267328eeb3dfd9e49b5913d9684 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:27:59 +1000 Subject: [PATCH 05/18] Consolidate more steps into the list ending overflow/hack --- src/management.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/management.c b/src/management.c index 63f893cd..653118cc 100644 --- a/src/management.c +++ b/src/management.c @@ -398,7 +398,7 @@ static void jsonrpc_stop (char *id, struct n3n_runtime_data *eee, conn_t *conn, jsonrpc_1uint(id, conn, *eee->keep_running); } -static void jsonrpc_listend_overflow (conn_t *conn) { +static void jsonrpc_listend_hack (conn_t *conn, const char *endch) { if(sb_overflowed(conn->request)) { // Make a clear indicator in the output sb_append(conn->request, "\"overflow\"", 10); @@ -408,6 +408,8 @@ static void jsonrpc_listend_overflow (conn_t *conn) { if(conn->request->str[conn->request->wr_pos-1] == ',') { conn->request->wr_pos--; } + // and replace with the relevant list ending char + sb_reprintf(&conn->request, endch); } static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *conn, const char *params) { @@ -454,8 +456,7 @@ static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *con } } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -498,8 +499,7 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con (community->auto_ip_net.net_addr == 0) ? "" : ip_subnet_to_str(ip_bit_str, &community->auto_ip_net)); } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -587,8 +587,7 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -663,8 +662,7 @@ static void jsonrpc_get_supernodes (char *id, struct n3n_runtime_data *eee, conn (uint32_t)peer->uptime); } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -770,8 +768,7 @@ static void jsonrpc_get_packetstats (char *id, struct n3n_runtime_data *eee, con "\"tx_pkt\":%u},", eee->stats.sn_errors); - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -844,8 +841,7 @@ static void jsonrpc_help_events (char *id, struct n3n_runtime_data *eee, conn_t ); } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } @@ -892,8 +888,7 @@ static void jsonrpc_help (char *id, struct n3n_runtime_data *eee, conn_t *conn, } - jsonrpc_listend_overflow(conn); - sb_reprintf(&conn->request, "]"); + jsonrpc_listend_hack(conn, "]"); jsonrpc_result_tail(conn, 200); } From f872b304aabf8f480dbeb399e6baba71947f4a2b Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:30:11 +1000 Subject: [PATCH 06/18] Allow providing some data with any json rpc errors --- src/management.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/management.c b/src/management.c index 653118cc..8bd28486 100644 --- a/src/management.c +++ b/src/management.c @@ -301,7 +301,7 @@ void mgmt_event_post (const enum n3n_event_topic topic, int data0, const void *d // - if the write returns EWOULDBLOCK, increment a metric and return } -static void jsonrpc_error (char *id, conn_t *conn, int code, char *message) { +static void jsonrpc_error (char *id, conn_t *conn, int code, char *message, int count) { // Reuse the request buffer sb_zero(conn->request); @@ -311,14 +311,26 @@ static void jsonrpc_error (char *id, conn_t *conn, int code, char *message) { "\"jsonrpc\":\"2.0\"," "\"id\":\"%s\"," "\"error\":{" - " \"code\":%i," - " \"message\":\"%s\"" - "}}", + "\"code\":%i," + "\"message\":\"%s\"", id, code, message ); + if(count) { + sb_reprintf( + &conn->request, + "," + "\"data\":" + "{" + "\"count\":%i" + "}", + count + ); + } + sb_reprintf(&conn->request, "}}"); + // Update the reply buffer after last potential realloc conn->reply = conn->request; generate_http_headers(conn, "application/json", code); @@ -364,12 +376,12 @@ static void jsonrpc_set_verbose (char *id, struct n3n_runtime_data *eee, conn_t } if(!params_in) { - jsonrpc_error(id, conn, 400, "missing param"); + jsonrpc_error(id, conn, 400, "missing param", 0); return; } if(*params_in != '[') { - jsonrpc_error(id, conn, 400, "expecting array"); + jsonrpc_error(id, conn, 400, "expecting array", 0); return; } @@ -464,7 +476,7 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con if(!eee->communities) { // This is an edge if(eee->conf.header_encryption != HEADER_ENCRYPTION_NONE) { - jsonrpc_error(id, conn, 403, "Forbidden"); + jsonrpc_error(id, conn, 403, "Forbidden", 0); return; } From 1cfc384df55fda93718093d9eb367457edf20354 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:35:09 +1000 Subject: [PATCH 07/18] Generate overflow messages as a real jsonrpc error, along with a count --- src/management.c | 49 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/src/management.c b/src/management.c index 8bd28486..7714cdb9 100644 --- a/src/management.c +++ b/src/management.c @@ -329,11 +329,7 @@ static void jsonrpc_error (char *id, conn_t *conn, int code, char *message, int count ); } - sb_reprintf(&conn->request, "}}"); - - // Update the reply buffer after last potential realloc - conn->reply = conn->request; - generate_http_headers(conn, "application/json", code); + sb_reprintf(&conn->request, "}"); } static void jsonrpc_result_head (char *id, conn_t *conn) { @@ -410,12 +406,18 @@ static void jsonrpc_stop (char *id, struct n3n_runtime_data *eee, conn_t *conn, jsonrpc_1uint(id, conn, *eee->keep_running); } -static void jsonrpc_listend_hack (conn_t *conn, const char *endch) { - if(sb_overflowed(conn->request)) { - // Make a clear indicator in the output - sb_append(conn->request, "\"overflow\"", 10); +static bool jsonrpc_error_overflow (char *id, conn_t *conn, int count) { + if(!sb_overflowed(conn->request)) { + // Nothing to do + return false; } + jsonrpc_error(id, conn, 507, "overflow", count); + jsonrpc_result_tail(conn, 507); + return true; +} + +static void jsonrpc_listend_hack (conn_t *conn, const char *endch) { // HACK: back up over the final ',' if(conn->request->str[conn->request->wr_pos-1] == ',') { conn->request->wr_pos--; @@ -436,6 +438,7 @@ static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *con struct sn_community *tmp_community; struct node_supernode_association *assoc; struct node_supernode_association *tmp_assoc; + int count = 0; HASH_ITER(hh, eee->communities, community, tmp_community) { HASH_ITER(hh, community->assoc, assoc, tmp_assoc) { @@ -465,6 +468,11 @@ static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *con port, (uint32_t)assoc->last_seen ); + + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; } } @@ -497,6 +505,8 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con jsonrpc_result_head(id, conn); sb_reprintf(&conn->request, "["); + int count = 0; + HASH_ITER(hh, eee->communities, community, tmp) { sb_reprintf(&conn->request, @@ -509,6 +519,11 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con community->purgeable, community->is_federation, (community->auto_ip_net.net_addr == 0) ? "" : ip_subnet_to_str(ip_bit_str, &community->auto_ip_net)); + + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; } jsonrpc_listend_hack(conn, "]"); @@ -566,6 +581,7 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c jsonrpc_result_head(id, conn); sb_reprintf(&conn->request, "["); + int count = 0; // dump nodes with forwarding through supernodes HASH_ITER(hh, eee->pending_peers, peer, tmpPeer) { jsonrpc_get_edges_row( @@ -574,6 +590,11 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c "pSp", eee->conf.community_name ); + + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; } // dump peer-to-peer nodes @@ -584,6 +605,11 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c "p2p", eee->conf.community_name ); + + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; } struct sn_community *community, *tmp; @@ -596,6 +622,11 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c (community->is_federation) ? "-/-" : community->community ); } + + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; } From 2e2eb73c5f5d3efde78b175ded422aded1581382 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:41:53 +1000 Subject: [PATCH 08/18] Define python jsonrpc error class at the right level --- scripts/n3nctl | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/scripts/n3nctl b/scripts/n3nctl index 389de2f8..e9e27eba 100755 --- a/scripts/n3nctl +++ b/scripts/n3nctl @@ -17,10 +17,6 @@ import urllib.parse import urllib.request -class Unauthenticated(Exception): - pass - - class UnixHandler(urllib.request.BaseHandler): def __init__(self, basepath): self.basepath = basepath @@ -102,6 +98,9 @@ class JsonRPC: req.add_header("Authorization", b"Basic " + encoded) return req + class Unauthenticated(Exception): + pass + def get(self, method, params=None): req = self._request_obj(method, params) @@ -109,11 +108,11 @@ class JsonRPC: r = urllib.request.urlopen(req, timeout=self.timeout) except urllib.error.HTTPError as e: if e.code == 401: - raise Unauthenticated + raise JsonRPC.Unauthenticated raise e if r.status == 401: - raise Unauthenticated + raise JsonRPC.Unauthenticated if r.status != 200: raise ValueError(f"urllib request got {r.status} {r.reason}") @@ -362,7 +361,7 @@ def main(): try: result = func(rpc, args) - except Unauthenticated: + except JsonRPC.Unauthenticated: print("This request requires an authentication key") exit(1) except FileNotFoundError: From 3a8e72783a4e49e9c432666b9f80c8b8437e996b Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:46:03 +1000 Subject: [PATCH 09/18] Document the use of overflow error API objects --- doc/ManagementAPI.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/ManagementAPI.md b/doc/ManagementAPI.md index 4c8d59c4..2e53ff7b 100644 --- a/doc/ManagementAPI.md +++ b/doc/ManagementAPI.md @@ -96,3 +96,9 @@ will check for a standard HTTP Authorization header in the request. The authentication is a simple password that the client must provide. It defaults to 'n3n' and can either be set with the config option `management.password` + +## Pagination + +If the result of an API call will overrun the size of the internal buffer, +the response will indicate an "overflow" condition by returning a 507 result +and a JsonRPC error object. From d523ffaaddd5ca490defdc2cabb1c5a3405ba14d Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 13:50:58 +1000 Subject: [PATCH 10/18] Detect and raise overflow conditions --- scripts/n3nctl | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/scripts/n3nctl b/scripts/n3nctl index e9e27eba..d1d0ba2a 100755 --- a/scripts/n3nctl +++ b/scripts/n3nctl @@ -99,8 +99,15 @@ class JsonRPC: return req class Unauthenticated(Exception): + """Raised if the request needs authentication added""" pass + class Overflow(Exception): + """Raised if the daemon overflows its internal buffer""" + def __init__(self, count): + self.count = count + super().__init__() + def get(self, method, params=None): req = self._request_obj(method, params) @@ -111,23 +118,25 @@ class JsonRPC: raise JsonRPC.Unauthenticated raise e - if r.status == 401: - raise JsonRPC.Unauthenticated - if r.status != 200: - raise ValueError(f"urllib request got {r.status} {r.reason}") - body = r.read() if self.debug: print("reply:", body) - r = json.loads(body) + body = json.loads(body) + + if r.status == 401: + raise JsonRPC.Unauthenticated + if r.status == 507: + raise JsonRPC.Overflow(body["error"]["data"]["count"]) + if r.status != 200: + raise ValueError(f"urllib request got {r.status} {r.reason}") - if "result" not in r: + if "result" not in body: raise ValueError("jsonrpc error") - assert (r['id'] == str(self.id)) + assert (body['id'] == str(self.id)) - return r['result'] + return body['result'] def str_table(rows, columns, orderby): From 1a23fad95a8be1fc4990ebdaa193a400dde0d435 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 14:49:11 +1000 Subject: [PATCH 11/18] Add automatic pagination to the CLI tool --- scripts/n3nctl | 72 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/scripts/n3nctl b/scripts/n3nctl index d1d0ba2a..6c304349 100755 --- a/scripts/n3nctl +++ b/scripts/n3nctl @@ -108,7 +108,9 @@ class JsonRPC: self.count = count super().__init__() - def get(self, method, params=None): + def get_nopagination(self, method, params=None): + """Makes the RPC request, with no automated pagination retry""" + req = self._request_obj(method, params) try: @@ -118,14 +120,15 @@ class JsonRPC: raise JsonRPC.Unauthenticated raise e + if r.status == 401: + raise JsonRPC.Unauthenticated + body = r.read() if self.debug: print("reply:", body) body = json.loads(body) - if r.status == 401: - raise JsonRPC.Unauthenticated if r.status == 507: raise JsonRPC.Overflow(body["error"]["data"]["count"]) if r.status != 200: @@ -138,6 +141,67 @@ class JsonRPC: return body['result'] + def get(self, method, offset=None, limit=None, params=None): + if params is not None and len(params) == 0: + # This can happen with the args passed from CLI + params = None + + # Populate the params if needed + if offset is not None: + if params is None: + params = dict() + params["offset"] = offset + if limit is not None: + if params is None: + params = dict() + params["limit"] = limit + + # Try once, possibly without pagination, to see if we overflow. And + # if so, to detect a good size for the limit + try: + return self.get_nopagination(method, params) + except JsonRPC.Overflow as e: + count_max = e.count + + if params is None: + params = dict() + + if not isinstance(params, dict): + """Since we overflowed, params must be compatible""" + raise ValueError(f"Cannot use params={params} with autopagination") + + params["limit"] = count_max + params["offset"] = 0 + + result = [] + + while True: + try: + partial = self.get_nopagination(method, params) + except JsonRPC.Overflow as e: + if count_max == e.count: + # If the limit didnt get smaller, there is a problem with + # the daemon, so we just bail out + raise + + # reduce our asking size and try again + count_max = e.count + params["limit"] = count_max + continue + + if not isinstance(partial, list): + # The current API only returns lists, but there could be + # dicts in the future, catch this + raise NotImplementedError + + result.extend(partial) + params["offset"] = len(result) + + if len(partial) < params["limit"]: + # We fetched less than we asked for, so we must be at the end + # of the list + return result + def str_table(rows, columns, orderby): """Given an array of dicts, do a simple table print""" @@ -300,7 +364,7 @@ def subcmd_default(rpc, args): """Just pass command through to edge""" method = args.cmd params = args.args - rows = rpc.get(method, params) + rows = rpc.get(method, params=params) return json.dumps(rows, sort_keys=True, indent=4) From a0569b8ff1c3652b5f5f9c177ccb6ebb1b86ec58 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Fri, 12 Jul 2024 15:44:38 +1000 Subject: [PATCH 12/18] Add pagination to management API - This splits long-running queries into multiple chunks - Reducing the size of the needed memory buffer - Allowing less blocking of the main loop - This does open a race condition for the underlying data to change, but this is considered a reasonable trade off - The implementation is not efficient as we would need to implement persistent cursors accessing the uthash data to avoid multiple list scans. --- doc/ManagementAPI.md | 11 ++++++ src/management.c | 94 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 7 deletions(-) diff --git a/doc/ManagementAPI.md b/doc/ManagementAPI.md index 2e53ff7b..fbb7baca 100644 --- a/doc/ManagementAPI.md +++ b/doc/ManagementAPI.md @@ -102,3 +102,14 @@ defaults to 'n3n' and can either be set with the config option If the result of an API call will overrun the size of the internal buffer, the response will indicate an "overflow" condition by returning a 507 result and a JsonRPC error object. + +When an overflow condition is returned, the error object may contain the +count of the number of items that could be added before the overflow occured. +The request can be retried with pagination parameters to avoid the overflow. +(Note that this also opens up a window for the internal data to change during +the paginated request) + +Add the offset and limit values to the param dictionary. + +The n3nctl tool has an example on how to use this implemented in its +JsonRPC.get() method diff --git a/src/management.c b/src/management.c index 7714cdb9..d74953f1 100644 --- a/src/management.c +++ b/src/management.c @@ -301,6 +301,26 @@ void mgmt_event_post (const enum n3n_event_topic topic, int data0, const void *d // - if the write returns EWOULDBLOCK, increment a metric and return } +static void extract_pagination (char *params, int *limit, int *offset) { + char *limitstr = json_find_field(params, "\"limit\""); + char *offsetstr = json_find_field(params, "\"offset\""); + + // do all the field finding first, since the value extractor will + // insert nulls at the end of its strings + + if(limitstr) { + *limit = atoi(json_extract_val(limitstr)); + } else { + *limit = 2147483647; // default + } + + if(offsetstr) { + *offset = atoi(json_extract_val(offsetstr)); + } else { + offset = 0; + } +} + static void jsonrpc_error (char *id, conn_t *conn, int code, char *message, int count) { // Reuse the request buffer sb_zero(conn->request); @@ -382,6 +402,7 @@ static void jsonrpc_set_verbose (char *id, struct n3n_runtime_data *eee, conn_t } // Avoid discarding the const attribute + // TODO: avoid malloc() char *params = strdup(params_in+1); char *arg1 = json_extract_val(params); @@ -438,9 +459,21 @@ static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *con struct sn_community *tmp_community; struct node_supernode_association *assoc; struct node_supernode_association *tmp_assoc; - int count = 0; + + int limit; // max number of items to add to this packet + int offset = 0; // Number of items to skip before adding + extract_pagination((char *)params, &limit, &offset); + int count = 0; // Number of items in this reply packet + int index = 0; // Track the current item number + HASH_ITER(hh, eee->communities, community, tmp_community) { HASH_ITER(hh, community->assoc, assoc, tmp_assoc) { + if(index < offset) { + index++; + continue; + } + index++; + char buf[1000]; char port[10]; @@ -473,6 +506,9 @@ static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *con return; } count++; + if(count >= limit) { + break; + } } } @@ -505,9 +541,18 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con jsonrpc_result_head(id, conn); sb_reprintf(&conn->request, "["); - int count = 0; + int limit; // max number of items to add to this packet + int offset = 0; // Number of items to skip before adding + extract_pagination((char *)params, &limit, &offset); + int count = 0; // Number of items in this reply packet + int index = 0; // Track the current item number HASH_ITER(hh, eee->communities, community, tmp) { + if(index < offset) { + index++; + continue; + } + index++; sb_reprintf(&conn->request, "{" @@ -524,6 +569,9 @@ static void jsonrpc_get_communities (char *id, struct n3n_runtime_data *eee, con return; } count++; + if(count >= limit) { + break; + } } jsonrpc_listend_hack(conn, "]"); @@ -581,9 +629,20 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c jsonrpc_result_head(id, conn); sb_reprintf(&conn->request, "["); - int count = 0; + int limit; // max number of items to add to this packet + int offset = 0; // Number of items to skip before adding + extract_pagination((char *)params, &limit, &offset); + int count = 0; // Number of items in this reply packet + int index = 0; // Track the current item number + // dump nodes with forwarding through supernodes HASH_ITER(hh, eee->pending_peers, peer, tmpPeer) { + if(index < offset) { + index++; + continue; + } + index++; + jsonrpc_get_edges_row( &conn->request, peer, @@ -595,10 +654,19 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c return; } count++; + if(count >= limit) { + break; + } } // dump peer-to-peer nodes HASH_ITER(hh, eee->known_peers, peer, tmpPeer) { + if(index < offset) { + index++; + continue; + } + index++; + jsonrpc_get_edges_row( &conn->request, peer, @@ -610,23 +678,35 @@ static void jsonrpc_get_edges (char *id, struct n3n_runtime_data *eee, conn_t *c return; } count++; + if(count >= limit) { + break; + } } struct sn_community *community, *tmp; HASH_ITER(hh, eee->communities, community, tmp) { HASH_ITER(hh, community->edges, peer, tmpPeer) { + if(index < offset) { + index++; + continue; + } + index++; + jsonrpc_get_edges_row( &conn->request, peer, "sn", (community->is_federation) ? "-/-" : community->community ); - } - if(jsonrpc_error_overflow(id, conn, count)) { - return; + if(jsonrpc_error_overflow(id, conn, count)) { + return; + } + count++; + if(count >= limit) { + break; + } } - count++; } From 167e91c14c804ac35b8b467cc2925c1310732203 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Sat, 13 Jul 2024 13:06:56 +1000 Subject: [PATCH 13/18] Bump version file --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index a0891f56..18091983 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.3.4 +3.4.0 From e480b7c63942dc51ae629ef599abbe2674fac8f5 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 16 Jul 2024 13:44:09 +1000 Subject: [PATCH 14/18] Fix code quality bug Once again, deal with the issue that github actions uses a different compiler to Debian stable, and thus has a slightly different set of compile-time warnings that can occur when running the test suite. --- src/management.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/management.c b/src/management.c index d74953f1..559e5cb5 100644 --- a/src/management.c +++ b/src/management.c @@ -444,7 +444,7 @@ static void jsonrpc_listend_hack (conn_t *conn, const char *endch) { conn->request->wr_pos--; } // and replace with the relevant list ending char - sb_reprintf(&conn->request, endch); + sb_reprintf(&conn->request, "%s", endch); } static void jsonrpc_get_mac (char *id, struct n3n_runtime_data *eee, conn_t *conn, const char *params) { From 1cbdd06d8366db5de8d54bfa685aebec5311660e Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 16 Jul 2024 13:58:28 +1000 Subject: [PATCH 15/18] Address some github actions warning messages --- .github/workflows/tests.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 385cb1b7..0d52a6ec 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -140,7 +140,6 @@ jobs: fail-fast: true matrix: os: - - macos-11 - macos-12 steps: @@ -151,11 +150,6 @@ jobs: run: | git fetch --force --tags - - - name: Install packages - run: | - brew install automake - - name: generate a makefile and use it to install more packages run: | ./autogen.sh From d533322c7afe38c804b566fd3f2cbe2b65fb7c53 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 16 Jul 2024 13:58:46 +1000 Subject: [PATCH 16/18] Add a amd64 openwrt build --- .github/workflows/tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 0d52a6ec..81f0b00e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -356,6 +356,9 @@ jobs: fail-fast: true matrix: include: + - name: arm64 + sdk_ver: 22.03.3 + sdk: https://downloads.openwrt.org/releases/22.03.3/targets/armvirt/64/openwrt-sdk-22.03.3-armvirt-64_gcc-11.2.0_musl.Linux-x86_64.tar.xz - name: mips_24kc sdk_ver: 22.03.3 sdk: https://downloads.openwrt.org/releases/22.03.3/targets/lantiq/xrx200/openwrt-sdk-22.03.3-lantiq-xrx200_gcc-11.2.0_musl.Linux-x86_64.tar.xz From 75bdcbab47ee2309d8a4b239b43bce8d8e57b697 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 16 Jul 2024 14:04:11 +1000 Subject: [PATCH 17/18] Learn again to ignore what github claims to want Reverts change made because github actions said "automake is already installed" --- .github/workflows/tests.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 81f0b00e..1e7e4616 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -150,6 +150,10 @@ jobs: run: | git fetch --force --tags + - name: Install packages + run: | + brew install automake + - name: generate a makefile and use it to install more packages run: | ./autogen.sh From 012a34f74e6d577b26bb4de085992729e1b94898 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Tue, 16 Jul 2024 14:18:35 +1000 Subject: [PATCH 18/18] Bump version file --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 18091983..47b322c9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.4.0 +3.4.1