From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZK12f-0001N5-Hv for qemu-devel@nongnu.org; Tue, 28 Jul 2015 05:19:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZK12a-0003ur-DD for qemu-devel@nongnu.org; Tue, 28 Jul 2015 05:19:21 -0400 Received: from [59.151.112.132] (port=33309 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZK12Y-0003r1-P8 for qemu-devel@nongnu.org; Tue, 28 Jul 2015 05:19:16 -0400 Message-ID: <55B74905.5040404@cn.fujitsu.com> Date: Tue, 28 Jul 2015 17:19:01 +0800 From: Yang Hongyang MIME-Version: 1.0 References: <55AF75E6.6070909@cn.fujitsu.com> <1437562536-20414-1-git-send-email-yanghy@cn.fujitsu.com> <55B082BE.2020703@redhat.com> <55B5C14F.5030808@cn.fujitsu.com> <55B5D214.5030506@redhat.com> <55B5D727.8010806@cn.fujitsu.com> <55B5DE34.9050409@redhat.com> <55B5E1A4.9010100@cn.fujitsu.com> <55B5E543.8050505@redhat.com> <55B5EE5F.4050003@cn.fujitsu.com> <55B5F706.3030808@redhat.com> <55B60209.6050401@cn.fujitsu.com> <55B6F6ED.5090900@redhat.com> <55B6FE61.80109@cn.fujitsu.com> In-Reply-To: <55B6FE61.80109@cn.fujitsu.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] RFC/net: Add a net filter List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang , qemu-devel@nongnu.org Cc: thuth@redhat.com, stefanha@redhat.com On 07/28/2015 12:00 PM, Yang Hongyang wrote: > On 07/28/2015 11:28 AM, Jason Wang wrote: >> On 07/27/2015 06:03 PM, Yang Hongyang wrote: >>> 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 ? >> >> This is ok but we have several choices, e.g you may want to have a next >> field like: >> >> - filter buffer,id=f0 >> - filter dump,id=f1,len=1000,next=f0 >> - netdev tap,XXX,filter_root=f1 > > This seems better, thank you! Sorry, when try to implement it, I found this qmp is not that good when it comes to dynamically add/remove filters, can I use below instead: -netdev tap,id=bn0 -netfilter dump,id=f0,netdev=bn0 -netfilter buffer,id=f1,netdev=bn0 by this, I can introduce a QTAILQ of filters to NetClentState and netdev_{add|del}_filter to dynamically add/remove filters. > >> >>> 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? >> >> I don't care, and in fact this also shows great advantages over the >> proposal of new netdevs. In that case, if you want two filters, two >> netdevs is needed and I can't image how to handle offloads or link >> status in this case. >> >>> 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. >> >> I think it's ok to do these. What we really need is an new layer before >> peer's receiving not new kinds of netdevs. >> >>> 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. >> >> I don't think much will be modified. Maybe just add callbacks on >> receive, initialization and cleanup. Most of the codes should be limited >> to filter itself. The point is 'filter' is not a kind of device or >> backend, so most of the fields will be unnecessary. Treating it as >> pseudo netdev will bring burdens too. E.g: dealing with vnet headers, >> offloads, be/le setting and link status and probably even more. > > Thank you for the explanation. will do 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.