Re: [Qemu-devel] [PATCH v3 04/12] net: add/remove filters from network backend

[Jason Wang <jasowang@redhat.com>]

On 08/03/2015 04:30 PM, Yang Hongyang wrote:
> add/remove filters from network backend
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> ---
>  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?

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