Command batch support

[alexeys85]

In our network we actively use smcroute to forward multicast traffic between sub-networks. The purpose of the network is very specific, so multicast routes change very often and use of the smcroutectl is required. The problem is that for every network change dozens of multicast routes have to be changed. Calling smcroutectl from, for example, shell script or even custom application is very slow, especially in embedded environment. Eventually I've come across few ideas:

1. API of IPC communication between smcrouted and smcroutectl is open, so it's possible to implement commands passing to smcrouted directly from our application;
2. smcrouted daemon accepts only one command at once then closes connection. So for every command it's necessary to create socket, make connect, send data, wait for an answer and close. May be it's more efficient to use SOCK_DGAM to make it possible to send multiple commands at once with help of sendmmsg?
3. Implement multiple commands parsing capability in smcrouted daemon within existing type of IPC.

I've implemented №1 as the most easy solution and situation became mush better but still far from ideal :)
The №2 requires a lot of changes in smcrouted and smcroutectl.
Then I've tried to implement №3. On our embedded system this reduces execution of 10 commands (like adding 10 new multicast routes) delay by 4x-5x times!
Below is the proof of concept how it could be implemented on the smcrouted side:

diff --git a/src/ipc.c b/src/ipc.c
index ccd7a49..93abab9 100644
--- a/src/ipc.c
+++ b/src/ipc.c
@@ -49,26 +49,40 @@ static void ipc_read(int sd)
 {
        char buf[MX_CMDPKT_SZ];
        struct ipc_msg *msg;
+       size_t rx;
+       char* buf_ptr;
 
        memset(buf, 0, sizeof(buf));
-       msg = (struct ipc_msg *)ipc_receive(sd, buf, sizeof(buf));
-       if (!msg) {
+       rx = ipc_receive(sd, buf, sizeof(buf));
+       if(!rx) {
                /* Skip logging client disconnects */
                if (errno != ECONNRESET)
                        smclog(LOG_WARNING, "Failed receiving IPC message from client: %s", strerror(errno));
                return;
        }
 
-       if (msg_do(sd, msg)) {
-               if (EINVAL == errno)
-                       smclog(LOG_WARNING, "Unknown or malformed IPC message '%c' from client.", msg->cmd);
-               errno = 0;
-               ipc_send(sd, log_message, strlen(log_message) + 1);
-       } else {
-               ipc_send(sd, "", 1);
-       }
+       buf_ptr = buf;
+       while(rx > 0) {
+               msg = (struct ipc_msg *)ipc_parse(buf_ptr, rx);
+               if (!msg) {
+                       smclog(LOG_WARNING, "Failed to parse IPC message from client: %s", strerror(errno));
+                       return;
+               }
+
+               if (msg_do(sd, msg)) {
+                       if (EINVAL == errno)
+                               smclog(LOG_WARNING, "Unknown or malformed IPC message '%c' from client.", msg->cmd);
+                       errno = 0;
+                       ipc_send(sd, log_message, strlen(log_message) + 1);
+               } else {
+                       ipc_send(sd, "", 1);
+               }
 
-       free(msg);
+               buf_ptr += msg->len;
+               rx -= msg->len;
+
+               free(msg);
+       }
 }
 
 static void ipc_accept(int sd, void *arg)
@@ -149,7 +163,7 @@ void ipc_exit(void)
  * Returns:
  * Number of bytes successfully sent, or -1 with @errno on failure.
  */
-int ipc_send(int sd, char *buf, size_t len)
+int ipc_send(int sd, const char *buf, size_t len)
 {
        if (write(sd, buf, len) != (ssize_t)len)
                return -1;
@@ -167,18 +181,34 @@ int ipc_send(int sd, char *buf, size_t len)
  * the size @len.  Connects and resets connection as necessary.
  *
  * Returns:
- * Pointer to a successfuly read command packet in @buf, or %NULL on error.
+ * Size of a successfuly read command packet in @buf, or 0 on error.
  */
-void *ipc_receive(int sd, char *buf, size_t len)
+size_t ipc_receive(int sd, char *buf, size_t len)
 {
-       size_t sz;
+       ssize_t sz;
 
        sz = recv(sd, buf, len - 1, 0);
-       if (!sz) {
-               errno = ECONNRESET;
-               return NULL;
+       if (sz <= 0) {
+               //errno = ECONNRESET;
+               return 0;
        }
 
+       return (size_t)sz;
+}
+
+/**
+ * ipc_server_parse - Parse IPC message(s) from client
+ * @buf: Buffer of message(s)
+ * @sz: Size of @buf in bytes
+ *
+ * Parse message(s) from the IPC socket, respecting
+ * the size @sz.
+ *
+ * Returns:
+ * Pointer to a successfuly read command packet in @buf, or NULL on error.
+ */
+void *ipc_parse(char *buf, size_t sz)
+{
        /* successful read */
        if (sz >= sizeof(struct ipc_msg)) {
                struct ipc_msg *msg = (struct ipc_msg *)buf;
@@ -186,7 +216,7 @@ void *ipc_receive(int sd, char *buf, size_t len)
                /* Make sure to always have at least one NUL, for strlen() */
                buf[sz] = 0;
 
-               if (sz == msg->len) {
+               if (sz >= msg->len) {
                        char *ptr;
                        size_t i, count;
 
@@ -206,7 +236,7 @@ void *ipc_receive(int sd, char *buf, size_t len)
                        ptr = buf + offsetof(struct ipc_msg, argv);
                        for (i = 0; i < count; i++) {
                                /* Verify ptr, attacker may set too large msg->count */
-                               if (ptr >= (buf + len)) {
+                               if (ptr >= (buf + msg->len)) {
                                        free(msg);
                                        errno = EBADMSG;
                                        return NULL;
diff --git a/src/ipc.h b/src/ipc.h
index 791e00d..cf8131c 100644
--- a/src/ipc.h
+++ b/src/ipc.h
@@ -7,8 +7,9 @@
 int   ipc_init    (char *path);
 void  ipc_exit    (void);
 
-int   ipc_send    (int sd, char *buf, size_t len);
-void *ipc_receive (int sd, char *buf, size_t len);
+int    ipc_send   (int sd, const char *buf, size_t len);
+size_t ipc_receive(int sd, char *buf, size_t len);
+void  *ipc_parse  (char *buf, size_t sz);
 
 #endif /* SMCROUTE_IPC_H_ */

What do you think?

[troglobit]

In theory I have nothing against this change (except for the minor detail of maybe using ssize_t as return type for ipc_recive() ;-), but you have to be aware that I cannot commit to freezing the API between daemon and client -- it is not an external API.

Submit a PR and we can take if from there.