* Fixed: Initialized session-id to 1 instead of 0 following ietf-netconf.yang

* Clearer debug levels `clicon_debug()`:
  1: Logical debug message
  2: Input and output packets
  3: Message dump in hex, xpath parse trees
This commit is contained in:
Olof hagsand 2023-01-18 13:32:30 +01:00
parent 3428f4d5ff
commit 923b998774
17 changed files with 40 additions and 35 deletions

View file

@ -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

View file

@ -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, "<hello xmlns=\"%s\"><session-id>%u</session-id></hello>",
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 <rpc-reply> messages sent that contained an <rpc-error> */
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

View file

@ -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() &&

View file

@ -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;

View file

@ -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

View file

@ -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

View file

@ -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 */
};

View file

@ -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.

View file

@ -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
*/

View file

@ -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:

View file

@ -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);

View file

@ -103,7 +103,7 @@ cat <<EOF > $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"

View file

@ -99,7 +99,7 @@ cat <<EOF > $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"

View file

@ -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"

View file

@ -65,7 +65,7 @@ cat <<EOF > $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

View file

@ -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

View file

@ -88,7 +88,7 @@ cat <<EOF > $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"