netconf client was limited to 8K byte messages. Now limit is 2^32

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Clixon CHANGELOG
 
 ## 3.3.3 Upcoming
+* netconf client was limited to 8K byte messages. Now limit is 2^32, (but needs scaling improvement)
 	
 ## 3.3.2 Aug 27 2017
 

apps/netconf/netconf_main.c

@@ -188,35 +188,37 @@ netconf_input_cb(int   s,
 	return retval;
     }
     memset(buf, 0, sizeof(buf));
-    if ((len = read(s, buf, sizeof(buf))) < 0){
+    while (1){
-	if (errno == ECONNRESET)
+	if ((len = read(s, buf, sizeof(buf))) < 0){
-	    len = 0; /* emulate EOF */
+	    if (errno == ECONNRESET)
-	else{
+		len = 0; /* emulate EOF */
-	    clicon_log(LOG_ERR, "%s: read: %s", __FUNCTION__, strerror(errno));
+	    else{
-	    goto done;
+		clicon_log(LOG_ERR, "%s: read: %s", __FUNCTION__, strerror(errno));
-	}
-    } /* read */
-    if (len == 0){ 	/* EOF */
-	cc_closed++;
-	close(s);
-	retval = 0;
-	goto done;
-    }
-
-    for (i=0; i<len; i++){
-	if (buf[i] == 0)
-	    continue; /* Skip NULL chars (eg from terminals) */
-	cprintf(cb, "%c", buf[i]);
-	if (detect_endtag("]]>]]>",
-			  buf[i],
-			  &xml_state)) {
-	    /* OK, we have an xml string from a client */
-	    if (process_incoming_packet(h, cb) < 0){
 		goto done;
 	    }
-	    if (cc_closed)
+	} /* read */
-		break;
+	if (len == 0){ 	/* EOF */
-	    cbuf_reset(cb);
+	    cc_closed++;
+	    close(s);
+	    retval = 0;
+	    goto done;
+	}
+
+	for (i=0; i<len; i++){
+	    if (buf[i] == 0)
+		continue; /* Skip NULL chars (eg from terminals) */
+	    cprintf(cb, "%c", buf[i]);
+	    if (detect_endtag("]]>]]>",
+			      buf[i],
+			      &xml_state)) {
+		/* OK, we have an xml string from a client */
+		if (process_incoming_packet(h, cb) < 0){
+		    goto done;
+		}
+		if (cc_closed)
+		    break;
+		cbuf_reset(cb);
+	    }
 	}
     }
     retval = 0;

apps/netconf/netconf_rpc.c

@@ -312,9 +312,7 @@ netconf_edit_config(clicon_handle h,
     cxobj              *x;
     cxobj              *xfilter;
     char               *ftype = NULL;
-    cxobj              *xcc;      /* child of config */
     char               *target;  /* db */
-    cbuf               *cbxml = NULL;
 
     /* must have target, and it should be candidate */
     if ((target = netconf_get_target(xn, "target")) == NULL ||
@@ -371,19 +369,12 @@ netconf_edit_config(clicon_handle h,
 	    goto ok;
 	}
 #endif
-	if ((cbxml = cbuf_new()) == NULL)
-	    goto done;
-	if ((xcc = xml_child_i(xc, 0)) != NULL)
-	    if (clicon_xml2cbuf(cbxml, xcc, 0, 0) < 0)
-		goto done;
 	if (clicon_rpc_netconf_xml(h, xml_parent(xn), xret, NULL) < 0)
 	     goto done;	
     }
  ok:
     retval = 0;
  done:
-    if (cbxml)
-	cbuf_free(cbxml);
     return retval;
 }
 

doc/FAQ.md

@@ -44,6 +44,15 @@ The example:
 	 sudo make install
 ```
 
+## Do I need to setup anything?
+
+The config demon requires a valid group to create a server UNIX socket.
+Define a valid CLICON_SOCK_GROUP in the config file or via the -g option
+or create the group and add the user to it. The default group is 'clicon'.
+On linux:
+  sudo groupadd clicon
+  sudo usermod -a -G clicon user
+
 ## What about reference documentation?
 Clixon uses Doxygen for reference documentation.
 Build using 'make doc' and aim your browser at doc/html/index.html or
@@ -103,11 +112,11 @@ Example:
 However, more useful is to run clixon_netconf as an SSH
 subsystem. Register the subsystem in /etc/sshd_config:
 ```
-	Subsystem netconf /usr/local/bin/clixon_netconf
+	Subsystem netconf /usr/local/bin/clixon_netconf -f /usr/local/etc/routing.conf
 ```
 and then invoke it from a client using
 ```
-	ssh -s netconf <host>
+	ssh -s <host> netconf
 ```
 
 ## How do I use restconf?

include/clixon_custom.h

@@ -43,3 +43,7 @@
 int strverscmp (__const char *__s1, __const char *__s2);
 #endif
 
+/* Replace the current clixon.conf.cpp.cpp options with XML file with Yang
+ * Why not use the config mechanisms that CLixon uses for its own config-file?
+ */
+#define CONFIGFILE_XML 0

lib/clixon/clixon_proto.h

@@ -51,7 +51,7 @@ enum format_enum{
 
 /* Protocol message header */
 struct clicon_msg {
-    uint16_t    op_len;      /* length of message. */
+    uint32_t    op_len;      /* length of message. network byte order. */
     char        op_body[0];  /* rest of message, actual data */
 };
 
@@ -85,7 +85,7 @@ int clicon_msg_rcv(int s, struct clicon_msg **msg, int *eof);
 
 int send_msg_notify(int s, int level, char *event);
 
-int send_msg_reply(int s, char *data, uint16_t datalen);
+int send_msg_reply(int s, char *data, uint32_t datalen);
 
 int detect_endtag(char *tag, char  ch, int  *state);
 

lib/src/clixon_options.c

@@ -65,7 +65,9 @@
 #include "clixon_yang.h"
 #include "clixon_plugin.h"
 #include "clixon_options.h"
-
+#if  CONFIGFILE_XML
+#include "clixon_xml.h"
+#endif
 /*
  * clicon_option_dump
  * Print registry on file. For debugging.
@@ -107,28 +109,56 @@ static int
 clicon_option_readfile(clicon_hash_t *copt, const char *filename)
 {
     struct stat st;
-    char opt[1024], val[1024];
+#if !CONFIGFILE_XML
-    char line[1024], *cp;
+    char        opt[1024];
-    FILE *f;
+    char        val[1024];
-    int retval = -1;
+    char        line[1024];
+    char       *cp;
+#endif
+    FILE       *f = NULL;
+    int         retval = -1;
 
     if (filename == NULL || !strlen(filename)){
 	clicon_err(OE_UNIX, 0, "Not specified");
-	return -1;
+	goto done;
     }
     if (stat(filename, &st) < 0){
 	clicon_err(OE_UNIX, errno, "%s", filename);
-	return -1;
+	goto done;
     }
     if (!S_ISREG(st.st_mode)){
 	clicon_err(OE_UNIX, 0, "%s is not a regular file", filename);
-	return -1;
+	goto done;
     }
     if ((f = fopen(filename, "r")) == NULL) {
 	clicon_err(OE_UNIX, errno, "configure file: %s", filename);
 	return -1;
     }
     clicon_debug(2, "Reading config file %s", __FUNCTION__, filename);
+#if CONFIGFILE_XML
+    {
+	int    fd = fileno(f);
+	cxobj *xt = NULL;
+	cxobj *x = NULL;
+	if (clicon_xml_parse_file(fd, &xt, "</clicon>") < 0)
+	    goto done;
+
+	// yspec should be clicon/yang
+	//	if (xml_apply(xt, CX_ELMNT, xml_spec_populate, yspec) < 0)
+	//goto done;	
+
+	while ((x = xml_child_each(xt, x, CX_ELMNT)) != NULL) {
+	    fprintf(stderr, "%s\n", xml_name(x));
+#if 0	    
+	    if ((hash_add(copt, 
+			  opt,
+			  val,
+			  strlen(val)+1)) == NULL)
+		goto done;
+#endif
+	}
+    }
+#else
     while (fgets(line, sizeof(line), f)) {
 	if ((cp = strchr(line, '\n')) != NULL) /* strip last \n */
 	    *cp = '\0';
@@ -141,11 +171,13 @@ clicon_option_readfile(clicon_hash_t *copt, const char *filename)
 		      opt,
 		      val,
 		      strlen(val)+1)) == NULL)
-	    goto catch;
+	    goto done;
     }
+#endif
     retval = 0;
-  catch:
+  done:
-    fclose(f);
+    if (f)
+	fclose(f);
     return retval;
 }
 
@@ -217,7 +249,7 @@ clicon_option_sanity(clicon_hash_t *copt)
 	goto done;
     }
     if (!hash_lookup(copt, "CLICON_BACKEND_DIR")){
-	clicon_err(OE_UNIX, 0, "CLICON_BACKEND_PIDFILE not defined in config file");
+	clicon_err(OE_UNIX, 0, "CLICON_BACKEND_DIR not defined in config file");
 	goto done;
     }
     if (!hash_lookup(copt, "CLICON_NETCONF_DIR")){

lib/src/clixon_proto.c

@@ -130,8 +130,8 @@ struct clicon_msg *
 clicon_msg_encode(char *format, ...)
 {
     va_list            args;
-    int                xmllen;
+    uint32_t           xmllen;
-    int                len;
+    uint32_t           len;
     struct clicon_msg *msg = NULL;
     int                hdrlen = sizeof(*msg);
 
@@ -146,7 +146,7 @@ clicon_msg_encode(char *format, ...)
     }
     memset(msg, 0, len);
     /* hdr */
-    msg->op_len = htons(len);
+    msg->op_len = htonl(len);
 
     /* body */
     va_start(args, format);
@@ -267,7 +267,7 @@ msg_dump(struct clicon_msg *msg)
     
     memset(buf2, 0, sizeof(buf2));
     snprintf(buf2, sizeof(buf2), "%s:", __FUNCTION__);
-    for (i=0; i<ntohs(msg->op_len); i++){
+    for (i=0; i<ntohl(msg->op_len); i++){
 	snprintf(buf, sizeof(buf), "%s%02x", buf2, ((char*)msg)[i]&0xff);
 	if ((i+1)%32==0){
 	    clicon_debug(2, buf);
@@ -294,13 +294,13 @@ clicon_msg_send(int                s,
     int retval = -1;
 
     clicon_debug(2, "%s: send msg len=%d", 
-		 __FUNCTION__, ntohs(msg->op_len));
+		 __FUNCTION__, ntohl(msg->op_len));
     if (debug > 2)
 	msg_dump(msg);
     if (atomicio((ssize_t (*)(int, void *, size_t))write, 
-		 s, msg, ntohs(msg->op_len)) < 0){
+		 s, msg, ntohl(msg->op_len)) < 0){
 	clicon_err(OE_CFG, errno, "%s", __FUNCTION__);
-	clicon_log(LOG_WARNING, "%s: write: %s len:%d msg:%s", __FUNCTION__,
+	clicon_log(LOG_WARNING, "%s: write: %s len:%u msg:%s", __FUNCTION__,
 		   strerror(errno), ntohs(msg->op_len), msg->op_body);
 	goto done;
     }
@@ -309,7 +309,6 @@ clicon_msg_send(int                s,
     return retval;
 }
 
-
 /*! Receive a CLICON message
  *
  * XXX: timeout? and signals?
@@ -333,9 +332,9 @@ clicon_msg_rcv(int                s,
     int       retval = -1;
     struct clicon_msg hdr;
     int       hlen;
-    int       len2;
+    uint32_t  len2;
     sigfn_t   oldhandler;
-    uint16_t  mlen;
+    uint32_t  mlen;
 
     *eof = 0;
     if (0)
@@ -354,7 +353,7 @@ clicon_msg_rcv(int                s,
 	clicon_err(OE_CFG, errno, "%s: header too short (%d)", __FUNCTION__, hlen);
 	goto done;
     }
-    mlen = ntohs(hdr.op_len);
+    mlen = ntohl(hdr.op_len);
     clicon_debug(2, "%s: rcv msg len=%d",  
 		 __FUNCTION__, mlen);
     if ((*msg = (struct clicon_msg *)malloc(mlen)) == NULL){
@@ -533,17 +532,17 @@ clicon_rpc(int                   s,
 int 
 send_msg_reply(int      s, 
 	       char    *data, 
-	       uint16_t datalen)
+	       uint32_t datalen)
 {
     int                retval = -1;
     struct clicon_msg *reply = NULL;
-    uint16_t           len;
+    uint32_t           len;
 
     len = sizeof(*reply) + datalen;
     if ((reply = (struct clicon_msg *)malloc(len)) == NULL)
 	goto done;
     memset(reply, 0, len);
-    reply->op_len = htons(len);
+    reply->op_len = htonl(len);
     if (datalen > 0)
       memcpy(reply->op_body, data, datalen);
     if (clicon_msg_send(s, reply) < 0)

lib/src/clixon_xml.c

@@ -1080,7 +1080,7 @@ FSM(char *tag,
  * @retval -1  Error with clicon_err called
  *
  * @code
- *  cxobj *xt;
+ *  cxobj *xt = NULL;
  *  clicon_xml_parse_file(0, &xt, "</clicon>");
  *  xml_free(xt);
  * @endcode

test/lib.sh

@@ -15,7 +15,7 @@ err(){
 new(){
     testnr=`expr $testnr + 1`
     testname=$1
-    echo "Test$testnr [$1]"
+    >&2 echo "Test$testnr [$1]"
 #    sleep 1
 }
 

test/test_perf.sh

@@ -0,0 +1,51 @@
+#!/bin/bash
+# Test2: backend and netconf basic functionality
+
+number=10000
+
+# include err() and new() functions
+. ./lib.sh
+
+# For memcheck
+# clixon_netconf="valgrind --leak-check=full --show-leak-kinds=all clixon_netconf"
+clixon_netconf=clixon_netconf
+
+# kill old backend (if any)
+new "kill old backend"
+sudo clixon_backend -zf $clixon_cf
+if [ $? -ne 0 ]; then
+    err
+fi
+new "start backend"
+# start new backend
+sudo clixon_backend -If $clixon_cf
+if [ $? -ne 0 ]; then
+    err
+fi
+
+new "netconf perf tests"
+
+str="<rpc><edit-config><target><candidate/></target><config><interfaces>"
+for (( i=0; i<$number; i++ ))
+do  
+   str+="<interface><name>eth$i</name></interface>"
+done
+str+="</interfaces></config></edit-config></rpc>]]>]]>"
+
+new "netconf edit large config"
+expecteof "$clixon_netconf -qf $clixon_cf" "$str" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
+
+new "netconf get large config"
+expecteof "$clixon_netconf -qf $clixon_cf" "<rpc><get-config><source><candidate/></source></get-config></rpc>]]>]]>" "^<rpc-reply><data><interfaces><interface><name>eth0</name><enabled>true</enabled></interface>"
+
+new "Kill backend"
+# Check if still alive
+pid=`pgrep clixon_backend`
+if [ -z "$pid" ]; then
+    err "backend already dead"
+fi
+# kill backend
+sudo clixon_backend -zf $clixon_cf
+if [ $? -ne 0 ]; then
+    err "kill backend"
+fi