Commit cff91499 authored by Cedric Roux's avatar Cedric Roux

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.
parent bd173433
...@@ -1769,6 +1769,7 @@ add_executable(lte-softmodem ...@@ -1769,6 +1769,7 @@ add_executable(lte-softmodem
${OPENAIR1_DIR}/SIMULATION/ETH_TRANSPORT/netlink_init.c ${OPENAIR1_DIR}/SIMULATION/ETH_TRANSPORT/netlink_init.c
${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c ${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c
${OPENAIR_DIR}/common/utils/utils.c ${OPENAIR_DIR}/common/utils/utils.c
${OPENAIR_DIR}/common/utils/system.c
${GTPU_need_ITTI} ${GTPU_need_ITTI}
${HW_SOURCE} ${HW_SOURCE}
${TRANSPORT_SOURCE} ${TRANSPORT_SOURCE}
...@@ -1900,6 +1901,7 @@ add_executable(oaisim ...@@ -1900,6 +1901,7 @@ add_executable(oaisim
${OPENAIR2_DIR}/RRC/NAS/rb_config.c ${OPENAIR2_DIR}/RRC/NAS/rb_config.c
${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c ${OPENAIR3_DIR}/NAS/UE/nas_ue_task.c
${OPENAIR_DIR}/common/utils/utils.c ${OPENAIR_DIR}/common/utils/utils.c
${OPENAIR_DIR}/common/utils/system.c
${GTPU_need_ITTI} ${GTPU_need_ITTI}
${OPENAIR_TARGETS}/COMMON/create_tasks.c ${OPENAIR_TARGETS}/COMMON/create_tasks.c
${HW_SOURCE} ${HW_SOURCE}
......
...@@ -716,6 +716,7 @@ ADD_EXECUTABLE(at_nas_ue ${OPENAIR_NAS_DIR}/UE/UEprocess.c ...@@ -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_proc.c
${OPENAIR_NAS_DIR}/UE/nas_user.c ${OPENAIR_NAS_DIR}/UE/nas_user.c
${OPENAIR_DIR}/common/utils/utils.c ${OPENAIR_DIR}/common/utils/utils.c
${OPENAIR_DIR}/common/utils/system.c
) )
target_link_libraries (at_nas_ue target_link_libraries (at_nas_ue
......
/*
* 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();
}
/*
* 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_ */
...@@ -47,6 +47,7 @@ Description Defines functions used to handle EPS bearer contexts. ...@@ -47,6 +47,7 @@ Description Defines functions used to handle EPS bearer contexts.
#include "esm_ebr_context.h" #include "esm_ebr_context.h"
#include "emm_sap.h" #include "emm_sap.h"
#include "system.h"
#if defined(ENABLE_ITTI) #if defined(ENABLE_ITTI)
# include "assertions.h" # include "assertions.h"
...@@ -286,7 +287,18 @@ int esm_ebr_context_create( ...@@ -286,7 +287,18 @@ int esm_ebr_context_create(
LOG_TRACE(INFO, "ESM-PROC - executing %s ", LOG_TRACE(INFO, "ESM-PROC - executing %s ",
command_line); 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; break;
......
...@@ -74,6 +74,8 @@ unsigned short config_frames[4] = {2,9,11,13}; ...@@ -74,6 +74,8 @@ unsigned short config_frames[4] = {2,9,11,13};
#include "create_tasks.h" #include "create_tasks.h"
#endif #endif
#include "system.h"
#ifdef XFORMS #ifdef XFORMS
#include "PHY/TOOLS/lte_phy_scope.h" #include "PHY/TOOLS/lte_phy_scope.h"
#include "stats.h" #include "stats.h"
...@@ -1371,6 +1373,8 @@ int main( int argc, char **argv ) { ...@@ -1371,6 +1373,8 @@ int main( int argc, char **argv ) {
int ret; int ret;
#endif #endif
start_background_system();
#ifdef DEBUG_CONSOLE #ifdef DEBUG_CONSOLE
setvbuf(stdout, NULL, _IONBF, 0); setvbuf(stdout, NULL, _IONBF, 0);
setvbuf(stderr, NULL, _IONBF, 0); setvbuf(stderr, NULL, _IONBF, 0);
......
...@@ -59,6 +59,7 @@ ...@@ -59,6 +59,7 @@
#include "SCHED/defs.h" #include "SCHED/defs.h"
#include "SCHED/vars.h" #include "SCHED/vars.h"
#include "system.h"
#include "PHY/TOOLS/lte_phy_scope.h" #include "PHY/TOOLS/lte_phy_scope.h"
...@@ -1194,6 +1195,8 @@ main (int argc, char **argv) ...@@ -1194,6 +1195,8 @@ main (int argc, char **argv)
clock_t t; clock_t t;
start_background_system();
#ifdef SMBV #ifdef SMBV
// Rohde&Schwarz SMBV100A vector signal generator // Rohde&Schwarz SMBV100A vector signal generator
strcpy(smbv_ip,DEFAULT_SMBV_IP); strcpy(smbv_ip,DEFAULT_SMBV_IP);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment