Re: [Qemu-devel] [PATCH] RFC/net: Add a net filter

[Yang Hongyang <yanghy@cn.fujitsu.com>]

On 07/27/2015 05:16 PM, Jason Wang wrote:
[...]
>>>>>> I think this won't work for the buffer case? If we want the buffer
>>>>>> case
>>>>>> to work under this, we should modify the generic netdev layer
>>>>>> code, to
>>>>>> check the return value of the filter function call.
>>>>>
>>>>> But checking return value is rather simpler than a new netdev type,
>>>>> isn't it?
>>>>
>>>> But how to implement a plugin which suppose to do the actual work on
>>>> the packets?
>>>
>>> Well, the filter get the packets, so it can do everything it wants.
>>>
>>>> how to configure params related to the plugin? different
>>>> plugins may need different params, implement as another netdev?
>>>
>>> I belive qmp can do this? something like -filter dump,id=f0,len=10000?
>>
>> So you mean implement another object filter?
>
> Yes.
>
>> and the structure is like netdev?
>
> No, it is embedded in netdev.

Ah, I see what you mean, thank you for the patience...
does the command line looks like:
-filter dump,id=f0,len=10000
-netdev tap,XXX,filter=dump

If I need both dump and packets buffering, how will the qmp be?
-filter dump,id=f0,len=10000
-filter buffer,XXX
-netdev tap,XXX,filter=dump:buffer:XXX ?
or
-netdev tap,id=bn0,XXX
-filter dump,id=f0,len=10000,netdev=bn0
-filter buffer,XXX,netdev=bn0 ?

And do you care if we add a filter list to NetClientInfo?
and modify the generic layer to deal with these filters?
E.g, init/cleanup filters, go through these filters, check
for return values, stop call peer's receiving.
This is our main concern at the beginning, we want to avoid
modify the netdev generic layer too much, and do all things
within the filter dev so that this could be a bolt-on
feature. But if you don't care about this, we could simply
implement it as you said.

>
>> That will duplicate some of the netdev layer code.
>
> Not at all, it only cares about how to deal with the packet.
>
>> Implement it as
>> a netdev can reuse the existing netdev design. And current dump is
>> implemented
>> as a netdev right?
>
> Right but it only works for hub, and that's why Thomas wrote his series
> to make it work for all other backends
>
>> even if we simply passing the packets to the filter function before
>> calling nc->info->receive{_raw}(), we might also need to implement as
>> a netdev as dump dose.
>
> Why? The reason why we still keep -netdev dump is for backward
> compatibility. If we only care about using it for new netdevs, we can
> get rid of all netdev stuffs from dump.

Thank you, I see the points.

>
>>
>>>
>>>>
>>>>>
>>>>>> And it is not as
>>>>>> extensible as we abstract the filter function to a netdev, We can
>>>>>> flexibly add/remove/change filter plugins on the fly.
>>>>>
>>>>> I don't see why we lose the flexibility like what I suggested.
>>>>> Actually,
>>>>> implement it through a netdev will complex this. E.g:
>>>>>
>>>>> -netdev tap,id=bn0  # you can use whatever backend as needed
>>>>> -netdev filter,id=f0,backend=bn0,plugin=dump
>>>>> -device e1000,netdev=f0
>>>>>
>>>>> How did you remove filter id=f0? Looks like you need also remove
>>>>> e1000 nic?
>>>>
>>>> No, when remove filter, we restore the connection between network
>>>> backend and
>>>> NIC. Just like filter does not ever exists.
>>>
>>> But e1000's peer is f0. You mean you will modify the peer pointer during
>>> filter removing?
>>
>> Yes.
>>
>>> Sounds scary.
>>>
>>>>
>>>>>
>>>>>
>>>>>
>>>>> .
>>>>>
>>>>
>>>
>>> .
>>>
>>
>
> .
>

-- 
Thanks,
Yang.