From cff9149920249b7923dc51f03eb93c5b4bf13f47 Mon Sep 17 00:00:00 2001
From: Cedric Roux <cedric.roux@eurecom.fr>
Date: Thu, 23 Mar 2017 12:34:24 +0100
Subject: [PATCH] fix issue 227 - UE IP settings disrupts realtime
see https://gitlab.eurecom.fr/oai/openairinterface5g/issues/227
When the UE connects to the eNodeB and receives its IP address from the
network, it calls system() to set it in the linux kernel world. This call
is not done in a realtime thread, but in the NAS, which uses its own thread,
independent of the realtime processing.
In some situations this totally disrupts realtime processing.
It is difficult to know precisely why that happens, but it seems that calling
fork(), as system() does, in a multi-threaded program is not a good idea. (So
say several people on the internet.) It is not clear why the softmodem is
impacted, but it seems that fork() is really what triggers the disruption.
Several tests lead to that conclusion.
To fix the problem, we create a child background process very early in main()
(before anything else basically). Then instead of calling system(), the main
process sends the string to the background process. The background process
gets the string, passes it to system() and reports the success/failure back
to the main process.
This solution involves a lot of system calls, but calling system() in the
first place is not cheap either. As long as no realtime thread uses this
mechanism, things should be fine. Time will tell.
---
cmake_targets/CMakeLists.txt | 2 +
cmake_targets/at_commands/CMakeLists.txt | 1 +
common/utils/system.c | 186 +++++++++++++++++++++++
common/utils/system.h | 39 +++++
openair3/NAS/UE/ESM/esm_ebr_context.c | 14 +-
targets/RT/USER/lte-softmodem.c | 4 +
targets/SIMU/USER/oaisim.c | 3 +
7 files changed, 248 insertions(+), 1 deletion(-)
create mode 100644 common/utils/system.c
create mode 100644 common/utils/system.h
diff --git a/cmake_targets/CMakeLists.txt b/cmake_targets/CMakeLists.txt
index d7cd81cf588..dd2eec6d2f9 100644
--- a/cmake_targets/CMakeLists.txt
+++ b/cmake_targets/CMakeLists.txt
@@ -1769,6 +1769,7 @@ add_executable(lte-softmodem
${OPENAIR1_DIR}/SIMULATION/ETH_TRANSPORT/netlink_init.c
${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c
${OPENAIR_DIR}/common/utils/utils.c
+ ${OPENAIR_DIR}/common/utils/system.c
${GTPU_need_ITTI}
${HW_SOURCE}
${TRANSPORT_SOURCE}
@@ -1900,6 +1901,7 @@ add_executable(oaisim
${OPENAIR2_DIR}/RRC/NAS/rb_config.c
${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c
${OPENAIR_DIR}/common/utils/utils.c
+ ${OPENAIR_DIR}/common/utils/system.c
${GTPU_need_ITTI}
${OPENAIR_TARGETS}/COMMON/create_tasks.c
${HW_SOURCE}
diff --git a/cmake_targets/at_commands/CMakeLists.txt b/cmake_targets/at_commands/CMakeLists.txt
index 61d1565bac9..db49a9181d6 100755
--- a/cmake_targets/at_commands/CMakeLists.txt
+++ b/cmake_targets/at_commands/CMakeLists.txt
@@ -716,6 +716,7 @@ ADD_EXECUTABLE(at_nas_ue ${OPENAIR_NAS_DIR}/UE/UEprocess.c
${OPENAIR_NAS_DIR}/UE/nas_proc.c
${OPENAIR_NAS_DIR}/UE/nas_user.c
${OPENAIR_DIR}/common/utils/utils.c
+ ${OPENAIR_DIR}/common/utils/system.c
)
target_link_libraries (at_nas_ue
diff --git a/common/utils/system.c b/common/utils/system.c
new file mode 100644
index 00000000000..52fb950e37f
--- /dev/null
+++ b/common/utils/system.c
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the OpenAirInterface (OAI) Software Alliance under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The OpenAirInterface Software Alliance licenses this file to You under
+ * the OAI Public License, Version 1.0 (the "License"); you may not use this file
+ * except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.openairinterface.org/?page_id=698
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *-------------------------------------------------------------------------------
+ * For more information about the OpenAirInterface (OAI) Software Alliance:
+ * contact@openairinterface.org
+ */
+
+/* This module provides a separate process to run system().
+ * The communication between this process and the main processing
+ * is done through unix pipes.
+ *
+ * Motivation: the UE sets its IP address using system() and
+ * that disrupts realtime processing in some cases. Having a
+ * separate process solves this problem.
+ */
+
+#include "system.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <pthread.h>
+#include <string.h>
+
+#define MAX_COMMAND 4096
+
+static int command_pipe_read;
+static int command_pipe_write;
+static int result_pipe_read;
+static int result_pipe_write;
+
+static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+
+static int module_initialized = 0;
+
+/********************************************************************/
+/* util functions */
+/********************************************************************/
+
+static void lock_system(void)
+{
+ if (pthread_mutex_lock(&lock) != 0) {
+ printf("pthread_mutex_lock fails\n");
+ abort();
+ }
+}
+
+static void unlock_system(void)
+{
+ if (pthread_mutex_unlock(&lock) != 0) {
+ printf("pthread_mutex_unlock fails\n");
+ abort();
+ }
+}
+
+static void write_pipe(int p, char *b, int size)
+{
+ while (size) {
+ int ret = write(p, b, size);
+ if (ret <= 0) exit(0);
+ b += ret;
+ size -= ret;
+ }
+}
+
+static void read_pipe(int p, char *b, int size)
+{
+ while (size) {
+ int ret = read(p, b, size);
+ if (ret <= 0) exit(0);
+ b += ret;
+ size -= ret;
+ }
+}
+
+/********************************************************************/
+/* background process */
+/********************************************************************/
+
+/* This function is run by background process. It waits for a command,
+ * runs it, and reports status back. It exits (in normal situations)
+ * when the main process exits, because then a "read" on the pipe
+ * will return 0, in which case "read_pipe" exits.
+ */
+static void background_system_process(void)
+{
+ int len;
+ int ret;
+ char command[MAX_COMMAND+1];
+
+ while (1) {
+ read_pipe(command_pipe_read, (char*)&len, sizeof(int));
+ read_pipe(command_pipe_read, command, len);
+ ret = system(command);
+ write_pipe(result_pipe_write, (char *)&ret, sizeof(int));
+ }
+}
+
+/********************************************************************/
+/* background_system() */
+/* return -1 on error, 0 on success */
+/********************************************************************/
+
+int background_system(char *command)
+{
+ int res;
+ int len;
+
+ if (module_initialized == 0) {
+ printf("FATAL: calling 'background_system' but 'start_background_system' was not called\n");
+ abort();
+ }
+
+ len = strlen(command)+1;
+ if (len > MAX_COMMAND) {
+ printf("FATAL: command too long. Increase MAX_COMMAND (%d).\n", MAX_COMMAND);
+ printf("command was: '%s'\n", command);
+ abort();
+ }
+ /* only one command can run at a time, so let's lock/unlock */
+ lock_system();
+ write_pipe(command_pipe_write, (char*)&len, sizeof(int));
+ write_pipe(command_pipe_write, command, len);
+ read_pipe(result_pipe_read, (char*)&res, sizeof(int));
+ unlock_system();
+ if (res == -1 || !WIFEXITED(res) || WEXITSTATUS(res) != 0) return -1;
+ return 0;
+}
+
+/********************************************************************/
+/* start_background_system() */
+/* initializes the "background system" module */
+/* to be called very early by the main processing */
+/********************************************************************/
+
+void start_background_system(void)
+{
+ int p[2];
+ pid_t son;
+
+ module_initialized = 1;
+
+ if (pipe(p) == -1) {
+ perror("pipe");
+ exit(1);
+ }
+ command_pipe_read = p[0];
+ command_pipe_write = p[1];
+
+ if (pipe(p) == -1) {
+ perror("pipe");
+ exit(1);
+ }
+ result_pipe_read = p[0];
+ result_pipe_write = p[1];
+
+ son = fork();
+ if (son == -1) {
+ perror("fork");
+ exit(1);
+ }
+
+ if (son) {
+ close(result_pipe_write);
+ close(command_pipe_read);
+ return;
+ }
+
+ close(result_pipe_read);
+ close(command_pipe_write);
+
+ background_system_process();
+}
diff --git a/common/utils/system.h b/common/utils/system.h
new file mode 100644
index 00000000000..784c15fc9a0
--- /dev/null
+++ b/common/utils/system.h
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the OpenAirInterface (OAI) Software Alliance under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The OpenAirInterface Software Alliance licenses this file to You under
+ * the OAI Public License, Version 1.0 (the "License"); you may not use this file
+ * except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.openairinterface.org/?page_id=698
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *-------------------------------------------------------------------------------
+ * For more information about the OpenAirInterface (OAI) Software Alliance:
+ * contact@openairinterface.org
+ */
+
+#ifndef _SYSTEM_H_OAI_
+#define _SYSTEM_H_OAI_
+
+/****************************************************
+ * send a command to the background process
+ * return -1 on error, 0 on success
+ ****************************************************/
+
+int background_system(char *command);
+
+/****************************************************
+ * initialize the background process
+ * to be called very early
+ ****************************************************/
+
+void start_background_system(void);
+
+#endif /* _SYSTEM_H_OAI_ */
diff --git a/openair3/NAS/UE/ESM/esm_ebr_context.c b/openair3/NAS/UE/ESM/esm_ebr_context.c
index bfa4a04cc82..4700187da17 100644
--- a/openair3/NAS/UE/ESM/esm_ebr_context.c
+++ b/openair3/NAS/UE/ESM/esm_ebr_context.c
@@ -47,6 +47,7 @@ Description Defines functions used to handle EPS bearer contexts.
#include "esm_ebr_context.h"
#include "emm_sap.h"
+#include "system.h"
#if defined(ENABLE_ITTI)
# include "assertions.h"
@@ -286,7 +287,18 @@ int esm_ebr_context_create(
LOG_TRACE(INFO, "ESM-PROC - executing %s ",
command_line);
- if (system(command_line)) ; /* TODO: what to do? */
+ /* Calling system() here disrupts UE's realtime processing in some cases.
+ * This may be because of the call to fork(), which, for some
+ * unidentified reason, interacts badly with other (realtime) threads.
+ * background_system() is a replacement mechanism relying on a
+ * background process that does the system() and reports result to
+ * the parent process (lte-softmodem, oaisim, ...). The background
+ * process is created very early in the life of the parent process.
+ * The processes interact through standard pipes. See
+ * common/utils/system.c for details.
+ */
+ if (background_system(command_line) != 0)
+ LOG_TRACE(ERROR, "ESM-PROC - failed command '%s'", command_line);
break;
diff --git a/targets/RT/USER/lte-softmodem.c b/targets/RT/USER/lte-softmodem.c
index 03f6ba1a2ed..ee31831a6b3 100644
--- a/targets/RT/USER/lte-softmodem.c
+++ b/targets/RT/USER/lte-softmodem.c
@@ -74,6 +74,8 @@ unsigned short config_frames[4] = {2,9,11,13};
#include "create_tasks.h"
#endif
+#include "system.h"
+
#ifdef XFORMS
#include "PHY/TOOLS/lte_phy_scope.h"
#include "stats.h"
@@ -1371,6 +1373,8 @@ int main( int argc, char **argv ) {
int ret;
#endif
+ start_background_system();
+
#ifdef DEBUG_CONSOLE
setvbuf(stdout, NULL, _IONBF, 0);
setvbuf(stderr, NULL, _IONBF, 0);
diff --git a/targets/SIMU/USER/oaisim.c b/targets/SIMU/USER/oaisim.c
index f1b3944a164..ab801d685c6 100644
--- a/targets/SIMU/USER/oaisim.c
+++ b/targets/SIMU/USER/oaisim.c
@@ -59,6 +59,7 @@
#include "SCHED/defs.h"
#include "SCHED/vars.h"
+#include "system.h"
#include "PHY/TOOLS/lte_phy_scope.h"
@@ -1194,6 +1195,8 @@ main (int argc, char **argv)
clock_t t;
+ start_background_system();
+
#ifdef SMBV
// Rohde&Schwarz SMBV100A vector signal generator
strcpy(smbv_ip,DEFAULT_SMBV_IP);
--
GitLab