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

[Github ODP bot]

lironhimi replied on github web page:

platform/linux-generic/odp_packet_io.c
line 10
@@ -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:
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_r145918618
updated_at 2017-10-20 09:35:13