Re: [Qemu-devel] [PATCH V2] net/traffic-mirror:Add traffic-mirror

[Jason Wang <jasowang@redhat.com>]

On 01/28/2016 05:52 PM, Zhang Chen wrote:
>
>
> On 01/28/2016 04:37 PM, Jason Wang wrote:
>>
>> On 01/28/2016 03:44 PM, Zhang Chen wrote:
>>>
>>> On 01/28/2016 01:44 PM, Jason Wang wrote:
>>>> On 01/27/2016 10:40 AM, Zhang Chen wrote:
>>>>> From: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>>
>>>>> Traffic-mirror is a netfilter plugin.
>>>>> It gives qemu the ability to copy and mirror guest's
>>>>> net packet. we output packet to chardev.
>>>>>
>>>>> usage:
>>>>>
>>>>> -netdev tap,id=hn0
>>>>> -chardev socket,id=mirror0,host=ip_primary,port=X,server,nowait
>>>>> -traffic-mirror,id=m0,netdev=hn0,queue=tx/rx/all,outdev=mirror0
>>>>>
>>>>> Signed-off-by: ZhangChen <zhangchen.fnst@cn.fujitsu.com>
>>>>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>>>>> Reviewed-by: Yang Hongyang <hongyang.yang@easystack.cn>
>>>> Thanks for the patch. Several questions:
>>>>
>>>> - I'm curious about how the patch was tested? Simple setup e.g:
>>>>
>>>> -netdev tap,id=hn0 -device virtio-net-pci,netdev=hn0 -chardev
>>>> socket,id=c0,host=localhost,port=4444,server,nowait -object
>>>> traffic-mirror,netdev=hn0,outdev=c0,id=f0 -netdev
>>>> socket,id=s0,connect=127.0.0.1:4444 -device e1000,netdev=s0
>>>>
>>>> does not works for me.
>>> I test it in this way.
>>> primary:
>>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>>> socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
>>>   -object traffic-mirror,id=f0,netdev=hn0,queue=tx,outdev=mirror0
>>>
>>> secondary:
>>> -netdev tap,id=hn0 -device e1000,netdev=hn0 -chardev
>>> socket,id=mirror0,host=3.3.3.3,port=9003 -object
>>> traffic-reader,id=f1,netdev=hn0,queue=rx,indev=mirror0
>>>
>>> I write a traffic-reader demo to read chardev socket and print it in
>>> monitor.
>> Ok, but maybe you can try socket backend. I think the protocol should be
>> at least compatible with it.
>
> Yes,I will try it.
>
>>>
>>>> - Is a reliable mirroring (e.g no packet drops during mirroring) is
>>>> needed for COLO? If yes, this patch seems could not guarantee this.
>>> I will fix it in V3
>>>
>>>> - Please consider to write a unit test for this patch.
>>> write a unit test like tests/test-netfilter.c ?
>> Even more for its basic function to work. E.g, start qemu with:
>>
>> -netdev socket,id=s0,listen=localhost:X -chardev
>> socket,id=c0,host=localhost,pory=Y,server,nowait -object
>> filter-mirror,netdev=hn0,outdev=c0
>>
>> Then you can inject packet from the socket connected to s0 and see if
>> you can read it from socket that connected from c0 (or your traffic
>> reader).
>
> I got it~ will add test in next version. 

Thanks and make it a separate patch on top.