refactor send/recv of message plus fixes

Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>

6 files changed, 239 insertions, 104 deletions

diff --git a/lib/libmuser.c b/lib/libmuser.c
index bb3254e..6cc8dd9 100644
--- a/lib/libmuser.c
+++ b/lib/libmuser.c
@@ -254,89 +254,160 @@ __free_s(char **p)
     free(*p);
 }
 
-static int
-set_version(lm_ctx_t *lm_ctx)
+int
+send_vfio_user_msg(int sock, uint16_t msg_id, bool is_reply,
+                   enum vfio_user_command cmd, void *data, int len,
+                   int *fds, int count)
 {
     int ret;
-    char *server_data __attribute__((__cleanup__(__free_s))) = NULL;
-    char *client_data __attribute__((__cleanup__(__free_s))) = NULL;
-    struct vfio_user_header hdr = {.msg_id = 0, .cmd = VFIO_USER_VERSION};
+    struct vfio_user_header hdr = {.msg_id = msg_id};
     struct iovec iov[2];
-    struct msghdr msg;
-    int client_mj, client_mn;
-
-    assert(lm_ctx != NULL);
+    struct msghdr msg = {.msg_iovlen = 1};
 
-    ret  = asprintf(&server_data, "{version: {\"major\": %d, \"minor\": %d}}",
-                    LIB_MUSER_VFIO_USER_VERS_MJ, LIB_MUSER_VFIO_USER_VERS_MN);
-    if (ret == -1) {
-        server_data = NULL;
-        return -1;
+    if (is_reply) {
+        hdr.flags.type = VFIO_USER_F_TYPE_REPLY;
+    } else {
+        hdr.cmd = cmd;
+        hdr.flags.type = VFIO_USER_F_TYPE_COMMAND;
     }
 
-    hdr.msg_size = sizeof(hdr) + ret;
+    hdr.msg_size = sizeof(hdr) + len;
 
     iov[0].iov_base = &hdr;
     iov[0].iov_len = sizeof(hdr);
-    iov[1].iov_base = server_data;
-    iov[1].iov_len = ret;
+
+    if (data != NULL) {
+        msg.msg_iovlen = 2;
+        iov[1].iov_base = data;
+        iov[1].iov_len = len;
+    }
 
     msg.msg_iov = iov;
-    msg.msg_iovlen = ARRAY_SIZE(iov);
 
-    ret = sendmsg(lm_ctx->conn_fd, &msg, 0);
+    if (fds != NULL) {
+    	size_t size = count * sizeof *fds;
+    	char buf[CMSG_SPACE(size)];
+
+    	msg.msg_control = buf;
+    	msg.msg_controllen = sizeof(buf);
+
+    	struct cmsghdr * cmsg = CMSG_FIRSTHDR(&msg);
+    	cmsg->cmsg_level = SOL_SOCKET;
+    	cmsg->cmsg_type = SCM_RIGHTS;
+    	cmsg->cmsg_len = CMSG_LEN(size);
+    	memcpy(CMSG_DATA(cmsg), fds, size);
+	    msg.msg_controllen = CMSG_SPACE(size);
+    }
+
+    ret = sendmsg(sock, &msg, 0);
     if (ret == -1) {
-        lm_log(lm_ctx, LM_DBG, "failed to send version message: %m");
         return -errno;
     }
+    return 0;
+
+}
+
+int
+send_version(int sock, int major, int minor, uint16_t msg_id, bool is_reply)
+{
+    int ret;
+    char *data __attribute__((__cleanup__(__free_s))) = NULL;
+
+    ret  = asprintf(&data, "{version: {\"major\": %d, \"minor\": %d}}",
+                    major, minor);
+    if (ret == -1) {
+        data = NULL;
+        return -1;
+    }
+
+    return send_vfio_user_msg(sock, msg_id, is_reply, VFIO_USER_VERSION, data,
+                              ret, NULL, 0); 
+}
 
-    ret = recv(lm_ctx->conn_fd, &hdr, sizeof(hdr), 0);
+int
+recv_version(int sock, int *major, int *minor, uint16_t *msg_id, bool is_reply)
+{
+    int ret;
+    struct vfio_user_header hdr;
+    char *data __attribute__((__cleanup__(__free_s))) = NULL;
+
+    ret = recv(sock, &hdr, sizeof(hdr), 0);
     if (ret == -1) {
-        lm_log(lm_ctx, LM_DBG, "failed to receive version header: %m");
         return -errno;
     }
-    if (ret <= (int)sizeof(hdr)) {
-        lm_log(lm_ctx, LM_DBG, "short read: %d", ret);
+    if (ret < (int)sizeof(hdr)) {
         return -EINVAL;
     }
-    if (hdr.msg_id != 0) {
-        lm_log(lm_ctx, LM_DBG, "bad message ID in response %d", hdr.msg_id);
-        return -EINVAL;
-    }
-    if (hdr.flags.error == 1U) {
-        if (hdr.error_no <= 0) {
-            lm_log(lm_ctx, LM_DBG, "bad error from client %d", ret);
-            hdr.error_no = EINVAL;
+
+    if (is_reply) {
+        if (hdr.msg_id != *msg_id) {
+            return -EINVAL;
+        }
+
+        if (hdr.flags.type != VFIO_USER_F_TYPE_REPLY) {
+            return -EINVAL;
         }
-        return -hdr.error_no;
+
+        if (hdr.flags.error == 1U) {
+            if (hdr.error_no <= 0) {
+                hdr.error_no = EINVAL;
+            }
+            return -hdr.error_no;
+        }
+    } else {
+        if (hdr.flags.type != VFIO_USER_F_TYPE_COMMAND) {
+            return -EINVAL;
+        }
+        *msg_id = hdr.msg_id;
     }
+
     hdr.msg_size -= sizeof(hdr);
-    client_data = malloc(hdr.msg_size);
-    if (client_data == NULL) {
+    data = malloc(hdr.msg_size);
+    if (data == NULL) {
         return -errno;
     }
-    ret = recv(lm_ctx->conn_fd, client_data, hdr.msg_size, 0);
+    ret = recv(sock, data, hdr.msg_size, 0);
     if (ret == -1) {
-        lm_log(lm_ctx, LM_DBG, "failed to receive client version data: %d");
         return -errno;
     }
     if (ret < (int)hdr.msg_size) {
-        lm_log(lm_ctx, LM_DBG, "short read: %d", ret);
         return -EINVAL;
     }
 
     /* FIXME use proper parsing */
-    ret = sscanf(client_data, "{version: {major: %d, minor: %d}}",
-                 &client_mj, &client_mn);
+    ret = sscanf(data, "{version: {\"major\": %d, \"minor\": %d}}", major, minor);
     if (ret != 2) {
-        lm_log(lm_ctx, LM_DBG, "bad version string '%s'", client_data);
         return -EINVAL;
     }
+    return 0;
+}
+
+static int
+set_version(lm_ctx_t *lm_ctx, int sock)
+{
+    int ret;
+    int client_mj, client_mn;
+    uint16_t msg_id = 0;
+
+    ret = send_version(sock, LIB_MUSER_VFIO_USER_VERS_MJ,
+                       LIB_MUSER_VFIO_USER_VERS_MN, msg_id, false);
+    if (ret < 0) {
+        lm_log(lm_ctx, LM_DBG, "failed to send version: %s", strerror(-ret));
+        return ret;
+    }
+
+    ret = recv_version(sock, &client_mj, &client_mn, &msg_id, true);
+    if (ret < 0) {
+        lm_log(lm_ctx, LM_DBG, "failed to receive version: %s", strerror(-ret));
+        return ret;
+    }
     if (client_mj != LIB_MUSER_VFIO_USER_VERS_MJ || client_mn != LIB_MUSER_VFIO_USER_VERS_MN) {
-        lm_log(lm_ctx, LM_DBG, "incompatible client version %d.%d",
+        lm_log(lm_ctx, LM_DBG, "version mismatch,  server=%d.%d, client=%d.%d",
+               LIB_MUSER_VFIO_USER_VERS_MJ, LIB_MUSER_VFIO_USER_VERS_MN,
                client_mj, client_mn);
         return -EINVAL;
     }
+
     return 0;
 }
 
@@ -348,15 +419,22 @@ set_version(lm_ctx_t *lm_ctx)
 static int
 open_sock(lm_ctx_t *lm_ctx)
 {
+    int ret;
+    int conn_fd;
+
     assert(lm_ctx != NULL);
 
-    lm_ctx->conn_fd = accept(lm_ctx->fd, NULL, NULL);
-    if (lm_ctx->conn_fd == -1) {
-        return lm_ctx->conn_fd;
+    conn_fd = accept(lm_ctx->fd, NULL, NULL);
+    if (conn_fd == -1) {
+        return conn_fd;
     }
 
     /* send version and caps */
-    return set_version(lm_ctx);
+    ret = set_version(lm_ctx, conn_fd);
+    if (ret < 0) {
+        return ret;
+    }
+    return conn_fd;
 }
 
 static int
@@ -368,12 +446,18 @@ close_sock(lm_ctx_t *lm_ctx)
 static int
 get_request_sock(lm_ctx_t *lm_ctx, struct muser_cmd *cmd)
 {
+    int ret;
+
     /*
      * TODO ideally we should set O_NONBLOCK on the fd so that the syscall is
      * faster (?). I tried that and get short reads, so we need to store the
      * partially received buffer somewhere and retry.
      */
-    return recv(lm_ctx->conn_fd, cmd, sizeof(*cmd), lm_ctx->sock_flags);
+    ret = recv(lm_ctx->conn_fd, cmd, sizeof(*cmd), lm_ctx->sock_flags);
+    if (ret == -1) {
+        return -errno;
+    }
+    return ret;
 }
 
 static int
@@ -1472,6 +1556,11 @@ out:
     return ret;
 }
 
+/*
+ * FIXME return value is messed up, sometimes we return -1 and set errno while
+ * other times we return -errno. Fix.
+ */
+
 static int
 process_request(lm_ctx_t *lm_ctx)
 {
@@ -1480,7 +1569,7 @@ process_request(lm_ctx_t *lm_ctx)
 
     err = transports_ops[lm_ctx->trans].get_request(lm_ctx, &cmd);
     if (unlikely(err < 0)) {
-        if (errno == EAGAIN || errno == EWOULDBLOCK) {
+        if (err == -EAGAIN || err == -EWOULDBLOCK) {
             return 0;
         }
         lm_log(lm_ctx, LM_ERR, "failed to receive request: %m");
@@ -1556,8 +1645,7 @@ lm_ctx_poll(lm_ctx_t *lm_ctx)
     int err;
 
     if (unlikely((lm_ctx->flags & LM_FLAG_ATTACH_NB) == 0)) {
-        errno = ENOTSUP;
-        return -1;
+        return -ENOTSUP;
     }
 
     err = process_request(lm_ctx);
@@ -1859,14 +1947,15 @@ lm_ctx_create(const lm_dev_info_t *dev_info)
         }
         lm_ctx->fd = err;
     }
+    err = 0;
 
     // Attach to the muser control device.
     if ((dev_info->flags & LM_FLAG_ATTACH_NB) == 0) {
         lm_ctx->conn_fd = transports_ops[dev_info->trans].attach(lm_ctx);
-        if (lm_ctx->conn_fd == -1) {
-                err = errno;
-                if (errno != EINTR) {
-                    lm_log(lm_ctx, LM_ERR, "failed to attach: %m\n");
+        if (lm_ctx->conn_fd < 0) {
+                err = lm_ctx->conn_fd;
+                if (err != EINTR) {
+                    lm_log(lm_ctx, LM_ERR, "failed to attach: %s", strerror(-err));
                 }
                 goto out;
         }
@@ -1890,7 +1979,7 @@ out:
             lm_ctx_destroy(lm_ctx);
             lm_ctx = NULL;
         }
-        errno = err;
+        errno = -err;
     }
 
     return lm_ctx;
diff --git a/lib/muser_priv.h b/lib/muser_priv.h
index aa29f5a..1b72aae 100644
--- a/lib/muser_priv.h
+++ b/lib/muser_priv.h
@@ -45,4 +45,15 @@ lm_get_region_info(lm_ctx_t *lm_ctx);
 uint64_t
 region_to_offset(uint32_t region);
 
+int
+send_vfio_user_msg(int sock, uint16_t msg_id, bool is_reply,
+                   enum vfio_user_command cmd, void *data, int len, int *fds,
+                   int count);
+
+int
+send_version(int sock, int major, int minor, uint16_t msg_id, bool is_reply);
+
+int
+recv_version(int sock, int *major, int *minor, uint16_t *msg_id, bool is_reply);
+
 #endif /* MUSER_PRIV_H */
diff --git a/lib/vfio_user.h b/lib/vfio_user.h
index 292458e..993e15c 100644
--- a/lib/vfio_user.h
+++ b/lib/vfio_user.h
@@ -66,6 +66,8 @@ struct vfio_user_header {
 	uint32_t	msg_size;
 	struct {
 		uint32_t	type     : 4;
+#define VFIO_USER_F_TYPE_COMMAND    0
+#define VFIO_USER_F_TYPE_REPLY      1
 		uint32_t	no_reply : 1;
 		uint32_t	error    : 1;
 		uint32_t	resvd    : 26;
@@ -73,14 +75,13 @@ struct vfio_user_header {
 	uint32_t	error_no;
 } __attribute__((packed));
 
-#define VFIO_USER_DMA_REGION_MAPPABLE		(0x1)
-
 struct vfio_user_dma_region {
 	uint64_t	addr;
 	uint64_t	size;
 	uint64_t	offset;
-	uint32_t	protections;
+	uint32_t	prot;
 	uint32_t	flags;
+#define VFIO_USER_F_DMA_REGION_MAPPABLE	(0x0)
 } __attribute__((packed));
 
 struct vfio_user_region_access {
diff --git a/samples/CMakeLists.txt b/samples/CMakeLists.txt
index 76b460c..1dad070 100644
--- a/samples/CMakeLists.txt
+++ b/samples/CMakeLists.txt
@@ -32,7 +32,7 @@ add_executable(test_read test_read.c)
 add_executable(test_mmap test_mmap.c)
 add_executable(test_dma_map test_dma_map.c)
 
-add_executable(client client.c)
+add_executable(client client.c ../lib/libmuser.c ../lib/libmuser_pci.c ../lib/dma.c ../lib/cap.c)
 
 add_executable(null null.c)
 target_link_libraries(null muser pthread)
diff --git a/samples/client.c b/samples/client.c
index 96162e2..be7b059 100644
--- a/samples/client.c
+++ b/samples/client.c
@@ -32,8 +32,11 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <stdlib.h>
+#include <errno.h>
+#include <sys/mman.h>
 
 #include "../lib/muser.h"
+#include "../lib/muser_priv.h"
 
 static int
 init_sock(const char *path)
@@ -56,77 +59,106 @@ init_sock(const char *path)
 	return sock;
 }
 
+#if 0
+static int
+map_dma(int sock)
+{
+    struct vfio_user_header hdr = {.msg_id = 1, .cmd = VFIO_USER_DMA_MAP};
+}
+#endif
+
 static int
 set_version(int sock)
 {
-    int ret;
-    char *server_data;
-    size_t size;
-    int server_mj, server_mn;
-    struct vfio_user_header hdr;
-
-    /* receive version from client */
-    ret = recv(sock, &hdr, sizeof(hdr), 0);
-    if (ret == -1) {
-        perror("failed to receive version header");
-        return -1;
-    }
+    int ret, mj, mn;
+    uint16_t msg_id;
 
-    if (ret < sizeof(hdr)) {
-        fprintf(stderr, "short version header: %d\n", ret);
-        return -1;
+    ret = recv_version(sock, &mj, &mn, &msg_id, false);
+    if (ret < 0) {
+        fprintf(stderr, "failed to receive version from server: %s\n",
+                strerror(-ret));
+        return ret;
     }
 
-    if (hdr.msg_size < sizeof(hdr)) {
-        fprintf(stderr, "bad version data size: %d\n", hdr.msg_size);
-        return -1;
-    }
-    size = hdr.msg_size - sizeof(hdr);
-    server_data = malloc(size);
-    if (server_data == NULL) {
-        perror(NULL);
-        return -1;
-    }
-    ret = recv(sock, server_data, size, 0);
-    if (ret == -1) {
-        perror("failed to receive server version");
-        return -1;
+    if (mj != LIB_MUSER_VFIO_USER_VERS_MJ || mn != LIB_MUSER_VFIO_USER_VERS_MN) {
+        fprintf(stderr, "bad server version %d.%d\n", mj, mn);
+        return -EINVAL;
     }
 
-    if (ret < size) {
-        fprintf(stderr, "short verson data read: %d", ret);
-        return -1;
+    ret = send_version(sock, mj, mn, msg_id, true); 
+    if (ret < 0) {
+        fprintf(stderr, "failed to send version to server: %s\n",
+                strerror(-ret));
+        return ret;
     }
 
-    ret = sscanf(server_data, "{version: {\"major\": %d, \"minor\": %d}}",
-                 &server_mj, &server_mn);
-    if (ret != 2) {
-        fprintf(stderr, "bad server version data %s\n", server_data);
-        return -1;
-    }
-    if (server_mj != 0 || server_mn != 1) {
-        fprintf(stderr, "bad server version %d.%d\n", server_mj, server_mn);
-        return -1;
+    return 0;
+}
+
+#if 0
+static int
+dma_map(int sock, int fd, uint64_t addr, int size, int offset) {
+    
+    struct vfio_user_dma_region dma_region = {
+        .addr = addr,
+        .size = size,
+        .offset = offset,
+        .prot = PROT_READ | PROT_WRITE,
+        .flags = VFIO_USER_F_DMA_REGION_MAPPABLE
+    };
+    int ret;
+
+    ret = send_vfio_user_msg(sock, 1, false, VFIO_USER_DMA_MAP, &dma_region,
+                             sizeof(dma_region), &fd, 1);
+    if (ret < 0) {
+        return ret;
     }
     return 0;
 }
+#endif
 
 int main(int argc, char *argv[])
 {
 	int ret, sock;
 
+#if 0
+    int dma_region_fd;
+    char template[] = "XXXXXX";
+#endif
+
     if (argc != 2) {
         fprintf(stderr, "usage: %s /path/to/socket\n", argv[0]);
         return -1;
     }
 
-    if ((sock = init_sock(argv[1])) == -1) {
+    if ((sock = init_sock(argv[1])) < 0) {
         return sock;
     }
 
-    if ((ret = set_version(sock)) == -1) {
+    /*
+     * The server proposes version upon connection, we need to send back the
+     * version the version we support.
+     */
+    if ((ret = set_version(sock)) < 0) {
+        return ret;
+    }
+
+#if 0
+    /* Tell the server we have a memory DMA region it can access. */
+    if ((dma_region_fd = mkstemp(template)) == -1) {
+        perror("failed to create DMA file");
+        return -1;
+    }
+    if ((ret = ftruncate(dma_region_fd, 2 * sysconf(_SC_PAGESIZE))) == -1) {
+        perror("failed to truncate file");
+        return -1;
+    }
+    ret = dma_map(sock, dma_region_fd, 0xdeadbeef, sysconf(_SC_PAGESIZE),
+                  sysconf(_SC_PAGESIZE));
+    if (ret < 0) {
         return ret;
     }
+#endif
 
     return 0;
 }
diff --git a/samples/gpio-pci-idio-16.c b/samples/gpio-pci-idio-16.c
index 3eb4cf0..3c87103 100644
--- a/samples/gpio-pci-idio-16.c
+++ b/samples/gpio-pci-idio-16.c
@@ -121,8 +121,10 @@ int main(int argc, char *argv[])
         return -1;
     }
     ret = lm_ctx_drive(lm_ctx);
-    if (ret != 0 && errno != EINTR) {
-        fprintf(stderr, "failed to realize device emulation: %m\n");
+    if (ret != 0) {
+        if (ret != -ENOTCONN && ret != -EINTR) {
+            fprintf(stderr, "failed to realize device emulation: %m\n");
+        }
     }
 out:
     lm_ctx_destroy(lm_ctx);