From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agREv-0005gx-6t for qemu-devel@nongnu.org; Thu, 17 Mar 2016 02:16:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agREq-0002sC-G4 for qemu-devel@nongnu.org; Thu, 17 Mar 2016 02:16:57 -0400 Received: from [59.151.112.132] (port=7351 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agREp-0002qi-JM for qemu-devel@nongnu.org; Thu, 17 Mar 2016 02:16:52 -0400 References: <1458036214-867-1-git-send-email-zhangchen.fnst@cn.fujitsu.com> <1458036214-867-2-git-send-email-zhangchen.fnst@cn.fujitsu.com> <56E916EB.3050704@redhat.com> <56E928C2.60100@cn.fujitsu.com> <56EA4A3B.8060909@redhat.com> From: Wen Congyang Message-ID: <56EA4C21.9000205@cn.fujitsu.com> Date: Thu, 17 Mar 2016 14:18:09 +0800 MIME-Version: 1.0 In-Reply-To: <56EA4A3B.8060909@redhat.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH V4 1/2] net/filter-mirror: implement filter-redirector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , Zhang Chen , qemu devel Cc: zhanghailiang , Li Zhijian , Gui jianfeng , "eddie.dong" , "Dr. David Alan Gilbert" , Yang Hongyang On 03/17/2016 02:10 PM, Jason Wang wrote: > > > On 03/16/2016 05:34 PM, Wen Congyang wrote: >> On 03/16/2016 04:18 PM, Jason Wang wrote: >>>> >>>> >>>> On 03/15/2016 06:03 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 >>>>>> Signed-off-by: Li Zhijian >>>>>> --- >>>>>> net/filter-mirror.c | 236 ++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> qemu-options.hx | 9 ++ >>>>>> vl.c | 3 +- >>>>>> 3 files changed, 247 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/net/filter-mirror.c b/net/filter-mirror.c >>>>>> index 1b1ec16..77ece41 100644 >>>>>> --- a/net/filter-mirror.c >>>>>> +++ b/net/filter-mirror.c >>>>>> @@ -26,12 +26,23 @@ >>>>>> #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 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 filter_mirror_send(CharDriverState *chr_out, >>>>>> @@ -68,6 +79,89 @@ err: >>>>>> return ret < 0 ? ret : -EIO; >>>>>> } >>>>>> >>>>>> +static void >>>>>> +redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len) >>>>>> +{ >>>>>> + struct iovec iov = { >>>>>> + .iov_base = (void *)buf, >>>>>> + .iov_len = len, >>>>>> + }; >>>>>> + >>>>>> + if (nf->direction == NET_FILTER_DIRECTION_ALL || >>>>>> + nf->direction == NET_FILTER_DIRECTION_TX) { >>>>>> + qemu_netfilter_pass_to_next(nf->netdev, 0, &iov, 1, nf); >>>>>> + } >>>>>> + >>>>>> + if (nf->direction == NET_FILTER_DIRECTION_ALL || >>>>>> + nf->direction == NET_FILTER_DIRECTION_RX) { >>>>>> + qemu_netfilter_pass_to_next(nf->netdev->peer, 0, &iov, 1, nf); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +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; >>>>>> + >>>>>> + if (size == 0) { >>>>>> + /* the peer is closed ? */ >>>>>> + return ; >>>>>> + } >>>> >>>> Looks like if you want to handle connection close, you need use event >>>> handler when calling qemu_chr_add_handlers(). >> In which case, we will see size is 0 if we don't have a event handler? > > Looking at tcp_chr_read(), it seems that we won't see zero size. > >> >> For redirector filter, I think we don't care about if the char device >> is disconnected. If the char device is ready again, we will continue >> to read from the char device. >> >> So I think we just add more comments here. >> >>>> >>>>>> + >>>>>> + /* most of code is stolen from net_socket_send */ >>>> >>>> This comment seems redundant. >>>> >>>>>> + 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"); >>>> >>>> error_report() looks better. >>>> >>>>>> + s->state = 0; >>>>>> + /* FIXME: do something ? */ >>>> >>>> This needs some thought, but at least reset the fd handler and state is >>>> needed. >> Maybe we can read the data from the chardev, and drop it? >> >> Thanks >> Wen Congyang >> > > No function difference, but reading and dropping will cost extra cpu > which seems not good. If we reset and receive it again, the unread data will be auto dropped? If so, I think reset the fd handler is better choice. Thanks Wen Congyang > > > > . >