Re: [Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-05-15 Thread Michael S. Tsirkin
On Fri, Apr 22, 2011 at 07:59:28PM +0700, Nguyễn Thái Ngọc Duy wrote:
 Also mention the default value 4096.
 
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com

IS the option really necessary? Why not just allocate 128K or so and be
done with it? That should be big enough even with GSO etc.

 ---
  free new buffers in net_socket_cleanup()
 
  net.c   |4 
  net/socket.c|   52 
  qemu-options.hx |   11 ++-
  3 files changed, 46 insertions(+), 21 deletions(-)
 
 diff --git a/net.c b/net.c
 index 8d6a555..6746bc7 100644
 --- a/net.c
 +++ b/net.c
 @@ -1001,6 +1001,10 @@ static const struct {
  .name = localaddr,
  .type = QEMU_OPT_STRING,
  .help = source address for multicast packets,
 +}, {
 +.name = mtu,
 +.type = QEMU_OPT_NUMBER,
 +.help = MTU size,
  },
  { /* end of list */ }
  },
 diff --git a/net/socket.c b/net/socket.c
 index 7337f4f..e932064 100644
 --- a/net/socket.c
 +++ b/net/socket.c
 @@ -38,7 +38,8 @@ typedef struct NetSocketState {
  int state; /* 0 = getting length, 1 = getting data */
  unsigned int index;
  unsigned int packet_len;
 -uint8_t buf[4096];
 +unsigned int mtu;
 +uint8_t *buf, *buf1;
  struct sockaddr_in dgram_dst; /* contains inet host and port destination 
 iff connectionless (SOCK_DGRAM) */
  } NetSocketState;
  
 @@ -47,6 +48,7 @@ typedef struct NetSocketListenState {
  char *model;
  char *name;
  int fd;
 +unsigned int mtu;
  } NetSocketListenState;
  
  /* XXX: we consider we can send the whole packet without blocking */
 @@ -73,10 +75,10 @@ static void net_socket_send(void *opaque)
  NetSocketState *s = opaque;
  int size, err;
  unsigned l;
 -uint8_t buf1[4096];
 +uint8_t *buf1 = s-buf1;
  const uint8_t *buf;
  
 -size = recv(s-fd, (void *)buf1, sizeof(buf1), 0);
 +size = recv(s-fd, (void *)buf1, s-mtu, 0);
  if (size  0) {
  err = socket_error();
  if (err != EWOULDBLOCK)
 @@ -111,7 +113,7 @@ static void net_socket_send(void *opaque)
  l = s-packet_len - s-index;
  if (l  size)
  l = size;
 -if (s-index + l = sizeof(s-buf)) {
 +if (s-index + l = s-mtu) {
  memcpy(s-buf + s-index, buf, l);
  } else {
  fprintf(stderr, serious error: oversized packet received,
 @@ -138,7 +140,7 @@ static void net_socket_send_dgram(void *opaque)
  NetSocketState *s = opaque;
  int size;
  
 -size = recv(s-fd, (void *)s-buf, sizeof(s-buf), 0);
 +size = recv(s-fd, (void *)s-buf, s-mtu, 0);
  if (size  0)
  return;
  if (size == 0) {
 @@ -228,6 +230,8 @@ static void net_socket_cleanup(VLANClientState *nc)
  NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
  qemu_set_fd_handler(s-fd, NULL, NULL, NULL);
  close(s-fd);
 +qemu_free(s-buf);
 +qemu_free(s-buf1);
  }
  
  static NetClientInfo net_dgram_socket_info = {
 @@ -238,6 +242,7 @@ static NetClientInfo net_dgram_socket_info = {
  };
  
  static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
 +unsigned int mtu,
  const char *model,
  const char *name,
  int fd, int is_connected)
 @@ -288,6 +293,10 @@ static NetSocketState 
 *net_socket_fd_init_dgram(VLANState *vlan,
  
  s = DO_UPCAST(NetSocketState, nc, nc);
  
 +s-mtu = mtu;
 +s-buf = qemu_malloc(s-mtu);
 +s-buf1 = qemu_malloc(s-mtu);
 +
  s-fd = fd;
  
  qemu_set_fd_handler(s-fd, net_socket_send_dgram, NULL, s);
 @@ -312,6 +321,7 @@ static NetClientInfo net_socket_info = {
  };
  
  static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
 + unsigned int mtu,
   const char *model,
   const char *name,
   int fd, int is_connected)
 @@ -325,6 +335,10 @@ static NetSocketState 
 *net_socket_fd_init_stream(VLANState *vlan,
  
  s = DO_UPCAST(NetSocketState, nc, nc);
  
 +s-mtu = mtu;
 +s-buf = qemu_malloc(s-mtu);
 +s-buf1 = qemu_malloc(s-mtu);
 +
  s-fd = fd;
  
  if (is_connected) {
 @@ -335,7 +349,7 @@ static NetSocketState 
 *net_socket_fd_init_stream(VLANState *vlan,
  return s;
  }
  
 -static NetSocketState *net_socket_fd_init(VLANState *vlan,
 +static NetSocketState *net_socket_fd_init(VLANState *vlan, unsigned int mtu,
const char *model, const char 
 *name,
int fd, int 

Re: [Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-05-15 Thread Nguyen Thai Ngoc Duy
2011/5/16 Michael S. Tsirkin m...@redhat.com:
 On Fri, Apr 22, 2011 at 07:59:28PM +0700, Nguyễn Thái Ngọc Duy wrote:
 Also mention the default value 4096.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com

 IS the option really necessary? Why not just allocate 128K or so and be
 done with it? That should be big enough even with GSO etc.

If it's big enough for practically everything, then yes, that way
would be simpler.
-- 
Duy



Re: [Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-04-23 Thread Stefan Hajnoczi
2011/4/22 Nguyễn Thái Ngọc Duy pclo...@gmail.com:
 Also mention the default value 4096.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  free new buffers in net_socket_cleanup()

  net.c           |    4 
  net/socket.c    |   52 
  qemu-options.hx |   11 ++-
  3 files changed, 46 insertions(+), 21 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com



Re: [Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-04-22 Thread Stefan Hajnoczi
2011/4/22 Nguyễn Thái Ngọc Duy pclo...@gmail.com:
 @@ -288,6 +291,10 @@ static NetSocketState 
 *net_socket_fd_init_dgram(VLANState *vlan,

     s = DO_UPCAST(NetSocketState, nc, nc);

 +    s-mtu = mtu;
 +    s-buf = qemu_malloc(s-mtu);
 +    s-buf1 = qemu_malloc(s-mtu);

These need to be freed in net_socket_cleanup().

Stefan



[Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-04-22 Thread Nguyễn Thái Ngọc Duy
Also mention the default value 4096.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 free new buffers in net_socket_cleanup()

 net.c   |4 
 net/socket.c|   52 
 qemu-options.hx |   11 ++-
 3 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/net.c b/net.c
index 8d6a555..6746bc7 100644
--- a/net.c
+++ b/net.c
@@ -1001,6 +1001,10 @@ static const struct {
 .name = localaddr,
 .type = QEMU_OPT_STRING,
 .help = source address for multicast packets,
+}, {
+.name = mtu,
+.type = QEMU_OPT_NUMBER,
+.help = MTU size,
 },
 { /* end of list */ }
 },
diff --git a/net/socket.c b/net/socket.c
index 7337f4f..e932064 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -38,7 +38,8 @@ typedef struct NetSocketState {
 int state; /* 0 = getting length, 1 = getting data */
 unsigned int index;
 unsigned int packet_len;
-uint8_t buf[4096];
+unsigned int mtu;
+uint8_t *buf, *buf1;
 struct sockaddr_in dgram_dst; /* contains inet host and port destination 
iff connectionless (SOCK_DGRAM) */
 } NetSocketState;
 
@@ -47,6 +48,7 @@ typedef struct NetSocketListenState {
 char *model;
 char *name;
 int fd;
+unsigned int mtu;
 } NetSocketListenState;
 
 /* XXX: we consider we can send the whole packet without blocking */
@@ -73,10 +75,10 @@ static void net_socket_send(void *opaque)
 NetSocketState *s = opaque;
 int size, err;
 unsigned l;
-uint8_t buf1[4096];
+uint8_t *buf1 = s-buf1;
 const uint8_t *buf;
 
-size = recv(s-fd, (void *)buf1, sizeof(buf1), 0);
+size = recv(s-fd, (void *)buf1, s-mtu, 0);
 if (size  0) {
 err = socket_error();
 if (err != EWOULDBLOCK)
@@ -111,7 +113,7 @@ static void net_socket_send(void *opaque)
 l = s-packet_len - s-index;
 if (l  size)
 l = size;
-if (s-index + l = sizeof(s-buf)) {
+if (s-index + l = s-mtu) {
 memcpy(s-buf + s-index, buf, l);
 } else {
 fprintf(stderr, serious error: oversized packet received,
@@ -138,7 +140,7 @@ static void net_socket_send_dgram(void *opaque)
 NetSocketState *s = opaque;
 int size;
 
-size = recv(s-fd, (void *)s-buf, sizeof(s-buf), 0);
+size = recv(s-fd, (void *)s-buf, s-mtu, 0);
 if (size  0)
 return;
 if (size == 0) {
@@ -228,6 +230,8 @@ static void net_socket_cleanup(VLANClientState *nc)
 NetSocketState *s = DO_UPCAST(NetSocketState, nc, nc);
 qemu_set_fd_handler(s-fd, NULL, NULL, NULL);
 close(s-fd);
+qemu_free(s-buf);
+qemu_free(s-buf1);
 }
 
 static NetClientInfo net_dgram_socket_info = {
@@ -238,6 +242,7 @@ static NetClientInfo net_dgram_socket_info = {
 };
 
 static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
+unsigned int mtu,
 const char *model,
 const char *name,
 int fd, int is_connected)
@@ -288,6 +293,10 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState 
*vlan,
 
 s = DO_UPCAST(NetSocketState, nc, nc);
 
+s-mtu = mtu;
+s-buf = qemu_malloc(s-mtu);
+s-buf1 = qemu_malloc(s-mtu);
+
 s-fd = fd;
 
 qemu_set_fd_handler(s-fd, net_socket_send_dgram, NULL, s);
@@ -312,6 +321,7 @@ static NetClientInfo net_socket_info = {
 };
 
 static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
+ unsigned int mtu,
  const char *model,
  const char *name,
  int fd, int is_connected)
@@ -325,6 +335,10 @@ static NetSocketState *net_socket_fd_init_stream(VLANState 
*vlan,
 
 s = DO_UPCAST(NetSocketState, nc, nc);
 
+s-mtu = mtu;
+s-buf = qemu_malloc(s-mtu);
+s-buf1 = qemu_malloc(s-mtu);
+
 s-fd = fd;
 
 if (is_connected) {
@@ -335,7 +349,7 @@ static NetSocketState *net_socket_fd_init_stream(VLANState 
*vlan,
 return s;
 }
 
-static NetSocketState *net_socket_fd_init(VLANState *vlan,
+static NetSocketState *net_socket_fd_init(VLANState *vlan, unsigned int mtu,
   const char *model, const char *name,
   int fd, int is_connected)
 {
@@ -348,13 +362,13 @@ 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, mtu, model, name, fd, 
is_connected);
 case SOCK_STREAM:
-

[Qemu-devel] [PATCH] net/socket: remove hardcoded packet size in favor of new mtu parameter

2011-04-21 Thread Nguyễn Thái Ngọc Duy
Also mention the default value 4096.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 I have a driver that sends 4352 byte packets. Tested with tcp socket only.
 There's also 4096 byte buffers in vde, but I don't use/test it.

 net.c   |4 
 net/socket.c|   54 ++
 qemu-options.hx |   11 +++
 3 files changed, 49 insertions(+), 20 deletions(-)

diff --git a/net.c b/net.c
index 4f777c3..74dffa3 100644
--- a/net.c
+++ b/net.c
@@ -1000,6 +1000,10 @@ static const struct {
 .name = localaddr,
 .type = QEMU_OPT_STRING,
 .help = source address for multicast packets,
+}, {
+.name = mtu,
+.type = QEMU_OPT_NUMBER,
+.help = MTU size,
 },
 { /* end of list */ }
 },
diff --git a/net/socket.c b/net/socket.c
index 7337f4f..99a9b39 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -38,7 +38,8 @@ typedef struct NetSocketState {
 int state; /* 0 = getting length, 1 = getting data */
 unsigned int index;
 unsigned int packet_len;
-uint8_t buf[4096];
+unsigned int mtu;
+uint8_t *buf, *buf1;
 struct sockaddr_in dgram_dst; /* contains inet host and port destination 
iff connectionless (SOCK_DGRAM) */
 } NetSocketState;
 
@@ -47,6 +48,7 @@ typedef struct NetSocketListenState {
 char *model;
 char *name;
 int fd;
+unsigned int mtu;
 } NetSocketListenState;
 
 /* XXX: we consider we can send the whole packet without blocking */
@@ -73,10 +75,10 @@ static void net_socket_send(void *opaque)
 NetSocketState *s = opaque;
 int size, err;
 unsigned l;
-uint8_t buf1[4096];
+uint8_t *buf1 = s-buf1;
 const uint8_t *buf;
 
-size = recv(s-fd, (void *)buf1, sizeof(buf1), 0);
+size = recv(s-fd, (void *)buf1, s-mtu, 0);
 if (size  0) {
 err = socket_error();
 if (err != EWOULDBLOCK)
@@ -111,7 +113,7 @@ static void net_socket_send(void *opaque)
 l = s-packet_len - s-index;
 if (l  size)
 l = size;
-if (s-index + l = sizeof(s-buf)) {
+if (s-index + l = s-mtu) {
 memcpy(s-buf + s-index, buf, l);
 } else {
 fprintf(stderr, serious error: oversized packet received,
@@ -138,7 +140,7 @@ static void net_socket_send_dgram(void *opaque)
 NetSocketState *s = opaque;
 int size;
 
-size = recv(s-fd, (void *)s-buf, sizeof(s-buf), 0);
+size = recv(s-fd, (void *)s-buf, s-mtu, 0);
 if (size  0)
 return;
 if (size == 0) {
@@ -238,6 +240,7 @@ static NetClientInfo net_dgram_socket_info = {
 };
 
 static NetSocketState *net_socket_fd_init_dgram(VLANState *vlan,
+unsigned int mtu,
 const char *model,
 const char *name,
 int fd, int is_connected)
@@ -288,6 +291,10 @@ static NetSocketState *net_socket_fd_init_dgram(VLANState 
*vlan,
 
 s = DO_UPCAST(NetSocketState, nc, nc);
 
+s-mtu = mtu;
+s-buf = qemu_malloc(s-mtu);
+s-buf1 = qemu_malloc(s-mtu);
+
 s-fd = fd;
 
 qemu_set_fd_handler(s-fd, net_socket_send_dgram, NULL, s);
@@ -312,6 +319,7 @@ static NetClientInfo net_socket_info = {
 };
 
 static NetSocketState *net_socket_fd_init_stream(VLANState *vlan,
+ unsigned int mtu,
  const char *model,
  const char *name,
  int fd, int is_connected)
@@ -325,6 +333,10 @@ static NetSocketState *net_socket_fd_init_stream(VLANState 
*vlan,
 
 s = DO_UPCAST(NetSocketState, nc, nc);
 
+s-mtu = mtu;
+s-buf = qemu_malloc(s-mtu);
+s-buf1 = qemu_malloc(s-mtu);
+
 s-fd = fd;
 
 if (is_connected) {
@@ -335,7 +347,7 @@ static NetSocketState *net_socket_fd_init_stream(VLANState 
*vlan,
 return s;
 }
 
-static NetSocketState *net_socket_fd_init(VLANState *vlan,
+static NetSocketState *net_socket_fd_init(VLANState *vlan, unsigned int mtu,
   const char *model, const char *name,
   int fd, int is_connected)
 {
@@ -348,13 +360,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, mtu, model, name,
+fd, is_connected);
 case SOCK_STREAM:
-return net_socket_fd_init_stream(vlan, model, name, fd, is_connected);
+return net_socket_fd_init_stream(vlan, mtu, model, name,
+