[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 04/12] net: add/remove filters from network b
From: |
Jason Wang |
Subject: |
Re: [Qemu-devel] [PATCH v3 04/12] net: add/remove filters from network backend |
Date: |
Tue, 04 Aug 2015 13:53:07 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 |
On 08/04/2015 01:39 PM, Yang Hongyang wrote:
> On 08/04/2015 12:56 PM, Jason Wang wrote:
>>
>>
>> On 08/03/2015 04:30 PM, Yang Hongyang wrote:
>>> add/remove filters from network backend
>>>
>>> Signed-off-by: Yang Hongyang <address@hidden>
>>> ---
>>> include/net/net.h | 8 ++++++++
>>> net/filter.c | 4 ++--
>>> net/net.c | 33 +++++++++++++++++++++++++++++++++
>>> 3 files changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/net/net.h b/include/net/net.h
>>> index 6a6cbef..5c5c109 100644
>>> --- a/include/net/net.h
>>> +++ b/include/net/net.h
>>> @@ -40,6 +40,11 @@ typedef struct NICConf {
>>>
>>>
>>> /* Net clients */
>>> +typedef struct Filter Filter;
>>> +struct Filter {
>>> + NetFilterState *nf;
>>> + QTAILQ_ENTRY(Filter) next;
>>> +};
>>
>> Didn't understand why need another structure here. Could we just use
>> NetFilterState?
>
> There's a QTAILQ_ENTRY next in NetFilterState, but used by filter
> layer, so
> that we can manage all filters together.
>
> This struct used by netdev. filter belongs to the specific netdev is
> in this queue.
>
> Just use NetFilterState in this case need to introduce another
> QTAILQ_ENTRY
> in NetFilterState, maybe will make it confusing?
Probably not.
> and it seems that it's
> not common to have 2 QTAILQ_ENTRYs in one struct?
I think this is ok. You can use something like global_list.
>
>>
>>>
>>> typedef void (NetPoll)(NetClientState *, bool enable);
>>> typedef int (NetCanReceive)(NetClientState *);
>>> @@ -92,6 +97,7 @@ struct NetClientState {
>>> NetClientDestructor *destructor;
>>> unsigned int queue_index;
>>> unsigned rxfilter_notify_enabled:1;
>>> + QTAILQ_HEAD(, Filter) filters;
>>> };
>>>
>>> typedef struct NICState {
>>> @@ -109,6 +115,8 @@ NetClientState
>>> *qemu_new_net_client(NetClientInfo *info,
>>> NetClientState *peer,
>>> const char *model,
>>> const char *name);
>>> +int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf);
>>> +void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState
>>> *nf);
>>> NICState *qemu_new_nic(NetClientInfo *info,
>>> NICConf *conf,
>>> const char *model,
>>> diff --git a/net/filter.c b/net/filter.c
>>> index 86eed8a..1ae9344 100644
>>> --- a/net/filter.c
>>> +++ b/net/filter.c
>>> @@ -38,14 +38,14 @@ NetFilterState
>>> *qemu_new_net_filter(NetFilterInfo *info,
>>> nf->netdev = netdev;
>>> nf->chain = chain;
>>> QTAILQ_INSERT_TAIL(&net_filters, nf, next);
>>> - /* TODO: attach netfilter to netdev */
>>> + qemu_netdev_add_filter(netdev, nf);
>>>
>>> return nf;
>>> }
>>>
>>> static void qemu_cleanup_net_filter(NetFilterState *nf)
>>> {
>>> - /* TODO: remove netfilter from netdev */
>>> + qemu_netdev_remove_filter(nf->netdev, nf);
>>>
>>> QTAILQ_REMOVE(&net_filters, nf, next);
>>>
>>> diff --git a/net/net.c b/net/net.c
>>> index 28a5597..00c5ca3 100644
>>> --- a/net/net.c
>>> +++ b/net/net.c
>>> @@ -287,6 +287,7 @@ static void qemu_net_client_setup(NetClientState
>>> *nc,
>>>
>>> nc->incoming_queue = qemu_new_net_queue(nc);
>>> nc->destructor = destructor;
>>> + QTAILQ_INIT(&nc->filters);
>>> }
>>>
>>> NetClientState *qemu_new_net_client(NetClientInfo *info,
>>> @@ -305,6 +306,38 @@ NetClientState
>>> *qemu_new_net_client(NetClientInfo *info,
>>> return nc;
>>> }
>>>
>>> +int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf)
>>> +{
>>> + Filter *filter = g_malloc0(sizeof(*filter));
>>> +
>>> + filter->nf = nf;
>>> + QTAILQ_INSERT_TAIL(&nc->filters, filter, next);
>>> + return 0;
>>> +}
>>> +
>>> +static void remove_filter(NetClientState *nc, Filter *filter)
>>> +{
>>> + if (!filter) {
>>> + return;
>>> + }
>>> +
>>> + QTAILQ_REMOVE(&nc->filters, filter, next);
>>> + g_free(filter);
>>> +}
>>> +
>>> +void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf)
>>> +{
>>> + Filter *filter = NULL;
>>> +
>>> + QTAILQ_FOREACH(filter, &nc->filters, next) {
>>> + if (filter->nf == nf) {
>>> + break;
>>> + }
>>> + }
>>> +
>>> + remove_filter(nc, filter);
>>> +}
>>> +
>>> NICState *qemu_new_nic(NetClientInfo *info,
>>> NICConf *conf,
>>> const char *model,
>>
>> Another thing may need consider is qemu_flush_queued_packets(). Look
>> like we need also flush packets inside each filter in this case?
>
> When a filter is removed, the filter itself will flush the packets.
> when a filter queued a packet, we assume the filter will take care
> of it. the filter is not a netdev, so I think we do not need to flush
> packets in qemu_flush_queued_packets(), otherwise, the buffer filter
> will be useless, because when qemu_flush_queued_packets() is called,
> it will flush the packets, it's not what we want.
Right.