diff --git a/CHANGELOG.md b/CHANGELOG.md index bb3489ea..bbbaead9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,11 +37,27 @@ ## 5.7.0 Expected: May 2022 +### API changes on existing protocol/config features + +Users may have to change how they access the system + +* CLI reconnects to backend if backend restarts with a warning + * Note that edits to the candidate database or locks will be lost + * To force the CLI to exit if backend restarts, undef `PROTO_RESTART_RECONNECT` + * This is an effect of the fix of [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312), the CLI behavior on backend restart is changed. + ### Corrected Bugs +* Fixed: [Broken pipe error seen in client (cli) when backend restarts and CLICON_SOCK is recreated](https://github.com/clicon/clixon/issues/312) * Fixed: [Xpath API do not support filter data by wildcard](https://github.com/clicon/clixon/issues/313) * Fixed: SEGV in cli show yang +### C/CLI-API changes on existing features + +Developers may need to change their code + +* Added `eof` parameter to `clicon_rpc()` and `clicon_rpc1()` and error handling modified + ## 5.6.0 8 March 2022 diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index 3ebe4362..ca515331 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -204,11 +204,23 @@ cli_sig_term(int arg) /*! Setup signal handlers */ -static void +static int cli_signal_init (clicon_handle h) { - cli_signal_block(h); - set_signal(SIGTERM, cli_sig_term, NULL); + int retval = -1; + + cli_signal_block(h); + if (set_signal(SIGTERM, cli_sig_term, NULL) < 0){ + clicon_err(OE_UNIX, errno, "Setting SIGTERM signal"); + goto done; + } + if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){ + clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal"); + goto done; + } + retval = 0; + done: + return retval; } /*! Interactive CLI command loop @@ -653,7 +665,8 @@ main(int argc, clicon_log_string_limit_set(nr); /* Setup signal handlers */ - cli_signal_init(h); + if (cli_signal_init(h) < 0) + goto done; /* Backward compatible mode, do not include keys in cgv-arrays in callbacks. Should be 0 but default is 1 since all legacy apps use 1 diff --git a/apps/cli/cli_plugin.c b/apps/cli/cli_plugin.c index 357e984e..725cd65b 100644 --- a/apps/cli/cli_plugin.c +++ b/apps/cli/cli_plugin.c @@ -606,8 +606,11 @@ clicon_parse(clicon_handle h, cli_output_reset(); if (!cligen_exiting(ch)) { clicon_err_reset(); - if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) + if ((ret = cligen_eval(ch, match_obj, cvv, callbacks)) < 0) { cli_handler_err(stdout); + if (clicon_suberrno == ESHUTDOWN) + goto done; + } } else ret = 0; diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index d21b5bd7..951133da 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -602,6 +602,23 @@ netconf_terminate(clicon_handle h) return 0; } + +/*! Setup signal handlers + */ +static int +netconf_signal_init (clicon_handle h) +{ + int retval = -1; + + if (set_signal(SIGPIPE, SIG_IGN, NULL) < 0){ + clicon_err(OE_UNIX, errno, "Setting DIGPIPE signal"); + goto done; + } + retval = 0; + done: + return retval; +} + static int timeout_fn(int s, void *arg) @@ -804,6 +821,10 @@ main(int argc, if (netconf_module_features(h) < 0) goto done; + /* Setup signal handlers, int particular PIPE that occurs if backend closes / restarts */ + if (netconf_signal_init(h) < 0) + goto done; + /* Initialize plugin module by creating a handle holding plugin and callback lists */ if (clixon_plugin_module_init(h) < 0) goto done; diff --git a/include/clixon_custom.h b/include/clixon_custom.h index fee4acd5..aad8a351 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -149,3 +149,10 @@ * See test/fuzz/http1 */ #undef RESTCONF_HTTP1_UNITTEST + +/*! If backend is restarted, cli and netconf client will retry (once) and reconnect + * Note, if client has locked or had edits in progress, these will be lost + * A warning will be printed + * If not set, client will exit + */ +#define PROTO_RESTART_RECONNECT diff --git a/lib/clixon/clixon_proto.h b/lib/clixon/clixon_proto.h index 6cc7edae..cc402e0f 100644 --- a/lib/clixon/clixon_proto.h +++ b/lib/clixon/clixon_proto.h @@ -79,9 +79,9 @@ int clicon_rpc_connect_inet(clicon_handle h, uint16_t port, int *sock0); -int clicon_rpc(int sock, struct clicon_msg *msg, char **xret); +int clicon_rpc(int sock, struct clicon_msg *msg, char **xret, int *eof); -int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret); +int clicon_rpc1(int sock, cbuf *msgin, cbuf *msgret, int *eof); int clicon_msg_send(int s, struct clicon_msg *msg); diff --git a/lib/src/clixon_client.c b/lib/src/clixon_client.c index 00e54b27..ee2612ae 100644 --- a/lib/src/clixon_client.c +++ b/lib/src/clixon_client.c @@ -151,11 +151,12 @@ clixon_client_lock(int sock, const int lock, const char *db) { - int retval = -1; - cxobj *xret = NULL; - cxobj *xd; - cbuf *msg = NULL; - cbuf *msgret = NULL; + int retval = -1; + cxobj *xret = NULL; + cxobj *xd; + cbuf *msg = NULL; + cbuf *msgret = NULL; + int eof = 0; clicon_debug(1, "%s", __FUNCTION__); if (db == NULL){ @@ -175,8 +176,13 @@ clixon_client_lock(int sock, NETCONF_BASE_NAMESPACE, NETCONF_MESSAGE_ID_ATTR, lock?"":"un", db, lock?"":"un"); - if (clicon_rpc1(sock, msg, msgret) < 0) + if (clicon_rpc1(sock, msg, msgret, &eof) < 0) goto done; + if (eof){ + close(sock); + clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); + goto done; + } if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0) goto done; if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){ @@ -403,13 +409,14 @@ clixon_client_get_xdata(int sock, const char *xpath, cxobj **xdata) { - int retval = -1; - cxobj *xret = NULL; - cxobj *xd; - cbuf *msg = NULL; - cbuf *msgret = NULL; - const char *db = "running"; - cvec *nsc = NULL; + int retval = -1; + cxobj *xret = NULL; + cxobj *xd; + cbuf *msg = NULL; + cbuf *msgret = NULL; + const char *db = "running"; + cvec *nsc = NULL; + int eof = 0; clicon_debug(1, "%s", __FUNCTION__); if ((msg = cbuf_new()) == NULL){ @@ -437,8 +444,13 @@ clixon_client_get_xdata(int sock, cprintf(msg, "/>"); } cprintf(msg, ""); - if (clicon_rpc1(sock, msg, msgret) < 0) + if (clicon_rpc1(sock, msg, msgret, &eof) < 0) goto done; + if (eof){ + close(sock); + clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); + goto done; + } if (clixon_xml_parse_string(cbuf_get(msgret), YB_NONE, NULL, &xret, NULL) < 0) goto done; if ((xd = xpath_first(xret, NULL, "/rpc-reply/rpc-error")) != NULL){ diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 6744a87f..9adfccd1 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -605,42 +605,40 @@ clicon_rpc_connect_inet(clicon_handle h, * * TBD: timeout, interrupt? * retval may be -1 and - * errno set to ENOTCONN which means that socket is now closed probably + * errno set to ENOTCONN/ESHUTDOWN which means that socket is now closed probably * due to remote peer disconnecting. The caller may have to do something,... * * @param[in] sock Socket / file descriptor * @param[in] msg CLICON msg data structure. It has fixed header and variable body. * @param[out] xret Returned data as netconf xml tree. - * @retval 0 OK + * @param[out] eof Set if eof encountered + * @retval 0 OK (check eof) * @retval -1 Error * @see clicon_rpc1 using plain NETCONF XML */ int clicon_rpc(int sock, struct clicon_msg *msg, - char **ret) + char **ret, + int *eof) { int retval = -1; struct clicon_msg *reply = NULL; - int eof; char *data = NULL; if (clicon_msg_send(sock, msg) < 0) goto done; - if (clicon_msg_rcv(sock, &reply, &eof) < 0) + if (clicon_msg_rcv(sock, &reply, eof) < 0) goto done; - if (eof){ - clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); - close(sock); /* assume socket */ - errno = ESHUTDOWN; - goto done; - } + if (*eof) + goto ok; data = reply->op_body; /* assume string */ if (ret && data) if ((*ret = strdup(data)) == NULL){ clicon_err(OE_UNIX, errno, "strdup"); goto done; } + ok: retval = 0; done: if (reply) @@ -655,6 +653,7 @@ clicon_rpc(int sock, * @param[in] sock Socket / file descriptor * @param[in] msgin CLICON msg data structure. It has fixed header and variable body. * @param[out] msgret Returned data as netconf xml tree. + * @param[out] eof Set if eof encountered * @retval 0 OK * @retval -1 Error * @see clicon_rpc using clicon_msg protocol header @@ -662,22 +661,16 @@ clicon_rpc(int sock, int clicon_rpc1(int sock, cbuf *msg, - cbuf *msgret) + cbuf *msgret, + int *eof) { int retval = -1; - int eof; clicon_debug(1, "%s", __FUNCTION__); if (clicon_msg_send1(sock, msg) < 0) goto done; - if (clicon_msg_rcv1(sock, msgret, &eof) < 0) + if (clicon_msg_rcv1(sock, msgret, eof) < 0) goto done; - if (eof){ - clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); - close(sock); - errno = ESHUTDOWN; - goto done; - } retval = 0; done: clicon_debug(1, "%s retval:%d", __FUNCTION__, retval); diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index bd6771ba..00dd01ea 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -127,6 +127,41 @@ clicon_rpc_connect(clicon_handle h, return retval; } +/*! Connect to backend or use cached socket and send RPC + * @param[in] h Clixonhandle + * @param[in] msg Encoded message + * @param[out] xret Returned data as netconf xml tree. + * @param[out] eof Set if eof encountered + */ +static int +clicon_rpc_msg_once(clicon_handle h, + struct clicon_msg *msg, + char **retdata, + int *eof, + int *sp) +{ + int retval = -1; + int s; + + if ((s = clicon_client_socket_get(h)) < 0){ + if (clicon_rpc_connect(h, &s) < 0) + goto done; + clicon_client_socket_set(h, s); + } + if (clicon_rpc(s, msg, retdata, eof) < 0){ + /* 2. check socket shutdown AFTER rpc */ + close(s); + s = -1; + clicon_client_socket_set(h, -1); + goto done; + } + if (sp) + *sp = s; + retval = 0; + done: + return retval; +} + /*! Send internal netconf rpc from client to backend * @param[in] h CLICON handle * @param[in] msg Encoded message. Deallocate with free @@ -145,20 +180,37 @@ clicon_rpc_msg(clicon_handle h, char *retdata = NULL; cxobj *xret = NULL; int s = -1; + int eof = 0; #ifdef RPC_USERNAME_ASSERT assert(strstr(msg->op_body, "username")!=NULL); /* XXX */ #endif clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body); /* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */ - if ((s = clicon_client_socket_get(h)) < 0){ - if (clicon_rpc_connect(h, &s) < 0) - goto done; - clicon_client_socket_set(h, s); - } - if (clicon_rpc(s, msg, &retdata) < 0) + if (clicon_rpc_msg_once(h, msg, &retdata, &eof, NULL) < 0) goto done; - + if (eof){ + /* 2. check socket shutdown AFTER rpc */ + close(s); + s = -1; + clicon_client_socket_set(h, -1); +#ifdef PROTO_RESTART_RECONNECT + if (clicon_rpc_msg_once(h, msg, &retdata, &eof, NULL) < 0) + goto done; + if (eof){ + close(s); + s = -1; + clicon_client_socket_set(h, -1); + clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); + goto done; + } + /* To disable this restart, unset PROTO_RESTART_RECONNECT */ + clicon_log(LOG_WARNING, "The backend was probably restarted and the client has reconnected to the backend. Any locks or candidate edits are lost."); +#else + clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); + goto done; +#endif + } clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata); if (retdata){ @@ -174,10 +226,6 @@ clicon_rpc_msg(clicon_handle h, } retval = 0; done: - if (retval < 0 && s >= 0){ - close(s); - clicon_client_socket_set(h, -1); - } if (retdata) free(retdata); if (xret) @@ -203,6 +251,7 @@ clicon_rpc_msg_persistent(clicon_handle h, char *retdata = NULL; cxobj *xret = NULL; int s = -1; + int eof = 0; if (sock0 == NULL){ clicon_err(OE_NETCONF, EINVAL, "Missing socket pointer"); @@ -213,11 +262,16 @@ clicon_rpc_msg_persistent(clicon_handle h, #endif clicon_debug(1, "%s request:%s", __FUNCTION__, msg->op_body); /* Create a socket and connect to it, either UNIX, IPv4 or IPv6 per config options */ - if (clicon_rpc_connect(h, &s) < 0) + if (clicon_rpc_msg_once(h, msg, &retdata, &eof, &s) < 0) goto done; - if (clicon_rpc(s, msg, &retdata) < 0) + if (eof){ + /* 2. check socket shutdown AFTER rpc */ + close(s); + s = -1; + clicon_client_socket_set(h, -1); + clicon_err(OE_PROTO, ESHUTDOWN, "Unexpected close of CLICON_SOCK. Clixon backend daemon may have crashed."); goto done; - + } clicon_debug(1, "%s retdata:%s", __FUNCTION__, retdata); if (retdata){ diff --git a/util/clixon_util_socket.c b/util/clixon_util_socket.c index 943153d0..c6d027a7 100644 --- a/util/clixon_util_socket.c +++ b/util/clixon_util_socket.c @@ -100,6 +100,7 @@ main(int argc, clicon_handle h; int dbg = 0; int s; + int eof = 0; /* In the startup, logs to stderr & debug flag set later */ clicon_log_init(__FILE__, LOG_INFO, CLICON_LOG_STDERR); @@ -177,7 +178,7 @@ main(int argc, else if (clicon_rpc_connect_inet(h, sockpath, 4535, &s) < 0) goto done; - if (clicon_rpc(s, msg, &retdata) < 0) + if (clicon_rpc(s, msg, &retdata, &eof) < 0) goto done; close(s); fprintf(stdout, "%s\n", retdata);