| Message ID | 1329556789-24944-1-git-send-email-zwu.kernel@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 02/18/2012 03:19 AM, zwu.kernel@gmail.com wrote: > From: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> > > The -net socket,listen option does not work with the newer -netdev > syntax: > http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html > > This patch makes it work now. > > Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> > --- > net.c | 26 +++++++++++++++++++++ > net.h | 2 + > net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- > 3 files changed, 84 insertions(+), 16 deletions(-) > > diff --git a/net.c b/net.c > index c34474f..60e7b35 100644 > --- a/net.c > +++ b/net.c > @@ -190,6 +190,32 @@ static ssize_t qemu_deliver_packet_iov(VLANClientState *sender, > int iovcnt, > void *opaque); > > +VLANClientState *qemu_lookup_net_client(VLANState *vlan, > + const char *name) > +{ > + VLANClientState *vc = NULL; > + > + if (vlan) { > + QTAILQ_FOREACH(vc,&vlan->clients, next) { > + if (!strcmp(vc->name, name)) { > + break; > + } > + } > + } else { > + QTAILQ_FOREACH(vc,&non_vlan_clients, next) { > + if (!strcmp(vc->name, name)) { > + break; > + } > + } > + } > + > + if (!vc) { > + return NULL; > + } > + > + return vc; > +} > + > VLANClientState *qemu_new_net_client(NetClientInfo *info, > VLANState *vlan, > VLANClientState *peer, > diff --git a/net.h b/net.h > index 75a8c15..7f73160 100644 > --- a/net.h > +++ b/net.h > @@ -90,6 +90,8 @@ struct VLANState { > > VLANState *qemu_find_vlan(int id, int allocate); > VLANClientState *qemu_find_netdev(const char *id); > +VLANClientState *qemu_lookup_net_client(VLANState *vlan, > + const char *name); > VLANClientState *qemu_new_net_client(NetClientInfo *info, > VLANState *vlan, > VLANClientState *peer, > diff --git a/net/socket.c b/net/socket.c > index d4c2002..3ecee59 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -43,6 +43,7 @@ typedef struct NetSocketState { > } NetSocketState; > > typedef struct NetSocketListenState { > + VLANClientState *nc; > VLANState *vlan; > char *model; > char *name; > @@ -247,7 +248,8 @@ static NetClientInfo net_dgram_socket_info = { > static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, > const char *model, > const char *name, > - int fd, int is_connected) > + int fd, int is_connected, > + int is_listen) > { > struct sockaddr_in saddr; > int newfd; > @@ -286,15 +288,28 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, > } > } > > - nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, name); > + > + if (!is_listen || (is_listen&& !is_connected)) { > + nc = qemu_new_net_client(&net_dgram_socket_info, > + vlan, NULL, model, name); > + } else { > + nc = qemu_lookup_net_client(vlan, name); > + if (!nc) { > + goto err; > + } > + } > + > + s = DO_UPCAST(NetSocketState, nc, nc); > + > + if (is_listen&& !is_connected) { > + return s; > + } > > snprintf(nc->info_str, sizeof(nc->info_str), > "socket: fd=%d (%s mcast=%s:%d)", > fd, is_connected ? "cloned" : "", > inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); > > - s = DO_UPCAST(NetSocketState, nc, nc); > - > s->fd = fd; > > qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s); > @@ -325,16 +340,29 @@ static NetClientInfo net_socket_info = { > static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, > const char *model, > const char *name, > - int fd, int is_connected) > + int fd, int is_connected, > + int is_listen) > { > VLANClientState *nc; > NetSocketState *s; > > - nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); > + if (!is_listen || (is_listen&& !is_connected)) { > + nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); > + } else { > + nc = qemu_lookup_net_client(vlan, name); > + if (!nc) { > + return NULL; > + } > + } > + > + s = DO_UPCAST(NetSocketState, nc, nc); > + > + if (is_listen&& !is_connected) { > + return s; > + } > > snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); > > - s = DO_UPCAST(NetSocketState, nc, nc); > > s->fd = fd; > > @@ -348,7 +376,8 @@ static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, > > static NetSocketState *net_socket_fd_init(VLANState *vlan, > const char *model, const char *name, > - int fd, int is_connected) > + int fd, int is_connected, > + int is_listen) > { > int so_type = -1, optlen=sizeof(so_type); > > @@ -361,13 +390,16 @@ static NetSocketState *net_socket_fd_init(VLANState *vlan, > } > switch(so_type) { > case SOCK_DGRAM: > - return net_socket_fd_init_dgram(vlan, model, name, fd, is_connected); > + return net_socket_fd_init_dgram(vlan, model, > + name, fd, is_connected, is_listen); > case SOCK_STREAM: > - return net_socket_fd_init_stream(vlan, model, name, fd, is_connected); > + return net_socket_fd_init_stream(vlan, model, > + name, fd, is_connected, is_listen); > default: > /* who knows ... this could be a eg. a pty, do warn and continue as stream */ > fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not SOCK_DGRAM or SOCK_STREAM\n", so_type, fd); > - return net_socket_fd_init_stream(vlan, model, name, fd, is_connected); > + return net_socket_fd_init_stream(vlan, model, > + name, fd, is_connected, is_listen); > } > return NULL; > } > @@ -389,14 +421,17 @@ static void net_socket_accept(void *opaque) > break; > } > } > - s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1); > + > + s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1, 1); > if (s1) { > snprintf(s1->nc.info_str, sizeof(s1->nc.info_str), > "socket: connection from %s:%d", > inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); > + s1->nc.link_down = false; I find this to be a little odd. Nothing else uses link_down link this. Why are you setting this flag? Regards, Anthony Liguori
On Fri, Feb 24, 2012 at 11:05 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: > On 02/18/2012 03:19 AM, zwu.kernel@gmail.com wrote: >> >> From: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> >> >> The -net socket,listen option does not work with the newer -netdev >> syntax: >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> This patch makes it work now. >> >> Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> >> --- >> net.c | 26 +++++++++++++++++++++ >> net.h | 2 + >> net/socket.c | 72 >> +++++++++++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 84 insertions(+), 16 deletions(-) >> >> diff --git a/net.c b/net.c >> index c34474f..60e7b35 100644 >> --- a/net.c >> +++ b/net.c >> @@ -190,6 +190,32 @@ static ssize_t >> qemu_deliver_packet_iov(VLANClientState *sender, >> int iovcnt, >> void *opaque); >> >> +VLANClientState *qemu_lookup_net_client(VLANState *vlan, >> + const char *name) >> +{ >> + VLANClientState *vc = NULL; >> + >> + if (vlan) { >> + QTAILQ_FOREACH(vc,&vlan->clients, next) { >> >> + if (!strcmp(vc->name, name)) { >> + break; >> + } >> + } >> + } else { >> + QTAILQ_FOREACH(vc,&non_vlan_clients, next) { >> >> + if (!strcmp(vc->name, name)) { >> + break; >> + } >> + } >> + } >> + >> + if (!vc) { >> + return NULL; >> + } >> + >> + return vc; >> +} >> + >> VLANClientState *qemu_new_net_client(NetClientInfo *info, >> VLANState *vlan, >> VLANClientState *peer, >> diff --git a/net.h b/net.h >> index 75a8c15..7f73160 100644 >> --- a/net.h >> +++ b/net.h >> @@ -90,6 +90,8 @@ struct VLANState { >> >> VLANState *qemu_find_vlan(int id, int allocate); >> VLANClientState *qemu_find_netdev(const char *id); >> +VLANClientState *qemu_lookup_net_client(VLANState *vlan, >> + const char *name); >> VLANClientState *qemu_new_net_client(NetClientInfo *info, >> VLANState *vlan, >> VLANClientState *peer, >> diff --git a/net/socket.c b/net/socket.c >> index d4c2002..3ecee59 100644 >> --- a/net/socket.c >> +++ b/net/socket.c >> @@ -43,6 +43,7 @@ typedef struct NetSocketState { >> } NetSocketState; >> >> typedef struct NetSocketListenState { >> + VLANClientState *nc; >> VLANState *vlan; >> char *model; >> char *name; >> @@ -247,7 +248,8 @@ static NetClientInfo net_dgram_socket_info = { >> static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, >> const char *model, >> const char *name, >> - int fd, int is_connected) >> + int fd, int is_connected, >> + int is_listen) >> { >> struct sockaddr_in saddr; >> int newfd; >> @@ -286,15 +288,28 @@ static NetSocketState >> *net_socket_fd_init_dgram(VLANState *vlan, >> } >> } >> >> - nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, >> name); >> + >> + if (!is_listen || (is_listen&& !is_connected)) { >> >> + nc = qemu_new_net_client(&net_dgram_socket_info, >> + vlan, NULL, model, name); >> + } else { >> + nc = qemu_lookup_net_client(vlan, name); >> + if (!nc) { >> + goto err; >> + } >> + } >> + >> + s = DO_UPCAST(NetSocketState, nc, nc); >> + >> + if (is_listen&& !is_connected) { >> >> + return s; >> + } >> >> snprintf(nc->info_str, sizeof(nc->info_str), >> "socket: fd=%d (%s mcast=%s:%d)", >> fd, is_connected ? "cloned" : "", >> inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); >> >> - s = DO_UPCAST(NetSocketState, nc, nc); >> - >> s->fd = fd; >> >> qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s); >> @@ -325,16 +340,29 @@ static NetClientInfo net_socket_info = { >> static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, >> const char *model, >> const char *name, >> - int fd, int >> is_connected) >> + int fd, int >> is_connected, >> + int is_listen) >> { >> VLANClientState *nc; >> NetSocketState *s; >> >> - nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); >> + if (!is_listen || (is_listen&& !is_connected)) { >> >> + nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, >> name); >> + } else { >> + nc = qemu_lookup_net_client(vlan, name); >> + if (!nc) { >> + return NULL; >> + } >> + } >> + >> + s = DO_UPCAST(NetSocketState, nc, nc); >> + >> + if (is_listen&& !is_connected) { >> >> + return s; >> + } >> >> snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); >> >> - s = DO_UPCAST(NetSocketState, nc, nc); >> >> s->fd = fd; >> >> @@ -348,7 +376,8 @@ static NetSocketState >> *net_socket_fd_init_stream(VLANState *vlan, >> >> static NetSocketState *net_socket_fd_init(VLANState *vlan, >> const char *model, const char >> *name, >> - int fd, int is_connected) >> + int fd, int is_connected, >> + int is_listen) >> { >> int so_type = -1, optlen=sizeof(so_type); >> >> @@ -361,13 +390,16 @@ static NetSocketState *net_socket_fd_init(VLANState >> *vlan, >> } >> switch(so_type) { >> case SOCK_DGRAM: >> - return net_socket_fd_init_dgram(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_dgram(vlan, model, >> + name, fd, is_connected, >> is_listen); >> case SOCK_STREAM: >> - return net_socket_fd_init_stream(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_stream(vlan, model, >> + name, fd, is_connected, >> is_listen); >> default: >> /* who knows ... this could be a eg. a pty, do warn and continue >> as stream */ >> fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not >> SOCK_DGRAM or SOCK_STREAM\n", so_type, fd); >> - return net_socket_fd_init_stream(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_stream(vlan, model, >> + name, fd, is_connected, >> is_listen); >> } >> return NULL; >> } >> @@ -389,14 +421,17 @@ static void net_socket_accept(void *opaque) >> break; >> } >> } >> - s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1); >> + >> + s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1, 1); >> if (s1) { >> snprintf(s1->nc.info_str, sizeof(s1->nc.info_str), >> "socket: connection from %s:%d", >> inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); >> + s1->nc.link_down = false; > > > I find this to be a little odd. Nothing else uses link_down link this. Why > are you setting this flag? s1->nc.link_down = false this mean that the link are re-up. In current upstream, the option -netdev socket,listen can not work. The reason is that when network card is created, it will look for its peer net client. Because socket is still in listen state, its net client object will not be created until this socket accept the client's request. This will cause that network card fails to look for its peer net client. This is the root reason for the option -netdev socket,listen can not work. In this patch, the net client object for socket is created in advance when socket is in listen state. This void that network card fail to look for its peer net client object. As you've known, at the moment, the client request has not arrived, and the socket is still in listen state, so the link state is set to down state to reject packet trasmission. When the client request arrives, socket accept its request, and the link state is re-set to up state, data packets can be trasmitted now. > > Regards, > > Anthony Liguori >
On Fri, Feb 24, 2012 at 11:05 PM, Anthony Liguori <aliguori@us.ibm.com> wrote: > On 02/18/2012 03:19 AM, zwu.kernel@gmail.com wrote: >> >> From: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> >> >> The -net socket,listen option does not work with the newer -netdev >> syntax: >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> This patch makes it work now. >> >> Signed-off-by: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com> >> --- >> net.c | 26 +++++++++++++++++++++ >> net.h | 2 + >> net/socket.c | 72 >> +++++++++++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 84 insertions(+), 16 deletions(-) >> >> diff --git a/net.c b/net.c >> index c34474f..60e7b35 100644 >> --- a/net.c >> +++ b/net.c >> @@ -190,6 +190,32 @@ static ssize_t >> qemu_deliver_packet_iov(VLANClientState *sender, >> int iovcnt, >> void *opaque); >> >> +VLANClientState *qemu_lookup_net_client(VLANState *vlan, >> + const char *name) >> +{ >> + VLANClientState *vc = NULL; >> + >> + if (vlan) { >> + QTAILQ_FOREACH(vc,&vlan->clients, next) { >> >> + if (!strcmp(vc->name, name)) { >> + break; >> + } >> + } >> + } else { >> + QTAILQ_FOREACH(vc,&non_vlan_clients, next) { >> >> + if (!strcmp(vc->name, name)) { >> + break; >> + } >> + } >> + } >> + >> + if (!vc) { >> + return NULL; >> + } >> + >> + return vc; >> +} >> + >> VLANClientState *qemu_new_net_client(NetClientInfo *info, >> VLANState *vlan, >> VLANClientState *peer, >> diff --git a/net.h b/net.h >> index 75a8c15..7f73160 100644 >> --- a/net.h >> +++ b/net.h >> @@ -90,6 +90,8 @@ struct VLANState { >> >> VLANState *qemu_find_vlan(int id, int allocate); >> VLANClientState *qemu_find_netdev(const char *id); >> +VLANClientState *qemu_lookup_net_client(VLANState *vlan, >> + const char *name); >> VLANClientState *qemu_new_net_client(NetClientInfo *info, >> VLANState *vlan, >> VLANClientState *peer, >> diff --git a/net/socket.c b/net/socket.c >> index d4c2002..3ecee59 100644 >> --- a/net/socket.c >> +++ b/net/socket.c >> @@ -43,6 +43,7 @@ typedef struct NetSocketState { >> } NetSocketState; >> >> typedef struct NetSocketListenState { >> + VLANClientState *nc; >> VLANState *vlan; >> char *model; >> char *name; >> @@ -247,7 +248,8 @@ static NetClientInfo net_dgram_socket_info = { >> static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, >> const char *model, >> const char *name, >> - int fd, int is_connected) >> + int fd, int is_connected, >> + int is_listen) >> { >> struct sockaddr_in saddr; >> int newfd; >> @@ -286,15 +288,28 @@ static NetSocketState >> *net_socket_fd_init_dgram(VLANState *vlan, >> } >> } >> >> - nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, >> name); >> + >> + if (!is_listen || (is_listen&& !is_connected)) { >> >> + nc = qemu_new_net_client(&net_dgram_socket_info, >> + vlan, NULL, model, name); >> + } else { >> + nc = qemu_lookup_net_client(vlan, name); >> + if (!nc) { >> + goto err; >> + } >> + } >> + >> + s = DO_UPCAST(NetSocketState, nc, nc); >> + >> + if (is_listen&& !is_connected) { >> >> + return s; >> + } >> >> snprintf(nc->info_str, sizeof(nc->info_str), >> "socket: fd=%d (%s mcast=%s:%d)", >> fd, is_connected ? "cloned" : "", >> inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); >> >> - s = DO_UPCAST(NetSocketState, nc, nc); >> - >> s->fd = fd; >> >> qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s); >> @@ -325,16 +340,29 @@ static NetClientInfo net_socket_info = { >> static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, >> const char *model, >> const char *name, >> - int fd, int >> is_connected) >> + int fd, int >> is_connected, >> + int is_listen) >> { >> VLANClientState *nc; >> NetSocketState *s; >> >> - nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); >> + if (!is_listen || (is_listen&& !is_connected)) { >> >> + nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, >> name); >> + } else { >> + nc = qemu_lookup_net_client(vlan, name); >> + if (!nc) { >> + return NULL; >> + } >> + } >> + >> + s = DO_UPCAST(NetSocketState, nc, nc); >> + >> + if (is_listen&& !is_connected) { >> >> + return s; >> + } >> >> snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); >> >> - s = DO_UPCAST(NetSocketState, nc, nc); >> >> s->fd = fd; >> >> @@ -348,7 +376,8 @@ static NetSocketState >> *net_socket_fd_init_stream(VLANState *vlan, >> >> static NetSocketState *net_socket_fd_init(VLANState *vlan, >> const char *model, const char >> *name, >> - int fd, int is_connected) >> + int fd, int is_connected, >> + int is_listen) >> { >> int so_type = -1, optlen=sizeof(so_type); >> >> @@ -361,13 +390,16 @@ static NetSocketState *net_socket_fd_init(VLANState >> *vlan, >> } >> switch(so_type) { >> case SOCK_DGRAM: >> - return net_socket_fd_init_dgram(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_dgram(vlan, model, >> + name, fd, is_connected, >> is_listen); >> case SOCK_STREAM: >> - return net_socket_fd_init_stream(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_stream(vlan, model, >> + name, fd, is_connected, >> is_listen); >> default: >> /* who knows ... this could be a eg. a pty, do warn and continue >> as stream */ >> fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not >> SOCK_DGRAM or SOCK_STREAM\n", so_type, fd); >> - return net_socket_fd_init_stream(vlan, model, name, fd, >> is_connected); >> + return net_socket_fd_init_stream(vlan, model, >> + name, fd, is_connected, >> is_listen); >> } >> return NULL; >> } >> @@ -389,14 +421,17 @@ static void net_socket_accept(void *opaque) >> break; >> } >> } >> - s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1); >> + >> + s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1, 1); >> if (s1) { >> snprintf(s1->nc.info_str, sizeof(s1->nc.info_str), >> "socket: connection from %s:%d", >> inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); >> + s1->nc.link_down = false; > > > I find this to be a little odd. Nothing else uses link_down link this. Why For other socket usage, when it creates net client object, this client request has arrived, socket accepts this request, at the moment, this link can trasmit packets, so it doesn't need to be set to down state. > are you setting this flag? > > Regards, > > Anthony Liguori >
On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: > From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > > The -net socket,listen option does not work with the newer -netdev > syntax: > http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html > > This patch makes it work now. > > Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > --- > net.c | 26 +++++++++++++++++++++ > net.h | 2 + > net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- > 3 files changed, 84 insertions(+), 16 deletions(-) I wanted to understand the problem better so I tried out -net socket,listen=. Here is its behavior: 1. A client can connect to QEMU, this creates a new socket VLANClientState on the VLAN. 2. If another client connects to QEMU, another VLANClientState is created. That means many socket clients can be added to the same "VLAN". 3. When a simple TCP client like netcat connects and then disconnects, the VLANClientState remains forever. There seems to be no cleanup. This patch does not handle the -net socket,listen= case where multiple clients connect. Also, the -netdev socket,listen= semantics cannot match -net socket-listen= semantics because there is only one peer at any time. Some options: 1. Do not accept new connections while a client is connected. Once the client disconnects we can accept a new connection. This maintains the 1-1 peer behavior. 2. Integrate with vlan-hub so that multiple clients can connect even with -netdev. Connections will create new NetClientStates and auto-attach to the hub. This mimics -net socket,listen= but requires a hub to be used. 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. I think #1 would be okay, although it no longer allows multiple connections, but I don't have a strong opinion either way. Stefan
On Sun, Feb 26, 2012 at 10:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> The -net socket,listen option does not work with the newer -netdev >> syntax: >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> This patch makes it work now. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> net.c | 26 +++++++++++++++++++++ >> net.h | 2 + >> net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 84 insertions(+), 16 deletions(-) > > I wanted to understand the problem better so I tried out -net > socket,listen=. Here is its behavior: > > 1. A client can connect to QEMU, this creates a new socket > VLANClientState on the VLAN. > 2. If another client connects to QEMU, another VLANClientState is > created. That means many socket clients can be added to the same > "VLAN". > 3. When a simple TCP client like netcat connects and then disconnects, > the VLANClientState remains forever. There seems to be no cleanup. > > This patch does not handle the -net socket,listen= case where multiple > clients connect. good catch, thanks. > > Also, the -netdev socket,listen= semantics cannot match -net > socket-listen= semantics because there is only one peer at any time. > Some options: > > 1. Do not accept new connections while a client is connected. Once > the client disconnects we can accept a new connection. This maintains > the 1-1 peer behavior. > 2. Integrate with vlan-hub so that multiple clients can connect even > with -netdev. Connections will create new NetClientStates and > auto-attach to the hub. This mimics -net socket,listen= but requires > a hub to be used. > 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. > > I think #1 would be okay, although it no longer allows multiple > connections, but I don't have a strong opinion either way. > > Stefan
On Sun, Feb 26, 2012 at 10:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> The -net socket,listen option does not work with the newer -netdev >> syntax: >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> This patch makes it work now. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> net.c | 26 +++++++++++++++++++++ >> net.h | 2 + >> net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 84 insertions(+), 16 deletions(-) > > I wanted to understand the problem better so I tried out -net > socket,listen=. Here is its behavior: > > 1. A client can connect to QEMU, this creates a new socket > VLANClientState on the VLAN. > 2. If another client connects to QEMU, another VLANClientState is > created. That means many socket clients can be added to the same > "VLAN". > 3. When a simple TCP client like netcat connects and then disconnects, > the VLANClientState remains forever. There seems to be no cleanup. > > This patch does not handle the -net socket,listen= case where multiple > clients connect. > > Also, the -netdev socket,listen= semantics cannot match -net > socket-listen= semantics because there is only one peer at any time. > Some options: > > 1. Do not accept new connections while a client is connected. Once > the client disconnects we can accept a new connection. This maintains > the 1-1 peer behavior. > 2. Integrate with vlan-hub so that multiple clients can connect even > with -netdev. Connections will create new NetClientStates and > auto-attach to the hub. This mimics -net socket,listen= but requires > a hub to be used. > 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. > > I think #1 would be okay, although it no longer allows multiple > connections, but I don't have a strong opinion either way. Now i prefer to support #2. Do you think of it? Should the usage "-netdev socket,listen" also been supported or forbidden? As you said, -netdev only has one peer, so their usage will be a bit different. > > Stefan
On Mon, May 28, 2012 at 03:49:11PM +0800, Zhi Yong Wu wrote: > On Sun, Feb 26, 2012 at 10:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > > On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: > >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > >> > >> The -net socket,listen option does not work with the newer -netdev > >> syntax: > >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html > >> > >> This patch makes it work now. > >> > >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> > >> --- > >> net.c | 26 +++++++++++++++++++++ > >> net.h | 2 + > >> net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- > >> 3 files changed, 84 insertions(+), 16 deletions(-) > > > > I wanted to understand the problem better so I tried out -net > > socket,listen=. Here is its behavior: > > > > 1. A client can connect to QEMU, this creates a new socket > > VLANClientState on the VLAN. > > 2. If another client connects to QEMU, another VLANClientState is > > created. That means many socket clients can be added to the same > > "VLAN". > > 3. When a simple TCP client like netcat connects and then disconnects, > > the VLANClientState remains forever. There seems to be no cleanup. > > > > This patch does not handle the -net socket,listen= case where multiple > > clients connect. > > > > Also, the -netdev socket,listen= semantics cannot match -net > > socket-listen= semantics because there is only one peer at any time. > > Some options: > > > > 1. Do not accept new connections while a client is connected. Once > > the client disconnects we can accept a new connection. This maintains > > the 1-1 peer behavior. > > 2. Integrate with vlan-hub so that multiple clients can connect even > > with -netdev. Connections will create new NetClientStates and > > auto-attach to the hub. This mimics -net socket,listen= but requires > > a hub to be used. > > 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. > > > > I think #1 would be okay, although it no longer allows multiple > > connections, but I don't have a strong opinion either way. > Now i prefer to support #2. Do you think of it? Should the usage > "-netdev socket,listen" also been supported or forbidden? As you said, > -netdev only has one peer, so their usage will be a bit different. I'm not sure how useful the multiple connections behavior is. Since -netdev socket,listen= has not worked in the past we have the freedom to decide how it should work (without breaking existing users' setups). Several folks have pointed out that vde or other external programs are better for virtual hubs/switches. I would implement #1 because it adds useful behavior but doesn't complicate QEMU much. But if you feel adding #2 would be worthwhile and not a big effort to code, then go ahead. My intuition is that #1 will be easier to get merged and can be extended to support #2 in the future, if necessary. Stefan
On Mon, May 28, 2012 at 6:51 PM, Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> wrote: > On Mon, May 28, 2012 at 03:49:11PM +0800, Zhi Yong Wu wrote: >> On Sun, Feb 26, 2012 at 10:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: >> > On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: >> >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> >> >> The -net socket,listen option does not work with the newer -netdev >> >> syntax: >> >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> >> >> This patch makes it work now. >> >> >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> --- >> >> net.c | 26 +++++++++++++++++++++ >> >> net.h | 2 + >> >> net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- >> >> 3 files changed, 84 insertions(+), 16 deletions(-) >> > >> > I wanted to understand the problem better so I tried out -net >> > socket,listen=. Here is its behavior: >> > >> > 1. A client can connect to QEMU, this creates a new socket >> > VLANClientState on the VLAN. >> > 2. If another client connects to QEMU, another VLANClientState is >> > created. That means many socket clients can be added to the same >> > "VLAN". >> > 3. When a simple TCP client like netcat connects and then disconnects, >> > the VLANClientState remains forever. There seems to be no cleanup. >> > >> > This patch does not handle the -net socket,listen= case where multiple >> > clients connect. >> > >> > Also, the -netdev socket,listen= semantics cannot match -net >> > socket-listen= semantics because there is only one peer at any time. >> > Some options: >> > >> > 1. Do not accept new connections while a client is connected. Once >> > the client disconnects we can accept a new connection. This maintains >> > the 1-1 peer behavior. >> > 2. Integrate with vlan-hub so that multiple clients can connect even >> > with -netdev. Connections will create new NetClientStates and >> > auto-attach to the hub. This mimics -net socket,listen= but requires >> > a hub to be used. >> > 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. >> > >> > I think #1 would be okay, although it no longer allows multiple >> > connections, but I don't have a strong opinion either way. >> Now i prefer to support #2. Do you think of it? Should the usage >> "-netdev socket,listen" also been supported or forbidden? As you said, >> -netdev only has one peer, so their usage will be a bit different. > > I'm not sure how useful the multiple connections behavior is. Since > -netdev socket,listen= has not worked in the past we have the freedom to > decide how it should work (without breaking existing users' setups). > > Several folks have pointed out that vde or other external programs are > better for virtual hubs/switches. I would implement #1 because it adds > useful behavior but doesn't complicate QEMU much. > > But if you feel adding #2 would be worthwhile and not a big effort to > code, then go ahead. My intuition is that #1 will be easier to get > merged and can be extended to support #2 in the future, if necessary. OK. i will try. > > Stefan >
On Sun, Feb 26, 2012 at 10:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Sat, Feb 18, 2012 at 9:19 AM, <zwu.kernel@gmail.com> wrote: >> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> >> The -net socket,listen option does not work with the newer -netdev >> syntax: >> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01508.html >> >> This patch makes it work now. >> >> Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> >> --- >> net.c | 26 +++++++++++++++++++++ >> net.h | 2 + >> net/socket.c | 72 +++++++++++++++++++++++++++++++++++++++++++++------------- >> 3 files changed, 84 insertions(+), 16 deletions(-) > > I wanted to understand the problem better so I tried out -net > socket,listen=. Here is its behavior: > > 1. A client can connect to QEMU, this creates a new socket > VLANClientState on the VLAN. > 2. If another client connects to QEMU, another VLANClientState is > created. That means many socket clients can be added to the same > "VLAN". > 3. When a simple TCP client like netcat connects and then disconnects, > the VLANClientState remains forever. There seems to be no cleanup. > > This patch does not handle the -net socket,listen= case where multiple > clients connect. > > Also, the -netdev socket,listen= semantics cannot match -net > socket-listen= semantics because there is only one peer at any time. > Some options: > > 1. Do not accept new connections while a client is connected. Once > the client disconnects we can accept a new connection. This maintains How will socket server know that the client disconnected? > the 1-1 peer behavior. > 2. Integrate with vlan-hub so that multiple clients can connect even > with -netdev. Connections will create new NetClientStates and > auto-attach to the hub. This mimics -net socket,listen= but requires > a hub to be used. > 3. Forbid -netdev socket,listen=, only allow -net socket,listen=. > > I think #1 would be okay, although it no longer allows multiple > connections, but I don't have a strong opinion either way. > > Stefan
diff --git a/net.c b/net.c index c34474f..60e7b35 100644 --- a/net.c +++ b/net.c @@ -190,6 +190,32 @@ static ssize_t qemu_deliver_packet_iov(VLANClientState *sender, int iovcnt, void *opaque); +VLANClientState *qemu_lookup_net_client(VLANState *vlan, + const char *name) +{ + VLANClientState *vc = NULL; + + if (vlan) { + QTAILQ_FOREACH(vc, &vlan->clients, next) { + if (!strcmp(vc->name, name)) { + break; + } + } + } else { + QTAILQ_FOREACH(vc, &non_vlan_clients, next) { + if (!strcmp(vc->name, name)) { + break; + } + } + } + + if (!vc) { + return NULL; + } + + return vc; +} + VLANClientState *qemu_new_net_client(NetClientInfo *info, VLANState *vlan, VLANClientState *peer, diff --git a/net.h b/net.h index 75a8c15..7f73160 100644 --- a/net.h +++ b/net.h @@ -90,6 +90,8 @@ struct VLANState { VLANState *qemu_find_vlan(int id, int allocate); VLANClientState *qemu_find_netdev(const char *id); +VLANClientState *qemu_lookup_net_client(VLANState *vlan, + const char *name); VLANClientState *qemu_new_net_client(NetClientInfo *info, VLANState *vlan, VLANClientState *peer, diff --git a/net/socket.c b/net/socket.c index d4c2002..3ecee59 100644 --- a/net/socket.c +++ b/net/socket.c @@ -43,6 +43,7 @@ typedef struct NetSocketState { } NetSocketState; typedef struct NetSocketListenState { + VLANClientState *nc; VLANState *vlan; char *model; char *name; @@ -247,7 +248,8 @@ static NetClientInfo net_dgram_socket_info = { static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, const char *model, const char *name, - int fd, int is_connected) + int fd, int is_connected, + int is_listen) { struct sockaddr_in saddr; int newfd; @@ -286,15 +288,28 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan, } } - nc = qemu_new_net_client(&net_dgram_socket_info, vlan, NULL, model, name); + + if (!is_listen || (is_listen && !is_connected)) { + nc = qemu_new_net_client(&net_dgram_socket_info, + vlan, NULL, model, name); + } else { + nc = qemu_lookup_net_client(vlan, name); + if (!nc) { + goto err; + } + } + + s = DO_UPCAST(NetSocketState, nc, nc); + + if (is_listen && !is_connected) { + return s; + } snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d (%s mcast=%s:%d)", fd, is_connected ? "cloned" : "", inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); - s = DO_UPCAST(NetSocketState, nc, nc); - s->fd = fd; qemu_set_fd_handler(s->fd, net_socket_send_dgram, NULL, s); @@ -325,16 +340,29 @@ static NetClientInfo net_socket_info = { static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, const char *model, const char *name, - int fd, int is_connected) + int fd, int is_connected, + int is_listen) { VLANClientState *nc; NetSocketState *s; - nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); + if (!is_listen || (is_listen && !is_connected)) { + nc = qemu_new_net_client(&net_socket_info, vlan, NULL, model, name); + } else { + nc = qemu_lookup_net_client(vlan, name); + if (!nc) { + return NULL; + } + } + + s = DO_UPCAST(NetSocketState, nc, nc); + + if (is_listen && !is_connected) { + return s; + } snprintf(nc->info_str, sizeof(nc->info_str), "socket: fd=%d", fd); - s = DO_UPCAST(NetSocketState, nc, nc); s->fd = fd; @@ -348,7 +376,8 @@ static NetSocketState *net_socket_fd_init_stream(VLANState *vlan, static NetSocketState *net_socket_fd_init(VLANState *vlan, const char *model, const char *name, - int fd, int is_connected) + int fd, int is_connected, + int is_listen) { int so_type = -1, optlen=sizeof(so_type); @@ -361,13 +390,16 @@ static NetSocketState *net_socket_fd_init(VLANState *vlan, } switch(so_type) { case SOCK_DGRAM: - return net_socket_fd_init_dgram(vlan, model, name, fd, is_connected); + return net_socket_fd_init_dgram(vlan, model, + name, fd, is_connected, is_listen); case SOCK_STREAM: - return net_socket_fd_init_stream(vlan, model, name, fd, is_connected); + return net_socket_fd_init_stream(vlan, model, + name, fd, is_connected, is_listen); default: /* who knows ... this could be a eg. a pty, do warn and continue as stream */ fprintf(stderr, "qemu: warning: socket type=%d for fd=%d is not SOCK_DGRAM or SOCK_STREAM\n", so_type, fd); - return net_socket_fd_init_stream(vlan, model, name, fd, is_connected); + return net_socket_fd_init_stream(vlan, model, + name, fd, is_connected, is_listen); } return NULL; } @@ -389,14 +421,17 @@ static void net_socket_accept(void *opaque) break; } } - s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1); + + s1 = net_socket_fd_init(s->vlan, s->model, s->name, fd, 1, 1); if (s1) { snprintf(s1->nc.info_str, sizeof(s1->nc.info_str), "socket: connection from %s:%d", inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); + s1->nc.link_down = false; } } + static int net_socket_listen_init(VLANState *vlan, const char *model, const char *name, @@ -405,6 +440,7 @@ static int net_socket_listen_init(VLANState *vlan, NetSocketListenState *s; int fd, val, ret; struct sockaddr_in saddr; + NetSocketState *ns; if (parse_host_port(&saddr, host_str) < 0) return -1; @@ -441,6 +477,10 @@ static int net_socket_listen_init(VLANState *vlan, s->model = g_strdup(model); s->name = name ? g_strdup(name) : NULL; s->fd = fd; + + ns = net_socket_fd_init(s->vlan, s->model, s->name, fd, 0, 1); + ns->nc.link_down = true; + qemu_set_fd_handler(fd, net_socket_accept, NULL, s); return 0; } @@ -486,7 +526,7 @@ static int net_socket_connect_init(VLANState *vlan, break; } } - s = net_socket_fd_init(vlan, model, name, fd, connected); + s = net_socket_fd_init(vlan, model, name, fd, connected, 0); if (!s) return -1; snprintf(s->nc.info_str, sizeof(s->nc.info_str), @@ -521,7 +561,7 @@ static int net_socket_mcast_init(VLANState *vlan, if (fd < 0) return -1; - s = net_socket_fd_init(vlan, model, name, fd, 0); + s = net_socket_fd_init(vlan, model, name, fd, 0, 0); if (!s) return -1; @@ -572,7 +612,7 @@ static int net_socket_udp_init(VLANState *vlan, return -1; } - s = net_socket_fd_init(vlan, model, name, fd, 0); + s = net_socket_fd_init(vlan, model, name, fd, 0, 0); if (!s) { return -1; } @@ -606,7 +646,7 @@ int net_init_socket(QemuOpts *opts, return -1; } - if (!net_socket_fd_init(vlan, "socket", name, fd, 1)) { + if (!net_socket_fd_init(vlan, "socket", name, fd, 1, 0)) { return -1; } } else if (qemu_opt_get(opts, "listen")) {