From a6664824c7cdc48b8df84e021d4675def272e14d Mon Sep 17 00:00:00 2001 From: Thanos Makatos Date: Wed, 23 Sep 2020 04:36:41 -0400 Subject: refactor send/recv of message plus fixes Signed-off-by: Thanos Makatos --- lib/libmuser.c | 199 ++++++++++++++++++++++++++++++++------------- lib/muser_priv.h | 11 +++ lib/vfio_user.h | 7 +- samples/CMakeLists.txt | 2 +- samples/client.c | 118 +++++++++++++++++---------- samples/gpio-pci-idio-16.c | 6 +- 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 #include #include +#include +#include #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); -- cgit v1.1