Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-19 Thread Honnappa Nagarahalli 
On 19 July 2017 at 22:15, Honnappa Nagarahalli
 wrote:
> On 19 July 2017 at 05:45, Elo, Matias (Nokia - FI/Espoo)
>  wrote:
>>
>>> On 19 Jul 2017, at 6:25, Honnappa Nagarahalli 
>>>  wrote:
>>>
>>> On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo)
>>>  wrote:

> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli 
>  wrote:
>
> On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
>  wrote:
>> Does this patch fix some real problem? At leas for me it only makes the 
>> scheduler interface harder to follow by spreading the functions into 
>> multiple headers.
>
> I have said this again and again. odp_schedule_if.h is a scheduler
> interface file. i.e this file is supposed to contain
> services/functions provided by scheduler to other components in ODP
> (similar to what has been done in odp_queue_if.h - appreciate if a
> closer attention is paid to this). Now, this file contains functions
> provided by packet I/O (among other things). Appreciate if you could
> explain why this file should contain these functions?
>
> Also, Petri has understood what this patch does, can you check with him?

 These functions are used by the schedulers to interface with other ODP
 components, so the scheduler_if.h is a logical place to define them. When
 implementing a new scheduler it's helpful to see all available  functions 
 from one
 place. I'm not fundamentally against this patch, but it's the task of the 
 patch
 submitter to justify why a change is needed, not the other way around.

 Petri was originally opposed to moving these functions into xyz_internal.h 
 headers,
 and only approved moving the functions into xyz_if.h files if it must be 
 done.

 I'm just trying to understand why this change is necessary. I patch like 
 this
 would be a lot easier to justify if it was sent as a part of the patch set 
 which requires
 this change. Without that, a more comprehensive commit log would be 
 helpful.
>>>
>>> Any suggestions on the commit message?
>>>
>>> Does adding the following sentence help?
>>>
>>> "odp_schedule_if.h is the scheduler interface file. i.e this file is
>>> supposed to contain services/functions provided by scheduler to other
>>> components in ODP"
>>
>> Based on your older message the motivation for moving these functions is 
>> that they are
>> related to the default scheduler and queue implementations. This would be a 
>> good point to
>> mention.
>>
>>>

 The naming of the odp_packet_io_if.h is now a bit confusing as we already 
 have the
 pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual 
 pktio interface.
>>>
>>> I definitely agree with you on this. That's how v1 of the patch was.
>>> It is changed after Petri's suggestion to move it to
>>> odp_packet_io_if.h.
>>
>> In a previous email Petri suggested filename odp_queue_sched_if.h. 
>> Extrapolating from this, odp_packet_io_if.h should be named 
>> odp_packet_io_sched_if.h.
>>
> odp_queue_sched_if.h is named such because it relates to queue and
> sched interface only and it is a tightly coupled interface. We have
> odp_queue_if.h for loosely coupled interfaces.
>
> The interface between scheduler and packet I/O is not a tightly
> coupled. Hence it was decided to name the file as odp_packet_io_if.h
> and in the future it can implement a modular interface.
>
> Petri is aware of this decision.
Refer to this discussion:
http://patches.opendataplane.org/patch/9322/. You can search for
"odp_packet_io_if.h"
>>

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-19 Thread Honnappa Nagarahalli 
On 19 July 2017 at 05:45, Elo, Matias (Nokia - FI/Espoo)
 wrote:
>
>> On 19 Jul 2017, at 6:25, Honnappa Nagarahalli 
>>  wrote:
>>
>> On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo)
>>  wrote:
>>>
 On 18 Jul 2017, at 6:58, Honnappa Nagarahalli 
  wrote:

 On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
  wrote:
> Does this patch fix some real problem? At leas for me it only makes the 
> scheduler interface harder to follow by spreading the functions into 
> multiple headers.

 I have said this again and again. odp_schedule_if.h is a scheduler
 interface file. i.e this file is supposed to contain
 services/functions provided by scheduler to other components in ODP
 (similar to what has been done in odp_queue_if.h - appreciate if a
 closer attention is paid to this). Now, this file contains functions
 provided by packet I/O (among other things). Appreciate if you could
 explain why this file should contain these functions?

 Also, Petri has understood what this patch does, can you check with him?
>>>
>>> These functions are used by the schedulers to interface with other ODP
>>> components, so the scheduler_if.h is a logical place to define them. When
>>> implementing a new scheduler it's helpful to see all available  functions 
>>> from one
>>> place. I'm not fundamentally against this patch, but it's the task of the 
>>> patch
>>> submitter to justify why a change is needed, not the other way around.
>>>
>>> Petri was originally opposed to moving these functions into xyz_internal.h 
>>> headers,
>>> and only approved moving the functions into xyz_if.h files if it must be 
>>> done.
>>>
>>> I'm just trying to understand why this change is necessary. I patch like 
>>> this
>>> would be a lot easier to justify if it was sent as a part of the patch set 
>>> which requires
>>> this change. Without that, a more comprehensive commit log would be helpful.
>>
>> Any suggestions on the commit message?
>>
>> Does adding the following sentence help?
>>
>> "odp_schedule_if.h is the scheduler interface file. i.e this file is
>> supposed to contain services/functions provided by scheduler to other
>> components in ODP"
>
> Based on your older message the motivation for moving these functions is that 
> they are
> related to the default scheduler and queue implementations. This would be a 
> good point to
> mention.
>
>>
>>>
>>> The naming of the odp_packet_io_if.h is now a bit confusing as we already 
>>> have the
>>> pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual 
>>> pktio interface.
>>
>> I definitely agree with you on this. That's how v1 of the patch was.
>> It is changed after Petri's suggestion to move it to
>> odp_packet_io_if.h.
>
> In a previous email Petri suggested filename odp_queue_sched_if.h. 
> Extrapolating from this, odp_packet_io_if.h should be named 
> odp_packet_io_sched_if.h.
>
odp_queue_sched_if.h is named such because it relates to queue and
sched interface only and it is a tightly coupled interface. We have
odp_queue_if.h for loosely coupled interfaces.

The interface between scheduler and packet I/O is not a tightly
coupled. Hence it was decided to name the file as odp_packet_io_if.h
and in the future it can implement a modular interface.

Petri is aware of this decision.
>

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-19 Thread Elo, Matias (Nokia - FI/Espoo) 

> On 19 Jul 2017, at 6:25, Honnappa Nagarahalli 
>  wrote:
> 
> On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo)
>  wrote:
>> 
>>> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli 
>>>  wrote:
>>> 
>>> On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
>>>  wrote:
 Does this patch fix some real problem? At leas for me it only makes the 
 scheduler interface harder to follow by spreading the functions into 
 multiple headers.
>>> 
>>> I have said this again and again. odp_schedule_if.h is a scheduler
>>> interface file. i.e this file is supposed to contain
>>> services/functions provided by scheduler to other components in ODP
>>> (similar to what has been done in odp_queue_if.h - appreciate if a
>>> closer attention is paid to this). Now, this file contains functions
>>> provided by packet I/O (among other things). Appreciate if you could
>>> explain why this file should contain these functions?
>>> 
>>> Also, Petri has understood what this patch does, can you check with him?
>> 
>> These functions are used by the schedulers to interface with other ODP
>> components, so the scheduler_if.h is a logical place to define them. When
>> implementing a new scheduler it's helpful to see all available  functions 
>> from one
>> place. I'm not fundamentally against this patch, but it's the task of the 
>> patch
>> submitter to justify why a change is needed, not the other way around.
>> 
>> Petri was originally opposed to moving these functions into xyz_internal.h 
>> headers,
>> and only approved moving the functions into xyz_if.h files if it must be 
>> done.
>> 
>> I'm just trying to understand why this change is necessary. I patch like this
>> would be a lot easier to justify if it was sent as a part of the patch set 
>> which requires
>> this change. Without that, a more comprehensive commit log would be helpful.
> 
> Any suggestions on the commit message?
> 
> Does adding the following sentence help?
> 
> "odp_schedule_if.h is the scheduler interface file. i.e this file is
> supposed to contain services/functions provided by scheduler to other
> components in ODP"

Based on your older message the motivation for moving these functions is that 
they are 
related to the default scheduler and queue implementations. This would be a 
good point to
mention.

> 
>> 
>> The naming of the odp_packet_io_if.h is now a bit confusing as we already 
>> have the
>> pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual pktio 
>> interface.
> 
> I definitely agree with you on this. That's how v1 of the patch was.
> It is changed after Petri's suggestion to move it to
> odp_packet_io_if.h.

In a previous email Petri suggested filename odp_queue_sched_if.h. 
Extrapolating from this, odp_packet_io_if.h should be named 
odp_packet_io_sched_if.h.

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-18 Thread Honnappa Nagarahalli 
On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo)
 wrote:
>
>> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli 
>>  wrote:
>>
>> On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
>>  wrote:
>>> Does this patch fix some real problem? At leas for me it only makes the 
>>> scheduler interface harder to follow by spreading the functions into 
>>> multiple headers.
>>
>> I have said this again and again. odp_schedule_if.h is a scheduler
>> interface file. i.e this file is supposed to contain
>> services/functions provided by scheduler to other components in ODP
>> (similar to what has been done in odp_queue_if.h - appreciate if a
>> closer attention is paid to this). Now, this file contains functions
>> provided by packet I/O (among other things). Appreciate if you could
>> explain why this file should contain these functions?
>>
>> Also, Petri has understood what this patch does, can you check with him?
>
> These functions are used by the schedulers to interface with other ODP
> components, so the scheduler_if.h is a logical place to define them. When
> implementing a new scheduler it's helpful to see all available  functions 
> from one
> place. I'm not fundamentally against this patch, but it's the task of the 
> patch
> submitter to justify why a change is needed, not the other way around.
>
> Petri was originally opposed to moving these functions into xyz_internal.h 
> headers,
> and only approved moving the functions into xyz_if.h files if it must be done.
>
> I'm just trying to understand why this change is necessary. I patch like this
> would be a lot easier to justify if it was sent as a part of the patch set 
> which requires
> this change. Without that, a more comprehensive commit log would be helpful.

Any suggestions on the commit message?

Does adding the following sentence help?

"odp_schedule_if.h is the scheduler interface file. i.e this file is
supposed to contain services/functions provided by scheduler to other
components in ODP"

>
> The naming of the odp_packet_io_if.h is now a bit confusing as we already 
> have the
> pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual pktio 
> interface.

I definitely agree with you on this. That's how v1 of the patch was.
It is changed after Petri's suggestion to move it to
odp_packet_io_if.h.

>
> -Matias
>

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-18 Thread Elo, Matias (Nokia - FI/Espoo) 

> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli 
>  wrote:
> 
> On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
>  wrote:
>> Does this patch fix some real problem? At leas for me it only makes the 
>> scheduler interface harder to follow by spreading the functions into 
>> multiple headers.
> 
> I have said this again and again. odp_schedule_if.h is a scheduler
> interface file. i.e this file is supposed to contain
> services/functions provided by scheduler to other components in ODP
> (similar to what has been done in odp_queue_if.h - appreciate if a
> closer attention is paid to this). Now, this file contains functions
> provided by packet I/O (among other things). Appreciate if you could
> explain why this file should contain these functions?
> 
> Also, Petri has understood what this patch does, can you check with him?

These functions are used by the schedulers to interface with other ODP
components, so the scheduler_if.h is a logical place to define them. When
implementing a new scheduler it's helpful to see all available  functions from 
one
place. I'm not fundamentally against this patch, but it's the task of the patch
submitter to justify why a change is needed, not the other way around.

Petri was originally opposed to moving these functions into xyz_internal.h 
headers,
and only approved moving the functions into xyz_if.h files if it must be done.

I'm just trying to understand why this change is necessary. I patch like this
would be a lot easier to justify if it was sent as a part of the patch set 
which requires
this change. Without that, a more comprehensive commit log would be helpful.

The naming of the odp_packet_io_if.h is now a bit confusing as we already have 
the
pktio_if_ops_t struct in odp_packet_io_internal.h, which is the actual pktio 
interface.

-Matias

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-17 Thread Honnappa Nagarahalli 
On 17 July 2017 at 14:46, Bill Fischofer  wrote:
> On Mon, Jul 17, 2017 at 4:23 AM, Elo, Matias (Nokia - FI/Espoo) <
> matias@nokia.com> wrote:
>
>> Does this patch fix some real problem? At leas for me it only makes the
>> scheduler interface harder to follow by spreading the functions into
>> multiple headers.
>>
>
> Is this perhaps related to Yi's modular framework? Since that is providing
> a more fundamental structure, I'd like to see how we get to that. In any
> event, I'll add this to the agenda for tomorrow's public call. Joyce, can
> you join us for that at 15:00 UTC?
>
Joyce will not be able to join the call. We can discuss on next Monday
(7/24) if required.
>
>>
>> -Matias
>>
>>
>> > On 17 Jul 2017, at 10:26, Joyce Kong  wrote:
>> >
>> > The modular scheduler interface in odp_schedule_if.h includes functions
>> from
>> > pktio and queue. It needs to be cleaned up.
>> >
>> > Signed-off-by: Joyce Kong 
>> > ---
>> > platform/linux-generic/Makefile.am |  2 ++
>> > platform/linux-generic/include/odp_packet_io_if.h  | 23
>> +
>> > .../linux-generic/include/odp_queue_sched_if.h | 24
>> ++
>> > platform/linux-generic/include/odp_schedule_if.h   |  9 
>> > platform/linux-generic/odp_packet_io.c |  1 +
>> > platform/linux-generic/odp_queue.c |  1 +
>> > platform/linux-generic/odp_schedule.c  |  2 ++
>> > platform/linux-generic/odp_schedule_iquery.c   |  2 ++
>> > platform/linux-generic/odp_schedule_sp.c   |  2 ++
>> > 9 files changed, 57 insertions(+), 9 deletions(-)
>> > create mode 100644 platform/linux-generic/include/odp_packet_io_if.h
>> > create mode 100644 platform/linux-generic/include/odp_queue_sched_if.h
>> >
>> > diff --git a/platform/linux-generic/Makefile.am
>> b/platform/linux-generic/Makefile.am
>> > index 26eba28..5295abb 100644
>> > --- a/platform/linux-generic/Makefile.am
>> > +++ b/platform/linux-generic/Makefile.am
>> > @@ -150,6 +150,7 @@ noinst_HEADERS = \
>> > ${srcdir}/include/odp_packet_io_internal.h \
>> > ${srcdir}/include/odp_packet_io_ipc_internal.h \
>> > ${srcdir}/include/odp_packet_io_ring_internal.h \
>> > +   ${srcdir}/include/odp_packet_io_if.h \
>> > ${srcdir}/include/odp_packet_netmap.h \
>> > ${srcdir}/include/odp_packet_dpdk.h \
>> > ${srcdir}/include/odp_packet_socket.h \
>> > @@ -160,6 +161,7 @@ noinst_HEADERS = \
>> > ${srcdir}/include/odp_queue_internal.h \
>> > ${srcdir}/include/odp_ring_internal.h \
>> > ${srcdir}/include/odp_queue_if.h \
>> > +   ${srcdir}/include/odp_queue_sched_if.h \
>> > ${srcdir}/include/odp_schedule_if.h \
>> > ${srcdir}/include/odp_sorted_list_internal.h \
>> > ${srcdir}/include/odp_shm_internal.h \
>> > diff --git a/platform/linux-generic/include/odp_packet_io_if.h
>> b/platform/linux-generic/include/odp_packet_io_if.h
>> > new file mode 100644
>> > index 000..e574f22
>> > --- /dev/null
>> > +++ b/platform/linux-generic/include/odp_packet_io_if.h
>> > @@ -0,0 +1,23 @@
>> > +/* Copyright (c) 2017, ARM Limited
>> > + * All rights reserved.
>> > + *
>> > + *SPDX-License-Identifier:   BSD-3-Clause
>> > + */
>> > +
>> > +#ifndef ODP_PACKET_IO_IF_H_
>> > +#define ODP_PACKET_IO_IF_H_
>> > +
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> > +/* Interface for the scheduler */
>> > +int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
>> > +void sched_cb_pktio_stop_finalize(int pktio_index);
>> > +int sched_cb_num_pktio(void);
>> > +
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> > +
>> > +#endif
>> > diff --git a/platform/linux-generic/include/odp_queue_sched_if.h
>> b/platform/linux-generic/include/odp_queue_sched_if.h
>> > new file mode 100644
>> > index 000..4a301f4
>> > --- /dev/null
>> > +++ b/platform/linux-generic/include/odp_queue_sched_if.h
>> > @@ -0,0 +1,24 @@
>> > +/* Copyright (c) 2017, ARM Limited
>> > + * All rights reserved.
>> > + *
>> > + *SPDX-License-Identifier:   BSD-3-Clause
>> > + */
>> > +
>> > +#ifndef ODP_QUEUE_SCHED_IF_H_
>> > +#define ODP_QUEUE_SCHED_IF_H_
>> > +
>> > +#ifdef __cplusplus
>> > +extern "C" {
>> > +#endif
>> > +
>> > +/* Interface for the scheduler */
>> > +odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
>> > +void sched_cb_queue_destroy_finalize(uint32_t queue_index);
>> > +int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[],
>> int num);
>> > +int sched_cb_queue_empty(uint32_t queue_index);
>> > +
>> > +#ifdef __cplusplus
>> > +}
>> > +#endif
>> > +
>> > +#endif
>> > diff --git a/platform/linux-generic/include/odp_schedule_if.h
>> b/platform/linux-generic/include/odp_schedule_if.h
>> > index 4cd8c3e..9a1f3ff 100644
>> > ---

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-17 Thread Honnappa Nagarahalli 
On 17 July 2017 at 04:23, Elo, Matias (Nokia - FI/Espoo)
 wrote:
> Does this patch fix some real problem? At leas for me it only makes the 
> scheduler interface harder to follow by spreading the functions into multiple 
> headers.

I have said this again and again. odp_schedule_if.h is a scheduler
interface file. i.e this file is supposed to contain
services/functions provided by scheduler to other components in ODP
(similar to what has been done in odp_queue_if.h - appreciate if a
closer attention is paid to this). Now, this file contains functions
provided by packet I/O (among other things). Appreciate if you could
explain why this file should contain these functions?

Also, Petri has understood what this patch does, can you check with him?

>
> -Matias
>
>
>> On 17 Jul 2017, at 10:26, Joyce Kong  wrote:
>>
>> The modular scheduler interface in odp_schedule_if.h includes functions from
>> pktio and queue. It needs to be cleaned up.
>>
>> Signed-off-by: Joyce Kong 
>> ---
>> platform/linux-generic/Makefile.am |  2 ++
>> platform/linux-generic/include/odp_packet_io_if.h  | 23 +
>> .../linux-generic/include/odp_queue_sched_if.h | 24 
>> ++
>> platform/linux-generic/include/odp_schedule_if.h   |  9 
>> platform/linux-generic/odp_packet_io.c |  1 +
>> platform/linux-generic/odp_queue.c |  1 +
>> platform/linux-generic/odp_schedule.c  |  2 ++
>> platform/linux-generic/odp_schedule_iquery.c   |  2 ++
>> platform/linux-generic/odp_schedule_sp.c   |  2 ++
>> 9 files changed, 57 insertions(+), 9 deletions(-)
>> create mode 100644 platform/linux-generic/include/odp_packet_io_if.h
>> create mode 100644 platform/linux-generic/include/odp_queue_sched_if.h
>>
>> diff --git a/platform/linux-generic/Makefile.am 
>> b/platform/linux-generic/Makefile.am
>> index 26eba28..5295abb 100644
>> --- a/platform/linux-generic/Makefile.am
>> +++ b/platform/linux-generic/Makefile.am
>> @@ -150,6 +150,7 @@ noinst_HEADERS = \
>> ${srcdir}/include/odp_packet_io_internal.h \
>> ${srcdir}/include/odp_packet_io_ipc_internal.h \
>> ${srcdir}/include/odp_packet_io_ring_internal.h \
>> +   ${srcdir}/include/odp_packet_io_if.h \
>> ${srcdir}/include/odp_packet_netmap.h \
>> ${srcdir}/include/odp_packet_dpdk.h \
>> ${srcdir}/include/odp_packet_socket.h \
>> @@ -160,6 +161,7 @@ noinst_HEADERS = \
>> ${srcdir}/include/odp_queue_internal.h \
>> ${srcdir}/include/odp_ring_internal.h \
>> ${srcdir}/include/odp_queue_if.h \
>> +   ${srcdir}/include/odp_queue_sched_if.h \
>> ${srcdir}/include/odp_schedule_if.h \
>> ${srcdir}/include/odp_sorted_list_internal.h \
>> ${srcdir}/include/odp_shm_internal.h \
>> diff --git a/platform/linux-generic/include/odp_packet_io_if.h 
>> b/platform/linux-generic/include/odp_packet_io_if.h
>> new file mode 100644
>> index 000..e574f22
>> --- /dev/null
>> +++ b/platform/linux-generic/include/odp_packet_io_if.h
>> @@ -0,0 +1,23 @@
>> +/* Copyright (c) 2017, ARM Limited
>> + * All rights reserved.
>> + *
>> + *SPDX-License-Identifier:   BSD-3-Clause
>> + */
>> +
>> +#ifndef ODP_PACKET_IO_IF_H_
>> +#define ODP_PACKET_IO_IF_H_
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/* Interface for the scheduler */
>> +int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
>> +void sched_cb_pktio_stop_finalize(int pktio_index);
>> +int sched_cb_num_pktio(void);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/platform/linux-generic/include/odp_queue_sched_if.h 
>> b/platform/linux-generic/include/odp_queue_sched_if.h
>> new file mode 100644
>> index 000..4a301f4
>> --- /dev/null
>> +++ b/platform/linux-generic/include/odp_queue_sched_if.h
>> @@ -0,0 +1,24 @@
>> +/* Copyright (c) 2017, ARM Limited
>> + * All rights reserved.
>> + *
>> + *SPDX-License-Identifier:   BSD-3-Clause
>> + */
>> +
>> +#ifndef ODP_QUEUE_SCHED_IF_H_
>> +#define ODP_QUEUE_SCHED_IF_H_
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +/* Interface for the scheduler */
>> +odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
>> +void sched_cb_queue_destroy_finalize(uint32_t queue_index);
>> +int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[], int 
>> num);
>> +int sched_cb_queue_empty(uint32_t queue_index);
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/platform/linux-generic/include/odp_schedule_if.h 
>> b/platform/linux-generic/include/odp_schedule_if.h
>> index 4cd8c3e..9a1f3ff 100644
>> --- a/platform/linux-generic/include/odp_schedule_if.h
>> +++ b/platform/linux-generic/include/odp_schedule_if.h
>> @@ -64,15 +64,6 @@ typedef struct

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-17 Thread Bill Fischofer 
On Mon, Jul 17, 2017 at 4:23 AM, Elo, Matias (Nokia - FI/Espoo) <
matias@nokia.com> wrote:

> Does this patch fix some real problem? At leas for me it only makes the
> scheduler interface harder to follow by spreading the functions into
> multiple headers.
>

Is this perhaps related to Yi's modular framework? Since that is providing
a more fundamental structure, I'd like to see how we get to that. In any
event, I'll add this to the agenda for tomorrow's public call. Joyce, can
you join us for that at 15:00 UTC?


>
> -Matias
>
>
> > On 17 Jul 2017, at 10:26, Joyce Kong  wrote:
> >
> > The modular scheduler interface in odp_schedule_if.h includes functions
> from
> > pktio and queue. It needs to be cleaned up.
> >
> > Signed-off-by: Joyce Kong 
> > ---
> > platform/linux-generic/Makefile.am |  2 ++
> > platform/linux-generic/include/odp_packet_io_if.h  | 23
> +
> > .../linux-generic/include/odp_queue_sched_if.h | 24
> ++
> > platform/linux-generic/include/odp_schedule_if.h   |  9 
> > platform/linux-generic/odp_packet_io.c |  1 +
> > platform/linux-generic/odp_queue.c |  1 +
> > platform/linux-generic/odp_schedule.c  |  2 ++
> > platform/linux-generic/odp_schedule_iquery.c   |  2 ++
> > platform/linux-generic/odp_schedule_sp.c   |  2 ++
> > 9 files changed, 57 insertions(+), 9 deletions(-)
> > create mode 100644 platform/linux-generic/include/odp_packet_io_if.h
> > create mode 100644 platform/linux-generic/include/odp_queue_sched_if.h
> >
> > diff --git a/platform/linux-generic/Makefile.am
> b/platform/linux-generic/Makefile.am
> > index 26eba28..5295abb 100644
> > --- a/platform/linux-generic/Makefile.am
> > +++ b/platform/linux-generic/Makefile.am
> > @@ -150,6 +150,7 @@ noinst_HEADERS = \
> > ${srcdir}/include/odp_packet_io_internal.h \
> > ${srcdir}/include/odp_packet_io_ipc_internal.h \
> > ${srcdir}/include/odp_packet_io_ring_internal.h \
> > +   ${srcdir}/include/odp_packet_io_if.h \
> > ${srcdir}/include/odp_packet_netmap.h \
> > ${srcdir}/include/odp_packet_dpdk.h \
> > ${srcdir}/include/odp_packet_socket.h \
> > @@ -160,6 +161,7 @@ noinst_HEADERS = \
> > ${srcdir}/include/odp_queue_internal.h \
> > ${srcdir}/include/odp_ring_internal.h \
> > ${srcdir}/include/odp_queue_if.h \
> > +   ${srcdir}/include/odp_queue_sched_if.h \
> > ${srcdir}/include/odp_schedule_if.h \
> > ${srcdir}/include/odp_sorted_list_internal.h \
> > ${srcdir}/include/odp_shm_internal.h \
> > diff --git a/platform/linux-generic/include/odp_packet_io_if.h
> b/platform/linux-generic/include/odp_packet_io_if.h
> > new file mode 100644
> > index 000..e574f22
> > --- /dev/null
> > +++ b/platform/linux-generic/include/odp_packet_io_if.h
> > @@ -0,0 +1,23 @@
> > +/* Copyright (c) 2017, ARM Limited
> > + * All rights reserved.
> > + *
> > + *SPDX-License-Identifier:   BSD-3-Clause
> > + */
> > +
> > +#ifndef ODP_PACKET_IO_IF_H_
> > +#define ODP_PACKET_IO_IF_H_
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/* Interface for the scheduler */
> > +int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
> > +void sched_cb_pktio_stop_finalize(int pktio_index);
> > +int sched_cb_num_pktio(void);
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/platform/linux-generic/include/odp_queue_sched_if.h
> b/platform/linux-generic/include/odp_queue_sched_if.h
> > new file mode 100644
> > index 000..4a301f4
> > --- /dev/null
> > +++ b/platform/linux-generic/include/odp_queue_sched_if.h
> > @@ -0,0 +1,24 @@
> > +/* Copyright (c) 2017, ARM Limited
> > + * All rights reserved.
> > + *
> > + *SPDX-License-Identifier:   BSD-3-Clause
> > + */
> > +
> > +#ifndef ODP_QUEUE_SCHED_IF_H_
> > +#define ODP_QUEUE_SCHED_IF_H_
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/* Interface for the scheduler */
> > +odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
> > +void sched_cb_queue_destroy_finalize(uint32_t queue_index);
> > +int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[],
> int num);
> > +int sched_cb_queue_empty(uint32_t queue_index);
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif
> > diff --git a/platform/linux-generic/include/odp_schedule_if.h
> b/platform/linux-generic/include/odp_schedule_if.h
> > index 4cd8c3e..9a1f3ff 100644
> > --- a/platform/linux-generic/include/odp_schedule_if.h
> > +++ b/platform/linux-generic/include/odp_schedule_if.h
> > @@ -64,15 +64,6 @@ typedef struct schedule_fn_t {
> > /* Interface towards the scheduler */
> > extern const schedule_fn_t *sched_fn;
> >
> > -/* Interface for the scheduler */
> > -int

Re: [lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-17 Thread Elo, Matias (Nokia - FI/Espoo) 
Does this patch fix some real problem? At leas for me it only makes the 
scheduler interface harder to follow by spreading the functions into multiple 
headers.

-Matias


> On 17 Jul 2017, at 10:26, Joyce Kong  wrote:
> 
> The modular scheduler interface in odp_schedule_if.h includes functions from
> pktio and queue. It needs to be cleaned up.
> 
> Signed-off-by: Joyce Kong 
> ---
> platform/linux-generic/Makefile.am |  2 ++
> platform/linux-generic/include/odp_packet_io_if.h  | 23 +
> .../linux-generic/include/odp_queue_sched_if.h | 24 ++
> platform/linux-generic/include/odp_schedule_if.h   |  9 
> platform/linux-generic/odp_packet_io.c |  1 +
> platform/linux-generic/odp_queue.c |  1 +
> platform/linux-generic/odp_schedule.c  |  2 ++
> platform/linux-generic/odp_schedule_iquery.c   |  2 ++
> platform/linux-generic/odp_schedule_sp.c   |  2 ++
> 9 files changed, 57 insertions(+), 9 deletions(-)
> create mode 100644 platform/linux-generic/include/odp_packet_io_if.h
> create mode 100644 platform/linux-generic/include/odp_queue_sched_if.h
> 
> diff --git a/platform/linux-generic/Makefile.am 
> b/platform/linux-generic/Makefile.am
> index 26eba28..5295abb 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -150,6 +150,7 @@ noinst_HEADERS = \
> ${srcdir}/include/odp_packet_io_internal.h \
> ${srcdir}/include/odp_packet_io_ipc_internal.h \
> ${srcdir}/include/odp_packet_io_ring_internal.h \
> +   ${srcdir}/include/odp_packet_io_if.h \
> ${srcdir}/include/odp_packet_netmap.h \
> ${srcdir}/include/odp_packet_dpdk.h \
> ${srcdir}/include/odp_packet_socket.h \
> @@ -160,6 +161,7 @@ noinst_HEADERS = \
> ${srcdir}/include/odp_queue_internal.h \
> ${srcdir}/include/odp_ring_internal.h \
> ${srcdir}/include/odp_queue_if.h \
> +   ${srcdir}/include/odp_queue_sched_if.h \
> ${srcdir}/include/odp_schedule_if.h \
> ${srcdir}/include/odp_sorted_list_internal.h \
> ${srcdir}/include/odp_shm_internal.h \
> diff --git a/platform/linux-generic/include/odp_packet_io_if.h 
> b/platform/linux-generic/include/odp_packet_io_if.h
> new file mode 100644
> index 000..e574f22
> --- /dev/null
> +++ b/platform/linux-generic/include/odp_packet_io_if.h
> @@ -0,0 +1,23 @@
> +/* Copyright (c) 2017, ARM Limited
> + * All rights reserved.
> + *
> + *SPDX-License-Identifier:   BSD-3-Clause
> + */
> +
> +#ifndef ODP_PACKET_IO_IF_H_
> +#define ODP_PACKET_IO_IF_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Interface for the scheduler */
> +int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
> +void sched_cb_pktio_stop_finalize(int pktio_index);
> +int sched_cb_num_pktio(void);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/platform/linux-generic/include/odp_queue_sched_if.h 
> b/platform/linux-generic/include/odp_queue_sched_if.h
> new file mode 100644
> index 000..4a301f4
> --- /dev/null
> +++ b/platform/linux-generic/include/odp_queue_sched_if.h
> @@ -0,0 +1,24 @@
> +/* Copyright (c) 2017, ARM Limited
> + * All rights reserved.
> + *
> + *SPDX-License-Identifier:   BSD-3-Clause
> + */
> +
> +#ifndef ODP_QUEUE_SCHED_IF_H_
> +#define ODP_QUEUE_SCHED_IF_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Interface for the scheduler */
> +odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
> +void sched_cb_queue_destroy_finalize(uint32_t queue_index);
> +int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[], int 
> num);
> +int sched_cb_queue_empty(uint32_t queue_index);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/platform/linux-generic/include/odp_schedule_if.h 
> b/platform/linux-generic/include/odp_schedule_if.h
> index 4cd8c3e..9a1f3ff 100644
> --- a/platform/linux-generic/include/odp_schedule_if.h
> +++ b/platform/linux-generic/include/odp_schedule_if.h
> @@ -64,15 +64,6 @@ typedef struct schedule_fn_t {
> /* Interface towards the scheduler */
> extern const schedule_fn_t *sched_fn;
> 
> -/* Interface for the scheduler */
> -int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
> -void sched_cb_pktio_stop_finalize(int pktio_index);
> -int sched_cb_num_pktio(void);
> -odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
> -void sched_cb_queue_destroy_finalize(uint32_t queue_index);
> -int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[], int 
> num);
> -int sched_cb_queue_empty(uint32_t queue_index);
> -
> /* API functions */
> typedef struct {
>   uint64_t (*schedule_wait_time)(uint64_t);
> diff --git a/platform/linux-generic/odp_packet_io.c 
>

[lng-odp] [PATCHv3] linux-gen: scheduler: clean up odp_scheduler_if.h

2017-07-17 Thread Joyce Kong 
The modular scheduler interface in odp_schedule_if.h includes functions from
pktio and queue. It needs to be cleaned up.

Signed-off-by: Joyce Kong 
---
 platform/linux-generic/Makefile.am |  2 ++
 platform/linux-generic/include/odp_packet_io_if.h  | 23 +
 .../linux-generic/include/odp_queue_sched_if.h | 24 ++
 platform/linux-generic/include/odp_schedule_if.h   |  9 
 platform/linux-generic/odp_packet_io.c |  1 +
 platform/linux-generic/odp_queue.c |  1 +
 platform/linux-generic/odp_schedule.c  |  2 ++
 platform/linux-generic/odp_schedule_iquery.c   |  2 ++
 platform/linux-generic/odp_schedule_sp.c   |  2 ++
 9 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 platform/linux-generic/include/odp_packet_io_if.h
 create mode 100644 platform/linux-generic/include/odp_queue_sched_if.h

diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 26eba28..5295abb 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -150,6 +150,7 @@ noinst_HEADERS = \
  ${srcdir}/include/odp_packet_io_internal.h \
  ${srcdir}/include/odp_packet_io_ipc_internal.h \
  ${srcdir}/include/odp_packet_io_ring_internal.h \
+ ${srcdir}/include/odp_packet_io_if.h \
  ${srcdir}/include/odp_packet_netmap.h \
  ${srcdir}/include/odp_packet_dpdk.h \
  ${srcdir}/include/odp_packet_socket.h \
@@ -160,6 +161,7 @@ noinst_HEADERS = \
  ${srcdir}/include/odp_queue_internal.h \
  ${srcdir}/include/odp_ring_internal.h \
  ${srcdir}/include/odp_queue_if.h \
+ ${srcdir}/include/odp_queue_sched_if.h \
  ${srcdir}/include/odp_schedule_if.h \
  ${srcdir}/include/odp_sorted_list_internal.h \
  ${srcdir}/include/odp_shm_internal.h \
diff --git a/platform/linux-generic/include/odp_packet_io_if.h 
b/platform/linux-generic/include/odp_packet_io_if.h
new file mode 100644
index 000..e574f22
--- /dev/null
+++ b/platform/linux-generic/include/odp_packet_io_if.h
@@ -0,0 +1,23 @@
+/* Copyright (c) 2017, ARM Limited
+ * All rights reserved.
+ *
+ *SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef ODP_PACKET_IO_IF_H_
+#define ODP_PACKET_IO_IF_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Interface for the scheduler */
+int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
+void sched_cb_pktio_stop_finalize(int pktio_index);
+int sched_cb_num_pktio(void);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/platform/linux-generic/include/odp_queue_sched_if.h 
b/platform/linux-generic/include/odp_queue_sched_if.h
new file mode 100644
index 000..4a301f4
--- /dev/null
+++ b/platform/linux-generic/include/odp_queue_sched_if.h
@@ -0,0 +1,24 @@
+/* Copyright (c) 2017, ARM Limited
+ * All rights reserved.
+ *
+ *SPDX-License-Identifier: BSD-3-Clause
+ */
+
+#ifndef ODP_QUEUE_SCHED_IF_H_
+#define ODP_QUEUE_SCHED_IF_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Interface for the scheduler */
+odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
+void sched_cb_queue_destroy_finalize(uint32_t queue_index);
+int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[], int num);
+int sched_cb_queue_empty(uint32_t queue_index);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/platform/linux-generic/include/odp_schedule_if.h 
b/platform/linux-generic/include/odp_schedule_if.h
index 4cd8c3e..9a1f3ff 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -64,15 +64,6 @@ typedef struct schedule_fn_t {
 /* Interface towards the scheduler */
 extern const schedule_fn_t *sched_fn;
 
-/* Interface for the scheduler */
-int sched_cb_pktin_poll(int pktio_index, int num_queue, int index[]);
-void sched_cb_pktio_stop_finalize(int pktio_index);
-int sched_cb_num_pktio(void);
-odp_queue_t sched_cb_queue_handle(uint32_t queue_index);
-void sched_cb_queue_destroy_finalize(uint32_t queue_index);
-int sched_cb_queue_deq_multi(uint32_t queue_index, odp_event_t ev[], int num);
-int sched_cb_queue_empty(uint32_t queue_index);
-
 /* API functions */
 typedef struct {
uint64_t (*schedule_wait_time)(uint64_t);
diff --git a/platform/linux-generic/odp_packet_io.c 
b/platform/linux-generic/odp_packet_io.c
index 6a181f5..8a14c85 100644
--- a/platform/linux-generic/odp_packet_io.c
+++ b/platform/linux-generic/odp_packet_io.c
@@ -7,6 +7,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/platform/linux-generic/odp_queue.c 
b/platform/linux-generic/odp_queue.c
index d52814b..67b3a07 100644
--- a/platform/linux-generic/odp_queue.c
+++ b/platform/linux-generic/odp_queue.c
@@ -7,6 +7,7 @@