From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34218) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b14Io-00087K-AD for qemu-devel@nongnu.org; Fri, 13 May 2016 00:02:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b14Ik-0007nr-0a for qemu-devel@nongnu.org; Fri, 13 May 2016 00:02:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b14Ij-0007nc-O9 for qemu-devel@nongnu.org; Fri, 13 May 2016 00:02:09 -0400 References: <1463044750-10424-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> From: Jason Wang Message-ID: <573551BA.4030809@redhat.com> Date: Fri, 13 May 2016 12:02:02 +0800 MIME-Version: 1.0 In-Reply-To: <1463044750-10424-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2] net/net: Add SocketReadState for reuse codes List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Chen , qemu devel Cc: Li Zhijian , Wen Congyang , "eddie . dong" , "Dr . David Alan Gilbert" On 2016=E5=B9=B405=E6=9C=8812=E6=97=A5 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 > Signed-off-by: Li Zhijian > Signed-off-by: Wen Congyang > --- > 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); > =20 > typedef struct NetClientInfo { > NetClientOptionsKind type; > @@ -102,6 +105,16 @@ typedef struct NICState { > bool peer_deleted; > } NICState; > =20 > +struct SocketReadState { > + int state; /* 0 =3D getting length, 1 =3D 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 *sen= der, > =20 > void print_net_client(Monitor *mon, NetClientState *nc); > void hmp_info_network(Monitor *mon, const QDict *qdict); > +void net_socket_rs_init(SocketReadState *rs); > =20 > /* NIC info */ > =20 > 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 =3D getting length, 1 =3D getting data */ > - unsigned int index; > - unsigned int packet_len; > - uint8_t buf[REDIRECTOR_MAX_LEN]; > + SocketReadState rs; > } MirrorState; > =20 > static int filter_mirror_send(CharDriverState *chr_out, > @@ -108,50 +105,15 @@ static void redirector_chr_read(void *opaque, con= st uint8_t *buf, int size) > { > NetFilterState *nf =3D opaque; > MirrorState *s =3D FILTER_REDIRECTOR(nf); > - unsigned int l; > - > - while (size > 0) { > - /* reassemble a packet from the network */ > - switch (s->state) { /* 0 =3D getting length, 1 =3D getting dat= a */ > - case 0: > - l =3D 4 - s->index; > - if (l > size) { > - l =3D size; > - } > - memcpy(s->buf + s->index, buf, l); > - buf +=3D l; > - size -=3D l; > - s->index +=3D l; > - if (s->index =3D=3D 4) { > - /* got length */ > - s->packet_len =3D ntohl(*(uint32_t *)s->buf); > - s->index =3D 0; > - s->state =3D 1; > - } > - break; > - case 1: > - l =3D s->packet_len - s->index; > - if (l > size) { > - l =3D size; > - } > - if (s->index + l <=3D sizeof(s->buf)) { > - memcpy(s->buf + s->index, buf, l); > - } else { > - error_report("serious error: oversized packet received= ."); > - s->index =3D s->state =3D 0; > - qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NUL= L); > - return; > - } > - > - s->index +=3D l; > - buf +=3D l; > - size -=3D l; > - if (s->index >=3D s->packet_len) { > - s->index =3D 0; > - s->state =3D 0; > - redirector_to_filter(nf, s->buf, s->packet_len); > - } > - break; > + int ret; > + > + ret =3D net_fill_rstate(&s->rs, buf, size); > + > + if (ret =3D=3D -1) { > + qemu_chr_add_handlers(s->chr_in, NULL, NULL, NULL, NULL); > + } else if (ret =3D=3D 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) > } > } > =20 > +static void redirector_rs_finalize(SocketReadState *rs) > +{ > + MirrorState *s =3D container_of(rs, MirrorState, rs); > + NetFilterState *nf =3D NETFILTER(s); > + > + redirector_to_filter(nf, rs->buf, rs->packet_len); > +} > + > static void filter_redirector_setup(NetFilterState *nf, Error **errp) > { > MirrorState *s =3D FILTER_REDIRECTOR(nf); > @@ -274,7 +244,11 @@ static void filter_redirector_setup(NetFilterState= *nf, Error **errp) > } > } > =20 > - s->state =3D s->index =3D 0; > + s->rs.init =3D net_socket_rs_init; > + s->rs.finalize =3D 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=20 redirector_rs_finalize to it. Then there's no need to do the open code he= re.