| Message ID | 1375812792-25849-1-git-send-email-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 6, 2013 at 9:13 PM, Marc Zyngier <marc.zyngier@arm.com> wrote: > When fa7226f (kvm tools: init network devices only when the virtio > driver is ready to go) was introduced, a tiny detail was overlooked: > > - Initialization of the uip layer is now coming in very late (only > when the guest driver says it is ready). > - In parallel, the rx thread is created quite early (as soon as the > queues are allocated). > > This cause the rx thread to call uip_rx, which calls uip_buf_get_used, > which starts to use buf_lock mutex/the buf_used_cond, which haven't > been initialized yet. Tears and devastation follow, not to mention a > certain lack of network connectivity for the unsuspecting guest. > > The (not so pretty) fix is to split uip_init: > - uip_static_init: initialize the lists, mutexes and conditions, > called from virtio_net__init_one. > - uip_init: perform the dynamic memory allocations, called from > notify_status. > > This allows the network to be safely initialized. > > Cc: Sasha Levin <sasha.levin@oracle.com> > Cc: Pekka Enberg <penberg@kernel.org> > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> Applied, thanks a lot! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tools/kvm/include/kvm/uip.h b/tools/kvm/include/kvm/uip.h index 4e63808..a9a8d85 100644 --- a/tools/kvm/include/kvm/uip.h +++ b/tools/kvm/include/kvm/uip.h @@ -334,6 +334,7 @@ static inline u16 uip_eth_hdrlen(struct uip_eth *eth) int uip_tx(struct iovec *iov, u16 out, struct uip_info *info); int uip_rx(struct iovec *iov, u16 in, struct uip_info *info); +void uip_static_init(struct uip_info *info); int uip_init(struct uip_info *info); int uip_tx_do_ipv4_udp_dhcp(struct uip_tx_arg *arg); diff --git a/tools/kvm/net/uip/core.c b/tools/kvm/net/uip/core.c index 789b814..b3cd8c2 100644 --- a/tools/kvm/net/uip/core.c +++ b/tools/kvm/net/uip/core.c @@ -94,19 +94,15 @@ int uip_rx(struct iovec *iov, u16 in, struct uip_info *info) return len; } -int uip_init(struct uip_info *info) +void uip_static_init(struct uip_info *info) { struct list_head *udp_socket_head; struct list_head *tcp_socket_head; struct list_head *buf_head; - struct uip_buf *buf; - int buf_nr; - int i; udp_socket_head = &info->udp_socket_head; tcp_socket_head = &info->tcp_socket_head; buf_head = &info->buf_head; - buf_nr = info->buf_nr; INIT_LIST_HEAD(udp_socket_head); INIT_LIST_HEAD(tcp_socket_head); @@ -119,6 +115,18 @@ int uip_init(struct uip_info *info) pthread_cond_init(&info->buf_used_cond, NULL); pthread_cond_init(&info->buf_free_cond, NULL); + info->buf_used_nr = 0; +} + +int uip_init(struct uip_info *info) +{ + struct list_head *buf_head; + struct uip_buf *buf; + int buf_nr; + int i; + + buf_head = &info->buf_head; + buf_nr = info->buf_nr; for (i = 0; i < buf_nr; i++) { buf = malloc(sizeof(*buf)); @@ -141,7 +149,6 @@ int uip_init(struct uip_info *info) } info->buf_free_nr = buf_nr; - info->buf_used_nr = 0; uip_dhcp_get_dns(info); diff --git a/tools/kvm/virtio/net.c b/tools/kvm/virtio/net.c index 298903c..2c34996 100644 --- a/tools/kvm/virtio/net.c +++ b/tools/kvm/virtio/net.c @@ -712,6 +712,7 @@ static int virtio_net__init_one(struct virtio_net_params *params) ndev->info.guest_netmask = ntohl(inet_addr("255.255.255.0")); ndev->info.buf_nr = 20, ndev->ops = &uip_ops; + uip_static_init(&ndev->info); } if (params->trans && strcmp(params->trans, "mmio") == 0)
When fa7226f (kvm tools: init network devices only when the virtio driver is ready to go) was introduced, a tiny detail was overlooked: - Initialization of the uip layer is now coming in very late (only when the guest driver says it is ready). - In parallel, the rx thread is created quite early (as soon as the queues are allocated). This cause the rx thread to call uip_rx, which calls uip_buf_get_used, which starts to use buf_lock mutex/the buf_used_cond, which haven't been initialized yet. Tears and devastation follow, not to mention a certain lack of network connectivity for the unsuspecting guest. The (not so pretty) fix is to split uip_init: - uip_static_init: initialize the lists, mutexes and conditions, called from virtio_net__init_one. - uip_init: perform the dynamic memory allocations, called from notify_status. This allows the network to be safely initialized. Cc: Sasha Levin <sasha.levin@oracle.com> Cc: Pekka Enberg <penberg@kernel.org> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- tools/kvm/include/kvm/uip.h | 1 + tools/kvm/net/uip/core.c | 19 +++++++++++++------ tools/kvm/virtio/net.c | 1 + 3 files changed, 15 insertions(+), 6 deletions(-)