Re: [lng-odp] [PATCH v3] linux-gen: pktio: ignore num_queues parameter if classifier enabled

[Github ODP bot]

Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:

platform/linux-generic/odp_packet_io.c
@@ -1229,7 +1229,12 @@ int odp_pktin_queue_config(odp_pktio_t pktio,
        if (mode == ODP_PKTIN_MODE_DISABLED)
                return 0;
 
-       num_queues = param->num_queues;
+       if (param->classifier_enable) {
+               ODP_DBG("num_queues ignored if classifier is enabled\n");
+               num_queues = 1;
+       } else {
+               num_queues = param->num_queues;
+       }


Comment:
Good point, though that could be optimized to:
```
        for (i = 0; i < num_queues; i++) {
                if (mode == ODP_PKTIN_MODE_QUEUE ||
                    mode == ODP_PKTIN_MODE_SCHED) {
                        odp_queue_param_t queue_param;
                        char name[ODP_QUEUE_NAME_LEN];
                        int pktio_id = pktio_to_id(pktio);

                        snprintf(name, sizeof(name), "odp-pktin-%i-%i",
                                 pktio_id, i);

-                       memcpy(&queue_param, &param->queue_param,
-                              sizeof(odp_queue_param_t));
-
+                       if (param->classifier_enable)
+                               odp_queue_param_init(&queue_param);
+                       else
+                              memcpy(&queue_param, &param->queue_param,
+                                          sizeof(odp_queue_param_t));
                        queue_param.type = ODP_QUEUE_TYPE_PLAIN;
```

> lironhimi wrote
> you should also ignore the input 'param->queue_param'. consider the following:
> ```
>       for (i = 0; i < num_queues; i++) {
>               if (mode == ODP_PKTIN_MODE_QUEUE ||
>                   mode == ODP_PKTIN_MODE_SCHED) {
>                       odp_queue_param_t queue_param;
>                       char name[ODP_QUEUE_NAME_LEN];
>                       int pktio_id = pktio_to_id(pktio);
> 
>                       snprintf(name, sizeof(name), "odp-pktin-%i-%i",
>                                pktio_id, i);
> 
>                       memcpy(&queue_param, &param->queue_param,
>                              sizeof(odp_queue_param_t));
> 
> +                     if (param->classifier_enable)
> +                             odp_queue_param_init(&queue_param);
>                       queue_param.type = ODP_QUEUE_TYPE_PLAIN;
> '''


>> muvarov wrote
>> yes, that will work. Will send v2.


>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>> Ok, that check may be too strict. If we're going to ignore it then the test 
>>> should be changed to:
>>> ```
>>> if (!param->classifier_enable && param->num_queues == 0) {
>>>         ODP_DBG("invalid num_queues for operation mode\n");
>>>         return -1;
>>> }
>>> 
>>> num_queues = param->classifier_enable ? 1 : param->num_queues;
>>> ```


>>>> muvarov wrote
>>>> API says " When classifier is enabled in odp_pktin_queue_config() this 
>>>> value is ignored".  Should we also correct API statement to "When 
>>>> classifier is enabled num_queues has to be 0" ?.


>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>> The requirement is that `num_queues` should be zero if and only if 
>>>>> `param->classifier_enable` is true, so a more complete check would be:
>>>>> ```
>>>>> if (param->classifier_enable ^ param->num_queues == 0) {
>>>>>         ODP_DBG("invalid num_queues for operation mode\n");
>>>>>         return -1;
>>>>> }
>>>>> 
>>>>> num_queues = param->classifier_enable ? 1 : param->num_queues;
>>>>> ```


https://github.com/Linaro/odp/pull/240#discussion_r145978457
updated_at 2017-10-20 14:31:40