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

[Jason Wang <jasowang@redhat.com>]

On 07/27/2015 04:39 PM, Yang Hongyang wrote:
> On 07/27/2015 04:01 PM, Jason Wang wrote:
>>
>> On 07/27/2015 03:45 PM, Yang Hongyang wrote:
>>> On 07/27/2015 03:31 PM, Jason Wang wrote:
>>>>
>>>>
>>>> On 07/27/2015 03:00 PM, Yang Hongyang wrote:
>>>>>
>>>>>
>>>>> On 07/27/2015 02:39 PM, Jason Wang wrote:
>>>>>>
>>>>>>
>>>>>> On 07/27/2015 01:27 PM, Yang Hongyang wrote:
>>>>>>> On 07/23/2015 01:59 PM, Jason Wang wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 07/22/2015 06:55 PM, Yang Hongyang wrote:
>>>>>>>>> This patch add a net filter between network backend and NIC
>>>>>>>>> devices.
>>>>>>>>> All packets will pass by this filter.
>>>>>>>>> TODO:
>>>>>>>>>      multiqueue support.
>>>>>>>>>      plugin support.
>>>>>>>>>
>>>>>>>>>                   +--------------+       +-------------+
>>>>>>>>> +----------+  |    filter    |       |frontend(NIC)|
>>>>>>>>> | real     |  |              |       |             |
>>>>>>>>> | network  <--+backend       <-------+             |
>>>>>>>>> | backend  |  |         peer +-------> peer        |
>>>>>>>>> +----------+  +--------------+       +-------------+
>>>>>>>>>
>>>>>>>>> Usage:
>>>>>>>>> -netdev tap,id=bn0  # you can use whatever backend as needed
>>>>>>>>> -netdev filter,id=f0,backend=bn0,plugin=dump
>>>>>>>>> -device e1000,netdev=f0
>>>>>>>>>
>>>>>>>>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>>>>>>>>
>>>>>>>> Hi:
>>>>>>>>
>>>>>>>> Several questions:
>>>>>>>>
>>>>>>>> - Looks like we can do more than filter, so may be something like
>>>>>>>> traffic control or other is more suitable?
>>>>>>>
>>>>>>> The filter is just a transparent proxy of a backend if no filter
>>>>>>> plugin
>>>>>>> is inserted. It just by pass all packets. Capture all traffic is
>>>>>>> the
>>>>>>> purpose
>>>>>>> of the filter. As long as we have an entry to capture all
>>>>>>> packets, we
>>>>>>> can do more, this is what a filter plugin will do. There are
>>>>>>> some use
>>>>>>> cases
>>>>>>> I can think of:
>>>>>>> - dump, by using filter, we can dump either output/input packets.
>>>>>>> - buffer, to buffer/release packets, this feature can be used when
>>>>>>> using
>>>>>>>              macrocheckpoing. Or other Remus like VM FT
>>>>>>> solutions. You
>>>>>>> can
>>>>>>>              also supply an interval to a buffer plugin, which will
>>>>>>> release
>>>>>>>              packets by interval.
>>>>>>
>>>>>> This sounds like traffic shaping.
>>>>>>
>>>>>>> May be other use cases based on this special backend.
>>>>>>>
>>>>>>>> - What's the advantages of introducing a new type of netdev? As
>>>>>>>> far
>>>>>>>> as I
>>>>>>>> can see, just replace the dump function in Tomas' series with a
>>>>>>>> configurable function pointer will do the trick? (Probably with
>>>>>>>> some
>>>>>>>> monitor commands). And then you won't even need to deal with vnet
>>>>>>>> hder
>>>>>>>> and offload stuffs?
>>>>>>>
>>>>>>> I think dump function focus on every netdev, it adds an
>>>>>>> dump_enabled to
>>>>>>> NetClientState, and dump the packet when the netdev receive been
>>>>>>> called,
>>>>>>> This filter function more focus on packets between
>>>>>>> backend/frontend,
>>>>>>> it's kind of an injection to the network packets flow.
>>>>>>> So the semantics are different I think.
>>>>>>
>>>>>> Yes, their functions are different. But the packet paths are
>>>>>> similar,
>>>>>> both require the packets go through themselves before reaching the
>>>>>> peers. So simply passing the packets to the filter function before
>>>>>> calling nc->info->receive{_raw}() in qemu_deliver_packet() will also
>>>>>> work?
>>>>>
>>>>> 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.

> 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.

>
>>
>>>
>>>>
>>>>> 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.
>>
>>>
>>>>
>>>>
>>>>
>>>> .
>>>>
>>>
>>
>> .
>>
>