diff --git a/CHANGELOG.md b/CHANGELOG.md index c9cb01e2..21ae2e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,11 +84,16 @@ Developers may need to change their code ### Minor features +* Clearer debug levels `clicon_debug()`: + * 1: Logical debug message + * 2: Input and output packets + * 3: Message dump in hex, xpath parse trees * Added `ISO/IEC 10646` encodings to XML parser: `&#[0-9]+;` and `&#[0-9a-fA-F]+;` * Added `CLIXON_CLIENT_SSH` to client API to communicate remotely via SSH netconf sub-system ### Corrected Bugs +* Fixed: Initialized session-id to 1 instead of 0 following ietf-netconf.yang * Fixed: [snmpwalk doesn't show properly SNMP boolean values which equal false](https://github.com/clicon/clixon/issues/400) * Fixed: yang-library: Remove revision if empty instead of sending empty revision * This was a change from RFC 7895 to RFC 8525 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index ec150d25..0c3bdf50 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1476,13 +1476,8 @@ from_client_hello(clicon_handle h, cbuf *cbret) { int retval = -1; - uint32_t id; char *val; - if (clicon_session_id_get(h, &id) < 0){ - clicon_err(OE_NETCONF, ENOENT, "session_id not set"); - goto done; - } if ((val = xml_find_type_value(x, "cl", "transport", CX_ATTR)) != NULL){ if ((ce->ce_transport = strdup(val)) == NULL){ clicon_err(OE_UNIX, errno, "strdup"); @@ -1495,16 +1490,15 @@ from_client_hello(clicon_handle h, goto done; } } - id++; - clicon_session_id_set(h, id); cprintf(cbret, "%u", - NETCONF_BASE_NAMESPACE, id); + NETCONF_BASE_NAMESPACE, ce->ce_id); retval = 0; done: return retval; } -/*! An internal clicon message has arrived from a client. Receive and dispatch. +/*! An internal clixon NETCONF message has arrived from a local client. Receive and dispatch. + * * @param[in] h Clicon handle * @param[in] ce Client entry (from) * @param[in] msg Incoming message @@ -1531,7 +1525,7 @@ from_client_msg(clicon_handle h, yang_stmt *ymod; cxobj *xnacm = NULL; cxobj *xret = NULL; - uint32_t id; + uint32_t op_id; /* session number from internal NETCONF protocol */ enum nacm_credentials_t creds; char *rpcname; char *rpcprefix; @@ -1550,7 +1544,7 @@ from_client_msg(clicon_handle h, /* Decode msg from client -> xml top (ct) and session id * Bind is a part of the decode function */ - if ((ret = clicon_msg_decode(msg, yspec, &id, &xt, &xret)) < 0){ + if ((ret = clicon_msg_decode(msg, yspec, &op_id, &xt, &xret)) < 0){ if (netconf_malformed_message(cbret, "XML parse error") < 0) goto done; goto reply; @@ -1560,6 +1554,14 @@ from_client_msg(clicon_handle h, goto done; goto reply; } + /* Sanity check: + * op_id from internal message can be out-of-sync from client's sessions-id for the following reasons: + * 1. Its a hello when the client starts with op_id=0 to get its proper id on hello reply + * 2. The backend has restarted and the client uses an old op_id + */ + if (op_id != 0 && ce->ce_id != op_id){ + clicon_debug(1, "%s out-of-order sequence op-id:%u ce_id:%u", __FUNCTION__, op_id, ce->ce_id); + } /* Check for empty frame (no mesaages), return empty message, not clear from RFC what to do */ if (xml_child_nr_type(xt, CX_ELMNT) == 0){ if (netconf_malformed_message(cbret, "Empty message in netconf rpc frame")< 0) @@ -1616,8 +1618,6 @@ from_client_msg(clicon_handle h, ce->ce_out_rpc_errors++; /* Number of messages sent that contained an */ goto reply; } - ce->ce_id = id; - /* As a side-effect, this expands xt with default values according to "report-all" * This may not be correct, the RFC does not mention expanding default values for * input RPC diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 7de8ac41..d23ecfc0 100644 --- a/apps/backend/backend_main.c +++ b/apps/backend/backend_main.c @@ -1053,7 +1053,7 @@ main(int argc, goto done; /* Start session-id for clients */ - clicon_session_id_set(h, 0); + clicon_session_id_set(h, 1); #if 0 /* debug */ /* Enable this to get prints of datastore and session status */ if (0 && clicon_debug_get() && diff --git a/apps/backend/clixon_backend_handle.c b/apps/backend/clixon_backend_handle.c index 2d738684..cb94559e 100644 --- a/apps/backend/clixon_backend_handle.c +++ b/apps/backend/clixon_backend_handle.c @@ -146,6 +146,11 @@ backend_client_add(clicon_handle h, memcpy(&ce->ce_addr, addr, sizeof(*addr)); ce->ce_next = bh->bh_ce_list; ce->ce_handle = h; + if (clicon_session_id_get(h, &ce->ce_id) < 0){ + clicon_err(OE_NETCONF, ENOENT, "session_id not set"); + return NULL; + } + clicon_session_id_set(h, ce->ce_id + 1); gettimeofday(&ce->ce_time, NULL); bh->bh_ce_list = ce; return ce; diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index f7abd04e..e65d662d 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -987,10 +987,6 @@ main(int argc, /* Call start function is all plugins before we go interactive */ if (clixon_plugin_start_all(h) < 0) goto done; -#if 1 - /* XXX get session id from backend hello */ - clicon_session_id_set(h, getpid()); -#endif /* Send hello request to backend to get session-id back * This is done once at the beginning of the session and then this is diff --git a/apps/snmp/snmp_main.c b/apps/snmp/snmp_main.c index 5a4ccd43..76a1b751 100644 --- a/apps/snmp/snmp_main.c +++ b/apps/snmp/snmp_main.c @@ -517,9 +517,6 @@ main(int argc, if (dbg) clicon_option_dump(h, dbg); - /* Get session id from backend hello */ - clicon_session_id_set(h, getpid()); - /* Send hello request to backend to get session-id back * This is done once at the beginning of the session and then this is * used by the client, even though new TCP sessions are created for diff --git a/lib/clixon/clixon_proto.h b/lib/clixon/clixon_proto.h index 15c69bf4..74f92f40 100644 --- a/lib/clixon/clixon_proto.h +++ b/lib/clixon/clixon_proto.h @@ -54,7 +54,7 @@ enum format_enum{ /* Protocol message header */ struct clicon_msg { uint32_t op_len; /* length of whole message: body+header, network byte order. */ - uint32_t op_id; /* session-id. network byte order. */ + uint32_t op_id; /* session-id. network byte order. 1..max(u32), can be zero in client hello */ char op_body[0]; /* rest of message, actual data */ }; diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c index ac5c5527..daaca13c 100644 --- a/lib/src/clixon_data.c +++ b/lib/src/clixon_data.c @@ -840,7 +840,7 @@ clicon_session_id_del(clicon_handle h) /*! Set session id * @param[in] h Clicon handle - * @param[in] id Session id + * @param[in] id Session id (in range 1..max uint32) * @retval 0 OK * @retval -1 Error * Session-ids survive TCP sessions that are created for each message sent to the backend. diff --git a/lib/src/clixon_log.c b/lib/src/clixon_log.c index fcb0bdb6..5db0dbed 100644 --- a/lib/src/clixon_log.c +++ b/lib/src/clixon_log.c @@ -367,10 +367,12 @@ clicon_debug_get(void) * print message if level >= dbglevel. * The message is sent to clicon_log. EIther to syslog, stderr or both, depending on * clicon_log_init() setting + * The dbgdebug level strategy is: + * 1: Logical debug message + * 2: Input and output packets + * 3: Message dump in hex, xpath parse trees * - * @param[in] dbglevel 0 always called (dont do this: not really a dbg message) - * 1 default level if passed -D - * 2.. Higher debug levels + * @param[in] dbglevel 0: No debug, 1-3: increasing levels of debug * @param[in] format Message to print as argv. * @see clicon_debug_xml Specialization for XML tree */ diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 6efef361..25ccf91c 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -417,7 +417,7 @@ clicon_msg_rcv(int s, clicon_err(OE_CFG, errno, "body too short"); goto done; } - if (clicon_debug_get() > 1) + if (clicon_debug_get() > 2) msg_dump(*msg); retval = 0; done: diff --git a/lib/src/clixon_xpath.c b/lib/src/clixon_xpath.c index f45dc0b1..fd2e7927 100644 --- a/lib/src/clixon_xpath.c +++ b/lib/src/clixon_xpath.c @@ -550,13 +550,13 @@ xpath_parse(const char *xpath, xpath_scan_exit(&xpy); goto done; } - if (clicon_debug_get() > 1){ + if (clicon_debug_get() > 2){ if ((cb = cbuf_new()) == NULL){ clicon_err(OE_XML, errno, "cbuf_new"); goto done; } xpath_tree_print_cb(cb, xpy.xpy_top); - clicon_debug(2, "xpath parse tree:\n%s", cbuf_get(cb)); + clicon_debug(3, "xpath parse tree:\n%s", cbuf_get(cb)); } xpath_parse_exit(&xpy); xpath_scan_exit(&xpy); diff --git a/test/test_snmp_entity.sh b/test/test_snmp_entity.sh index cf806d91..49755114 100755 --- a/test/test_snmp_entity.sh +++ b/test/test_snmp_entity.sh @@ -103,7 +103,7 @@ cat < $fstate EOF function testinit(){ - new "test params: -f $cfg -- -sS $fstate" + new "test params: -s init -f $cfg -- -sS $fstate" if [ $BE -ne 0 ]; then # Kill old backend and start a new one new "kill old backend" diff --git a/test/test_snmp_get.sh b/test/test_snmp_get.sh index e82ccb30..d6463fea 100755 --- a/test/test_snmp_get.sh +++ b/test/test_snmp_get.sh @@ -99,7 +99,7 @@ cat < $fstate EOF function testinit(){ - new "test params: -f $cfg -- -sS $fstate" + new "test params: -s init -f $cfg -- -sS $fstate" if [ $BE -ne 0 ]; then # Kill old backend and start a new one new "kill old backend" diff --git a/test/test_snmp_ifmib.sh b/test/test_snmp_ifmib.sh index 5dc419b6..1b574cbc 100755 --- a/test/test_snmp_ifmib.sh +++ b/test/test_snmp_ifmib.sh @@ -163,7 +163,7 @@ EOF # IF-MIB::ifSpecific.1 = OID: SNMPv2-SMI::zeroDotZero function testinit(){ - new "test params: -f $cfg -- -sS $fstate" + new "test params: -s init -f $cfg -- -sS $fstate" if [ $BE -ne 0 ]; then # Kill old backend and start a new one new "kill old backend" diff --git a/test/test_snmp_rowstatus.sh b/test/test_snmp_rowstatus.sh index 7a3995b3..eb9382d4 100755 --- a/test/test_snmp_rowstatus.sh +++ b/test/test_snmp_rowstatus.sh @@ -65,7 +65,7 @@ cat < $dir/startup_db EOF function testinit(){ - new "test params: -f $cfg" + new "test params: -s startup -f $cfg" if [ $BE -ne 0 ]; then # Kill old backend and start a new one diff --git a/test/test_snmp_set.sh b/test/test_snmp_set.sh index d2de145b..e2d2b3d6 100755 --- a/test/test_snmp_set.sh +++ b/test/test_snmp_set.sh @@ -115,7 +115,7 @@ EOF fi function testinit(){ - new "test params: -f $cfg" + new "test params: -s startup -f $cfg" if [ $BE -ne 0 ]; then # Kill old backend and start a new one diff --git a/test/test_snmp_system.sh b/test/test_snmp_system.sh index 98bd59f3..19c2e156 100755 --- a/test/test_snmp_system.sh +++ b/test/test_snmp_system.sh @@ -88,7 +88,7 @@ cat < $fstate EOF function testinit(){ - new "test params: -f $cfg -- -sS $fstate" + new "test params: -s init -f $cfg -- -sS $fstate" if [ $BE -ne 0 ]; then # Kill old backend and start a new one new "kill old backend"