From b40dafb691618032d568cbf333e20e776901df0c Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 16 Jun 2021 11:58:13 +0200 Subject: [PATCH] * Event exit API changed to a single decrementing counter where 1 means exit. * Removed: `clicon_exit_reset()` * Changed: `clicon_exit_set()` --> `clixon_exit_set(int nr)` * Changed: `clicon_exit_get()` --> `clixon_exit_get()` * native restconf special case upgrade handling from http/1 -> http/2 and restconf restart: delay the restart using event exit counter --- .travis.yml | 4 +-- CHANGELOG.md | 9 ++++++ apps/backend/backend_main.c | 2 +- apps/restconf/restconf_main_fcgi.c | 4 +-- apps/restconf/restconf_main_native.c | 13 ++++++++- apps/restconf/restconf_nghttp2.c | 7 +++-- apps/restconf/restconf_stream_fcgi.c | 8 +++--- lib/clixon/clixon_event.h | 6 ++-- lib/src/clixon_event.c | 41 +++++++++++++++++----------- lib/src/clixon_validate.c | 5 +--- 10 files changed, 63 insertions(+), 36 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1e73a976..d281f199 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,6 @@ branches: script: ./configure --with-restconf=fcgi && make && sudo make install && make test before_script: - sudo apt-get install -y libfcgi-dev - - - if [[ -n "${DOCKERUSER}" ]]; then echo "$DOCKERPASSWD" | docker login -u "$DOCKERUSER" --password-stdin; fi +# - if [[ -n "${DOCKERUSER}" ]]; then echo "$DOCKERPASSWD" | docker login -u "$DOCKERUSER" --password-stdin; fi + - echo "$DOCKERPASSWD" | docker login -u "$DOCKERUSER" --password-stdin - ./test/travis/before_script.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index d4c74257..9b9a8920 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,15 @@ Users may have to change how they access the system * Previous meaning (wrong): Return all `a` elements. * New meaning (correct): Return the `a` instance with empty key string: "". +### C/CLI-API changes on existing features + +Developers may need to change their code + +* Event exit API changed to a single decrementing counter where 1 means exit. + * Removed: `clicon_exit_reset()` + * Changed: `clicon_exit_set()` --> `clixon_exit_set(int nr)` + * Changed: `clicon_exit_get()` --> `clixon_exit_get()` + ### Minor features * Added autotool check for getresuid (+ related functions) necessary for lowering of priviliges for backend and restconf diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 47931b42..dda4b759 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -154,7 +154,7 @@ backend_sig_term(int arg) if (i++ == 0) clicon_log(LOG_NOTICE, "%s: %s: pid: %u Signal %d", __PROGRAM__, __FUNCTION__, getpid(), arg); - clicon_exit_set(); /* checked in clixon_event_loop() */ + clixon_exit_set(1); /* checked in clixon_event_loop() */ } /*! wait for killed child diff --git a/apps/restconf/restconf_main_fcgi.c b/apps/restconf/restconf_main_fcgi.c index dae155b6..162f10c8 100644 --- a/apps/restconf/restconf_main_fcgi.c +++ b/apps/restconf/restconf_main_fcgi.c @@ -153,7 +153,7 @@ restconf_sig_term(int arg) * is entered, it will terminate. * However there may be a case of sockets closing rather abruptly for clients */ - clicon_exit_set(); + clixon_exit_set(1); close(_MYSOCK); } @@ -623,7 +623,7 @@ main(int argc, goto done; if (finish) FCGX_Finish_r(req); - else if (clicon_exit_get()){ + else if (clixon_exit_get()){ FCGX_Finish_r(req); break; } diff --git a/apps/restconf/restconf_main_native.c b/apps/restconf/restconf_main_native.c index f2fda9a8..a4594639 100644 --- a/apps/restconf/restconf_main_native.c +++ b/apps/restconf/restconf_main_native.c @@ -849,6 +849,17 @@ restconf_connection(int s, /* Use params from original http/1 session to http/2 stream */ if (http2_exec(rc, sd, rc->rc_ngsession, 1) < 0) goto done; + /* + * Very special case for http/1->http/2 upgrade and restconf "restart" + * That is, the restconf daemon is restarted under the hood, and the session + * is closed in mid-step: it needs a couple of extra rounds to complete the http/2 + * settings before it completes. + * Maybe a more precise way would be to encode that semantics using recieved http/2 + * frames instead of just postponing nrof events? + */ + if (clixon_exit_get() == 1){ + clixon_exit_set(4); + } } #endif break; @@ -1790,7 +1801,7 @@ restconf_sig_term(int arg) * is entered, it will terminate. * However there may be a case of sockets closing rather abruptly for clients */ - clicon_exit_set(); + clixon_exit_set(1); } /*! Usage help routine diff --git a/apps/restconf/restconf_nghttp2.c b/apps/restconf/restconf_nghttp2.c index 65f83ea2..4c296489 100644 --- a/apps/restconf/restconf_nghttp2.c +++ b/apps/restconf/restconf_nghttp2.c @@ -402,7 +402,6 @@ restconf_submit_response(nghttp2_session *session, int i = 0; char valstr[16]; - clicon_debug(1, "%s", __FUNCTION__); data_prd.source.ptr = sd; data_prd.read_callback = restconf_sd_read; if ((hdrs = (nghttp2_nv*)calloc(1+cvec_len(sd->sd_outp_hdrs), sizeof(nghttp2_nv))) == NULL){ @@ -412,6 +411,7 @@ restconf_submit_response(nghttp2_session *session, hdr = &hdrs[i++]; hdr->name = (uint8_t*)":status"; snprintf(valstr, 15, "%u", sd->sd_code); + clicon_debug(1, "%s status %d", __FUNCTION__, sd->sd_code); hdr->value = (uint8_t*)valstr; hdr->namelen = strlen(":status"); hdr->valuelen = strlen(valstr); @@ -421,6 +421,7 @@ restconf_submit_response(nghttp2_session *session, while ((cv = cvec_each(sd->sd_outp_hdrs, cv)) != NULL){ hdr = &hdrs[i++]; hdr->name = (uint8_t*)cv_name_get(cv); + clicon_debug(1, "%s hdr: %s", __FUNCTION__, hdr->name); hdr->value = (uint8_t*)cv_string_get(cv); hdr->namelen = strlen(cv_name_get(cv)); hdr->valuelen = strlen(cv_string_get(cv)); @@ -435,6 +436,7 @@ restconf_submit_response(nghttp2_session *session, } retval = 0; done: + clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); return retval; } @@ -448,6 +450,7 @@ http2_exec(restconf_conn *rc, { int retval = -1; + clicon_debug(1, "%s", __FUNCTION__); if ((sd->sd_path = restconf_uripath(rc->rc_h)) == NULL) goto done; sd->sd_proto = HTTP_2; /* XXX is this necessary? */ @@ -475,9 +478,9 @@ http2_exec(restconf_conn *rc, else { /* 500 Internal server error ? */ } - retval = 0; done: + clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); return retval; } diff --git a/apps/restconf/restconf_stream_fcgi.c b/apps/restconf/restconf_stream_fcgi.c index df165101..680f8680 100644 --- a/apps/restconf/restconf_stream_fcgi.c +++ b/apps/restconf/restconf_stream_fcgi.c @@ -190,7 +190,7 @@ restconf_stream_cb(int s, FCGX_FPrintF(r->out, "SHUTDOWN\r\n"); FCGX_FPrintF(r->out, "\r\n"); FCGX_FFlush(r->out); - clicon_exit_set(); + clixon_exit_set(1); goto done; } if ((ret = clicon_msg_decode(reply, NULL, NULL, &xtop, NULL)) < 0) /* XXX pass yang_spec */ @@ -335,7 +335,7 @@ stream_checkuplink(int s, clicon_debug(1, "%s", __FUNCTION__); if (FCGX_GetError(r->out) != 0){ /* break loop */ clicon_debug(1, "%s FCGX_GetError upstream", __FUNCTION__); - clicon_exit_set(); + clixon_exit_set(1); } return 0; } @@ -351,7 +351,7 @@ stream_timeout(int s, clicon_debug(1, "%s", __FUNCTION__); if (FCGX_GetError(r->out) != 0){ /* break loop */ clicon_debug(1, "%s FCGX_GetError upstream", __FUNCTION__); - clicon_exit_set(); + clixon_exit_set(1); } else{ gettimeofday(&t, NULL); @@ -485,7 +485,7 @@ api_stream(clicon_handle h, clixon_event_unreg_fd(rfcgi->listen_sock, restconf_stream_cb); clixon_event_unreg_timeout(stream_timeout, (void*)req); - clicon_exit_reset(); + clixon_exit_set(0); /* reset */ #ifdef STREAM_FORK FCGX_Finish_r(rfcgi); FCGX_Free(rfcgi, 0); diff --git a/lib/clixon/clixon_event.h b/lib/clixon/clixon_event.h index f4b3d7f7..03f5ecf0 100644 --- a/lib/clixon/clixon_event.h +++ b/lib/clixon/clixon_event.h @@ -43,11 +43,9 @@ /* * Prototypes */ -int clicon_exit_set(void); +int clixon_exit_set(int nr); -int clicon_exit_reset(void); - -int clicon_exit_get(void); +int clixon_exit_get(void); int clicon_sig_child_set(int val); diff --git a/lib/src/clixon_event.c b/lib/src/clixon_event.c index 2ab4ed79..0d61cc47 100644 --- a/lib/src/clixon_event.c +++ b/lib/src/clixon_event.c @@ -101,34 +101,39 @@ static int _clicon_sig_child = 0; static int _clicon_sig_ignore = 0; /*! For signal handlers: instead of doing exit, set a global variable to exit - * Status is then checked in event_loop. + * - zero means dont exit, + * - one means exit, + * - more than one means decrement and make another event loop + * Status is checked in event_loop and decremented by one. + * When it reaches one the exit is made. * Note it maybe would be better to do use on a handle basis, but a signal * handler is global */ int -clicon_exit_set(void) +clixon_exit_set(int nr) { _clicon_exit++; return 0; } -/*! Set exit to 0 - */ -int -clicon_exit_reset(void) -{ - _clicon_exit = 0; - return 0; -} - /*! Get the status of global exit variable, usually set by signal handlers */ int -clicon_exit_get(void) +clixon_exit_get(void) { return _clicon_exit; } +/*! If > 1 decrement exit counter + */ +static int +clixon_exit_decr(void) +{ + if (_clicon_exit > 1) + _clicon_exit--; + return 0; +} + int clicon_sig_child_set(int val) { @@ -340,7 +345,7 @@ clixon_event_loop(clicon_handle h) fd_set fdset; int retval = -1; - while (!clicon_exit_get()){ + while (clixon_exit_get() != 1){ FD_ZERO(&fdset); if (clicon_sig_child_get()){ /* Go through processes and wait for child processes */ @@ -361,8 +366,9 @@ clixon_event_loop(clicon_handle h) } else n = select(FD_SETSIZE, &fdset, NULL, NULL, NULL); - if (clicon_exit_get()) + if (clixon_exit_get() == 1){ break; + } if (n == -1) { if (errno == EINTR){ /* Signals are checked and are in three classes: @@ -376,7 +382,7 @@ clixon_event_loop(clicon_handle h) * (3) Other signals result in an error and return -1. */ clicon_debug(1, "%s select: %s", __FUNCTION__, strerror(errno)); - if (clicon_exit_get()){ + if (clixon_exit_get() == 1){ clicon_err(OE_EVENTS, errno, "select"); retval = 0; } @@ -410,8 +416,9 @@ clixon_event_loop(clicon_handle h) } _ee_unreg = 0; for (e=ee; e; e=e_next){ - if (clicon_exit_get()) + if (clixon_exit_get() == 1){ break; + } e_next = e->e_next; if(e->e_type == EVENT_FD && FD_ISSET(e->e_fd, &fdset)){ clicon_debug(2, "%s: FD_ISSET: %s", __FUNCTION__, e->e_string); @@ -426,8 +433,10 @@ clixon_event_loop(clicon_handle h) } } } + clixon_exit_decr(); /* If exit is set and > 1, decrement it (and exit when 1) */ continue; err: + clicon_debug(1, "%s err", __FUNCTION__); break; } clicon_debug(1, "%s done:%d", __FUNCTION__, retval); diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index d611f198..dd97facf 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -340,13 +340,10 @@ xml_yang_validate_rpc(clicon_handle h, goto done; } rpcprefix = xml_prefix(xrpc); - if (xml2ns(xrpc, rpcprefix, &namespace) < 0){ - fprintf(stderr, "%s ERROR\n", __FUNCTION__); + if (xml2ns(xrpc, rpcprefix, &namespace) < 0) goto done; - } /* Only accept resolved NETCONF base namespace */ if (namespace == NULL || strcmp(namespace, NETCONF_BASE_NAMESPACE) != 0){ - fprintf(stderr, "%s UNKNOWN\n", __FUNCTION__); if (netconf_unknown_namespace_xml(xret, "protocol", rpcprefix, "No appropriate namespace associated with prefix")< 0) goto done; goto fail;