* Re: [Qemu-devel] [PATCH V2] net/net: Add SocketReadState for reuse codes [not found] <1463044750-10424-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> @ 2016-05-13 4:02 ` Jason Wang 2016-05-13 6:10 ` Zhang Chen 0 siblings, 1 reply; 2+ messages in thread From: Jason Wang @ 2016-05-13 4:02 UTC (permalink / raw) To: Zhang Chen, qemu devel Cc: Li Zhijian, Wen Congyang, eddie . dong, Dr . David Alan Gilbert On 2016年05月12日 17:19, Zhang Chen wrote: > 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. > > v2: > - rename ReadState to SocketReadState > - add SocketReadState init and finalize callback > > v1: > - init patch > > 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 | 14 +++++++++ > net/filter-mirror.c | 72 ++++++++++++++----------------------------- > net/net.c | 65 ++++++++++++++++++++++++++++++++++++++ > net/socket.c | 89 +++++++++++++++++++++-------------------------------- > 4 files changed, 137 insertions(+), 103 deletions(-) > > diff --git a/include/net/net.h b/include/net/net.h > index 73e4c46..4f6b6bf 100644 > --- a/include/net/net.h > +++ b/include/net/net.h > @@ -57,6 +57,9 @@ typedef void (SetOffload)(NetClientState *, int, int, int, int, int); > typedef void (SetVnetHdrLen)(NetClientState *, int); > typedef int (SetVnetLE)(NetClientState *, bool); > typedef int (SetVnetBE)(NetClientState *, bool); > +typedef struct SocketReadState SocketReadState; > +typedef void (SocketReadStateInit)(SocketReadState *rs); > +typedef void (SocketReadStateFinalize)(SocketReadState *rs); > > typedef struct NetClientInfo { > NetClientOptionsKind type; > @@ -102,6 +105,16 @@ typedef struct NICState { > bool peer_deleted; > } NICState; > > +struct SocketReadState { > + int state; /* 0 = getting length, 1 = getting data */ > + uint32_t index; > + uint32_t packet_len; > + uint8_t buf[NET_BUFSIZE]; > + SocketReadStateInit *init; > + SocketReadStateFinalize *finalize; > +}; > + > +int net_fill_rstate(SocketReadState *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, > @@ -160,6 +173,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState *sender, > > void print_net_client(Monitor *mon, NetClientState *nc); > void hmp_info_network(Monitor *mon, const QDict *qdict); > +void net_socket_rs_init(SocketReadState *rs); > > /* NIC info */ > > diff --git a/net/filter-mirror.c b/net/filter-mirror.c > index c0c4dc6..4e55924 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]; > + SocketReadState rs; > } MirrorState; > > static int filter_mirror_send(CharDriverState *chr_out, > @@ -108,50 +105,15 @@ 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) { > + if (s->rs.finalize) { > + s->rs.finalize(&s->rs); Why not simply call this in net_fill_rstate()? > } > } > } > @@ -258,6 +220,14 @@ static void filter_mirror_setup(NetFilterState *nf, Error **errp) > } > } > > +static void redirector_rs_finalize(SocketReadState *rs) > +{ > + MirrorState *s = container_of(rs, MirrorState, rs); > + NetFilterState *nf = NETFILTER(s); > + > + redirector_to_filter(nf, rs->buf, rs->packet_len); > +} > + > static void filter_redirector_setup(NetFilterState *nf, Error **errp) > { > MirrorState *s = FILTER_REDIRECTOR(nf); > @@ -274,7 +244,11 @@ static void filter_redirector_setup(NetFilterState *nf, Error **errp) > } > } > > - s->state = s->index = 0; > + s->rs.init = net_socket_rs_init; > + s->rs.finalize = redirector_rs_finalize; > + if (s->rs.init) { > + s->rs.init(&s->rs); > + } This looks odd, you can call net_socket_rs_init() directly here and pass redirector_rs_finalize to it. Then there's no need to do the open code here. ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH V2] net/net: Add SocketReadState for reuse codes 2016-05-13 4:02 ` [Qemu-devel] [PATCH V2] net/net: Add SocketReadState for reuse codes Jason Wang @ 2016-05-13 6:10 ` Zhang Chen 0 siblings, 0 replies; 2+ messages in thread From: Zhang Chen @ 2016-05-13 6:10 UTC (permalink / raw) To: Jason Wang, qemu devel Cc: Li Zhijian, Wen Congyang, eddie . dong, Dr . David Alan Gilbert On 05/13/2016 12:02 PM, Jason Wang wrote: > > > On 2016年05月12日 17:19, Zhang Chen wrote: >> 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. >> >> v2: >> - rename ReadState to SocketReadState >> - add SocketReadState init and finalize callback >> >> v1: >> - init patch >> >> 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 | 14 +++++++++ >> net/filter-mirror.c | 72 ++++++++++++++----------------------------- >> net/net.c | 65 ++++++++++++++++++++++++++++++++++++++ >> net/socket.c | 89 >> +++++++++++++++++++++-------------------------------- >> 4 files changed, 137 insertions(+), 103 deletions(-) >> >> diff --git a/include/net/net.h b/include/net/net.h >> index 73e4c46..4f6b6bf 100644 >> --- a/include/net/net.h >> +++ b/include/net/net.h >> @@ -57,6 +57,9 @@ typedef void (SetOffload)(NetClientState *, int, >> int, int, int, int); >> typedef void (SetVnetHdrLen)(NetClientState *, int); >> typedef int (SetVnetLE)(NetClientState *, bool); >> typedef int (SetVnetBE)(NetClientState *, bool); >> +typedef struct SocketReadState SocketReadState; >> +typedef void (SocketReadStateInit)(SocketReadState *rs); >> +typedef void (SocketReadStateFinalize)(SocketReadState *rs); >> typedef struct NetClientInfo { >> NetClientOptionsKind type; >> @@ -102,6 +105,16 @@ typedef struct NICState { >> bool peer_deleted; >> } NICState; >> +struct SocketReadState { >> + int state; /* 0 = getting length, 1 = getting data */ >> + uint32_t index; >> + uint32_t packet_len; >> + uint8_t buf[NET_BUFSIZE]; >> + SocketReadStateInit *init; >> + SocketReadStateFinalize *finalize; >> +}; >> + >> +int net_fill_rstate(SocketReadState *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, >> @@ -160,6 +173,7 @@ ssize_t qemu_deliver_packet_iov(NetClientState >> *sender, >> void print_net_client(Monitor *mon, NetClientState *nc); >> void hmp_info_network(Monitor *mon, const QDict *qdict); >> +void net_socket_rs_init(SocketReadState *rs); >> /* NIC info */ >> diff --git a/net/filter-mirror.c b/net/filter-mirror.c >> index c0c4dc6..4e55924 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]; >> + SocketReadState rs; >> } MirrorState; >> static int filter_mirror_send(CharDriverState *chr_out, >> @@ -108,50 +105,15 @@ 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) { >> + if (s->rs.finalize) { >> + s->rs.finalize(&s->rs); > > Why not simply call this in net_fill_rstate()? Good idea. I will move it in next version. > >> } >> } >> } >> @@ -258,6 +220,14 @@ static void filter_mirror_setup(NetFilterState >> *nf, Error **errp) >> } >> } >> +static void redirector_rs_finalize(SocketReadState *rs) >> +{ >> + MirrorState *s = container_of(rs, MirrorState, rs); >> + NetFilterState *nf = NETFILTER(s); >> + >> + redirector_to_filter(nf, rs->buf, rs->packet_len); >> +} >> + >> static void filter_redirector_setup(NetFilterState *nf, Error **errp) >> { >> MirrorState *s = FILTER_REDIRECTOR(nf); >> @@ -274,7 +244,11 @@ static void >> filter_redirector_setup(NetFilterState *nf, Error **errp) >> } >> } >> - s->state = s->index = 0; >> + s->rs.init = net_socket_rs_init; >> + s->rs.finalize = redirector_rs_finalize; >> + if (s->rs.init) { >> + s->rs.init(&s->rs); >> + } > > This looks odd, you can call net_socket_rs_init() directly here and > pass redirector_rs_finalize to it. Then there's no need to do the open > code here. > > OK Thanks Zhang Chen > > . > -- Thanks zhangchen ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-05-13 8:18 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1463044750-10424-1-git-send-email-zhangchen.fnst@cn.fujitsu.com>
2016-05-13 4:02 ` [Qemu-devel] [PATCH V2] net/net: Add SocketReadState for reuse codes Jason Wang
2016-05-13 6:10 ` 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.