From 290e6e113b65ae86cfa37f2ff5ca5eb57dbf824c Mon Sep 17 00:00:00 2001 From: Fam Zheng Date: Wed, 15 Feb 2017 16:31:43 +0800 Subject: net: Remove useless local var pkt This has been pointless since commit 605d52e62, which was a search-and-replace, overlooked the redundancy. Signed-off-by: Fam Zheng Reviewed-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/net_rx_pkt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index 1019b50c18..7f928d7339 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -159,7 +159,6 @@ void net_rx_pkt_attach_iovec_ex(struct NetRxPkt *pkt, void net_rx_pkt_dump(struct NetRxPkt *pkt) { #ifdef NET_RX_PKT_DEBUG - NetRxPkt *pkt = (NetRxPkt *)pkt; assert(pkt); printf("RX PKT: tot_len: %d, vlan_stripped: %d, vlan_tag: %d\n", -- cgit v1.2.3 From 566342c3125ac2e73abd36c650222318164517ed Mon Sep 17 00:00:00 2001 From: Dmitry Fleytman Date: Thu, 16 Feb 2017 14:29:32 +0200 Subject: eth: Extend vlan stripping functions Make VLAN stripping functions return number of bytes copied to given Ethernet header buffer. This information should be used to re-compose packet IOV after VLAN stripping. Cc: qemu-stable@nongnu.org Signed-off-by: Dmitry Fleytman Signed-off-by: Jason Wang --- include/net/eth.h | 4 ++-- net/eth.c | 25 ++++++++++++++----------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/net/eth.h b/include/net/eth.h index 2013175857..afeb45be34 100644 --- a/include/net/eth.h +++ b/include/net/eth.h @@ -331,12 +331,12 @@ eth_get_pkt_tci(const void *p) } } -bool +size_t eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff, uint8_t *new_ehdr_buf, uint16_t *payload_offset, uint16_t *tci); -bool +size_t eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, uint16_t vet, uint8_t *new_ehdr_buf, uint16_t *payload_offset, uint16_t *tci); diff --git a/net/eth.c b/net/eth.c index df81efb676..5b9ba26a56 100644 --- a/net/eth.c +++ b/net/eth.c @@ -232,7 +232,7 @@ void eth_get_protocols(const struct iovec *iov, int iovcnt, } } -bool +size_t eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff, uint8_t *new_ehdr_buf, uint16_t *payload_offset, uint16_t *tci) @@ -244,7 +244,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff, new_ehdr, sizeof(*new_ehdr)); if (copied < sizeof(*new_ehdr)) { - return false; + return 0; } switch (be16_to_cpu(new_ehdr->h_proto)) { @@ -254,7 +254,7 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff, &vlan_hdr, sizeof(vlan_hdr)); if (copied < sizeof(vlan_hdr)) { - return false; + return 0; } new_ehdr->h_proto = vlan_hdr.h_proto; @@ -268,18 +268,21 @@ eth_strip_vlan(const struct iovec *iov, int iovcnt, size_t iovoff, PKT_GET_VLAN_HDR(new_ehdr), sizeof(vlan_hdr)); if (copied < sizeof(vlan_hdr)) { - return false; + return 0; } *payload_offset += sizeof(vlan_hdr); + + return sizeof(struct eth_header) + sizeof(struct vlan_header); + } else { + return sizeof(struct eth_header); } - return true; default: - return false; + return 0; } } -bool +size_t eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, uint16_t vet, uint8_t *new_ehdr_buf, uint16_t *payload_offset, uint16_t *tci) @@ -291,7 +294,7 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, new_ehdr, sizeof(*new_ehdr)); if (copied < sizeof(*new_ehdr)) { - return false; + return 0; } if (be16_to_cpu(new_ehdr->h_proto) == vet) { @@ -299,17 +302,17 @@ eth_strip_vlan_ex(const struct iovec *iov, int iovcnt, size_t iovoff, &vlan_hdr, sizeof(vlan_hdr)); if (copied < sizeof(vlan_hdr)) { - return false; + return 0; } new_ehdr->h_proto = vlan_hdr.h_proto; *tci = be16_to_cpu(vlan_hdr.h_tci); *payload_offset = iovoff + sizeof(*new_ehdr) + sizeof(vlan_hdr); - return true; + return sizeof(struct eth_header); } - return false; + return 0; } void -- cgit v1.2.3 From df8bf7a7fe75eb5d5caffa55f5cd4292b757aea6 Mon Sep 17 00:00:00 2001 From: Dmitry Fleytman Date: Thu, 16 Feb 2017 14:29:33 +0200 Subject: NetRxPkt: Fix memory corruption on VLAN header stripping This patch fixed a problem that was introduced in commit eb700029. When net_rx_pkt_attach_iovec() calls eth_strip_vlan() this can result in pkt->ehdr_buf being overflowed, because ehdr_buf is only sizeof(struct eth_header) bytes large but eth_strip_vlan() can write sizeof(struct eth_header) + sizeof(struct vlan_header) bytes into it. Devices affected by this problem: vmxnet3. Cc: qemu-stable@nongnu.org Reported-by: Peter Maydell Signed-off-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/net_rx_pkt.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index 7f928d7339..3361d7ebe0 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -23,13 +23,13 @@ struct NetRxPkt { struct virtio_net_hdr virt_hdr; - uint8_t ehdr_buf[sizeof(struct eth_header)]; + uint8_t ehdr_buf[sizeof(struct eth_header) + sizeof(struct vlan_header)]; struct iovec *vec; uint16_t vec_len_total; uint16_t vec_len; uint32_t tot_len; uint16_t tci; - bool vlan_stripped; + size_t ehdr_buf_len; bool has_virt_hdr; eth_pkt_types_e packet_type; @@ -88,15 +88,13 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt, const struct iovec *iov, int iovcnt, size_t ploff) { - if (pkt->vlan_stripped) { + if (pkt->ehdr_buf_len) { net_rx_pkt_iovec_realloc(pkt, iovcnt + 1); pkt->vec[0].iov_base = pkt->ehdr_buf; - pkt->vec[0].iov_len = sizeof(pkt->ehdr_buf); - - pkt->tot_len = - iov_size(iov, iovcnt) - ploff + sizeof(struct eth_header); + pkt->vec[0].iov_len = pkt->ehdr_buf_len; + pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len; pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1, iov, iovcnt, ploff, pkt->tot_len); } else { @@ -123,11 +121,12 @@ void net_rx_pkt_attach_iovec(struct NetRxPkt *pkt, uint16_t tci = 0; uint16_t ploff = iovoff; assert(pkt); - pkt->vlan_stripped = false; if (strip_vlan) { - pkt->vlan_stripped = eth_strip_vlan(iov, iovcnt, iovoff, pkt->ehdr_buf, - &ploff, &tci); + pkt->ehdr_buf_len = eth_strip_vlan(iov, iovcnt, iovoff, pkt->ehdr_buf, + &ploff, &tci); + } else { + pkt->ehdr_buf_len = 0; } pkt->tci = tci; @@ -143,12 +142,13 @@ void net_rx_pkt_attach_iovec_ex(struct NetRxPkt *pkt, uint16_t tci = 0; uint16_t ploff = iovoff; assert(pkt); - pkt->vlan_stripped = false; if (strip_vlan) { - pkt->vlan_stripped = eth_strip_vlan_ex(iov, iovcnt, iovoff, vet, - pkt->ehdr_buf, - &ploff, &tci); + pkt->ehdr_buf_len = eth_strip_vlan_ex(iov, iovcnt, iovoff, vet, + pkt->ehdr_buf, + &ploff, &tci); + } else { + pkt->ehdr_buf_len = 0; } pkt->tci = tci; @@ -161,8 +161,8 @@ void net_rx_pkt_dump(struct NetRxPkt *pkt) #ifdef NET_RX_PKT_DEBUG assert(pkt); - printf("RX PKT: tot_len: %d, vlan_stripped: %d, vlan_tag: %d\n", - pkt->tot_len, pkt->vlan_stripped, pkt->tci); + printf("RX PKT: tot_len: %d, ehdr_buf_len: %lu, vlan_tag: %d\n", + pkt->tot_len, pkt->ehdr_buf_len, pkt->tci); #endif } @@ -425,7 +425,7 @@ bool net_rx_pkt_is_vlan_stripped(struct NetRxPkt *pkt) { assert(pkt); - return pkt->vlan_stripped; + return pkt->ehdr_buf_len ? true : false; } bool net_rx_pkt_has_virt_hdr(struct NetRxPkt *pkt) -- cgit v1.2.3 From d5e772146d2bbc92e5126c145eddef3b2843d026 Mon Sep 17 00:00:00 2001 From: Dmitry Fleytman Date: Thu, 16 Feb 2017 14:29:34 +0200 Subject: NetRxPkt: Do not try to pull more data than present In case of VLAN stripping, ETH header put into a separate buffer, therefore amont of data copied from original IOV should be smaller. Cc: qemu-stable@nongnu.org Signed-off-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/net_rx_pkt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index 3361d7ebe0..0003a0e88b 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -96,7 +96,8 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt, pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len; pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1, - iov, iovcnt, ploff, pkt->tot_len); + iov, iovcnt, ploff, + pkt->tot_len - pkt->ehdr_buf_len); } else { net_rx_pkt_iovec_realloc(pkt, iovcnt); -- cgit v1.2.3 From c5d083c561a4f5297cc2e44a2f3cef3324d77a88 Mon Sep 17 00:00:00 2001 From: Dmitry Fleytman Date: Thu, 16 Feb 2017 14:29:35 +0200 Subject: NetRxPkt: Account buffer with ETH header in IOV length In case of VLAN stripping ETH header is stored in a separate chunk and length of IOV should take this into account. This patch fixes checksum validation for RX packets with VLAN header. Devices affected by this problem: e1000e and vmxnet3. Cc: qemu-stable@nongnu.org Signed-off-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/net_rx_pkt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index 0003a0e88b..2649d40b5c 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -97,7 +97,7 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt, pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len; pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1, iov, iovcnt, ploff, - pkt->tot_len - pkt->ehdr_buf_len); + pkt->tot_len - pkt->ehdr_buf_len) + 1; } else { net_rx_pkt_iovec_realloc(pkt, iovcnt); -- cgit v1.2.3 From 002d394fd492f837083058832edd7ee97a8c3280 Mon Sep 17 00:00:00 2001 From: Dmitry Fleytman Date: Thu, 16 Feb 2017 14:29:36 +0200 Subject: NetRxPkt: Remove code duplication in net_rx_pkt_pull_data() This is a refactoring commit that does not change behavior. Signed-off-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/net_rx_pkt.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/hw/net/net_rx_pkt.c b/hw/net/net_rx_pkt.c index 2649d40b5c..cef1c2e0d1 100644 --- a/hw/net/net_rx_pkt.c +++ b/hw/net/net_rx_pkt.c @@ -88,20 +88,21 @@ net_rx_pkt_pull_data(struct NetRxPkt *pkt, const struct iovec *iov, int iovcnt, size_t ploff) { + uint32_t pllen = iov_size(iov, iovcnt) - ploff; + if (pkt->ehdr_buf_len) { net_rx_pkt_iovec_realloc(pkt, iovcnt + 1); pkt->vec[0].iov_base = pkt->ehdr_buf; pkt->vec[0].iov_len = pkt->ehdr_buf_len; - pkt->tot_len = iov_size(iov, iovcnt) - ploff + pkt->ehdr_buf_len; + pkt->tot_len = pllen + pkt->ehdr_buf_len; pkt->vec_len = iov_copy(pkt->vec + 1, pkt->vec_len_total - 1, - iov, iovcnt, ploff, - pkt->tot_len - pkt->ehdr_buf_len) + 1; + iov, iovcnt, ploff, pllen) + 1; } else { net_rx_pkt_iovec_realloc(pkt, iovcnt); - pkt->tot_len = iov_size(iov, iovcnt) - ploff; + pkt->tot_len = pllen; pkt->vec_len = iov_copy(pkt->vec, pkt->vec_len_total, iov, iovcnt, ploff, pkt->tot_len); } -- cgit v1.2.3 From 66d2a2423ef3f2a3b066d7e9e3301e4fd42f51b3 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 17 Feb 2017 10:53:11 +0800 Subject: colo-compare: use g_timeout_source_new() to process the stale packets Instead of using qemu timer to process the stale packets, We re-use the colo compare thread to process these packets by creating a new timeout coroutine. Besides, since we process all the same vNIC's net connection/packets in one thread, it is safe to remove the timer_check_lock. Signed-off-by: zhanghailiang Signed-off-by: Jason Wang --- net/colo-compare.c | 62 +++++++++++++++++++----------------------------------- 1 file changed, 22 insertions(+), 40 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 162fd6a570..fdde788bb9 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -83,9 +83,6 @@ typedef struct CompareState { GHashTable *connection_track_table; /* compare thread, a thread for each NIC */ QemuThread thread; - /* Timer used on the primary to find packets that are never matched */ - QEMUTimer *timer; - QemuMutex timer_check_lock; } CompareState; typedef struct CompareClass { @@ -374,9 +371,7 @@ static void colo_compare_connection(void *opaque, void *user_data) while (!g_queue_is_empty(&conn->primary_list) && !g_queue_is_empty(&conn->secondary_list)) { - qemu_mutex_lock(&s->timer_check_lock); pkt = g_queue_pop_tail(&conn->primary_list); - qemu_mutex_unlock(&s->timer_check_lock); switch (conn->ip_proto) { case IPPROTO_TCP: result = g_queue_find_custom(&conn->secondary_list, @@ -411,9 +406,7 @@ static void colo_compare_connection(void *opaque, void *user_data) * until next comparison. */ trace_colo_compare_main("packet different"); - qemu_mutex_lock(&s->timer_check_lock); g_queue_push_tail(&conn->primary_list, pkt); - qemu_mutex_unlock(&s->timer_check_lock); /* TODO: colo_notify_checkpoint();*/ break; } @@ -486,11 +479,26 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size) } } +/* + * Check old packet regularly so it can watch for any packets + * that the secondary hasn't produced equivalents of. + */ +static gboolean check_old_packet_regular(void *opaque) +{ + CompareState *s = opaque; + + /* if have old packet we will notify checkpoint */ + colo_old_packet_check(s); + + return TRUE; +} + static void *colo_compare_thread(void *opaque) { GMainContext *worker_context; GMainLoop *compare_loop; CompareState *s = opaque; + GSource *timeout_source; worker_context = g_main_context_new(); @@ -501,8 +509,15 @@ static void *colo_compare_thread(void *opaque) compare_loop = g_main_loop_new(worker_context, FALSE); + /* To kick any packets that the secondary doesn't match */ + timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS); + g_source_set_callback(timeout_source, + (GSourceFunc)check_old_packet_regular, s, NULL); + g_source_attach(timeout_source, worker_context); + g_main_loop_run(compare_loop); + g_source_unref(timeout_source); g_main_loop_unref(compare_loop); g_main_context_unref(worker_context); return NULL; @@ -603,26 +618,6 @@ static int find_and_check_chardev(Chardev **chr, return 0; } -/* - * Check old packet regularly so it can watch for any packets - * that the secondary hasn't produced equivalents of. - */ -static void check_old_packet_regular(void *opaque) -{ - CompareState *s = opaque; - - timer_mod(s->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + - REGULAR_PACKET_CHECK_MS); - /* if have old packet we will notify checkpoint */ - /* - * TODO: Make timer handler run in compare thread - * like qemu_chr_add_handlers_full. - */ - qemu_mutex_lock(&s->timer_check_lock); - colo_old_packet_check(s); - qemu_mutex_unlock(&s->timer_check_lock); -} - /* * Called from the main thread on the primary * to setup colo-compare. @@ -665,7 +660,6 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize); g_queue_init(&s->conn_list); - qemu_mutex_init(&s->timer_check_lock); s->connection_track_table = g_hash_table_new_full(connection_key_hash, connection_key_equal, @@ -678,12 +672,6 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) QEMU_THREAD_JOINABLE); compare_id++; - /* A regular timer to kick any packets that the secondary doesn't match */ - s->timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, /* Only when guest runs */ - check_old_packet_regular, s); - timer_mod(s->timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + - REGULAR_PACKET_CHECK_MS); - return; } @@ -723,12 +711,6 @@ static void colo_compare_finalize(Object *obj) qemu_thread_join(&s->thread); } - if (s->timer) { - timer_del(s->timer); - } - - qemu_mutex_destroy(&s->timer_check_lock); - g_free(s->pri_indev); g_free(s->sec_indev); g_free(s->outdev); -- cgit v1.2.3 From dfd917a9c2bed578c31043126c9f558190bf21e4 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 17 Feb 2017 10:53:12 +0800 Subject: colo-compare: kick compare thread to exit after some cleanup in finalization We should call g_main_loop_quit() to notify colo compare thread to exit, Or it will run in g_main_loop_run() forever. Besides, the finalizing process can't happen in context of colo thread, it is reasonable to remove the 'if (qemu_thread_is_self(&s->thread))' branch. Before compare thead exits, some cleanup works need to be done, All unhandled packets need to be released and connection_track_table needs to be freed, or there will be memory leak. Signed-off-by: zhanghailiang Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index fdde788bb9..37ce75c3e4 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -83,6 +83,8 @@ typedef struct CompareState { GHashTable *connection_track_table; /* compare thread, a thread for each NIC */ QemuThread thread; + + GMainLoop *compare_loop; } CompareState; typedef struct CompareClass { @@ -496,7 +498,6 @@ static gboolean check_old_packet_regular(void *opaque) static void *colo_compare_thread(void *opaque) { GMainContext *worker_context; - GMainLoop *compare_loop; CompareState *s = opaque; GSource *timeout_source; @@ -507,7 +508,7 @@ static void *colo_compare_thread(void *opaque) qemu_chr_fe_set_handlers(&s->chr_sec_in, compare_chr_can_read, compare_sec_chr_in, NULL, s, worker_context, true); - compare_loop = g_main_loop_new(worker_context, FALSE); + s->compare_loop = g_main_loop_new(worker_context, FALSE); /* To kick any packets that the secondary doesn't match */ timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS); @@ -515,10 +516,10 @@ static void *colo_compare_thread(void *opaque) (GSourceFunc)check_old_packet_regular, s, NULL); g_source_attach(timeout_source, worker_context); - g_main_loop_run(compare_loop); + g_main_loop_run(s->compare_loop); g_source_unref(timeout_source); - g_main_loop_unref(compare_loop); + g_main_loop_unref(s->compare_loop); g_main_context_unref(worker_context); return NULL; } @@ -675,6 +676,23 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp) return; } +static void colo_flush_packets(void *opaque, void *user_data) +{ + CompareState *s = user_data; + Connection *conn = opaque; + Packet *pkt = NULL; + + while (!g_queue_is_empty(&conn->primary_list)) { + pkt = g_queue_pop_head(&conn->primary_list); + compare_chr_send(&s->chr_out, pkt->data, pkt->size); + packet_destroy(pkt, NULL); + } + while (!g_queue_is_empty(&conn->secondary_list)) { + pkt = g_queue_pop_head(&conn->secondary_list); + packet_destroy(pkt, NULL); + } +} + static void colo_compare_class_init(ObjectClass *oc, void *data) { UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc); @@ -703,14 +721,15 @@ static void colo_compare_finalize(Object *obj) qemu_chr_fe_deinit(&s->chr_sec_in); qemu_chr_fe_deinit(&s->chr_out); - g_queue_free(&s->conn_list); + g_main_loop_quit(s->compare_loop); + qemu_thread_join(&s->thread); - if (qemu_thread_is_self(&s->thread)) { - /* compare connection */ - g_queue_foreach(&s->conn_list, colo_compare_connection, s); - qemu_thread_join(&s->thread); - } + /* Release all unhandled packets after compare thead exited */ + g_queue_foreach(&s->conn_list, colo_flush_packets, s); + + g_queue_free(&s->conn_list); + g_hash_table_destroy(s->connection_track_table); g_free(s->pri_indev); g_free(s->sec_indev); g_free(s->outdev); -- cgit v1.2.3 From 8487ce45f890cab208541f01e79579d7b25e1615 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 17 Feb 2017 10:53:13 +0800 Subject: char: remove the right fd been watched in qemu_chr_fe_set_handlers() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can call qemu_chr_fe_set_handlers() to add/remove fd been watched in 'context' which can be either default main context or other explicit context. But the original logic is not correct, we didn't remove the right fd because we call g_main_context_find_source_by_id(NULL, tag) which always try to find the Gsource from default context. Fix it by passing the right context to g_main_context_find_source_by_id(). Cc: Paolo Bonzini Cc: Marc-André Lureau Signed-off-by: zhanghailiang Reviewed-by: Marc-André Lureau Signed-off-by: Jason Wang --- chardev/char-fd.c | 6 +++--- chardev/char-io.c | 8 ++++---- chardev/char-io.h | 2 +- chardev/char-pty.c | 2 +- chardev/char-socket.c | 4 ++-- chardev/char-udp.c | 6 +++--- chardev/char.c | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/chardev/char-fd.c b/chardev/char-fd.c index fb51ab4234..548dd4cdd9 100644 --- a/chardev/char-fd.c +++ b/chardev/char-fd.c @@ -58,7 +58,7 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) ret = qio_channel_read( chan, (gchar *)buf, len, NULL); if (ret == 0) { - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); qemu_chr_be_event(chr, CHR_EVENT_CLOSED); return FALSE; } @@ -89,7 +89,7 @@ static void fd_chr_update_read_handler(Chardev *chr, { FDChardev *s = FD_CHARDEV(chr); - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); if (s->ioc_in) { chr->fd_in_tag = io_add_watch_poll(chr, s->ioc_in, fd_chr_read_poll, @@ -103,7 +103,7 @@ static void char_fd_finalize(Object *obj) Chardev *chr = CHARDEV(obj); FDChardev *s = FD_CHARDEV(obj); - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); if (s->ioc_in) { object_unref(OBJECT(s->ioc_in)); } diff --git a/chardev/char-io.c b/chardev/char-io.c index 7dfc3f22ba..b4bb094ea3 100644 --- a/chardev/char-io.c +++ b/chardev/char-io.c @@ -127,14 +127,14 @@ guint io_add_watch_poll(Chardev *chr, return tag; } -static void io_remove_watch_poll(guint tag) +static void io_remove_watch_poll(guint tag, GMainContext *context) { GSource *source; IOWatchPoll *iwp; g_return_if_fail(tag > 0); - source = g_main_context_find_source_by_id(NULL, tag); + source = g_main_context_find_source_by_id(context, tag); g_return_if_fail(source != NULL); iwp = io_watch_poll_from_source(source); @@ -146,10 +146,10 @@ static void io_remove_watch_poll(guint tag) g_source_destroy(&iwp->parent); } -void remove_fd_in_watch(Chardev *chr) +void remove_fd_in_watch(Chardev *chr, GMainContext *context) { if (chr->fd_in_tag) { - io_remove_watch_poll(chr->fd_in_tag); + io_remove_watch_poll(chr->fd_in_tag, context); chr->fd_in_tag = 0; } } diff --git a/chardev/char-io.h b/chardev/char-io.h index d7ae5f1585..842be56bda 100644 --- a/chardev/char-io.h +++ b/chardev/char-io.h @@ -36,7 +36,7 @@ guint io_add_watch_poll(Chardev *chr, gpointer user_data, GMainContext *context); -void remove_fd_in_watch(Chardev *chr); +void remove_fd_in_watch(Chardev *chr, GMainContext *context); int io_channel_send(QIOChannel *ioc, const void *buf, size_t len); diff --git a/chardev/char-pty.c b/chardev/char-pty.c index ecf2c7a5c4..a6337be8aa 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -199,7 +199,7 @@ static void pty_chr_state(Chardev *chr, int connected) g_source_remove(s->open_tag); s->open_tag = 0; } - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); s->connected = 0; /* (re-)connect poll interval for idle guests: once per second. * We check more frequently in case the guests sends data to diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 865c52762e..d7e92e1bd3 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -328,7 +328,7 @@ static void tcp_chr_free_connection(Chardev *chr) } tcp_set_msgfds(chr, NULL, 0); - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); object_unref(OBJECT(s->sioc)); s->sioc = NULL; object_unref(OBJECT(s->ioc)); @@ -498,7 +498,7 @@ static void tcp_chr_update_read_handler(Chardev *chr, return; } - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); if (s->ioc) { chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, tcp_chr_read_poll, diff --git a/chardev/char-udp.c b/chardev/char-udp.c index 2c6c7ddd73..804bd22efa 100644 --- a/chardev/char-udp.c +++ b/chardev/char-udp.c @@ -81,7 +81,7 @@ static gboolean udp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque) ret = qio_channel_read( s->ioc, (char *)s->buf, sizeof(s->buf), NULL); if (ret <= 0) { - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); return FALSE; } s->bufcnt = ret; @@ -101,7 +101,7 @@ static void udp_chr_update_read_handler(Chardev *chr, { UdpChardev *s = UDP_CHARDEV(chr); - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); if (s->ioc) { chr->fd_in_tag = io_add_watch_poll(chr, s->ioc, udp_chr_read_poll, @@ -115,7 +115,7 @@ static void char_udp_finalize(Object *obj) Chardev *chr = CHARDEV(obj); UdpChardev *s = UDP_CHARDEV(obj); - remove_fd_in_watch(chr); + remove_fd_in_watch(chr, NULL); if (s->ioc) { object_unref(OBJECT(s->ioc)); } diff --git a/chardev/char.c b/chardev/char.c index 54cd5f4081..3df116350b 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -560,7 +560,7 @@ void qemu_chr_fe_set_handlers(CharBackend *b, cc = CHARDEV_GET_CLASS(s); if (!opaque && !fd_can_read && !fd_read && !fd_event) { fe_open = 0; - remove_fd_in_watch(s); + remove_fd_in_watch(s, context); } else { fe_open = 1; } -- cgit v1.2.3 From b43decb015a6efeb9e3cdbdb80f6547ad7248a4c Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 17 Feb 2017 10:53:14 +0800 Subject: colo-compare: Fix removing fds been watched incorrectly in finalization We will catch the bellow error report while try to delete compare object by qmp command: chardev/char-io.c:91: io_watch_poll_finalize: Assertion `iwp->src == ((void *)0)' failed. This is caused by failing to remove the right fd been watched while call qemu_chr_fe_set_handlers(); Fix it by pass the worker_context parameter to qemu_chr_fe_set_handlers(). Signed-off-by: zhanghailiang Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 37ce75c3e4..a6fc2ff48b 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -84,6 +84,7 @@ typedef struct CompareState { /* compare thread, a thread for each NIC */ QemuThread thread; + GMainContext *worker_context; GMainLoop *compare_loop; } CompareState; @@ -497,30 +498,29 @@ static gboolean check_old_packet_regular(void *opaque) static void *colo_compare_thread(void *opaque) { - GMainContext *worker_context; CompareState *s = opaque; GSource *timeout_source; - worker_context = g_main_context_new(); + s->worker_context = g_main_context_new(); qemu_chr_fe_set_handlers(&s->chr_pri_in, compare_chr_can_read, - compare_pri_chr_in, NULL, s, worker_context, true); + compare_pri_chr_in, NULL, s, s->worker_context, true); qemu_chr_fe_set_handlers(&s->chr_sec_in, compare_chr_can_read, - compare_sec_chr_in, NULL, s, worker_context, true); + compare_sec_chr_in, NULL, s, s->worker_context, true); - s->compare_loop = g_main_loop_new(worker_context, FALSE); + s->compare_loop = g_main_loop_new(s->worker_context, FALSE); /* To kick any packets that the secondary doesn't match */ timeout_source = g_timeout_source_new(REGULAR_PACKET_CHECK_MS); g_source_set_callback(timeout_source, (GSourceFunc)check_old_packet_regular, s, NULL); - g_source_attach(timeout_source, worker_context); + g_source_attach(timeout_source, s->worker_context); g_main_loop_run(s->compare_loop); g_source_unref(timeout_source); g_main_loop_unref(s->compare_loop); - g_main_context_unref(worker_context); + g_main_context_unref(s->worker_context); return NULL; } @@ -717,8 +717,10 @@ static void colo_compare_finalize(Object *obj) { CompareState *s = COLO_COMPARE(obj); - qemu_chr_fe_deinit(&s->chr_pri_in); - qemu_chr_fe_deinit(&s->chr_sec_in); + qemu_chr_fe_set_handlers(&s->chr_pri_in, NULL, NULL, NULL, NULL, + s->worker_context, true); + qemu_chr_fe_set_handlers(&s->chr_sec_in, NULL, NULL, NULL, NULL, + s->worker_context, true); qemu_chr_fe_deinit(&s->chr_out); g_main_loop_quit(s->compare_loop); -- cgit v1.2.3 From 727c2d764fef0c9492a2e220ba8a6d0979836d36 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Wed, 22 Feb 2017 13:16:06 +0800 Subject: net/colo-compare: Fix memory free error We use g_queue_init() to init s->conn_list, so we should use g_queue_clear() to instead of g_queue_free(). Signed-off-by: Zhang Chen Reviewed-by: zhanghailiang Signed-off-by: Jason Wang --- net/colo-compare.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index a6fc2ff48b..300f017b59 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -729,7 +729,7 @@ static void colo_compare_finalize(Object *obj) /* Release all unhandled packets after compare thead exited */ g_queue_foreach(&s->conn_list, colo_flush_packets, s); - g_queue_free(&s->conn_list); + g_queue_clear(&s->conn_list); g_hash_table_destroy(s->connection_track_table); g_free(s->pri_indev); -- cgit v1.2.3 From 5504bba1fbc987bb3d1d47e169f53b2fe999716c Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Thu, 15 Dec 2016 20:05:08 +0000 Subject: vmxnet3: Convert ring values to uint32_t's The index's in the Vmxnet3Ring were migrated as 32bit ints yet are declared as size_t's. They appear to be derived from 32bit values loaded from guest memory, so actually store them as that. Signed-off-by: Dr. David Alan Gilbert Acked-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/vmxnet3.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index e13a798b3b..224c109a53 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -141,17 +141,17 @@ typedef struct VMXNET3Class { /* Cyclic ring abstraction */ typedef struct { hwaddr pa; - size_t size; - size_t cell_size; - size_t next; + uint32_t size; + uint32_t cell_size; + uint32_t next; uint8_t gen; } Vmxnet3Ring; static inline void vmxnet3_ring_init(PCIDevice *d, Vmxnet3Ring *ring, hwaddr pa, - size_t size, - size_t cell_size, + uint32_t size, + uint32_t cell_size, bool zero_region) { ring->pa = pa; @@ -166,7 +166,7 @@ static inline void vmxnet3_ring_init(PCIDevice *d, } #define VMXNET3_RING_DUMP(macro, ring_name, ridx, r) \ - macro("%s#%d: base %" PRIx64 " size %zu cell_size %zu gen %d next %zu", \ + macro("%s#%d: base %" PRIx64 " size %u cell_size %u gen %d next %u", \ (ring_name), (ridx), \ (r)->pa, (r)->size, (r)->cell_size, (r)->gen, (r)->next) -- cgit v1.2.3 From a11f5cb005f9854f0d78da97fc23adf5bc8c83f3 Mon Sep 17 00:00:00 2001 From: "Dr. David Alan Gilbert" Date: Thu, 15 Dec 2016 20:05:09 +0000 Subject: vmxnet3: VMStatify rx/tx q_descr and int_state Fairly simple mechanical conversion of all fields. TODO!!!! The problem is vmxnet3-ring size/cell_size/next are declared as size_t but written as 32bit. Signed-off-by: Dr. David Alan Gilbert Acked-by: Dmitry Fleytman Signed-off-by: Jason Wang --- hw/net/vmxnet3.c | 272 ++++++++++++++++++------------------------------------- 1 file changed, 90 insertions(+), 182 deletions(-) diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c index 224c109a53..8b1fab24fd 100644 --- a/hw/net/vmxnet3.c +++ b/hw/net/vmxnet3.c @@ -2403,155 +2403,87 @@ static const VMStateDescription vmxstate_vmxnet3_mcast_list = { } }; -static void vmxnet3_get_ring_from_file(QEMUFile *f, Vmxnet3Ring *r) -{ - r->pa = qemu_get_be64(f); - r->size = qemu_get_be32(f); - r->cell_size = qemu_get_be32(f); - r->next = qemu_get_be32(f); - r->gen = qemu_get_byte(f); -} - -static void vmxnet3_put_ring_to_file(QEMUFile *f, Vmxnet3Ring *r) -{ - qemu_put_be64(f, r->pa); - qemu_put_be32(f, r->size); - qemu_put_be32(f, r->cell_size); - qemu_put_be32(f, r->next); - qemu_put_byte(f, r->gen); -} - -static void vmxnet3_get_tx_stats_from_file(QEMUFile *f, - struct UPT1_TxStats *tx_stat) -{ - tx_stat->TSOPktsTxOK = qemu_get_be64(f); - tx_stat->TSOBytesTxOK = qemu_get_be64(f); - tx_stat->ucastPktsTxOK = qemu_get_be64(f); - tx_stat->ucastBytesTxOK = qemu_get_be64(f); - tx_stat->mcastPktsTxOK = qemu_get_be64(f); - tx_stat->mcastBytesTxOK = qemu_get_be64(f); - tx_stat->bcastPktsTxOK = qemu_get_be64(f); - tx_stat->bcastBytesTxOK = qemu_get_be64(f); - tx_stat->pktsTxError = qemu_get_be64(f); - tx_stat->pktsTxDiscard = qemu_get_be64(f); -} - -static void vmxnet3_put_tx_stats_to_file(QEMUFile *f, - struct UPT1_TxStats *tx_stat) -{ - qemu_put_be64(f, tx_stat->TSOPktsTxOK); - qemu_put_be64(f, tx_stat->TSOBytesTxOK); - qemu_put_be64(f, tx_stat->ucastPktsTxOK); - qemu_put_be64(f, tx_stat->ucastBytesTxOK); - qemu_put_be64(f, tx_stat->mcastPktsTxOK); - qemu_put_be64(f, tx_stat->mcastBytesTxOK); - qemu_put_be64(f, tx_stat->bcastPktsTxOK); - qemu_put_be64(f, tx_stat->bcastBytesTxOK); - qemu_put_be64(f, tx_stat->pktsTxError); - qemu_put_be64(f, tx_stat->pktsTxDiscard); -} - -static int vmxnet3_get_txq_descr(QEMUFile *f, void *pv, size_t size, - VMStateField *field) -{ - Vmxnet3TxqDescr *r = pv; - - vmxnet3_get_ring_from_file(f, &r->tx_ring); - vmxnet3_get_ring_from_file(f, &r->comp_ring); - r->intr_idx = qemu_get_byte(f); - r->tx_stats_pa = qemu_get_be64(f); - - vmxnet3_get_tx_stats_from_file(f, &r->txq_stats); - - return 0; -} - -static int vmxnet3_put_txq_descr(QEMUFile *f, void *pv, size_t size, - VMStateField *field, QJSON *vmdesc) -{ - Vmxnet3TxqDescr *r = pv; - - vmxnet3_put_ring_to_file(f, &r->tx_ring); - vmxnet3_put_ring_to_file(f, &r->comp_ring); - qemu_put_byte(f, r->intr_idx); - qemu_put_be64(f, r->tx_stats_pa); - vmxnet3_put_tx_stats_to_file(f, &r->txq_stats); - - return 0; -} - -static const VMStateInfo txq_descr_info = { - .name = "txq_descr", - .get = vmxnet3_get_txq_descr, - .put = vmxnet3_put_txq_descr +static const VMStateDescription vmstate_vmxnet3_ring = { + .name = "vmxnet3-ring", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_UINT64(pa, Vmxnet3Ring), + VMSTATE_UINT32(size, Vmxnet3Ring), + VMSTATE_UINT32(cell_size, Vmxnet3Ring), + VMSTATE_UINT32(next, Vmxnet3Ring), + VMSTATE_UINT8(gen, Vmxnet3Ring), + VMSTATE_END_OF_LIST() + } }; -static void vmxnet3_get_rx_stats_from_file(QEMUFile *f, - struct UPT1_RxStats *rx_stat) -{ - rx_stat->LROPktsRxOK = qemu_get_be64(f); - rx_stat->LROBytesRxOK = qemu_get_be64(f); - rx_stat->ucastPktsRxOK = qemu_get_be64(f); - rx_stat->ucastBytesRxOK = qemu_get_be64(f); - rx_stat->mcastPktsRxOK = qemu_get_be64(f); - rx_stat->mcastBytesRxOK = qemu_get_be64(f); - rx_stat->bcastPktsRxOK = qemu_get_be64(f); - rx_stat->bcastBytesRxOK = qemu_get_be64(f); - rx_stat->pktsRxOutOfBuf = qemu_get_be64(f); - rx_stat->pktsRxError = qemu_get_be64(f); -} - -static void vmxnet3_put_rx_stats_to_file(QEMUFile *f, - struct UPT1_RxStats *rx_stat) -{ - qemu_put_be64(f, rx_stat->LROPktsRxOK); - qemu_put_be64(f, rx_stat->LROBytesRxOK); - qemu_put_be64(f, rx_stat->ucastPktsRxOK); - qemu_put_be64(f, rx_stat->ucastBytesRxOK); - qemu_put_be64(f, rx_stat->mcastPktsRxOK); - qemu_put_be64(f, rx_stat->mcastBytesRxOK); - qemu_put_be64(f, rx_stat->bcastPktsRxOK); - qemu_put_be64(f, rx_stat->bcastBytesRxOK); - qemu_put_be64(f, rx_stat->pktsRxOutOfBuf); - qemu_put_be64(f, rx_stat->pktsRxError); -} - -static int vmxnet3_get_rxq_descr(QEMUFile *f, void *pv, size_t size, - VMStateField *field) -{ - Vmxnet3RxqDescr *r = pv; - int i; - - for (i = 0; i < VMXNET3_RX_RINGS_PER_QUEUE; i++) { - vmxnet3_get_ring_from_file(f, &r->rx_ring[i]); +static const VMStateDescription vmstate_vmxnet3_tx_stats = { + .name = "vmxnet3-tx-stats", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_UINT64(TSOPktsTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(TSOBytesTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(ucastPktsTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(ucastBytesTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(mcastPktsTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(mcastBytesTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(bcastPktsTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(bcastBytesTxOK, struct UPT1_TxStats), + VMSTATE_UINT64(pktsTxError, struct UPT1_TxStats), + VMSTATE_UINT64(pktsTxDiscard, struct UPT1_TxStats), + VMSTATE_END_OF_LIST() } +}; - vmxnet3_get_ring_from_file(f, &r->comp_ring); - r->intr_idx = qemu_get_byte(f); - r->rx_stats_pa = qemu_get_be64(f); - - vmxnet3_get_rx_stats_from_file(f, &r->rxq_stats); - - return 0; -} - -static int vmxnet3_put_rxq_descr(QEMUFile *f, void *pv, size_t size, - VMStateField *field, QJSON *vmdesc) -{ - Vmxnet3RxqDescr *r = pv; - int i; - - for (i = 0; i < VMXNET3_RX_RINGS_PER_QUEUE; i++) { - vmxnet3_put_ring_to_file(f, &r->rx_ring[i]); +static const VMStateDescription vmstate_vmxnet3_txq_descr = { + .name = "vmxnet3-txq-descr", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_STRUCT(tx_ring, Vmxnet3TxqDescr, 0, vmstate_vmxnet3_ring, + Vmxnet3Ring), + VMSTATE_STRUCT(comp_ring, Vmxnet3TxqDescr, 0, vmstate_vmxnet3_ring, + Vmxnet3Ring), + VMSTATE_UINT8(intr_idx, Vmxnet3TxqDescr), + VMSTATE_UINT64(tx_stats_pa, Vmxnet3TxqDescr), + VMSTATE_STRUCT(txq_stats, Vmxnet3TxqDescr, 0, vmstate_vmxnet3_tx_stats, + struct UPT1_TxStats), + VMSTATE_END_OF_LIST() } +}; - vmxnet3_put_ring_to_file(f, &r->comp_ring); - qemu_put_byte(f, r->intr_idx); - qemu_put_be64(f, r->rx_stats_pa); - vmxnet3_put_rx_stats_to_file(f, &r->rxq_stats); +static const VMStateDescription vmstate_vmxnet3_rx_stats = { + .name = "vmxnet3-rx-stats", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_UINT64(LROPktsRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(LROBytesRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(ucastPktsRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(ucastBytesRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(mcastPktsRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(mcastBytesRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(bcastPktsRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(bcastBytesRxOK, struct UPT1_RxStats), + VMSTATE_UINT64(pktsRxOutOfBuf, struct UPT1_RxStats), + VMSTATE_UINT64(pktsRxError, struct UPT1_RxStats), + VMSTATE_END_OF_LIST() + } +}; - return 0; -} +static const VMStateDescription vmstate_vmxnet3_rxq_descr = { + .name = "vmxnet3-rxq-descr", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_STRUCT_ARRAY(rx_ring, Vmxnet3RxqDescr, + VMXNET3_RX_RINGS_PER_QUEUE, 0, + vmstate_vmxnet3_ring, Vmxnet3Ring), + VMSTATE_STRUCT(comp_ring, Vmxnet3RxqDescr, 0, vmstate_vmxnet3_ring, + Vmxnet3Ring), + VMSTATE_UINT8(intr_idx, Vmxnet3RxqDescr), + VMSTATE_UINT64(rx_stats_pa, Vmxnet3RxqDescr), + VMSTATE_STRUCT(rxq_stats, Vmxnet3RxqDescr, 0, vmstate_vmxnet3_rx_stats, + struct UPT1_RxStats), + VMSTATE_END_OF_LIST() + } +}; static int vmxnet3_post_load(void *opaque, int version_id) { @@ -2577,40 +2509,15 @@ static int vmxnet3_post_load(void *opaque, int version_id) return 0; } -static const VMStateInfo rxq_descr_info = { - .name = "rxq_descr", - .get = vmxnet3_get_rxq_descr, - .put = vmxnet3_put_rxq_descr -}; - -static int vmxnet3_get_int_state(QEMUFile *f, void *pv, size_t size, - VMStateField *field) -{ - Vmxnet3IntState *r = pv; - - r->is_masked = qemu_get_byte(f); - r->is_pending = qemu_get_byte(f); - r->is_asserted = qemu_get_byte(f); - - return 0; -} - -static int vmxnet3_put_int_state(QEMUFile *f, void *pv, size_t size, - VMStateField *field, QJSON *vmdesc) -{ - Vmxnet3IntState *r = pv; - - qemu_put_byte(f, r->is_masked); - qemu_put_byte(f, r->is_pending); - qemu_put_byte(f, r->is_asserted); - - return 0; -} - -static const VMStateInfo int_state_info = { - .name = "int_state", - .get = vmxnet3_get_int_state, - .put = vmxnet3_put_int_state +static const VMStateDescription vmstate_vmxnet3_int_state = { + .name = "vmxnet3-int-state", + .version_id = 0, + .fields = (VMStateField[]) { + VMSTATE_BOOL(is_masked, Vmxnet3IntState), + VMSTATE_BOOL(is_pending, Vmxnet3IntState), + VMSTATE_BOOL(is_asserted, Vmxnet3IntState), + VMSTATE_END_OF_LIST() + } }; static bool vmxnet3_vmstate_need_pcie_device(void *opaque) @@ -2667,14 +2574,15 @@ static const VMStateDescription vmstate_vmxnet3 = { VMSTATE_UINT64(drv_shmem, VMXNET3State), VMSTATE_UINT64(temp_shared_guest_driver_memory, VMXNET3State), - VMSTATE_ARRAY(txq_descr, VMXNET3State, - VMXNET3_DEVICE_MAX_TX_QUEUES, 0, txq_descr_info, + VMSTATE_STRUCT_ARRAY(txq_descr, VMXNET3State, + VMXNET3_DEVICE_MAX_TX_QUEUES, 0, vmstate_vmxnet3_txq_descr, Vmxnet3TxqDescr), - VMSTATE_ARRAY(rxq_descr, VMXNET3State, - VMXNET3_DEVICE_MAX_RX_QUEUES, 0, rxq_descr_info, + VMSTATE_STRUCT_ARRAY(rxq_descr, VMXNET3State, + VMXNET3_DEVICE_MAX_RX_QUEUES, 0, vmstate_vmxnet3_rxq_descr, Vmxnet3RxqDescr), - VMSTATE_ARRAY(interrupt_states, VMXNET3State, VMXNET3_MAX_INTRS, - 0, int_state_info, Vmxnet3IntState), + VMSTATE_STRUCT_ARRAY(interrupt_states, VMXNET3State, + VMXNET3_MAX_INTRS, 0, vmstate_vmxnet3_int_state, + Vmxnet3IntState), VMSTATE_END_OF_LIST() }, -- cgit v1.2.3 From 0e79668e1ffcfabb259bea6c2a2bae00a6b27252 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Tue, 28 Feb 2017 11:54:18 +0800 Subject: net/colo: fix memory double free error The 'primary_list' and 'secondary_list' members of struct Connection is not allocated through dynamically g_queue_new(), but we free it by using g_queue_free(), which will lead to a double-free bug. Reviewed-by: Zhang Chen Signed-off-by: zhanghailiang Signed-off-by: Jason Wang --- net/colo.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/colo.c b/net/colo.c index 6a6eacd2dc..8cc166bc22 100644 --- a/net/colo.c +++ b/net/colo.c @@ -147,9 +147,9 @@ void connection_destroy(void *opaque) Connection *conn = opaque; g_queue_foreach(&conn->primary_list, packet_destroy, NULL); - g_queue_free(&conn->primary_list); + g_queue_clear(&conn->primary_list); g_queue_foreach(&conn->secondary_list, packet_destroy, NULL); - g_queue_free(&conn->secondary_list); + g_queue_clear(&conn->secondary_list); g_slice_free(Connection, conn); } -- cgit v1.2.3 From db0a762e4be965b8976abe9df82c6d47c57336fc Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Tue, 28 Feb 2017 11:54:19 +0800 Subject: filter-rewriter: skip net_checksum_calculate() while offset = 0 While the offset of packets's sequence for primary side and secondary side is zero, it is unnecessary to call net_checksum_calculate() to recalculate the checksume value of packets. Signed-off-by: zhanghailiang Signed-off-by: Jason Wang --- net/filter-rewriter.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c index c4ab91cdee..afa06e8919 100644 --- a/net/filter-rewriter.c +++ b/net/filter-rewriter.c @@ -93,10 +93,12 @@ static int handle_primary_tcp_pkt(NetFilterState *nf, conn->offset -= (ntohl(tcp_pkt->th_ack) - 1); conn->syn_flag = 0; } - /* handle packets to the secondary from the primary */ - tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); + if (conn->offset) { + /* handle packets to the secondary from the primary */ + tcp_pkt->th_ack = htonl(ntohl(tcp_pkt->th_ack) + conn->offset); - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + } } return 0; @@ -129,10 +131,13 @@ static int handle_secondary_tcp_pkt(NetFilterState *nf, } if ((tcp_pkt->th_flags & (TH_ACK | TH_SYN)) == TH_ACK) { - /* handle packets to the primary from the secondary*/ - tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); + /* Only need to adjust seq while offset is Non-zero */ + if (conn->offset) { + /* handle packets to the primary from the secondary*/ + tcp_pkt->th_seq = htonl(ntohl(tcp_pkt->th_seq) - conn->offset); - net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + net_checksum_calculate((uint8_t *)pkt->data, pkt->size); + } } return 0; -- cgit v1.2.3 From 2ad7ca4c81733cba5c5c464078a643aba61044f8 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Thu, 2 Mar 2017 17:54:16 +0800 Subject: COLO-compare: Rename compare function and remove duplicate codes Rename colo_packet_compare() to colo_packet_compare_common() that make tcp_compare udp_compare icmp_compare reuse this function. Remove minimum packet size check in icmp_compare, because we have check this in parse_packet_early(). Signed-off-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 300f017b59..602a758343 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode) * return: 0 means packet same * > 0 || < 0 means packet different */ -static int colo_packet_compare(Packet *ppkt, Packet *spkt) +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt) { trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src), inet_ntoa(ppkt->ip->ip_dst), spkt->size, @@ -190,6 +190,7 @@ static int colo_packet_compare(Packet *ppkt, Packet *spkt) if (ppkt->size == spkt->size) { return memcmp(ppkt->data, spkt->data, spkt->size); } else { + trace_colo_compare_main("Net packet size are not the same"); return -1; } } @@ -205,6 +206,7 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) int res; trace_colo_compare_main("compare tcp"); + if (ppkt->size != spkt->size) { if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { trace_colo_compare_main("pkt size not same"); @@ -263,7 +265,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) int ret; trace_colo_compare_main("compare udp"); - ret = colo_packet_compare(ppkt, spkt); + + ret = colo_packet_compare_common(ppkt, spkt); if (ret) { trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); @@ -281,16 +284,9 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) */ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) { - int network_length; - trace_colo_compare_main("compare icmp"); - network_length = ppkt->ip->ip_hl * 4; - if (ppkt->size != spkt->size || - ppkt->size < network_length + ETH_HLEN) { - return -1; - } - if (colo_packet_compare(ppkt, spkt)) { + if (colo_packet_compare_common(ppkt, spkt)) { trace_colo_compare_icmp_miscompare("primary pkt size", ppkt->size); qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", @@ -316,7 +312,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) inet_ntoa(ppkt->ip->ip_dst), spkt->size, inet_ntoa(spkt->ip->ip_src), inet_ntoa(spkt->ip->ip_dst)); - return colo_packet_compare(ppkt, spkt); + return colo_packet_compare_common(ppkt, spkt); } static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) -- cgit v1.2.3 From 6efeb3286dd80c8c943f50fbb5f611d525cd6f8a Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Thu, 2 Mar 2017 17:54:17 +0800 Subject: COLO-compare: Optimize compare_common and compare_tcp Add offset args for colo_packet_compare_common, optimize colo_packet_compare_icmp() and colo_packet_compare_udp() just compare the IP payload. Before compare all tcp packet, we compare tcp checksum firstly, this function can get better performance. Signed-off-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 50 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 602a758343..9f5968dc0b 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -180,7 +180,7 @@ static int packet_enqueue(CompareState *s, int mode) * return: 0 means packet same * > 0 || < 0 means packet different */ -static int colo_packet_compare_common(Packet *ppkt, Packet *spkt) +static int colo_packet_compare_common(Packet *ppkt, Packet *spkt, int offset) { trace_colo_compare_ip_info(ppkt->size, inet_ntoa(ppkt->ip->ip_src), inet_ntoa(ppkt->ip->ip_dst), spkt->size, @@ -188,7 +188,8 @@ static int colo_packet_compare_common(Packet *ppkt, Packet *spkt) inet_ntoa(spkt->ip->ip_dst)); if (ppkt->size == spkt->size) { - return memcmp(ppkt->data, spkt->data, spkt->size); + return memcmp(ppkt->data + offset, spkt->data + offset, + spkt->size - offset); } else { trace_colo_compare_main("Net packet size are not the same"); return -1; @@ -207,13 +208,6 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) trace_colo_compare_main("compare tcp"); - if (ppkt->size != spkt->size) { - if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { - trace_colo_compare_main("pkt size not same"); - } - return -1; - } - ptcp = (struct tcphdr *)ppkt->transport_header; stcp = (struct tcphdr *)spkt->transport_header; @@ -231,8 +225,11 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) spkt->ip->ip_sum = ppkt->ip->ip_sum; } - res = memcmp(ppkt->data + ETH_HLEN, spkt->data + ETH_HLEN, - (spkt->size - ETH_HLEN)); + if (ptcp->th_sum == stcp->th_sum) { + res = colo_packet_compare_common(ppkt, spkt, ETH_HLEN); + } else { + res = -1; + } if (res != 0 && trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { trace_colo_compare_pkt_info_src(inet_ntoa(ppkt->ip->ip_src), @@ -263,10 +260,22 @@ static int colo_packet_compare_tcp(Packet *spkt, Packet *ppkt) static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) { int ret; + int network_header_length = ppkt->ip->ip_hl * 4; trace_colo_compare_main("compare udp"); - ret = colo_packet_compare_common(ppkt, spkt); + /* + * Because of ppkt and spkt are both in the same connection, + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are + * same with spkt. In addition, IP header's Identification is a random + * field, we can handle it in IP fragmentation function later. + * COLO just concern the response net packet payload from primary guest + * and secondary guest are same or not, So we ignored all IP header include + * other field like TOS,TTL,IP Checksum. we only need to compare + * the ip payload here. + */ + ret = colo_packet_compare_common(ppkt, spkt, + network_header_length + ETH_HLEN); if (ret) { trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); @@ -284,9 +293,22 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) */ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) { + int network_header_length = ppkt->ip->ip_hl * 4; + trace_colo_compare_main("compare icmp"); - if (colo_packet_compare_common(ppkt, spkt)) { + /* + * Because of ppkt and spkt are both in the same connection, + * The ppkt's src ip, dst ip, src port, dst port, ip_proto all are + * same with spkt. In addition, IP header's Identification is a random + * field, we can handle it in IP fragmentation function later. + * COLO just concern the response net packet payload from primary guest + * and secondary guest are same or not, So we ignored all IP header include + * other field like TOS,TTL,IP Checksum. we only need to compare + * the ip payload here. + */ + if (colo_packet_compare_common(ppkt, spkt, + network_header_length + ETH_HLEN)) { trace_colo_compare_icmp_miscompare("primary pkt size", ppkt->size); qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", @@ -312,7 +334,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet *ppkt) inet_ntoa(ppkt->ip->ip_dst), spkt->size, inet_ntoa(spkt->ip->ip_src), inet_ntoa(spkt->ip->ip_dst)); - return colo_packet_compare_common(ppkt, spkt); + return colo_packet_compare_common(ppkt, spkt, 0); } static int colo_old_packet_check_one(Packet *pkt, int64_t *check_time) -- cgit v1.2.3 From 1723a7f7cfdee92c5479aa497a6c36dec3d0ebd5 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Thu, 2 Mar 2017 17:54:18 +0800 Subject: COLO-compare: Fix icmp and udp compare different packet always dump bug Signed-off-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 9f5968dc0b..282727b28a 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -279,9 +279,13 @@ static int colo_packet_compare_udp(Packet *spkt, Packet *ppkt) if (ret) { trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size); - qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", ppkt->size); trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size); - qemu_hexdump((char *)spkt->data, stderr, "colo-compare", spkt->size); + if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { + qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt", + ppkt->size); + qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", + spkt->size); + } } return ret; @@ -311,12 +315,14 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet *ppkt) network_header_length + ETH_HLEN)) { trace_colo_compare_icmp_miscompare("primary pkt size", ppkt->size); - qemu_hexdump((char *)ppkt->data, stderr, "colo-compare", - ppkt->size); trace_colo_compare_icmp_miscompare("Secondary pkt size", spkt->size); - qemu_hexdump((char *)spkt->data, stderr, "colo-compare", - spkt->size); + if (trace_event_get_state(TRACE_COLO_COMPARE_MISCOMPARE)) { + qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt", + ppkt->size); + qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt", + spkt->size); + } return -1; } else { return 0; -- cgit v1.2.3 From f0aabd5c4ae11fde704d138e8f06c69e5c902a16 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Thu, 2 Mar 2017 11:59:30 +0800 Subject: net/filter-mirror: Follow CODING_STYLE Signed-off-by: Zhang Chen Signed-off-by: Jason Wang --- net/filter-mirror.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/filter-mirror.c b/net/filter-mirror.c index aa0aa98fa5..72fa7c2b6c 100644 --- a/net/filter-mirror.c +++ b/net/filter-mirror.c @@ -49,7 +49,7 @@ static int filter_mirror_send(CharBackend *chr_out, { int ret = 0; ssize_t size = 0; - uint32_t len = 0; + uint32_t len = 0; char *buf; size = iov_size(iov, iovcnt); @@ -77,8 +77,9 @@ err: return ret < 0 ? ret : -EIO; } -static void -redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len) +static void redirector_to_filter(NetFilterState *nf, + const uint8_t *buf, + int len) { struct iovec iov = { .iov_base = (void *)buf, -- cgit v1.2.3