On 23 November 2015 at 04:16, Maxim Uvarov <[email protected]> wrote:
> On 11/21/2015 00:13, Mike Holmes wrote: > >> >> >> On 20 November 2015 at 13:41, Maxim Uvarov <[email protected] >> <mailto:[email protected]>> wrote: >> >> Signed-off-by: Maxim Uvarov <[email protected] >> <mailto:[email protected]>> >> --- >> platform/linux-generic/Makefile.am | 4 + >> .../linux-generic/include/odp_buffer_internal.h | 3 + >> .../linux-generic/include/odp_packet_io_internal.h | 38 ++ >> .../include/odp_packet_io_ipc_internal.h | 47 ++ >> platform/linux-generic/include/odp_shm_internal.h | 21 + >> platform/linux-generic/odp_packet_io.c | 1 + >> platform/linux-generic/odp_pool.c | 14 +- >> platform/linux-generic/odp_shared_memory.c | 14 +- >> platform/linux-generic/pktio/io_ops.c | 1 + >> platform/linux-generic/pktio/ipc.c | 729 >> +++++++++++++++++++++ >> platform/linux-generic/pktio/ring.c | 1 + >> 11 files changed, 868 insertions(+), 5 deletions(-) >> create mode 100644 >> platform/linux-generic/include/odp_packet_io_ipc_internal.h >> create mode 100644 platform/linux-generic/include/odp_shm_internal.h >> create mode 100644 platform/linux-generic/pktio/ipc.c >> create mode 120000 platform/linux-generic/pktio/ring.c >> >> >> The file ring.c is a symlink, >> > > Right. That was decided on some meeting, some time ago. Instead of having > 2 copies on one file, do symlink. It was a probably not well thought though then, do you have a link to the discussion, maybe we left a note why we agreed. > > > > we dont want the functions defined in odph_* in linux-generic. We dont >> want these functions in two libraries and we dont want to force platforms >> reusing linux-generic to be forced to meet these new dependencies on the >> helpers code such as #include "odph_debug.h" >> >> > When you say "we" I don't really understand who is answering :) > We = the odp community. I cant see how an odp helper function can have an implementation in linux-generic, clearly it should then be in the Linux generic implementation fully as that is the only documented user of that code, or it is linked library like openSSL. > > --- >> >> In my opinion helpers is already an ugly mix of code to help three >> different use cases: >> >> * applications >> * linux-generic >> * support test framework >> >> The linux-generic implementation should not depend helpers. >> > > That is not a requirement. It may depend or may not. Criteria is logical > use and re-usage of code. It's the same as linux-generic depends > on openssl, ethtool, netmap and etc. Linux-generic is free to use anything > also as algorithms from helpers. Reuse is good, yes it could use helpers, but it must link to them then, in all the other cases we do that, it is the mechanism of reuse that is the problem How ever there is some work > planned to move function from helper to main odp api (pause (Petri), and I > think ring can go also.). Ok, I am happy if you send a patch to make it part of the public API and move it also. > So if you need inside implementation > calculate check sum, hash table, work with IP headers that helpers have to > be used. You can refer to our classification code for example. > And more likely we need to update TM code to use TCAM optimized hash > tables from helpers. > As a linked library sure, not as hidden shared code in this way: we committed code for that decision in July to break the dependency on helpers, then we decided that we would not compile in helper code. commit 38de6769b9b96b90b8ef6fbbf2049ce03aadd5c6 Author: Mike Holmes <[email protected]> Date: Mon Jul 6 15:13:52 2015 -0400 helper: convert to a library Remove the need to build helper source files into the linux-generic library by converting helpers to be their own library. This removes the need for all other platforms to also build in the helpers which are optional just to run the tests. Signed-off-by: Mike Holmes <[email protected]> Reviewed-by: Christophe Milard <[email protected]> Signed-off-by: Maxim Uvarov <[email protected]> > > There is only one case where I can agree with you, it's not use helper > prints inside odp lib. I see 2 cases to fix it: > 1. ) remove prints all all. There are only 2 prints. One references to > count providing and other to memory allocation. I can > just change it to __odp_errno = ENOMEM and __odp_errno = EINVAL; and it > should be enough to debug why ring_create() failed. > > 2) Add -DODP_HELPER and ifdefs in code to choose right print function > depending on how we print. > > I think first case is better. Will send updated patches. > The ring code needs to just be part of linux-generic, or linked and not smeared between both. > Best regards, > Maxim. > > -- >> Mike Holmes >> Technical Manager - Linaro Networking Group >> Linaro.org <http://www.linaro.org/>***│ *Open source software for ARM >> SoCs >> >> > -- Mike Holmes Technical Manager - Linaro Networking Group Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs
_______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
