* [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
@ 2016-05-06 10:56 Zhang Chen
2016-05-10 2:57 ` Li Zhijian
2016-05-11 9:01 ` Jason Wang
0 siblings, 2 replies; 10+ messages in thread
From: Zhang Chen @ 2016-05-06 10:56 UTC (permalink / raw)
To: qemu devel, Jason Wang
Cc: Zhang Chen, Li Zhijian, Wen Congyang, eddie . dong,
Dr . David Alan Gilbert
Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
---
include/net/net.h | 8 ++++++
net/filter-mirror.c | 60 ++++++++------------------------------------
net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
net/socket.c | 71 +++++++++++++----------------------------------------
4 files changed, 91 insertions(+), 104 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 73e4c46..1439cf9 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -102,6 +102,14 @@ typedef struct NICState {
bool peer_deleted;
} NICState;
+typedef struct ReadState {
+ int state; /* 0 = getting length, 1 = getting data */
+ uint32_t index;
+ uint32_t packet_len;
+ uint8_t buf[NET_BUFSIZE];
+} ReadState;
+
+int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
char *qemu_mac_strdup_printf(const uint8_t *macaddr);
NetClientState *qemu_find_netdev(const char *id);
int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
diff --git a/net/filter-mirror.c b/net/filter-mirror.c
index c0c4dc6..bcec509 100644
--- a/net/filter-mirror.c
+++ b/net/filter-mirror.c
@@ -40,10 +40,7 @@ typedef struct MirrorState {
char *outdev;
CharDriverState *chr_in;
CharDriverState *chr_out;
- int state; /* 0 = getting length, 1 = getting data */
- unsigned int index;
- unsigned int packet_len;
- uint8_t buf[REDIRECTOR_MAX_LEN];
+ ReadState rs;
} MirrorState;
static int filter_mirror_send(CharDriverState *chr_out,
@@ -108,51 +105,14 @@ static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
{
NetFilterState *nf = opaque;
MirrorState *s = FILTER_REDIRECTOR(nf);
- unsigned int l;
-
- while (size > 0) {
- /* reassemble a packet from the network */
- switch (s->state) { /* 0 = getting length, 1 = getting data */
- case 0:
- l = 4 - s->index;
- if (l > size) {
- l = size;
- }
- memcpy(s->buf + s->index, buf, l);
- buf += l;
- size -= l;
- s->index += l;
- if (s->index == 4) {
- /* got length */
- s->packet_len = ntohl(*(uint32_t *)s->buf);
- s->index = 0;
- s->state = 1;
- }
- break;
- case 1:
- l = s->packet_len - s->index;
- if (l > size) {
- l = size;
- }
- if (s->index + l <= sizeof(s->buf)) {
- memcpy(s->buf + s->index, buf, l);
- } else {
- error_report("serious error: oversized packet received.");
- s->index = s->state = 0;
- qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
- return;
- }
-
- s->index += l;
- buf += l;
- size -= l;
- if (s->index >= s->packet_len) {
- s->index = 0;
- s->state = 0;
- redirector_to_filter(nf, s->buf, s->packet_len);
- }
- break;
- }
+ int ret;
+
+ ret = net_fill_rstate(&s->rs, buf, size);
+
+ if (ret == -1) {
+ qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
+ } else if (ret == 1) {
+ redirector_to_filter(nf, s->rs.buf, s->rs.packet_len);
}
}
@@ -274,7 +234,7 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp)
}
}
- s->state = s->index = 0;
+ s->rs.state = s->rs.index = 0;
if (s->indev) {
s->chr_in = qemu_chr_find(s->indev);
diff --git a/net/net.c b/net/net.c
index 0ad6217..926457e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1573,3 +1573,59 @@ QemuOptsList qemu_net_opts = {
{ /* end of list */ }
},
};
+
+/*
+ * Returns
+ * 0: readstate is not ready
+ * 1: readstate is ready
+ * otherwise error occurs
+ */
+int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size)
+{
+ unsigned int l;
+ while (size > 0) {
+ /* reassemble a packet from the network */
+ switch (rs->state) { /* 0 = getting length, 1 = getting data */
+ case 0:
+ l = 4 - rs->index;
+ if (l > size) {
+ l = size;
+ }
+ memcpy(rs->buf + rs->index, buf, l);
+ buf += l;
+ size -= l;
+ rs->index += l;
+ if (rs->index == 4) {
+ /* got length */
+ rs->packet_len = ntohl(*(uint32_t *)rs->buf);
+ rs->index = 0;
+ rs->state = 1;
+ }
+ break;
+ case 1:
+ l = rs->packet_len - rs->index;
+ if (l > size) {
+ l = size;
+ }
+ if (rs->index + l <= sizeof(rs->buf)) {
+ memcpy(rs->buf + rs->index, buf, l);
+ } else {
+ fprintf(stderr, "serious error: oversized packet received,"
+ "connection terminated.\n");
+ rs->index = rs->state = 0;
+ return -1;
+ }
+
+ rs->index += l;
+ buf += l;
+ size -= l;
+ if (rs->index >= rs->packet_len) {
+ rs->index = 0;
+ rs->state = 0;
+ return 1;
+ }
+ break;
+ }
+ }
+ return 0;
+}
diff --git a/net/socket.c b/net/socket.c
index 9fa2cd8..9ecdd3b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -38,11 +38,8 @@ typedef struct NetSocketState {
NetClientState nc;
int listen_fd;
int fd;
- int state; /* 0 = getting length, 1 = getting data */
- unsigned int index;
- unsigned int packet_len;
+ ReadState rs;
unsigned int send_index; /* number of bytes sent (only SOCK_STREAM) */
- uint8_t buf[NET_BUFSIZE];
struct sockaddr_in dgram_dst; /* contains inet host and port destination iff connectionless (SOCK_DGRAM) */
IOHandler *send_fn; /* differs between SOCK_STREAM/SOCK_DGRAM */
bool read_poll; /* waiting to receive data? */
@@ -147,7 +144,7 @@ static void net_socket_send(void *opaque)
{
NetSocketState *s = opaque;
int size;
- unsigned l;
+ int ret;
uint8_t buf1[NET_BUFSIZE];
const uint8_t *buf;
@@ -166,60 +163,26 @@ static void net_socket_send(void *opaque)
closesocket(s->fd);
s->fd = -1;
- s->state = 0;
- s->index = 0;
- s->packet_len = 0;
+ s->rs.state = 0;
+ s->rs.index = 0;
+ s->rs.packet_len = 0;
s->nc.link_down = true;
- memset(s->buf, 0, sizeof(s->buf));
+ memset(s->rs.buf, 0, sizeof(s->rs.buf));
memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
return;
}
buf = buf1;
- while (size > 0) {
- /* reassemble a packet from the network */
- switch(s->state) {
- case 0:
- l = 4 - s->index;
- if (l > size)
- l = size;
- memcpy(s->buf + s->index, buf, l);
- buf += l;
- size -= l;
- s->index += l;
- if (s->index == 4) {
- /* got length */
- s->packet_len = ntohl(*(uint32_t *)s->buf);
- s->index = 0;
- s->state = 1;
- }
- break;
- case 1:
- l = s->packet_len - s->index;
- if (l > size)
- l = size;
- if (s->index + l <= sizeof(s->buf)) {
- memcpy(s->buf + s->index, buf, l);
- } else {
- fprintf(stderr, "serious error: oversized packet received,"
- "connection terminated.\n");
- s->state = 0;
- goto eoc;
- }
- s->index += l;
- buf += l;
- size -= l;
- if (s->index >= s->packet_len) {
- s->index = 0;
- s->state = 0;
- if (qemu_send_packet_async(&s->nc, s->buf, s->packet_len,
- net_socket_send_completed) == 0) {
- net_socket_read_poll(s, false);
- break;
- }
- }
- break;
+ ret = net_fill_rstate(&s->rs, buf, size);
+
+ if (ret == -1) {
+ goto eoc;
+ } else if (ret == 1) {
+ if (qemu_send_packet_async(&s->nc, s->rs.buf,
+ s->rs.packet_len,
+ net_socket_send_completed) == 0) {
+ net_socket_read_poll(s, false);
}
}
}
@@ -229,7 +192,7 @@ static void net_socket_send_dgram(void *opaque)
NetSocketState *s = opaque;
int size;
- size = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
+ size = qemu_recv(s->fd, s->rs.buf, sizeof(s->rs.buf), 0);
if (size < 0)
return;
if (size == 0) {
@@ -238,7 +201,7 @@ static void net_socket_send_dgram(void *opaque)
net_socket_write_poll(s, false);
return;
}
- if (qemu_send_packet_async(&s->nc, s->buf, size,
+ if (qemu_send_packet_async(&s->nc, s->rs.buf, size,
net_socket_send_completed) == 0) {
net_socket_read_poll(s, false);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-06 10:56 [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes Zhang Chen
@ 2016-05-10 2:57 ` Li Zhijian
2016-05-10 5:33 ` Zhang Chen
2016-05-11 7:01 ` Zhang Chen
2016-05-11 9:01 ` Jason Wang
1 sibling, 2 replies; 10+ messages in thread
From: Li Zhijian @ 2016-05-10 2:57 UTC (permalink / raw)
To: Zhang Chen, qemu devel, Jason Wang
Cc: Wen Congyang, eddie . dong, Dr . David Alan Gilbert
On 05/06/2016 06:56 PM, Zhang Chen wrote:
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> ---
> include/net/net.h | 8 ++++++
> net/filter-mirror.c | 60 ++++++++------------------------------------
> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> net/socket.c | 71 +++++++++++++----------------------------------------
> 4 files changed, 91 insertions(+), 104 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 73e4c46..1439cf9 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -102,6 +102,14 @@ typedef struct NICState {
> bool peer_deleted;
> } NICState;
>
> +typedef struct ReadState {
> + int state; /* 0 = getting length, 1 = getting data */
> + uint32_t index;
> + uint32_t packet_len;
> + uint8_t buf[NET_BUFSIZE];
> +} ReadState;
> +
> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
> NetClientState *qemu_find_netdev(const char *id);
> int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index c0c4dc6..bcec509 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -40,10 +40,7 @@ typedef struct MirrorState {
> char *outdev;
> CharDriverState *chr_in;
> CharDriverState *chr_out;
> - int state; /* 0 = getting length, 1 = getting data */
> - unsigned int index;
> - unsigned int packet_len;
> - uint8_t buf[REDIRECTOR_MAX_LEN];
you may need to remove 'REDIRECTOR_MAX_LEN' defination too.
Thanks
Li Zhijian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-10 2:57 ` Li Zhijian
@ 2016-05-10 5:33 ` Zhang Chen
2016-05-11 7:01 ` Zhang Chen
1 sibling, 0 replies; 10+ messages in thread
From: Zhang Chen @ 2016-05-10 5:33 UTC (permalink / raw)
To: Li Zhijian, qemu devel, Jason Wang
Cc: Wen Congyang, eddie . dong, Dr . David Alan Gilbert
On 05/10/2016 10:57 AM, Li Zhijian wrote:
>
>
> On 05/06/2016 06:56 PM, Zhang Chen wrote:
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> include/net/net.h | 8 ++++++
>> net/filter-mirror.c | 60 ++++++++------------------------------------
>> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>> net/socket.c | 71
>> +++++++++++++----------------------------------------
>> 4 files changed, 91 insertions(+), 104 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 73e4c46..1439cf9 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -102,6 +102,14 @@ typedef struct NICState {
>> bool peer_deleted;
>> } NICState;
>>
>> +typedef struct ReadState {
>> + int state; /* 0 = getting length, 1 = getting data */
>> + uint32_t index;
>> + uint32_t packet_len;
>> + uint8_t buf[NET_BUFSIZE];
>> +} ReadState;
>> +
>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
>> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>> NetClientState *qemu_find_netdev(const char *id);
>> int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index c0c4dc6..bcec509 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -40,10 +40,7 @@ typedef struct MirrorState {
>> char *outdev;
>> CharDriverState *chr_in;
>> CharDriverState *chr_out;
>> - int state; /* 0 = getting length, 1 = getting data */
>> - unsigned int index;
>> - unsigned int packet_len;
>> - uint8_t buf[REDIRECTOR_MAX_LEN];
> you may need to remove 'REDIRECTOR_MAX_LEN' defination too.
>
OK,I will fix it in next version.
Thanks
Zhang Chen
> Thanks
> Li Zhijian
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-10 2:57 ` Li Zhijian
2016-05-10 5:33 ` Zhang Chen
@ 2016-05-11 7:01 ` Zhang Chen
1 sibling, 0 replies; 10+ messages in thread
From: Zhang Chen @ 2016-05-11 7:01 UTC (permalink / raw)
To: Li Zhijian, qemu devel, Jason Wang
Cc: Wen Congyang, eddie . dong, Dr . David Alan Gilbert
On 05/10/2016 10:57 AM, Li Zhijian wrote:
>
>
> On 05/06/2016 06:56 PM, Zhang Chen wrote:
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> ---
>> include/net/net.h | 8 ++++++
>> net/filter-mirror.c | 60 ++++++++------------------------------------
>> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>> net/socket.c | 71
>> +++++++++++++----------------------------------------
>> 4 files changed, 91 insertions(+), 104 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 73e4c46..1439cf9 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -102,6 +102,14 @@ typedef struct NICState {
>> bool peer_deleted;
>> } NICState;
>>
>> +typedef struct ReadState {
>> + int state; /* 0 = getting length, 1 = getting data */
>> + uint32_t index;
>> + uint32_t packet_len;
>> + uint8_t buf[NET_BUFSIZE];
>> +} ReadState;
>> +
>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
>> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>> NetClientState *qemu_find_netdev(const char *id);
>> int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index c0c4dc6..bcec509 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -40,10 +40,7 @@ typedef struct MirrorState {
>> char *outdev;
>> CharDriverState *chr_in;
>> CharDriverState *chr_out;
>> - int state; /* 0 = getting length, 1 = getting data */
>> - unsigned int index;
>> - unsigned int packet_len;
>> - uint8_t buf[REDIRECTOR_MAX_LEN];
> you may need to remove 'REDIRECTOR_MAX_LEN' defination too.
Sorry, in redirector_chr_can_read() use 'REDIRECTOR_MAX_LEN' defination too.
so we can't remove it.
Thanks
Zhang Chen
>
> Thanks
> Li Zhijian
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-06 10:56 [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes Zhang Chen
2016-05-10 2:57 ` Li Zhijian
@ 2016-05-11 9:01 ` Jason Wang
2016-05-11 11:20 ` Zhang Chen
1 sibling, 1 reply; 10+ messages in thread
From: Jason Wang @ 2016-05-11 9:01 UTC (permalink / raw)
To: Zhang Chen, qemu devel
Cc: Li Zhijian, Wen Congyang, eddie . dong, Dr . David Alan Gilbert
On 2016年05月06日 18:56, Zhang Chen wrote:
> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Looks good, just few nits.
It's better to have a commit log.
> ---
> include/net/net.h | 8 ++++++
> net/filter-mirror.c | 60 ++++++++------------------------------------
> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
> net/socket.c | 71 +++++++++++++----------------------------------------
> 4 files changed, 91 insertions(+), 104 deletions(-)
>
> diff --git a/include/net/net.h b/include/net/net.h
> index 73e4c46..1439cf9 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -102,6 +102,14 @@ typedef struct NICState {
> bool peer_deleted;
> } NICState;
>
> +typedef struct ReadState {
> + int state; /* 0 = getting length, 1 = getting data */
> + uint32_t index;
> + uint32_t packet_len;
> + uint8_t buf[NET_BUFSIZE];
> +} ReadState;
I think SocketReadState is better here.
> +
> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
> NetClientState *qemu_find_netdev(const char *id);
> int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index c0c4dc6..bcec509 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -40,10 +40,7 @@ typedef struct MirrorState {
> char *outdev;
> CharDriverState *chr_in;
> CharDriverState *chr_out;
> - int state; /* 0 = getting length, 1 = getting data */
> - unsigned int index;
> - unsigned int packet_len;
> - uint8_t buf[REDIRECTOR_MAX_LEN];
> + ReadState rs;
> } MirrorState;
>
> static int filter_mirror_send(CharDriverState *chr_out,
> @@ -108,51 +105,14 @@ static void redirector_chr_read(void *opaque, const uint8_t *buf, int size)
> {
> NetFilterState *nf = opaque;
> MirrorState *s = FILTER_REDIRECTOR(nf);
> - unsigned int l;
> -
> - while (size > 0) {
> - /* reassemble a packet from the network */
> - switch (s->state) { /* 0 = getting length, 1 = getting data */
> - case 0:
> - l = 4 - s->index;
> - if (l > size) {
> - l = size;
> - }
> - memcpy(s->buf + s->index, buf, l);
> - buf += l;
> - size -= l;
> - s->index += l;
> - if (s->index == 4) {
> - /* got length */
> - s->packet_len = ntohl(*(uint32_t *)s->buf);
> - s->index = 0;
> - s->state = 1;
> - }
> - break;
> - case 1:
> - l = s->packet_len - s->index;
> - if (l > size) {
> - l = size;
> - }
> - if (s->index + l <= sizeof(s->buf)) {
> - memcpy(s->buf + s->index, buf, l);
> - } else {
> - error_report("serious error: oversized packet received.");
> - s->index = s->state = 0;
> - qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
> - return;
> - }
> -
> - s->index += l;
> - buf += l;
> - size -= l;
> - if (s->index >= s->packet_len) {
> - s->index = 0;
> - s->state = 0;
> - redirector_to_filter(nf, s->buf, s->packet_len);
> - }
> - break;
> - }
> + int ret;
> +
> + ret = net_fill_rstate(&s->rs, buf, size);
> +
> + if (ret == -1) {
> + qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
> + } else if (ret == 1) {
> + redirector_to_filter(nf, s->rs.buf, s->rs.packet_len);
> }
> }
>
> @@ -274,7 +234,7 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp)
> }
> }
>
> - s->state = s->index = 0;
> + s->rs.state = s->rs.index = 0;
>
> if (s->indev) {
> s->chr_in = qemu_chr_find(s->indev);
> diff --git a/net/net.c b/net/net.c
> index 0ad6217..926457e 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1573,3 +1573,59 @@ QemuOptsList qemu_net_opts = {
> { /* end of list */ }
> },
> };
> +
> +/*
> + * Returns
> + * 0: readstate is not ready
> + * 1: readstate is ready
> + * otherwise error occurs
> + */
> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size)
> +{
> + unsigned int l;
> + while (size > 0) {
> + /* reassemble a packet from the network */
> + switch (rs->state) { /* 0 = getting length, 1 = getting data */
> + case 0:
> + l = 4 - rs->index;
> + if (l > size) {
> + l = size;
> + }
> + memcpy(rs->buf + rs->index, buf, l);
> + buf += l;
> + size -= l;
> + rs->index += l;
> + if (rs->index == 4) {
> + /* got length */
> + rs->packet_len = ntohl(*(uint32_t *)rs->buf);
> + rs->index = 0;
> + rs->state = 1;
> + }
> + break;
> + case 1:
> + l = rs->packet_len - rs->index;
> + if (l > size) {
> + l = size;
> + }
> + if (rs->index + l <= sizeof(rs->buf)) {
> + memcpy(rs->buf + rs->index, buf, l);
> + } else {
> + fprintf(stderr, "serious error: oversized packet received,"
> + "connection terminated.\n");
> + rs->index = rs->state = 0;
> + return -1;
> + }
> +
> + rs->index += l;
> + buf += l;
> + size -= l;
> + if (rs->index >= rs->packet_len) {
> + rs->index = 0;
> + rs->state = 0;
> + return 1;
> + }
> + break;
> + }
> + }
> + return 0;
> +}
> diff --git a/net/socket.c b/net/socket.c
> index 9fa2cd8..9ecdd3b 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -38,11 +38,8 @@ typedef struct NetSocketState {
> NetClientState nc;
> int listen_fd;
> int fd;
> - int state; /* 0 = getting length, 1 = getting data */
> - unsigned int index;
> - unsigned int packet_len;
> + ReadState rs;
> unsigned int send_index; /* number of bytes sent (only SOCK_STREAM) */
> - uint8_t buf[NET_BUFSIZE];
> struct sockaddr_in dgram_dst; /* contains inet host and port destination iff connectionless (SOCK_DGRAM) */
> IOHandler *send_fn; /* differs between SOCK_STREAM/SOCK_DGRAM */
> bool read_poll; /* waiting to receive data? */
> @@ -147,7 +144,7 @@ static void net_socket_send(void *opaque)
> {
> NetSocketState *s = opaque;
> int size;
> - unsigned l;
> + int ret;
> uint8_t buf1[NET_BUFSIZE];
> const uint8_t *buf;
>
> @@ -166,60 +163,26 @@ static void net_socket_send(void *opaque)
> closesocket(s->fd);
>
> s->fd = -1;
> - s->state = 0;
> - s->index = 0;
> - s->packet_len = 0;
> + s->rs.state = 0;
> + s->rs.index = 0;
> + s->rs.packet_len = 0;
> s->nc.link_down = true;
> - memset(s->buf, 0, sizeof(s->buf));
> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
How about introduce a helper to do the initialization?
> memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>
> return;
> }
> buf = buf1;
> - while (size > 0) {
> - /* reassemble a packet from the network */
> - switch(s->state) {
> - case 0:
> - l = 4 - s->index;
> - if (l > size)
> - l = size;
> - memcpy(s->buf + s->index, buf, l);
> - buf += l;
> - size -= l;
> - s->index += l;
> - if (s->index == 4) {
> - /* got length */
> - s->packet_len = ntohl(*(uint32_t *)s->buf);
> - s->index = 0;
> - s->state = 1;
> - }
> - break;
> - case 1:
> - l = s->packet_len - s->index;
> - if (l > size)
> - l = size;
> - if (s->index + l <= sizeof(s->buf)) {
> - memcpy(s->buf + s->index, buf, l);
> - } else {
> - fprintf(stderr, "serious error: oversized packet received,"
> - "connection terminated.\n");
> - s->state = 0;
> - goto eoc;
> - }
>
> - s->index += l;
> - buf += l;
> - size -= l;
> - if (s->index >= s->packet_len) {
> - s->index = 0;
> - s->state = 0;
> - if (qemu_send_packet_async(&s->nc, s->buf, s->packet_len,
> - net_socket_send_completed) == 0) {
> - net_socket_read_poll(s, false);
> - break;
> - }
> - }
> - break;
> + ret = net_fill_rstate(&s->rs, buf, size);
> +
> + if (ret == -1) {
> + goto eoc;
> + } else if (ret == 1) {
> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
> + s->rs.packet_len,
> + net_socket_send_completed) == 0) {
> + net_socket_read_poll(s, false);
This looks not elegant, maybe we could use callback (which was
initialized by the helper I mention above) to do this. Any thoughts on this?
> }
> }
> }
> @@ -229,7 +192,7 @@ static void net_socket_send_dgram(void *opaque)
> NetSocketState *s = opaque;
> int size;
>
> - size = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
> + size = qemu_recv(s->fd, s->rs.buf, sizeof(s->rs.buf), 0);
> if (size < 0)
> return;
> if (size == 0) {
> @@ -238,7 +201,7 @@ static void net_socket_send_dgram(void *opaque)
> net_socket_write_poll(s, false);
> return;
> }
> - if (qemu_send_packet_async(&s->nc, s->buf, size,
> + if (qemu_send_packet_async(&s->nc, s->rs.buf, size,
> net_socket_send_completed) == 0) {
> net_socket_read_poll(s, false);
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-11 9:01 ` Jason Wang
@ 2016-05-11 11:20 ` Zhang Chen
2016-05-12 1:11 ` Jason Wang
0 siblings, 1 reply; 10+ messages in thread
From: Zhang Chen @ 2016-05-11 11:20 UTC (permalink / raw)
To: Jason Wang, qemu devel
Cc: Li Zhijian, Wen Congyang, eddie . dong, Dr . David Alan Gilbert
On 05/11/2016 05:01 PM, Jason Wang wrote:
>
>
> On 2016年05月06日 18:56, Zhang Chen wrote:
>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>
> Looks good, just few nits.
>
> It's better to have a commit log.
OK, I will add this log:
This function is from net/socket.c, move it to net.c and net.h.
Add SocketReadState to make others reuse net_fill_rstate().
suggestion from jason.
>
>> ---
>> include/net/net.h | 8 ++++++
>> net/filter-mirror.c | 60 ++++++++------------------------------------
>> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>> net/socket.c | 71
>> +++++++++++++----------------------------------------
>> 4 files changed, 91 insertions(+), 104 deletions(-)
>>
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 73e4c46..1439cf9 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -102,6 +102,14 @@ typedef struct NICState {
>> bool peer_deleted;
>> } NICState;
>> +typedef struct ReadState {
>> + int state; /* 0 = getting length, 1 = getting data */
>> + uint32_t index;
>> + uint32_t packet_len;
>> + uint8_t buf[NET_BUFSIZE];
>> +} ReadState;
>
> I think SocketReadState is better here.
>
I will rename it in next version.
>> +
>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
>> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>> NetClientState *qemu_find_netdev(const char *id);
>> int qemu_find_net_clients_except(const char *id, NetClientState **ncs,
>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>> index c0c4dc6..bcec509 100644
>> --- a/net/filter-mirror.c
>> +++ b/net/filter-mirror.c
>> @@ -40,10 +40,7 @@ typedef struct MirrorState {
>> char *outdev;
>> CharDriverState *chr_in;
>> CharDriverState *chr_out;
>> - int state; /* 0 = getting length, 1 = getting data */
>> - unsigned int index;
>> - unsigned int packet_len;
>> - uint8_t buf[REDIRECTOR_MAX_LEN];
>> + ReadState rs;
>> } MirrorState;
>> static int filter_mirror_send(CharDriverState *chr_out,
>> @@ -108,51 +105,14 @@ static void redirector_chr_read(void *opaque,
>> const uint8_t *buf, int size)
>> {
>> NetFilterState *nf = opaque;
>> MirrorState *s = FILTER_REDIRECTOR(nf);
>> - unsigned int l;
>> -
>> - while (size > 0) {
>> - /* reassemble a packet from the network */
>> - switch (s->state) { /* 0 = getting length, 1 = getting data */
>> - case 0:
>> - l = 4 - s->index;
>> - if (l > size) {
>> - l = size;
>> - }
>> - memcpy(s->buf + s->index, buf, l);
>> - buf += l;
>> - size -= l;
>> - s->index += l;
>> - if (s->index == 4) {
>> - /* got length */
>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>> - s->index = 0;
>> - s->state = 1;
>> - }
>> - break;
>> - case 1:
>> - l = s->packet_len - s->index;
>> - if (l > size) {
>> - l = size;
>> - }
>> - if (s->index + l <= sizeof(s->buf)) {
>> - memcpy(s->buf + s->index, buf, l);
>> - } else {
>> - error_report("serious error: oversized packet
>> received.");
>> - s->index = s->state = 0;
>> - qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL,
>> NULL);
>> - return;
>> - }
>> -
>> - s->index += l;
>> - buf += l;
>> - size -= l;
>> - if (s->index >= s->packet_len) {
>> - s->index = 0;
>> - s->state = 0;
>> - redirector_to_filter(nf, s->buf, s->packet_len);
>> - }
>> - break;
>> - }
>> + int ret;
>> +
>> + ret = net_fill_rstate(&s->rs, buf, size);
>> +
>> + if (ret == -1) {
>> + qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
>> + } else if (ret == 1) {
>> + redirector_to_filter(nf, s->rs.buf, s->rs.packet_len);
>> }
>> }
>> @@ -274,7 +234,7 @@ static void
>> filter_redirector_setup(NetFilterState *nf, Error **errp)
>> }
>> }
>> - s->state = s->index = 0;
>> + s->rs.state = s->rs.index = 0;
>> if (s->indev) {
>> s->chr_in = qemu_chr_find(s->indev);
>> diff --git a/net/net.c b/net/net.c
>> index 0ad6217..926457e 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -1573,3 +1573,59 @@ QemuOptsList qemu_net_opts = {
>> { /* end of list */ }
>> },
>> };
>> +
>> +/*
>> + * Returns
>> + * 0: readstate is not ready
>> + * 1: readstate is ready
>> + * otherwise error occurs
>> + */
>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size)
>> +{
>> + unsigned int l;
>> + while (size > 0) {
>> + /* reassemble a packet from the network */
>> + switch (rs->state) { /* 0 = getting length, 1 = getting data */
>> + case 0:
>> + l = 4 - rs->index;
>> + if (l > size) {
>> + l = size;
>> + }
>> + memcpy(rs->buf + rs->index, buf, l);
>> + buf += l;
>> + size -= l;
>> + rs->index += l;
>> + if (rs->index == 4) {
>> + /* got length */
>> + rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>> + rs->index = 0;
>> + rs->state = 1;
>> + }
>> + break;
>> + case 1:
>> + l = rs->packet_len - rs->index;
>> + if (l > size) {
>> + l = size;
>> + }
>> + if (rs->index + l <= sizeof(rs->buf)) {
>> + memcpy(rs->buf + rs->index, buf, l);
>> + } else {
>> + fprintf(stderr, "serious error: oversized packet
>> received,"
>> + "connection terminated.\n");
>> + rs->index = rs->state = 0;
>> + return -1;
>> + }
>> +
>> + rs->index += l;
>> + buf += l;
>> + size -= l;
>> + if (rs->index >= rs->packet_len) {
>> + rs->index = 0;
>> + rs->state = 0;
>> + return 1;
>> + }
>> + break;
>> + }
>> + }
>> + return 0;
>> +}
>> diff --git a/net/socket.c b/net/socket.c
>> index 9fa2cd8..9ecdd3b 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -38,11 +38,8 @@ typedef struct NetSocketState {
>> NetClientState nc;
>> int listen_fd;
>> int fd;
>> - int state; /* 0 = getting length, 1 = getting data */
>> - unsigned int index;
>> - unsigned int packet_len;
>> + ReadState rs;
>> unsigned int send_index; /* number of bytes sent (only
>> SOCK_STREAM) */
>> - uint8_t buf[NET_BUFSIZE];
>> struct sockaddr_in dgram_dst; /* contains inet host and port
>> destination iff connectionless (SOCK_DGRAM) */
>> IOHandler *send_fn; /* differs between
>> SOCK_STREAM/SOCK_DGRAM */
>> bool read_poll; /* waiting to receive data? */
>> @@ -147,7 +144,7 @@ static void net_socket_send(void *opaque)
>> {
>> NetSocketState *s = opaque;
>> int size;
>> - unsigned l;
>> + int ret;
>> uint8_t buf1[NET_BUFSIZE];
>> const uint8_t *buf;
>> @@ -166,60 +163,26 @@ static void net_socket_send(void *opaque)
>> closesocket(s->fd);
>> s->fd = -1;
>> - s->state = 0;
>> - s->index = 0;
>> - s->packet_len = 0;
>> + s->rs.state = 0;
>> + s->rs.index = 0;
>> + s->rs.packet_len = 0;
>> s->nc.link_down = true;
>> - memset(s->buf, 0, sizeof(s->buf));
>> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
>
> How about introduce a helper to do the initialization?
Do you mean
remove
+ s->rs.state = 0;
+ s->rs.index = 0;
+ s->rs.packet_len = 0;
+ memset(s->rs.buf, 0, sizeof(s->rs.buf));
add
s->rs->cb = xxx_cb;
s->rs->opxx = xxx;
void xxx_cb(void *opaque)
{
NetSocketState *s = opaque;
s->rs.state = 0;
s->rs.index = 0;
s->rs.packet_len = 0;
memset(s->rs.buf, 0, sizeof(s->rs.buf));
}
>
>> memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>> return;
>> }
>> buf = buf1;
>> - while (size > 0) {
>> - /* reassemble a packet from the network */
>> - switch(s->state) {
>> - case 0:
>> - l = 4 - s->index;
>> - if (l > size)
>> - l = size;
>> - memcpy(s->buf + s->index, buf, l);
>> - buf += l;
>> - size -= l;
>> - s->index += l;
>> - if (s->index == 4) {
>> - /* got length */
>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>> - s->index = 0;
>> - s->state = 1;
>> - }
>> - break;
>> - case 1:
>> - l = s->packet_len - s->index;
>> - if (l > size)
>> - l = size;
>> - if (s->index + l <= sizeof(s->buf)) {
>> - memcpy(s->buf + s->index, buf, l);
>> - } else {
>> - fprintf(stderr, "serious error: oversized packet
>> received,"
>> - "connection terminated.\n");
>> - s->state = 0;
>> - goto eoc;
>> - }
>> - s->index += l;
>> - buf += l;
>> - size -= l;
>> - if (s->index >= s->packet_len) {
>> - s->index = 0;
>> - s->state = 0;
>> - if (qemu_send_packet_async(&s->nc, s->buf,
>> s->packet_len,
>> - net_socket_send_completed) == 0) {
>> - net_socket_read_poll(s, false);
>> - break;
>> - }
>> - }
>> - break;
>> + ret = net_fill_rstate(&s->rs, buf, size);
>> +
>> + if (ret == -1) {
>> + goto eoc;
>> + } else if (ret == 1) {
>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>> + s->rs.packet_len,
>> + net_socket_send_completed) == 0) {
>> + net_socket_read_poll(s, false);
>
> This looks not elegant, maybe we could use callback (which was
> initialized by the helper I mention above) to do this. Any thoughts on
> this?
Do you mean:
remove
+ if (qemu_send_packet_async(&s->nc, s->rs.buf,
+ s->rs.packet_len,
+ net_socket_send_completed) == 0) {
+ net_socket_read_poll(s, false);
add
s->rs->done
void socket_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
{
NetSocketState *s = opaque;
if (qemu_send_packet_async(&s->nc, srs->buf,
srs->packet_len,
net_socket_send_completed) == 0) {
net_socket_read_poll(s, false);
}
}
>
>> }
>> }
>> }
>> @@ -229,7 +192,7 @@ static void net_socket_send_dgram(void *opaque)
>> NetSocketState *s = opaque;
>> int size;
>> - size = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
>> + size = qemu_recv(s->fd, s->rs.buf, sizeof(s->rs.buf), 0);
>> if (size < 0)
>> return;
>> if (size == 0) {
>> @@ -238,7 +201,7 @@ static void net_socket_send_dgram(void *opaque)
>> net_socket_write_poll(s, false);
>> return;
>> }
>> - if (qemu_send_packet_async(&s->nc, s->buf, size,
>> + if (qemu_send_packet_async(&s->nc, s->rs.buf, size,
>> net_socket_send_completed) == 0) {
>> net_socket_read_poll(s, false);
>> }
>
>
>
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-11 11:20 ` Zhang Chen
@ 2016-05-12 1:11 ` Jason Wang
2016-05-12 6:33 ` Zhang Chen
0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2016-05-12 1:11 UTC (permalink / raw)
To: Zhang Chen, qemu devel; +Cc: eddie . dong, Li Zhijian, Dr . David Alan Gilbert
On 2016年05月11日 19:20, Zhang Chen wrote:
>
>
> On 05/11/2016 05:01 PM, Jason Wang wrote:
>>
>>
>> On 2016年05月06日 18:56, Zhang Chen wrote:
>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>
>> Looks good, just few nits.
>>
>> It's better to have a commit log.
>
> OK, I will add this log:
>
> This function is from net/socket.c, move it to net.c and net.h.
> Add SocketReadState to make others reuse net_fill_rstate().
> suggestion from jason.
Looks good. Thanks
>
>>
>>> ---
>>> include/net/net.h | 8 ++++++
>>> net/filter-mirror.c | 60 ++++++++------------------------------------
>>> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>>> net/socket.c | 71
>>> +++++++++++++----------------------------------------
>>> 4 files changed, 91 insertions(+), 104 deletions(-)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 73e4c46..1439cf9 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -102,6 +102,14 @@ typedef struct NICState {
>>> bool peer_deleted;
>>> } NICState;
>>> +typedef struct ReadState {
>>> + int state; /* 0 = getting length, 1 = getting data */
>>> + uint32_t index;
>>> + uint32_t packet_len;
>>> + uint8_t buf[NET_BUFSIZE];
>>> +} ReadState;
>>
>> I think SocketReadState is better here.
>>
>
> I will rename it in next version.
>
>>> +
>>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
>>> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>>> NetClientState *qemu_find_netdev(const char *id);
>>> int qemu_find_net_clients_except(const char *id, NetClientState
>>> **ncs,
>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>> index c0c4dc6..bcec509 100644
>>> --- a/net/filter-mirror.c
>>> +++ b/net/filter-mirror.c
>>> @@ -40,10 +40,7 @@ typedef struct MirrorState {
>>> char *outdev;
>>> CharDriverState *chr_in;
>>> CharDriverState *chr_out;
>>> - int state; /* 0 = getting length, 1 = getting data */
>>> - unsigned int index;
>>> - unsigned int packet_len;
>>> - uint8_t buf[REDIRECTOR_MAX_LEN];
>>> + ReadState rs;
>>> } MirrorState;
>>> static int filter_mirror_send(CharDriverState *chr_out,
>>> @@ -108,51 +105,14 @@ static void redirector_chr_read(void *opaque,
>>> const uint8_t *buf, int size)
>>> {
>>> NetFilterState *nf = opaque;
>>> MirrorState *s = FILTER_REDIRECTOR(nf);
>>> - unsigned int l;
>>> -
>>> - while (size > 0) {
>>> - /* reassemble a packet from the network */
>>> - switch (s->state) { /* 0 = getting length, 1 = getting data */
>>> - case 0:
>>> - l = 4 - s->index;
>>> - if (l > size) {
>>> - l = size;
>>> - }
>>> - memcpy(s->buf + s->index, buf, l);
>>> - buf += l;
>>> - size -= l;
>>> - s->index += l;
>>> - if (s->index == 4) {
>>> - /* got length */
>>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>>> - s->index = 0;
>>> - s->state = 1;
>>> - }
>>> - break;
>>> - case 1:
>>> - l = s->packet_len - s->index;
>>> - if (l > size) {
>>> - l = size;
>>> - }
>>> - if (s->index + l <= sizeof(s->buf)) {
>>> - memcpy(s->buf + s->index, buf, l);
>>> - } else {
>>> - error_report("serious error: oversized packet
>>> received.");
>>> - s->index = s->state = 0;
>>> - qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL,
>>> NULL);
>>> - return;
>>> - }
>>> -
>>> - s->index += l;
>>> - buf += l;
>>> - size -= l;
>>> - if (s->index >= s->packet_len) {
>>> - s->index = 0;
>>> - s->state = 0;
>>> - redirector_to_filter(nf, s->buf, s->packet_len);
>>> - }
>>> - break;
>>> - }
>>> + int ret;
>>> +
>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>> +
>>> + if (ret == -1) {
>>> + qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
>>> + } else if (ret == 1) {
>>> + redirector_to_filter(nf, s->rs.buf, s->rs.packet_len);
>>> }
>>> }
>>> @@ -274,7 +234,7 @@ static void
>>> filter_redirector_setup(NetFilterState *nf, Error **errp)
>>> }
>>> }
>>> - s->state = s->index = 0;
>>> + s->rs.state = s->rs.index = 0;
>>> if (s->indev) {
>>> s->chr_in = qemu_chr_find(s->indev);
>>> diff --git a/net/net.c b/net/net.c
>>> index 0ad6217..926457e 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -1573,3 +1573,59 @@ QemuOptsList qemu_net_opts = {
>>> { /* end of list */ }
>>> },
>>> };
>>> +
>>> +/*
>>> + * Returns
>>> + * 0: readstate is not ready
>>> + * 1: readstate is ready
>>> + * otherwise error occurs
>>> + */
>>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size)
>>> +{
>>> + unsigned int l;
>>> + while (size > 0) {
>>> + /* reassemble a packet from the network */
>>> + switch (rs->state) { /* 0 = getting length, 1 = getting
>>> data */
>>> + case 0:
>>> + l = 4 - rs->index;
>>> + if (l > size) {
>>> + l = size;
>>> + }
>>> + memcpy(rs->buf + rs->index, buf, l);
>>> + buf += l;
>>> + size -= l;
>>> + rs->index += l;
>>> + if (rs->index == 4) {
>>> + /* got length */
>>> + rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>>> + rs->index = 0;
>>> + rs->state = 1;
>>> + }
>>> + break;
>>> + case 1:
>>> + l = rs->packet_len - rs->index;
>>> + if (l > size) {
>>> + l = size;
>>> + }
>>> + if (rs->index + l <= sizeof(rs->buf)) {
>>> + memcpy(rs->buf + rs->index, buf, l);
>>> + } else {
>>> + fprintf(stderr, "serious error: oversized packet
>>> received,"
>>> + "connection terminated.\n");
>>> + rs->index = rs->state = 0;
>>> + return -1;
>>> + }
>>> +
>>> + rs->index += l;
>>> + buf += l;
>>> + size -= l;
>>> + if (rs->index >= rs->packet_len) {
>>> + rs->index = 0;
>>> + rs->state = 0;
>>> + return 1;
>>> + }
>>> + break;
>>> + }
>>> + }
>>> + return 0;
>>> +}
>>> diff --git a/net/socket.c b/net/socket.c
>>> index 9fa2cd8..9ecdd3b 100644
>>> --- a/net/socket.c
>>> +++ b/net/socket.c
>>> @@ -38,11 +38,8 @@ typedef struct NetSocketState {
>>> NetClientState nc;
>>> int listen_fd;
>>> int fd;
>>> - int state; /* 0 = getting length, 1 = getting data */
>>> - unsigned int index;
>>> - unsigned int packet_len;
>>> + ReadState rs;
>>> unsigned int send_index; /* number of bytes sent (only
>>> SOCK_STREAM) */
>>> - uint8_t buf[NET_BUFSIZE];
>>> struct sockaddr_in dgram_dst; /* contains inet host and port
>>> destination iff connectionless (SOCK_DGRAM) */
>>> IOHandler *send_fn; /* differs between
>>> SOCK_STREAM/SOCK_DGRAM */
>>> bool read_poll; /* waiting to receive data? */
>>> @@ -147,7 +144,7 @@ static void net_socket_send(void *opaque)
>>> {
>>> NetSocketState *s = opaque;
>>> int size;
>>> - unsigned l;
>>> + int ret;
>>> uint8_t buf1[NET_BUFSIZE];
>>> const uint8_t *buf;
>>> @@ -166,60 +163,26 @@ static void net_socket_send(void *opaque)
>>> closesocket(s->fd);
>>> s->fd = -1;
>>> - s->state = 0;
>>> - s->index = 0;
>>> - s->packet_len = 0;
>>> + s->rs.state = 0;
>>> + s->rs.index = 0;
>>> + s->rs.packet_len = 0;
>>> s->nc.link_down = true;
>>> - memset(s->buf, 0, sizeof(s->buf));
>>> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
>>
>> How about introduce a helper to do the initialization?
>
> Do you mean
>
> remove
> + s->rs.state = 0;
> + s->rs.index = 0;
> + s->rs.packet_len = 0;
> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
>
> add
> s->rs->cb = xxx_cb;
> s->rs->opxx = xxx;
>
> void xxx_cb(void *opaque)
> {
> NetSocketState *s = opaque;
>
> s->rs.state = 0;
> s->rs.index = 0;
> s->rs.packet_len = 0;
> memset(s->rs.buf, 0, sizeof(s->rs.buf));
> }
>
Kind of, and looks like there's no need for opaque, you can just pass
pointer to SocketReadState. And another parameter of callbacks.
>
>>
>>> memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>>> return;
>>> }
>>> buf = buf1;
>>> - while (size > 0) {
>>> - /* reassemble a packet from the network */
>>> - switch(s->state) {
>>> - case 0:
>>> - l = 4 - s->index;
>>> - if (l > size)
>>> - l = size;
>>> - memcpy(s->buf + s->index, buf, l);
>>> - buf += l;
>>> - size -= l;
>>> - s->index += l;
>>> - if (s->index == 4) {
>>> - /* got length */
>>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>>> - s->index = 0;
>>> - s->state = 1;
>>> - }
>>> - break;
>>> - case 1:
>>> - l = s->packet_len - s->index;
>>> - if (l > size)
>>> - l = size;
>>> - if (s->index + l <= sizeof(s->buf)) {
>>> - memcpy(s->buf + s->index, buf, l);
>>> - } else {
>>> - fprintf(stderr, "serious error: oversized packet
>>> received,"
>>> - "connection terminated.\n");
>>> - s->state = 0;
>>> - goto eoc;
>>> - }
>>> - s->index += l;
>>> - buf += l;
>>> - size -= l;
>>> - if (s->index >= s->packet_len) {
>>> - s->index = 0;
>>> - s->state = 0;
>>> - if (qemu_send_packet_async(&s->nc, s->buf,
>>> s->packet_len,
>>> - net_socket_send_completed) == 0) {
>>> - net_socket_read_poll(s, false);
>>> - break;
>>> - }
>>> - }
>>> - break;
>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>> +
>>> + if (ret == -1) {
>>> + goto eoc;
>>> + } else if (ret == 1) {
>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>> + s->rs.packet_len,
>>> + net_socket_send_completed) == 0) {
>>> + net_socket_read_poll(s, false);
>>
>> This looks not elegant, maybe we could use callback (which was
>> initialized by the helper I mention above) to do this. Any thoughts
>> on this?
>
> Do you mean:
>
> remove
> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
> + s->rs.packet_len,
> + net_socket_send_completed) == 0) {
> + net_socket_read_poll(s, false);
>
> add
>
> s->rs->done
>
> void socket_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
> {
> NetSocketState *s = opaque;
>
> if (qemu_send_packet_async(&s->nc, srs->buf,
> srs->packet_len,
> net_socket_send_completed) == 0) {
> net_socket_read_poll(s, false);
> }
> }
Yes, but there's no need for opaque, we can infer the container by
container_of().
>
>>
>>> }
>>> }
>>> }
>>> @@ -229,7 +192,7 @@ static void net_socket_send_dgram(void *opaque)
>>> NetSocketState *s = opaque;
>>> int size;
>>> - size = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
>>> + size = qemu_recv(s->fd, s->rs.buf, sizeof(s->rs.buf), 0);
>>> if (size < 0)
>>> return;
>>> if (size == 0) {
>>> @@ -238,7 +201,7 @@ static void net_socket_send_dgram(void *opaque)
>>> net_socket_write_poll(s, false);
>>> return;
>>> }
>>> - if (qemu_send_packet_async(&s->nc, s->buf, size,
>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf, size,
>>> net_socket_send_completed) == 0) {
>>> net_socket_read_poll(s, false);
>>> }
>>
>>
>>
>> .
>>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-12 1:11 ` Jason Wang
@ 2016-05-12 6:33 ` Zhang Chen
2016-05-12 8:07 ` Jason Wang
0 siblings, 1 reply; 10+ messages in thread
From: Zhang Chen @ 2016-05-12 6:33 UTC (permalink / raw)
To: Jason Wang, qemu devel; +Cc: eddie . dong, Li Zhijian, Dr . David Alan Gilbert
On 05/12/2016 09:11 AM, Jason Wang wrote:
>
>
> On 2016年05月11日 19:20, Zhang Chen wrote:
>>
>>
>> On 05/11/2016 05:01 PM, Jason Wang wrote:
>>>
>>>
>>> On 2016年05月06日 18:56, Zhang Chen wrote:
>>>> Signed-off-by: Zhang Chen <zhangchen.fnst@cn.fujitsu.com>
>>>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>
>>> Looks good, just few nits.
>>>
>>> It's better to have a commit log.
>>
>> OK, I will add this log:
>>
>> This function is from net/socket.c, move it to net.c and net.h.
>> Add SocketReadState to make others reuse net_fill_rstate().
>> suggestion from jason.
>
> Looks good. Thanks
>
>>
>>>
>>>> ---
>>>> include/net/net.h | 8 ++++++
>>>> net/filter-mirror.c | 60
>>>> ++++++++------------------------------------
>>>> net/net.c | 56 ++++++++++++++++++++++++++++++++++++++++++
>>>> net/socket.c | 71
>>>> +++++++++++++----------------------------------------
>>>> 4 files changed, 91 insertions(+), 104 deletions(-)
>>>>
>>>> diff --git a/include/net/net.h b/include/net/net.h
>>>> index 73e4c46..1439cf9 100644
>>>> --- a/include/net/net.h
>>>> +++ b/include/net/net.h
>>>> @@ -102,6 +102,14 @@ typedef struct NICState {
>>>> bool peer_deleted;
>>>> } NICState;
>>>> +typedef struct ReadState {
>>>> + int state; /* 0 = getting length, 1 = getting data */
>>>> + uint32_t index;
>>>> + uint32_t packet_len;
>>>> + uint8_t buf[NET_BUFSIZE];
>>>> +} ReadState;
>>>
>>> I think SocketReadState is better here.
>>>
>>
>> I will rename it in next version.
>>
>>>> +
>>>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size);
>>>> char *qemu_mac_strdup_printf(const uint8_t *macaddr);
>>>> NetClientState *qemu_find_netdev(const char *id);
>>>> int qemu_find_net_clients_except(const char *id, NetClientState
>>>> **ncs,
>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
>>>> index c0c4dc6..bcec509 100644
>>>> --- a/net/filter-mirror.c
>>>> +++ b/net/filter-mirror.c
>>>> @@ -40,10 +40,7 @@ typedef struct MirrorState {
>>>> char *outdev;
>>>> CharDriverState *chr_in;
>>>> CharDriverState *chr_out;
>>>> - int state; /* 0 = getting length, 1 = getting data */
>>>> - unsigned int index;
>>>> - unsigned int packet_len;
>>>> - uint8_t buf[REDIRECTOR_MAX_LEN];
>>>> + ReadState rs;
>>>> } MirrorState;
>>>> static int filter_mirror_send(CharDriverState *chr_out,
>>>> @@ -108,51 +105,14 @@ static void redirector_chr_read(void *opaque,
>>>> const uint8_t *buf, int size)
>>>> {
>>>> NetFilterState *nf = opaque;
>>>> MirrorState *s = FILTER_REDIRECTOR(nf);
>>>> - unsigned int l;
>>>> -
>>>> - while (size > 0) {
>>>> - /* reassemble a packet from the network */
>>>> - switch (s->state) { /* 0 = getting length, 1 = getting
>>>> data */
>>>> - case 0:
>>>> - l = 4 - s->index;
>>>> - if (l > size) {
>>>> - l = size;
>>>> - }
>>>> - memcpy(s->buf + s->index, buf, l);
>>>> - buf += l;
>>>> - size -= l;
>>>> - s->index += l;
>>>> - if (s->index == 4) {
>>>> - /* got length */
>>>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>>>> - s->index = 0;
>>>> - s->state = 1;
>>>> - }
>>>> - break;
>>>> - case 1:
>>>> - l = s->packet_len - s->index;
>>>> - if (l > size) {
>>>> - l = size;
>>>> - }
>>>> - if (s->index + l <= sizeof(s->buf)) {
>>>> - memcpy(s->buf + s->index, buf, l);
>>>> - } else {
>>>> - error_report("serious error: oversized packet
>>>> received.");
>>>> - s->index = s->state = 0;
>>>> - qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL,
>>>> NULL);
>>>> - return;
>>>> - }
>>>> -
>>>> - s->index += l;
>>>> - buf += l;
>>>> - size -= l;
>>>> - if (s->index >= s->packet_len) {
>>>> - s->index = 0;
>>>> - s->state = 0;
>>>> - redirector_to_filter(nf, s->buf, s->packet_len);
>>>> - }
>>>> - break;
>>>> - }
>>>> + int ret;
>>>> +
>>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>>> +
>>>> + if (ret == -1) {
>>>> + qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL);
>>>> + } else if (ret == 1) {
>>>> + redirector_to_filter(nf, s->rs.buf, s->rs.packet_len);
>>>> }
>>>> }
>>>> @@ -274,7 +234,7 @@ static void
>>>> filter_redirector_setup(NetFilterState *nf, Error **errp)
>>>> }
>>>> }
>>>> - s->state = s->index = 0;
>>>> + s->rs.state = s->rs.index = 0;
>>>> if (s->indev) {
>>>> s->chr_in = qemu_chr_find(s->indev);
>>>> diff --git a/net/net.c b/net/net.c
>>>> index 0ad6217..926457e 100644
>>>> --- a/net/net.c
>>>> +++ b/net/net.c
>>>> @@ -1573,3 +1573,59 @@ QemuOptsList qemu_net_opts = {
>>>> { /* end of list */ }
>>>> },
>>>> };
>>>> +
>>>> +/*
>>>> + * Returns
>>>> + * 0: readstate is not ready
>>>> + * 1: readstate is ready
>>>> + * otherwise error occurs
>>>> + */
>>>> +int net_fill_rstate(ReadState *rs, const uint8_t *buf, int size)
>>>> +{
>>>> + unsigned int l;
>>>> + while (size > 0) {
>>>> + /* reassemble a packet from the network */
>>>> + switch (rs->state) { /* 0 = getting length, 1 = getting
>>>> data */
>>>> + case 0:
>>>> + l = 4 - rs->index;
>>>> + if (l > size) {
>>>> + l = size;
>>>> + }
>>>> + memcpy(rs->buf + rs->index, buf, l);
>>>> + buf += l;
>>>> + size -= l;
>>>> + rs->index += l;
>>>> + if (rs->index == 4) {
>>>> + /* got length */
>>>> + rs->packet_len = ntohl(*(uint32_t *)rs->buf);
>>>> + rs->index = 0;
>>>> + rs->state = 1;
>>>> + }
>>>> + break;
>>>> + case 1:
>>>> + l = rs->packet_len - rs->index;
>>>> + if (l > size) {
>>>> + l = size;
>>>> + }
>>>> + if (rs->index + l <= sizeof(rs->buf)) {
>>>> + memcpy(rs->buf + rs->index, buf, l);
>>>> + } else {
>>>> + fprintf(stderr, "serious error: oversized packet
>>>> received,"
>>>> + "connection terminated.\n");
>>>> + rs->index = rs->state = 0;
>>>> + return -1;
>>>> + }
>>>> +
>>>> + rs->index += l;
>>>> + buf += l;
>>>> + size -= l;
>>>> + if (rs->index >= rs->packet_len) {
>>>> + rs->index = 0;
>>>> + rs->state = 0;
>>>> + return 1;
>>>> + }
>>>> + break;
>>>> + }
>>>> + }
>>>> + return 0;
>>>> +}
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index 9fa2cd8..9ecdd3b 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -38,11 +38,8 @@ typedef struct NetSocketState {
>>>> NetClientState nc;
>>>> int listen_fd;
>>>> int fd;
>>>> - int state; /* 0 = getting length, 1 = getting data */
>>>> - unsigned int index;
>>>> - unsigned int packet_len;
>>>> + ReadState rs;
>>>> unsigned int send_index; /* number of bytes sent (only
>>>> SOCK_STREAM) */
>>>> - uint8_t buf[NET_BUFSIZE];
>>>> struct sockaddr_in dgram_dst; /* contains inet host and port
>>>> destination iff connectionless (SOCK_DGRAM) */
>>>> IOHandler *send_fn; /* differs between
>>>> SOCK_STREAM/SOCK_DGRAM */
>>>> bool read_poll; /* waiting to receive data? */
>>>> @@ -147,7 +144,7 @@ static void net_socket_send(void *opaque)
>>>> {
>>>> NetSocketState *s = opaque;
>>>> int size;
>>>> - unsigned l;
>>>> + int ret;
>>>> uint8_t buf1[NET_BUFSIZE];
>>>> const uint8_t *buf;
>>>> @@ -166,60 +163,26 @@ static void net_socket_send(void *opaque)
>>>> closesocket(s->fd);
>>>> s->fd = -1;
>>>> - s->state = 0;
>>>> - s->index = 0;
>>>> - s->packet_len = 0;
>>>> + s->rs.state = 0;
>>>> + s->rs.index = 0;
>>>> + s->rs.packet_len = 0;
>>>> s->nc.link_down = true;
>>>> - memset(s->buf, 0, sizeof(s->buf));
>>>> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
>>>
>>> How about introduce a helper to do the initialization?
>>
>> Do you mean
>>
>> remove
>> + s->rs.state = 0;
>> + s->rs.index = 0;
>> + s->rs.packet_len = 0;
>> + memset(s->rs.buf, 0, sizeof(s->rs.buf));
>>
>> add
>> s->rs->cb = xxx_cb;
>> s->rs->opxx = xxx;
>>
>> void xxx_cb(void *opaque)
>> {
>> NetSocketState *s = opaque;
>>
>> s->rs.state = 0;
>> s->rs.index = 0;
>> s->rs.packet_len = 0;
>> memset(s->rs.buf, 0, sizeof(s->rs.buf));
>> }
>>
>
> Kind of, and looks like there's no need for opaque, you can just pass
> pointer to SocketReadState. And another parameter of callbacks.
OK,will fix it.
>
>>
>>>
>>>> memset(s->nc.info_str, 0, sizeof(s->nc.info_str));
>>>> return;
>>>> }
>>>> buf = buf1;
>>>> - while (size > 0) {
>>>> - /* reassemble a packet from the network */
>>>> - switch(s->state) {
>>>> - case 0:
>>>> - l = 4 - s->index;
>>>> - if (l > size)
>>>> - l = size;
>>>> - memcpy(s->buf + s->index, buf, l);
>>>> - buf += l;
>>>> - size -= l;
>>>> - s->index += l;
>>>> - if (s->index == 4) {
>>>> - /* got length */
>>>> - s->packet_len = ntohl(*(uint32_t *)s->buf);
>>>> - s->index = 0;
>>>> - s->state = 1;
>>>> - }
>>>> - break;
>>>> - case 1:
>>>> - l = s->packet_len - s->index;
>>>> - if (l > size)
>>>> - l = size;
>>>> - if (s->index + l <= sizeof(s->buf)) {
>>>> - memcpy(s->buf + s->index, buf, l);
>>>> - } else {
>>>> - fprintf(stderr, "serious error: oversized packet
>>>> received,"
>>>> - "connection terminated.\n");
>>>> - s->state = 0;
>>>> - goto eoc;
>>>> - }
>>>> - s->index += l;
>>>> - buf += l;
>>>> - size -= l;
>>>> - if (s->index >= s->packet_len) {
>>>> - s->index = 0;
>>>> - s->state = 0;
>>>> - if (qemu_send_packet_async(&s->nc, s->buf,
>>>> s->packet_len,
>>>> - net_socket_send_completed) == 0) {
>>>> - net_socket_read_poll(s, false);
>>>> - break;
>>>> - }
>>>> - }
>>>> - break;
>>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>>> +
>>>> + if (ret == -1) {
>>>> + goto eoc;
>>>> + } else if (ret == 1) {
>>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>>> + s->rs.packet_len,
>>>> + net_socket_send_completed) == 0) {
>>>> + net_socket_read_poll(s, false);
>>>
>>> This looks not elegant, maybe we could use callback (which was
>>> initialized by the helper I mention above) to do this. Any thoughts
>>> on this?
>>
>> Do you mean:
>>
>> remove
>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>> + s->rs.packet_len,
>> + net_socket_send_completed) == 0) {
>> + net_socket_read_poll(s, false);
>>
>> add
>>
>> s->rs->done
>>
>> void socket_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
>> {
>> NetSocketState *s = opaque;
>>
>> if (qemu_send_packet_async(&s->nc, srs->buf,
>> srs->packet_len,
>> net_socket_send_completed) == 0) {
>> net_socket_read_poll(s, false);
>> }
>> }
>
> Yes, but there's no need for opaque, we can infer the container by
> container_of().
>
But in filter-mirror.c we need do this:
void redirector_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
{
NetFilterState *nf = opaque;
redirector_to_filter(nf, srs->buf, srs->packet_len);
}
so,I think we have to use void *opaque.
>>
>>>
>>>> }
>>>> }
>>>> }
>>>> @@ -229,7 +192,7 @@ static void net_socket_send_dgram(void *opaque)
>>>> NetSocketState *s = opaque;
>>>> int size;
>>>> - size = qemu_recv(s->fd, s->buf, sizeof(s->buf), 0);
>>>> + size = qemu_recv(s->fd, s->rs.buf, sizeof(s->rs.buf), 0);
>>>> if (size < 0)
>>>> return;
>>>> if (size == 0) {
>>>> @@ -238,7 +201,7 @@ static void net_socket_send_dgram(void *opaque)
>>>> net_socket_write_poll(s, false);
>>>> return;
>>>> }
>>>> - if (qemu_send_packet_async(&s->nc, s->buf, size,
>>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf, size,
>>>> net_socket_send_completed) == 0) {
>>>> net_socket_read_poll(s, false);
>>>> }
>>>
>>>
>>>
>>> .
>>>
>>
>
>
>
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-12 6:33 ` Zhang Chen
@ 2016-05-12 8:07 ` Jason Wang
2016-05-12 8:23 ` Zhang Chen
0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2016-05-12 8:07 UTC (permalink / raw)
To: Zhang Chen, qemu devel; +Cc: eddie . dong, Dr . David Alan Gilbert, Li Zhijian
On 2016年05月12日 14:33, Zhang Chen wrote:
>>>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>>>> +
>>>>> + if (ret == -1) {
>>>>> + goto eoc;
>>>>> + } else if (ret == 1) {
>>>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>>>> + s->rs.packet_len,
>>>>> + net_socket_send_completed) == 0) {
>>>>> + net_socket_read_poll(s, false);
>>>>
>>>> This looks not elegant, maybe we could use callback (which was
>>>> initialized by the helper I mention above) to do this. Any thoughts
>>>> on this?
>>>
>>> Do you mean:
>>>
>>> remove
>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>> + s->rs.packet_len,
>>> + net_socket_send_completed) == 0) {
>>> + net_socket_read_poll(s, false);
>>>
>>> add
>>>
>>> s->rs->done
>>>
>>> void socket_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
>>> {
>>> NetSocketState *s = opaque;
>>>
>>> if (qemu_send_packet_async(&s->nc, srs->buf,
>>> srs->packet_len,
>>> net_socket_send_completed) == 0) {
>>> net_socket_read_poll(s, false);
>>> }
>>> }
>>
>> Yes, but there's no need for opaque, we can infer the container by
>> container_of().
>>
>
> But in filter-mirror.c we need do this:
>
>
> void redirector_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
> {
> NetFilterState *nf = opaque;
>
> redirector_to_filter(nf, srs->buf, srs->packet_len);
> }
>
> so,I think we have to use void *opaque.
>
>
>
You mean you need to get nf? Since SocketReadState were embedded in
MirrorState, so you could get the address of MirrorState, then it's not
hard to get nf address?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes
2016-05-12 8:07 ` Jason Wang
@ 2016-05-12 8:23 ` Zhang Chen
0 siblings, 0 replies; 10+ messages in thread
From: Zhang Chen @ 2016-05-12 8:23 UTC (permalink / raw)
To: Jason Wang, qemu devel; +Cc: eddie . dong, Dr . David Alan Gilbert, Li Zhijian
On 05/12/2016 04:07 PM, Jason Wang wrote:
>
>
> On 2016年05月12日 14:33, Zhang Chen wrote:
>>>>>> + ret = net_fill_rstate(&s->rs, buf, size);
>>>>>> +
>>>>>> + if (ret == -1) {
>>>>>> + goto eoc;
>>>>>> + } else if (ret == 1) {
>>>>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>>>>> + s->rs.packet_len,
>>>>>> + net_socket_send_completed) == 0) {
>>>>>> + net_socket_read_poll(s, false);
>>>>>
>>>>> This looks not elegant, maybe we could use callback (which was
>>>>> initialized by the helper I mention above) to do this. Any
>>>>> thoughts on this?
>>>>
>>>> Do you mean:
>>>>
>>>> remove
>>>> + if (qemu_send_packet_async(&s->nc, s->rs.buf,
>>>> + s->rs.packet_len,
>>>> + net_socket_send_completed) == 0) {
>>>> + net_socket_read_poll(s, false);
>>>>
>>>> add
>>>>
>>>> s->rs->done
>>>>
>>>> void socket_fill_rsstate_done_cb(SocketReadState *srs, void *opaque)
>>>> {
>>>> NetSocketState *s = opaque;
>>>>
>>>> if (qemu_send_packet_async(&s->nc, srs->buf,
>>>> srs->packet_len,
>>>> net_socket_send_completed) == 0) {
>>>> net_socket_read_poll(s, false);
>>>> }
>>>> }
>>>
>>> Yes, but there's no need for opaque, we can infer the container by
>>> container_of().
>>>
>>
>> But in filter-mirror.c we need do this:
>>
>>
>> void redirector_fill_rsstate_done_cb(SocketReadState *srs, void
>> *opaque)
>> {
>> NetFilterState *nf = opaque;
>>
>> redirector_to_filter(nf, srs->buf, srs->packet_len);
>> }
>>
>> so,I think we have to use void *opaque.
>>
>>
>>
>
> You mean you need to get nf? Since SocketReadState were embedded in
> MirrorState, so you could get the address of MirrorState, then it's
> not hard to get nf address?
>
>
Got it~~
will fix it in next version.
Thanks
Zhang Chen
> .
>
--
Thanks
zhangchen
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-12 8:24 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-06 10:56 [Qemu-devel] [PATCH] net/net: Add ReadState for reuse codes Zhang Chen
2016-05-10 2:57 ` Li Zhijian
2016-05-10 5:33 ` Zhang Chen
2016-05-11 7:01 ` Zhang Chen
2016-05-11 9:01 ` Jason Wang
2016-05-11 11:20 ` Zhang Chen
2016-05-12 1:11 ` Jason Wang
2016-05-12 6:33 ` Zhang Chen
2016-05-12 8:07 ` Jason Wang
2016-05-12 8:23 ` Zhang Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.