From e9c5287c366fbb60c92e440471b548ac6b281543 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 1 Nov 2023 13:02:32 +0100 Subject: [PATCH] Debug: improved debug level 2 with socket-description, also for notification --- apps/backend/backend_client.c | 100 +++++++++++++++---------- apps/backend/backend_commit.c | 10 ++- apps/backend/clixon_backend_client.h | 1 + apps/backend/clixon_backend_handle.c | 6 +- apps/restconf/restconf_native.c | 1 - example/main/autocli.xml | 14 ++++ example/main/restconf.xml | 7 ++ lib/src/clixon_proto.c | 2 +- test/test_transaction_restart.sh | 2 +- yang/clixon/clixon-lib@2023-11-01.yang | 2 +- 10 files changed, 97 insertions(+), 48 deletions(-) create mode 100644 example/main/autocli.xml create mode 100644 example/main/restconf.xml diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index ab8d0b6cb..0b4d8df1d 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -89,12 +89,51 @@ ce_find_byid(struct client_entry *ce_list, return NULL; } +/*! Construct a client string description from client_entry information for logging + * + * @param[in] ce Client entry struct + * @param[out] cbp Cligen buffer, deallocate with cbuf_free + * @retval 0 OK + * @retval -1 Error + */ +static int +ce_client_descr(struct client_entry *ce, + cbuf **cbp) +{ + int retval = -1; + cbuf *cb = NULL; + char *id = NULL; + + if (ce == NULL || cbp == NULL){ + clicon_err(OE_UNIX, EINVAL, "ce or cbp is NULL"); + goto done; + } + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + if (ce->ce_transport){ + if (nodeid_split(ce->ce_transport, NULL, &id) < 0) + goto done; + cprintf(cb, "%s:", id); + } + cprintf(cb, "%u", ce->ce_id); + *cbp = cb; + retval = 0; + done: + if (id) + free(id); + return retval; +} + /*! Stream callback for netconf stream notification (RFC 5277) * * @param[in] h Clixon handle * @param[in] op 0:event, 1:rm * @param[in] event Event as XML * @param[in] arg Extra argument provided in stream_ss_add + * @retval 0 OK + * @retval -1 Error * @see stream_ss_add */ int @@ -103,7 +142,9 @@ ce_event_cb(clicon_handle h, cxobj *event, void *arg) { + int retval = -1; struct client_entry *ce = (struct client_entry *)arg; + cbuf *cbce = NULL; clixon_debug(CLIXON_DBG_DEFAULT, "%s op:%d", __FUNCTION__, op); switch (op){ @@ -113,7 +154,9 @@ ce_event_cb(clicon_handle h, backend_client_rm(h, ce); break; default: - if (send_msg_notify_xml(h, ce->ce_s, ce->ce_source_host, event) < 0){ + if (ce_client_descr(ce, &cbce) < 0) + goto done; + if (send_msg_notify_xml(h, ce->ce_s, cbuf_get(cbce), event) < 0){ if (errno == ECONNRESET || errno == EPIPE){ clicon_log(LOG_WARNING, "client %d reset", ce->ce_nr); } @@ -123,47 +166,14 @@ ce_event_cb(clicon_handle h, ce->ce_out_notifications++; netconf_monitoring_counter_inc(h, "out-notifications"); } - return 0; -} - -/*! Construct a client string from client_entry information for logging - * - * @param[in] ce Client entry struct - * @param[out] cbp Cligen buffer, deallocate with cbuf_free - * @retval 0 OK - * @retval -1 Error - */ -static int -ce_client_string(struct client_entry *ce, - cbuf **cbp) -{ - int retval = -1; - cbuf *cb = NULL; - char *id = NULL; - - if (ce == NULL || cbp == NULL){ - clicon_err(OE_UNIX, EINVAL, "ce or cbp is NULL"); - goto done; - } - if ((cb = cbuf_new()) == NULL){ - clicon_err(OE_UNIX, errno, "cbuf_new"); - goto done; - - } - if (ce->ce_transport){ - if (nodeid_split(ce->ce_transport, NULL, &id) < 0) - goto done; - cprintf(cb, "%s", id); - } - cprintf(cb, "%u", ce->ce_id); - *cbp = cb; retval = 0; done: - if (id) - free(id); + if (cbce) + cbuf_free(cbce); return retval; } + /*! Unlock all db:s of a client and call user unlock calback * * @see xmldb_unlock_all unlocks, but does not call user callbacks which is a backend thing @@ -290,6 +300,7 @@ backend_monitoring_state_get(clicon_handle h, * @param[in] ce Client handle * @retval 0 Ok * @retval -1 Error (fatal) + * @see backend_client_add for adding * @see backend_client_delete for actual deallocation of client entry struct */ int @@ -1628,7 +1639,18 @@ from_client_msg(clicon_handle h, * 3. Its a create-subscription message that uses a separate socket(=client) */ if (op_id != 0 && ce->ce_id != op_id && strcmp(rpcname, "create-subscription")){ + client_entry *ce0; + clixon_debug(CLIXON_DBG_DEFAULT, "%s Warning: incoming session-id:%u does not match ce_id:%u on socket: %d", __FUNCTION__, op_id, ce->ce_id, ce->ce_s); + /* Copy transport from orig client-entry */ + if (ce->ce_transport == NULL && + (ce0 = ce_find_byid(backend_client_list(h), op_id)) != NULL && + ce0->ce_transport){ + if ((ce->ce_transport = strdup(ce0->ce_transport)) == NULL){ + clicon_err(OE_UNIX, errno, "strdup"); + goto done; + } + } } /* Note that this validation is also made in xml_yang_validate_rpc, but not for hello */ @@ -1775,7 +1797,7 @@ from_client_msg(clicon_handle h, // XXX clixon_debug(CLIXON_DBG_MSG, "Reply:%s", cbuf_get(cbret)); /* XXX problem here is that cbret has not been parsed so may contain parse errors */ - if (ce_client_string(ce, &cbce) < 0) + if (ce_client_descr(ce, &cbce) < 0) goto done; if (send_msg_reply(ce->ce_s, cbuf_get(cbce), cbuf_get(cbret), cbuf_len(cbret)+1) < 0){ switch (errno){ @@ -1843,7 +1865,7 @@ from_client(int s, clicon_err(OE_NETCONF, EINVAL, "Internal error: s != ce->ce_s"); goto done; } - if (ce_client_string(ce, &cbce) < 0) + if (ce_client_descr(ce, &cbce) < 0) goto done; if (clicon_msg_rcv(ce->ce_s, cbuf_get(cbce), 0, &msg, &eof) < 0) goto done; diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index da1eb2a86..b1d8b8d07 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -828,7 +828,9 @@ from_client_commit(clicon_handle h, goto done; goto ok; } - if (ret == 1) + if (ret == 0) + clixon_debug(CLIXON_DBG_DEFAULT, "Commit candidate failed"); + else cprintf(cbret, "", NETCONF_BASE_NAMESPACE); ok: retval = 0; @@ -929,13 +931,13 @@ from_client_validate(clicon_handle h, /*! Restart specific backend plugins without full backend restart * - * Note, depending on plugin callbacks, there may be other dependencies which may make this + * @note, depending on plugin callbacks, there may be other dependencies which may make this * difficult in the general case. */ int -from_client_restart_one(clicon_handle h, +from_client_restart_one(clicon_handle h, clixon_plugin_t *cp, - cbuf *cbret) + cbuf *cbret) { int retval = -1; char *db = "tmp"; diff --git a/apps/backend/clixon_backend_client.h b/apps/backend/clixon_backend_client.h index c972afe77..bc31befae 100644 --- a/apps/backend/clixon_backend_client.h +++ b/apps/backend/clixon_backend_client.h @@ -53,6 +53,7 @@ * client socket and a separate notification client socket. * But they are the same session. * But the clixon client-entry do not differentiate + * @see backend_client_add */ struct client_entry{ struct client_entry *ce_next; /* The clients linked list */ diff --git a/apps/backend/clixon_backend_handle.c b/apps/backend/clixon_backend_handle.c index cdcccb62f..65f41c8a7 100644 --- a/apps/backend/clixon_backend_handle.c +++ b/apps/backend/clixon_backend_handle.c @@ -101,7 +101,11 @@ struct backend_handle { clicon_handle backend_handle_init(void) { - return clicon_handle_init0(sizeof(struct backend_handle)); + struct backend_handle *bh; + + bh = (struct backend_handle *)clicon_handle_init0(sizeof(struct backend_handle)); + bh->bh_ce_nr = 1; /* To align with session-id */ + return (clicon_handle)bh; } /*! Deallocates a backend handle, including all client structs diff --git a/apps/restconf/restconf_native.c b/apps/restconf/restconf_native.c index 373c7e71a..e70f53b4d 100644 --- a/apps/restconf/restconf_native.c +++ b/apps/restconf/restconf_native.c @@ -1377,7 +1377,6 @@ restconf_ssl_accept_client(clicon_handle h, */ if (restconf_auth_type_get(h) == CLIXON_AUTH_CLIENT_CERTIFICATE){ X509 *peercert; - // XXX SSL_get1_peer_certificate(ssl) #if OPENSSL_VERSION_NUMBER < 0x30000000L if ((peercert = SSL_get_peer_certificate(rc->rc_ssl)) != NULL){ X509_free(peercert); diff --git a/example/main/autocli.xml b/example/main/autocli.xml new file mode 100644 index 000000000..6d92bd0ac --- /dev/null +++ b/example/main/autocli.xml @@ -0,0 +1,14 @@ + + + false + kw-nokey + false + list container + true + + include clixon-example + clixon-example + enable + + + diff --git a/example/main/restconf.xml b/example/main/restconf.xml new file mode 100644 index 000000000..ac878aae6 --- /dev/null +++ b/example/main/restconf.xml @@ -0,0 +1,7 @@ + + + true + none + default
0.0.0.0
80false + + diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 3e5d404e3..8d67cd7df 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -812,7 +812,7 @@ send_msg_reply(int s, static int send_msg_notify(int s, const char *descr, - char *event) + char *event) { int retval = -1; struct clicon_msg *msg = NULL; diff --git a/test/test_transaction_restart.sh b/test/test_transaction_restart.sh index 0781bc97a..1f3281e80 100755 --- a/test/test_transaction_restart.sh +++ b/test/test_transaction_restart.sh @@ -128,7 +128,7 @@ for op in begin validate complete commit commit_done end; do done # Negative test: restart a plugin that does not exist -new "Send restart to nonexistatn plugin expect fail" +new "Send restart to nonexisting plugin expect fail" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "xxx" "" "applicationbad-elementpluginerrorNo such plugin" if [ $BE -ne 0 ]; then diff --git a/yang/clixon/clixon-lib@2023-11-01.yang b/yang/clixon/clixon-lib@2023-11-01.yang index cadc51ca8..620bfedc0 100644 --- a/yang/clixon/clixon-lib@2023-11-01.yang +++ b/yang/clixon/clixon-lib@2023-11-01.yang @@ -185,7 +185,7 @@ module clixon-lib { } identity restconf { description - "RESTCONF either as HTTP/1 or /2, TLS or not, reverese proxy (eg fcgi/nginx) or native"; + "RESTCONF either as HTTP/1 or /2, TLS or not, reverse proxy (eg fcgi/nginx) or native"; base ncm:transport; } identity cli {