| Message ID | 1432800194-1192-1-git-send-email-thibaut.collet@6wind.com |
|---|---|
| State | New |
| Headers | show |
On Thu, May 28, 2015 at 10:03:14AM +0200, Thibaut Collet wrote: > For netdev backend with no receive callback, packets must not be enqueued. When > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That causes a > qemu segfault due to a call of a NULL pointer function. virtio-net NIC <-> vhost-user should bypass the QEMU net subsystem since rx/tx virtqueues are handled outside the QEMU process. Who is enqueuing packets on vhost-user's incoming_queue?
Hi, I agree that virtio-net NIC never enqueues packet to vhost-user but qemu_announce_self function (savevm.c file) can do it through the qemu_announce_self_iter / qemu_send_packet_raw sequence. Regards Thibaut. On Fri, May 29, 2015 at 3:12 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Thu, May 28, 2015 at 10:03:14AM +0200, Thibaut Collet wrote: > > For netdev backend with no receive callback, packets must not be > enqueued. When > > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That > causes a > > qemu segfault due to a call of a NULL pointer function. > > virtio-net NIC <-> vhost-user should bypass the QEMU net subsystem since > rx/tx virtqueues are handled outside the QEMU process. Who is enqueuing > packets on vhost-user's incoming_queue? >
On 05/28/2015 04:03 PM, Thibaut Collet wrote: > For netdev backend with no receive callback, packets must not be enqueued. When > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That causes a > qemu segfault due to a call of a NULL pointer function. > > Function to create net client is modified to allow backend to specify the size > of the queue. > For netdev backend with no receive callback, the queue size is set to 0 to > prevent enqueuing of packets. > For other netdev backend, a default queue size is provided. This value (set to > 10000) is the value previously set to any queue. > > Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com> > --- > include/net/net.h | 3 ++- > include/net/queue.h | 5 ++++- > net/dump.c | 3 ++- > net/hub.c | 3 ++- > net/l2tpv3.c | 3 ++- > net/net.c | 10 ++++++---- > net/netmap.c | 3 ++- > net/queue.c | 4 ++-- > net/slirp.c | 3 ++- > net/socket.c | 9 ++++++--- > net/tap-win32.c | 3 ++- > net/tap.c | 3 ++- > net/vde.c | 3 ++- > net/vhost-user.c | 4 +++- > 14 files changed, 39 insertions(+), 20 deletions(-) > > diff --git a/include/net/net.h b/include/net/net.h > index e66ca03..cb3e2df 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs, > NetClientState *qemu_new_net_client(NetClientInfo *info, > NetClientState *peer, > const char *model, > - const char *name); > + const char *name, > + uint32_t queue_size); > NICState *qemu_new_nic(NetClientInfo *info, > NICConf *conf, > const char *model, > diff --git a/include/net/queue.h b/include/net/queue.h > index fc02b33..4659c5a 100644 > --- a/include/net/queue.h > +++ b/include/net/queue.h > @@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret); > #define QEMU_NET_PACKET_FLAG_NONE 0 > #define QEMU_NET_PACKET_FLAG_RAW (1<<0) > > -NetQueue *qemu_new_net_queue(void *opaque); > +#define QEMU_DEFAULT_QUEUE_SIZE 10000 > +#define QEMU_EMPTY_QUEUE 0 > + > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size); > > void qemu_del_net_queue(NetQueue *queue); > > diff --git a/net/dump.c b/net/dump.c > index 9d3a09e..299b09e 100644 > --- a/net/dump.c > +++ b/net/dump.c > @@ -129,7 +129,8 @@ static int net_dump_init(NetClientState *peer, const char *device, > return -1; > } > > - nc = qemu_new_net_client(&net_dump_info, peer, device, name); > + nc = qemu_new_net_client(&net_dump_info, peer, device, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > snprintf(nc->info_str, sizeof(nc->info_str), > "dump to %s (len=%d)", filename, len); > diff --git a/net/hub.c b/net/hub.c > index 2b60ab9..a42cac3 100644 > --- a/net/hub.c > +++ b/net/hub.c > @@ -151,7 +151,8 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const char *name) > name = default_name; > } > > - nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name); > + nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name, > + QEMU_DEFAULT_QUEUE_SIZE); > port = DO_UPCAST(NetHubPort, nc, nc); > port->id = id; > port->hub = hub; > diff --git a/net/l2tpv3.c b/net/l2tpv3.c > index 8c598b0..1d2e4e5 100644 > --- a/net/l2tpv3.c > +++ b/net/l2tpv3.c > @@ -548,7 +548,8 @@ int net_init_l2tpv3(const NetClientOptions *opts, > struct addrinfo *result = NULL; > char *srcport, *dstport; > > - nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name); > + nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name, > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetL2TPV3State, nc, nc); > > diff --git a/net/net.c b/net/net.c > index 7427f6a..bcf69da 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -214,6 +214,7 @@ static void qemu_net_client_setup(NetClientState *nc, > NetClientState *peer, > const char *model, > const char *name, > + uint32_t queue_size, > NetClientDestructor *destructor) > { > nc->info = info; > @@ -231,21 +232,22 @@ static void qemu_net_client_setup(NetClientState *nc, > } > QTAILQ_INSERT_TAIL(&net_clients, nc, next); > > - nc->incoming_queue = qemu_new_net_queue(nc); > + nc->incoming_queue = qemu_new_net_queue(nc, queue_size); > nc->destructor = destructor; > } > > NetClientState *qemu_new_net_client(NetClientInfo *info, > NetClientState *peer, > const char *model, > - const char *name) > + const char *name, > + uint32_t queue_size) > { > NetClientState *nc; > > assert(info->size >= sizeof(NetClientState)); > > nc = g_malloc0(info->size); > - qemu_net_client_setup(nc, info, peer, model, name, > + qemu_net_client_setup(nc, info, peer, model, name, queue_size, > qemu_net_client_destructor); > > return nc; > @@ -271,7 +273,7 @@ NICState *qemu_new_nic(NetClientInfo *info, > > for (i = 0; i < queues; i++) { > qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name, > - NULL); > + QEMU_DEFAULT_QUEUE_SIZE, NULL); > nic->ncs[i].queue_index = i; > } > > diff --git a/net/netmap.c b/net/netmap.c > index 0c1772b..3ddfc8e 100644 > --- a/net/netmap.c > +++ b/net/netmap.c > @@ -461,7 +461,8 @@ int net_init_netmap(const NetClientOptions *opts, > return -1; > } > /* Create the object. */ > - nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name); > + nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name, > + QEMU_DEFAULT_QUEUE_SIZE); > s = DO_UPCAST(NetmapState, nc, nc); > s->me = me; > s->vnet_hdr_len = 0; > diff --git a/net/queue.c b/net/queue.c > index ebbe2bb..d6ddda5 100644 > --- a/net/queue.c > +++ b/net/queue.c > @@ -58,14 +58,14 @@ struct NetQueue { > unsigned delivering : 1; > }; > > -NetQueue *qemu_new_net_queue(void *opaque) > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size) > { > NetQueue *queue; > > queue = g_new0(NetQueue, 1); > > queue->opaque = opaque; > - queue->nq_maxlen = 10000; > + queue->nq_maxlen = queue_size; > queue->nq_count = 0; > > QTAILQ_INIT(&queue->packets); > diff --git a/net/slirp.c b/net/slirp.c > index 9bbed74..c91e929 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -234,7 +234,8 @@ static int net_slirp_init(NetClientState *peer, const char *model, > } > #endif > > - nc = qemu_new_net_client(&net_slirp_info, peer, model, name); > + nc = qemu_new_net_client(&net_slirp_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > snprintf(nc->info_str, sizeof(nc->info_str), > "net=%s,restrict=%s", inet_ntoa(net), > diff --git a/net/socket.c b/net/socket.c > index c30e03f..d5c718c 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -387,7 +387,8 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer, > } > } > > - nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name); > + nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetSocketState, nc, nc); > > @@ -436,7 +437,8 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer, > NetClientState *nc; > NetSocketState *s; > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); > > @@ -543,7 +545,8 @@ static int net_socket_listen_init(NetClientState *peer, > return -1; > } > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > s = DO_UPCAST(NetSocketState, nc, nc); > s->fd = -1; > s->listen_fd = fd; > diff --git a/net/tap-win32.c b/net/tap-win32.c > index 8aee611..dd6046a 100644 > --- a/net/tap-win32.c > +++ b/net/tap-win32.c > @@ -737,7 +737,8 @@ static int tap_win32_init(NetClientState *peer, const char *model, > return -1; > } > > - nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name); > + nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(TAPState, nc, nc); > > diff --git a/net/tap.c b/net/tap.c > index 968df46..383d3ad 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -346,7 +346,8 @@ static TAPState *net_tap_fd_init(NetClientState *peer, > NetClientState *nc; > TAPState *s; > > - nc = qemu_new_net_client(&net_tap_info, peer, model, name); > + nc = qemu_new_net_client(&net_tap_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(TAPState, nc, nc); > > diff --git a/net/vde.c b/net/vde.c > index 2a619fb..b6d28cf 100644 > --- a/net/vde.c > +++ b/net/vde.c > @@ -95,7 +95,8 @@ static int net_vde_init(NetClientState *peer, const char *model, > return -1; > } > > - nc = qemu_new_net_client(&net_vde_info, peer, model, name); > + nc = qemu_new_net_client(&net_vde_info, peer, model, name, > + QEMU_DEFAULT_QUEUE_SIZE); > > snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d", > sock, vde_datafd(vde)); > diff --git a/net/vhost-user.c b/net/vhost-user.c > index 1d86a2b..71f8758 100644 > --- a/net/vhost-user.c > +++ b/net/vhost-user.c > @@ -137,7 +137,9 @@ static int net_vhost_user_init(NetClientState *peer, const char *device, > NetClientState *nc; > VhostUserState *s; > > - nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name); > + /* We don't provide a valid queue: no receive callback */ > + nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name, > + QEMU_EMPTY_QUEUE); > > snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to %s", > chr->label); Two questions: - vhost-user set received_disabled to true to prevent this from happening. but looks like qemu_flush_or_purge_queue_packets() change it to false unconditionally. And I don't quite understand this, probably we can just remove this and the issue is fixed? - If not, maybe you just need another flag like receive_disabled. This seems can save a lot of codes. Thanks
Hi, >- vhost-user set received_disabled to true to prevent this from >happening. but looks like qemu_flush_or_purge_queue_packets() change it >to false unconditionally. And I don't quite understand this, probably we >can just remove this and the issue is fixed? I am afraid that solution can cause issues with other netdev backends. If for any reason a netdev backend is no more able to treat packets it is unvalidated by setting the received_disabled to true. This boolean is reset to false only by the qemu_flush_or_purge_queue_packets function. This way allows to restart communication with a netdev backend that will be temporary done. I do not know if this case can occur but I do not want to break this mechanism. >- If not, maybe you just need another flag like receive_disabled. This >seems can save a lot of codes. Idea of my fix is to avoid enqueuing packet. This solution has the advantage to provide dynamic configuration of queue size for future use but modified several files. Your suggestion is more vhost user centric, and has the disadvantage to enqueue packet that will be never send, nor free during the flush operation. Memory will be normally freed by the system when qemu stops but it can increase the risk of memory leak. Regards. Thibaut. On Mon, Jun 1, 2015 at 11:17 AM, Jason Wang <jasowang@redhat.com> wrote: > > > On 05/28/2015 04:03 PM, Thibaut Collet wrote: > > For netdev backend with no receive callback, packets must not be > enqueued. When > > VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That > causes a > > qemu segfault due to a call of a NULL pointer function. > > > > Function to create net client is modified to allow backend to specify > the size > > of the queue. > > For netdev backend with no receive callback, the queue size is set to 0 > to > > prevent enqueuing of packets. > > For other netdev backend, a default queue size is provided. This value > (set to > > 10000) is the value previously set to any queue. > > > > Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com> > > --- > > include/net/net.h | 3 ++- > > include/net/queue.h | 5 ++++- > > net/dump.c | 3 ++- > > net/hub.c | 3 ++- > > net/l2tpv3.c | 3 ++- > > net/net.c | 10 ++++++---- > > net/netmap.c | 3 ++- > > net/queue.c | 4 ++-- > > net/slirp.c | 3 ++- > > net/socket.c | 9 ++++++--- > > net/tap-win32.c | 3 ++- > > net/tap.c | 3 ++- > > net/vde.c | 3 ++- > > net/vhost-user.c | 4 +++- > > 14 files changed, 39 insertions(+), 20 deletions(-) > > > > diff --git a/include/net/net.h b/include/net/net.h > > index e66ca03..cb3e2df 100644 > > --- a/include/net/net.h > > +++ b/include/net/net.h > > @@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char *id, > NetClientState **ncs, > > NetClientState *qemu_new_net_client(NetClientInfo *info, > > NetClientState *peer, > > const char *model, > > - const char *name); > > + const char *name, > > + uint32_t queue_size); > > NICState *qemu_new_nic(NetClientInfo *info, > > NICConf *conf, > > const char *model, > > diff --git a/include/net/queue.h b/include/net/queue.h > > index fc02b33..4659c5a 100644 > > --- a/include/net/queue.h > > +++ b/include/net/queue.h > > @@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState *sender, > ssize_t ret); > > #define QEMU_NET_PACKET_FLAG_NONE 0 > > #define QEMU_NET_PACKET_FLAG_RAW (1<<0) > > > > -NetQueue *qemu_new_net_queue(void *opaque); > > +#define QEMU_DEFAULT_QUEUE_SIZE 10000 > > +#define QEMU_EMPTY_QUEUE 0 > > + > > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size); > > > > void qemu_del_net_queue(NetQueue *queue); > > > > diff --git a/net/dump.c b/net/dump.c > > index 9d3a09e..299b09e 100644 > > --- a/net/dump.c > > +++ b/net/dump.c > > @@ -129,7 +129,8 @@ static int net_dump_init(NetClientState *peer, const > char *device, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_dump_info, peer, device, name); > > + nc = qemu_new_net_client(&net_dump_info, peer, device, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), > > "dump to %s (len=%d)", filename, len); > > diff --git a/net/hub.c b/net/hub.c > > index 2b60ab9..a42cac3 100644 > > --- a/net/hub.c > > +++ b/net/hub.c > > @@ -151,7 +151,8 @@ static NetHubPort *net_hub_port_new(NetHub *hub, > const char *name) > > name = default_name; > > } > > > > - nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name); > > + nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > port = DO_UPCAST(NetHubPort, nc, nc); > > port->id = id; > > port->hub = hub; > > diff --git a/net/l2tpv3.c b/net/l2tpv3.c > > index 8c598b0..1d2e4e5 100644 > > --- a/net/l2tpv3.c > > +++ b/net/l2tpv3.c > > @@ -548,7 +548,8 @@ int net_init_l2tpv3(const NetClientOptions *opts, > > struct addrinfo *result = NULL; > > char *srcport, *dstport; > > > > - nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name); > > + nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(NetL2TPV3State, nc, nc); > > > > diff --git a/net/net.c b/net/net.c > > index 7427f6a..bcf69da 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -214,6 +214,7 @@ static void qemu_net_client_setup(NetClientState *nc, > > NetClientState *peer, > > const char *model, > > const char *name, > > + uint32_t queue_size, > > NetClientDestructor *destructor) > > { > > nc->info = info; > > @@ -231,21 +232,22 @@ static void qemu_net_client_setup(NetClientState > *nc, > > } > > QTAILQ_INSERT_TAIL(&net_clients, nc, next); > > > > - nc->incoming_queue = qemu_new_net_queue(nc); > > + nc->incoming_queue = qemu_new_net_queue(nc, queue_size); > > nc->destructor = destructor; > > } > > > > NetClientState *qemu_new_net_client(NetClientInfo *info, > > NetClientState *peer, > > const char *model, > > - const char *name) > > + const char *name, > > + uint32_t queue_size) > > { > > NetClientState *nc; > > > > assert(info->size >= sizeof(NetClientState)); > > > > nc = g_malloc0(info->size); > > - qemu_net_client_setup(nc, info, peer, model, name, > > + qemu_net_client_setup(nc, info, peer, model, name, queue_size, > > qemu_net_client_destructor); > > > > return nc; > > @@ -271,7 +273,7 @@ NICState *qemu_new_nic(NetClientInfo *info, > > > > for (i = 0; i < queues; i++) { > > qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name, > > - NULL); > > + QEMU_DEFAULT_QUEUE_SIZE, NULL); > > nic->ncs[i].queue_index = i; > > } > > > > diff --git a/net/netmap.c b/net/netmap.c > > index 0c1772b..3ddfc8e 100644 > > --- a/net/netmap.c > > +++ b/net/netmap.c > > @@ -461,7 +461,8 @@ int net_init_netmap(const NetClientOptions *opts, > > return -1; > > } > > /* Create the object. */ > > - nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name); > > + nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetmapState, nc, nc); > > s->me = me; > > s->vnet_hdr_len = 0; > > diff --git a/net/queue.c b/net/queue.c > > index ebbe2bb..d6ddda5 100644 > > --- a/net/queue.c > > +++ b/net/queue.c > > @@ -58,14 +58,14 @@ struct NetQueue { > > unsigned delivering : 1; > > }; > > > > -NetQueue *qemu_new_net_queue(void *opaque) > > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size) > > { > > NetQueue *queue; > > > > queue = g_new0(NetQueue, 1); > > > > queue->opaque = opaque; > > - queue->nq_maxlen = 10000; > > + queue->nq_maxlen = queue_size; > > queue->nq_count = 0; > > > > QTAILQ_INIT(&queue->packets); > > diff --git a/net/slirp.c b/net/slirp.c > > index 9bbed74..c91e929 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -234,7 +234,8 @@ static int net_slirp_init(NetClientState *peer, > const char *model, > > } > > #endif > > > > - nc = qemu_new_net_client(&net_slirp_info, peer, model, name); > > + nc = qemu_new_net_client(&net_slirp_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), > > "net=%s,restrict=%s", inet_ntoa(net), > > diff --git a/net/socket.c b/net/socket.c > > index c30e03f..d5c718c 100644 > > --- a/net/socket.c > > +++ b/net/socket.c > > @@ -387,7 +387,8 @@ static NetSocketState > *net_socket_fd_init_dgram(NetClientState *peer, > > } > > } > > > > - nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name); > > + nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(NetSocketState, nc, nc); > > > > @@ -436,7 +437,8 @@ static NetSocketState > *net_socket_fd_init_stream(NetClientState *peer, > > NetClientState *nc; > > NetSocketState *s; > > > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); > > > > @@ -543,7 +545,8 @@ static int net_socket_listen_init(NetClientState > *peer, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetSocketState, nc, nc); > > s->fd = -1; > > s->listen_fd = fd; > > diff --git a/net/tap-win32.c b/net/tap-win32.c > > index 8aee611..dd6046a 100644 > > --- a/net/tap-win32.c > > +++ b/net/tap-win32.c > > @@ -737,7 +737,8 @@ static int tap_win32_init(NetClientState *peer, > const char *model, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name); > > + nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(TAPState, nc, nc); > > > > diff --git a/net/tap.c b/net/tap.c > > index 968df46..383d3ad 100644 > > --- a/net/tap.c > > +++ b/net/tap.c > > @@ -346,7 +346,8 @@ static TAPState *net_tap_fd_init(NetClientState > *peer, > > NetClientState *nc; > > TAPState *s; > > > > - nc = qemu_new_net_client(&net_tap_info, peer, model, name); > > + nc = qemu_new_net_client(&net_tap_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(TAPState, nc, nc); > > > > diff --git a/net/vde.c b/net/vde.c > > index 2a619fb..b6d28cf 100644 > > --- a/net/vde.c > > +++ b/net/vde.c > > @@ -95,7 +95,8 @@ static int net_vde_init(NetClientState *peer, const > char *model, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_vde_info, peer, model, name); > > + nc = qemu_new_net_client(&net_vde_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d", > > sock, vde_datafd(vde)); > > diff --git a/net/vhost-user.c b/net/vhost-user.c > > index 1d86a2b..71f8758 100644 > > --- a/net/vhost-user.c > > +++ b/net/vhost-user.c > > @@ -137,7 +137,9 @@ static int net_vhost_user_init(NetClientState *peer, > const char *device, > > NetClientState *nc; > > VhostUserState *s; > > > > - nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name); > > + /* We don't provide a valid queue: no receive callback */ > > + nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name, > > + QEMU_EMPTY_QUEUE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to %s", > > chr->label); > > Two questions: > > - vhost-user set received_disabled to true to prevent this from > happening. but looks like qemu_flush_or_purge_queue_packets() change it > to false unconditionally. And I don't quite understand this, probably we > can just remove this and the issue is fixed? > - If not, maybe you just need another flag like receive_disabled. This > seems can save a lot of codes. > > Thanks >
On Fri, May 29, 2015 at 04:28:48PM +0200, Thibaut Collet wrote: > I agree that virtio-net NIC never enqueues packet to vhost-user > but qemu_announce_self function (savevm.c file) can do it through > the qemu_announce_self_iter / qemu_send_packet_raw sequence. Does vhost-user support live migration? If no, then qemu_announce_self is never called. If yes, then the packet should not be discarded since the ARP announce has a purpose. Stefan
Hi, thanks for your point, I did not notice the problem of the lost of ARP announce by discarding the message. So I must rewrite my patch to define a queue to transmit the ARP announce to the guest. Is it the proper solution? Thanks. Best regards. Thibaut. On Tue, Jun 2, 2015 at 12:34 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > On Fri, May 29, 2015 at 04:28:48PM +0200, Thibaut Collet wrote: > > I agree that virtio-net NIC never enqueues packet to vhost-user > > but qemu_announce_self function (savevm.c file) can do it through > > the qemu_announce_self_iter / qemu_send_packet_raw sequence. > > Does vhost-user support live migration? > > If no, then qemu_announce_self is never called. > > If yes, then the packet should not be discarded since the ARP announce > has a purpose. > > Stefan >
On 06/01/2015 06:14 PM, Thibaut Collet wrote: > Hi, > > >- vhost-user set received_disabled to true to prevent this from > >happening. but looks like qemu_flush_or_purge_queue_packets() change it > >to false unconditionally. And I don't quite understand this, probably we > >can just remove this and the issue is fixed? > > I am afraid that solution can cause issues with other netdev backends. > If for any reason a netdev backend is no more able to treat packets it > is unvalidated by setting the received_disabled to true. This boolean > is reset to false only by the qemu_flush_or_purge_queue_packets > function. This way allows to restart communication with a netdev > backend that will be temporary done. I do not know if this case can > occur but I do not want to break this mechanism. Then we can only set receive_disabled to zero when doing flush. And keep the value when purge. > > >- If not, maybe you just need another flag like receive_disabled. This > >seems can save a lot of codes. > > Idea of my fix is to avoid enqueuing packet. This solution has the > advantage to provide dynamic configuration of queue size for future > use but modified several files. > > Your suggestion is more vhost user centric, and has the disadvantage > to enqueue packet that will be never send, nor free during the flush > operation. Memory will be normally freed by the system when qemu stops > but it can increase the risk of memory leak. The issue is this only works when no sent_cb (though it was not an issue for vhost-user since it does not even use a sent_cb). My suggestion is to just check this flag and drop the packet if it was true in qemu_net_queue_append{_iov}(). But I'm ok to your method also. > > Regards. > > Thibaut. > > On Mon, Jun 1, 2015 at 11:17 AM, Jason Wang <jasowang@redhat.com > <mailto:jasowang@redhat.com>> wrote: > > > > On 05/28/2015 04:03 PM, Thibaut Collet wrote: > > For netdev backend with no receive callback, packets must not be > enqueued. When > > VM stops the enqueued packets are purged (see fixes > ca77d85e1dbf). That causes a > > qemu segfault due to a call of a NULL pointer function. > > > > Function to create net client is modified to allow backend to > specify the size > > of the queue. > > For netdev backend with no receive callback, the queue size is > set to 0 to > > prevent enqueuing of packets. > > For other netdev backend, a default queue size is provided. This > value (set to > > 10000) is the value previously set to any queue. > > > > Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com > <mailto:thibaut.collet@6wind.com>> > > --- > > include/net/net.h | 3 ++- > > include/net/queue.h | 5 ++++- > > net/dump.c | 3 ++- > > net/hub.c | 3 ++- > > net/l2tpv3.c | 3 ++- > > net/net.c | 10 ++++++---- > > net/netmap.c | 3 ++- > > net/queue.c | 4 ++-- > > net/slirp.c | 3 ++- > > net/socket.c | 9 ++++++--- > > net/tap-win32.c | 3 ++- > > net/tap.c | 3 ++- > > net/vde.c | 3 ++- > > net/vhost-user.c | 4 +++- > > 14 files changed, 39 insertions(+), 20 deletions(-) > > > > diff --git a/include/net/net.h b/include/net/net.h > > index e66ca03..cb3e2df 100644 > > --- a/include/net/net.h > > +++ b/include/net/net.h > > @@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char > *id, NetClientState **ncs, > > NetClientState *qemu_new_net_client(NetClientInfo *info, > > NetClientState *peer, > > const char *model, > > - const char *name); > > + const char *name, > > + uint32_t queue_size); > > NICState *qemu_new_nic(NetClientInfo *info, > > NICConf *conf, > > const char *model, > > diff --git a/include/net/queue.h b/include/net/queue.h > > index fc02b33..4659c5a 100644 > > --- a/include/net/queue.h > > +++ b/include/net/queue.h > > @@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState > *sender, ssize_t ret); > > #define QEMU_NET_PACKET_FLAG_NONE 0 > > #define QEMU_NET_PACKET_FLAG_RAW (1<<0) > > > > -NetQueue *qemu_new_net_queue(void *opaque); > > +#define QEMU_DEFAULT_QUEUE_SIZE 10000 > > +#define QEMU_EMPTY_QUEUE 0 > > + > > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size); > > > > void qemu_del_net_queue(NetQueue *queue); > > > > diff --git a/net/dump.c b/net/dump.c > > index 9d3a09e..299b09e 100644 > > --- a/net/dump.c > > +++ b/net/dump.c > > @@ -129,7 +129,8 @@ static int net_dump_init(NetClientState > *peer, const char *device, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_dump_info, peer, device, name); > > + nc = qemu_new_net_client(&net_dump_info, peer, device, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), > > "dump to %s (len=%d)", filename, len); > > diff --git a/net/hub.c b/net/hub.c > > index 2b60ab9..a42cac3 100644 > > --- a/net/hub.c > > +++ b/net/hub.c > > @@ -151,7 +151,8 @@ static NetHubPort *net_hub_port_new(NetHub > *hub, const char *name) > > name = default_name; > > } > > > > - nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", > name); > > + nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > port = DO_UPCAST(NetHubPort, nc, nc); > > port->id = id; > > port->hub = hub; > > diff --git a/net/l2tpv3.c b/net/l2tpv3.c > > index 8c598b0..1d2e4e5 100644 > > --- a/net/l2tpv3.c > > +++ b/net/l2tpv3.c > > @@ -548,7 +548,8 @@ int net_init_l2tpv3(const NetClientOptions > *opts, > > struct addrinfo *result = NULL; > > char *srcport, *dstport; > > > > - nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", > name); > > + nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", > name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(NetL2TPV3State, nc, nc); > > > > diff --git a/net/net.c b/net/net.c > > index 7427f6a..bcf69da 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -214,6 +214,7 @@ static void > qemu_net_client_setup(NetClientState *nc, > > NetClientState *peer, > > const char *model, > > const char *name, > > + uint32_t queue_size, > > NetClientDestructor *destructor) > > { > > nc->info = info; > > @@ -231,21 +232,22 @@ static void > qemu_net_client_setup(NetClientState *nc, > > } > > QTAILQ_INSERT_TAIL(&net_clients, nc, next); > > > > - nc->incoming_queue = qemu_new_net_queue(nc); > > + nc->incoming_queue = qemu_new_net_queue(nc, queue_size); > > nc->destructor = destructor; > > } > > > > NetClientState *qemu_new_net_client(NetClientInfo *info, > > NetClientState *peer, > > const char *model, > > - const char *name) > > + const char *name, > > + uint32_t queue_size) > > { > > NetClientState *nc; > > > > assert(info->size >= sizeof(NetClientState)); > > > > nc = g_malloc0(info->size); > > - qemu_net_client_setup(nc, info, peer, model, name, > > + qemu_net_client_setup(nc, info, peer, model, name, queue_size, > > qemu_net_client_destructor); > > > > return nc; > > @@ -271,7 +273,7 @@ NICState *qemu_new_nic(NetClientInfo *info, > > > > for (i = 0; i < queues; i++) { > > qemu_net_client_setup(&nic->ncs[i], info, peers[i], > model, name, > > - NULL); > > + QEMU_DEFAULT_QUEUE_SIZE, NULL); > > nic->ncs[i].queue_index = i; > > } > > > > diff --git a/net/netmap.c b/net/netmap.c > > index 0c1772b..3ddfc8e 100644 > > --- a/net/netmap.c > > +++ b/net/netmap.c > > @@ -461,7 +461,8 @@ int net_init_netmap(const NetClientOptions > *opts, > > return -1; > > } > > /* Create the object. */ > > - nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", > name); > > + nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", > name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetmapState, nc, nc); > > s->me = me; > > s->vnet_hdr_len = 0; > > diff --git a/net/queue.c b/net/queue.c > > index ebbe2bb..d6ddda5 100644 > > --- a/net/queue.c > > +++ b/net/queue.c > > @@ -58,14 +58,14 @@ struct NetQueue { > > unsigned delivering : 1; > > }; > > > > -NetQueue *qemu_new_net_queue(void *opaque) > > +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size) > > { > > NetQueue *queue; > > > > queue = g_new0(NetQueue, 1); > > > > queue->opaque = opaque; > > - queue->nq_maxlen = 10000; > > + queue->nq_maxlen = queue_size; > > queue->nq_count = 0; > > > > QTAILQ_INIT(&queue->packets); > > diff --git a/net/slirp.c b/net/slirp.c > > index 9bbed74..c91e929 100644 > > --- a/net/slirp.c > > +++ b/net/slirp.c > > @@ -234,7 +234,8 @@ static int net_slirp_init(NetClientState > *peer, const char *model, > > } > > #endif > > > > - nc = qemu_new_net_client(&net_slirp_info, peer, model, name); > > + nc = qemu_new_net_client(&net_slirp_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), > > "net=%s,restrict=%s", inet_ntoa(net), > > diff --git a/net/socket.c b/net/socket.c > > index c30e03f..d5c718c 100644 > > --- a/net/socket.c > > +++ b/net/socket.c > > @@ -387,7 +387,8 @@ static NetSocketState > *net_socket_fd_init_dgram(NetClientState *peer, > > } > > } > > > > - nc = qemu_new_net_client(&net_dgram_socket_info, peer, > model, name); > > + nc = qemu_new_net_client(&net_dgram_socket_info, peer, > model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(NetSocketState, nc, nc); > > > > @@ -436,7 +437,8 @@ static NetSocketState > *net_socket_fd_init_stream(NetClientState *peer, > > NetClientState *nc; > > NetSocketState *s; > > > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "socket: > fd=%d", fd); > > > > @@ -543,7 +545,8 @@ static int > net_socket_listen_init(NetClientState *peer, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_socket_info, peer, model, name); > > + nc = qemu_new_net_client(&net_socket_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > s = DO_UPCAST(NetSocketState, nc, nc); > > s->fd = -1; > > s->listen_fd = fd; > > diff --git a/net/tap-win32.c b/net/tap-win32.c > > index 8aee611..dd6046a 100644 > > --- a/net/tap-win32.c > > +++ b/net/tap-win32.c > > @@ -737,7 +737,8 @@ static int tap_win32_init(NetClientState > *peer, const char *model, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_tap_win32_info, peer, model, > name); > > + nc = qemu_new_net_client(&net_tap_win32_info, peer, model, > name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(TAPState, nc, nc); > > > > diff --git a/net/tap.c b/net/tap.c > > index 968df46..383d3ad 100644 > > --- a/net/tap.c > > +++ b/net/tap.c > > @@ -346,7 +346,8 @@ static TAPState > *net_tap_fd_init(NetClientState *peer, > > NetClientState *nc; > > TAPState *s; > > > > - nc = qemu_new_net_client(&net_tap_info, peer, model, name); > > + nc = qemu_new_net_client(&net_tap_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > s = DO_UPCAST(TAPState, nc, nc); > > > > diff --git a/net/vde.c b/net/vde.c > > index 2a619fb..b6d28cf 100644 > > --- a/net/vde.c > > +++ b/net/vde.c > > @@ -95,7 +95,8 @@ static int net_vde_init(NetClientState *peer, > const char *model, > > return -1; > > } > > > > - nc = qemu_new_net_client(&net_vde_info, peer, model, name); > > + nc = qemu_new_net_client(&net_vde_info, peer, model, name, > > + QEMU_DEFAULT_QUEUE_SIZE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d", > > sock, vde_datafd(vde)); > > diff --git a/net/vhost-user.c b/net/vhost-user.c > > index 1d86a2b..71f8758 100644 > > --- a/net/vhost-user.c > > +++ b/net/vhost-user.c > > @@ -137,7 +137,9 @@ static int > net_vhost_user_init(NetClientState *peer, const char *device, > > NetClientState *nc; > > VhostUserState *s; > > > > - nc = qemu_new_net_client(&net_vhost_user_info, peer, > device, name); > > + /* We don't provide a valid queue: no receive callback */ > > + nc = qemu_new_net_client(&net_vhost_user_info, peer, > device, name, > > + QEMU_EMPTY_QUEUE); > > > > snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to > %s", > > chr->label); > > Two questions: > > - vhost-user set received_disabled to true to prevent this from > happening. but looks like qemu_flush_or_purge_queue_packets() > change it > to false unconditionally. And I don't quite understand this, > probably we > can just remove this and the issue is fixed? > - If not, maybe you just need another flag like receive_disabled. This > seems can save a lot of codes. > > Thanks > >
On 06/03/2015 03:56 PM, Thibaut Collet wrote: > Hi, > > thanks for your point, I did not notice the problem of the lost of ARP > announce by discarding the message. > So I must rewrite my patch to define a queue to transmit the ARP > announce to the guest. Is it the proper solution? > Please have a look at VIRTIO_NET_F_GUEST_ANNOUNCE. It can notify guest after migration to let it to send arp announce. Recent linux driver has this support. > Thanks. > > Best regards. > > Thibaut. > > On Tue, Jun 2, 2015 at 12:34 PM, Stefan Hajnoczi <stefanha@redhat.com > <mailto:stefanha@redhat.com>> wrote: > > On Fri, May 29, 2015 at 04:28:48PM +0200, Thibaut Collet wrote: > > I agree that virtio-net NIC never enqueues packet to vhost-user > > but qemu_announce_self function (savevm.c file) can do it through > > the qemu_announce_self_iter / qemu_send_packet_raw sequence. > > Does vhost-user support live migration? > > If no, then qemu_announce_self is never called. > > If yes, then the packet should not be discarded since the ARP announce > has a purpose. > > Stefan > >
On Wed, Jun 03, 2015 at 09:56:57AM +0200, Thibaut Collet wrote: > thanks for your point, I did not notice the problem of the lost of ARP > announce by discarding the message. > So I must rewrite my patch to define a queue to transmit the ARP announce > to the guest. Is it the proper solution? I'm not sure if vhost-user is designed to support live migration in the first place. It's only worth adding ARP packet injection support if live migration is supported with vhost-user. You don't need to define a queue, instead implement the .receive() function for vhost-user and invoke a new virtio-net-specific vhost-user request type that injects a packet (independent of the tx virtqueue). Stefan
On Wed, Jun 3, 2015 at 10:42 AM, Jason Wang <jasowang@redhat.com> wrote: > > > On 06/03/2015 03:56 PM, Thibaut Collet wrote: >> Hi, >> >> thanks for your point, I did not notice the problem of the lost of ARP >> announce by discarding the message. >> So I must rewrite my patch to define a queue to transmit the ARP >> announce to the guest. Is it the proper solution? >> > > Please have a look at VIRTIO_NET_F_GUEST_ANNOUNCE. It can notify guest > after migration to let it to send arp announce. Recent linux driver has > this support. This looks like a useful feature. I couldn't see a place in the code where qemu_announce_self() skips virtio-net NICs that have this feature bit enabled. Does this mean these NICs will send announces twice (once from the guest and once from QEMU)? My hope was that with the virtio-net announce feature the qemu_announce_self() function would skip the NIC. Stefan
On 06/03/2015 09:40 PM, Stefan Hajnoczi wrote: > On Wed, Jun 3, 2015 at 10:42 AM, Jason Wang <jasowang@redhat.com> wrote: >> >> On 06/03/2015 03:56 PM, Thibaut Collet wrote: >>> Hi, >>> >>> thanks for your point, I did not notice the problem of the lost of ARP >>> announce by discarding the message. >>> So I must rewrite my patch to define a queue to transmit the ARP >>> announce to the guest. Is it the proper solution? >>> >> Please have a look at VIRTIO_NET_F_GUEST_ANNOUNCE. It can notify guest >> after migration to let it to send arp announce. Recent linux driver has >> this support. > This looks like a useful feature. > > I couldn't see a place in the code where qemu_announce_self() skips > virtio-net NICs that have this feature bit enabled. Does this mean > these NICs will send announces twice (once from the guest and once > from QEMU)? Yes, for simplicity, the changes were limited to virito-net only. > > My hope was that with the virtio-net announce feature the > qemu_announce_self() function would skip the NIC. > > Stefan Yes and will try to post patch to do this. Thanks
diff --git a/include/net/net.h b/include/net/net.h index e66ca03..cb3e2df 100644 --- a/include/net/net.h +++ b/include/net/net.h @@ -104,7 +104,8 @@ int qemu_find_net_clients_except(const char *id, NetClientState **ncs, NetClientState *qemu_new_net_client(NetClientInfo *info, NetClientState *peer, const char *model, - const char *name); + const char *name, + uint32_t queue_size); NICState *qemu_new_nic(NetClientInfo *info, NICConf *conf, const char *model, diff --git a/include/net/queue.h b/include/net/queue.h index fc02b33..4659c5a 100644 --- a/include/net/queue.h +++ b/include/net/queue.h @@ -34,7 +34,10 @@ typedef void (NetPacketSent) (NetClientState *sender, ssize_t ret); #define QEMU_NET_PACKET_FLAG_NONE 0 #define QEMU_NET_PACKET_FLAG_RAW (1<<0) -NetQueue *qemu_new_net_queue(void *opaque); +#define QEMU_DEFAULT_QUEUE_SIZE 10000 +#define QEMU_EMPTY_QUEUE 0 + +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size); void qemu_del_net_queue(NetQueue *queue); diff --git a/net/dump.c b/net/dump.c index 9d3a09e..299b09e 100644 --- a/net/dump.c +++ b/net/dump.c @@ -129,7 +129,8 @@ static int net_dump_init(NetClientState *peer, const char *device, return -1; } - nc = qemu_new_net_client(&net_dump_info, peer, device, name); + nc = qemu_new_net_client(&net_dump_info, peer, device, name, + QEMU_DEFAULT_QUEUE_SIZE); snprintf(nc->info_str, sizeof(nc->info_str), "dump to %s (len=%d)", filename, len); diff --git a/net/hub.c b/net/hub.c index 2b60ab9..a42cac3 100644 --- a/net/hub.c +++ b/net/hub.c @@ -151,7 +151,8 @@ static NetHubPort *net_hub_port_new(NetHub *hub, const char *name) name = default_name; } - nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name); + nc = qemu_new_net_client(&net_hub_port_info, NULL, "hub", name, + QEMU_DEFAULT_QUEUE_SIZE); port = DO_UPCAST(NetHubPort, nc, nc); port->id = id; port->hub = hub; diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 8c598b0..1d2e4e5 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -548,7 +548,8 @@ int net_init_l2tpv3(const NetClientOptions *opts, struct addrinfo *result = NULL; char *srcport, *dstport; - nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name); + nc = qemu_new_net_client(&net_l2tpv3_info, peer, "l2tpv3", name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(NetL2TPV3State, nc, nc); diff --git a/net/net.c b/net/net.c index 7427f6a..bcf69da 100644 --- a/net/net.c +++ b/net/net.c @@ -214,6 +214,7 @@ static void qemu_net_client_setup(NetClientState *nc, NetClientState *peer, const char *model, const char *name, + uint32_t queue_size, NetClientDestructor *destructor) { nc->info = info; @@ -231,21 +232,22 @@ static void qemu_net_client_setup(NetClientState *nc, } QTAILQ_INSERT_TAIL(&net_clients, nc, next); - nc->incoming_queue = qemu_new_net_queue(nc); + nc->incoming_queue = qemu_new_net_queue(nc, queue_size); nc->destructor = destructor; } NetClientState *qemu_new_net_client(NetClientInfo *info, NetClientState *peer, const char *model, - const char *name) + const char *name, + uint32_t queue_size) { NetClientState *nc; assert(info->size >= sizeof(NetClientState)); nc = g_malloc0(info->size); - qemu_net_client_setup(nc, info, peer, model, name, + qemu_net_client_setup(nc, info, peer, model, name, queue_size, qemu_net_client_destructor); return nc; @@ -271,7 +273,7 @@ NICState *qemu_new_nic(NetClientInfo *info, for (i = 0; i < queues; i++) { qemu_net_client_setup(&nic->ncs[i], info, peers[i], model, name, - NULL); + QEMU_DEFAULT_QUEUE_SIZE, NULL); nic->ncs[i].queue_index = i; } diff --git a/net/netmap.c b/net/netmap.c index 0c1772b..3ddfc8e 100644 --- a/net/netmap.c +++ b/net/netmap.c @@ -461,7 +461,8 @@ int net_init_netmap(const NetClientOptions *opts, return -1; } /* Create the object. */ - nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name); + nc = qemu_new_net_client(&net_netmap_info, peer, "netmap", name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(NetmapState, nc, nc); s->me = me; s->vnet_hdr_len = 0; diff --git a/net/queue.c b/net/queue.c index ebbe2bb..d6ddda5 100644 --- a/net/queue.c +++ b/net/queue.c @@ -58,14 +58,14 @@ struct NetQueue { unsigned delivering : 1; }; -NetQueue *qemu_new_net_queue(void *opaque) +NetQueue *qemu_new_net_queue(void *opaque, uint32_t queue_size) { NetQueue *queue; queue = g_new0(NetQueue, 1); queue->opaque = opaque; - queue->nq_maxlen = 10000; + queue->nq_maxlen = queue_size; queue->nq_count = 0; QTAILQ_INIT(&queue->packets); diff --git a/net/slirp.c b/net/slirp.c index 9bbed74..c91e929 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -234,7 +234,8 @@ static int net_slirp_init(NetClientState *peer, const char *model, } #endif - nc = qemu_new_net_client(&net_slirp_info, peer, model, name); + nc = qemu_new_net_client(&net_slirp_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); snprintf(nc->info_str, sizeof(nc->info_str), "net=%s,restrict=%s", inet_ntoa(net), diff --git a/net/socket.c b/net/socket.c index c30e03f..d5c718c 100644 --- a/net/socket.c +++ b/net/socket.c @@ -387,7 +387,8 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer, } } - nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name); + nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(NetSocketState, nc, nc); @@ -436,7 +437,8 @@ static NetSocketState *net_socket_fd_init_stream(NetClientState *peer, NetClientState *nc; NetSocketState *s; - nc = qemu_new_net_client(&net_socket_info, peer, model, name); + nc = qemu_new_net_client(&net_socket_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); @@ -543,7 +545,8 @@ static int net_socket_listen_init(NetClientState *peer, return -1; } - nc = qemu_new_net_client(&net_socket_info, peer, model, name); + nc = qemu_new_net_client(&net_socket_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(NetSocketState, nc, nc); s->fd = -1; s->listen_fd = fd; diff --git a/net/tap-win32.c b/net/tap-win32.c index 8aee611..dd6046a 100644 --- a/net/tap-win32.c +++ b/net/tap-win32.c @@ -737,7 +737,8 @@ static int tap_win32_init(NetClientState *peer, const char *model, return -1; } - nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name); + nc = qemu_new_net_client(&net_tap_win32_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(TAPState, nc, nc); diff --git a/net/tap.c b/net/tap.c index 968df46..383d3ad 100644 --- a/net/tap.c +++ b/net/tap.c @@ -346,7 +346,8 @@ static TAPState *net_tap_fd_init(NetClientState *peer, NetClientState *nc; TAPState *s; - nc = qemu_new_net_client(&net_tap_info, peer, model, name); + nc = qemu_new_net_client(&net_tap_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); s = DO_UPCAST(TAPState, nc, nc); diff --git a/net/vde.c b/net/vde.c index 2a619fb..b6d28cf 100644 --- a/net/vde.c +++ b/net/vde.c @@ -95,7 +95,8 @@ static int net_vde_init(NetClientState *peer, const char *model, return -1; } - nc = qemu_new_net_client(&net_vde_info, peer, model, name); + nc = qemu_new_net_client(&net_vde_info, peer, model, name, + QEMU_DEFAULT_QUEUE_SIZE); snprintf(nc->info_str, sizeof(nc->info_str), "sock=%s,fd=%d", sock, vde_datafd(vde)); diff --git a/net/vhost-user.c b/net/vhost-user.c index 1d86a2b..71f8758 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -137,7 +137,9 @@ static int net_vhost_user_init(NetClientState *peer, const char *device, NetClientState *nc; VhostUserState *s; - nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name); + /* We don't provide a valid queue: no receive callback */ + nc = qemu_new_net_client(&net_vhost_user_info, peer, device, name, + QEMU_EMPTY_QUEUE); snprintf(nc->info_str, sizeof(nc->info_str), "vhost-user to %s", chr->label);
For netdev backend with no receive callback, packets must not be enqueued. When VM stops the enqueued packets are purged (see fixes ca77d85e1dbf). That causes a qemu segfault due to a call of a NULL pointer function. Function to create net client is modified to allow backend to specify the size of the queue. For netdev backend with no receive callback, the queue size is set to 0 to prevent enqueuing of packets. For other netdev backend, a default queue size is provided. This value (set to 10000) is the value previously set to any queue. Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com> --- include/net/net.h | 3 ++- include/net/queue.h | 5 ++++- net/dump.c | 3 ++- net/hub.c | 3 ++- net/l2tpv3.c | 3 ++- net/net.c | 10 ++++++---- net/netmap.c | 3 ++- net/queue.c | 4 ++-- net/slirp.c | 3 ++- net/socket.c | 9 ++++++--- net/tap-win32.c | 3 ++- net/tap.c | 3 ++- net/vde.c | 3 ++- net/vhost-user.c | 4 +++- 14 files changed, 39 insertions(+), 20 deletions(-)