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 <zhangchen.f...@cn.fujitsu.com>
>>> Signed-off-by: Wen Congyang <we...@cn.fujitsu.com>
>>> ---
>>> 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
