[dpdk-dev,v2,1/3] eal: add quiet mode to suppress log output to stdout

Message ID 1483635001-15473-2-git-send-email-slawomirx.mrozowicz@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Pablo de Lara Guarch
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel compilation success Compilation OK

Commit Message

Slawomir Mrozowicz Jan. 5, 2017, 4:49 p.m. UTC
  Add EAL option to suppresses all log output to stdout.

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 lib/librte_eal/common/eal_common_log.c          | 13 +++++++++++++
 lib/librte_eal/common/eal_common_options.c      | 10 +++++++++-
 lib/librte_eal/common/eal_options.h             |  2 ++
 lib/librte_eal/common/include/rte_log.h         | 15 +++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_log.c           |  8 +++++---
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  8 ++++++++
 6 files changed, 52 insertions(+), 4 deletions(-)
  

Comments

Thomas Monjalon Jan. 5, 2017, 3:40 p.m. UTC | #1
2017-01-05 17:49, Slawomir Mrozowicz:
> Add EAL option to suppresses all log output to stdout.
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
[...]
>  	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
> +		"  -s, --"OPT_LOG_STDOUT_SILENT" Silent mode, suppresses log "
> +		"output to stdout\n"

How does it behave when the app configure a custom log function which
prints to stdout?

Generally speaking, we should stop adding such options in DPDK and
move this kind of responsibility to the applications (providing some API
and helpers).
  
Doherty, Declan Jan. 10, 2017, 10:41 a.m. UTC | #2
On 05/01/17 15:40, Thomas Monjalon wrote:
> 2017-01-05 17:49, Slawomir Mrozowicz:
>> Add EAL option to suppresses all log output to stdout.
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> [...]
>>  	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
>> +		"  -s, --"OPT_LOG_STDOUT_SILENT" Silent mode, suppresses log "
>> +		"output to stdout\n"
>
> How does it behave when the app configure a custom log function which
> prints to stdout?
>
> Generally speaking, we should stop adding such options in DPDK and
> move this kind of responsibility to the applications (providing some API
> and helpers).
>
Hey Thomas,

this option wouldn't affect a custom log function as the flag this 
controls is only used within by the default Linux log file stream in the 
console_log_write function. Maybe a better option would be to change the 
general behaviour of the default logging function and remove the printf 
to stdout completely and add a generic verbose option applicable to all 
environments which would allow log output to be directed to stdout. I 
did notice that the default behaviour between Linux/BSD does seem to be 
different with Linux defaulting to syslog/stdout and BSD to stderr.

One way or another I would like to able to disable streaming of log 
output to stdout when using the default log stream in Linux, but I'm 
open to suggestions as to the best way of doing this?
  

Patch

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index e45d326..701b309 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -45,6 +45,7 @@ 
 struct rte_logs rte_logs = {
 	.type = ~0,
 	.level = RTE_LOG_DEBUG,
+	.silent = 0,
 	.file = NULL,
 };
 
@@ -87,6 +88,18 @@  rte_get_log_level(void)
 	return rte_logs.level;
 }
 
+void
+rte_log_silence_stdout(void)
+{
+	rte_logs.silent = 1;
+}
+
+int
+rte_log_stdout_silent(void)
+{
+	return rte_logs.silent;
+}
+
 /* Set global log type */
 void
 rte_set_log_type(uint32_t type, int enable)
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 611e581..c47e02b 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -69,6 +69,7 @@  eal_short_options[] =
 	"r:" /* memory ranks */
 	"v"  /* version */
 	"w:" /* pci-whitelist */
+	"s"  /* silence log output to stdout */
 	;
 
 const struct option
@@ -95,6 +96,7 @@  eal_long_options[] = {
 	{OPT_VFIO_INTR,         1, NULL, OPT_VFIO_INTR_NUM        },
 	{OPT_VMWARE_TSC_MAP,    0, NULL, OPT_VMWARE_TSC_MAP_NUM   },
 	{OPT_XEN_DOM0,          0, NULL, OPT_XEN_DOM0_NUM         },
+	{OPT_LOG_STDOUT_SILENT, 0, NULL, OPT_LOG_STDOUT_SILENT_NUM },
 	{0,                     0, NULL, 0                        }
 };
 
@@ -844,6 +846,9 @@  eal_parse_common_option(int opt, const char *optarg,
 		 * even if info or warning messages are disabled */
 		RTE_LOG(CRIT, EAL, "RTE Version: '%s'\n", rte_version());
 		break;
+	case 's':
+		rte_log_silence_stdout();
+		break;
 
 	/* long options */
 	case OPT_HUGE_UNLINK_NUM:
@@ -1055,9 +1060,12 @@  eal_common_usage(void)
 	       "  -d LIB.so|DIR       Add a driver or driver directory\n"
 	       "                      (can be used multiple times)\n"
 	       "  --"OPT_VMWARE_TSC_MAP"    Use VMware TSC map instead of native RDTSC\n"
-	       "  --"OPT_PROC_TYPE"         Type of this process (primary|secondary|auto)\n"
+		"  --"OPT_PROC_TYPE"         Type of this process "
+		"(primary|secondary|auto)\n"
 	       "  --"OPT_SYSLOG"            Set syslog facility\n"
 	       "  --"OPT_LOG_LEVEL"         Set default log level\n"
+		"  -s, --"OPT_LOG_STDOUT_SILENT" Silent mode, suppresses log "
+		"output to stdout\n"
 	       "  -v                  Display version information on startup\n"
 	       "  -h, --help          This help\n"
 	       "\nEAL options for DEBUG use only:\n"
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..bd9778f 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -41,6 +41,8 @@  enum {
 	OPT_PCI_BLACKLIST_NUM   = 'b',
 #define OPT_PCI_WHITELIST     "pci-whitelist"
 	OPT_PCI_WHITELIST_NUM   = 'w',
+#define OPT_LOG_STDOUT_SILENT  "log-stdout-silent"
+	OPT_LOG_STDOUT_SILENT_NUM   = 's',
 
 	/* first long only option value must be >= 256, so that we won't
 	 * conflict with short options */
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 671e274..3898676 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -54,6 +54,7 @@  extern "C" {
 struct rte_logs {
 	uint32_t type;  /**< Bitfield with enabled logs. */
 	uint32_t level; /**< Log level. */
+	uint32_t silent; /**< silence logging to stdout */
 	FILE *file;     /**< Output file set by rte_openlog_stream, or NULL. */
 };
 
@@ -132,6 +133,20 @@  void rte_set_log_level(uint32_t level);
 uint32_t rte_get_log_level(void);
 
 /**
+ * Silence output to stdout by logging facilities
+ */
+void rte_log_silence_stdout(void);
+
+/**
+ * Check if echoing log output to stdout is enabled.
+ *
+ * @return
+ * - Returns 0 if echoing to logging to stdout is enabled
+ * - Returns 1 if logging is in silent mode
+ */
+int rte_log_stdout_silent(void);
+
+/**
  * Enable or disable the log type.
  *
  * @param type
diff --git a/lib/librte_eal/linuxapp/eal/eal_log.c b/lib/librte_eal/linuxapp/eal/eal_log.c
index e3a50aa..d88ed82 100644
--- a/lib/librte_eal/linuxapp/eal/eal_log.c
+++ b/lib/librte_eal/linuxapp/eal/eal_log.c
@@ -56,12 +56,14 @@  static ssize_t
 console_log_write(__attribute__((unused)) void *c, const char *buf, size_t size)
 {
 	char copybuf[BUFSIZ + 1];
-	ssize_t ret;
+	ssize_t ret = 0;
 	uint32_t loglevel;
 
 	/* write on stdout */
-	ret = fwrite(buf, 1, size, stdout);
-	fflush(stdout);
+	if (!rte_log_stdout_silent()) {
+		ret = fwrite(buf, 1, size, stdout);
+		fflush(stdout);
+	}
 
 	/* truncate message if too big (should not happen) */
 	if (size > BUFSIZ)
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 83721ba..f60c3f7 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -178,3 +178,11 @@  DPDK_16.11 {
 	rte_eal_vdrv_unregister;
 
 } DPDK_16.07;
+
+DPDK_17.02 {
+	global:
+
+	rte_log_stdout_silent;
+	rte_log_silence_stdout;
+
+} DPDK_16.11;