Re: [lng-odp] [EXT] Re: odp_pktin_queue_param_t - classifier_enable & num_queues
Hi Bill,
I opened ‘https://bugs.linaro.org/show_bug.cgi?id=3289’ for this issue.
I think there is another issue related to this topic in case PKTIO was
configure in schedule mode.
The ‘start’ function assumes ‘num_in_queue’ is bigger than ‘0’ but in the
classifier_enable case it is ‘0’ so pktio is register with no real indexes,
actually in ‘index’ array there is garbage. (see code below)
If I want to have a local fix, what do you think should be the number of
queues? ‘max_input_queues’ in capabilities ?
Regards,
Liron
int odp_pktio_start(odp_pktio_t hdl)
{
…
mode = entry->s.param.in_mode;
if (mode == ODP_PKTIN_MODE_SCHED) {
unsigned i;
unsigned num = entry->s.num_in_queue;
int index[num];
for (i = 0; i < num; i++) {
index[i] = i;
if (entry->s.in_queue[i].queue
== ODP_QUEUE_INVALID) {
ODP_ERR("No
input queue\n");
return -1;
}
}
sched_fn->pktio_start(pktio_to_id(hdl), num,
index);
}
return res;
}
From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
Sent: Monday, October 02, 2017 14:52
To: Liron Himi
Cc: lng-odp@lists.linaro.org
Subject: [EXT] Re: [lng-odp] odp_pktin_queue_param_t - classifier_enable &
num_queues
External Email
Thanks Liron. This appears to be a bug in the implementation. It's only an
error for num_queues to be zero if hash_enabled is set.
On Mon, Oct 2, 2017 at 6:02 AM, Liron Himi
mailto:lir...@marvell.com>> wrote:
Hi,
According to the API if 'classifier_enable' is enabled than 'num_queues' and
'queue_param' should be ignored.
But looking at linux-generic implementation those parameters are always being
checked regardless of 'classifier_enable' value.
'classifier_enable' is actually ignore at linux-generic implementation.
The main problem is if 'num_queues' is '0' than 'odp_pktin_queue_config' return
an error although it can be '0' if 'classifier_enable' is enabled.
Regards,
Liron
Re: [lng-odp] [EXT] Re: odp_pktin_queue_param_t - classifier_enable & num_queues
Thanks Liron, I've checked the code and that seems to be another issue. The
intent is that when classifier_enable is set the ODP classifier creates
queues as needed.
+cc Bala for additional commentary.
On Tue, Oct 3, 2017 at 8:48 AM, Liron Himi wrote:
> Hi Bill,
>
>
>
> I opened ‘https://bugs.linaro.org/show_bug.cgi?id=3289’ for this issue.
>
>
>
> I think there is another issue related to this topic in case PKTIO was
> configure in schedule mode.
>
> The ‘start’ function assumes ‘num_in_queue’ is bigger than ‘0’ but in the
> classifier_enable case it is ‘0’ so pktio is register with no real indexes,
> actually in ‘index’ array there is garbage. (see code below)
>
>
>
> If I want to have a local fix, what do you think should be the number of
> queues? ‘max_input_queues’ in capabilities ?
>
>
>
> Regards,
>
> Liron
>
>
>
>
>
> int odp_pktio_start(odp_pktio_t hdl)
>
> {
>
> …
>
> mode = entry->s.param.in_mode;
>
>
>
> if (mode == ODP_PKTIN_MODE_SCHED) {
>
> unsigned i;
>
> unsigned num = entry->s.num_in_queue;
>
> int index[num];
>
>
>
> for (i = 0; i < num; i++) {
>
> index[i] = i;
>
>
>
> if
> (entry->s.in_queue[i].queue == ODP_QUEUE_INVALID) {
>
>
> ODP_ERR("No input queue\n");
>
> return -1;
>
> }
>
> }
>
>
>
> sched_fn->pktio_start(pktio_to_id(hdl),
> num, index);
>
> }
>
>
>
> return res;
>
> }
>
> *From:* Bill Fischofer [mailto:bill.fischo...@linaro.org]
> *Sent:* Monday, October 02, 2017 14:52
> *To:* Liron Himi
> *Cc:* lng-odp@lists.linaro.org
> *Subject:* [EXT] Re: [lng-odp] odp_pktin_queue_param_t -
> classifier_enable & num_queues
>
>
>
> External Email
> --
>
> Thanks Liron. This appears to be a bug in the implementation. It's only an
> error for num_queues to be zero if hash_enabled is set.
>
>
>
> On Mon, Oct 2, 2017 at 6:02 AM, Liron Himi wrote:
>
> Hi,
>
> According to the API if 'classifier_enable' is enabled than 'num_queues'
> and 'queue_param' should be ignored.
> But looking at linux-generic implementation those parameters are always
> being checked regardless of 'classifier_enable' value.
> 'classifier_enable' is actually ignore at linux-generic implementation.
> The main problem is if 'num_queues' is '0' than 'odp_pktin_queue_config'
> return an error although it can be '0' if 'classifier_enable' is enabled.
>
> Regards,
> Liron
>
>
>
Re: [lng-odp] [EXT] Re: odp_pktin_queue_param_t - classifier_enable & num_queues
Yes. This is an implementation bug. Will fix the bug.
Minor correction, when classifier_enabled is set the current design is
to create all the queues in a single shot rather than as needed since
it will create network jitter if queues are created at random.
Regards,
Bala
On 3 October 2017 at 09:59, Bill Fischofer wrote:
> Thanks Liron, I've checked the code and that seems to be another issue. The
> intent is that when classifier_enable is set the ODP classifier creates
> queues as needed.
>
> +cc Bala for additional commentary.
>
> On Tue, Oct 3, 2017 at 8:48 AM, Liron Himi wrote:
>>
>> Hi Bill,
>>
>>
>>
>> I opened ‘https://bugs.linaro.org/show_bug.cgi?id=3289’ for this issue.
>>
>>
>>
>> I think there is another issue related to this topic in case PKTIO was
>> configure in schedule mode.
>>
>> The ‘start’ function assumes ‘num_in_queue’ is bigger than ‘0’ but in the
>> classifier_enable case it is ‘0’ so pktio is register with no real indexes,
>> actually in ‘index’ array there is garbage. (see code below)
>>
>>
>>
>> If I want to have a local fix, what do you think should be the number of
>> queues? ‘max_input_queues’ in capabilities ?
>>
>>
>>
>> Regards,
>>
>> Liron
>>
>>
>>
>>
>>
>> int odp_pktio_start(odp_pktio_t hdl)
>>
>> {
>>
>> …
>>
>> mode = entry->s.param.in_mode;
>>
>>
>>
>> if (mode == ODP_PKTIN_MODE_SCHED) {
>>
>> unsigned i;
>>
>> unsigned num = entry->s.num_in_queue;
>>
>> int index[num];
>>
>>
>>
>> for (i = 0; i < num; i++) {
>>
>> index[i] = i;
>>
>>
>>
>> if
>> (entry->s.in_queue[i].queue == ODP_QUEUE_INVALID) {
>>
>>
>> ODP_ERR("No input queue\n");
>>
>> return -1;
>>
>> }
>>
>> }
>>
>>
>>
>> sched_fn->pktio_start(pktio_to_id(hdl),
>> num, index);
>>
>> }
>>
>>
>>
>> return res;
>>
>> }
>>
>> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
>> Sent: Monday, October 02, 2017 14:52
>> To: Liron Himi
>> Cc: lng-odp@lists.linaro.org
>> Subject: [EXT] Re: [lng-odp] odp_pktin_queue_param_t - classifier_enable &
>> num_queues
>>
>>
>>
>> External Email
>>
>>
>>
>> Thanks Liron. This appears to be a bug in the implementation. It's only an
>> error for num_queues to be zero if hash_enabled is set.
>>
>>
>>
>> On Mon, Oct 2, 2017 at 6:02 AM, Liron Himi wrote:
>>
>> Hi,
>>
>> According to the API if 'classifier_enable' is enabled than 'num_queues'
>> and 'queue_param' should be ignored.
>> But looking at linux-generic implementation those parameters are always
>> being checked regardless of 'classifier_enable' value.
>> 'classifier_enable' is actually ignore at linux-generic implementation.
>> The main problem is if 'num_queues' is '0' than 'odp_pktin_queue_config'
>> return an error although it can be '0' if 'classifier_enable' is enabled.
>>
>> Regards,
>> Liron
>>
>>
>
>
Re: [lng-odp] [EXT] Re: odp_pktin_queue_param_t - classifier_enable & num_queues
Hi,
See comments inline
-Original Message-
From: Bala Manoharan [mailto:bala.manoha...@linaro.org]
Sent: Tuesday, October 03, 2017 21:13
To: Bill Fischofer
Cc: Liron Himi ; lng-odp@lists.linaro.org
Subject: Re: [EXT] Re: [lng-odp] odp_pktin_queue_param_t - classifier_enable &
num_queues
Yes. This is an implementation bug. Will fix the bug.
[L.H.] As part of the bug ticket I already opened? Or should I open new one?
Minor correction, when classifier_enabled is set the current design is to
create all the queues in a single shot rather than as needed since it will
create network jitter if queues are created at random.
Regards,
Bala
On 3 October 2017 at 09:59, Bill Fischofer wrote:
> Thanks Liron, I've checked the code and that seems to be another
> issue. The intent is that when classifier_enable is set the ODP
> classifier creates queues as needed.
[L.H.] Yes, this is true, when working with classifier user should create queue
and attach it to cos which in the end will be related to some pktio.
But the current scheduler mechanism expect that pktio will have at least one
“internal” input queue that will be registered with and by doing that will be
able to be triggered by the scheduler. See ‘sched_cb_pktin_poll’.
So I wonder if I can trick it and create one internal input queue and use it to
represent all my HW queues.
Seems like I can do it, but I saw that 'odp_pktin_event_queue' can be called if
PKTIO is in schedule mode, I wonder it can also be called if classifier is
enable?
>
> +cc Bala for additional commentary.
>
> On Tue, Oct 3, 2017 at 8:48 AM, Liron Himi wrote:
>>
>> Hi Bill,
>>
>>
>>
>> I opened ‘https://bugs.linaro.org/show_bug.cgi?id=3289’ for this issue.
>>
>>
>>
>> I think there is another issue related to this topic in case PKTIO
>> was configure in schedule mode.
>>
>> The ‘start’ function assumes ‘num_in_queue’ is bigger than ‘0’ but in
>> the classifier_enable case it is ‘0’ so pktio is register with no
>> real indexes, actually in ‘index’ array there is garbage. (see code
>> below)
>>
>>
>>
>> If I want to have a local fix, what do you think should be the number
>> of queues? ‘max_input_queues’ in capabilities ?
>>
>>
>>
>> Regards,
>>
>> Liron
>>
>>
>>
>>
>>
>> int odp_pktio_start(odp_pktio_t hdl)
>>
>> {
>>
>> …
>>
>> mode = entry->s.param.in_mode;
>>
>>
>>
>> if (mode == ODP_PKTIN_MODE_SCHED) {
>>
>> unsigned i;
>>
>> unsigned num = entry->s.num_in_queue;
>>
>> int index[num];
>>
>>
>>
>> for (i = 0; i < num; i++) {
>>
>> index[i] = i;
>>
>>
>>
>> if
>> (entry->s.in_queue[i].queue == ODP_QUEUE_INVALID) {
>>
>>
>> ODP_ERR("No input queue\n");
>>
>>
>> return -1;
>>
>> }
>>
>> }
>>
>>
>>
>>
>> sched_fn->pktio_start(pktio_to_id(hdl),
>> num, index);
>>
>> }
>>
>>
>>
>> return res;
>>
>> }
>>
>> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
>> Sent: Monday, October 02, 2017 14:52
>> To: Liron Himi
>> Cc: lng-odp@lists.linaro.org
>> Subject: [EXT] Re: [lng-odp] odp_pktin_queue_param_t -
>> classifier_enable & num_queues
>>
>>
>>
>> External Email
>>
>>
>>
>> Thanks Liron. This appears to be a bug in the implementation. It's
>> only an error for num_queues to be zero if hash_enabled is set.
>>
>>
>>
>> On Mon, Oct 2, 2017 at 6:02 AM, Liron Himi wrote:
>>
>> Hi,
>>
>> According to the API if 'classifier_enable' is enabled than 'num_queues'
>> and 'queue_param' should be ignored.
>> But looking at linux-generic implementation those parameters are
>> always being checked regardless of 'classifier_enable' value.
>> 'classifier_enable' is actually ignore at linux-generic implementation.
>> The main problem is if 'num_queues' is '0' than 'odp_pktin_queue_config'
>> return an error although it can be '0' if 'classifier_enable' is enabled.
>>
>> Regards,
>> Liron
>>
>>
>
>
