Egress also uses odp_ring.
For odp_ring I do not see anything bad to have it in helper. So
applications can reuse that software ring if needed. Which might be very
useful in number of cases. If we put in inside implementation then
obviously it will be hard to reuse it.
Maxim.
On 06/12/15 11:26, Savolainen, Petri (Nokia - FI/Espoo) wrote:
Yes, helper API and implementation should be cleaned from all internals.
E.g. these must be removed from helper/linux.c:
#include <odp_internal.h>
#include <odp/thread.h>
#include <odp/init.h>
#include <odp/system_info.h>
#include <odp_debug_internal.h>
“This is compounded by the notion that helpers should generally be
callable before an odp_init call, …”
It’s actually the opposite, helpers can be called _/only after/_
odp_init_global() + odp_init_local(). The idea of a helper is that it
does (or may do) a series of ODP API calls exactly the same way the
application would do those. For example, odph_linux_pthread_create
helps the user to create and pin a number of threads to cpus, while
doing that it does couple of ODP API calls, which require that ODP has
been successfully initialized.
-Petri
int odph_linux_pthread_create(odph_linux_pthread_t *thread_tbl,
const odp_cpumask_t *mask_in,
void *(*start_routine) (void *), void *arg)
{
int i;
int num;
odp_cpumask_t mask;
int cpu_count;
int cpu;
int ret;
*odp_cpumask_copy*(&mask, mask_in);
num = odp_cpumask_count(&mask);
memset(thread_tbl, 0, num * sizeof(odph_linux_pthread_t));
cpu_count = *odp_cpu_count*();
if (num < 1 || num > cpu_count) {
ODP_ERR("Invalid number of threads: %d (%d cores available)\n",
num, cpu_count);
return 0;
}
cpu = *odp_cpumask_first*(&mask);
for (i = 0; i < num; i++) {
odp_cpumask_t thd_mask;
*odp_cpumask_zero*(&thd_mask);
*odp_cpumask_set*(&thd_mask, cpu);
pthread_attr_init(&thread_tbl[i].attr);
thread_tbl[i].cpu = cpu;
pthread_attr_setaffinity_np(&thread_tbl[i].attr,
sizeof(cpu_set_t), &thd_mask.set);
…
*From:*ext Mike Holmes [mailto:[email protected]]
*Sent:* Thursday, June 11, 2015 11:31 PM
*To:* Maxim Uvarov; Petri Savolainen
*Cc:* lng-odp
*Subject:* RFC move the ring implementation into linux-generic
ODP helper should be cleaned up so that it only depends on the ODP
public API and does not require implementations to provide internal
platform support for the helpers. This is compounded by the notion
that helpers should generally be callable before an odp_init call,
indeed you might expect to be able to use odph_rings without calling
any ODP_API. Neither of these goals if met currently.
Assuming that helpers should not use implementation internals we need
to remove the following internal includes from the helpers :-
odp_spin_internal.h
odp_internal.h
odp_debug_internal.h
odp_align_internal.h
I tried to clean this up and was successful with minimal change except
that the ring helper implementation calls odp_spin() which is a more
complex internal function of linux-generic.
Given that Maxim wants to implement linux-generic PKTIO- IPC on the
ring implementation that makes a mess:-
Linux-generic will now depend on an odp helper that depends on the
linux-generic internal implementation which is a bit of a loop.
In the case of linux-generic it would make sense to make the ring
functionality part of the internal implementation and drop it as a
helper. Currently no test or application uses the ring helper so there
would be no impact at all to this move.
Since the ring is not a HW abstraction its API does not belong in the
ODP API in my opinion, so I would keep the ring as an internal API of
the linux-generic implementation. Derived works such as ODP-DPDK,
ODP-NETMAP and ODP-KS2 can of course use the existing mechanisms to
reuse the internal ring code for their implementations.
Summary:
As part of the decoupling I propose moving helper/ring.c ->
platform/linux-generic/ring.c
--
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
