Re: [Qemu-devel] [PATCH v11 12/12] netfilter: add multiqueue support

[Jason Wang <jasowang@redhat.com>]

On 09/22/2015 04:07 PM, Yang Hongyang wrote:
> On 09/22/2015 03:36 PM, Jason Wang wrote:
>>
>>
>> On 09/16/2015 08:16 PM, Yang Hongyang wrote:
>>> From: Yang Hongyang <burnef@gmail.com>
>>>
>>> add multiqueue support, if there's multiqueue, we add multi netfilter
>>> objects, other netfilter objects is the child of the first added
>>> netfilter
>>> object. So when we delete a netfilter, the other netfilter objects we
>>> added will be automatically deleted.
>>>
>>> Signed-off-by: Yang Hongyang <burnef@gmail.com>
>>> ---
>>> v11: initial patch
>>> ---
>>>   net/filter.c | 86
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>>   1 file changed, 79 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/net/filter.c b/net/filter.c
>>> index aea619a..cc27528 100644
>>> --- a/net/filter.c
>>> +++ b/net/filter.c
>>> @@ -142,16 +142,25 @@ static void netfilter_finalize(Object *obj)
>>>       g_free(nf->name);
>>>   }
>>>
>>> +static void proptb_free_val_func(gpointer data)
>>> +{
>>> +    g_free(data);
>>> +}
>>> +
>>>   static void netfilter_complete(UserCreatable *uc, Error **errp)
>>>   {
>>> -    NetFilterState *nf = NETFILTER(uc);
>>> +    NetFilterState *nf = NETFILTER(uc), *nfq = NULL;
>>>       NetClientState *ncs[MAX_QUEUE_NUM];
>>> -    NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
>>> -    int queues;
>>> +    NetFilterClass *nfc = NETFILTER_GET_CLASS(uc), *nfqc = NULL;
>>> +    int queues, i;
>>>       Error *local_err = NULL;
>>> -    char *str, *info;
>>> +    char *str, *info, *name;
>>>       ObjectProperty *prop;
>>>       StringOutputVisitor *ov;
>>> +    Object *obj = NULL;
>>> +    GHashTable *proptable = NULL;
>>> +    GHashTableIter iter;
>>> +    gpointer key, value;
>>>
>>>       if (!nf->netdev_id) {
>>>           error_setg(errp, "Parameter 'netdev' is required");
>>> @@ -165,9 +174,6 @@ static void netfilter_complete(UserCreatable
>>> *uc, Error **errp)
>>>           error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "netdev",
>>>                      "a network backend id");
>>>           return;
>>> -    } else if (queues > 1) {
>>> -        error_setg(errp, "Multi queue is not supported");
>>> -        return;
>>>       }
>>>
>>>       if (get_vhost_net(ncs[0])) {
>>> @@ -187,6 +193,17 @@ static void netfilter_complete(UserCreatable
>>> *uc, Error **errp)
>>>       }
>>>       QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
>>>
>>> +    if (queues > 1) {
>>> +        /*
>>> +         * Store the properties of the filter except "type" property.
>>> +         * When there's multiqueue, we will create a new filter object
>>> +         * of the same type and same properties. this hashtable is
>>> used
>>> +         * to set newly created object properties.
>>> +         */
>>> +        proptable = g_hash_table_new_full(NULL, NULL, NULL,
>>> +                                          proptb_free_val_func);
>>> +    }
>>
>> I'm thinking whether or not duplicate all the properties in each
>> netfilters is a good method. Maybe we can have a another ojbect with
>> array of pointers to NetFilter objects embedded? Another question is
>> whether or not we need to do this at this level. Maybe we can make the
>> necessary Netfilter multiqueue aware. E.g let buffer filter to have
>> multiqueue also? Then you may only need a single timer?
>
> Sorry I don't get it at first. I also thought about make the buffer
> filter to
> have multiqueue, but there comes problem, how to distinguish which queue
> we should go in when receive the packet, we need to add a mechanism to
> distinguish which queue belongs to which net client's queue, that will
> be more complex, while current multiqueue implementation of net clients
> is multiple net clients with the same name, current solution is the
> simplest
> solution I can think of...

I'm not sure I get this. But there's a queue_index filed in each
NetClientState which may help in this case.