All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] log: Fix the syslog spam when running tests
@ 2020-09-12 18:28 Simon Glass
  2020-09-12 18:28 ` [PATCH 1/4] log: Add a flag to enable log drivers Simon Glass
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Simon Glass @ 2020-09-12 18:28 UTC (permalink / raw)
  To: u-boot

At present running some sandbox tests produces unwanted ethernet errors
from the syslog driver, e.g.:

$ ./u-boot -c "ut bloblist"

Running 5 bloblist tests
Test: bloblist_test_bad_blob
Test: bloblist_test_blob
Test: bloblist_test_blob_ensure
Failed to allocate e0 bytes size=100, need size=130
No ethernet found.
bloblist add: returning err=-28
No ethernet found.
Test: bloblist_test_checksum
Checksum f3129fae != 338fbdc
No ethernet found.
Bad checksum: returning err=-5
No ethernet found.
...

This series fixes this by making the syslog driver disabled by default. It
can be enabled with a function call.


Simon Glass (4):
  log: Add a flag to enable log drivers
  log: Drop #ifdef in log_test
  log: Add a way to enable/disable a log device
  log: Disable the syslog driver by default

 common/log.c                  | 42 ++++++++++++++++++++++++++++++++++-
 common/log_console.c          |  1 +
 include/log.h                 | 28 ++++++++++++++++++++++-
 test/log/log_test.c           |  9 ++++++--
 test/log/syslog_test.c        | 24 ++++++++++++++++++++
 test/log/syslog_test.h        | 16 +++++++++++++
 test/log/syslog_test_ndebug.c |  2 ++
 test/py/tests/test_log.py     |  8 +++++++
 8 files changed, 126 insertions(+), 4 deletions(-)

-- 
2.28.0.618.gf4bc123cb7-goog

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] log: Add a flag to enable log drivers
  2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
@ 2020-09-12 18:28 ` Simon Glass
  2020-10-12  1:14   ` Tom Rini
  2020-09-12 18:28 ` [PATCH 2/4] log: Drop #ifdef in log_test Simon Glass
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2020-09-12 18:28 UTC (permalink / raw)
  To: u-boot

At present there is no way to disable a log driver. But the syslog driver
causes (attempted) network traffic in sandbox every time a log message
is printed, which is often.

Add a flag to enable a log driver. Adjust struct log_device to use a short
for next_filter_num so that no more memory is used for devices. Also fix
a missing line in the struct log_driver comment while here.

To maintain compatibility, enable it for all drivers for now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/log.c         |  4 +++-
 common/log_console.c |  1 +
 common/log_syslog.c  |  1 +
 include/log.h        | 11 ++++++++++-
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/common/log.c b/common/log.c
index f44f15743ff..ce528fe4535 100644
--- a/common/log.c
+++ b/common/log.c
@@ -196,7 +196,8 @@ static int log_dispatch(struct log_rec *rec)
 	struct log_device *ldev;
 
 	list_for_each_entry(ldev, &gd->log_head, sibling_node) {
-		if (log_passes_filters(ldev, rec))
+		if ((ldev->flags & LOGDF_ENABLE) &&
+		    log_passes_filters(ldev, rec))
 			ldev->drv->emit(ldev, rec);
 	}
 
@@ -318,6 +319,7 @@ int log_init(void)
 		}
 		INIT_LIST_HEAD(&ldev->filter_head);
 		ldev->drv = drv;
+		ldev->flags = drv->flags;
 		list_add_tail(&ldev->sibling_node,
 			      (struct list_head *)&gd->log_head);
 		drv++;
diff --git a/common/log_console.c b/common/log_console.c
index bb3f8464b98..8776fd47039 100644
--- a/common/log_console.c
+++ b/common/log_console.c
@@ -44,4 +44,5 @@ static int log_console_emit(struct log_device *ldev, struct log_rec *rec)
 LOG_DRIVER(console) = {
 	.name	= "console",
 	.emit	= log_console_emit,
+	.flags	= LOGDF_ENABLE,
 };
diff --git a/common/log_syslog.c b/common/log_syslog.c
index 149ff5af310..cf0dbba9bf5 100644
--- a/common/log_syslog.c
+++ b/common/log_syslog.c
@@ -115,4 +115,5 @@ out:
 LOG_DRIVER(syslog) = {
 	.name	= "syslog",
 	.emit	= log_syslog_emit,
+	.flags	= LOGDF_ENABLE,
 };
diff --git a/include/log.h b/include/log.h
index 86c8d7be09d..d28bc1ef0e4 100644
--- a/include/log.h
+++ b/include/log.h
@@ -307,10 +307,16 @@ struct log_rec {
 
 struct log_device;
 
+enum log_device_flags {
+	LOGDF_ENABLE		= BIT(0),	/* Device is enabled */
+};
+
 /**
  * struct log_driver - a driver which accepts and processes log records
  *
  * @name: Name of driver
+ * @emit: Method to call to emit a log record via this device
+ * @flags: Initial value for flags (use LOGDF_ENABLE to enable on start-up)
  */
 struct log_driver {
 	const char *name;
@@ -321,6 +327,7 @@ struct log_driver {
 	 * for processing. The filter is checked before calling this function.
 	 */
 	int (*emit)(struct log_device *ldev, struct log_rec *rec);
+	unsigned short flags;
 };
 
 /**
@@ -333,12 +340,14 @@ struct log_driver {
  * @next_filter_num: Seqence number of next filter filter added (0=no filters
  *	yet). This increments with each new filter on the device, but never
  *	decrements
+ * @flags: Flags for this filter (enum log_device_flags)
  * @drv: Pointer to driver for this device
  * @filter_head: List of filters for this device
  * @sibling_node: Next device in the list of all devices
  */
 struct log_device {
-	int next_filter_num;
+	unsigned short next_filter_num;
+	unsigned short flags;
 	struct log_driver *drv;
 	struct list_head filter_head;
 	struct list_head sibling_node;
-- 
2.28.0.618.gf4bc123cb7-goog

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/4] log: Drop #ifdef in log_test
  2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
  2020-09-12 18:28 ` [PATCH 1/4] log: Add a flag to enable log drivers Simon Glass
@ 2020-09-12 18:28 ` Simon Glass
  2020-10-12  1:14   ` Tom Rini
  2020-09-12 18:28 ` [PATCH 3/4] log: Add a way to enable/disable a log device Simon Glass
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2020-09-12 18:28 UTC (permalink / raw)
  To: u-boot

This is not needed as the Makefile only builds the file if CONFIG_LOG_TEST
is enabled. Drop it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 test/log/log_test.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/log/log_test.c b/test/log/log_test.c
index 4245372d65f..fdee5e6757f 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -201,7 +201,6 @@ static int log_test(int testnum)
 	return 0;
 }
 
-#ifdef CONFIG_LOG_TEST
 int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	int testnum = 0;
@@ -216,4 +215,3 @@ int do_log_test(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 
 	return ret ? CMD_RET_FAILURE : 0;
 }
-#endif
-- 
2.28.0.618.gf4bc123cb7-goog

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/4] log: Add a way to enable/disable a log device
  2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
  2020-09-12 18:28 ` [PATCH 1/4] log: Add a flag to enable log drivers Simon Glass
  2020-09-12 18:28 ` [PATCH 2/4] log: Drop #ifdef in log_test Simon Glass
@ 2020-09-12 18:28 ` Simon Glass
  2020-09-12 20:24   ` Heinrich Schuchardt
  2020-10-12  1:15   ` Tom Rini
  2020-09-12 18:28 ` [PATCH 4/4] log: Disable the syslog driver by default Simon Glass
  2020-09-14  8:24 ` [PATCH 0/4] log: Fix the syslog spam when running tests Heinrich Schuchardt
  4 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2020-09-12 18:28 UTC (permalink / raw)
  To: u-boot

At present all log devices are enabled by default. Add a function to allow
devices to be disabled or enabled at runtime.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/log.c              | 38 ++++++++++++++++++++++++++++++++++++++
 include/log.h             | 17 +++++++++++++++++
 test/log/log_test.c       |  7 +++++++
 test/py/tests/test_log.py |  8 ++++++++
 4 files changed, 70 insertions(+)

diff --git a/common/log.c b/common/log.c
index ce528fe4535..a9aacfaf354 100644
--- a/common/log.c
+++ b/common/log.c
@@ -297,6 +297,44 @@ int log_remove_filter(const char *drv_name, int filter_num)
 	return -ENOENT;
 }
 
+/**
+ * log_find_device_by_drv() - Find a device by its driver
+ *
+ * @drv: Log driver
+ * @return Device associated with that driver, or NULL if not found
+ */
+static struct log_device *log_find_device_by_drv(struct log_driver *drv)
+{
+	struct log_device *ldev;
+
+	list_for_each_entry(ldev, &gd->log_head, sibling_node) {
+		if (ldev->drv == drv)
+			return ldev;
+	}
+	/*
+	 * It is quite hard to pass an invalid driver since passing an unknown
+	 * LOG_GET_DRIVER(xxx) would normally produce a compilation error. But
+	 * it is possible to pass NULL, for example, so this
+	 */
+
+	return NULL;
+}
+
+int log_device_set_enable(struct log_driver *drv, bool enable)
+{
+	struct log_device *ldev;
+
+	ldev = log_find_device_by_drv(drv);
+	if (!ldev)
+		return -ENOENT;
+	if (enable)
+		ldev->flags |= LOGDF_ENABLE;
+	else
+		ldev->flags &= ~LOGDF_ENABLE;
+
+	return 0;
+}
+
 int log_init(void)
 {
 	struct log_driver *drv = ll_entry_start(struct log_driver, log_driver);
diff --git a/include/log.h b/include/log.h
index d28bc1ef0e4..4acc087b2e9 100644
--- a/include/log.h
+++ b/include/log.h
@@ -388,6 +388,10 @@ struct log_filter {
 #define LOG_DRIVER(_name) \
 	ll_entry_declare(struct log_driver, _name, log_driver)
 
+/* Get a pointer to a given driver */
+#define LOG_GET_DRIVER(__name)						\
+	ll_entry_get(struct log_driver, __name, log_driver)
+
 /**
  * log_get_cat_name() - Get the name of a category
  *
@@ -465,6 +469,19 @@ int log_add_filter(const char *drv_name, enum log_category_t cat_list[],
  */
 int log_remove_filter(const char *drv_name, int filter_num);
 
+/**
+ * log_device_set_enable() - Enable or disable a log device
+ *
+ * Devices are referenced by their driver, so use LOG_GET_DRIVER(name) to pass
+ * the driver to this function. For example if the driver is declared with
+ * LOG_DRIVER(wibble) then pass LOG_GET_DRIVER(wibble) here.
+ *
+ * @drv: Driver of device to enable
+ * @enable: true to enable, false to disable
+ * @return 0 if OK, -ENOENT if the driver was not found
+ */
+int log_device_set_enable(struct log_driver *drv, bool enable);
+
 #if CONFIG_IS_ENABLED(LOG)
 /**
  * log_init() - Set up the log system ready for use
diff --git a/test/log/log_test.c b/test/log/log_test.c
index fdee5e6757f..6a60ff6be3c 100644
--- a/test/log/log_test.c
+++ b/test/log/log_test.c
@@ -196,6 +196,13 @@ static int log_test(int testnum)
 		log_io("level %d\n", LOGL_DEBUG_IO);
 		break;
 	}
+	case 11:
+		log_err("default\n");
+		ret = log_device_set_enable(LOG_GET_DRIVER(console), false);
+		log_err("disabled\n");
+		ret = log_device_set_enable(LOG_GET_DRIVER(console), true);
+		log_err("enabled\n");
+		break;
 	}
 
 	return 0;
diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
index ddc28f19ee8..275f9382d2f 100644
--- a/test/py/tests/test_log.py
+++ b/test/py/tests/test_log.py
@@ -92,6 +92,13 @@ def test_log(u_boot_console):
         for i in range(7):
             assert 'log_test() level %d' % i == next(lines)
 
+    def test11():
+        """Test use of log_device_set_enable()"""
+        lines = run_test(11)
+        assert 'log_test() default'
+        # disabled should not be displayed
+        assert 'log_test() enabled'
+
     # TODO(sjg at chromium.org): Consider structuring this as separate tests
     cons = u_boot_console
     test0()
@@ -105,6 +112,7 @@ def test_log(u_boot_console):
     test8()
     test9()
     test10()
+    test11()
 
 @pytest.mark.buildconfigspec('cmd_log')
 def test_log_format(u_boot_console):
-- 
2.28.0.618.gf4bc123cb7-goog

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
                   ` (2 preceding siblings ...)
  2020-09-12 18:28 ` [PATCH 3/4] log: Add a way to enable/disable a log device Simon Glass
@ 2020-09-12 18:28 ` Simon Glass
  2020-09-12 20:16   ` Heinrich Schuchardt
  2020-10-12  1:15   ` Tom Rini
  2020-09-14  8:24 ` [PATCH 0/4] log: Fix the syslog spam when running tests Heinrich Schuchardt
  4 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2020-09-12 18:28 UTC (permalink / raw)
  To: u-boot

This driver interferes with other sandbox tests since it causes log output
to be interspersed with "No ethernet found." messages. Disable this driver
by default.

Enable it for the syslog tests so that they still pass.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/log_syslog.c           |  1 -
 test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
 test/log/syslog_test.h        | 16 ++++++++++++++++
 test/log/syslog_test_ndebug.c |  2 ++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/common/log_syslog.c b/common/log_syslog.c
index cf0dbba9bf5..149ff5af310 100644
--- a/common/log_syslog.c
+++ b/common/log_syslog.c
@@ -115,5 +115,4 @@ out:
 LOG_DRIVER(syslog) = {
 	.name	= "syslog",
 	.emit	= log_syslog_emit,
-	.flags	= LOGDF_ENABLE,
 };
diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
index abcb9ffd28b..febaca68e8d 100644
--- a/test/log/syslog_test.c
+++ b/test/log/syslog_test.c
@@ -56,6 +56,20 @@ int sb_log_tx_handler(struct udevice *dev, void *packet, unsigned int len)
 	return 0;
 }
 
+int syslog_test_setup(struct unit_test_state *uts)
+{
+	ut_assertok(log_device_set_enable(LOG_GET_DRIVER(syslog), true));
+
+	return 0;
+}
+
+int syslog_test_finish(struct unit_test_state *uts)
+{
+	ut_assertok(log_device_set_enable(LOG_GET_DRIVER(syslog), false));
+
+	return 0;
+}
+
 /**
  * log_test_syslog_err() - test log_err() function
  *
@@ -67,6 +81,7 @@ static int log_test_syslog_err(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
@@ -82,6 +97,7 @@ static int log_test_syslog_err(struct unit_test_state *uts)
 	sandbox_eth_set_tx_handler(0, NULL);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
@@ -98,6 +114,7 @@ static int log_test_syslog_warning(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
@@ -114,6 +131,7 @@ static int log_test_syslog_warning(struct unit_test_state *uts)
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
@@ -130,6 +148,7 @@ static int log_test_syslog_notice(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
@@ -146,6 +165,7 @@ static int log_test_syslog_notice(struct unit_test_state *uts)
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
@@ -162,6 +182,7 @@ static int log_test_syslog_info(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
@@ -178,6 +199,7 @@ static int log_test_syslog_info(struct unit_test_state *uts)
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
@@ -194,6 +216,7 @@ static int log_test_syslog_debug(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_DEBUG;
 	env_set("ethact", "eth at 10002000");
@@ -210,6 +233,7 @@ static int log_test_syslog_debug(struct unit_test_state *uts)
 	ut_assertnull(env.expected);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
diff --git a/test/log/syslog_test.h b/test/log/syslog_test.h
index 75900f3aad3..1310257bfe1 100644
--- a/test/log/syslog_test.h
+++ b/test/log/syslog_test.h
@@ -47,4 +47,20 @@ struct sb_log_env {
  */
 int sb_log_tx_handler(struct udevice *dev, void *packet, unsigned int len);
 
+/**
+ * syslog_test_setup() - Enable syslog logging ready for tests
+ *
+ * @uts: Test state
+ * @return 0 if OK, -ENOENT if the syslog log driver is not found
+ */
+int syslog_test_setup(struct unit_test_state *uts);
+
+/**
+ * syslog_test_finish() - Disable syslog logging after tests
+ *
+ * @uts: Test state
+ * @return 0 if OK, -ENOENT if the syslog log driver is not found
+ */
+int syslog_test_finish(struct unit_test_state *uts);
+
 #endif
diff --git a/test/log/syslog_test_ndebug.c b/test/log/syslog_test_ndebug.c
index 7815977ea27..c7f5a60861f 100644
--- a/test/log/syslog_test_ndebug.c
+++ b/test/log/syslog_test_ndebug.c
@@ -33,6 +33,7 @@ static int log_test_syslog_nodebug(struct unit_test_state *uts)
 	int old_log_level = gd->default_log_level;
 	struct sb_log_env env;
 
+	ut_assertok(syslog_test_setup(uts));
 	gd->log_fmt = LOGF_TEST;
 	gd->default_log_level = LOGL_INFO;
 	env_set("ethact", "eth at 10002000");
@@ -49,6 +50,7 @@ static int log_test_syslog_nodebug(struct unit_test_state *uts)
 	ut_assertnonnull(env.expected);
 	gd->default_log_level = old_log_level;
 	gd->log_fmt = log_get_default_format();
+	ut_assertok(syslog_test_finish(uts));
 
 	return 0;
 }
-- 
2.28.0.618.gf4bc123cb7-goog

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 18:28 ` [PATCH 4/4] log: Disable the syslog driver by default Simon Glass
@ 2020-09-12 20:16   ` Heinrich Schuchardt
  2020-09-12 20:24     ` Simon Glass
  2020-10-12  1:15   ` Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2020-09-12 20:16 UTC (permalink / raw)
  To: u-boot

Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass <sjg@chromium.org>:
>This driver interferes with other sandbox tests since it causes log
>output
>to be interspersed with "No ethernet found." messages. Disable this
>driver
>by default.
>
>Enable it for the syslog tests so that they still pass.
>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> common/log_syslog.c           |  1 -
> test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
> test/log/syslog_test.h        | 16 ++++++++++++++++
> test/log/syslog_test_ndebug.c |  2 ++
> 4 files changed, 42 insertions(+), 1 deletion(-)
>
>diff --git a/common/log_syslog.c b/common/log_syslog.c
>index cf0dbba9bf5..149ff5af310 100644
>--- a/common/log_syslog.c
>+++ b/common/log_syslog.c
>@@ -115,5 +115,4 @@ out:
> LOG_DRIVER(syslog) = {
> 	.name	= "syslog",
> 	.emit	= log_syslog_emit,
>-	.flags	= LOGDF_ENABLE,

What does the flag removal change outside Python testing?

Best regards

Heinrich

> };
>diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
>index abcb9ffd28b..febaca68e8d 100644
>--- a/test/log/syslog_test.c
>+++ b/test/log/syslog_test.c
>@@ -56,6 +56,20 @@ int sb_log_tx_handler(struct udevice *dev, void
>*packet, unsigned int len)
> 	return 0;
> }
> 
>+int syslog_test_setup(struct unit_test_state *uts)
>+{
>+	ut_assertok(log_device_set_enable(LOG_GET_DRIVER(syslog), true));
>+
>+	return 0;
>+}
>+
>+int syslog_test_finish(struct unit_test_state *uts)
>+{
>+	ut_assertok(log_device_set_enable(LOG_GET_DRIVER(syslog), false));
>+
>+	return 0;
>+}
>+
> /**
>  * log_test_syslog_err() - test log_err() function
>  *
>@@ -67,6 +81,7 @@ static int log_test_syslog_err(struct unit_test_state
>*uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_INFO;
> 	env_set("ethact", "eth at 10002000");
>@@ -82,6 +97,7 @@ static int log_test_syslog_err(struct unit_test_state
>*uts)
> 	sandbox_eth_set_tx_handler(0, NULL);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }
>@@ -98,6 +114,7 @@ static int log_test_syslog_warning(struct
>unit_test_state *uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_INFO;
> 	env_set("ethact", "eth at 10002000");
>@@ -114,6 +131,7 @@ static int log_test_syslog_warning(struct
>unit_test_state *uts)
> 	ut_assertnull(env.expected);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }
>@@ -130,6 +148,7 @@ static int log_test_syslog_notice(struct
>unit_test_state *uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_INFO;
> 	env_set("ethact", "eth at 10002000");
>@@ -146,6 +165,7 @@ static int log_test_syslog_notice(struct
>unit_test_state *uts)
> 	ut_assertnull(env.expected);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }
>@@ -162,6 +182,7 @@ static int log_test_syslog_info(struct
>unit_test_state *uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_INFO;
> 	env_set("ethact", "eth at 10002000");
>@@ -178,6 +199,7 @@ static int log_test_syslog_info(struct
>unit_test_state *uts)
> 	ut_assertnull(env.expected);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }
>@@ -194,6 +216,7 @@ static int log_test_syslog_debug(struct
>unit_test_state *uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_DEBUG;
> 	env_set("ethact", "eth at 10002000");
>@@ -210,6 +233,7 @@ static int log_test_syslog_debug(struct
>unit_test_state *uts)
> 	ut_assertnull(env.expected);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }
>diff --git a/test/log/syslog_test.h b/test/log/syslog_test.h
>index 75900f3aad3..1310257bfe1 100644
>--- a/test/log/syslog_test.h
>+++ b/test/log/syslog_test.h
>@@ -47,4 +47,20 @@ struct sb_log_env {
>  */
>int sb_log_tx_handler(struct udevice *dev, void *packet, unsigned int
>len);
> 
>+/**
>+ * syslog_test_setup() - Enable syslog logging ready for tests
>+ *
>+ * @uts: Test state
>+ * @return 0 if OK, -ENOENT if the syslog log driver is not found
>+ */
>+int syslog_test_setup(struct unit_test_state *uts);
>+
>+/**
>+ * syslog_test_finish() - Disable syslog logging after tests
>+ *
>+ * @uts: Test state
>+ * @return 0 if OK, -ENOENT if the syslog log driver is not found
>+ */
>+int syslog_test_finish(struct unit_test_state *uts);
>+
> #endif
>diff --git a/test/log/syslog_test_ndebug.c
>b/test/log/syslog_test_ndebug.c
>index 7815977ea27..c7f5a60861f 100644
>--- a/test/log/syslog_test_ndebug.c
>+++ b/test/log/syslog_test_ndebug.c
>@@ -33,6 +33,7 @@ static int log_test_syslog_nodebug(struct
>unit_test_state *uts)
> 	int old_log_level = gd->default_log_level;
> 	struct sb_log_env env;
> 
>+	ut_assertok(syslog_test_setup(uts));
> 	gd->log_fmt = LOGF_TEST;
> 	gd->default_log_level = LOGL_INFO;
> 	env_set("ethact", "eth at 10002000");
>@@ -49,6 +50,7 @@ static int log_test_syslog_nodebug(struct
>unit_test_state *uts)
> 	ut_assertnonnull(env.expected);
> 	gd->default_log_level = old_log_level;
> 	gd->log_fmt = log_get_default_format();
>+	ut_assertok(syslog_test_finish(uts));
> 
> 	return 0;
> }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 20:16   ` Heinrich Schuchardt
@ 2020-09-12 20:24     ` Simon Glass
  2020-09-12 20:34       ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2020-09-12 20:24 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Sat, 12 Sep 2020 at 14:16, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >This driver interferes with other sandbox tests since it causes log
> >output
> >to be interspersed with "No ethernet found." messages. Disable this
> >driver
> >by default.
> >
> >Enable it for the syslog tests so that they still pass.
> >
> >Signed-off-by: Simon Glass <sjg@chromium.org>
> >---
> >
> > common/log_syslog.c           |  1 -
> > test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
> > test/log/syslog_test.h        | 16 ++++++++++++++++
> > test/log/syslog_test_ndebug.c |  2 ++
> > 4 files changed, 42 insertions(+), 1 deletion(-)
> >
> >diff --git a/common/log_syslog.c b/common/log_syslog.c
> >index cf0dbba9bf5..149ff5af310 100644
> >--- a/common/log_syslog.c
> >+++ b/common/log_syslog.c
> >@@ -115,5 +115,4 @@ out:
> > LOG_DRIVER(syslog) = {
> >       .name   = "syslog",
> >       .emit   = log_syslog_emit,
> >-      .flags  = LOGDF_ENABLE,
>
> What does the flag removal change outside Python testing?

If this driver is to be used on a board, you must call log_device_set_enable().

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] log: Add a way to enable/disable a log device
  2020-09-12 18:28 ` [PATCH 3/4] log: Add a way to enable/disable a log device Simon Glass
@ 2020-09-12 20:24   ` Heinrich Schuchardt
  2020-09-13  1:24     ` Simon Glass
  2020-10-12  1:15   ` Tom Rini
  1 sibling, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2020-09-12 20:24 UTC (permalink / raw)
  To: u-boot

Am 12. September 2020 20:28:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
>At present all log devices are enabled by default. Add a function to
>allow
>devices to be disabled or enabled at runtime.

Should this capability be added to the log command?

Regards

Heinrich

>
>Signed-off-by: Simon Glass <sjg@chromium.org>
>---
>
> common/log.c              | 38 ++++++++++++++++++++++++++++++++++++++
> include/log.h             | 17 +++++++++++++++++
> test/log/log_test.c       |  7 +++++++
> test/py/tests/test_log.py |  8 ++++++++
> 4 files changed, 70 insertions(+)
>
>diff --git a/common/log.c b/common/log.c
>index ce528fe4535..a9aacfaf354 100644
>--- a/common/log.c
>+++ b/common/log.c
>@@ -297,6 +297,44 @@ int log_remove_filter(const char *drv_name, int
>filter_num)
> 	return -ENOENT;
> }
> 
>+/**
>+ * log_find_device_by_drv() - Find a device by its driver
>+ *
>+ * @drv: Log driver
>+ * @return Device associated with that driver, or NULL if not found
>+ */
>+static struct log_device *log_find_device_by_drv(struct log_driver
>*drv)
>+{
>+	struct log_device *ldev;
>+
>+	list_for_each_entry(ldev, &gd->log_head, sibling_node) {
>+		if (ldev->drv == drv)
>+			return ldev;
>+	}
>+	/*
>+	 * It is quite hard to pass an invalid driver since passing an
>unknown
>+	 * LOG_GET_DRIVER(xxx) would normally produce a compilation error.
>But
>+	 * it is possible to pass NULL, for example, so this
>+	 */
>+
>+	return NULL;
>+}
>+
>+int log_device_set_enable(struct log_driver *drv, bool enable)
>+{
>+	struct log_device *ldev;
>+
>+	ldev = log_find_device_by_drv(drv);
>+	if (!ldev)
>+		return -ENOENT;
>+	if (enable)
>+		ldev->flags |= LOGDF_ENABLE;
>+	else
>+		ldev->flags &= ~LOGDF_ENABLE;
>+
>+	return 0;
>+}
>+
> int log_init(void)
> {
>	struct log_driver *drv = ll_entry_start(struct log_driver,
>log_driver);
>diff --git a/include/log.h b/include/log.h
>index d28bc1ef0e4..4acc087b2e9 100644
>--- a/include/log.h
>+++ b/include/log.h
>@@ -388,6 +388,10 @@ struct log_filter {
> #define LOG_DRIVER(_name) \
> 	ll_entry_declare(struct log_driver, _name, log_driver)
> 
>+/* Get a pointer to a given driver */
>+#define LOG_GET_DRIVER(__name)						\
>+	ll_entry_get(struct log_driver, __name, log_driver)
>+
> /**
>  * log_get_cat_name() - Get the name of a category
>  *
>@@ -465,6 +469,19 @@ int log_add_filter(const char *drv_name, enum
>log_category_t cat_list[],
>  */
> int log_remove_filter(const char *drv_name, int filter_num);
> 
>+/**
>+ * log_device_set_enable() - Enable or disable a log device
>+ *
>+ * Devices are referenced by their driver, so use LOG_GET_DRIVER(name)
>to pass
>+ * the driver to this function. For example if the driver is declared
>with
>+ * LOG_DRIVER(wibble) then pass LOG_GET_DRIVER(wibble) here.
>+ *
>+ * @drv: Driver of device to enable
>+ * @enable: true to enable, false to disable
>+ * @return 0 if OK, -ENOENT if the driver was not found
>+ */
>+int log_device_set_enable(struct log_driver *drv, bool enable);
>+
> #if CONFIG_IS_ENABLED(LOG)
> /**
>  * log_init() - Set up the log system ready for use
>diff --git a/test/log/log_test.c b/test/log/log_test.c
>index fdee5e6757f..6a60ff6be3c 100644
>--- a/test/log/log_test.c
>+++ b/test/log/log_test.c
>@@ -196,6 +196,13 @@ static int log_test(int testnum)
> 		log_io("level %d\n", LOGL_DEBUG_IO);
> 		break;
> 	}
>+	case 11:
>+		log_err("default\n");
>+		ret = log_device_set_enable(LOG_GET_DRIVER(console), false);
>+		log_err("disabled\n");
>+		ret = log_device_set_enable(LOG_GET_DRIVER(console), true);
>+		log_err("enabled\n");
>+		break;
> 	}
> 
> 	return 0;
>diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py
>index ddc28f19ee8..275f9382d2f 100644
>--- a/test/py/tests/test_log.py
>+++ b/test/py/tests/test_log.py
>@@ -92,6 +92,13 @@ def test_log(u_boot_console):
>         for i in range(7):
>             assert 'log_test() level %d' % i == next(lines)
> 
>+    def test11():
>+        """Test use of log_device_set_enable()"""
>+        lines = run_test(11)
>+        assert 'log_test() default'
>+        # disabled should not be displayed
>+        assert 'log_test() enabled'
>+
>  # TODO(sjg at chromium.org): Consider structuring this as separate tests
>     cons = u_boot_console
>     test0()
>@@ -105,6 +112,7 @@ def test_log(u_boot_console):
>     test8()
>     test9()
>     test10()
>+    test11()
> 
> @pytest.mark.buildconfigspec('cmd_log')
> def test_log_format(u_boot_console):

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 20:24     ` Simon Glass
@ 2020-09-12 20:34       ` Heinrich Schuchardt
  2020-09-13  1:24         ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2020-09-12 20:34 UTC (permalink / raw)
  To: u-boot

Am 12. September 2020 22:24:24 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Sat, 12 Sep 2020 at 14:16, Heinrich Schuchardt <xypron.glpk@gmx.de>
>wrote:
>>
>> Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass
><sjg@chromium.org>:
>> >This driver interferes with other sandbox tests since it causes log
>> >output
>> >to be interspersed with "No ethernet found." messages. Disable this
>> >driver
>> >by default.
>> >
>> >Enable it for the syslog tests so that they still pass.
>> >
>> >Signed-off-by: Simon Glass <sjg@chromium.org>
>> >---
>> >
>> > common/log_syslog.c           |  1 -
>> > test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
>> > test/log/syslog_test.h        | 16 ++++++++++++++++
>> > test/log/syslog_test_ndebug.c |  2 ++
>> > 4 files changed, 42 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/common/log_syslog.c b/common/log_syslog.c
>> >index cf0dbba9bf5..149ff5af310 100644
>> >--- a/common/log_syslog.c
>> >+++ b/common/log_syslog.c
>> >@@ -115,5 +115,4 @@ out:
>> > LOG_DRIVER(syslog) = {
>> >       .name   = "syslog",
>> >       .emit   = log_syslog_emit,
>> >-      .flags  = LOGDF_ENABLE,
>>
>> What does the flag removal change outside Python testing?
>
>If this driver is to be used on a board, you must call
>log_device_set_enable().

This would pervert the nice log driver system that you have provided. I definitively want syslog with vanilla code just by customizing.

If you want to mute the network stack, then move it to use log and filter on the network uclass.

Or simply adjust the test that has hickups.

Best regards

Heinrich

>
>Regards,
>Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] log: Add a way to enable/disable a log device
  2020-09-12 20:24   ` Heinrich Schuchardt
@ 2020-09-13  1:24     ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2020-09-13  1:24 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Sat, 12 Sep 2020 at 14:25, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 12. September 2020 20:28:49 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >At present all log devices are enabled by default. Add a function to
> >allow
> >devices to be disabled or enabled at runtime.
>
> Should this capability be added to the log command?

It's probably not a bad idea. I'll take a look.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 20:34       ` Heinrich Schuchardt
@ 2020-09-13  1:24         ` Simon Glass
  2020-09-13  6:47           ` Heinrich Schuchardt
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2020-09-13  1:24 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Sat, 12 Sep 2020 at 14:34, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 12. September 2020 22:24:24 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Sat, 12 Sep 2020 at 14:16, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >wrote:
> >>
> >> Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass
> ><sjg@chromium.org>:
> >> >This driver interferes with other sandbox tests since it causes log
> >> >output
> >> >to be interspersed with "No ethernet found." messages. Disable this
> >> >driver
> >> >by default.
> >> >
> >> >Enable it for the syslog tests so that they still pass.
> >> >
> >> >Signed-off-by: Simon Glass <sjg@chromium.org>
> >> >---
> >> >
> >> > common/log_syslog.c           |  1 -
> >> > test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
> >> > test/log/syslog_test.h        | 16 ++++++++++++++++
> >> > test/log/syslog_test_ndebug.c |  2 ++
> >> > 4 files changed, 42 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/common/log_syslog.c b/common/log_syslog.c
> >> >index cf0dbba9bf5..149ff5af310 100644
> >> >--- a/common/log_syslog.c
> >> >+++ b/common/log_syslog.c
> >> >@@ -115,5 +115,4 @@ out:
> >> > LOG_DRIVER(syslog) = {
> >> >       .name   = "syslog",
> >> >       .emit   = log_syslog_emit,
> >> >-      .flags  = LOGDF_ENABLE,
> >>
> >> What does the flag removal change outside Python testing?
> >
> >If this driver is to be used on a board, you must call
> >log_device_set_enable().
>
> This would pervert the nice log driver system that you have provided. I definitively want syslog with vanilla code just by customizing.
>
> If you want to mute the network stack, then move it to use log and filter on the network uclass.

Well sandbox has a network but it fails to send syslog messages, which
is probably a good thing, at least without someone saying they want it
to happen.

>
> Or simply adjust the test that has hickups.

I don't like that idea because any test can produce log output.

Another option would be to only disable the device with sandbox, e.g.

#ifndef CONFIG_SANDBOX
   .flags  = LOGDF_ENABLE,
#endif

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-13  1:24         ` Simon Glass
@ 2020-09-13  6:47           ` Heinrich Schuchardt
  2020-09-13 13:26             ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: Heinrich Schuchardt @ 2020-09-13  6:47 UTC (permalink / raw)
  To: u-boot

Am 13. September 2020 03:24:02 MESZ schrieb Simon Glass <sjg@chromium.org>:
>Hi Heinrich,
>
>On Sat, 12 Sep 2020 at 14:34, Heinrich Schuchardt <xypron.glpk@gmx.de>
>wrote:
>>
>> Am 12. September 2020 22:24:24 MESZ schrieb Simon Glass
><sjg@chromium.org>:
>> >Hi Heinrich,
>> >
>> >On Sat, 12 Sep 2020 at 14:16, Heinrich Schuchardt
><xypron.glpk@gmx.de>
>> >wrote:
>> >>
>> >> Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass
>> ><sjg@chromium.org>:
>> >> >This driver interferes with other sandbox tests since it causes
>log
>> >> >output
>> >> >to be interspersed with "No ethernet found." messages. Disable
>this
>> >> >driver
>> >> >by default.
>> >> >
>> >> >Enable it for the syslog tests so that they still pass.
>> >> >
>> >> >Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> >---
>> >> >
>> >> > common/log_syslog.c           |  1 -
>> >> > test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
>> >> > test/log/syslog_test.h        | 16 ++++++++++++++++
>> >> > test/log/syslog_test_ndebug.c |  2 ++
>> >> > 4 files changed, 42 insertions(+), 1 deletion(-)
>> >> >
>> >> >diff --git a/common/log_syslog.c b/common/log_syslog.c
>> >> >index cf0dbba9bf5..149ff5af310 100644
>> >> >--- a/common/log_syslog.c
>> >> >+++ b/common/log_syslog.c
>> >> >@@ -115,5 +115,4 @@ out:
>> >> > LOG_DRIVER(syslog) = {
>> >> >       .name   = "syslog",
>> >> >       .emit   = log_syslog_emit,
>> >> >-      .flags  = LOGDF_ENABLE,
>> >>
>> >> What does the flag removal change outside Python testing?
>> >
>> >If this driver is to be used on a board, you must call
>> >log_device_set_enable().
>>
>> This would pervert the nice log driver system that you have provided.
>I definitively want syslog with vanilla code just by customizing.
>>
>> If you want to mute the network stack, then move it to use log and
>filter on the network uclass.
>
>Well sandbox has a network but it fails to send syslog messages, which
>is probably a good thing, at least without someone saying they want it
>to happen.
>
>>
>> Or simply adjust the test that has hickups.
>
>I don't like that idea because any test can produce log output.
>
>Another option would be to only disable the device with sandbox, e.g.
>
>#ifndef CONFIG_SANDBOX
>   .flags  = LOGDF_ENABLE,
>#endif

The decision to send syslog messages is not board specific but application specific. This is why I think it must be customizable. I would be fine if you would simply remove log_syslog from sandbox_defconfig.

What I am missing most in the log command is the possibility to set the verbosity per log area/uclass. This would allow to see debug messages only from the area of interest. Especially network does not work when debugging messages are shown per packet, while the user might be interested in debug messages from other areas e.g. EFI.

Concerning your problem such a change would allow to selectively mute network messages.

Would such a change make sense to you?

Best regards

Heinrich

>
>Regards,
>Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-13  6:47           ` Heinrich Schuchardt
@ 2020-09-13 13:26             ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2020-09-13 13:26 UTC (permalink / raw)
  To: u-boot

Hi Heinrich,

On Sun, 13 Sep 2020 at 00:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> Am 13. September 2020 03:24:02 MESZ schrieb Simon Glass <sjg@chromium.org>:
> >Hi Heinrich,
> >
> >On Sat, 12 Sep 2020 at 14:34, Heinrich Schuchardt <xypron.glpk@gmx.de>
> >wrote:
> >>
> >> Am 12. September 2020 22:24:24 MESZ schrieb Simon Glass
> ><sjg@chromium.org>:
> >> >Hi Heinrich,
> >> >
> >> >On Sat, 12 Sep 2020 at 14:16, Heinrich Schuchardt
> ><xypron.glpk@gmx.de>
> >> >wrote:
> >> >>
> >> >> Am 12. September 2020 20:28:50 MESZ schrieb Simon Glass
> >> ><sjg@chromium.org>:
> >> >> >This driver interferes with other sandbox tests since it causes
> >log
> >> >> >output
> >> >> >to be interspersed with "No ethernet found." messages. Disable
> >this
> >> >> >driver
> >> >> >by default.
> >> >> >
> >> >> >Enable it for the syslog tests so that they still pass.
> >> >> >
> >> >> >Signed-off-by: Simon Glass <sjg@chromium.org>
> >> >> >---
> >> >> >
> >> >> > common/log_syslog.c           |  1 -
> >> >> > test/log/syslog_test.c        | 24 ++++++++++++++++++++++++
> >> >> > test/log/syslog_test.h        | 16 ++++++++++++++++
> >> >> > test/log/syslog_test_ndebug.c |  2 ++
> >> >> > 4 files changed, 42 insertions(+), 1 deletion(-)
> >> >> >
> >> >> >diff --git a/common/log_syslog.c b/common/log_syslog.c
> >> >> >index cf0dbba9bf5..149ff5af310 100644
> >> >> >--- a/common/log_syslog.c
> >> >> >+++ b/common/log_syslog.c
> >> >> >@@ -115,5 +115,4 @@ out:
> >> >> > LOG_DRIVER(syslog) = {
> >> >> >       .name   = "syslog",
> >> >> >       .emit   = log_syslog_emit,
> >> >> >-      .flags  = LOGDF_ENABLE,
> >> >>
> >> >> What does the flag removal change outside Python testing?
> >> >
> >> >If this driver is to be used on a board, you must call
> >> >log_device_set_enable().
> >>
> >> This would pervert the nice log driver system that you have provided.
> >I definitively want syslog with vanilla code just by customizing.
> >>
> >> If you want to mute the network stack, then move it to use log and
> >filter on the network uclass.
> >
> >Well sandbox has a network but it fails to send syslog messages, which
> >is probably a good thing, at least without someone saying they want it
> >to happen.
> >
> >>
> >> Or simply adjust the test that has hickups.
> >
> >I don't like that idea because any test can produce log output.
> >
> >Another option would be to only disable the device with sandbox, e.g.
> >
> >#ifndef CONFIG_SANDBOX
> >   .flags  = LOGDF_ENABLE,
> >#endif
>
> The decision to send syslog messages is not board specific but application specific. This is why I think it must be customizable. I would be fine if you would simply remove log_syslog from sandbox_defconfig.

But then we would lose test coverage for syslog. I agree that making
it customisable is a good thing. But what do you think is the solution
to the problem at hand? As above, the driver is disabled by default
only on sandbox, since it must be enabled for testing, but is
intrusive otherwise.

>
> What I am missing most in the log command is the possibility to set the verbosity per log area/uclass. This would allow to see debug messages only from the area of interest. Especially network does not work when debugging messages are shown per packet, while the user might be interested in debug messages from other areas e.g. EFI.
>
> Concerning your problem such a change would allow to selectively mute network messages.
>
> Would such a change make sense to you?

Yes, but I feel these are separate issues:
- add a command enabling / disabling a log driver
- add a command to add/remove log filters based on category/level/file

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/4] log: Fix the syslog spam when running tests
  2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
                   ` (3 preceding siblings ...)
  2020-09-12 18:28 ` [PATCH 4/4] log: Disable the syslog driver by default Simon Glass
@ 2020-09-14  8:24 ` Heinrich Schuchardt
  4 siblings, 0 replies; 18+ messages in thread
From: Heinrich Schuchardt @ 2020-09-14  8:24 UTC (permalink / raw)
  To: u-boot

On 9/12/20 8:28 PM, Simon Glass wrote:
> At present running some sandbox tests produces unwanted ethernet errors
> from the syslog driver, e.g.:
>
> $ ./u-boot -c "ut bloblist"
>
> Running 5 bloblist tests
> Test: bloblist_test_bad_blob
> Test: bloblist_test_blob
> Test: bloblist_test_blob_ensure
> Failed to allocate e0 bytes size=100, need size=130
> No ethernet found.
> bloblist add: returning err=-28
> No ethernet found.
> Test: bloblist_test_checksum
> Checksum f3129fae != 338fbdc
> No ethernet found.
> Bad checksum: returning err=-5
> No ethernet found.
> ...
>
> This series fixes this by making the syslog driver disabled by default. It
> can be enabled with a function call.

Hello Simon,

in log_syslog_emit() I already set processing_msg = 1 to fend of
messages from the network driver when writing messages to syslog.

The idea was right but not in the right place. This logic should be
moved to log_dispatch() to securely avoid that writing messages creates
new messages which even might lead to endless recursion.

See
[PATCH 1/1] log: mute messages generated by log drivers
https://lists.denx.de/pipermail/u-boot/2020-September/426418.html

Best regards

Heinrich

>
>
> Simon Glass (4):
>   log: Add a flag to enable log drivers
>   log: Drop #ifdef in log_test
>   log: Add a way to enable/disable a log device
>   log: Disable the syslog driver by default
>
>  common/log.c                  | 42 ++++++++++++++++++++++++++++++++++-
>  common/log_console.c          |  1 +
>  include/log.h                 | 28 ++++++++++++++++++++++-
>  test/log/log_test.c           |  9 ++++++--
>  test/log/syslog_test.c        | 24 ++++++++++++++++++++
>  test/log/syslog_test.h        | 16 +++++++++++++
>  test/log/syslog_test_ndebug.c |  2 ++
>  test/py/tests/test_log.py     |  8 +++++++
>  8 files changed, 126 insertions(+), 4 deletions(-)
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] log: Add a flag to enable log drivers
  2020-09-12 18:28 ` [PATCH 1/4] log: Add a flag to enable log drivers Simon Glass
@ 2020-10-12  1:14   ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2020-10-12  1:14 UTC (permalink / raw)
  To: u-boot

On Sat, Sep 12, 2020 at 12:28:47PM -0600, Simon Glass wrote:

> At present there is no way to disable a log driver. But the syslog driver
> causes (attempted) network traffic in sandbox every time a log message
> is printed, which is often.
> 
> Add a flag to enable a log driver. Adjust struct log_device to use a short
> for next_filter_num so that no more memory is used for devices. Also fix
> a missing line in the struct log_driver comment while here.
> 
> To maintain compatibility, enable it for all drivers for now.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201011/d716825b/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/4] log: Drop #ifdef in log_test
  2020-09-12 18:28 ` [PATCH 2/4] log: Drop #ifdef in log_test Simon Glass
@ 2020-10-12  1:14   ` Tom Rini
  0 siblings, 0 replies; 18+ messages in thread
From: Tom Rini @ 2020-10-12  1:14 UTC (permalink / raw)
  To: u-boot

On Sat, Sep 12, 2020 at 12:28:48PM -0600, Simon Glass wrote:

> This is not needed as the Makefile only builds the file if CONFIG_LOG_TEST
> is enabled. Drop it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201011/ff7c3bc1/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 3/4] log: Add a way to enable/disable a log device
  2020-09-12 18:28 ` [PATCH 3/4] log: Add a way to enable/disable a log device Simon Glass
  2020-09-12 20:24   ` Heinrich Schuchardt
@ 2020-10-12  1:15   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2020-10-12  1:15 UTC (permalink / raw)
  To: u-boot

On Sat, Sep 12, 2020 at 12:28:49PM -0600, Simon Glass wrote:

> At present all log devices are enabled by default. Add a function to allow
> devices to be disabled or enabled at runtime.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201011/7f9d01e4/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] log: Disable the syslog driver by default
  2020-09-12 18:28 ` [PATCH 4/4] log: Disable the syslog driver by default Simon Glass
  2020-09-12 20:16   ` Heinrich Schuchardt
@ 2020-10-12  1:15   ` Tom Rini
  1 sibling, 0 replies; 18+ messages in thread
From: Tom Rini @ 2020-10-12  1:15 UTC (permalink / raw)
  To: u-boot

On Sat, Sep 12, 2020 at 12:28:50PM -0600, Simon Glass wrote:

> This driver interferes with other sandbox tests since it causes log output
> to be interspersed with "No ethernet found." messages. Disable this driver
> by default.
> 
> Enable it for the syslog tests so that they still pass.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201011/dd788e57/attachment.sig>

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-10-12  1:15 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-12 18:28 [PATCH 0/4] log: Fix the syslog spam when running tests Simon Glass
2020-09-12 18:28 ` [PATCH 1/4] log: Add a flag to enable log drivers Simon Glass
2020-10-12  1:14   ` Tom Rini
2020-09-12 18:28 ` [PATCH 2/4] log: Drop #ifdef in log_test Simon Glass
2020-10-12  1:14   ` Tom Rini
2020-09-12 18:28 ` [PATCH 3/4] log: Add a way to enable/disable a log device Simon Glass
2020-09-12 20:24   ` Heinrich Schuchardt
2020-09-13  1:24     ` Simon Glass
2020-10-12  1:15   ` Tom Rini
2020-09-12 18:28 ` [PATCH 4/4] log: Disable the syslog driver by default Simon Glass
2020-09-12 20:16   ` Heinrich Schuchardt
2020-09-12 20:24     ` Simon Glass
2020-09-12 20:34       ` Heinrich Schuchardt
2020-09-13  1:24         ` Simon Glass
2020-09-13  6:47           ` Heinrich Schuchardt
2020-09-13 13:26             ` Simon Glass
2020-10-12  1:15   ` Tom Rini
2020-09-14  8:24 ` [PATCH 0/4] log: Fix the syslog spam when running tests Heinrich Schuchardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.