Re: [Qemu-devel] [PATCH v3 07/12] netfilter: add an API to pass the packet to next filter

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

On 08/04/2015 01:00 PM, Jason Wang wrote:
>
>
> On 08/03/2015 04:30 PM, Yang Hongyang wrote:
>> add an API qemu_netfilter_pass_to_next_iov() to pass the packet
>> to next filter, and a wrapper qemu_netfilter_pass_to_next().
>>
>> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
>> ---
>>   include/net/filter.h | 12 ++++++++++++
>>   include/net/net.h    |  1 +
>>   net/filter.c         | 31 +++++++++++++++++++++++++++++++
>>   net/net.c            | 13 +++++++++++++
>>   4 files changed, 57 insertions(+)
>>
>> diff --git a/include/net/filter.h b/include/net/filter.h
>> index 7f0c949..c2be970 100644
>> --- a/include/net/filter.h
>> +++ b/include/net/filter.h
>> @@ -53,4 +53,16 @@ void qemu_del_net_filter(NetFilterState *nf);
>>   void netfilter_add(QemuOpts *opts, Error **errp);
>>   void qmp_netfilter_add(QDict *qdict, QObject **ret, Error **errp);
>>
>> +/* pass the packet to the next filter */
>> +void qemu_netfilter_pass_to_next_iov(NetFilterState *nf,
>> +                                     NetClientState *sender,
>> +                                     unsigned flags,
>> +                                     const struct iovec *iov,
>> +                                     int iovcnt);
>> +void qemu_netfilter_pass_to_next(NetFilterState *nf,
>> +                                 NetClientState *sender,
>> +                                 unsigned flags,
>> +                                 const uint8_t *data,
>> +                                 size_t size);
>> +
>>   #endif /* QEMU_NET_FILTER_H */
>> diff --git a/include/net/net.h b/include/net/net.h
>> index 5c5c109..d3bfe12 100644
>> --- a/include/net/net.h
>> +++ b/include/net/net.h
>> @@ -117,6 +117,7 @@ NetClientState *qemu_new_net_client(NetClientInfo *info,
>>                                       const char *name);
>>   int qemu_netdev_add_filter(NetClientState *nc, NetFilterState *nf);
>>   void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf);
>> +NetFilterState *qemu_netdev_next_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 bf113e9..84845b1 100644
>> --- a/net/filter.c
>> +++ b/net/filter.c
>> @@ -131,6 +131,37 @@ void qmp_netfilter_del(const char *id, Error **errp)
>>       qemu_opts_del(opts);
>>   }
>>
>> +void qemu_netfilter_pass_to_next_iov(NetFilterState *nf,
>> +                                     NetClientState *sender,
>> +                                     unsigned flags,
>> +                                     const struct iovec *iov,
>> +                                     int iovcnt)
>> +{
>> +    NetFilterState *next = qemu_netdev_next_filter(nf->netdev, nf);
>> +
>> +    while (next) {
>> +        if (next->chain == nf->chain || next->chain == NET_FILTER_ALL) {
>> +            next->info->receive_iov(next, sender, flags, iov, iovcnt);
>
> If a packet is not held by one filter, we need pass it to next filter?

Yes, will fix, thank you.

>
>> +            return;
>> +        }
>> +        next = qemu_netdev_next_filter(next->netdev, next);
>> +    }
>
> If a packet has gone through all filters, we probably need to pass it
> the receiver's incoming queue.

Ok, will do, thanks.

>
>> +}
>
> Looks like no real user for this helper?

No real user in this series, I see your suggestion on the buffer
filter, will use this api in that filter, thank you.

>
>> +
>> +void qemu_netfilter_pass_to_next(NetFilterState *nf,
>> +                                 NetClientState *sender,
>> +                                 unsigned flags,
>> +                                 const uint8_t *data,
>> +                                 size_t size)
>> +{
>> +    struct iovec iov = {
>> +        .iov_base = (void *)data,
>> +        .iov_len = size
>> +    };
>> +
>> +    return qemu_netfilter_pass_to_next_iov(nf, sender, flags, &iov, 1);
>> +}
>> +
>>   typedef int (NetFilterInit)(const NetFilterOptions *opts,
>>                               const char *name, int chain,
>>                               NetClientState *netdev, Error **errp);
>> diff --git a/net/net.c b/net/net.c
>> index f774c39..e087763 100644
>> --- a/net/net.c
>> +++ b/net/net.c
>> @@ -339,6 +339,19 @@ void qemu_netdev_remove_filter(NetClientState *nc, NetFilterState *nf)
>>       remove_filter(nc, filter);
>>   }
>>
>> +NetFilterState *qemu_netdev_next_filter(NetClientState *nc, NetFilterState *nf)
>> +{
>> +    Filter *filter = NULL;
>> +
>> +    QTAILQ_FOREACH(filter, &nc->filters, next) {
>> +        if (filter->nf == nf) {
>> +            break;
>> +        }
>> +    }
>
> A little bit confused, can we just use net->next?

it's because we use struct Filter to manage filters attached to a netdev,
not struct NetFilterState.

>
>> +
>> +    return QTAILQ_NEXT(filter, next)->nf;
>> +}
>> +
>>   NICState *qemu_new_nic(NetClientInfo *info,
>>                          NICConf *conf,
>>                          const char *model,
>
> .
>

-- 
Thanks,
Yang.