From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acrAZ-0006ke-Ry for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:09:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1acrAW-0005bz-CP for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:09:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47023) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acrAW-0005bv-4C for qemu-devel@nongnu.org; Mon, 07 Mar 2016 04:09:36 -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> From: Jason Wang Message-ID: <56DD4543.1060808@redhat.com> Date: Mon, 7 Mar 2016 17:09:23 +0800 MIME-Version: 1.0 In-Reply-To: <56DD3B35.1000609@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252 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: Li Zhijian , Zhang Chen , qemu devel Cc: zhanghailiang , Gui jianfeng , "eddie.dong" , "Dr. David Alan Gilbert" , Yang Hongyang 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 > Li Zhijian