[dpdk-dev] Application framework vs. library
Hi, as a developer of a product that uses DPDK I inherited a custom changeset towards DPDK 2.0 to allow better integration into our application stack. I am currently underway of updating the used DPDK release and would like to get rid of these local changes in the future by influencing the upstream development to some degree. Concerning the integration we try to cope with the circumstance that DPDK is more comparable to an application framework and less to a library. The main part of the changes deal with EAL and specifically with rte_eal_init() and prune or replace functionality. The pruned features are in the responsibility of other proprietary libraries and modules within our application. For illustration purposes, I have included an old changeset towards v2.0.0. for reference, but due to the nature of the whole changeset it does not make sense to apply it directly, as there is a total lack of generality in the specific implementation (e.g. only the linuxapp part is customized, incomplete implemented concept in termination behavior ...). Nonetheless I am underway of researching concepts and approaches that are general enough for upstream inclusion. The main reason for attaching this patchset is to be able to discuss possible approaches of further development, as I would like to get feedback on possible development approaches to address the named problems. In detail the changes address the following areas: Logging behavior Opening a connection to the system logger for our program is within the responsibility of our specific logging module that is also used in the proprietary parts of the application. For this reason openlog() should not be called in our use case. In addition, using rte_openlog_stream() is not usable before rte_eal_init() as logging init will overwrite the used log stream. For this reason a large part of the init logs can not be handled by a custom log stream. Btw, after removal of the log caching, is there a fundamental difference between early log and normal logging? A possible approach for the future could be to initialize rte_logs.file to NULL (in fact it is, as it is static) and only set it during initialization if it is NULL with the goal to be able to use rte_openlog_stream() before rte_eal_init(). As the openlog() call is only relevant when the default log stream is used (and architecture dependent!) I introduced a specific entry point for calling openlog. The main complexity to this changeset is added due to two reasons. First reason is the circumstance that rte_eal_common_log_init() is used several times during early logging and real logging initialization. Second aspect is that a user might revert to the use of the default_log_stream after a custom log stream has be used straight from the beginning (even before rte_eal_init()). A dedicated changeset towards v16.07 for this improvement is attached for review, feedback and possible inclusion. Process termination behavior As the DPDK functionality is only a small part of the whole application ownership of the running process is not with DPDK and it is not allowed to cancel the running process as a result of an error. For this reason, the current changeset instead of calling rte_panic or rte_exit returns an error code and handles it in the calling function. Unfortunately this change was only implemented in rte_eal_init() and not in other places (drivers, libs ...), so there is currently no safety that an unknown code part terminates the whole application on the spot. Future changes and patches could change the error handling in a more thorough approach, but I would like to have some feedback and opinions before I start the heavy-lifting work in over 700 code locations. Even some interfaces might be affected, as lots of functions return void now, Thread creation In our application thread creation and setting the affinity is already handled in a proprietary module. Frames are polled and processed in non-EAL pthreads. For this reason the lcore thread creation in rte_eal_init() is completely removed as we do not want additional threads to be assigned to processing CPUs. Unfortunately setting the coremask to 0 is not feasible directly. For the non-EAL threads a custom function within the application sets the per_thread _lcore variable to the right value to try to enable mbuf cacheing. The changeset also disables the interrupt handling thread as in our application it currently seems not necessary (maybe make this optional?) In an experiment I removed the error check for the non-zero number of lcores and quick-fixed certain locations where rte_lcore_count() is used for memory size computations and at least got a running application. A possible approach could be to enhance enum rte_lcore_role_t with an auxiliary thread type and introduce additional eal arguments to assign lcores with these auxiliary role. I would like to receive feedback on possible development directions. Thank you, Steffen Bauch -- Steffen Bauch Sen
[dpdk-dev] Application framework vs. library
>From c3be5420d921325559de9b1079354e1c4314220a Mon Sep 17 00:00:00 2001
From: Steffen Bauch
Date: Mon, 25 Jul 2016 16:13:02 +0200
Subject: [PATCH] allow the call to rte_openlog_stream() before
rte_eal_init()
- only initialize the default_log_stream if it was not initialized
before main initialization of EAL
- save facility and logid in rte_default_log_args structure
- call openlog only on setup of the default_log_stream in
rte_openlog_stream
---
lib/librte_eal/bsdapp/eal/eal_log.c | 6 ++---
lib/librte_eal/common/eal_common_log.c | 47
++---
lib/librte_eal/common/eal_private.h | 40 +---
lib/librte_eal/common/include/rte_log.h | 7 +
lib/librte_eal/linuxapp/eal/eal_log.c | 24 +
5 files changed, 110 insertions(+), 14 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/eal_log.c
b/lib/librte_eal/bsdapp/eal/eal_log.c
index a425f7a..5abd906 100644
--- a/lib/librte_eal/bsdapp/eal/eal_log.c
+++ b/lib/librte_eal/bsdapp/eal/eal_log.c
@@ -42,9 +42,9 @@
* once memzones are available.
*/
int
-rte_eal_log_init(const char *id __rte_unused, int facility __rte_unused)
+rte_eal_log_init(const char *id, int facility)
{
- if (rte_eal_common_log_init(stderr) < 0)
+ if (rte_eal_common_log_init(stderr, id, facility) < 0)
return -1;
return 0;
}
@@ -52,6 +52,6 @@ rte_eal_log_init(const char *id __rte_unused, int
facility __rte_unused)
int
rte_eal_log_early_init(void)
{
- rte_openlog_stream(stderr);
+ rte_openlog_stream_initial(stderr);
return 0;
}
diff --git a/lib/librte_eal/common/eal_common_log.c
b/lib/librte_eal/common/eal_common_log.c
index 7916c78..197e6c9 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -48,6 +48,14 @@ struct rte_logs rte_logs = {
.file = NULL,
};
+struct rte_default_log_args rte_default_log_args =
+{
+.facility = 0,
+.log_id = NULL,
+.prepare_default_log = NULL,
+};
+
+
static FILE *default_log_stream;
/**
@@ -82,9 +90,30 @@ int
rte_openlog_stream(FILE *f)
{
if (f == NULL)
+ {
+ if (rte_default_log_args.prepare_default_log)
+ {
+ if
(rte_default_log_args.prepare_default_log(rte_default_log_args.log_id,
+ rte_default_log_args.facility) <
0)
+ return -1;
+ }
rte_logs.file = default_log_stream;
+ }
else
+ {
rte_logs.file = f;
+ }
+ return 0;
+}
+
+int
+rte_openlog_stream_initial(FILE *f)
+{
+ /* only initialize if not configured before rte_eal_init() */
+ if (NULL == rte_logs.file)
+ {
+ return rte_openlog_stream(f);
+ }
return 0;
}
@@ -176,14 +205,26 @@ rte_log(uint32_t level, uint32_t logtype, const char
*format, ...)
return ret;
}
+int rte_eal_set_default_log_stream(FILE *default_log, const char *id,
int facility,
+int (*prepare_log)(const char
*log_id, int facility))
+{
+ rte_default_log_args.facility = facility;
+ rte_default_log_args.log_id = id;
+ rte_default_log_args.prepare_default_log = prepare_log;
+ default_log_stream = default_log;
+
+ return 0;
+}
+
/*
* called by environment-specific log init function
*/
int
-rte_eal_common_log_init(FILE *default_log)
+rte_eal_common_log_init(FILE *default_log, const char *id, int facility,
+int (*prepare_log)(const char *log_id, int
facility))
{
- default_log_stream = default_log;
- rte_openlog_stream(default_log);
+ rte_eal_set_default_log_stream(default_log, id, facility,
prepare_log);
+ rte_openlog_stream_initial(NULL); /* enable default log stream */
#if RTE_LOG_LEVEL >= RTE_LOG_DEBUG
RTE_LOG(NOTICE, EAL, "Debug logs available - lower
performance\n");
diff --git a/lib/librte_eal/common/eal_private.h
b/lib/librte_eal/common/eal_private.h
index 857dc3e..f12a00d 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -49,13 +49,26 @@ int rte_eal_memzone_init(void);
/**
* Common log initialization function (private to eal).
*
- * @param default_log
- * The default log stream to be used.
+ * @param default_log The default log stream to be used.
+ * @param id the id used for openlog call
+ * @param facility the facility used for openlog call
+ * @param prepare_log a platform specific log prepare function
* @return
* - 0 on success
* - Negative on error
*/
-int rte_eal_common_log_init(FILE *default_log);
+int rte_eal_common_log_init(FILE *default_log, const char *id, int
facility,
+ int (*prepare_log)(const char *log_id, int
facility));
+
+/**
+ * A function that only sets the log stream if it has not previously been
set.
+ *
+ * T
[dpdk-dev] Application framework vs. library
diff --git a/lib/librte_eal/linuxapp/eal/eal.c
b/lib/librte_eal/linuxapp/eal/eal.c
index bd770cf..f63f2f8 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -664,12 +664,6 @@ eal_check_mem_on_local_socket(void)
"memory on local socket!\n");
}
-static int
-sync_func(__attribute__((unused)) void *arg)
-{
- return 0;
-}
-
inline static void
rte_eal_mcfg_complete(void)
{
@@ -699,26 +693,17 @@ rte_eal_iopl_init(void)
int
rte_eal_init(int argc, char **argv)
{
- int i, fctret, ret;
- pthread_t thread_id;
- static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
+ int fctret;
struct shared_driver *solib = NULL;
const char *logid;
- char cpuset[RTE_CPU_AFFINITY_STR_LEN];
-
- if (!rte_atomic32_test_and_set(&run_once))
- return -1;
- logid = strrchr(argv[0], '/');
- logid = strdup(logid ? logid + 1: argv[0]);
-
- thread_id = pthread_self();
+ logid = NULL;
if (rte_eal_log_early_init() < 0)
- rte_panic("Cannot init early logs\n");
+ return -1;
if (rte_eal_cpu_init() < 0)
- rte_panic("Cannot detect lcores\n");
+ return -1;
fctret = eal_parse_args(argc, argv);
if (fctret < 0)
@@ -731,7 +716,7 @@ rte_eal_init(int argc, char **argv)
internal_config.process_type != RTE_PROC_SECONDARY
&&
internal_config.xen_dom0_support == 0 &&
eal_hugepage_info_init() < 0)
- rte_panic("Cannot get hugepage information\n");
+ return -1;
if (internal_config.memory == 0 && internal_config.force_sockets
== 0) {
if (internal_config.no_hugetlbfs)
@@ -756,41 +741,43 @@ rte_eal_init(int argc, char **argv)
rte_config_init();
if (rte_eal_pci_init() < 0)
- rte_panic("Cannot init PCI\n");
+ return -1;
#ifdef RTE_LIBRTE_IVSHMEM
if (rte_eal_ivshmem_init() < 0)
- rte_panic("Cannot init IVSHMEM\n");
+ return -1;
#endif
if (rte_eal_memory_init() < 0)
- rte_panic("Cannot init memory\n");
+ return -1;
/* the directories are locked during eal_hugepage_info_init */
eal_hugedirs_unlock();
if (rte_eal_memzone_init() < 0)
- rte_panic("Cannot init memzone\n");
+ return -1;
if (rte_eal_tailqs_init() < 0)
- rte_panic("Cannot init tail queues for objects\n");
+ return -1;
#ifdef RTE_LIBRTE_IVSHMEM
if (rte_eal_ivshmem_obj_init() < 0)
- rte_panic("Cannot init IVSHMEM objects\n");
+ return -1;
#endif
if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
- rte_panic("Cannot init logs\n");
+ return -1;
if (rte_eal_alarm_init() < 0)
- rte_panic("Cannot init interrupt-handling thread\n");
+ return -1;
+ /* interrupt handling will be initialized but the thread is
patched to immediately exit */
if (rte_eal_intr_init() < 0)
- rte_panic("Cannot init interrupt-handling thread\n");
+ return -1;
+ /* timer stuff is initialized but hpet should be disabled in
configuration */
if (rte_eal_timer_init() < 0)
- rte_panic("Cannot init HPET or TSC timers\n");
+ return -1;
eal_check_mem_on_local_socket();
@@ -803,47 +790,12 @@ rte_eal_init(int argc, char **argv)
RTE_LOG(WARNING, EAL, "%s\n", dlerror());
}
- eal_thread_init_master(rte_config.master_lcore);
-
- ret = eal_thread_dump_affinity(cpuset, RTE_CPU_AFFINITY_STR_LEN);
-
- RTE_LOG(DEBUG, EAL, "Master lcore %u is ready
(tid=%x;cpuset=[%s%s])\n",
- rte_config.master_lcore, (int)thread_id, cpuset,
- ret == 0 ? "" : "...");
-
if (rte_eal_dev_init() < 0)
- rte_panic("Cannot init pmd devices\n");
-
- RTE_LCORE_FOREACH_SLAVE(i) {
-
- /*
-* create communication pipes between master thread
-* and children
-*/
- if (pipe(lcore_config[i].pipe_master2slave) < 0)
- rte_panic("Cannot create pipe\n");
- if (pipe(lcore_config[i].pipe_slave2master) < 0)
- rte_panic("Cannot create pipe\n");
-
- lcore_config[i].state = WAIT;
-
- /* create a thread for each lcore */
- ret = pthread_create(&lcore_config[i].thread_id, NULL,
-eal_thread_loop, NULL);
- if (ret != 0)
- rte_panic("Cannot create thread\n");
- }
-
- /*
-* Launch a dummy function on all slave lcores, so that master
lcore
-
