From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36002) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acrvW-00014t-50 for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:58:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1acrvS-0000qH-Ng for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:58:09 -0500 Received: from [59.151.112.132] (port=63948 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acrvR-0000pT-Ec for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:58:06 -0500 References: <1457092906-19828-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1457092906-19828-3-git-send-email-zhangchen.fnst@cn.fujitsu.com> <56DD3414.2040301@redhat.com> <56DD3B35.1000609@cn.fujitsu.com> <56DD4543.1060808@redhat.com> From: Li Zhijian Message-ID: <56DD50A4.6020002@cn.fujitsu.com> Date: Mon, 7 Mar 2016 17:57:56 +0800 MIME-Version: 1.0 In-Reply-To: <56DD4543.1060808@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V3 2/3] net/filter-mirror:Add filter-redirector func List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , Zhang Chen , qemu devel Cc: zhanghailiang , Gui jianfeng , "eddie.dong" , "Dr. David Alan Gilbert" , Yang Hongyang On 03/07/2016 05:09 PM, Jason Wang wrote: > > > On 03/07/2016 04:26 PM, Li Zhijian wrote: >> >> >> On 03/07/2016 03:56 PM, Jason Wang wrote: >>> >>> >>> On 03/04/2016 08:01 PM, Zhang Chen wrote: >>>> Filter-redirector is a netfilter plugin. >>>> It gives qemu the ability to redirect net packet. >>>> redirector can redirect filter's net packet to outdev. >>>> and redirect indev's packet to filter. >>>> >>>> filter >>>> + >>>> | >>>> | >>>> redirector | >>>> +--------------+ >>>> | | | >>>> | | | >>>> | | | >>>> indev +-----------+ +----------> outdev >>>> | | | >>>> | | | >>>> | | | >>>> +--------------+ >>>> | >>>> | >>>> v >>>> filter >>>> >>>> usage: >>>> >>>> -netdev user,id=hn0 >>>> -chardev socket,id=s0,host=ip_primary,port=X,server,nowait >>>> -chardev socket,id=s1,host=ip_primary,port=Y,server,nowait >>>> -filter-redirector,id=r0,netdev=hn0,queue=tx/rx/all,indev=s0,outdev=s1 >>>> >>>> Signed-off-by: Zhang Chen >>>> Signed-off-by: Wen Congyang >>>> --- >>>> net/filter-mirror.c | 211 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> qemu-options.hx | 8 ++ >>>> vl.c | 3 +- >>>> 3 files changed, 221 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c >>>> index 4ff7619..d137168 100644 >>>> --- a/net/filter-mirror.c >>>> +++ b/net/filter-mirror.c >>>> @@ -25,11 +25,19 @@ >>>> #define FILTER_MIRROR(obj) \ >>>> OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_MIRROR) >>>> >>>> +#define FILTER_REDIRECTOR(obj) \ >>>> + OBJECT_CHECK(MirrorState, (obj), TYPE_FILTER_REDIRECTOR) >>>> + >>>> #define TYPE_FILTER_MIRROR "filter-mirror" >>>> +#define TYPE_FILTER_REDIRECTOR "filter-redirector" >>>> +#define REDIRECT_HEADER_LEN sizeof(uint32_t) >>>> >>>> typedef struct MirrorState { >>>> NetFilterState parent_obj; >>>> + NetQueue *incoming_queue; >>>> + char *indev; >>>> char *outdev; >>>> + CharDriverState *chr_in; >>>> CharDriverState *chr_out; >>>> } MirrorState; >>>> >>>> @@ -67,6 +75,68 @@ err: >>>> return ret < 0 ? ret : -EIO; >>>> } >>>> >>>> +static int redirector_chr_can_read(void *opaque) >>>> +{ >>>> + return REDIRECT_HEADER_LEN; >>>> +} >>>> + >>>> +static void redirector_chr_read(void *opaque, const uint8_t *buf, >>>> int size) >>>> +{ >>>> + NetFilterState *nf = opaque; >>>> + MirrorState *s = FILTER_REDIRECTOR(nf); >>>> + uint32_t len; >>>> + int ret = 0; >>>> + uint8_t *recv_buf; >>>> + >>>> + memcpy(&len, buf, size); >>> >>> stack overflow if size > sizeof(len)? >> IIUC, it seems never happend because the 'size' will never greater >> than the return value of >> redirector_chr_can_read() which will always return REDIRECT_HEADER_LEN ? > > Right, so it's safe. > >> >> >>> >>>> + if (size < REDIRECT_HEADER_LEN) { >>>> + ret = qemu_chr_fe_read_all(s->chr_in, ((uint8_t *)&len) + >>>> size, >>>> + REDIRECT_HEADER_LEN - size); >>> >>> There maybe some misunderstanding for my previous reply. You can have a >>> look at net_socket_send() for reference. You could >>> >>> - use a buffer for storing len >>> - each time when you receive partial len, store them in the buffer and >>> advance the pointer until you receive at least sizeof(len) bytes. >> qemu_chr_fe_read_all() seem have done this work. > > Not the same. qemu_chr_fe_read_all() will do loop reading and usleep > which is suboptimal. My proposal does not have this issue. It will make > redirector_chr_read() can handle arbitrary length of data and won't do > any busy reading. > >> Do you mean that >> we implement a similar code to do that instead of qemu_chr_fe_read_all() > > Nope, if you have a look at net_socket_send() it won't do any usleep and > loop reading, it will return immediately when it does not get sufficient > data. But it's really your call, not a must but worth to be optimized on > top in the future. > Thanks for you explain. Is it just like bellow code(not completed) ? #define REDIRECTOR_MAX_LEN NET_BUFSIZE typedef struct MirrorState { NetFilterState parent_obj; char *indev; 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]; } MirrorState; static int redirector_chr_can_read(void *opaque) { return REDIRECTOR_MAX_LEN; } 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) { 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; /* TODO: do something */ } s->index += l; buf += l; size -= l; if (s->index >= s->packet_len) { s->index = 0; s->state = 0; /* * TODO: pass packet to next filter */ } break; } } } Thanks Li Zhijian