Holger Hans Peter Freyther | 2 May 13:52 2011
Picon

Re: TODO: openbsc.cfg console logging options

On 04/30/2011 01:49 AM, Pablo Neira Ayuso wrote:

> Could you develop the scenario a bit? The problem that you refer would
> occur if `-d' and `-e' are used?
> 

Hi,

the attached config file is the result of 'write' in the VTY code, e.g. it
contains the current stderr config.

$ ./src/osmo-bsc/osmo-bsc -c openbsc.bsc -e 1

now the -e 1 is applied (log_set_log_level(stderr_target, atoi(optarg));)
before the call to bsc_bootstrap_network. So somehow we should define what
takes precedence and make this work across our utilities. E.g. osmo-bsc and
osmi-nitb behave the same right now (-e 1 being ignored if stderr is
configured in the config file).

With osmo-bsc one sees a lot more messages on startup if -e 1 is applied.

holger

!
! OsmoBSC (0.9.13.88-0b03) configuration saved from vty
!!
password foo
!
(Continue reading)

pablo | 3 May 22:40 2011

[PATCH 0/5] libosmocore: logging framework updates

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

Hi!

This patchset updates the logging framework, it contains one fix and
several updates/improvements.

You can find it in pablo/logging branch.

Please, merge it!

Pablo Neira Ayuso (5):
  logging: fix missing description of global loglevel
  logging: several memory allocation belong to tall_log_ctx context
  logging: rework _output() function
  logging: remove workaround now that _output() has been reworked
  logging: make sure the output is null-terminated

 src/logging.c |   77 +++++++++++++++++++++++++++++---------------------------
 1 files changed, 40 insertions(+), 37 deletions(-)

--

-- 
1.2.3.4

pablo | 3 May 22:40 2011

[PATCH 1/5] logging: fix missing description of global loglevel

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

OpenBSC> logging level
  all    Global setting for all subsystems <----- this description was missing
  rll    A-bis Radio Link Layer (RLL)
[...]

This problem was introduced by myself in:
"vty: integration with logging framework"
04139f14b6197e3ec996133a945af3fa8a68fb7a
---
 src/logging.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/src/logging.c b/src/logging.c
index 77c8a50..7b1ab13 100644
--- a/src/logging.c
+++ b/src/logging.c
 <at>  <at>  -517,6 +517,7  <at>  <at>  const char *log_vty_command_description(const struct log_info *info)
 	for (i = 0; i < LOGLEVEL_DEFS; i++)
 		size += strlen(loglevel_descriptions[i]) + 1;

+	size += strlen("Global setting for all subsystems") + 1;
 	rem = size;
 	str = talloc_zero_size(NULL, size);
 	if (!str)
 <at>  <at>  -528,6 +529,12  <at>  <at>  const char *log_vty_command_description(const struct log_info *info)
 		goto err;
 	OSMO_SNPRINTF_RET(ret, rem, offset, len);

(Continue reading)

pablo | 3 May 22:40 2011

[PATCH 2/5] logging: several memory allocation belong to tall_log_ctx context

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

Several talloc_zero in logging use NULL context, use tall_log_ctx
instead.
---
 src/logging.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/logging.c b/src/logging.c
index 7b1ab13..7fabfcb 100644
--- a/src/logging.c
+++ b/src/logging.c
 <at>  <at>  -448,7 +448,7  <at>  <at>  const char *log_vty_command_string(const struct log_info *info)
 		size += strlen(loglevel_strs[i].str) + 1;

 	rem = size;
-	str = talloc_zero_size(NULL, size);
+	str = talloc_zero_size(tall_log_ctx, size);
 	if (!str)
 		return NULL;

 <at>  <at>  -519,7 +519,7  <at>  <at>  const char *log_vty_command_description(const struct log_info *info)

 	size += strlen("Global setting for all subsystems") + 1;
 	rem = size;
-	str = talloc_zero_size(NULL, size);
+	str = talloc_zero_size(tall_log_ctx, size);
 	if (!str)
 		return NULL;

(Continue reading)

pablo | 3 May 22:40 2011

[PATCH 3/5] logging: rework _output() function

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

This patch reworks _output() to handle snprintf() return value
appropriately and to use one single buffer to build the logging
string, instead of four.
---
 src/logging.c |   51 +++++++++++++++++++++++++++------------------------
 1 files changed, 27 insertions(+), 24 deletions(-)

diff --git a/src/logging.c b/src/logging.c
index 7fabfcb..526b39e 100644
--- a/src/logging.c
+++ b/src/logging.c
 <at>  <at>  -143,29 +143,19  <at>  <at>  static void _output(struct log_target *target, unsigned int subsys,
 		    unsigned int level, char *file, int line, int cont,
 		    const char *format, va_list ap)
 {
-	char col[30];
-	char sub[30];
-	char tim[30];
 	char buf[4096];
-	char final[4096];
-
-	/* prepare the data */
-	col[0] = '\0';
-	sub[0] = '\0';
-	tim[0] = '\0';
-	buf[0] = '\0';
+	int ret, len = 0, offset = 0, rem = sizeof(buf);

(Continue reading)

pablo | 3 May 22:40 2011

[PATCH 4/5] logging: remove workaround now that _output() has been reworked

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

This patch removes a workaround to fix some strange memory corruption
now that _output() has been completely reworked and we make use of
snprintf appropriately.
---
 src/logging.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/logging.c b/src/logging.c
index 526b39e..6797afc 100644
--- a/src/logging.c
+++ b/src/logging.c
 <at>  <at>  -221,19 +221,10  <at>  <at>  static void _logp(unsigned int subsys, int level, char *file, int line,
 		else if (osmo_log_info->filter_fn)
 			output = osmo_log_info->filter_fn(&log_context,
 						       tar);
+		if (!output)
+			continue;

-		if (output) {
-			/* FIXME: copying the va_list is an ugly
-			 * workaround against a bug hidden somewhere in
-			 * _output.  If we do not copy here, the first
-			 * call to _output() will corrupt the va_list
-			 * contents, and any further _output() calls
-			 * with the same va_list will segfault */
-			va_list bp;
-			va_copy(bp, ap);
-			_output(tar, subsys, level, file, line, cont, format, bp);
(Continue reading)

pablo | 3 May 22:40 2011

[PATCH 5/5] logging: make sure the output is null-terminated

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

If we reach the buffer size or snprintf fails, we want to make sure
that the output is null-terminated.
---
 src/logging.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/logging.c b/src/logging.c
index 6797afc..0911010 100644
--- a/src/logging.c
+++ b/src/logging.c
 <at>  <at>  -493,6 +493,7  <at>  <at>  const char *log_vty_command_string(const struct log_info *info)
 		goto err;
 	OSMO_SNPRINTF_RET(ret, rem, offset, len);
 err:
+	str[size-1] = '\0';
 	return str;
 }

 <at>  <at>  -544,6 +545,7  <at>  <at>  const char *log_vty_command_description(const struct log_info *info)
 		OSMO_SNPRINTF_RET(ret, rem, offset, len);
 	}
 err:
+	str[size-1] = '\0';
 	return str;
 }

--

-- 
1.7.2.3
(Continue reading)

pablo | 3 May 22:44 2011

[PATCH 0/3] openBSC: several misc updates

From: Pablo Neira Ayuso <pablo <at> netfilter.org>

Hi!

The following patchset contains one fix and two improvements.
You can find them in the pablo/updates branch.

Please, merge it!

Pablo Neira Ayuso (3):
  msc: bail out if subscriber VTY command fails
  abis: skip e1_input nesting if empty
  bsc: on-demand setup of nanoBTS and HSL femto sockets

 openbsc/src/libabis/e1_input_vty.c        |    8 ++++++--
 openbsc/src/libbsc/bsc_init.c             |   20 +++++++++++++++-----
 openbsc/src/libmsc/vty_interface_layer3.c |   11 ++++++++++-
 3 files changed, 31 insertions(+), 8 deletions(-)

--

-- 
1.7.2.3

pablo | 3 May 22:44 2011

[PATCH 1/3] msc: bail out if subscriber VTY command fails

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

This patch adds several messages that would be displayed if:

* the Ki argument is missing.
* you pass an invalid Ki.
* the database fails to perform the operation (add/delete/update).

Before this patch, no messages were spotted on this errors.

I noticed this while adding Ki to the existing subscribers in the
nanoBTS setup: I introduced a wrong Ki but the VTY command line did
not report any error. A quick look at the database via sqlite
command confirmed that the new authkey information was not added.
---
 openbsc/src/libmsc/vty_interface_layer3.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/openbsc/src/libmsc/vty_interface_layer3.c b/openbsc/src/libmsc/vty_interface_layer3.c
index 2d3dd14..6ac2c65 100644
--- a/openbsc/src/libmsc/vty_interface_layer3.c
+++ b/openbsc/src/libmsc/vty_interface_layer3.c
 <at>  <at>  -575,6 +575,8  <at>  <at>  DEFUN(ena_subscr_a3a8,
 	} else {
 		/* Unknown method */
 		subscr_put(subscr);
+		vty_out(vty, "%% Unknown auth method %s%s",
+				alg_str, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
(Continue reading)

pablo | 3 May 22:44 2011

[PATCH 2/3] abis: skip e1_input nesting if empty

From: Pablo Neira Ayuso <pablo <at> gnumonks.org>

With this patch, we don't including e1_input if it's empty

[...]
    timeslot 7
     phys_chan_config TCH/F
     hopping enabled 0
e1_input <----------------- empty, it should not show up.
msc
[...]
---
 openbsc/src/libabis/e1_input_vty.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/openbsc/src/libabis/e1_input_vty.c b/openbsc/src/libabis/e1_input_vty.c
index 7dbf17a..af984bc 100644
--- a/openbsc/src/libabis/e1_input_vty.c
+++ b/openbsc/src/libabis/e1_input_vty.c
 <at>  <at>  -19,6 +19,7  <at>  <at> 

 #include <stdlib.h>
 #include <unistd.h>
+#include <stdbool.h>

 #include <osmocom/vty/command.h>
 #include <osmocom/vty/buffer.h>
 <at>  <at>  -75,10 +76,13  <at>  <at>  DEFUN(cfg_e1inp, cfg_e1inp_cmd,
 static int e1inp_config_write(struct vty *vty)
 {
(Continue reading)


Gmane