From eb108bf4f9a995ad3f3520a4f3f9b79f77874573 Mon Sep 17 00:00:00 2001 From: Andriy Berestovskyy Date: Fri, 22 Jan 2016 14:01:15 +0100 Subject: [PATCH] DPDK: fix vRouter and Agent restart keeping the VM connectivity vRouter restart: recover vring base after a crash from a shared memory. Agent restart: close both the FD we listen and the FD we have accepted, so QEMU detect the chardev disconnection, reconnects and replays all the virtio messages. Other minor changes (logs, typos, includes, renames, etc) Change-Id: I4ebab47d40c162f3b31aa7c3782becece38bdae5 Closes-bug: #1537043 --- dpdk/vr_dpdk_netlink.c | 3 + dpdk/vr_dpdk_virtio.c | 50 +++++++- dpdk/vr_dpdk_virtio.h | 1 + dpdk/vr_uvhost.c | 2 - dpdk/vr_uvhost_client.c | 7 +- dpdk/vr_uvhost_msg.c | 255 ++++++++++++++++++++++++---------------- dpdk/vr_uvhost_util.c | 32 +++-- dpdk/vr_uvhost_util.h | 6 +- 8 files changed, 238 insertions(+), 118 deletions(-) diff --git a/dpdk/vr_dpdk_netlink.c b/dpdk/vr_dpdk_netlink.c index 71ada5b45..5631910ca 100644 --- a/dpdk/vr_dpdk_netlink.c +++ b/dpdk/vr_dpdk_netlink.c @@ -98,6 +98,7 @@ dpdk_netlink_receive(void *usockp, char *nl_buf, int ret; struct vr_message request; + memset(&request, 0, sizeof(request)); request.vr_message_buf = nl_buf + HDR_LEN; request.vr_message_len = nl_len - HDR_LEN; @@ -159,6 +160,7 @@ vr_netlink_uvhost_vif_del(unsigned int vif_idx) { vrnu_msg_t msg; + memset(&msg, 0, sizeof(msg)); msg.vrnum_type = VRNU_MSG_VIF_DEL; msg.vrnum_vif_del.vrnu_vif_idx = vif_idx; @@ -188,6 +190,7 @@ vr_netlink_uvhost_vif_add(char *vif_name, unsigned int vif_idx, { vrnu_msg_t msg; + memset(&msg, 0, sizeof(msg)); msg.vrnum_type = VRNU_MSG_VIF_ADD; strncpy(msg.vrnum_vif_add.vrnu_vif_name, vif_name, sizeof(msg.vrnum_vif_add.vrnu_vif_name) - 1); diff --git a/dpdk/vr_dpdk_virtio.c b/dpdk/vr_dpdk_virtio.c index e9a95f91c..a35c620b0 100644 --- a/dpdk/vr_dpdk_virtio.c +++ b/dpdk/vr_dpdk_virtio.c @@ -217,8 +217,8 @@ vr_dpdk_virtio_uvh_get_blk_size(int fd, uint64_t *const blksize) if (!ret){ *blksize = (uint64_t)fd_stat.st_blksize; } else { - RTE_LOG(DEBUG, UVHOST, "Function fstat() failed: %s %s \n", - __func__, strerror(errno)); + RTE_LOG(DEBUG, UVHOST, "Error getting file status for FD %d: %s (%d)\n", + fd, strerror(errno), errno); } return ret; @@ -242,8 +242,8 @@ vr_dpdk_virtio_uvh_vif_munmap(vr_dpdk_uvh_vif_mmap_addr_t *const vif_mmap_addrs) ret = vr_dpdk_virtio_uvh_vif_region_munmap(vif_data_mmap); if (ret) { RTE_LOG(INFO, UVHOST, - "munmap() failed: %s , memleak: vif_idx_region %d %s\n", - strerror(errno), i, __func__); + "Error unmapping memory region %d: %s (%d)\n", + i, strerror(errno), errno); } memset(vif_data_mmap, 0, sizeof(vr_dpdk_uvh_mmap_addr_t)); } @@ -619,6 +619,8 @@ dpdk_virtio_from_vm_rx(void *port, struct rte_mbuf **pkts, uint32_t max_pkts) vq->vdv_last_used_idx += pkts_sent; rte_wmb(); vq->vdv_used->idx += pkts_sent; + RTE_LOG(DEBUG, VROUTER, "%s: vif %d vq %p last_used_idx %d used->idx %d\n", + __func__, vq->vdv_vif_idx, vq, vq->vdv_last_used_idx, vq->vdv_used->idx); /* call guest if required (fixes iperf issue) */ if (unlikely(!(vq->vdv_avail->flags & VRING_AVAIL_F_NO_INTERRUPT))) { p->nb_syscalls++; @@ -829,6 +831,8 @@ dpdk_virtio_dev_to_vm_tx_burst(struct dpdk_virtio_writer *p, *(volatile uint16_t *)&vq->vdv_used->idx += count; vq->vdv_last_used_idx = res_end_idx; + RTE_LOG(DEBUG, VROUTER, "%s: vif %d vq %p last_used_idx %d used->idx %d\n", + __func__, vq->vdv_vif_idx, vq, vq->vdv_last_used_idx, vq->vdv_used->idx); /* flush used->idx update before we read avail->flags. */ rte_mb(); @@ -969,10 +973,44 @@ vr_dpdk_virtio_get_vring_base(unsigned int vif_idx, unsigned int vring_idx, } /* - * vr_dpdk_set_vring_addr - Sets the address of the virtio descruptor and + * vr_dpdk_virtio_recover_vring_base - recovers the vring base from the shared + * memory after vRouter crash. + * + * Returns 0 on success, -1 otherwise. + */ +int +vr_dpdk_virtio_recover_vring_base(unsigned int vif_idx, unsigned int vring_idx) +{ + vr_dpdk_virtioq_t *vq; + + if ((vif_idx >= VR_MAX_INTERFACES) + || (vring_idx >= (2 * VR_DPDK_VIRTIO_MAX_QUEUES))) { + return -1; + } + + if (vring_idx & 1) { + vq = &vr_dpdk_virtio_rxqs[vif_idx][vring_idx/2]; + } else { + vq = &vr_dpdk_virtio_txqs[vif_idx][vring_idx/2]; + } + + if (vq->vdv_used) { + /* Reading base index from the shared memory. */ + if (vq->vdv_last_used_idx != vq->vdv_used->idx) { + RTE_LOG(INFO, UVHOST, " recovering vring base %d -> %d\n", + vq->vdv_last_used_idx, vq->vdv_used->idx); + vr_dpdk_virtio_set_vring_base(vif_idx, vring_idx, vq->vdv_used->idx); + } + } + + return 0; +} + +/* + * vr_dpdk_set_vring_addr - Sets the address of the virtio descriptor and * available/used rings based on messages sent by the vhost client. * - * Returns 0 on suucess, -1 otherwise. + * Returns 0 on success, -1 otherwise. */ int vr_dpdk_set_vring_addr(unsigned int vif_idx, unsigned int vring_idx, diff --git a/dpdk/vr_dpdk_virtio.h b/dpdk/vr_dpdk_virtio.h index 7f977a1f9..d0db590b0 100644 --- a/dpdk/vr_dpdk_virtio.h +++ b/dpdk/vr_dpdk_virtio.h @@ -75,6 +75,7 @@ int vr_dpdk_virtio_set_vring_base(unsigned int vif_idx, unsigned int vring_idx, unsigned int vring_base); int vr_dpdk_virtio_get_vring_base(unsigned int vif_idx, unsigned int vring_idx, unsigned int *vring_basep); +int vr_dpdk_virtio_recover_vring_base(unsigned int vif_idx, unsigned int vring_idx); int vr_dpdk_set_vring_addr(unsigned int vif_idx, unsigned int vring_idx, struct vring_desc *vrucv_desc, struct vring_avail *vrucv_avail, diff --git a/dpdk/vr_uvhost.c b/dpdk/vr_uvhost.c index e3face112..578fc3cc2 100644 --- a/dpdk/vr_uvhost.c +++ b/dpdk/vr_uvhost.c @@ -5,8 +5,6 @@ * Copyright (c) 2014 Juniper Networks, Inc. All rights reserved. */ -#include - #include "vr_dpdk.h" #include "vr_dpdk_usocket.h" #include "vr_uvhost.h" diff --git a/dpdk/vr_uvhost_client.c b/dpdk/vr_uvhost_client.c index 93e167b06..a9d2d86a2 100644 --- a/dpdk/vr_uvhost_client.c +++ b/dpdk/vr_uvhost_client.c @@ -7,6 +7,7 @@ #include "vr_dpdk.h" #include "vr_uvhost_client.h" +#include "vr_uvhost_util.h" static vr_uvh_client_t vr_uvh_clients[VR_UVH_MAX_CLIENTS]; @@ -57,9 +58,9 @@ vr_uvhost_new_client(int fd, char *path, int cidx) void vr_uvhost_del_client(vr_uvh_client_t *vru_cl) { - if (vru_cl->vruc_fd > 0) { - close(vru_cl->vruc_fd); - } + /* Remove both the socket we listen for and the socket we have accepted */ + vr_uvhost_del_fds_by_arg(vru_cl); + vru_cl->vruc_fd = -1; unlink(vru_cl->vruc_path); diff --git a/dpdk/vr_uvhost_msg.c b/dpdk/vr_uvhost_msg.c index ef8aaa542..a18dcf032 100644 --- a/dpdk/vr_uvhost_msg.c +++ b/dpdk/vr_uvhost_msg.c @@ -22,34 +22,37 @@ #include #include + typedef int (*vr_uvh_msg_handler_fn)(vr_uvh_client_t *vru_cl); +#define uvhm_client_name(vru_cl) (vru_cl->vruc_path + strlen(VR_UVH_VIF_PREFIX)) /* * Prototypes for user space vhost message handlers */ static int vr_uvmh_get_features(vr_uvh_client_t *vru_cl); +static int vr_uvmh_set_features(vr_uvh_client_t *vru_cl); static int vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl); -static int vr_uvhm_set_ring_num_desc(vr_uvh_client_t *vru_cl); +static int vr_uvhm_set_vring_num(vr_uvh_client_t *vru_cl); static int vr_uvhm_set_vring_addr(vr_uvh_client_t *vru_cl); static int vr_uvhm_set_vring_base(vr_uvh_client_t *vru_cl); static int vr_uvhm_get_vring_base(vr_uvh_client_t *vru_cl); -static int vr_uvhm_set_call_fd(vr_uvh_client_t *vru_cl); +static int vr_uvhm_set_vring_call(vr_uvh_client_t *vru_cl); static vr_uvh_msg_handler_fn vr_uvhost_cl_msg_handlers[] = { NULL, vr_uvmh_get_features, - NULL, + vr_uvmh_set_features, NULL, NULL, vr_uvhm_set_mem_table, NULL, NULL, - vr_uvhm_set_ring_num_desc, + vr_uvhm_set_vring_num, vr_uvhm_set_vring_addr, vr_uvhm_set_vring_base, vr_uvhm_get_vring_base, NULL, - vr_uvhm_set_call_fd, + vr_uvhm_set_vring_call, NULL }; @@ -68,12 +71,29 @@ vr_uvmh_get_features(vr_uvh_client_t *vru_cl) (1ULL << VIRTIO_NET_F_CSUM) | (1ULL << VIRTIO_NET_F_GUEST_CSUM) | (1ULL << VHOST_F_LOG_ALL); + vr_uvhost_log(" GET FEATURES: returns 0x%"PRIx64"\n", + vru_cl->vruc_msg.u64); vru_cl->vruc_msg.size = sizeof(vru_cl->vruc_msg.u64); return 0; } +/* + * vr_uvmh_set_features - handle VHOST_USER_SET_FEATURES message from user space + * vhost client. + * + * Returns 0 on success, -1 otherwise. + */ +static int +vr_uvmh_set_features(vr_uvh_client_t *vru_cl) +{ + vr_uvhost_log(" SET FEATURES: 0x%"PRIx64"\n", + vru_cl->vruc_msg.u64); + + return 0; +} + /* * vr_uvhm_set_mem_table - handles VHOST_USER_SET_MEM_TABLE message from * user space vhost client to learn the memory map of the guest. @@ -94,11 +114,12 @@ vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl) &(vr_dpdk_virtio_uvh_vif_mmap[vru_cl->vruc_idx])); vum_msg = &vru_cl->vruc_msg.memory; - vr_uvhost_log("Number of memory regions: %d\n", vum_msg->nregions); + vr_uvhost_log(" SET MEM TABLE:\n"); for (i = 0; i < vum_msg->nregions; i++) { - vr_uvhost_log("Region %d: physical address 0x%" PRIx64 ", size 0x%" - PRIx64 ", offset 0x%" PRIx64 "\n", - i, vum_msg->regions[i].guest_phys_addr, + vr_uvhost_log(" %d: FD %d addr 0x%" PRIx64 " size 0x%" + PRIx64 " off 0x%" PRIx64 "\n", + i, vru_cl->vruc_fds_sent[i], + vum_msg->regions[i].guest_phys_addr, vum_msg->regions[i].memory_size, vum_msg->regions[i].mmap_offset); @@ -120,11 +141,11 @@ vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl) 0); if (region->vrucmr_mmap_addr == ((uint64_t)MAP_FAILED)) { - vr_uvhost_log("mmap for size 0x%" PRIx64 " failed for FD %d" - " on vhost client %s (%s)\n", - size, - vru_cl->vruc_fds_sent[i], - vru_cl->vruc_path, rte_strerror(errno)); + vr_uvhost_log("Client %s: error mmaping FD %d size 0x%" PRIx64 + ": %s (%d)\n", + uvhm_client_name(vru_cl), + vru_cl->vruc_fds_sent[i], size, + rte_strerror(errno), errno); /* the file descriptor is no longer needed */ close(vru_cl->vruc_fds_sent[i]); vru_cl->vruc_fds_sent[i] = -1; @@ -134,8 +155,9 @@ vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl) ret = vr_dpdk_virtio_uvh_get_blk_size(vru_cl->vruc_fds_sent[i], &vif_mmap_addrs->vu_mmap_data[i].unmap_blksz); if (ret) { - vr_uvhost_log("Get block size failed for FD %d on vhost client %s \n", - vru_cl->vruc_fds_sent[i], vru_cl->vruc_path); + vr_uvhost_log("Client %s: error getting block size for FD %d\n", + uvhm_client_name(vru_cl), + vru_cl->vruc_fds_sent[i]); return -1; } @@ -149,11 +171,12 @@ vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl) vif_mmap_addrs->vu_mmap_data[i].unmap_blksz); if (madvise(madv_addr, madv_size, MADV_DONTDUMP)) { - vr_uvhost_log("Error in madvise at addr 0x%" PRIx64 ", size 0x%" - PRIx64 "for fd %d on vhost client %s (%s)\n", + vr_uvhost_log("Client %s: error in madvise at addr 0x%" PRIx64 ", size 0x%" + PRIx64 "for FD %d: %s (%d)\n", + uvhm_client_name(vru_cl), region->vrucmr_mmap_addr, size, vru_cl->vruc_fds_sent[i], - vru_cl->vruc_path, rte_strerror(errno)); + rte_strerror(errno), errno); /* * Failure is not catastrophic, so continue below. */ @@ -178,32 +201,32 @@ vr_uvhm_set_mem_table(vr_uvh_client_t *vru_cl) } /* - * vr_uvhm_set_ring_num_desc - handles VHOST_USER_SET_VRING_NUM message from + * vr_uvhm_set_vring_num - handles VHOST_USER_SET_VRING_NUM message from * the user space vhost client to set the number of descriptors in the virtio * ring. * * Returns 0 on success, -1 otherwise. */ static int -vr_uvhm_set_ring_num_desc(vr_uvh_client_t *vru_cl) +vr_uvhm_set_vring_num(vr_uvh_client_t *vru_cl) { VhostUserMsg *vum_msg; unsigned int vring_idx; vum_msg = &vru_cl->vruc_msg; vring_idx = vum_msg->state.index; + vr_uvhost_log(" SET VRING NUM: vring %u num %u\n", vring_idx, + vum_msg->state.num); if (vring_idx >= VHOST_CLIENT_MAX_VRINGS) { - vr_uvhost_log("Bad ring index %d received by vhost server\n", - vring_idx); + vr_uvhost_log("Client %s: error setting vring %u num: invalid vring index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } if (vr_dpdk_set_ring_num_desc(vru_cl->vruc_idx, vring_idx, vum_msg->state.num)) { - vr_uvhost_log("Could set number of vring descriptors in vhost server" - " %d %d %d\n", - vru_cl->vruc_idx, vring_idx, - vum_msg->state.num); + vr_uvhost_log("Client %s: error setting vring %u size %u\n", + uvhm_client_name(vru_cl), vring_idx, vum_msg->state.num); return -1; } @@ -236,6 +259,49 @@ vr_uvhm_map_addr(vr_uvh_client_t *vru_cl, uint64_t addr) return NULL; } +/* + * uvhm_check_vring_ready - check if virtual queue is ready to use and + * set the ready status. + * + * Returns 1 if vring ready, 0 otherwise. + */ +static int +uvhm_check_vring_ready(vr_uvh_client_t *vru_cl, unsigned int vring_idx) +{ + unsigned int vif_idx = vru_cl->vruc_idx; + vr_dpdk_virtioq_t *vq; + + if (vif_idx >= VR_MAX_INTERFACES) { + return 0; + } + + if (vring_idx & 1) { + vq = &vr_dpdk_virtio_rxqs[vif_idx][vring_idx/2]; + } else { + vq = &vr_dpdk_virtio_txqs[vif_idx][vring_idx/2]; + } + + /* vring is ready if both call FD and addresses are set */ + if (vq->vdv_desc && vq->vdv_callfd > 0 && vq->vdv_ready_state != VQ_READY) { + /* + * Now the virtio queue is ready for forwarding. + * TODO - need a memory barrier here for non-x86 CPU? + */ + if (vr_dpdk_set_virtq_ready(vru_cl->vruc_idx, vring_idx, VQ_READY)) { + vr_uvhost_log("Client %s: error setting vring %u ready state\n", + uvhm_client_name(vru_cl), vring_idx); + return -1; + } + + vr_uvhost_log("Client %s: vring %d is ready\n", + uvhm_client_name(vru_cl), vring_idx); + + return 1; + } + + return 0; +} + /* * vr_uvhm_set_vring_addr - handles a VHOST_USER_SET_VRING_ADDR message from * the user space vhost client to set the address of the virtio rings. @@ -252,11 +318,15 @@ vr_uvhm_set_vring_addr(vr_uvh_client_t *vru_cl) struct vring_used *vrucv_used; vaddr = &vru_cl->vruc_msg.addr; - vring_idx = vaddr->index; + vr_uvhost_log(" SET VRING ADDR: vring %u flags 0x%x desc 0x%llx" + " used 0x%llx avail 0x%llx\n", + vring_idx, vaddr->flags, vaddr->desc_user_addr, + vaddr->used_user_addr, vaddr->avail_user_addr); + if (vring_idx >= VHOST_CLIENT_MAX_VRINGS) { - vr_uvhost_log("Bad ring index %d received by vhost server\n", - vring_idx); + vr_uvhost_log("Client %s: error setting vring %u addr: invalid vring index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } @@ -272,23 +342,15 @@ vr_uvhm_set_vring_addr(vr_uvh_client_t *vru_cl) if (vr_dpdk_set_vring_addr(vru_cl->vruc_idx, vring_idx, vrucv_desc, vrucv_avail, vrucv_used)) { - vr_uvhost_log("Couldn't set vring addresses in vhost server, %d %d\n", - vru_cl->vruc_idx, vring_idx); + vr_uvhost_log("Client %s: error setting vring %u addresses\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } - /* - * Now that the addresses have been set, the virtio queue is ready for - * forwarding. This is the last message from qemu, so call fd has been set. - * - * TODO - need a memory barrier here for non-x86 CPU. - */ - if (vr_dpdk_set_virtq_ready(vru_cl->vruc_idx, vring_idx, VQ_READY)) { - vr_uvhost_log("Couldn't set virtio queue ready in vhost server, " - "%d %d\n", - vru_cl->vruc_idx, vring_idx); - return -1; - } + /* Try to recover from the vRouter crash. */ + vr_dpdk_virtio_recover_vring_base(vru_cl->vruc_idx, vring_idx); + + uvhm_check_vring_ready(vru_cl, vring_idx); return 0; } @@ -307,17 +369,19 @@ vr_uvhm_set_vring_base(vr_uvh_client_t *vru_cl) vum_msg = &vru_cl->vruc_msg; vring_idx = vum_msg->state.index; + vr_uvhost_log(" SET VRING BASE: vring %u base %u\n", + vring_idx, vum_msg->state.num); if (vring_idx >= VHOST_CLIENT_MAX_VRINGS) { - vr_uvhost_log("Bad ring index %d received by vhost server\n", - vring_idx); + vr_uvhost_log("Client %s: error setting vring %u base: invalid vring index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } if (vr_dpdk_virtio_set_vring_base(vru_cl->vruc_idx, vring_idx, vum_msg->state.num)) { - vr_uvhost_log("Couldn't set vring base in vhost server %d %d %d\n", - vru_cl->vruc_idx, vring_idx, vum_msg->state.num); + vr_uvhost_log("Client %s: error setting vring %u base %u\n", + uvhm_client_name(vru_cl), vring_idx, vum_msg->state.num); return -1; } @@ -338,58 +402,62 @@ vr_uvhm_get_vring_base(vr_uvh_client_t *vru_cl) vum_msg = &vru_cl->vruc_msg; vring_idx = vum_msg->state.index; + vr_uvhost_log(" GET VRING BASE: vring %u\n", vring_idx); if (vring_idx >= VHOST_CLIENT_MAX_VRINGS) { - vr_uvhost_log("Bad ring index %d received by vhost server\n", - vring_idx); + vr_uvhost_log("Client %s: error getting vring %u base: invalid vring index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } if (vr_dpdk_virtio_get_vring_base(vru_cl->vruc_idx, vring_idx, &vum_msg->state.num)) { - vr_uvhost_log("Couldn't get vring base in vhost server %d %d\n", - vru_cl->vruc_idx, vring_idx); + vr_uvhost_log("Client %s: error getting vring %u base index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } vum_msg->size = sizeof(struct vhost_vring_state); + vr_uvhost_log(" GET VRING BASE: returns %u\n", vum_msg->state.num); return 0; } /* - * vr_uvhm_set_call_fd - handles a VHOST_USER_SET_VRING_CALL messsage + * vr_uvhm_set_vring_call - handles a VHOST_USER_SET_VRING_CALL messsage * from the vhost user client to set the eventfd to be used to interrupt the * guest, if required. * * Returns 0 on success, -1 otherwise. */ static int -vr_uvhm_set_call_fd(vr_uvh_client_t *vru_cl) +vr_uvhm_set_vring_call(vr_uvh_client_t *vru_cl) { VhostUserMsg *vum_msg; unsigned int vring_idx; vum_msg = &vru_cl->vruc_msg; vring_idx = vum_msg->state.index; + vr_uvhost_log(" SET VRING CALL: vring %u FD %d\n", vring_idx, + vru_cl->vruc_fds_sent[0]); if (vring_idx >= VHOST_CLIENT_MAX_VRINGS) { - vr_uvhost_log("Bad ring index %d received by vhost server in callfd\n", - vring_idx); + vr_uvhost_log("Client %s: error setting vring %u call: invalid vring index\n", + uvhm_client_name(vru_cl), vring_idx); return -1; } if (vr_dpdk_set_ring_callfd(vru_cl->vruc_idx, vring_idx, vru_cl->vruc_fds_sent[0])) { - vr_uvhost_log("Could not set callfd in vhost server" - " %d %d %d\n", - vru_cl->vruc_idx, vring_idx, - vru_cl->vruc_fds_sent[0]); + vr_uvhost_log("Client %s: error setting vring %u call FD %d\n", + uvhm_client_name(vru_cl), vring_idx, vru_cl->vruc_fds_sent[0]); return -1; } - /* set FD to -1, so we do not close it on cleanup */ + /* set FD to -1, so we do not close it in vr_uvh_cl_msg_handler() */ vru_cl->vruc_fds_sent[0] = -1; + uvhm_check_vring_ready(vru_cl, vring_idx); + return 0; } @@ -403,7 +471,6 @@ static int vr_uvh_cl_call_handler(vr_uvh_client_t *vru_cl) { VhostUserMsg *msg = &vru_cl->vruc_msg; - int i; if ((msg->request <= VHOST_USER_NONE) || (msg->request >= VHOST_USER_MAX)) { @@ -412,19 +479,11 @@ vr_uvh_cl_call_handler(vr_uvh_client_t *vru_cl) if (vr_uvhost_cl_msg_handlers[msg->request]) { vr_uvhost_log("Client %s: handling message %d\n", - /* strip socket prefix */ - vru_cl->vruc_path + strlen(VR_UVH_VIF_PREFIX), msg->request); - if (vru_cl->vruc_num_fds_sent > 0) { - for (i = 0; i < vru_cl->vruc_num_fds_sent; i++) { - vr_uvhost_log(" message %d sent FD: %d\n", - msg->request, vru_cl->vruc_fds_sent[i]); - } - } + uvhm_client_name(vru_cl), msg->request); return vr_uvhost_cl_msg_handlers[msg->request](vru_cl); } else { vr_uvhost_log("Client %s: no handler defined for message %d\n", - /* strip socket prefix */ - vru_cl->vruc_path + strlen(VR_UVH_VIF_PREFIX), msg->request); + uvhm_client_name(vru_cl), msg->request); } return 0; @@ -466,8 +525,8 @@ vr_uvh_cl_send_reply(int fd, vr_uvh_client_t *vru_cl) * Socket to qemu should never be full as it sleeps waiting * for a reply to the previous request. */ - vr_uvhost_log("Error sending vhost user reply to %s\n", - vru_cl->vruc_path); + vr_uvhost_log("Client %s: error sending vhost user reply\n", + uvhm_client_name(vru_cl)); return -1; } @@ -520,14 +579,14 @@ vr_uvh_cl_msg_handler(int fd, void *arg) goto cleanup; } - vr_uvhost_log("Receive returned %d in vhost server for client %s\n", - ret, vru_cl->vruc_path); + vr_uvhost_log("Client %s: error receiving message: %s (%d)\n", + uvhm_client_name(vru_cl), strerror(errno), errno); ret = -1; goto cleanup; } else if (ret > 0) { if (mhdr.msg_flags & MSG_CTRUNC) { - vr_uvhost_log("Truncated control message from vhost client %s\n", - vru_cl->vruc_path); + vr_uvhost_log("Client %s: error receiving message: truncated\n", + uvhm_client_name(vru_cl)); ret = -1; goto cleanup; } @@ -539,8 +598,10 @@ vr_uvh_cl_msg_handler(int fd, void *arg) vru_cl->vruc_num_fds_sent = (cmsg->cmsg_len - CMSG_LEN(0))/ sizeof(int); if (vru_cl->vruc_num_fds_sent > VHOST_MEMORY_MAX_NREGIONS) { - vr_uvhost_log("Too many FDs sent for client %s: %d\n", - vru_cl->vruc_path, vru_cl->vruc_num_fds_sent); + vr_uvhost_log("Client %s: error handling FDs: too many FDs (%d > %d)\n", + uvhm_client_name(vru_cl), + vru_cl->vruc_num_fds_sent, + VHOST_MEMORY_MAX_NREGIONS); vru_cl->vruc_num_fds_sent = VHOST_MEMORY_MAX_NREGIONS; } @@ -559,8 +620,8 @@ vr_uvh_cl_msg_handler(int fd, void *arg) /* * recvmsg returned 0, so return error. */ - vr_uvhost_log("Receive returned %d in vhost server for client %s\n", - ret, vru_cl->vruc_path); + vr_uvhost_log("Client %s: shutdown at message receiving\n", + uvhm_client_name(vru_cl)); ret = -1; goto cleanup; } @@ -600,14 +661,13 @@ vr_uvh_cl_msg_handler(int fd, void *arg) } vr_uvhost_log( - "Error: read returned %d, %d %d %d in vhost server for client %s\n", - ret, errno, read_len, - vru_cl->vruc_msg_bytes_read, vru_cl->vruc_path); + "Client %s: error reading message: %s (%d)\n", + uvhm_client_name(vru_cl), strerror(errno), errno); ret = -1; goto cleanup; } else if (ret == 0) { - vr_uvhost_log("Read returned %d in vhost server for client %s\n", - ret, vru_cl->vruc_path); + vr_uvhost_log("Client %s: shutdown at message reading\n", + uvhm_client_name(vru_cl)); ret = -1; goto cleanup; } @@ -627,16 +687,16 @@ vr_uvh_cl_msg_handler(int fd, void *arg) ret = vr_uvh_cl_call_handler(vru_cl); if (ret < 0) { - vr_uvhost_log("Error handling message %d client %s\n", - vru_cl->vruc_msg.request, vru_cl->vruc_path); + vr_uvhost_log("Client %s: error handling message %d\n", + uvhm_client_name(vru_cl), vru_cl->vruc_msg.request); ret = -1; goto cleanup; } ret = vr_uvh_cl_send_reply(fd, vru_cl); if (ret < 0) { - vr_uvhost_log("Error sending reply for message %d client %s\n", - vru_cl->vruc_msg.request, vru_cl->vruc_path); + vr_uvhost_log("Client %s: error sending reply for message %d\n", + uvhm_client_name(vru_cl), vru_cl->vruc_msg.request); ret = -1; goto cleanup; } @@ -743,9 +803,10 @@ vr_uvh_nl_vif_del_handler(vrnu_vif_del_t *msg) unsigned int cidx = msg->vrnu_vif_idx; vr_uvh_client_t *vru_cl; + vr_uvhost_log("Deleting vif %d virtual device\n", cidx); + if (cidx >= VR_UVH_MAX_CLIENTS) { - vr_uvhost_log("Couldn't delete vhost client due to bad index %d\n", - cidx); + vr_uvhost_log(" error deleting vif %u: invalid vif index\n", cidx); return -1; } @@ -753,7 +814,7 @@ vr_uvh_nl_vif_del_handler(vrnu_vif_del_t *msg) vru_cl = vr_uvhost_get_client(cidx); if (vru_cl == NULL) { - vr_uvhost_log("Couldn't find vhost client %d for deletion\n", + vr_uvhost_log(" error deleting vif %d: no client found\n", cidx); return -1; } @@ -761,10 +822,6 @@ vr_uvh_nl_vif_del_handler(vrnu_vif_del_t *msg) * Unmmaps Qemu's FD */ vr_dpdk_virtio_uvh_vif_munmap(&vr_dpdk_virtio_uvh_vif_mmap[cidx]); - if (vru_cl->vruc_fd != -1) { - vr_uvhost_del_fd(vru_cl->vruc_fd, UVH_FD_READ); - } - vr_uvhost_del_client(vru_cl); return 0; @@ -907,7 +964,7 @@ vr_uvh_nl_msg_handler(int fd, void *arg) } if (ret != sizeof(msg)) { - vr_uvhost_log("Received msg of length %d, expected %d in vhost server", + vr_uvhost_log("Received msg of length %d, expected %zu in vhost server", ret, sizeof(msg)); return 0; } diff --git a/dpdk/vr_uvhost_util.c b/dpdk/vr_uvhost_util.c index 6126248de..6b561ff35 100644 --- a/dpdk/vr_uvhost_util.c +++ b/dpdk/vr_uvhost_util.c @@ -5,8 +5,6 @@ * Copyright (c) 2014 Juniper Networks, Inc. All rights reserved. */ -#include - #include "vr_dpdk.h" #include "vr_uvhost_util.h" @@ -53,7 +51,7 @@ vr_uvhost_del_fd(int fd, uvh_fd_type_t fd_type) int i; uvh_fd_t *fds; - RTE_LOG(DEBUG, UVHOST, "Deleting FD %d from the poll pool...\n", fd); + RTE_LOG(DEBUG, UVHOST, "Deleting FD %d...\n", fd); if (fd_type == UVH_FD_READ) { fds = uvh_rfds; } else if (fd_type == UVH_FD_WRITE) { @@ -76,12 +74,34 @@ vr_uvhost_del_fd(int fd, uvh_fd_type_t fd_type) } fds[i].uvh_fd = -1; + fds[i].uvh_fd_arg = NULL; close(fd); return 0; } +/* + * vr_uvhost_del_fds_by_arg - deletes all FDs from the read/write lists matching + * the given argument (pointer to a client). All the FDs found will be closed. + * + * Returns 0 on success, -1 otherwise. + */ +int +vr_uvhost_del_fds_by_arg(void *arg) +{ + int i; + + for (i = 0; i < MAX_UVHOST_FDS; i++) { + if (uvh_rfds[i].uvh_fd > 0 && uvh_rfds[i].uvh_fd_arg == arg) + vr_uvhost_del_fd(uvh_rfds[i].uvh_fd, UVH_FD_READ); + if (uvh_wfds[i].uvh_fd > 0 && uvh_wfds[i].uvh_fd_arg == arg) + vr_uvhost_del_fd(uvh_wfds[i].uvh_fd, UVH_FD_WRITE); + } + + return 0; +} + /* * vr_uvhost_add_fd - adds the specified FD into the read/write list that * the user space vhost server is waiting on. The type indicates if it @@ -97,7 +117,7 @@ vr_uvhost_add_fd(int fd, uvh_fd_type_t fd_type, void *fd_handler_arg, int i; uvh_fd_t *fds; - RTE_LOG(DEBUG, UVHOST, "Adding FD %d to the poll pool...\n", fd); + RTE_LOG(DEBUG, UVHOST, "Adding FD %d...\n", fd); if (fd_type == UVH_FD_READ) { fds = uvh_rfds; } else if (fd_type == UVH_FD_WRITE) { @@ -116,7 +136,7 @@ vr_uvhost_add_fd(int fd, uvh_fd_type_t fd_type, void *fd_handler_arg, } } - vr_uvhost_log("Error adding FD %d: no space left\n", fd); + vr_uvhost_log("Error adding FD %d: no room for a new FD\n", fd); return -1; } @@ -182,8 +202,6 @@ vr_uvh_call_fd_handlers(struct pollfd *fds, nfds_t nfds) if (fds[i].revents & POLLIN) { ret = vr_uvh_call_fd_handlers_internal(uvh_rfds, fds[i].fd); if (ret) { - RTE_LOG(INFO, UVHOST, "Error: deleting fd %d " - "from poll\n", fds[i].fd); vr_uvhost_del_fd(fds[i].fd, UVH_FD_READ); } } diff --git a/dpdk/vr_uvhost_util.h b/dpdk/vr_uvhost_util.h index 1230f4313..839c56e37 100644 --- a/dpdk/vr_uvhost_util.h +++ b/dpdk/vr_uvhost_util.h @@ -8,6 +8,8 @@ #ifndef __VR_UVHOST_UTIL_H__ #define __VR_UVHOST_UTIL_H__ +#include + #define MAX_UVHOST_FDS VR_MAX_INTERFACES typedef int (*uvh_fd_handler_t)(int fd, void *arg); @@ -21,7 +23,9 @@ void vr_uvhost_fds_init(void); int vr_uvhost_add_fd(int fd, uvh_fd_type_t fd_type, void *fd_handler_arg, uvh_fd_handler_t fd_handler); int vr_uvhost_del_fd(int fd, uvh_fd_type_t fd_type); -void vr_uvhost_log(const char *format, ...); +int vr_uvhost_del_fds_by_arg(void *arg); +void vr_uvhost_log(const char *format, ...) + __attribute__((format(printf, 1, 2))); void vr_uvh_call_fd_handlers(struct pollfd *fds, nfds_t nfds); void vr_uvh_init_pollfds(struct pollfd *fds, nfds_t *nfds);