From fe1d614a913fd0433ec7d9858954dec5da0658d0 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Wed, 19 Jul 2023 06:38:38 -0700 Subject: [PATCH 01/13] Use separate socket for server->client commands This change adds support for a separate socket to carry commands in the server-to-client direction. It has proven problematic to send commands in both directions over a single socket, since matching replies to commands can become non-trivial when both sides send commands at the same time and adds significant complexity. See issue #279 for details. To set up the reverse communication channel, the client indicates support for it via a new capability flag in the version message. The server will then create a fresh pair of sockets and pass one end to the client in its version reply. When the server wishes to send commands to the client at a later point, it now uses its end of the new socket pair rather than the main socket. Corresponding replies are also passed back over the new socket pair. Signed-off-by: Mattias Nissler --- lib/private.h | 1 + lib/tran.c | 52 ++++++++++++++++++++++++++++++++++++++++++------ lib/tran.h | 5 +++-- lib/tran_pipe.c | 2 +- lib/tran_sock.c | 11 +++++++--- samples/client.c | 2 +- 6 files changed, 60 insertions(+), 13 deletions(-) diff --git a/lib/private.h b/lib/private.h index fdd804f6..02007aef 100644 --- a/lib/private.h +++ b/lib/private.h @@ -168,6 +168,7 @@ struct vfu_ctx { int client_max_fds; size_t client_max_data_xfer_size; + bool enable_cmd_conn; struct vfu_ctx_pending_info pending; bool quiesced; diff --git a/lib/tran.c b/lib/tran.c index dff47eff..db02f7bb 100644 --- a/lib/tran.c +++ b/lib/tran.c @@ -35,6 +35,7 @@ #include #include #include +#include #include @@ -63,7 +64,8 @@ */ int tran_parse_version_json(const char *json_str, int *client_max_fdsp, - size_t *client_max_data_xfer_sizep, size_t *pgsizep) + size_t *client_max_data_xfer_sizep, size_t *pgsizep, + bool *enable_cmd_conn) { struct json_object *jo_caps = NULL; struct json_object *jo_top = NULL; @@ -129,6 +131,19 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, } } + if (json_object_object_get_ex(jo_caps, "cmd_conn", &jo)) { + if (json_object_get_type(jo) != json_type_boolean) { + goto out; + } + + errno = 0; + *enable_cmd_conn = json_object_get_boolean(jo); + + if (errno != 0) { + goto out; + } + } + ret = 0; out: @@ -207,7 +222,7 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, ret = tran_parse_version_json(json_str, &vfu_ctx->client_max_fds, &vfu_ctx->client_max_data_xfer_size, - &pgsize); + &pgsize, &vfu_ctx->enable_cmd_conn); if (ret < 0) { /* No client-supplied strings in the log for release build. */ @@ -267,13 +282,15 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, static int send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, - struct vfio_user_version *cversion) + struct vfio_user_version *cversion, int *cmd_conn_fd) { struct vfio_user_version sversion = { 0 }; struct iovec iovecs[2] = { { 0 } }; char server_caps[1024]; vfu_msg_t msg = { { 0 } }; int slen; + int ret; + int cmd_conn_fds[2] = {-1, -1}; if (vfu_ctx->migration == NULL) { slen = snprintf(server_caps, sizeof(server_caps), @@ -313,11 +330,34 @@ send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, msg.out_iovecs = iovecs; msg.nr_out_iovecs = 2; - return vfu_ctx->tran->reply(vfu_ctx, &msg, 0); + if (vfu_ctx->enable_cmd_conn && vfu_ctx->client_max_fds > 0 && + cmd_conn_fd != NULL) { + if (socketpair(AF_UNIX, SOCK_STREAM, 0, cmd_conn_fds) == -1) { + return -1; + } + + msg.out.fds = cmd_conn_fds; + msg.out.nr_fds = 1; + } + + ret = vfu_ctx->tran->reply(vfu_ctx, &msg, 0); + + /* + * The remote end of the cmd connection is no longer needed, the local only + * if successful. + */ + close_safely(&cmd_conn_fds[0]); + if (ret == -1) { + close_safely(&cmd_conn_fds[1]); + } else if (cmd_conn_fd) { + *cmd_conn_fd = cmd_conn_fds[1]; + } + + return ret; } int -tran_negotiate(vfu_ctx_t *vfu_ctx) +tran_negotiate(vfu_ctx_t *vfu_ctx, int *cmd_conn_fd) { struct vfio_user_version *client_version = NULL; uint16_t msg_id = 0x0bad; @@ -330,7 +370,7 @@ tran_negotiate(vfu_ctx_t *vfu_ctx) return ret; } - ret = send_version(vfu_ctx, msg_id, client_version); + ret = send_version(vfu_ctx, msg_id, client_version, cmd_conn_fd); free(client_version); diff --git a/lib/tran.h b/lib/tran.h index fee96e8c..7cf6081c 100644 --- a/lib/tran.h +++ b/lib/tran.h @@ -72,10 +72,11 @@ struct transport_ops { */ int tran_parse_version_json(const char *json_str, int *client_max_fdsp, - size_t *client_max_data_xfer_sizep, size_t *pgsizep); + size_t *client_max_data_xfer_sizep, size_t *pgsizep, + bool *enable_cmd_conn); int -tran_negotiate(vfu_ctx_t *vfu_ctx); +tran_negotiate(vfu_ctx_t *vfu_ctx, int *cmd_conn_fd); #endif /* LIB_VFIO_USER_TRAN_H */ diff --git a/lib/tran_pipe.c b/lib/tran_pipe.c index d1428abf..f333130d 100644 --- a/lib/tran_pipe.c +++ b/lib/tran_pipe.c @@ -285,7 +285,7 @@ tran_pipe_attach(vfu_ctx_t *vfu_ctx) tp->in_fd = STDIN_FILENO; tp->out_fd = STDOUT_FILENO; - ret = tran_negotiate(vfu_ctx); + ret = tran_negotiate(vfu_ctx, NULL); if (ret < 0) { ret = errno; tp->in_fd = -1; diff --git a/lib/tran_sock.c b/lib/tran_sock.c index fb0ccbb6..0263a1b7 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -46,6 +46,7 @@ typedef struct { int listen_fd; int conn_fd; + int cmd_conn_fd; } tran_sock_t; int @@ -380,6 +381,7 @@ tran_sock_init(vfu_ctx_t *vfu_ctx) ts->listen_fd = -1; ts->conn_fd = -1; + ts->cmd_conn_fd = -1; if ((ts->listen_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { ret = errno; @@ -464,7 +466,7 @@ tran_sock_attach(vfu_ctx_t *vfu_ctx) return -1; } - ret = tran_negotiate(vfu_ctx); + ret = tran_negotiate(vfu_ctx, &ts->cmd_conn_fd); if (ret < 0) { close_safely(&ts->conn_fd); return -1; @@ -621,8 +623,10 @@ tran_sock_send_msg(vfu_ctx_t *vfu_ctx, uint16_t msg_id, ts = vfu_ctx->tran_data; - return tran_sock_msg(ts->conn_fd, msg_id, cmd, send_data, send_len, - hdr, recv_data, recv_len); + /* NB. use the command socket file descriptor if available. */ + return tran_sock_msg(ts->cmd_conn_fd != -1 ? ts->cmd_conn_fd : ts->conn_fd, + msg_id, cmd, send_data, send_len, hdr, recv_data, + recv_len); } static void @@ -636,6 +640,7 @@ tran_sock_detach(vfu_ctx_t *vfu_ctx) if (ts != NULL) { close_safely(&ts->conn_fd); + close_safely(&ts->cmd_conn_fd); } } diff --git a/samples/client.c b/samples/client.c index 0086fd60..ed66a302 100644 --- a/samples/client.c +++ b/samples/client.c @@ -197,7 +197,7 @@ recv_version(int sock, int *server_max_fds, size_t *server_max_data_xfer_size, } ret = tran_parse_version_json(json_str, server_max_fds, - server_max_data_xfer_size, pgsize); + server_max_data_xfer_size, pgsize, NULL); if (ret < 0) { err(EXIT_FAILURE, "failed to parse server JSON \"%s\"", json_str); From 4025f6e6acd6a32500d6dc4e3e9bfe5f8528dff0 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Mon, 24 Jul 2023 05:34:17 -0700 Subject: [PATCH 02/13] Introduce Client class in python tests Many tests have a `sock` global holding the client socket. In order to allow additional client context to be managed, introduce a Client class and replace the `sock` global with `client` global object. Signed-off-by: Mattias Nissler --- test/py/libvfio_user.py | 43 ++++++----- test/py/test_destroy.py | 4 +- test/py/test_device_get_info.py | 10 +-- test/py/test_device_get_irq_info.py | 22 +++--- test/py/test_device_get_region_info.py | 49 ++++++------- .../test_device_get_region_info_zero_size.py | 12 ++-- test/py/test_device_get_region_io_fds.py | 34 ++++----- test/py/test_device_set_irqs.py | 66 ++++++++--------- test/py/test_dirty_pages.py | 50 ++++++------- test/py/test_dma_map.py | 36 +++++----- test/py/test_dma_unmap.py | 44 ++++++------ test/py/test_irq_trigger.py | 8 +-- test/py/test_migration.py | 30 ++++---- test/py/test_negotiate.py | 8 +-- test/py/test_pci_caps.py | 72 +++++++++---------- test/py/test_pci_ext_caps.py | 52 +++++++------- test/py/test_quiesce.py | 62 ++++++++-------- test/py/test_request_errors.py | 48 ++++++------- test/py/test_setup_region.py | 36 +++++----- test/py/test_sgl_get_put.py | 18 ++--- test/py/test_shadow_ioeventfd.py | 4 +- 21 files changed, 360 insertions(+), 348 deletions(-) diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 6d60798d..bb1dbdaf 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -685,25 +685,36 @@ def connect_sock(): return sock -def connect_client(ctx): - sock = connect_sock() - - json = b'{ "capabilities": { "max_msg_fds": 8 } }' - # struct vfio_user_version - payload = struct.pack("HH%dsc" % len(json), LIBVFIO_USER_MAJOR, - LIBVFIO_USER_MINOR, json, b'\0') - hdr = vfio_user_header(VFIO_USER_VERSION, size=len(payload)) - sock.send(hdr + payload) - vfu_attach_ctx(ctx, expect=0) - payload = get_reply(sock, expect=0) - return sock +class Client: + """Models a VFIO-user client connected to the server under test.""" + + def __init__(self, sock=None): + self.sock = sock + + def connect(self, ctx): + self.sock = connect_sock() + json = b'{ "capabilities": { "max_msg_fds": 8 } }' + # struct vfio_user_version + payload = struct.pack("HH%dsc" % len(json), LIBVFIO_USER_MAJOR, + LIBVFIO_USER_MINOR, json, b'\0') + hdr = vfio_user_header(VFIO_USER_VERSION, size=len(payload)) + self.sock.send(hdr + payload) + vfu_attach_ctx(ctx, expect=0) + payload = get_reply(self.sock, expect=0) -def disconnect_client(ctx, sock): - sock.close() + def disconnect(self, ctx): + self.sock.close() + self.sock = None - # notice client closed connection - vfu_run_ctx(ctx, errno.ENOTCONN) + # notice client closed connection + vfu_run_ctx(ctx, errno.ENOTCONN) + + +def connect_client(ctx): + client = Client() + client.connect(ctx) + return client def get_reply(sock, expect=0): diff --git a/test/py/test_destroy.py b/test/py/test_destroy.py index ffe45065..94968ab7 100644 --- a/test/py/test_destroy.py +++ b/test/py/test_destroy.py @@ -35,10 +35,10 @@ def setup_function(function): - global ctx, sock + global ctx, client ctx = prepare_ctx_for_dma() assert ctx is not None - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): diff --git a/test/py/test_device_get_info.py b/test/py/test_device_get_info.py index 0a266148..73371996 100644 --- a/test/py/test_device_get_info.py +++ b/test/py/test_device_get_info.py @@ -49,11 +49,11 @@ def test_device_get_info(): # test short write - sock = connect_client(ctx) + client = connect_client(ctx) payload = struct.pack("II", 0, 0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_INFO, payload, expect=errno.EINVAL) # bad argsz @@ -61,7 +61,7 @@ def test_device_get_info(): payload = vfio_user_device_info(argsz=8, flags=0, num_regions=0, num_irqs=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_INFO, payload, expect=errno.EINVAL) # valid with larger argsz @@ -69,7 +69,7 @@ def test_device_get_info(): payload = vfio_user_device_info(argsz=32, flags=0, num_regions=0, num_irqs=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_INFO, payload) (argsz, flags, num_regions, num_irqs) = struct.unpack("IIII", result) @@ -78,7 +78,7 @@ def test_device_get_info(): assert num_regions == VFU_PCI_DEV_NUM_REGIONS assert num_irqs == VFU_DEV_NUM_IRQS - disconnect_client(ctx, sock) + client.disconnect(ctx) vfu_destroy_ctx(ctx) diff --git a/test/py/test_device_get_irq_info.py b/test/py/test_device_get_irq_info.py index 0a9b089d..8ac67599 100644 --- a/test/py/test_device_get_irq_info.py +++ b/test/py/test_device_get_irq_info.py @@ -31,13 +31,13 @@ import errno ctx = None -sock = None +client = None argsz = len(vfio_irq_info()) def test_device_get_irq_info_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -55,27 +55,27 @@ def test_device_get_irq_info_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def test_device_get_irq_info_bad_in(): payload = struct.pack("II", 0, 0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, expect=errno.EINVAL) # bad argsz payload = vfio_irq_info(argsz=8, flags=0, index=VFU_DEV_REQ_IRQ, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, expect=errno.EINVAL) # bad index payload = vfio_irq_info(argsz=argsz, flags=0, index=VFU_DEV_NUM_IRQS, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload, expect=errno.EINVAL) @@ -86,12 +86,12 @@ def test_device_get_irq_info(): payload = vfio_irq_info(argsz=argsz + 16, flags=0, index=VFU_DEV_REQ_IRQ, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) payload = vfio_irq_info(argsz=argsz, flags=0, index=VFU_DEV_REQ_IRQ, count=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) info, _ = vfio_irq_info.pop_from_buffer(result) @@ -103,7 +103,7 @@ def test_device_get_irq_info(): payload = vfio_irq_info(argsz=argsz, flags=0, index=VFU_DEV_ERR_IRQ, count=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) info, _ = vfio_irq_info.pop_from_buffer(result) @@ -115,7 +115,7 @@ def test_device_get_irq_info(): payload = vfio_irq_info(argsz=argsz, flags=0, index=VFU_DEV_MSIX_IRQ, count=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_IRQ_INFO, payload) info, _ = vfio_irq_info.pop_from_buffer(result) @@ -126,7 +126,7 @@ def test_device_get_irq_info(): def test_device_get_irq_info_cleanup(): - disconnect_client(ctx, sock) + client.disconnect(ctx) vfu_destroy_ctx(ctx) diff --git a/test/py/test_device_get_region_info.py b/test/py/test_device_get_region_info.py index 62d67406..f847cb47 100644 --- a/test/py/test_device_get_region_info.py +++ b/test/py/test_device_get_region_info.py @@ -32,7 +32,7 @@ import tempfile ctx = None -sock = None +client = None argsz = len(vfio_region_info()) migr_region_size = 2 << PAGE_SHIFT @@ -40,7 +40,7 @@ def test_device_get_region_info_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -89,14 +89,14 @@ def test_device_get_region_info_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def test_device_get_region_info_short_write(): payload = struct.pack("II", 0, 0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, expect=errno.EINVAL) @@ -106,7 +106,7 @@ def test_device_get_region_info_bad_argsz(): index=VFU_PCI_DEV_BAR1_REGION_IDX, cap_offset=0, size=0, offset=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, expect=errno.EINVAL) @@ -116,7 +116,7 @@ def test_device_get_region_info_bad_index(): index=VFU_PCI_DEV_NUM_REGIONS, cap_offset=0, size=0, offset=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload, expect=errno.EINVAL) @@ -126,7 +126,7 @@ def test_device_get_region_info_larger_argsz(): index=VFU_PCI_DEV_BAR1_REGION_IDX, cap_offset=0, size=0, offset=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) assert len(result) == argsz @@ -142,13 +142,13 @@ def test_device_get_region_info_larger_argsz(): def test_device_get_region_info_small_argsz_caps(): - global sock + global client payload = vfio_region_info(argsz=argsz, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, cap_offset=0, size=0, offset=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) info, _ = vfio_region_info.pop_from_buffer(result) @@ -167,20 +167,21 @@ def test_device_get_region_info_small_argsz_caps(): assert info.offset == 0x8000 # skip reading the SCM_RIGHTS - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_device_get_region_info_caps(): - global sock + global client - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_region_info(argsz=80, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, cap_offset=0, size=0, offset=0) payload = bytes(payload) + b'\0' * (80 - 32) - fds, result = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + fds, result = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, + payload) info, result = vfio_region_info.pop_from_buffer(result) cap, result = vfio_region_info_cap_sparse_mmap.pop_from_buffer(result) @@ -203,20 +204,20 @@ def test_device_get_region_info_caps(): assert area2.size == 0x2000 assert len(fds) == 1 - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_device_get_region_info_migr(): - global sock + global client - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_region_info(argsz=80, flags=0, index=VFU_PCI_DEV_MIGR_REGION_IDX, cap_offset=0, size=0, offset=0) payload = bytes(payload) + b'\0' * (80 - 32) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) info, result = vfio_region_info.pop_from_buffer(result) mcap, result = vfio_region_info_cap_type.pop_from_buffer(result) @@ -241,7 +242,7 @@ def test_device_get_region_info_migr(): assert area.size == migr_mmap_areas[0][1] # skip reading the SCM_RIGHTS - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_device_get_region_info_cleanup(): @@ -260,13 +261,13 @@ def test_device_get_pci_config_space_info_implicit_pci_init(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_region_info(argsz=argsz + 8, flags=0, index=VFU_PCI_DEV_CFG_REGION_IDX, cap_offset=0, size=0, offset=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) assert len(result) == argsz @@ -281,7 +282,7 @@ def test_device_get_pci_config_space_info_implicit_pci_init(): assert info.size == PCI_CFG_SPACE_EXP_SIZE assert info.offset == 0 - disconnect_client(ctx, sock) + client.disconnect(ctx) vfu_destroy_ctx(ctx) @@ -296,13 +297,13 @@ def test_device_get_pci_config_space_info_implicit_no_pci_init(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_region_info(argsz=argsz + 8, flags=0, index=VFU_PCI_DEV_CFG_REGION_IDX, cap_offset=0, size=0, offset=0) - result = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) + result = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_INFO, payload) assert len(result) == argsz @@ -315,7 +316,7 @@ def test_device_get_pci_config_space_info_implicit_no_pci_init(): assert info.size == PCI_CFG_SPACE_SIZE assert info.offset == 0 - disconnect_client(ctx, sock) + client.disconnect(ctx) vfu_destroy_ctx(ctx) diff --git a/test/py/test_device_get_region_info_zero_size.py b/test/py/test_device_get_region_info_zero_size.py index 42a6ae0b..146e812f 100644 --- a/test/py/test_device_get_region_info_zero_size.py +++ b/test/py/test_device_get_region_info_zero_size.py @@ -30,13 +30,13 @@ from libvfio_user import * ctx = None -sock = None +client = None argsz = len(vfio_region_info()) def test_device_get_region_info_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -44,13 +44,13 @@ def test_device_get_region_info_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def test_device_get_region_info_zero_sized_region(): """Tests that a zero-sized region has no caps.""" - global sock + global client for index in [VFU_PCI_DEV_BAR1_REGION_IDX, VFU_PCI_DEV_MIGR_REGION_IDX]: payload = vfio_region_info(argsz=argsz, flags=0, @@ -59,9 +59,9 @@ def test_device_get_region_info_zero_sized_region(): hdr = vfio_user_header(VFIO_USER_DEVICE_GET_REGION_INFO, size=len(payload)) - sock.send(hdr + payload) + client.sock.send(hdr + payload) vfu_run_ctx(ctx) - result = get_reply(sock) + result = get_reply(client.sock) assert len(result) == argsz diff --git a/test/py/test_device_get_region_io_fds.py b/test/py/test_device_get_region_io_fds.py index 63b433f7..47ce3251 100644 --- a/test/py/test_device_get_region_io_fds.py +++ b/test/py/test_device_get_region_io_fds.py @@ -34,12 +34,12 @@ import struct ctx = None -sock = None +client = None fds = [] def test_device_get_region_io_fds_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -71,7 +71,7 @@ def test_device_get_region_io_fds_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) for i in range(0, 6): tmp = eventfd(0, 0) fds.append(tmp) @@ -86,7 +86,7 @@ def test_device_get_region_io_fds_bad_flags(): len(vfio_user_sub_region_ioeventfd()) * 5, flags=1, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -97,7 +97,7 @@ def test_device_get_region_io_fds_bad_count(): len(vfio_user_sub_region_ioeventfd()) * 5, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -107,7 +107,7 @@ def test_device_get_region_io_fds_buffer_too_small(): argsz=len(vfio_user_region_io_fds_reply()) - 1, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -118,7 +118,7 @@ def test_device_get_region_io_fds_buffer_too_large(): index=VFU_PCI_DEV_BAR2_REGION_IDX, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -127,7 +127,7 @@ def test_device_get_region_io_fds_no_fds(): payload = vfio_user_region_io_fds_request(argsz=512, flags=0, index=VFU_PCI_DEV_BAR1_REGION_IDX, count=0) - ret = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + ret = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret) @@ -143,7 +143,7 @@ def test_device_get_region_io_fds_no_regions_setup(): payload = vfio_user_region_io_fds_request(argsz=512, flags=0, index=VFU_PCI_DEV_BAR3_REGION_IDX, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -152,7 +152,7 @@ def test_device_get_region_io_fds_region_no_mmap(): payload = vfio_user_region_io_fds_request(argsz=512, flags=0, index=VFU_PCI_DEV_BAR5_REGION_IDX, count=0) - ret = msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + ret = msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret) @@ -168,7 +168,7 @@ def test_device_get_region_io_fds_region_out_of_range(): payload = vfio_user_region_io_fds_request(argsz=512, flags=0, index=512, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=errno.EINVAL) @@ -179,7 +179,7 @@ def test_device_get_region_io_fds_fds_read_write(): len(vfio_user_sub_region_ioeventfd()) * 10, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) assert len(newfds) == 6 @@ -209,7 +209,7 @@ def test_device_get_region_io_fds_full(): len(vfio_user_sub_region_ioeventfd()) * 6, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret) @@ -238,7 +238,7 @@ def test_device_get_region_io_fds_fds_read_write_nothing(): argsz=len(vfio_user_region_io_fds_reply()), flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) assert len(newfds) == 0 @@ -263,7 +263,7 @@ def test_device_get_region_io_fds_fds_read_write_dupe_fd(): len(vfio_user_sub_region_ioeventfd()) * 8, flags=0, index=VFU_PCI_DEV_BAR2_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret) assert len(newfds) == 7 @@ -337,7 +337,7 @@ def test_device_get_region_io_fds_invalid_fd(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) fds = [] @@ -370,7 +370,7 @@ def test_device_get_region_io_fds_invalid_fd(): len(vfio_user_sub_region_ioeventfd()) * 5, flags=0, index=VFU_PCI_DEV_BAR0_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) # two unique fds diff --git a/test/py/test_device_set_irqs.py b/test/py/test_device_set_irqs.py index 1aead710..a2a27018 100644 --- a/test/py/test_device_set_irqs.py +++ b/test/py/test_device_set_irqs.py @@ -35,13 +35,13 @@ import os ctx = None -sock = None +client = None argsz = len(vfio_irq_set()) def test_device_set_irqs_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -62,20 +62,20 @@ def test_device_set_irqs_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def test_device_set_irqs_no_irq_set(): hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=0) - sock.send(hdr) + client.sock.send(hdr) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_device_set_irqs_short_write(): payload = struct.pack("II", 0, 0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -84,7 +84,7 @@ def test_device_set_irqs_bad_argsz(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_REQ_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -93,7 +93,7 @@ def test_device_set_irqs_bad_index(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_NUM_IRQS, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -102,7 +102,7 @@ def test_device_set_irqs_bad_flags_MASK_and_UNMASK(): VFIO_IRQ_SET_ACTION_UNMASK, index=VFU_DEV_MSIX_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -111,7 +111,7 @@ def test_device_set_irqs_bad_flags_DATA_NONE_and_DATA_BOOL(): VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_DATA_BOOL, index=VFU_DEV_MSIX_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -120,7 +120,7 @@ def test_device_set_irqs_bad_start_count_range(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_MSIX_IRQ, start=2047, count=2) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -129,7 +129,7 @@ def test_device_set_irqs_bad_start_count_range2(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_MSIX_IRQ, start=2049, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -138,7 +138,7 @@ def test_device_set_irqs_bad_action_for_err_irq(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_ERR_IRQ, start=0, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -147,7 +147,7 @@ def test_device_set_irqs_bad_action_for_req_irq(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_REQ_IRQ, start=0, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -156,7 +156,7 @@ def test_device_set_irqs_bad_start_count_range_for_err_irq(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_ERR_IRQ, start=0, count=2) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -165,7 +165,7 @@ def test_device_set_irqs_bad_start_count_range_for_req_irq(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_REQ_IRQ, start=0, count=2) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -174,7 +174,7 @@ def test_device_set_irqs_bad_start_for_count_0(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_MSIX_IRQ, start=1, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -183,7 +183,7 @@ def test_device_set_irqs_bad_action_for_count_0(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_MSIX_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -192,7 +192,7 @@ def test_device_set_irqs_bad_action_and_data_type_for_count_0(): VFIO_IRQ_SET_DATA_BOOL, index=VFU_DEV_MSIX_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -205,7 +205,7 @@ def test_device_set_irqs_bad_fds_for_DATA_BOOL(): fd = eventfd() - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL, fds=[fd]) os.close(fd) @@ -218,7 +218,7 @@ def test_device_set_irqs_bad_fds_for_DATA_NONE(): fd = eventfd() - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL, fds=[fd]) os.close(fd) @@ -231,7 +231,7 @@ def test_device_set_irqs_bad_fds_for_count_2(): fd = eventfd() - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL, fds=[fd]) os.close(fd) @@ -242,13 +242,13 @@ def test_device_set_irqs_disable(): VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_REQ_IRQ, start=0, count=0) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload) payload = vfio_irq_set(argsz=argsz, flags=VFIO_IRQ_SET_ACTION_TRIGGER | VFIO_IRQ_SET_DATA_EVENTFD, index=VFU_DEV_REQ_IRQ, start=0, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload) def test_device_set_irqs_enable(): @@ -258,7 +258,7 @@ def test_device_set_irqs_enable(): fd = eventfd() - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) def test_device_set_irqs_trigger_bool_too_small(): @@ -267,7 +267,7 @@ def test_device_set_irqs_trigger_bool_too_small(): start=0, count=2) payload = bytes(payload) + struct.pack("?", False) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -277,7 +277,7 @@ def test_device_set_irqs_trigger_bool_too_large(): start=0, count=2) payload = bytes(payload) + struct.pack("???", False, False, False) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, expect=errno.EINVAL) @@ -288,7 +288,7 @@ def test_device_set_irqs_enable_update(): fd = eventfd() - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) def test_device_set_irqs_enable_trigger_none(): @@ -299,13 +299,13 @@ def test_device_set_irqs_enable_trigger_none(): fd1 = eventfd(initval=4) fd2 = eventfd(initval=8) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd1, fd2]) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd1, fd2]) payload = vfio_irq_set(argsz=argsz, flags=VFIO_IRQ_SET_ACTION_TRIGGER | VFIO_IRQ_SET_DATA_NONE, index=VFU_DEV_MSIX_IRQ, start=1, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload) assert struct.unpack("Q", os.read(fd1, 8))[0] == 4 assert struct.unpack("Q", os.read(fd2, 8))[0] == 9 @@ -319,14 +319,14 @@ def test_device_set_irqs_enable_trigger_bool(): fd1 = eventfd(initval=4) fd2 = eventfd(initval=8) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd1, fd2]) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd1, fd2]) payload = vfio_irq_set(argsz=argsz + 2, flags=VFIO_IRQ_SET_ACTION_TRIGGER | VFIO_IRQ_SET_DATA_BOOL, index=VFU_DEV_MSIX_IRQ, start=0, count=2) payload = bytes(payload) + struct.pack("??", False, True) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload) assert struct.unpack("Q", os.read(fd1, 8))[0] == 4 assert struct.unpack("Q", os.read(fd2, 8))[0] == 9 @@ -341,7 +341,7 @@ def test_irq_state(mock_irq_state): index=VFU_DEV_MSIX_IRQ, start=0, count=1) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload) mock_irq_state.assert_called_with(ANY, 0, 1, True) diff --git a/test/py/test_dirty_pages.py b/test/py/test_dirty_pages.py index b3d4e342..f3e42198 100644 --- a/test/py/test_dirty_pages.py +++ b/test/py/test_dirty_pages.py @@ -56,7 +56,7 @@ def quiesce_cb(ctx): def test_dirty_pages_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -82,7 +82,7 @@ def test_dirty_pages_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) f = tempfile.TemporaryFile() f.truncate(0x10 << PAGE_SHIFT) @@ -91,19 +91,19 @@ def test_dirty_pages_setup(): flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10 << PAGE_SHIFT, size=0x20 << PAGE_SHIFT) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x40 << PAGE_SHIFT, size=0x10 << PAGE_SHIFT) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) def test_dirty_pages_short_write(): payload = struct.pack("I", 8) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -111,7 +111,7 @@ def test_dirty_pages_bad_argsz(): payload = vfio_user_dirty_pages(argsz=4, flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -119,7 +119,7 @@ def test_dirty_pages_start_no_migration(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.ENOTSUP) @@ -137,14 +137,14 @@ def test_dirty_pages_start_bad_flags(): flags=(VFIO_IOMMU_DIRTY_PAGES_FLAG_START | VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP)) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=(VFIO_IOMMU_DIRTY_PAGES_FLAG_START | VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP)) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -152,7 +152,7 @@ def start_logging(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) def test_dirty_pages_start(): @@ -165,7 +165,7 @@ def test_dirty_pages_get_short_read(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -182,7 +182,7 @@ def test_dirty_pages_get_sub_range(): payload = bytes(dirty_pages) + bytes(br) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.ENOTSUP) @@ -196,7 +196,7 @@ def test_dirty_pages_get_bad_page_size(): payload = bytes(dirty_pages) + bytes(br) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -210,7 +210,7 @@ def test_dirty_pages_get_bad_bitmap_size(): payload = bytes(dirty_pages) + bytes(br) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -224,7 +224,7 @@ def test_dirty_pages_get_bad_argsz(): payload = bytes(dirty_pages) + bytes(br) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -237,7 +237,7 @@ def test_dirty_pages_get_short_reply(): payload = bytes(dirty_pages) + bytes(br) - result = msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + result = msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) assert len(result) == len(vfio_user_dirty_pages()) @@ -260,7 +260,7 @@ def test_get_dirty_page_bitmap_unmapped(): payload = bytes(dirty_pages) + bytes(br) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, expect=errno.EINVAL) @@ -275,7 +275,7 @@ def test_dirty_pages_get_unmodified(): payload = bytes(dirty_pages) + bytes(br) - result = msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + result = msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) assert len(result) == argsz @@ -304,7 +304,7 @@ def get_dirty_page_bitmap(): payload = bytes(dirty_pages) + bytes(br) - result = msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + result = msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) _, result = vfio_user_dirty_pages.pop_from_buffer(result) _, result = vfio_user_bitmap_range.pop_from_buffer(result) @@ -433,7 +433,7 @@ def test_dirty_pages_stop(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) def test_dirty_pages_start_with_quiesce(): @@ -444,13 +444,13 @@ def test_dirty_pages_start_with_quiesce(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, rsp=False, busy=True) + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 # now should be able to get the reply - get_reply(sock, expect=0) + get_reply(client.sock, expect=0) quiesce_errno = 0 @@ -480,19 +480,19 @@ def test_dirty_pages_stop_with_quiesce(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload, rsp=False, busy=True) + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 # now should be able to get the reply - get_reply(sock, expect=0) + get_reply(client.sock, expect=0) quiesce_errno = 0 def test_dirty_pages_cleanup(): - disconnect_client(ctx, sock) + client.disconnect(ctx) vfu_destroy_ctx(ctx) # ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: diff --git a/test/py/test_dma_map.py b/test/py/test_dma_map.py index 12d1f6d6..65a998e7 100644 --- a/test/py/test_dma_map.py +++ b/test/py/test_dma_map.py @@ -42,31 +42,31 @@ def setup_function(function): - global ctx, sock + global ctx, client ctx = prepare_ctx_for_dma() assert ctx is not None - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): - global ctx, sock - disconnect_client(ctx, sock) + global ctx, client + client.disconnect(ctx) vfu_destroy_ctx(ctx) def test_dma_region_too_big(): - global ctx, sock + global ctx, client payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10 << PAGE_SHIFT, size=MAX_DMA_SIZE + PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, expect=errno.ENOSPC) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, expect=errno.ENOSPC) def test_dma_region_too_many(): - global ctx, sock + global ctx, client for i in range(1, MAX_DMA_REGIONS + 2): payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), @@ -79,7 +79,7 @@ def test_dma_region_too_many(): else: expect = 0 - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, expect=expect) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, expect=expect) @patch('libvfio_user.quiesce_cb', side_effect=fail_with_errno(errno.EBUSY)) @@ -90,14 +90,14 @@ def test_dma_map_busy(mock_dma_register, mock_quiesce): quiescing, and then eventually quiesces, the DMA map operation succeeds. """ - global ctx, sock + global ctx, client payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10 << PAGE_SHIFT, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, rsp=False, + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, rsp=False, busy=True) assert mock_dma_register.call_count == 0 @@ -111,7 +111,7 @@ def test_dma_map_busy(mock_dma_register, mock_quiesce): mmap.PROT_READ | mmap.PROT_WRITE) mock_dma_register.assert_called_once_with(ctx, dma_info) - get_reply(sock) + get_reply(client.sock) ret = vfu_run_ctx(ctx) assert ret == 0 @@ -139,13 +139,13 @@ def test_dma_map_reply_fail(mock_dma_register, mock_quiesce, mock_reset): """Tests mapping a DMA region where the quiesce callback returns 0 and replying fails.""" - global ctx, sock + global ctx, client # The only chance we have to allow the message to be received but for the # reply to fail is in the DMA map callback, where the message has been # received but reply hasn't been sent yet. def side_effect(ctx, info): - sock.close() + client.sock.close() mock_dma_register.side_effect = side_effect @@ -156,13 +156,13 @@ def side_effect(ctx, info): VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10 << PAGE_SHIFT, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, rsp=False) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, rsp=False) vfu_run_ctx(ctx, errno.ENOTCONN) # TODO not sure whether the following is worth it? try: - get_reply(sock) + get_reply(client.sock) except OSError as e: assert e.errno == errno.EBADF else: @@ -186,7 +186,7 @@ def test_dma_map_busy_reply_fail(mock_dma_register, mock_quiesce, mock_reset): replying fails. """ - global ctx, sock + global ctx, client # Send a DMA map command. payload = vfio_user_dma_map( @@ -195,13 +195,13 @@ def test_dma_map_busy_reply_fail(mock_dma_register, mock_quiesce, mock_reset): VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10 << PAGE_SHIFT, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, rsp=False, + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, rsp=False, busy=True) mock_quiesce.assert_called_once_with(ctx) # pretend there's a connection failure while the device is still quiescing - sock.close() + client.sock.close() mock_dma_register.assert_not_called() mock_reset.assert_not_called() diff --git a/test/py/test_dma_unmap.py b/test/py/test_dma_unmap.py index 063dedcf..a1fa94b3 100644 --- a/test/py/test_dma_unmap.py +++ b/test/py/test_dma_unmap.py @@ -33,40 +33,40 @@ from libvfio_user import * ctx = None -sock = None +client = None def setup_function(function): - global ctx, sock + global ctx, client ctx = prepare_ctx_for_dma() assert ctx is not None ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): - global ctx, sock - disconnect_client(ctx, sock) + global ctx, client + client.disconnect(ctx) vfu_destroy_ctx(ctx) def setup_dma_regions(dma_regions=[(0x0, PAGE_SIZE)]): - global ctx, sock + global ctx, client for dma_region in dma_regions: payload = struct.pack("II", 0, 0) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=dma_region[0], size=dma_region[1]) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) def test_dma_unmap_short_write(): payload = struct.pack("II", 0, 0) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -74,7 +74,7 @@ def test_dma_unmap_bad_argsz(): payload = vfio_user_dma_unmap(argsz=8, flags=0, addr=PAGE_SIZE, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -82,7 +82,7 @@ def test_dma_unmap_bad_argsz2(): payload = vfio_user_dma_unmap(argsz=SERVER_MAX_DATA_XFER_SIZE + 8, flags=0, addr=PAGE_SIZE, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -95,7 +95,7 @@ def test_dma_unmap_dirty_bad_argsz(): bitmap = vfio_user_bitmap(pgsize=PAGE_SIZE, size=(UINT64_MAX - argsz) + 8) payload = bytes(unmap) + bytes(bitmap) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -109,7 +109,7 @@ def test_dma_unmap_dirty_not_tracking(): bitmap = vfio_user_bitmap(pgsize=PAGE_SIZE, size=8) payload = bytes(unmap) + bytes(bitmap) + bytes(8) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -120,7 +120,7 @@ def test_dma_unmap_dirty_not_mapped(): payload = vfio_user_dirty_pages(argsz=len(vfio_user_dirty_pages()), flags=VFIO_IOMMU_DIRTY_PAGES_FLAG_START) - msg(ctx, sock, VFIO_USER_DIRTY_PAGES, payload) + msg(ctx, client.sock, VFIO_USER_DIRTY_PAGES, payload) argsz = len(vfio_user_dma_unmap()) + len(vfio_user_bitmap()) + 8 unmap = vfio_user_dma_unmap(argsz=argsz, @@ -129,7 +129,7 @@ def test_dma_unmap_dirty_not_mapped(): bitmap = vfio_user_bitmap(pgsize=PAGE_SIZE, size=8) payload = bytes(unmap) + bytes(bitmap) + bytes(8) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -138,7 +138,7 @@ def test_dma_unmap_invalid_flags(): setup_dma_regions() payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), flags=0x4, addr=PAGE_SIZE, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -147,7 +147,7 @@ def test_dma_unmap(): setup_dma_regions() payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), flags=0, addr=0x0, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload) + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload) def test_dma_unmap_invalid_addr(): @@ -156,7 +156,7 @@ def test_dma_unmap_invalid_addr(): payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), addr=0x10 << PAGE_SHIFT, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.ENOENT) @@ -167,13 +167,13 @@ def test_dma_unmap_async(mock_quiesce): mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), flags=0, addr=0x0, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, rsp=False, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 - get_reply(sock) + get_reply(client.sock) ret = vfu_run_ctx(ctx) assert ret == 0 @@ -185,7 +185,7 @@ def test_dma_unmap_all(): setup_dma_regions(dma_regions) payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), flags=VFIO_DMA_UNMAP_FLAG_ALL, addr=0, size=0) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload) + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload) def test_dma_unmap_all_invalid_addr(): @@ -193,7 +193,7 @@ def test_dma_unmap_all_invalid_addr(): payload = vfio_user_dma_unmap(argsz=len(vfio_user_dma_unmap()), flags=VFIO_DMA_UNMAP_FLAG_ALL, addr=0x10 << PAGE_SHIFT, size=PAGE_SIZE) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) @@ -203,7 +203,7 @@ def test_dma_unmap_all_invalid_flags(): flags=(VFIO_DMA_UNMAP_FLAG_ALL | VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP), addr=0, size=0) - msg(ctx, sock, VFIO_USER_DMA_UNMAP, payload, + msg(ctx, client.sock, VFIO_USER_DMA_UNMAP, payload, expect=errno.EINVAL) # FIXME need to add unit tests that test errors in get_request_header, diff --git a/test/py/test_irq_trigger.py b/test/py/test_irq_trigger.py index 18469a42..b835a58f 100644 --- a/test/py/test_irq_trigger.py +++ b/test/py/test_irq_trigger.py @@ -32,11 +32,11 @@ import errno ctx = None -sock = None +client = None def test_irq_trigger_setup(): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -50,7 +50,7 @@ def test_irq_trigger_setup(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def test_irq_trigger_bad_subindex(): @@ -76,7 +76,7 @@ def test_irq_trigger(): fd = eventfd(initval=4) - msg(ctx, sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) + msg(ctx, client.sock, VFIO_USER_DEVICE_SET_IRQS, payload, fds=[fd]) vfu_irq_trigger(ctx, 8) diff --git a/test/py/test_migration.py b/test/py/test_migration.py index 614a6156..a6327d82 100644 --- a/test/py/test_migration.py +++ b/test/py/test_migration.py @@ -33,11 +33,11 @@ from unittest.mock import patch ctx = None -sock = 0 +client = None def setup_function(function): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -54,7 +54,7 @@ def setup_function(function): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): @@ -73,10 +73,10 @@ def test_migration_bad_access(mock_trans, mock_quiesce): checking for a register-sized access, otherwise we'll change migration state without having quiesced. """ - global ctx, sock + global ctx, client data = VFIO_DEVICE_STATE_V1_SAVING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data)-1, data=data, expect=errno.EINVAL) mock_trans.assert_not_called() @@ -89,10 +89,10 @@ def test_migration_trans_sync(mock_trans, mock_quiesce): Tests transitioning to the saving state. """ - global ctx, sock + global ctx, client data = VFIO_DEVICE_STATE_V1_SAVING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data) ret = vfu_run_ctx(ctx) @@ -105,10 +105,10 @@ def test_migration_trans_sync_err(mock_trans): Tests the device returning an error when the migration state is written to. """ - global ctx, sock + global ctx, client data = VFIO_DEVICE_STATE_V1_SAVING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data, expect=errno.EPERM) ret = vfu_run_ctx(ctx) @@ -123,18 +123,18 @@ def test_migration_trans_async(mock_trans, mock_quiesce): quiescing. """ - global ctx, sock + global ctx, client mock_quiesce data = VFIO_DEVICE_STATE_V1_SAVING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 - get_reply(sock) + get_reply(client.sock) ret = vfu_run_ctx(ctx) assert ret == 0 @@ -149,10 +149,10 @@ def test_migration_trans_async_err(mock_trans, mock_quiesce): the new migration state. """ - global ctx, sock + global ctx, client data = VFIO_DEVICE_STATE_V1_RUNNING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data, rsp=False, busy=True) @@ -160,7 +160,7 @@ def test_migration_trans_async_err(mock_trans, mock_quiesce): assert ret == 0 print("waiting for reply") - get_reply(sock, errno.ENOTTY) + get_reply(client.sock, errno.ENOTTY) print("received reply") # ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: # diff --git a/test/py/test_negotiate.py b/test/py/test_negotiate.py index b0188400..74049026 100644 --- a/test/py/test_negotiate.py +++ b/test/py/test_negotiate.py @@ -161,15 +161,15 @@ def test_invalid_json_bad_pgsize2(): def test_valid_negotiate_no_json(): - sock = connect_sock() + client = Client(sock=connect_sock()) payload = struct.pack("HH", LIBVFIO_USER_MAJOR, LIBVFIO_USER_MINOR) hdr = vfio_user_header(VFIO_USER_VERSION, size=len(payload)) - sock.send(hdr + payload) + client.sock.send(hdr + payload) vfu_attach_ctx(ctx) - payload = get_reply(sock) + payload = get_reply(client.sock) (major, minor, json_str, _) = struct.unpack("HH%dsc" % (len(payload) - 5), payload) assert major == LIBVFIO_USER_MAJOR @@ -179,7 +179,7 @@ def test_valid_negotiate_no_json(): assert json.capabilities.max_data_xfer_size == SERVER_MAX_DATA_XFER_SIZE # FIXME: migration object checks - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_valid_negotiate_empty_json(): diff --git a/test/py/test_pci_caps.py b/test/py/test_pci_caps.py index b7ad07b8..5a3521b5 100644 --- a/test/py/test_pci_caps.py +++ b/test/py/test_pci_caps.py @@ -153,16 +153,16 @@ def test_add_caps(mock_pci_region_cb): __test_find_caps() - sock = connect_client(ctx) + client = connect_client(ctx) - __test_pci_cap_write_hdr(sock) - __test_pci_cap_readonly(sock) + __test_pci_cap_write_hdr(client.sock) + __test_pci_cap_readonly(client.sock) # FIXME assignment to PCI config space from callback is ignored # Ideally we should ignore this test via pytest command line but this isn't # and individual test, and making it one requires a bit of effort. if not is_32bit(): - __test_pci_cap_callback(sock) - __test_pci_cap_write_pmcs(sock) + __test_pci_cap_callback(client.sock) + __test_pci_cap_write_pmcs(client.sock) def __test_find_caps(): @@ -321,14 +321,14 @@ def test_pci_cap_write_px(mock_quiesce, mock_reset): Tests function level reset. """ setup_pci_dev(realize=True) - sock = connect_client(ctx) + client = connect_client(ctx) setup_flrc(ctx) # iflr offset = PCI_STD_HEADER_SIZEOF + 8 data = b'\x00\x80' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) mock_quiesce.assert_called_once_with(ctx) @@ -337,14 +337,14 @@ def test_pci_cap_write_px(mock_quiesce, mock_reset): # bad access for _off in (-1, +1): for _len in (-1, +1): - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset+_off, count=len(data)+_len, data=data, expect=errno.EINVAL) def test_pci_cap_write_msi(): setup_pci_dev(realize=True) - sock = connect_client(ctx) + client = connect_client(ctx) # Set MMC to 100b (16 interrupt vectors) mmc = 0b00001000 @@ -366,20 +366,20 @@ def test_pci_cap_write_msi(): # Test if write fails as expected # as MME is out of bounds, 111b is over the max of 101b (32 vectors) data = b'\xff\xff' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSI_FLAGS, count=len(data), data=data, expect=errno.EINVAL) # Test if write fails as expected # as MME is over MMC, 101b (32 vectors) > 100b (16 vectors) data = to_bytes_le(mme_bad, 2) - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSI_FLAGS, count=len(data), data=data, expect=errno.EINVAL) # Test good write, MSI Enable + good MME data = to_bytes_le(PCI_MSI_FLAGS_ENABLE | mme_good, 2) - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSI_FLAGS, count=len(data), data=data) @@ -388,21 +388,21 @@ def test_pci_cap_write_msi(): # reset data = size_before_flags * b'\x00' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSI_FLAGS, count=len(data), data=data) # Check if MMC is still present after reset (since it is RO) expected = (to_bytes_le(PCI_CAP_ID_MSI) + b'\x00' + to_bytes_le(mmc, 2) + (size_after_flags * b'\x00')) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(expected)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(expected)) assert expected == payload def test_pci_cap_write_msix(): setup_pci_dev(realize=True) - sock = connect_client(ctx) + client = connect_client(ctx) flags = PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE pos = vfu_pci_add_capability(ctx, pos=0, flags=0, @@ -415,51 +415,51 @@ def test_pci_cap_write_msix(): # write exactly to Message Control: mask all vectors and enable MSI-X data = b'\xff\xff' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_FLAGS, count=len(data), data=data) data = b'\xff\xff' + to_bytes_le(flags, 2) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00' + \ to_bytes_le(PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE, 2) assert expected == payload # reset data = b'\x00\x00' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_FLAGS, count=len(data), data=data) expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00\x00' - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(expected)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(expected)) assert expected == payload # write 2 bytes to Message Control + 1: mask all vectors and enable MSI-X # This looks bizarre, but some versions of QEMU do this. data = to_bytes_le(flags >> 8) + b'\xff' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_FLAGS + 1, count=len(data), data=data) # read back entire MSI-X expected = to_bytes_le(PCI_CAP_ID_MSIX) + b'\x00' + \ to_bytes_le(flags, 2) + b'\x00\x00\x00\x00' + b'\x00\x00\x00\x00' - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=PCI_CAP_MSIX_SIZEOF) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=PCI_CAP_MSIX_SIZEOF) assert expected == payload # reset with MSI-X enabled data = to_bytes_le(PCI_MSIX_FLAGS_ENABLE, 2) - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_FLAGS, count=len(data), data=data) # write 1 byte past Message Control, MSI-X should still be enabled data = b'\x00' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_TABLE, count=len(data), data=data) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset + PCI_MSIX_FLAGS, count=2) assert payload == to_bytes_le(PCI_MSIX_FLAGS_ENABLE, 2) @@ -467,34 +467,34 @@ def test_pci_cap_write_msix(): def test_pci_cap_write_pxdc2(): setup_pci_dev(realize=True) - sock = connect_client(ctx) + client = connect_client(ctx) setup_flrc(ctx) offset = (vfu_pci_find_capability(ctx, False, PCI_CAP_ID_EXP) + PCI_EXP_DEVCTL2) data = b'\xde\xad' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) assert payload == data def test_pci_cap_write_pxlc2(): setup_pci_dev(realize=True) - sock = connect_client(ctx) + client = connect_client(ctx) setup_flrc(ctx) offset = (vfu_pci_find_capability(ctx, False, PCI_CAP_ID_EXP) + PCI_EXP_LNKCTL2) data = b'\xbe\xef' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) assert payload == data diff --git a/test/py/test_pci_ext_caps.py b/test/py/test_pci_ext_caps.py index c425c8b5..53af9df9 100644 --- a/test/py/test_pci_ext_caps.py +++ b/test/py/test_pci_ext_caps.py @@ -224,37 +224,37 @@ def test_find_ext_caps(): def test_pci_ext_cap_write_hdr(): - sock = connect_client(ctx) + client = connect_client(ctx) # struct pcie_ext_cap_hdr offset = cap_offsets[0] data = b'\x01' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data, expect=errno.EPERM) # struct pcie_ext_cap_vsc_hdr also offset = cap_offsets[1] + 4 - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data, expect=errno.EPERM) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_pci_ext_cap_readonly(): - sock = connect_client(ctx) + client = connect_client(ctx) # start of vendor payload offset = cap_offsets[1] + 8 data = b'\x01' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data, expect=errno.EPERM) offset = cap_offsets[1] + 8 - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=5) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=5) assert payload == b'abcde' - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_pci_ext_cap_callback(): @@ -262,42 +262,42 @@ def test_pci_ext_cap_callback(): # FIXME assignment to PCI config space from callback is ignored if is_32bit(): return - sock = connect_client(ctx) + client = connect_client(ctx) # start of vendor payload offset = cap_offsets[2] + 8 data = b"Hello world." - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) assert payload == data data = b"Bye world." - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) assert payload == data - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_pci_ext_cap_write_dsn(): - sock = connect_client(ctx) + client = connect_client(ctx) data = struct.pack("II", 1, 2) offset = cap_offsets[0] + 4 - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data, expect=errno.EPERM) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) # unchanged! assert payload == struct.pack("II", 4, 8) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_pci_ext_cap_write_vendor(): @@ -306,20 +306,20 @@ def test_pci_ext_cap_write_vendor(): if is_32bit(): return - sock = connect_client(ctx) + client = connect_client(ctx) data = struct.pack("II", 0x1, 0x2) # start of vendor payload offset = cap_offsets[2] + 8 - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, - count=len(data)) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=offset, count=len(data)) assert payload == data - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_pci_ext_cap_cleanup(): diff --git a/test/py/test_quiesce.py b/test/py/test_quiesce.py index b1fb2fdc..3f728270 100644 --- a/test/py/test_quiesce.py +++ b/test/py/test_quiesce.py @@ -37,10 +37,10 @@ def setup_function(function): - global ctx, sock + global ctx, client ctx = prepare_ctx_for_dma(migration_callbacks=True) assert ctx is not None - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): @@ -69,14 +69,14 @@ def test_device_quiesce_error(mock_quiesce): that requested it also fails with the same error. """ - global ctx, sock + global ctx, client payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10000, size=0x1000) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, errno.ENOTTY) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, errno.ENOTTY) @patch('libvfio_user.dma_register') @@ -86,14 +86,14 @@ def test_device_quiesce_error_after_busy(mock_quiesce, mock_dma_register): Checks that the device fails to quiesce after it was busy quiescing. """ - global ctx, sock + global ctx, client payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x10000, size=0x1000) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, rsp=False, + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, errno.ENOTTY) @@ -145,10 +145,10 @@ def _unmap_dma_region(ctx, sock, busy=False): def test_allowed_funcs_in_quiesced_dma_register(mock_quiesce, mock_dma_register): - global ctx, sock + global ctx, client # FIXME assert quiesce callback is called - _map_dma_region(ctx, sock) + _map_dma_region(ctx, client.sock) # FIXME it's difficult to check that mock_dma_register has been called with # the expected DMA info because we don't know the vaddr and the mapping # (2nd and 3rd arguments of vfu_dma_info_t) as they're values returned from @@ -163,9 +163,9 @@ def test_allowed_funcs_in_quiesced_dma_register(mock_quiesce, def test_allowed_funcs_in_quiesced_dma_unregister(mock_quiesce, mock_dma_unregister): - global ctx, sock - _map_dma_region(ctx, sock) - _unmap_dma_region(ctx, sock) + global ctx, client + _map_dma_region(ctx, client.sock) + _unmap_dma_region(ctx, client.sock) mock_dma_unregister.assert_called_once_with(ctx, mock.ANY) @@ -174,8 +174,8 @@ def test_allowed_funcs_in_quiesced_dma_unregister(mock_quiesce, def test_allowed_funcs_in_quiesced_dma_register_busy(mock_quiesce, mock_dma_register): - global ctx, sock - _map_dma_region(ctx, sock, errno.EBUSY) + global ctx, client + _map_dma_region(ctx, client.sock, errno.EBUSY) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 mock_dma_register.assert_called_once_with(ctx, mock.ANY) @@ -186,10 +186,10 @@ def test_allowed_funcs_in_quiesced_dma_register_busy(mock_quiesce, def test_allowed_funcs_in_quiesced_dma_unregister_busy(mock_quiesce, mock_dma_unregister): - global ctx, sock - _map_dma_region(ctx, sock) + global ctx, client + _map_dma_region(ctx, client.sock) mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) - _unmap_dma_region(ctx, sock, busy=True) + _unmap_dma_region(ctx, client.sock, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 mock_dma_unregister.assert_called_once_with(ctx, mock.ANY) @@ -200,10 +200,10 @@ def test_allowed_funcs_in_quiesced_dma_unregister_busy(mock_quiesce, def test_allowed_funcs_in_quiesed_migration(mock_quiesce, mock_trans): - global ctx, sock - _map_dma_region(ctx, sock) + global ctx, client + _map_dma_region(ctx, client.sock) data = VFIO_DEVICE_STATE_V1_SAVING.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data) mock_trans.assert_called_once_with(ctx, VFIO_DEVICE_STATE_V1_SAVING) @@ -213,11 +213,11 @@ def test_allowed_funcs_in_quiesed_migration(mock_quiesce, def test_allowed_funcs_in_quiesed_migration_busy(mock_quiesce, mock_trans): - global ctx, sock - _map_dma_region(ctx, sock) + global ctx, client + _map_dma_region(ctx, client.sock) mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) data = VFIO_DEVICE_STATE_V1_STOP.to_bytes(c.sizeof(c.c_int), 'little') - write_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, + write_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, offset=0, count=len(data), data=data, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) @@ -228,19 +228,19 @@ def test_allowed_funcs_in_quiesed_migration_busy(mock_quiesce, @patch('libvfio_user.reset_cb', side_effect=_side_effect) @patch('libvfio_user.quiesce_cb') def test_allowed_funcs_in_quiesced_reset(mock_quiesce, mock_reset): - global ctx, sock - _map_dma_region(ctx, sock) - msg(ctx, sock, VFIO_USER_DEVICE_RESET) + global ctx, client + _map_dma_region(ctx, client.sock) + msg(ctx, client.sock, VFIO_USER_DEVICE_RESET) mock_reset.assert_called_once_with(ctx, VFU_RESET_DEVICE) @patch('libvfio_user.reset_cb', side_effect=_side_effect) @patch('libvfio_user.quiesce_cb') def test_allowed_funcs_in_quiesced_reset_busy(mock_quiesce, mock_reset): - global ctx, sock - _map_dma_region(ctx, sock) + global ctx, client + _map_dma_region(ctx, client.sock) mock_quiesce.side_effect = fail_with_errno(errno.EBUSY) - msg(ctx, sock, VFIO_USER_DEVICE_RESET, rsp=False, + msg(ctx, client.sock, VFIO_USER_DEVICE_RESET, rsp=False, busy=True) ret = vfu_device_quiesced(ctx, 0) assert ret == 0 @@ -253,16 +253,16 @@ def test_flr(mock_quiesce, mock_reset): """Test that an FLR reset callback is still able to call functions not allowed in quiescent state.""" - global ctx, sock + global ctx, client - _map_dma_region(ctx, sock) + _map_dma_region(ctx, client.sock) setup_flrc(ctx) # iflr offset = PCI_STD_HEADER_SIZEOF + 8 data = b'\x00\x80' - write_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, + write_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=offset, count=len(data), data=data) mock_quiesce.assert_called_with(ctx) diff --git a/test/py/test_request_errors.py b/test/py/test_request_errors.py index 79af0f27..c25a7158 100644 --- a/test/py/test_request_errors.py +++ b/test/py/test_request_errors.py @@ -33,12 +33,12 @@ import os ctx = None -sock = None +client = None argsz = len(vfio_irq_set()) def setup_function(function): - global ctx, sock + global ctx, client ctx = vfu_create_ctx(flags=LIBVFIO_USER_FLAG_ATTACH_NB) assert ctx is not None @@ -64,7 +64,7 @@ def setup_function(function): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) def teardown_function(function): @@ -77,9 +77,9 @@ def test_too_small(): hdr = struct.pack("HHIII", 0xbad1, VFIO_USER_DEVICE_SET_IRQS, SIZEOF_VFIO_USER_HEADER - 1, VFIO_USER_F_TYPE_COMMAND, 0) - sock.send(hdr) + client.sock.send(hdr) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_too_large(): @@ -87,9 +87,9 @@ def test_too_large(): hdr = struct.pack("HHIII", 0xbad1, VFIO_USER_DEVICE_SET_IRQS, SERVER_MAX_MSG_SIZE + 1, VFIO_USER_F_TYPE_COMMAND, 0) - sock.send(hdr) + client.sock.send(hdr) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_unsolicited_reply(): @@ -97,24 +97,24 @@ def test_unsolicited_reply(): hdr = struct.pack("HHIII", 0xbad2, VFIO_USER_DEVICE_SET_IRQS, SIZEOF_VFIO_USER_HEADER, VFIO_USER_F_TYPE_REPLY, 0) - sock.send(hdr) + client.sock.send(hdr) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_bad_command(): hdr = vfio_user_header(VFIO_USER_MAX, size=1) - sock.send(hdr + b'\0') + client.sock.send(hdr + b'\0') vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_no_payload(): hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=0) - sock.send(hdr) + client.sock.send(hdr) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) def test_bad_request_closes_fds(): @@ -126,10 +126,10 @@ def test_bad_request_closes_fds(): fd2 = eventfd() hdr = vfio_user_header(VFIO_USER_DEVICE_SET_IRQS, size=len(payload)) - sock.sendmsg([hdr + payload], [(socket.SOL_SOCKET, socket.SCM_RIGHTS, - struct.pack("II", fd1, fd2))]) + client.sock.sendmsg([hdr + payload], [(socket.SOL_SOCKET, + socket.SCM_RIGHTS, struct.pack("II", fd1, fd2))]) vfu_run_ctx(ctx) - get_reply(sock, expect=errno.EINVAL) + get_reply(client.sock, expect=errno.EINVAL) # # It's a little cheesy, but this is just ensuring no fd's remain open past @@ -149,8 +149,8 @@ def test_disconnected_socket(mock_quiesce, mock_reset): """Tests that calling vfu_run_ctx on a disconnected socket results in resetting the context and returning ENOTCONN.""" - global ctx, sock - sock.close() + global ctx, client + client.sock.close() vfu_run_ctx(ctx, errno.ENOTCONN) @@ -165,8 +165,8 @@ def test_disconnected_socket_quiesce_busy(mock_quiesce): """Tests that calling vfu_run_ctx on a disconnected socket results in resetting the context which returns EBUSY.""" - global ctx, sock - sock.close() + global ctx, client + client.sock.close() vfu_run_ctx(ctx, errno.EBUSY) @@ -194,16 +194,16 @@ def test_reply_fail_quiesce_busy(mock_get_pending_bytes, mock_quiesce, mock_reset): """Tests failing to reply and the quiesce callback returning EBUSY.""" - global ctx, sock + global ctx, client def get_pending_bytes_side_effect(ctx): - sock.close() + client.sock.close() return 0 mock_get_pending_bytes.side_effect = get_pending_bytes_side_effect # read the get_pending_bytes register, it should close the socket causing # the reply to fail - read_region(ctx, sock, VFU_PCI_DEV_MIGR_REGION_IDX, + read_region(ctx, client.sock, VFU_PCI_DEV_MIGR_REGION_IDX, vfio_user_migration_info.pending_bytes.offset, vfio_user_migration_info.pending_bytes.size, rsp=False, busy=True) @@ -227,7 +227,7 @@ def get_pending_bytes_side_effect(ctx): mock_reset.assert_called_once_with(ctx, VFU_RESET_LOST_CONN) try: - get_reply(sock) + get_reply(client.sock) except OSError as e: assert e.errno == errno.EBADF else: diff --git a/test/py/test_setup_region.py b/test/py/test_setup_region.py index d00de689..05e64574 100644 --- a/test/py/test_setup_region.py +++ b/test/py/test_setup_region.py @@ -165,13 +165,13 @@ def test_setup_region_cfg_always_cb(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) - payload = read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + payload = read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=0, count=2) assert payload == b'\xcc\xcc' - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_region_offset_overflow(): @@ -185,12 +185,12 @@ def test_region_offset_overflow(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) - read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=UINT64_MAX, count=256, expect=errno.EINVAL) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_access_region_zero_count(): @@ -203,40 +203,40 @@ def test_access_region_zero_count(): ret = vfu_realize_ctx(ctx) assert ret == 0 - sock = connect_client(ctx) + client = connect_client(ctx) - payload = read_region(ctx, sock, VFU_PCI_DEV_BAR0_REGION_IDX, offset=0, - count=0) + payload = read_region(ctx, client.sock, VFU_PCI_DEV_BAR0_REGION_IDX, + offset=0, count=0) assert payload == b'' - write_region(ctx, sock, VFU_PCI_DEV_BAR0_REGION_IDX, offset=0, count=0, - data=payload) + write_region(ctx, client.sock, VFU_PCI_DEV_BAR0_REGION_IDX, offset=0, + count=0, data=payload) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_access_region_large_count(): global ctx - sock = connect_client(ctx) + client = connect_client(ctx) - read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=0, + read_region(ctx, client.sock, VFU_PCI_DEV_CFG_REGION_IDX, offset=0, count=SERVER_MAX_DATA_XFER_SIZE + 8, expect=errno.EINVAL) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_region_offset_too_short(): global ctx - sock = connect_client(ctx) + client = connect_client(ctx) payload = struct.pack("Q", 0) - msg(ctx, sock, VFIO_USER_REGION_WRITE, payload, + msg(ctx, client.sock, VFIO_USER_REGION_WRITE, payload, expect=errno.EINVAL) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_setup_region_cleanup(): diff --git a/test/py/test_sgl_get_put.py b/test/py/test_sgl_get_put.py index d44dc6ee..53203fca 100644 --- a/test/py/test_sgl_get_put.py +++ b/test/py/test_sgl_get_put.py @@ -54,13 +54,13 @@ def test_sgl_get_with_invalid_region(): def test_sgl_get_without_fd(): - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x1000, size=4096) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) sg = dma_sg_t() iovec = iovec_t() sg.region = 0 @@ -68,11 +68,11 @@ def test_sgl_get_without_fd(): assert ret == -1 assert ctypes.get_errno() == errno.EFAULT - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_get_multiple_sge(): - sock = connect_client(ctx) + client = connect_client(ctx) regions = 4 f = tempfile.TemporaryFile() f.truncate(0x1000 * regions) @@ -81,7 +81,7 @@ def test_get_multiple_sge(): flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x1000 * i, size=4096) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) ret, sg = vfu_addr_to_sgl(ctx, dma_addr=0x1000, length=4096 * 3, max_nr_sgs=3, prot=mmap.PROT_READ) @@ -94,11 +94,11 @@ def test_get_multiple_sge(): assert iovec[1].iov_len == 4096 assert iovec[2].iov_len == 4096 - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_sgl_put(): - sock = connect_client(ctx) + client = connect_client(ctx) regions = 4 f = tempfile.TemporaryFile() f.truncate(0x1000 * regions) @@ -107,7 +107,7 @@ def test_sgl_put(): flags=(VFIO_USER_F_DMA_REGION_READ | VFIO_USER_F_DMA_REGION_WRITE), offset=0, addr=0x1000 * i, size=4096) - msg(ctx, sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload, fds=[f.fileno()]) ret, sg = vfu_addr_to_sgl(ctx, dma_addr=0x1000, length=4096 * 3, max_nr_sgs=3, prot=mmap.PROT_READ) @@ -118,7 +118,7 @@ def test_sgl_put(): assert ret == 0 vfu_sgl_put(ctx, sg, iovec, cnt=3) - disconnect_client(ctx, sock) + client.disconnect(ctx) def test_sgl_get_put_cleanup(): diff --git a/test/py/test_shadow_ioeventfd.py b/test/py/test_shadow_ioeventfd.py index e64c2538..1375769f 100644 --- a/test/py/test_shadow_ioeventfd.py +++ b/test/py/test_shadow_ioeventfd.py @@ -60,12 +60,12 @@ def test_shadow_ioeventfd(): assert ret == 0 # client queries I/O region FDs - sock = connect_client(ctx) + client = connect_client(ctx) payload = vfio_user_region_io_fds_request( argsz=len(vfio_user_region_io_fds_reply()) + len(vfio_user_sub_region_ioeventfd()), flags=0, index=VFU_PCI_DEV_BAR0_REGION_IDX, count=0) - newfds, ret = msg_fds(ctx, sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, + newfds, ret = msg_fds(ctx, client.sock, VFIO_USER_DEVICE_GET_REGION_IO_FDS, payload, expect=0) reply, ret = vfio_user_region_io_fds_reply.pop_from_buffer(ret) assert reply.count == 1 # 1 eventfd From c01b7fe81ad6f2ca45a13d6e8f21f731d1be00bd Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Wed, 19 Jul 2023 00:01:32 -0700 Subject: [PATCH 03/13] Add pytest for vfu_sgl_{read,write} The new tests verify behavior of vfu_sgl_{read,write} for success and error cases. Signed-off-by: Mattias Nissler --- test/py/libvfio_user.py | 123 +++++++++++++++++----- test/py/meson.build | 1 + test/py/test_sgl_read_write.py | 182 +++++++++++++++++++++++++++++++++ 3 files changed, 280 insertions(+), 26 deletions(-) create mode 100644 test/py/test_sgl_read_write.py diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index bb1dbdaf..9f14db22 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -481,6 +481,14 @@ class vfio_user_dma_unmap(Structure): ] +class vfio_user_dma_region_access(Structure): + _pack_ = 1 + _fields_ = [ + ("addr", c.c_uint64), + ("count", c.c_uint64), + ] + + class vfu_dma_info_t(Structure): _fields_ = [ ("iova", iovec_t), @@ -639,6 +647,10 @@ class vfio_user_migration_info(Structure): c.POINTER(iovec_t), c.c_size_t, c.c_int) lib.vfu_sgl_put.argtypes = (c.c_void_p, c.POINTER(dma_sg_t), c.POINTER(iovec_t), c.c_size_t) +lib.vfu_sgl_read.argtypes = (c.c_void_p, c.POINTER(dma_sg_t), c.c_size_t, + c.c_void_p) +lib.vfu_sgl_write.argtypes = (c.c_void_p, c.POINTER(dma_sg_t), c.c_size_t, + c.c_void_p) lib.vfu_create_ioeventfd.argtypes = (c.c_void_p, c.c_uint32, c.c_int, c.c_size_t, c.c_uint32, c.c_uint32, @@ -690,30 +702,46 @@ class Client: def __init__(self, sock=None): self.sock = sock + self.cmd_sock = None - def connect(self, ctx): + def connect(self, ctx, + max_data_xfer_size=VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE, + cmd_conn=False): self.sock = connect_sock() - json = b'{ "capabilities": { "max_msg_fds": 8 } }' + json = f''' + {{ + "capabilities": {{ + "max_data_xfer_size": {max_data_xfer_size}, + "max_msg_fds": 8, + "cmd_conn": {str(cmd_conn).lower()} + }} + }} + ''' # struct vfio_user_version payload = struct.pack("HH%dsc" % len(json), LIBVFIO_USER_MAJOR, - LIBVFIO_USER_MINOR, json, b'\0') + LIBVFIO_USER_MINOR, json.encode(), b'\0') hdr = vfio_user_header(VFIO_USER_VERSION, size=len(payload)) self.sock.send(hdr + payload) vfu_attach_ctx(ctx, expect=0) - payload = get_reply(self.sock, expect=0) + fds, payload = get_reply_fds(self.sock, expect=0) + self.cmd_sock = socket.socket(fileno=fds[0]) if fds else None + return self.sock def disconnect(self, ctx): self.sock.close() self.sock = None + if self.cmd_sock is not None: + self.cmd_sock.close() + self.cmd_sock = None # notice client closed connection vfu_run_ctx(ctx, errno.ENOTCONN) -def connect_client(ctx): +def connect_client(*args, **kwargs): client = Client() - client.connect(ctx) + client.connect(*args, **kwargs) return client @@ -725,6 +753,23 @@ def get_reply(sock, expect=0): return buf[16:] +def send_msg(sock, cmd, msg_type, payload=bytearray(), fds=None, msg_id=None, + error_no=0): + """ + Sends a message on the given socket. Can be used on either end of the + socket to send commands and replies. + """ + hdr = vfio_user_header(cmd, size=len(payload), msg_type=msg_type, + msg_id=msg_id, error=error_no != 0, + error_no=error_no) + + if fds: + sock.sendmsg([hdr + payload], [(socket.SOL_SOCKET, socket.SCM_RIGHTS, + struct.pack("I" * len(fds), *fds))]) + else: + sock.send(hdr + payload) + + def msg(ctx, sock, cmd, payload=bytearray(), expect=0, fds=None, rsp=True, busy=False): """ @@ -737,13 +782,7 @@ def msg(ctx, sock, cmd, payload=bytearray(), expect=0, fds=None, response: it can later be retrieved, post vfu_device_quiesced(), with get_reply(). """ - hdr = vfio_user_header(cmd, size=len(payload)) - - if fds: - sock.sendmsg([hdr + payload], [(socket.SOL_SOCKET, socket.SCM_RIGHTS, - struct.pack("I" * len(fds), *fds))]) - else: - sock.send(hdr + payload) + send_msg(sock, cmd, VFIO_USER_F_TYPE_COMMAND, payload, fds) if busy: vfu_run_ctx(ctx, errno.EBUSY) @@ -756,12 +795,14 @@ def msg(ctx, sock, cmd, payload=bytearray(), expect=0, fds=None, return get_reply(sock, expect=expect) -def get_reply_fds(sock, expect=0): - """Receives a message from a socket and pulls the returned file descriptors - out of the message.""" +def get_msg_fds(sock, expect_type, expect=0): + """ + Receives a message from a socket and pulls the returned file descriptors + out of the message. + """ fds = array.array("i") - data, ancillary, flags, addr = sock.recvmsg(4096, - socket.CMSG_LEN(64 * fds.itemsize)) + data, ancillary, flags, addr = sock.recvmsg(SERVER_MAX_MSG_SIZE, + socket.CMSG_LEN(64 * fds.itemsize)) (msg_id, cmd, msg_size, msg_flags, errno) = struct.unpack("HHIII", data[0:16]) assert errno == expect @@ -773,8 +814,18 @@ def get_reply_fds(sock, expect=0): [unpacked_fd] = struct.unpack_from("i", packed_fd, offset=i) unpacked_fds.append(unpacked_fd) assert len(packed_fd)/4 == len(unpacked_fds) - assert (msg_flags & VFIO_USER_F_TYPE_REPLY) != 0 - return (unpacked_fds, data[16:]) + assert (msg_flags & 0xf) == expect_type + return (unpacked_fds, msg_id, cmd, data[16:]) + + +def get_reply_fds(sock, expect=0): + """ + Receives a reply from a socket and returns the included file descriptors + and message payload data. + """ + (unpacked_fds, _, _, data) = get_msg_fds(sock, VFIO_USER_F_TYPE_REPLY, + expect) + return (unpacked_fds, data) def msg_fds(ctx, sock, cmd, payload, expect=0, fds=None): @@ -973,7 +1024,7 @@ def prepare_ctx_for_dma(dma_register=__dma_register, # -msg_id = 1 +next_msg_id = 1 @c.CFUNCTYPE(None, c.c_void_p, c.c_int, c.c_char_p) @@ -989,13 +1040,21 @@ def log(ctx, level, msg): print(lvl2str[level] + ": " + msg.decode("utf-8")) -def vfio_user_header(cmd, size, no_reply=False, error=False, error_no=0): - global msg_id +def vfio_user_header(cmd, + size, + msg_type=VFIO_USER_F_TYPE_COMMAND, + msg_id=None, + no_reply=False, + error=False, + error_no=0): + global next_msg_id - buf = struct.pack("HHIII", msg_id, cmd, SIZEOF_VFIO_USER_HEADER + size, - VFIO_USER_F_TYPE_COMMAND, error_no) + if msg_id is None: + msg_id = next_msg_id + next_msg_id += 1 - msg_id += 1 + buf = struct.pack("HHIII", msg_id, cmd, SIZEOF_VFIO_USER_HEADER + size, + msg_type | (no_reply << 4) | (error << 5), error_no) return buf @@ -1237,6 +1296,18 @@ def vfu_sgl_put(ctx, sg, iovec, cnt=1): return lib.vfu_sgl_put(ctx, sg, iovec, cnt) +def vfu_sgl_read(ctx, sg, cnt=1): + data = bytearray(sum([sge.length for sge in sg])) + buf = (c.c_byte * len(data)).from_buffer(data) + return lib.vfu_sgl_read(ctx, sg, cnt, buf), data + + +def vfu_sgl_write(ctx, sg, cnt=1, data=bytearray()): + assert len(data) == sum([sge.length for sge in sg]) + buf = (c.c_byte * len(data)).from_buffer(data) + return lib.vfu_sgl_write(ctx, sg, cnt, buf) + + def vfu_create_ioeventfd(ctx, region_idx, fd, gpa_offset, size, flags, datamatch, shadow_fd=-1, shadow_offset=0): assert ctx is not None diff --git a/test/py/meson.build b/test/py/meson.build index 0ea9f08b..ecd2fe2c 100644 --- a/test/py/meson.build +++ b/test/py/meson.build @@ -45,6 +45,7 @@ python_tests = [ 'test_request_errors.py', 'test_setup_region.py', 'test_sgl_get_put.py', + 'test_sgl_read_write.py', 'test_vfu_create_ctx.py', 'test_vfu_realize_ctx.py', ] diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py new file mode 100644 index 00000000..8334ea44 --- /dev/null +++ b/test/py/test_sgl_read_write.py @@ -0,0 +1,182 @@ +# +# Copyright (c) 2023 Nutanix Inc. All rights reserved. +# Copyright (c) 2023 Rivos Inc. All rights reserved. +# +# Authors: Mattias Nissler +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# * Neither the name of Nutanix nor the names of its contributors may be +# used to endorse or promote products derived from this software without +# specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL BE LIABLE FOR ANY +# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH +# DAMAGE. +# + +from libvfio_user import * +import select +import threading + +MAP_ADDR = 0x10000000 +MAP_SIZE = 16 << PAGE_SHIFT + +ctx = None +client = None + + +class DMARegionHandler: + """ + A helper to service DMA region accesses arriving over a socket. Accesses + are performed against an internal bytearray buffer. DMA request processing + takes place on a separate thread so as to not block the test code. + """ + + def handle_requests(sock, pipe, buf, lock, addr, error_no): + while True: + (ready, _, _) = select.select([sock, pipe], [], []) + if pipe in ready: + break + + # Read a command from the socket and service it. + _, msg_id, cmd, payload = get_msg_fds(sock, + VFIO_USER_F_TYPE_COMMAND) + assert cmd in [VFIO_USER_DMA_READ, VFIO_USER_DMA_WRITE] + access, data = vfio_user_dma_region_access.pop_from_buffer(payload) + + assert access.addr >= addr + assert access.addr + access.count <= addr + len(buf) + + offset = access.addr - addr + with lock: + if cmd == VFIO_USER_DMA_READ: + data = buf[offset:offset + access.count] + else: + buf[offset:offset + access.count] = data + data = bytearray() + + send_msg(sock, + cmd, + VFIO_USER_F_TYPE_REPLY, + payload=payload[:c.sizeof(access)] + data, + msg_id=msg_id, + error_no=error_no) + + os.close(pipe) + sock.close() + + def __init__(self, sock, addr, size, error_no=0): + self.data = bytearray(size) + self.data_lock = threading.Lock() + self.addr = addr + (pipe_r, self.pipe_w) = os.pipe() + # Duplicate the socket file descriptor so the thread can own it and + # make sure it gets closed only when terminating the thread. + sock = socket.socket(fileno=os.dup(sock.fileno())) + thread = threading.Thread( + target=DMARegionHandler.handle_requests, + args=[sock, pipe_r, self.data, self.data_lock, addr, error_no]) + thread.start() + + def shutdown(self): + # Closing the pipe's write end will signal the thread to terminate. + os.close(self.pipe_w) + + def read(self, addr, size): + offset = addr - self.addr + with self.data_lock: + return self.data[offset:offset + size] + + +def setup_function(function): + global ctx, client, dma_handler + ctx = prepare_ctx_for_dma() + assert ctx is not None + client = connect_client(ctx, max_data_xfer_size=PAGE_SIZE, cmd_conn=True) + + payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), + flags=(VFIO_USER_F_DMA_REGION_READ + | VFIO_USER_F_DMA_REGION_WRITE), + offset=0, + addr=MAP_ADDR, + size=MAP_SIZE) + + msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) + + dma_handler = DMARegionHandler(client.cmd_sock, payload.addr, payload.size) + + +def teardown_function(function): + dma_handler.shutdown() + client.disconnect(ctx) + vfu_destroy_ctx(ctx) + + +def test_dma_read_write(): + ret, sg = vfu_addr_to_sgl(ctx, + dma_addr=MAP_ADDR + 0x1000, + length=64, + max_nr_sgs=1, + prot=mmap.PROT_READ | mmap.PROT_WRITE) + assert ret == 1 + + data = bytearray([x & 0xff for x in range(0, sg[0].length)]) + assert vfu_sgl_write(ctx, sg, 1, data) == 0 + + assert vfu_sgl_read(ctx, sg, 1) == (0, data) + + assert dma_handler.read(sg[0].dma_addr + sg[0].offset, + sg[0].length) == data + + +def test_dma_read_write_large(): + ret, sg = vfu_addr_to_sgl(ctx, + dma_addr=MAP_ADDR + 0x1000, + length=2 * PAGE_SIZE, + max_nr_sgs=1, + prot=mmap.PROT_READ | mmap.PROT_WRITE) + assert ret == 1 + + data = bytearray([x & 0xff for x in range(0, sg[0].length)]) + assert vfu_sgl_write(ctx, sg, 1, data) == 0 + + assert vfu_sgl_read(ctx, sg, 1) == (0, data) + + assert dma_handler.read(sg[0].dma_addr + sg[0].offset, + sg[0].length) == data + + +def test_dma_read_write_error(): + # Reinitialize the handler to return EIO. + global dma_handler + dma_handler.shutdown() + dma_handler = DMARegionHandler(client.cmd_sock, MAP_ADDR, MAP_SIZE, + error_no=errno.EIO) + + ret, sg = vfu_addr_to_sgl(ctx, + dma_addr=MAP_ADDR + 0x1000, + length=64, + max_nr_sgs=1, + prot=mmap.PROT_READ | mmap.PROT_WRITE) + assert ret == 1 + + ret, _ = vfu_sgl_read(ctx, sg, 1) + assert ret == -1 + assert c.get_errno() == errno.EIO + + +# ex: set tabstop=4 shiftwidth=4 softtabstop=4 expandtab: # From 7eb008210e703d2608ef26f691213e923bec79b1 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Mon, 24 Jul 2023 09:11:16 -0700 Subject: [PATCH 04/13] Describe socket separation in protocol specification Include rationale and operation of separate server-to-client command channel in the protocol description and indicate which commands should be processed on the separate socket. Signed-off-by: Mattias Nissler --- docs/vfio-user.rst | 82 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 24 deletions(-) diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index 9e1314e3..94e7145a 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -204,12 +204,32 @@ A server can serve: 1) one or more clients, and/or 2) one or more virtual devices, belonging to one or more clients. -The current protocol specification requires a dedicated socket per -client/server connection. It is a server-side implementation detail whether a -single server handles multiple virtual devices from the same or multiple -clients. The location of the socket is implementation-specific. Multiplexing -clients, devices, and servers over the same socket is not supported in this -version of the protocol. +The current protocol specification requires dedicated sockets per +client/server connection. Commands in the client-to-server direction are +handled on the main communication socket which the client connects to, and +replies to these commands are passed on the same socket. Commands sent in the +other direction from the server to the client as well as their corresponding +replies, are exchanged on a separate socket, which is set up during negotiation +(AF_UNIX servers just pass the file descriptor). + +Using separate sockets for each command channel avoids introducing an +artificial point of synchronization between the channels. This simplifies +implementations since it obviates the need to demultiplex incoming messages +into commands and replies and interleave command handling and reply processing. +Note that it is still illegal for implementations to stall command or reply +processing indefinitely while waiting for replies on the other channel, as this +may lead to deadlocks. However, since incoming commands and requests arrive on +different sockets, it's possible to meet this requirement e.g. by running two +independent request processing threads that can internally operate +synchronously. It is expected that this is simpler to implement than fully +asynchronous message handling code. Implementations may still choose a fully +asynchronous, event-based design for other reasons, and the protocol fully +supports it. + +It is a server-side implementation detail whether a single server handles +multiple virtual devices from the same or multiple clients. The location of the +socket is implementation-specific. Multiplexing clients, devices, and servers +over the same socket is not supported in this version of the protocol. Authentication -------------- @@ -488,21 +508,31 @@ format: Capabilities: -+--------------------+--------+------------------------------------------------+ -| Name | Type | Description | -+====================+========+================================================+ -| max_msg_fds | number | Maximum number of file descriptors that can be | -| | | received by the sender in one message. | -| | | Optional. If not specified then the receiver | -| | | must assume a value of ``1``. | -+--------------------+--------+------------------------------------------------+ -| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | -| | | see `Read and Write Operations`_. Optional, | -| | | with a default value of 1048576 bytes. | -+--------------------+--------+------------------------------------------------+ -| migration | object | Migration capability parameters. If missing | -| | | then migration is not supported by the sender. | -+--------------------+--------+------------------------------------------------+ ++--------------------+---------+-----------------------------------------------+ +| Name | Type | Description | ++====================+=========+===============================================+ +| max_msg_fds | number | Maximum number of file descriptors that can | +| | | be received by the sender in one message. | +| | | Optional. If not specified then the receiver | +| | | must assume a value of ``1``. | ++--------------------+---------+-----------------------------------------------+ +| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | +| | | see `Read and Write Operations`_. Optional, | +| | | with a default value of 1048576 bytes. | ++--------------------+---------+-----------------------------------------------+ +| migration | object | Migration capability parameters. If missing | +| | | then migration is not supported by the | +| | | sender. | ++--------------------+---------+-----------------------------------------------+ +| cmd_conn | boolean | Indicates whether the client is capable of | +| | | using a separate socket as the channel for | +| | | server-to-client commands. If specified, the | +| | | server will pass a file descriptor along with | +| | | its reply. This is enabled by request only | +| | | for the benefit of clients that implement | +| | | older drafts of this specification which did | +| | | not include independent sockets per channel. | ++--------------------+---------+-----------------------------------------------+ The migration capability contains the following name/value pairs: @@ -517,7 +547,9 @@ Reply ^^^^^ The same message format is used in the server's reply with the semantics -described above. +described above. In case the client specified ``cmd_conn`` in its capabilities, +the server may pass a file descriptor to use for the server-to-client command +channel. ``VFIO_USER_DMA_MAP`` --------------------- @@ -1399,7 +1431,8 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -read from guest memory. +read from guest memory. This message and its reply are passed over the separate +server-to-client socket if negotiated at connection setup. Request ^^^^^^^ @@ -1437,7 +1470,8 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -write to guest memory. +write to guest memory. This message and its reply are passed over the separate +server-to-client socket if negotiated at connection setup. Request ^^^^^^^ From ee1de6f4e4c8c8cc1174d8f13b55d4f511c62a52 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Wed, 23 Aug 2023 04:59:36 -0700 Subject: [PATCH 05/13] Address John's review comments. --- docs/vfio-user.rst | 21 ++++++------ lib/tran.c | 61 ++++++++++++++++++---------------- lib/tran.h | 4 +-- lib/tran_sock.c | 16 +++++---- test/py/libvfio_user.py | 14 ++++---- test/py/test_sgl_read_write.py | 8 +++-- 6 files changed, 66 insertions(+), 58 deletions(-) diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index 94e7145a..1b38308c 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -524,14 +524,15 @@ Capabilities: | | | then migration is not supported by the | | | | sender. | +--------------------+---------+-----------------------------------------------+ -| cmd_conn | boolean | Indicates whether the client is capable of | +| reverse_cmd_socket | boolean | Indicates whether the client is capable of | | | | using a separate socket as the channel for | -| | | server-to-client commands. If specified, the | -| | | server will pass a file descriptor along with | -| | | its reply. This is enabled by request only | -| | | for the benefit of clients that implement | -| | | older drafts of this specification which did | -| | | not include independent sockets per channel. | +| | | server-to-client commands. If specified and | +| | | the server supports it, it will pass a file | +| | | descriptor along with its reply. This is | +| | | enabled by request only for the benefit of | +| | | clients that implement older drafts of this | +| | | specification which did not include | +| | | independent sockets per channel. | +--------------------+---------+-----------------------------------------------+ The migration capability contains the following name/value pairs: @@ -547,9 +548,9 @@ Reply ^^^^^ The same message format is used in the server's reply with the semantics -described above. In case the client specified ``cmd_conn`` in its capabilities, -the server may pass a file descriptor to use for the server-to-client command -channel. +described above. In case the client specified ``reverse_cmd_socket`` in its +capabilities, the server may pass a file descriptor to use for the +server-to-client command channel. ``VFIO_USER_DMA_MAP`` --------------------- diff --git a/lib/tran.c b/lib/tran.c index db02f7bb..755c8a9c 100644 --- a/lib/tran.c +++ b/lib/tran.c @@ -56,6 +56,7 @@ * "migration": { * "pgsize": 4096 * } + * "reverse_cmd_socket": true, * } * } * @@ -65,7 +66,7 @@ int tran_parse_version_json(const char *json_str, int *client_max_fdsp, size_t *client_max_data_xfer_sizep, size_t *pgsizep, - bool *enable_cmd_conn) + bool *reverse_cmd_socket_supportedp) { struct json_object *jo_caps = NULL; struct json_object *jo_top = NULL; @@ -131,13 +132,13 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, } } - if (json_object_object_get_ex(jo_caps, "cmd_conn", &jo)) { + if (json_object_object_get_ex(jo_caps, "reverse_cmd_socket", &jo)) { if (json_object_get_type(jo) != json_type_boolean) { goto out; } errno = 0; - *enable_cmd_conn = json_object_get_boolean(jo); + *reverse_cmd_socket_supportedp = json_object_get_boolean(jo); if (errno != 0) { goto out; @@ -157,7 +158,8 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, static int recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, - struct vfio_user_version **versionp) + struct vfio_user_version **versionp, + bool *reverse_cmd_socket_supportedp) { struct vfio_user_version *cversion = NULL; vfu_msg_t msg = { { 0 } }; @@ -222,7 +224,7 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, ret = tran_parse_version_json(json_str, &vfu_ctx->client_max_fds, &vfu_ctx->client_max_data_xfer_size, - &pgsize, &vfu_ctx->enable_cmd_conn); + &pgsize, reverse_cmd_socket_supportedp); if (ret < 0) { /* No client-supplied strings in the log for release build. */ @@ -282,7 +284,7 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, static int send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, - struct vfio_user_version *cversion, int *cmd_conn_fd) + struct vfio_user_version *cversion, int reverse_cmd_socket_fd) { struct vfio_user_version sversion = { 0 }; struct iovec iovecs[2] = { { 0 } }; @@ -290,7 +292,6 @@ send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, vfu_msg_t msg = { { 0 } }; int slen; int ret; - int cmd_conn_fds[2] = {-1, -1}; if (vfu_ctx->migration == NULL) { slen = snprintf(server_caps, sizeof(server_caps), @@ -329,53 +330,55 @@ send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, msg.hdr.msg_id = msg_id; msg.out_iovecs = iovecs; msg.nr_out_iovecs = 2; - - if (vfu_ctx->enable_cmd_conn && vfu_ctx->client_max_fds > 0 && - cmd_conn_fd != NULL) { - if (socketpair(AF_UNIX, SOCK_STREAM, 0, cmd_conn_fds) == -1) { - return -1; - } - - msg.out.fds = cmd_conn_fds; + if (reverse_cmd_socket_fd != -1) { + msg.out.fds = &reverse_cmd_socket_fd; msg.out.nr_fds = 1; } ret = vfu_ctx->tran->reply(vfu_ctx, &msg, 0); - /* - * The remote end of the cmd connection is no longer needed, the local only - * if successful. - */ - close_safely(&cmd_conn_fds[0]); - if (ret == -1) { - close_safely(&cmd_conn_fds[1]); - } else if (cmd_conn_fd) { - *cmd_conn_fd = cmd_conn_fds[1]; - } - return ret; } int -tran_negotiate(vfu_ctx_t *vfu_ctx, int *cmd_conn_fd) +tran_negotiate(vfu_ctx_t *vfu_ctx, int *reverse_cmd_socket_fdp) { struct vfio_user_version *client_version = NULL; + int reverse_cmd_socket_fds[2] = { -1, -1 }; + bool reverse_cmd_socket_supported = false; uint16_t msg_id = 0x0bad; int ret; - ret = recv_version(vfu_ctx, &msg_id, &client_version); + ret = recv_version(vfu_ctx, &msg_id, &client_version, + &reverse_cmd_socket_supported); if (ret < 0) { vfu_log(vfu_ctx, LOG_ERR, "failed to recv version: %m"); return ret; } - ret = send_version(vfu_ctx, msg_id, client_version, cmd_conn_fd); + if (reverse_cmd_socket_supported && reverse_cmd_socket_fdp != NULL && + vfu_ctx->client_max_fds > 0) { + if (socketpair(AF_UNIX, SOCK_STREAM, 0, reverse_cmd_socket_fds) == -1) { + return -1; + } + } + + ret = send_version(vfu_ctx, msg_id, client_version, + reverse_cmd_socket_fds[0]); free(client_version); + /* + * The remote end of the reverse cmd socket pair is no longer needed, the + * local one only if successful. + */ + close_safely(&reverse_cmd_socket_fds[0]); if (ret < 0) { vfu_log(vfu_ctx, LOG_ERR, "failed to send version: %m"); + close_safely(&reverse_cmd_socket_fds[1]); + } else if (reverse_cmd_socket_fdp != NULL) { + *reverse_cmd_socket_fdp = reverse_cmd_socket_fds[1]; } return ret; diff --git a/lib/tran.h b/lib/tran.h index 7cf6081c..8a5aafe0 100644 --- a/lib/tran.h +++ b/lib/tran.h @@ -73,10 +73,10 @@ struct transport_ops { int tran_parse_version_json(const char *json_str, int *client_max_fdsp, size_t *client_max_data_xfer_sizep, size_t *pgsizep, - bool *enable_cmd_conn); + bool *reverse_cmd_socket_supportedp); int -tran_negotiate(vfu_ctx_t *vfu_ctx, int *cmd_conn_fd); +tran_negotiate(vfu_ctx_t *vfu_ctx, int *reverse_cmd_socket_fdp); #endif /* LIB_VFIO_USER_TRAN_H */ diff --git a/lib/tran_sock.c b/lib/tran_sock.c index 0263a1b7..c4323910 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -46,7 +46,7 @@ typedef struct { int listen_fd; int conn_fd; - int cmd_conn_fd; + int reverse_cmd_socket_fd; } tran_sock_t; int @@ -381,7 +381,7 @@ tran_sock_init(vfu_ctx_t *vfu_ctx) ts->listen_fd = -1; ts->conn_fd = -1; - ts->cmd_conn_fd = -1; + ts->reverse_cmd_socket_fd = -1; if ((ts->listen_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { ret = errno; @@ -466,7 +466,7 @@ tran_sock_attach(vfu_ctx_t *vfu_ctx) return -1; } - ret = tran_negotiate(vfu_ctx, &ts->cmd_conn_fd); + ret = tran_negotiate(vfu_ctx, &ts->reverse_cmd_socket_fd); if (ret < 0) { close_safely(&ts->conn_fd); return -1; @@ -617,15 +617,17 @@ tran_sock_send_msg(vfu_ctx_t *vfu_ctx, uint16_t msg_id, void *recv_data, size_t recv_len) { tran_sock_t *ts; + int fd; assert(vfu_ctx != NULL); assert(vfu_ctx->tran_data != NULL); ts = vfu_ctx->tran_data; - /* NB. use the command socket file descriptor if available. */ - return tran_sock_msg(ts->cmd_conn_fd != -1 ? ts->cmd_conn_fd : ts->conn_fd, - msg_id, cmd, send_data, send_len, hdr, recv_data, + /* NB. use the reverse command socket file descriptor if available. */ + fd = ts->reverse_cmd_socket_fd != -1 ? ts->reverse_cmd_socket_fd + : ts->conn_fd; + return tran_sock_msg(fd, msg_id, cmd, send_data, send_len, hdr, recv_data, recv_len); } @@ -640,7 +642,7 @@ tran_sock_detach(vfu_ctx_t *vfu_ctx) if (ts != NULL) { close_safely(&ts->conn_fd); - close_safely(&ts->cmd_conn_fd); + close_safely(&ts->reverse_cmd_socket_fd); } } diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 9f14db22..ffd9353c 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -702,11 +702,11 @@ class Client: def __init__(self, sock=None): self.sock = sock - self.cmd_sock = None + self.reverse_cmd_sock = None def connect(self, ctx, max_data_xfer_size=VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE, - cmd_conn=False): + reverse_cmd_socket=False): self.sock = connect_sock() json = f''' @@ -714,7 +714,7 @@ def connect(self, ctx, "capabilities": {{ "max_data_xfer_size": {max_data_xfer_size}, "max_msg_fds": 8, - "cmd_conn": {str(cmd_conn).lower()} + "reverse_cmd_socket": {str(reverse_cmd_socket).lower()} }} }} ''' @@ -725,15 +725,15 @@ def connect(self, ctx, self.sock.send(hdr + payload) vfu_attach_ctx(ctx, expect=0) fds, payload = get_reply_fds(self.sock, expect=0) - self.cmd_sock = socket.socket(fileno=fds[0]) if fds else None + self.reverse_cmd_sock = socket.socket(fileno=fds[0]) if fds else None return self.sock def disconnect(self, ctx): self.sock.close() self.sock = None - if self.cmd_sock is not None: - self.cmd_sock.close() - self.cmd_sock = None + if self.reverse_cmd_sock is not None: + self.reverse_cmd_sock.close() + self.reverse_cmd_sock = None # notice client closed connection vfu_run_ctx(ctx, errno.ENOTCONN) diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py index 8334ea44..6150d4d4 100644 --- a/test/py/test_sgl_read_write.py +++ b/test/py/test_sgl_read_write.py @@ -106,7 +106,8 @@ def setup_function(function): global ctx, client, dma_handler ctx = prepare_ctx_for_dma() assert ctx is not None - client = connect_client(ctx, max_data_xfer_size=PAGE_SIZE, cmd_conn=True) + client = connect_client(ctx, max_data_xfer_size=PAGE_SIZE, + reverse_cmd_socket=True) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ @@ -117,7 +118,8 @@ def setup_function(function): msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) - dma_handler = DMARegionHandler(client.cmd_sock, payload.addr, payload.size) + dma_handler = DMARegionHandler(client.reverse_cmd_sock, payload.addr, + payload.size) def teardown_function(function): @@ -164,7 +166,7 @@ def test_dma_read_write_error(): # Reinitialize the handler to return EIO. global dma_handler dma_handler.shutdown() - dma_handler = DMARegionHandler(client.cmd_sock, MAP_ADDR, MAP_SIZE, + dma_handler = DMARegionHandler(client.reverse_cmd_sock, MAP_ADDR, MAP_SIZE, error_no=errno.EIO) ret, sg = vfu_addr_to_sgl(ctx, From 6cae619aa45977ef74196c55c6120a9d2a7b5b63 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Tue, 29 Aug 2023 01:27:08 -0700 Subject: [PATCH 06/13] More changes to address review comments by John and Thanos. --- docs/vfio-user.rst | 31 ++++++++++--------- include/libvfio-user.h | 2 +- lib/tran.c | 50 +++++++++++++++---------------- lib/tran.h | 4 +-- lib/tran_sock.c | 14 ++++----- test/py/libvfio_user.py | 54 ++++++++++++++++++++-------------- test/py/test_sgl_read_write.py | 21 ++++++++----- 7 files changed, 96 insertions(+), 80 deletions(-) diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index 1b38308c..0a75f5f9 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -209,8 +209,8 @@ client/server connection. Commands in the client-to-server direction are handled on the main communication socket which the client connects to, and replies to these commands are passed on the same socket. Commands sent in the other direction from the server to the client as well as their corresponding -replies, are exchanged on a separate socket, which is set up during negotiation -(AF_UNIX servers just pass the file descriptor). +replies can optionally be passed across a separate socket, which is set up +during negotiation (AF_UNIX servers just pass the file descriptor). Using separate sockets for each command channel avoids introducing an artificial point of synchronization between the channels. This simplifies @@ -524,15 +524,16 @@ Capabilities: | | | then migration is not supported by the | | | | sender. | +--------------------+---------+-----------------------------------------------+ -| reverse_cmd_socket | boolean | Indicates whether the client is capable of | -| | | using a separate socket as the channel for | -| | | server-to-client commands. If specified and | -| | | the server supports it, it will pass a file | -| | | descriptor along with its reply. This is | -| | | enabled by request only for the benefit of | -| | | clients that implement older drafts of this | -| | | specification which did not include | -| | | independent sockets per channel. | +| twin_socket | boolean | Indicates whether the client wants to use a | +| | | separate channel for server-to-client | +| | | commands. If specified and the server | +| | | supports it, it will include the file | +| | | descriptor for the client end of a separate | +| | | socket pair along with its reply. Some server | +| | | implementations may not support this, but it | +| | | is strongly recommended for servers which do | +| | | send server-to-client commands to implement | +| | | twin-socket support. | +--------------------+---------+-----------------------------------------------+ The migration capability contains the following name/value pairs: @@ -548,9 +549,11 @@ Reply ^^^^^ The same message format is used in the server's reply with the semantics -described above. In case the client specified ``reverse_cmd_socket`` in its -capabilities, the server may pass a file descriptor to use for the -server-to-client command channel. +described above. In case the client set ``twin_socket`` to true in its +capabilities, the server may include a file descriptor to use for the +server-to-client command channel in the reply. The index of the file descriptor +in the ancillary data of the reply is given by the ``twin_socket`` capability +field in the reply. ``VFIO_USER_DMA_MAP`` --------------------- diff --git a/include/libvfio-user.h b/include/libvfio-user.h index 72369b62..21cb99a5 100644 --- a/include/libvfio-user.h +++ b/include/libvfio-user.h @@ -61,7 +61,7 @@ extern "C" { #endif #define LIB_VFIO_USER_MAJOR 0 -#define LIB_VFIO_USER_MINOR 1 +#define LIB_VFIO_USER_MINOR 2 /* DMA addresses cannot be directly de-referenced. */ typedef void *vfu_dma_addr_t; diff --git a/lib/tran.c b/lib/tran.c index 755c8a9c..d2720b9a 100644 --- a/lib/tran.c +++ b/lib/tran.c @@ -56,7 +56,7 @@ * "migration": { * "pgsize": 4096 * } - * "reverse_cmd_socket": true, + * "twin_socket": true, * } * } * @@ -66,7 +66,7 @@ int tran_parse_version_json(const char *json_str, int *client_max_fdsp, size_t *client_max_data_xfer_sizep, size_t *pgsizep, - bool *reverse_cmd_socket_supportedp) + bool *twin_socket_supportedp) { struct json_object *jo_caps = NULL; struct json_object *jo_top = NULL; @@ -132,13 +132,13 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, } } - if (json_object_object_get_ex(jo_caps, "reverse_cmd_socket", &jo)) { + if (json_object_object_get_ex(jo_caps, "twin_socket", &jo)) { if (json_object_get_type(jo) != json_type_boolean) { goto out; } errno = 0; - *reverse_cmd_socket_supportedp = json_object_get_boolean(jo); + *twin_socket_supportedp = json_object_get_boolean(jo); if (errno != 0) { goto out; @@ -159,7 +159,7 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, static int recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, struct vfio_user_version **versionp, - bool *reverse_cmd_socket_supportedp) + bool *twin_socket_supportedp) { struct vfio_user_version *cversion = NULL; vfu_msg_t msg = { { 0 } }; @@ -224,7 +224,7 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, ret = tran_parse_version_json(json_str, &vfu_ctx->client_max_fds, &vfu_ctx->client_max_data_xfer_size, - &pgsize, reverse_cmd_socket_supportedp); + &pgsize, twin_socket_supportedp); if (ret < 0) { /* No client-supplied strings in the log for release build. */ @@ -284,14 +284,13 @@ recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, static int send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, - struct vfio_user_version *cversion, int reverse_cmd_socket_fd) + struct vfio_user_version *cversion, int client_cmd_socket_fd) { struct vfio_user_version sversion = { 0 }; struct iovec iovecs[2] = { { 0 } }; char server_caps[1024]; vfu_msg_t msg = { { 0 } }; int slen; - int ret; if (vfu_ctx->migration == NULL) { slen = snprintf(server_caps, sizeof(server_caps), @@ -330,55 +329,54 @@ send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, msg.hdr.msg_id = msg_id; msg.out_iovecs = iovecs; msg.nr_out_iovecs = 2; - if (reverse_cmd_socket_fd != -1) { - msg.out.fds = &reverse_cmd_socket_fd; + if (client_cmd_socket_fd != -1) { + msg.out.fds = &client_cmd_socket_fd; msg.out.nr_fds = 1; } - ret = vfu_ctx->tran->reply(vfu_ctx, &msg, 0); - - return ret; + return vfu_ctx->tran->reply(vfu_ctx, &msg, 0); } int -tran_negotiate(vfu_ctx_t *vfu_ctx, int *reverse_cmd_socket_fdp) +tran_negotiate(vfu_ctx_t *vfu_ctx, int *client_cmd_socket_fdp) { struct vfio_user_version *client_version = NULL; - int reverse_cmd_socket_fds[2] = { -1, -1 }; - bool reverse_cmd_socket_supported = false; + int client_cmd_socket_fds[2] = { -1, -1 }; + bool twin_socket_supported = false; uint16_t msg_id = 0x0bad; int ret; ret = recv_version(vfu_ctx, &msg_id, &client_version, - &reverse_cmd_socket_supported); + &twin_socket_supported); if (ret < 0) { vfu_log(vfu_ctx, LOG_ERR, "failed to recv version: %m"); return ret; } - if (reverse_cmd_socket_supported && reverse_cmd_socket_fdp != NULL && + if (twin_socket_supported && client_cmd_socket_fdp != NULL && vfu_ctx->client_max_fds > 0) { - if (socketpair(AF_UNIX, SOCK_STREAM, 0, reverse_cmd_socket_fds) == -1) { + if (socketpair(AF_UNIX, SOCK_STREAM, 0, client_cmd_socket_fds) == -1) { + vfu_log(vfu_ctx, LOG_ERR, "failed to create cmd socket: %m"); return -1; } } ret = send_version(vfu_ctx, msg_id, client_version, - reverse_cmd_socket_fds[0]); + client_cmd_socket_fds[0]); free(client_version); /* - * The remote end of the reverse cmd socket pair is no longer needed, the - * local one only if successful. + * The remote end of the client command socket pair is no longer needed. + * The local end is kept only if passed to the caller on successful return. */ - close_safely(&reverse_cmd_socket_fds[0]); + close_safely(&client_cmd_socket_fds[0]); if (ret < 0) { vfu_log(vfu_ctx, LOG_ERR, "failed to send version: %m"); - close_safely(&reverse_cmd_socket_fds[1]); - } else if (reverse_cmd_socket_fdp != NULL) { - *reverse_cmd_socket_fdp = reverse_cmd_socket_fds[1]; + close_safely(&client_cmd_socket_fds[1]); + } else if (client_cmd_socket_fdp != NULL) { + *client_cmd_socket_fdp = client_cmd_socket_fds[1]; } return ret; diff --git a/lib/tran.h b/lib/tran.h index 8a5aafe0..9ad1203d 100644 --- a/lib/tran.h +++ b/lib/tran.h @@ -73,10 +73,10 @@ struct transport_ops { int tran_parse_version_json(const char *json_str, int *client_max_fdsp, size_t *client_max_data_xfer_sizep, size_t *pgsizep, - bool *reverse_cmd_socket_supportedp); + bool *twin_socket_supportedp); int -tran_negotiate(vfu_ctx_t *vfu_ctx, int *reverse_cmd_socket_fdp); +tran_negotiate(vfu_ctx_t *vfu_ctx, int *client_cmd_socket_fdp); #endif /* LIB_VFIO_USER_TRAN_H */ diff --git a/lib/tran_sock.c b/lib/tran_sock.c index c4323910..d8330dcb 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -46,7 +46,7 @@ typedef struct { int listen_fd; int conn_fd; - int reverse_cmd_socket_fd; + int client_cmd_socket_fd; } tran_sock_t; int @@ -381,7 +381,7 @@ tran_sock_init(vfu_ctx_t *vfu_ctx) ts->listen_fd = -1; ts->conn_fd = -1; - ts->reverse_cmd_socket_fd = -1; + ts->client_cmd_socket_fd = -1; if ((ts->listen_fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { ret = errno; @@ -466,7 +466,7 @@ tran_sock_attach(vfu_ctx_t *vfu_ctx) return -1; } - ret = tran_negotiate(vfu_ctx, &ts->reverse_cmd_socket_fd); + ret = tran_negotiate(vfu_ctx, &ts->client_cmd_socket_fd); if (ret < 0) { close_safely(&ts->conn_fd); return -1; @@ -624,9 +624,9 @@ tran_sock_send_msg(vfu_ctx_t *vfu_ctx, uint16_t msg_id, ts = vfu_ctx->tran_data; - /* NB. use the reverse command socket file descriptor if available. */ - fd = ts->reverse_cmd_socket_fd != -1 ? ts->reverse_cmd_socket_fd - : ts->conn_fd; + /* NB. use the client command socket file descriptor if available. */ + fd = ts->client_cmd_socket_fd != -1 ? ts->client_cmd_socket_fd + : ts->conn_fd; return tran_sock_msg(fd, msg_id, cmd, send_data, send_len, hdr, recv_data, recv_len); } @@ -642,7 +642,7 @@ tran_sock_detach(vfu_ctx_t *vfu_ctx) if (ts != NULL) { close_safely(&ts->conn_fd); - close_safely(&ts->reverse_cmd_socket_fd); + close_safely(&ts->client_cmd_socket_fd); } } diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index ffd9353c..e7f7f9bb 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -32,6 +32,7 @@ # from types import SimpleNamespace +import collections.abc import ctypes as c import array import errno @@ -482,6 +483,7 @@ class vfio_user_dma_unmap(Structure): class vfio_user_dma_region_access(Structure): + """Payload for VFIO_USER_DMA_READ and VFIO_USER_DMA_WRITE.""" _pack_ = 1 _fields_ = [ ("addr", c.c_uint64), @@ -702,38 +704,46 @@ class Client: def __init__(self, sock=None): self.sock = sock - self.reverse_cmd_sock = None + self.client_cmd_socket = None - def connect(self, ctx, - max_data_xfer_size=VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE, - reverse_cmd_socket=False): + def connect(self, ctx, capabilities={}): self.sock = connect_sock() - json = f''' - {{ - "capabilities": {{ - "max_data_xfer_size": {max_data_xfer_size}, - "max_msg_fds": 8, - "reverse_cmd_socket": {str(reverse_cmd_socket).lower()} - }} - }} - ''' + effective_caps = { + "capabilities": { + "max_data_xfer_size": VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE, + "max_msg_fds": 8, + "twin_socket": False, + }, + } + + def update(target, overrides): + for k, v in overrides.items(): + if isinstance(v, collections.abc.Mapping): + target[k] = target.get(k, {}) + update(target[k], v) + else: + target[k] = v + + update(effective_caps, capabilities) + caps_json = json.dumps(effective_caps) + # struct vfio_user_version - payload = struct.pack("HH%dsc" % len(json), LIBVFIO_USER_MAJOR, - LIBVFIO_USER_MINOR, json.encode(), b'\0') + payload = struct.pack("HH%dsc" % len(caps_json), LIBVFIO_USER_MAJOR, + LIBVFIO_USER_MINOR, caps_json.encode(), b'\0') hdr = vfio_user_header(VFIO_USER_VERSION, size=len(payload)) self.sock.send(hdr + payload) vfu_attach_ctx(ctx, expect=0) fds, payload = get_reply_fds(self.sock, expect=0) - self.reverse_cmd_sock = socket.socket(fileno=fds[0]) if fds else None + self.client_cmd_socket = socket.socket(fileno=fds[0]) if fds else None return self.sock def disconnect(self, ctx): self.sock.close() self.sock = None - if self.reverse_cmd_sock is not None: - self.reverse_cmd_sock.close() - self.reverse_cmd_sock = None + if self.client_cmd_socket is not None: + self.client_cmd_socket.close() + self.client_cmd_socket = None # notice client closed connection vfu_run_ctx(ctx, errno.ENOTCONN) @@ -795,7 +805,7 @@ def msg(ctx, sock, cmd, payload=bytearray(), expect=0, fds=None, return get_reply(sock, expect=expect) -def get_msg_fds(sock, expect_type, expect=0): +def get_msg_fds(sock, expect_msg_type, expect_errno=0): """ Receives a message from a socket and pulls the returned file descriptors out of the message. @@ -805,7 +815,7 @@ def get_msg_fds(sock, expect_type, expect=0): socket.CMSG_LEN(64 * fds.itemsize)) (msg_id, cmd, msg_size, msg_flags, errno) = struct.unpack("HHIII", data[0:16]) - assert errno == expect + assert errno == expect_errno cmsg_level, cmsg_type, packed_fd = ancillary[0] if len(ancillary) != 0 \ else (0, 0, []) @@ -814,7 +824,7 @@ def get_msg_fds(sock, expect_type, expect=0): [unpacked_fd] = struct.unpack_from("i", packed_fd, offset=i) unpacked_fds.append(unpacked_fd) assert len(packed_fd)/4 == len(unpacked_fds) - assert (msg_flags & 0xf) == expect_type + assert (msg_flags & 0xf) == expect_msg_type return (unpacked_fds, msg_id, cmd, data[16:]) diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py index 6150d4d4..fa386a62 100644 --- a/test/py/test_sgl_read_write.py +++ b/test/py/test_sgl_read_write.py @@ -46,13 +46,13 @@ class DMARegionHandler: takes place on a separate thread so as to not block the test code. """ - def handle_requests(sock, pipe, buf, lock, addr, error_no): + def __handle_requests(sock, pipe, buf, lock, addr, error_no): while True: (ready, _, _) = select.select([sock, pipe], [], []) if pipe in ready: break - # Read a command from the socket and service it. +# Read a command from the socket and service it. _, msg_id, cmd, payload = get_msg_fds(sock, VFIO_USER_F_TYPE_COMMAND) assert cmd in [VFIO_USER_DMA_READ, VFIO_USER_DMA_WRITE] @@ -88,7 +88,7 @@ def __init__(self, sock, addr, size, error_no=0): # make sure it gets closed only when terminating the thread. sock = socket.socket(fileno=os.dup(sock.fileno())) thread = threading.Thread( - target=DMARegionHandler.handle_requests, + target=DMARegionHandler.__handle_requests, args=[sock, pipe_r, self.data, self.data_lock, addr, error_no]) thread.start() @@ -106,8 +106,13 @@ def setup_function(function): global ctx, client, dma_handler ctx = prepare_ctx_for_dma() assert ctx is not None - client = connect_client(ctx, max_data_xfer_size=PAGE_SIZE, - reverse_cmd_socket=True) + caps = { + "capabilities": { + "max_data_xfer_size": PAGE_SIZE, + "twin_socket": True, + } + } + client = connect_client(ctx, caps) payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ @@ -118,7 +123,7 @@ def setup_function(function): msg(ctx, client.sock, VFIO_USER_DMA_MAP, payload) - dma_handler = DMARegionHandler(client.reverse_cmd_sock, payload.addr, + dma_handler = DMARegionHandler(client.client_cmd_socket, payload.addr, payload.size) @@ -166,8 +171,8 @@ def test_dma_read_write_error(): # Reinitialize the handler to return EIO. global dma_handler dma_handler.shutdown() - dma_handler = DMARegionHandler(client.reverse_cmd_sock, MAP_ADDR, MAP_SIZE, - error_no=errno.EIO) + dma_handler = DMARegionHandler(client.client_cmd_socket, MAP_ADDR, + MAP_SIZE, error_no=errno.EIO) ret, sg = vfu_addr_to_sgl(ctx, dma_addr=MAP_ADDR + 0x1000, From 54b70e1eff3c368c726bb7d115ea8e6ed973a9cb Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Thu, 31 Aug 2023 03:46:09 -0700 Subject: [PATCH 07/13] Remove spec changes, they are now in a different MR. --- docs/vfio-user.rst | 86 +++++++++++++--------------------------------- 1 file changed, 24 insertions(+), 62 deletions(-) diff --git a/docs/vfio-user.rst b/docs/vfio-user.rst index 0a75f5f9..9e1314e3 100644 --- a/docs/vfio-user.rst +++ b/docs/vfio-user.rst @@ -204,32 +204,12 @@ A server can serve: 1) one or more clients, and/or 2) one or more virtual devices, belonging to one or more clients. -The current protocol specification requires dedicated sockets per -client/server connection. Commands in the client-to-server direction are -handled on the main communication socket which the client connects to, and -replies to these commands are passed on the same socket. Commands sent in the -other direction from the server to the client as well as their corresponding -replies can optionally be passed across a separate socket, which is set up -during negotiation (AF_UNIX servers just pass the file descriptor). - -Using separate sockets for each command channel avoids introducing an -artificial point of synchronization between the channels. This simplifies -implementations since it obviates the need to demultiplex incoming messages -into commands and replies and interleave command handling and reply processing. -Note that it is still illegal for implementations to stall command or reply -processing indefinitely while waiting for replies on the other channel, as this -may lead to deadlocks. However, since incoming commands and requests arrive on -different sockets, it's possible to meet this requirement e.g. by running two -independent request processing threads that can internally operate -synchronously. It is expected that this is simpler to implement than fully -asynchronous message handling code. Implementations may still choose a fully -asynchronous, event-based design for other reasons, and the protocol fully -supports it. - -It is a server-side implementation detail whether a single server handles -multiple virtual devices from the same or multiple clients. The location of the -socket is implementation-specific. Multiplexing clients, devices, and servers -over the same socket is not supported in this version of the protocol. +The current protocol specification requires a dedicated socket per +client/server connection. It is a server-side implementation detail whether a +single server handles multiple virtual devices from the same or multiple +clients. The location of the socket is implementation-specific. Multiplexing +clients, devices, and servers over the same socket is not supported in this +version of the protocol. Authentication -------------- @@ -508,33 +488,21 @@ format: Capabilities: -+--------------------+---------+-----------------------------------------------+ -| Name | Type | Description | -+====================+=========+===============================================+ -| max_msg_fds | number | Maximum number of file descriptors that can | -| | | be received by the sender in one message. | -| | | Optional. If not specified then the receiver | -| | | must assume a value of ``1``. | -+--------------------+---------+-----------------------------------------------+ -| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | -| | | see `Read and Write Operations`_. Optional, | -| | | with a default value of 1048576 bytes. | -+--------------------+---------+-----------------------------------------------+ -| migration | object | Migration capability parameters. If missing | -| | | then migration is not supported by the | -| | | sender. | -+--------------------+---------+-----------------------------------------------+ -| twin_socket | boolean | Indicates whether the client wants to use a | -| | | separate channel for server-to-client | -| | | commands. If specified and the server | -| | | supports it, it will include the file | -| | | descriptor for the client end of a separate | -| | | socket pair along with its reply. Some server | -| | | implementations may not support this, but it | -| | | is strongly recommended for servers which do | -| | | send server-to-client commands to implement | -| | | twin-socket support. | -+--------------------+---------+-----------------------------------------------+ ++--------------------+--------+------------------------------------------------+ +| Name | Type | Description | ++====================+========+================================================+ +| max_msg_fds | number | Maximum number of file descriptors that can be | +| | | received by the sender in one message. | +| | | Optional. If not specified then the receiver | +| | | must assume a value of ``1``. | ++--------------------+--------+------------------------------------------------+ +| max_data_xfer_size | number | Maximum ``count`` for data transfer messages; | +| | | see `Read and Write Operations`_. Optional, | +| | | with a default value of 1048576 bytes. | ++--------------------+--------+------------------------------------------------+ +| migration | object | Migration capability parameters. If missing | +| | | then migration is not supported by the sender. | ++--------------------+--------+------------------------------------------------+ The migration capability contains the following name/value pairs: @@ -549,11 +517,7 @@ Reply ^^^^^ The same message format is used in the server's reply with the semantics -described above. In case the client set ``twin_socket`` to true in its -capabilities, the server may include a file descriptor to use for the -server-to-client command channel in the reply. The index of the file descriptor -in the ancillary data of the reply is given by the ``twin_socket`` capability -field in the reply. +described above. ``VFIO_USER_DMA_MAP`` --------------------- @@ -1435,8 +1399,7 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -read from guest memory. This message and its reply are passed over the separate -server-to-client socket if negotiated at connection setup. +read from guest memory. Request ^^^^^^^ @@ -1474,8 +1437,7 @@ Reply ----------------------- If the client has not shared mappable memory, the server can use this message to -write to guest memory. This message and its reply are passed over the separate -server-to-client socket if negotiated at connection setup. +write to guest memory. Request ^^^^^^^ From 654829e012e0a372c7beacbea95ef2a951550554 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Tue, 29 Aug 2023 08:03:18 -0700 Subject: [PATCH 08/13] Specify client command socket file descriptor index in server capabilities. --- lib/tran.c | 54 ++++++++++++++++++++++++++-------- test/py/libvfio_user.py | 17 +++++++---- test/py/test_sgl_read_write.py | 5 +++- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/lib/tran.c b/lib/tran.c index 12301bf0..af6b813f 100644 --- a/lib/tran.c +++ b/lib/tran.c @@ -53,11 +53,14 @@ * { * "capabilities": { * "max_msg_fds": 32, - * "max_data_xfer_size": 1048576 + * "max_data_xfer_size": 1048576, * "migration": { * "pgsize": 4096 + * }, + * "twin_socket": { + * "enable": true, + * "fd_index": 0 * } - * "twin_socket": true, * } * } * @@ -134,15 +137,23 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, } if (json_object_object_get_ex(jo_caps, "twin_socket", &jo)) { - if (json_object_get_type(jo) != json_type_boolean) { + struct json_object *jo2 = NULL; + + if (json_object_get_type(jo) != json_type_object) { goto out; } - errno = 0; - *twin_socket_supportedp = json_object_get_boolean(jo); + if (json_object_object_get_ex(jo, "enable", &jo2)) { + if (json_object_get_type(jo2) != json_type_boolean) { + goto out; + } - if (errno != 0) { - goto out; + errno = 0; + *twin_socket_supportedp = json_object_get_boolean(jo2); + + if (errno != 0) { + goto out; + } } } @@ -159,8 +170,7 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, static int recv_version(vfu_ctx_t *vfu_ctx, uint16_t *msg_idp, - struct vfio_user_version **versionp, - bool *twin_socket_supportedp) + struct vfio_user_version **versionp, bool *twin_socket_supportedp) { struct vfio_user_version *cversion = NULL; vfu_msg_t msg = { { 0 } }; @@ -329,8 +339,9 @@ json_add_uint64(struct json_object *jso, const char *key, uint64_t value) * be freed by the caller. */ static char * -format_server_capabilities(vfu_ctx_t *vfu_ctx) +format_server_capabilities(vfu_ctx_t *vfu_ctx, int twin_socket_fd_index) { + struct json_object *jo_twin_socket = NULL; struct json_object *jo_migration = NULL; struct json_object *jo_caps = NULL; struct json_object *jo_top = NULL; @@ -364,6 +375,21 @@ format_server_capabilities(vfu_ctx_t *vfu_ctx) } } + if (twin_socket_fd_index >= 0) { + if ((jo_twin_socket = json_object_new_object()) == NULL) { + goto out; + } + + if (json_add_uint64(jo_twin_socket, "fd_index", + twin_socket_fd_index) < 0) { + goto out; + } + + if (json_add(jo_caps, "twin_socket", &jo_twin_socket) < 0) { + goto out; + } + } + if ((jo_top = json_object_new_object()) == NULL || json_add(jo_top, "capabilities", &jo_caps) < 0) { goto out; @@ -372,6 +398,7 @@ format_server_capabilities(vfu_ctx_t *vfu_ctx) caps_str = strdup(json_object_to_json_string(jo_top)); out: + json_object_put(jo_twin_socket); json_object_put(jo_migration); json_object_put(jo_caps); json_object_put(jo_top); @@ -382,13 +409,15 @@ static int send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, struct vfio_user_version *cversion, int client_cmd_socket_fd) { + int twin_socket_fd_index = client_cmd_socket_fd >= 0 ? 0 : -1; struct vfio_user_version sversion = { 0 }; struct iovec iovecs[2] = { { 0 } }; vfu_msg_t msg = { { 0 } }; char *server_caps = NULL; int ret; - if ((server_caps = format_server_capabilities(vfu_ctx)) == NULL) { + server_caps = format_server_capabilities(vfu_ctx, twin_socket_fd_index); + if (server_caps == NULL) { errno = ENOMEM; return -1; } @@ -408,9 +437,10 @@ send_version(vfu_ctx_t *vfu_ctx, uint16_t msg_id, msg.hdr.msg_id = msg_id; msg.out_iovecs = iovecs; msg.nr_out_iovecs = 2; - if (client_cmd_socket_fd != -1) { + if (client_cmd_socket_fd >= 0) { msg.out.fds = &client_cmd_socket_fd; msg.out.nr_fds = 1; + assert(msg.out.fds[twin_socket_fd_index] == client_cmd_socket_fd); } ret = vfu_ctx->tran->reply(vfu_ctx, &msg, 0); diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 0c0687c7..33830582 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -712,11 +712,10 @@ def __init__(self, sock=None): def connect(self, ctx, capabilities={}): self.sock = connect_sock() - effective_caps = { + client_caps = { "capabilities": { "max_data_xfer_size": VFIO_USER_DEFAULT_MAX_DATA_XFER_SIZE, "max_msg_fds": 8, - "twin_socket": False, }, } @@ -728,8 +727,8 @@ def update(target, overrides): else: target[k] = v - update(effective_caps, capabilities) - caps_json = json.dumps(effective_caps) + update(client_caps, capabilities) + caps_json = json.dumps(client_caps) # struct vfio_user_version payload = struct.pack("HH%dsc" % len(caps_json), LIBVFIO_USER_MAJOR, @@ -738,7 +737,15 @@ def update(target, overrides): self.sock.send(hdr + payload) vfu_attach_ctx(ctx, expect=0) fds, payload = get_reply_fds(self.sock, expect=0) - self.client_cmd_socket = socket.socket(fileno=fds[0]) if fds else None + + server_caps = json.loads(payload[struct.calcsize("HH"):-1].decode()) + try: + if client_caps["capabilities"]["twin_socket"]["enable"]: + index = server_caps["capabilities"]["twin_socket"]["fd_index"] + self.client_cmd_socket = socket.socket(fileno=fds[index]) + except KeyError: + pass + return self.sock def disconnect(self, ctx): diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py index fa386a62..efe4feaf 100644 --- a/test/py/test_sgl_read_write.py +++ b/test/py/test_sgl_read_write.py @@ -109,10 +109,13 @@ def setup_function(function): caps = { "capabilities": { "max_data_xfer_size": PAGE_SIZE, - "twin_socket": True, + "twin_socket": { + "enable": True, + }, } } client = connect_client(ctx, caps) + assert client.client_cmd_socket is not None payload = vfio_user_dma_map(argsz=len(vfio_user_dma_map()), flags=(VFIO_USER_F_DMA_REGION_READ From cc2551ee4ab9460e6dcae200f445cca22f851b7f Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Tue, 29 Aug 2023 08:19:09 -0700 Subject: [PATCH 09/13] Add concurrent command warning. --- lib/tran_sock.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/tran_sock.c b/lib/tran_sock.c index a03f94ef..0777c601 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -609,6 +609,23 @@ tran_sock_reply(vfu_ctx_t *vfu_ctx, vfu_msg_t *msg, int err) return ret; } +static void maybe_print_cmd_collision_warning(vfu_ctx_t *vfu_ctx) { + static bool warning_printed = false; + static const char *warning_msg = + "You are using libvfio-user in a configuration that issues " + "client-to-server commands, but without the twin_socket feature " + "enabled. This is known to break when client and server send a command " + "at the same time. See " + "https://github.com/nutanix/libvfio-user/issues/279 for details."; + + if (!warning_printed) { + /* Print to log and stderr. */ + fprintf(stderr, "WARNING: %s\n", warning_msg); + vfu_log(vfu_ctx, LOG_WARNING, "%s", warning_msg); + warning_printed = true; + } +} + static int tran_sock_send_msg(vfu_ctx_t *vfu_ctx, uint16_t msg_id, enum vfio_user_command cmd, @@ -624,9 +641,12 @@ tran_sock_send_msg(vfu_ctx_t *vfu_ctx, uint16_t msg_id, ts = vfu_ctx->tran_data; - /* NB. use the client command socket file descriptor if available. */ - fd = ts->client_cmd_socket_fd != -1 ? ts->client_cmd_socket_fd - : ts->conn_fd; + fd = ts->client_cmd_socket_fd; + if (fd == -1) { + maybe_print_cmd_collision_warning(vfu_ctx); + fd = ts->conn_fd; + } + return tran_sock_msg(fd, msg_id, cmd, send_data, send_len, hdr, recv_data, recv_len); } From 3f3d8d555d79a714890b1b099d92d2b6d4158b1b Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Thu, 31 Aug 2023 03:56:31 -0700 Subject: [PATCH 10/13] Fix accidental indentation edit. --- test/py/test_sgl_read_write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py index efe4feaf..d0c4a7f8 100644 --- a/test/py/test_sgl_read_write.py +++ b/test/py/test_sgl_read_write.py @@ -52,7 +52,7 @@ def __handle_requests(sock, pipe, buf, lock, addr, error_no): if pipe in ready: break -# Read a command from the socket and service it. + # Read a command from the socket and service it. _, msg_id, cmd, payload = get_msg_fds(sock, VFIO_USER_F_TYPE_COMMAND) assert cmd in [VFIO_USER_DMA_READ, VFIO_USER_DMA_WRITE] From ce3218e43ec6ea11ea367b107692b68f950a61a7 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Thu, 31 Aug 2023 04:00:06 -0700 Subject: [PATCH 11/13] Remove stray struct field missed in a previous iteration. --- lib/private.h | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/private.h b/lib/private.h index 02007aef..fdd804f6 100644 --- a/lib/private.h +++ b/lib/private.h @@ -168,7 +168,6 @@ struct vfu_ctx { int client_max_fds; size_t client_max_data_xfer_size; - bool enable_cmd_conn; struct vfu_ctx_pending_info pending; bool quiesced; From beb8d78249c6da57738b646d831bb78e012699ae Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Thu, 7 Sep 2023 06:42:44 -0700 Subject: [PATCH 12/13] Print the warning message only to the log, not stderr. --- lib/tran_sock.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/tran_sock.c b/lib/tran_sock.c index 0777c601..8a652c7a 100644 --- a/lib/tran_sock.c +++ b/lib/tran_sock.c @@ -619,8 +619,6 @@ static void maybe_print_cmd_collision_warning(vfu_ctx_t *vfu_ctx) { "https://github.com/nutanix/libvfio-user/issues/279 for details."; if (!warning_printed) { - /* Print to log and stderr. */ - fprintf(stderr, "WARNING: %s\n", warning_msg); vfu_log(vfu_ctx, LOG_WARNING, "%s", warning_msg); warning_printed = true; } From 4d56e0b0888c47a1b5580a745f33006c1e7c8d34 Mon Sep 17 00:00:00 2001 From: Mattias Nissler Date: Thu, 7 Sep 2023 06:53:21 -0700 Subject: [PATCH 13/13] Adjustments to match spec changes regarding the "twin_socket.supported" field. --- lib/tran.c | 10 +++++++--- test/py/libvfio_user.py | 3 ++- test/py/test_sgl_read_write.py | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/tran.c b/lib/tran.c index af6b813f..46f5874f 100644 --- a/lib/tran.c +++ b/lib/tran.c @@ -58,7 +58,7 @@ * "pgsize": 4096 * }, * "twin_socket": { - * "enable": true, + * "supported": true, * "fd_index": 0 * } * } @@ -143,7 +143,7 @@ tran_parse_version_json(const char *json_str, int *client_max_fdsp, goto out; } - if (json_object_object_get_ex(jo, "enable", &jo2)) { + if (json_object_object_get_ex(jo, "supported", &jo2)) { if (json_object_get_type(jo2) != json_type_boolean) { goto out; } @@ -376,11 +376,15 @@ format_server_capabilities(vfu_ctx_t *vfu_ctx, int twin_socket_fd_index) } if (twin_socket_fd_index >= 0) { + struct json_object *jo_supported = NULL; + if ((jo_twin_socket = json_object_new_object()) == NULL) { goto out; } - if (json_add_uint64(jo_twin_socket, "fd_index", + if ((jo_supported = json_object_new_boolean(true)) == NULL || + json_add(jo_twin_socket, "supported", &jo_supported) < 0 || + json_add_uint64(jo_twin_socket, "fd_index", twin_socket_fd_index) < 0) { goto out; } diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 33830582..a701d1b7 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -740,7 +740,8 @@ def update(target, overrides): server_caps = json.loads(payload[struct.calcsize("HH"):-1].decode()) try: - if client_caps["capabilities"]["twin_socket"]["enable"]: + if (client_caps["capabilities"]["twin_socket"]["supported"] and + server_caps["capabilities"]["twin_socket"]["supported"]): index = server_caps["capabilities"]["twin_socket"]["fd_index"] self.client_cmd_socket = socket.socket(fileno=fds[index]) except KeyError: diff --git a/test/py/test_sgl_read_write.py b/test/py/test_sgl_read_write.py index d0c4a7f8..2f4e9929 100644 --- a/test/py/test_sgl_read_write.py +++ b/test/py/test_sgl_read_write.py @@ -110,7 +110,7 @@ def setup_function(function): "capabilities": { "max_data_xfer_size": PAGE_SIZE, "twin_socket": { - "enable": True, + "supported": True, }, } }