On 18 July 2017 at 06:37, Elo, Matias (Nokia - FI/Espoo) <matias....@nokia.com> wrote: > >> On 18 Jul 2017, at 6:58, Honnappa Nagarahalli >> <honnappa.nagaraha...@linaro.org> wrote: >> >> On 17 July 2017 at 04:23, 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. >> >> 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 >
