[dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[Andrew Rybchenko]

Unfortunately we're a bit late with dynamic logging implementation.
So, it can wait for 18.05 release cycle if required.

The series adds EXPERIMENTAL EAL feature which removes dependency
on EAL arguments processing and log types registration. It stores
EAL loglevel arguments in the list and adds API function to register
a new log type and pick up its value from EAL arguments.

For us it is important since we would like to be able to control
per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.

The series already follows log type names format defined recently.

Ivan Malov (6):
  eal: register log type and pick level from EAL args
  net/sfc: add support for driver-wide dynamic logging
  net/sfc: add support for per-port dynamic logging
  net/sfc: prepare to merge init logs with main log type
  net/sfc: remove dedicated init log parameter
  net/sfc: add dynamic log level for MCDI messages

 doc/guides/nics/sfc_efx.rst                | 41 ++++++++++++----
 drivers/net/sfc/efsys.h                    |  3 +-
 drivers/net/sfc/sfc.c                      | 38 +++++++++++++--
 drivers/net/sfc/sfc.h                      |  8 +++-
 drivers/net/sfc/sfc_dp.c                   |  5 +-
 drivers/net/sfc/sfc_dp.h                   |  6 ++-
 drivers/net/sfc/sfc_ef10_rx.c              |  4 +-
 drivers/net/sfc/sfc_ethdev.c               | 36 ++++++++------
 drivers/net/sfc/sfc_intr.c                 |  6 +--
 drivers/net/sfc/sfc_kvargs.c               |  2 -
 drivers/net/sfc/sfc_kvargs.h               |  4 --
 drivers/net/sfc/sfc_log.h                  | 77 ++++++++++++++++++++++++------
 drivers/net/sfc/sfc_mcdi.c                 | 25 +++++-----
 drivers/net/sfc/sfc_rx.c                   |  2 +-
 drivers/net/sfc/sfc_tx.c                   |  2 +-
 lib/librte_eal/common/eal_common_log.c     | 36 ++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 23 +++++++++
 lib/librte_eal/common/include/rte_log.h    | 48 +++++++++++++++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 19 files changed, 294 insertions(+), 73 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/6] eal: register log type and pick level from EAL args

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Dynamic log types are registered on RTE_INIT() step.
This allows one to set log levels by EAL options on
application launch. However, this does not allow to
manage log types if they are created during runtime.

EAL does not store log levels and types passed from
the command line. Thus, they cannot be picked later.
This is an obvious flaw since it would be better to
be able to pick levels for dynamic types registered
for runtime-determined facilities such as NIC ports.

This patch provides a mechanism to store log levels
passed from EAL options and adds an API to register
log types and pick levels from the internal storage.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 lib/librte_eal/common/eal_common_log.c     | 36 ++++++++++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 23 ++++++++++++++
 lib/librte_eal/common/include/rte_log.h    | 48 ++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 4 files changed, 108 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 5a3400e..6ef3003 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -23,6 +23,10 @@ struct rte_logs rte_logs = {
 	.file = NULL,
 };
 
+/** Global list of valid EAL log level options */
+struct rte_eal_opt_loglevel_list opt_loglevel_list =
+	TAILQ_HEAD_INITIALIZER(opt_loglevel_list);
+
 /* Stream to use for logging if rte_logs.file is NULL */
 static FILE *default_log_stream;
 
@@ -184,6 +188,38 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+/* Register an extended log type and try to pick its level from EAL options */
+int
+rte_log_register_type_and_pick_level(const char *name, uint32_t level_def)
+{
+	struct rte_eal_opt_loglevel *opt_ll;
+	uint32_t level = level_def;
+	int type;
+
+	type = rte_log_register(name);
+	if (type < 0)
+		return type;
+
+	TAILQ_FOREACH(opt_ll, &opt_loglevel_list, next) {
+		regex_t r;
+
+		if (opt_ll->level > RTE_LOG_DEBUG)
+			continue;
+
+		if (regcomp(&r, opt_ll->re_type, 0) != 0)
+			continue;
+
+		if (regexec(&r, name, 0, NULL, 0) == 0)
+			level = opt_ll->level;
+
+		regfree(&r);
+	}
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return type;
+}
+
 struct logtype {
 	uint32_t log_id;
 	const char *logtype;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 996a034..b7b44d5 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -971,6 +971,29 @@ eal_parse_log_level(const char *arg)
 		printf("cannot set log level %s,%lu\n",
 			type, tmp);
 		goto fail;
+	} else {
+		struct rte_eal_opt_loglevel *opt_ll;
+
+		/*
+		 * Save the type (regexp string) and the loglevel
+		 * in the global storage so that it could be used
+		 * to configure dynamic logtypes which are absent
+		 * at the moment of EAL option processing but may
+		 * be registered during runtime.
+		 */
+		opt_ll = malloc(sizeof(*opt_ll));
+		if (opt_ll == NULL)
+			goto fail;
+
+		opt_ll->re_type = strdup(type);
+		if (opt_ll->re_type == NULL) {
+			free(opt_ll);
+			goto fail;
+		}
+
+		opt_ll->level = tmp;
+
+		TAILQ_INSERT_HEAD(&opt_loglevel_list, opt_ll, next);
 	}
 
 	free(str);
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 9029c78..5f4799e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -20,6 +20,7 @@ extern "C" {
 #include <stdint.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <sys/queue.h>
 
 #include <rte_common.h>
 #include <rte_config.h>
@@ -85,6 +86,32 @@ extern struct rte_logs rte_logs;
 #define RTE_LOG_DEBUG    8U  /**< Debug-level messages.             */
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Entry definition for the storage to keep EAL log level options
+ * which are found to have log type regular expressions specified.
+ */
+struct rte_eal_opt_loglevel {
+	/** Next list entry */
+	TAILQ_ENTRY(rte_eal_opt_loglevel) next;
+	/** Regular expression string obtained from the option */
+	char *re_type;
+	/** Log level value obtained from the option */
+	uint32_t level;
+};
+
+TAILQ_HEAD(rte_eal_opt_loglevel_list, rte_eal_opt_loglevel);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Global list of EAL log level options featuring log type expressions
+ */
+extern struct rte_eal_opt_loglevel_list opt_loglevel_list;
+
+/**
  * Change the stream that will be used by the logging system.
  *
  * This can be done at any time. The f argument represents the stream
@@ -195,6 +222,27 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Register a dynamic log type and try to pick its level from EAL options
+ *
+ * rte_log_register() is called inside. If successful, the function tries
+ * to search for matching regexp in the list of EAL log level options and
+ * pick the level from the last matching entry. If nothing can be applied
+ * from the list, the level will be set to the user-defined default value.
+ *
+ * @param name
+ *    Name for the log type to be registered
+ * @param level_def
+ *    Fallback level to be set if the global list has no matching options
+ * @return
+ *    - >=0: the newly registered log type
+ *    - <0: rte_log_register() error value
+ */
+int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def);
+
+/**
  * Dump log information.
  *
  * Dump the global level and the registered log types.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 7088b72..7e931a9 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -217,6 +217,7 @@ EXPERIMENTAL {
 	rte_eal_devargs_remove;
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
+	rte_log_register_type_and_pick_level;
 	rte_service_attr_get;
 	rte_service_attr_reset_all;
 	rte_service_component_register;
-- 
2.7.4

[dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst   | 19 +++++++++++++++++++
 drivers/net/sfc/efsys.h       |  3 ++-
 drivers/net/sfc/sfc_dp.c      |  5 +++--
 drivers/net/sfc/sfc_dp.h      |  6 ++++--
 drivers/net/sfc/sfc_ef10_rx.c |  4 ++--
 drivers/net/sfc/sfc_ethdev.c  | 13 +++++++++++++
 drivers/net/sfc/sfc_log.h     | 12 ++++++++++++
 7 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 8e0782c..4d89cfd 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -305,3 +305,22 @@ boolean parameters value.
   only possible to set an arbitrary value on SFN8xxx provided that
   firmware version is 6.2.1.1033 or higher, otherwise any positive
   value will select a fixed update period of **1000** milliseconds
+
+
+Dynamic Logging Parameters
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+One may leverage EAL option "--log-level" to change default levels
+for the log types supported by the driver. The option is used with
+an argument typically consisting of two parts separated by a comma.
+
+Level value is the last part which takes an integer greater than 0.
+Log type is the former part which may contain a regular expression.
+Depending on the choice of the expression, the given log level may
+be used either for some specific log type or for a subset of types.
+
+SFC EFX PMD provides the following log types available for control:
+
+- ``pmd.net.sfc.driver`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Affects driver-wide messages unrelated to any particular devices.
diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
index c7a54c3..8dd225e 100644
--- a/drivers/net/sfc/efsys.h
+++ b/drivers/net/sfc/efsys.h
@@ -26,6 +26,7 @@
 #include <rte_io.h>
 
 #include "sfc_debug.h"
+#include "sfc_log.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -721,7 +722,7 @@ typedef uint64_t	efsys_stat_t;
 #define EFSYS_ERR(_esip, _code, _dword0, _dword1)			\
 	do {								\
 		(void)(_esip);						\
-		RTE_LOG(ERR, PMD, "FATAL ERROR #%u (0x%08x%08x)\n",	\
+		SFC_GENERIC_LOG(ERR, "FATAL ERROR #%u (0x%08x%08x)",	\
 			(_code), (_dword0), (_dword1));			\
 		_NOTE(CONSTANTCONDITION);				\
 	} while (B_FALSE)
diff --git a/drivers/net/sfc/sfc_dp.c b/drivers/net/sfc/sfc_dp.c
index 9a5ca20..b121dc0 100644
--- a/drivers/net/sfc/sfc_dp.c
+++ b/drivers/net/sfc/sfc_dp.c
@@ -14,6 +14,7 @@
 #include <rte_log.h>
 
 #include "sfc_dp.h"
+#include "sfc_log.h"
 
 void
 sfc_dp_queue_init(struct sfc_dp_queue *dpq, uint16_t port_id, uint16_t queue_id,
@@ -63,8 +64,8 @@ int
 sfc_dp_register(struct sfc_dp_list *head, struct sfc_dp *entry)
 {
 	if (sfc_dp_find_by_name(head, entry->type, entry->name) != NULL) {
-		rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD,
-			"sfc %s dapapath '%s' already registered\n",
+		SFC_GENERIC_LOG(ERR,
+			"sfc %s dapapath '%s' already registered",
 			entry->type == SFC_DP_RX ? "Rx" :
 			entry->type == SFC_DP_TX ? "Tx" :
 			"unknown",
diff --git a/drivers/net/sfc/sfc_dp.h b/drivers/net/sfc/sfc_dp.h
index b142532..26e7195 100644
--- a/drivers/net/sfc/sfc_dp.h
+++ b/drivers/net/sfc/sfc_dp.h
@@ -15,6 +15,8 @@
 
 #include <rte_pci.h>
 
+#include "sfc_log.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -58,10 +60,10 @@ void sfc_dp_queue_init(struct sfc_dp_queue *dpq,
 		const struct sfc_dp_queue *_dpq = (dpq);		\
 		const struct rte_pci_addr *_addr = &(_dpq)->pci_addr;	\
 									\
-		RTE_LOG(level, PMD,					\
+		SFC_GENERIC_LOG(level,					\
 			RTE_FMT("%s " PCI_PRI_FMT			\
 				" #%" PRIu16 ".%" PRIu16 ": "		\
-				RTE_FMT_HEAD(__VA_ARGS__,) "\n",	\
+				RTE_FMT_HEAD(__VA_ARGS__ ,),		\
 				dp_name,				\
 				_addr->domain, _addr->bus,		\
 				_addr->devid, _addr->function,		\
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index 0b3e8fb..93617e7 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -587,8 +587,8 @@ sfc_ef10_supported_ptypes_get(uint32_t tunnel_encaps)
 	      1u << EFX_TUNNEL_PROTOCOL_NVGRE):
 		return ef10_overlay_ptypes;
 	default:
-		RTE_LOG(ERR, PMD,
-			"Unexpected set of supported tunnel encapsulations: %#x\n",
+		SFC_GENERIC_LOG(ERR,
+			"Unexpected set of supported tunnel encapsulations: %#x",
 			tunnel_encaps);
 		/* FALLTHROUGH */
 	case 0:
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 89a4529..f167120 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -27,6 +27,8 @@
 #include "sfc_dp.h"
 #include "sfc_dp_rx.h"
 
+uint32_t sfc_logtype_driver;
+
 static struct sfc_dp_list sfc_dp_head =
 	TAILQ_HEAD_INITIALIZER(sfc_dp_head);
 
@@ -2082,3 +2084,14 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
 	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
 	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
+
+RTE_INIT(sfc_driver_register_logtype);
+static void
+sfc_driver_register_logtype(void)
+{
+	int ret;
+
+	ret = rte_log_register_type_and_pick_level(SFC_LOGTYPE_PREFIX "driver",
+						   RTE_LOG_NOTICE);
+	sfc_logtype_driver = (ret < 0) ? RTE_LOGTYPE_PMD : ret;
+}
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index a18191e..ecafffc 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -10,6 +10,18 @@
 #ifndef _SFC_LOG_H_
 #define _SFC_LOG_H_
 
+/** Generic driver log type */
+extern uint32_t sfc_logtype_driver;
+
+/** Common log type name prefix */
+#define SFC_LOGTYPE_PREFIX	"pmd.net.sfc."
+
+/** Log PMD generic message, add a prefix and a line break */
+#define SFC_GENERIC_LOG(level, ...) \
+	rte_log(RTE_LOG_ ## level, sfc_logtype_driver,			\
+		RTE_FMT("PMD: " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
+			RTE_FMT_TAIL(__VA_ARGS__ ,)))
+
 /* Log PMD message, automatically add prefix and \n */
 #define SFC_LOG(sa, level, ...) \
 	do {								\
-- 
2.7.4

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Stephen Hemminger]

On Thu, 25 Jan 2018 17:00:43 +0000
Andrew Rybchenko <arybchenko@solarflare.com> wrote:

> diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
> index c7a54c3..8dd225e 100644
> --- a/drivers/net/sfc/efsys.h
> +++ b/drivers/net/sfc/efsys.h
> @@ -26,6 +26,7 @@
>  #include <rte_io.h>
>  
>  #include "sfc_debug.h"
> +#include "sfc_log.h"
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -721,7 +722,7 @@ typedef uint64_t	efsys_stat_t;
>  #define EFSYS_ERR(_esip, _code, _dword0, _dword1)			\
>  	do {								\
>  		(void)(_esip);						\
> -		RTE_LOG(ERR, PMD, "FATAL ERROR #%u (0x%08x%08x)\n",	\
> +		SFC_GENERIC_LOG(ERR, "FATAL ERROR #%u (0x%08x%08x)",	\
>  			(_code), (_dword0), (_dword1));			\
>  		_NOTE(CONSTANTCONDITION);				\
>  	} while (B_FALSE)

Off topic, but why is this header file having C++ wrapper?
It is driver private, and driver is always built with C.

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Andrew Rybchenko]

On 01/25/2018 09:42 PM, Stephen Hemminger wrote:
> On Thu, 25 Jan 2018 17:00:43 +0000
> Andrew Rybchenko <arybchenko@solarflare.com> wrote:
>
>> diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
>> index c7a54c3..8dd225e 100644
>> --- a/drivers/net/sfc/efsys.h
>> +++ b/drivers/net/sfc/efsys.h
>> @@ -26,6 +26,7 @@
>>   #include <rte_io.h>
>>   
>>   #include "sfc_debug.h"
>> +#include "sfc_log.h"
>>   
>>   #ifdef __cplusplus
>>   extern "C" {
>> @@ -721,7 +722,7 @@ typedef uint64_t	efsys_stat_t;
>>   #define EFSYS_ERR(_esip, _code, _dword0, _dword1)			\
>>   	do {								\
>>   		(void)(_esip);						\
>> -		RTE_LOG(ERR, PMD, "FATAL ERROR #%u (0x%08x%08x)\n",	\
>> +		SFC_GENERIC_LOG(ERR, "FATAL ERROR #%u (0x%08x%08x)",	\
>>   			(_code), (_dword0), (_dword1));			\
>>   		_NOTE(CONSTANTCONDITION);				\
>>   	} while (B_FALSE)
> Off topic, but why is this header file having C++ wrapper?
> It is driver private, and driver is always built with C.

In this particular case it is just few lines which, as I understand,
never hurt. So, it is better to have and not think about it.
I have no strong opinion on it.

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Ferruh Yigit]

On 1/25/2018 5:00 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>

<...>

> @@ -2082,3 +2084,14 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
>  	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
>  	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
>  	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
> +
> +RTE_INIT(sfc_driver_register_logtype);
> +static void
> +sfc_driver_register_logtype(void)
> +{
> +	int ret;
> +
> +	ret = rte_log_register_type_and_pick_level(SFC_LOGTYPE_PREFIX "driver",
> +						   RTE_LOG_NOTICE);

No benefit of using rte_log_register_type_and_pick_level() here, in this stage
"opt_loglevel_list" will be empty and this will be same as rte_log_register()

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Andrew Rybchenko]

On 03/05/2018 05:59 PM, Ferruh Yigit wrote:
> On 1/25/2018 5:00 PM, Andrew Rybchenko wrote:
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>
>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
> <...>
>
>> @@ -2082,3 +2084,14 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
>>   	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
>>   	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
>>   	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
>> +
>> +RTE_INIT(sfc_driver_register_logtype);
>> +static void
>> +sfc_driver_register_logtype(void)
>> +{
>> +	int ret;
>> +
>> +	ret = rte_log_register_type_and_pick_level(SFC_LOGTYPE_PREFIX "driver",
>> +						   RTE_LOG_NOTICE);
> No benefit of using rte_log_register_type_and_pick_level() here, in this stage
> "opt_loglevel_list" will be empty and this will be same as rte_log_register()

That's true except "uniform approach is good". I.e. simply use
rte_log_register_type_and_pick_level() everywhere to make it safe against
code movements.
In fact it was raised during internal review and we kept as you can see it.

Other option is to avoid usage of constructor here at all and move it to 
probe.
Yes, it will be tried many times, but there is no harm if it is already 
registered.

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Andrew Rybchenko]

On 03/06/2018 05:45 PM, Andrew Rybchenko wrote:
> On 03/05/2018 05:59 PM, Ferruh Yigit wrote:
>> On 1/25/2018 5:00 PM, Andrew Rybchenko wrote:
>>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>>
>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> Reviewed-by: Andy Moreton <amoreton@solarflare.com>
>> <...>
>>
>>> @@ -2082,3 +2084,14 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
>>>       SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
>>>       SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
>>>       SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
>>> +
>>> +RTE_INIT(sfc_driver_register_logtype);
>>> +static void
>>> +sfc_driver_register_logtype(void)
>>> +{
>>> +    int ret;
>>> +
>>> +    ret = rte_log_register_type_and_pick_level(SFC_LOGTYPE_PREFIX 
>>> "driver",
>>> +                           RTE_LOG_NOTICE);
>> No benefit of using rte_log_register_type_and_pick_level() here, in 
>> this stage
>> "opt_loglevel_list" will be empty and this will be same as 
>> rte_log_register()
>
> That's true except "uniform approach is good". I.e. simply use
> rte_log_register_type_and_pick_level() everywhere to make it safe against
> code movements.
> In fact it was raised during internal review and we kept as you can 
> see it.
>
> Other option is to avoid usage of constructor here at all and move it 
> to probe.
> Yes, it will be tried many times, but there is no harm if it is 
> already registered.

In fact it could be really required if dynamic library is used and it is
pulled later using dlopen() - don't know if there are any restrictions in
DPDK which prevent it.

Re: [dpdk-dev] [PATCH 2/6] net/sfc: add support for driver-wide dynamic logging

[Ferruh Yigit]

On 3/6/2018 2:56 PM, Andrew Rybchenko wrote:
> 
> In fact it could be really required if dynamic library is used and it is
> pulled later using dlopen() - don't know if there are any restrictions in
> DPDK which prevent it.

That function has constructor attribute, not sure how it works for that case.
I am good as long as this is not missed but decided to implement this way.

[dpdk-dev] [PATCH 3/6] net/sfc: add support for per-port dynamic logging

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  |  6 ++++++
 drivers/net/sfc/sfc.c        | 32 ++++++++++++++++++++++++++++++++
 drivers/net/sfc/sfc.h        |  5 +++++
 drivers/net/sfc/sfc_ethdev.c |  3 +++
 drivers/net/sfc/sfc_log.h    | 41 ++++++++++++++++++++++++++++++++---------
 5 files changed, 78 insertions(+), 9 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 4d89cfd..8d5eb30 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -324,3 +324,9 @@ SFC EFX PMD provides the following log types available for control:
 - ``pmd.net.sfc.driver`` (default level is **6** - ``RTE_LOG_NOTICE``)
 
   Affects driver-wide messages unrelated to any particular devices.
+
+- ``pmd.net.sfc.main`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Matches a subset of per-port log types registered during runtime.
+  A full name for a particular type may be obtained by appending a
+  dot and a PCI device identifier (``XXXX:XX:XX.X``) to the prefix.
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index ac5fdca..d31b0b1 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -843,3 +843,35 @@ sfc_unprobe(struct sfc_adapter *sa)
 	sfc_flow_fini(sa);
 	sa->state = SFC_ADAPTER_UNINITIALIZED;
 }
+
+uint32_t
+sfc_register_logtype(struct sfc_adapter *sa, const char *lt_prefix_str,
+		     uint32_t ll_default)
+{
+	size_t lt_prefix_str_size = strlen(lt_prefix_str);
+	size_t lt_str_size_max;
+	char *lt_str = NULL;
+	int ret;
+
+	if (SIZE_MAX - PCI_PRI_STR_SIZE - 1 > lt_prefix_str_size) {
+		++lt_prefix_str_size; /* Reserve space for prefix separator */
+		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
+	} else {
+		return RTE_LOGTYPE_PMD;
+	}
+
+	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
+	if (lt_str == NULL)
+		return RTE_LOGTYPE_PMD;
+
+	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
+	lt_str[lt_prefix_str_size - 1] = '.';
+	rte_pci_device_name(&sa->pci_addr, lt_str + lt_prefix_str_size,
+			    lt_str_size_max - lt_prefix_str_size);
+	lt_str[lt_str_size_max - 1] = '\0';
+
+	ret = rte_log_register_type_and_pick_level(lt_str, ll_default);
+	rte_free(lt_str);
+
+	return (ret < 0) ? RTE_LOGTYPE_PMD : ret;
+}
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 7557534..768578e 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -170,6 +170,7 @@ struct sfc_adapter {
 	uint16_t			port_id;
 	struct rte_eth_dev		*eth_dev;
 	struct rte_kvargs		*kvargs;
+	uint32_t			logtype_main;
 	bool				debug_init;
 	int				socket_id;
 	efsys_bar_t			mem_bar;
@@ -302,6 +303,10 @@ int sfc_dma_alloc(const struct sfc_adapter *sa, const char *name, uint16_t id,
 		  size_t len, int socket_id, efsys_mem_t *esmp);
 void sfc_dma_free(const struct sfc_adapter *sa, efsys_mem_t *esmp);
 
+uint32_t sfc_register_logtype(struct sfc_adapter *sa,
+			      const char *lt_prefix_str,
+			      uint32_t ll_default);
+
 int sfc_probe(struct sfc_adapter *sa);
 void sfc_unprobe(struct sfc_adapter *sa);
 int sfc_attach(struct sfc_adapter *sa);
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f167120..a244d27 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1937,6 +1937,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	/* Copy PCI device info to the dev->data */
 	rte_eth_copy_pci_info(dev, pci_dev);
 
+	sa->logtype_main = sfc_register_logtype(sa, SFC_LOGTYPE_MAIN_STR,
+						RTE_LOG_NOTICE);
+
 	rc = sfc_kvargs_parse(sa);
 	if (rc != 0)
 		goto fail_kvargs_parse;
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index ecafffc..ec903db 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -22,14 +22,17 @@ extern uint32_t sfc_logtype_driver;
 		RTE_FMT("PMD: " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
 			RTE_FMT_TAIL(__VA_ARGS__ ,)))
 
+/** Name prefix for the per-device log type used to report basic information */
+#define SFC_LOGTYPE_MAIN_STR	SFC_LOGTYPE_PREFIX "main"
+
 /* Log PMD message, automatically add prefix and \n */
-#define SFC_LOG(sa, level, ...) \
+#define SFC_LOG(sa, level, type, ...) \
 	do {								\
 		const struct sfc_adapter *__sa = (sa);			\
 									\
-		RTE_LOG(level, PMD,					\
-			RTE_FMT("sfc_efx " PCI_PRI_FMT " #%" PRIu8 ": "	\
-				RTE_FMT_HEAD(__VA_ARGS__,) "\n",	\
+		rte_log(level, type,					\
+			RTE_FMT("PMD: sfc_efx " PCI_PRI_FMT " #%" PRIu8	\
+				": " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
 				__sa->pci_addr.domain,			\
 				__sa->pci_addr.bus,			\
 				__sa->pci_addr.devid,			\
@@ -39,23 +42,43 @@ extern uint32_t sfc_logtype_driver;
 	} while (0)
 
 #define sfc_err(sa, ...) \
-	SFC_LOG(sa, ERR, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_ERR, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_warn(sa, ...) \
-	SFC_LOG(sa, WARNING, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_WARNING, _sa->logtype_main,	\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_notice(sa, ...) \
-	SFC_LOG(sa, NOTICE, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_NOTICE, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_info(sa, ...) \
-	SFC_LOG(sa, INFO, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_log_init(sa, ...) \
 	do {								\
 		const struct sfc_adapter *_sa = (sa);			\
 									\
 		if (_sa->debug_init)					\
-			SFC_LOG(_sa, INFO,				\
+			SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,	\
 				RTE_FMT("%s(): "			\
 					RTE_FMT_HEAD(__VA_ARGS__,),	\
 					__func__,			\
-- 
2.7.4

Re: [dpdk-dev] [PATCH 3/6] net/sfc: add support for per-port dynamic logging

[Ferruh Yigit]

On 1/25/2018 5:00 PM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>

<...>

> +uint32_t
> +sfc_register_logtype(struct sfc_adapter *sa, const char *lt_prefix_str,
> +		     uint32_t ll_default)
> +{
> +	size_t lt_prefix_str_size = strlen(lt_prefix_str);
> +	size_t lt_str_size_max;
> +	char *lt_str = NULL;
> +	int ret;
> +
> +	if (SIZE_MAX - PCI_PRI_STR_SIZE - 1 > lt_prefix_str_size) {
> +		++lt_prefix_str_size; /* Reserve space for prefix separator */
> +		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
> +	} else {
> +		return RTE_LOGTYPE_PMD;
> +	}
> +
> +	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
> +	if (lt_str == NULL)
> +		return RTE_LOGTYPE_PMD;
> +
> +	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
> +	lt_str[lt_prefix_str_size - 1] = '.';
> +	rte_pci_device_name(&sa->pci_addr, lt_str + lt_prefix_str_size,
> +			    lt_str_size_max - lt_prefix_str_size);


Not able to find "rte_pci_device_name()" in shared library build [1].
Need to link with rte_pci library [2].

[1]
sfc.o: In function `sfc_register_logtype':
.../dpdk/drivers/net/sfc/sfc.c:(.text+0x1a52): undefined reference to
`rte_pci_device_name'
clang-4.0: error: linker command failed with exit code 1 (use -v to see invocation)

[2]
  --- a/drivers/net/sfc/Makefile
  +++ b/drivers/net/sfc/Makefile
  @@ -52,3 +52,3 @@ LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
   LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
  -LDLIBS += -lrte_bus_pci
  +LDLIBS += -lrte_bus_pci -lrte_pci

<...>

[dpdk-dev] [PATCH 4/6] net/sfc: prepare to merge init logs with main log type

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Conversion to dynamic logging done in the previous patches
makes it possible to simplify internal controls for init
logging. This patch allows to prepare for such a change.
It makes init-unrelated messages use NOTICE level so that
the following patch will be able to convert init logging
to use INFO level and remain silent by default.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 drivers/net/sfc/sfc.c        |  6 +++---
 drivers/net/sfc/sfc_ethdev.c | 10 +++++-----
 drivers/net/sfc/sfc_intr.c   |  6 +++---
 drivers/net/sfc/sfc_mcdi.c   |  6 +++---
 drivers/net/sfc/sfc_rx.c     |  2 +-
 drivers/net/sfc/sfc_tx.c     |  2 +-
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index d31b0b1..2381505 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -344,7 +344,7 @@ sfc_start(struct sfc_adapter *sa)
 	case SFC_ADAPTER_CONFIGURED:
 		break;
 	case SFC_ADAPTER_STARTED:
-		sfc_info(sa, "already started");
+		sfc_notice(sa, "already started");
 		return 0;
 	default:
 		rc = EINVAL;
@@ -383,7 +383,7 @@ sfc_stop(struct sfc_adapter *sa)
 	case SFC_ADAPTER_STARTED:
 		break;
 	case SFC_ADAPTER_CONFIGURED:
-		sfc_info(sa, "already stopped");
+		sfc_notice(sa, "already stopped");
 		return;
 	default:
 		sfc_err(sa, "stop in unexpected state %u", sa->state);
@@ -454,7 +454,7 @@ sfc_schedule_restart(struct sfc_adapter *sa)
 	else if (rc != 0)
 		sfc_err(sa, "cannot arm restart alarm (rc=%d)", rc);
 	else
-		sfc_info(sa, "restart scheduled");
+		sfc_notice(sa, "restart scheduled");
 }
 
 int
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index a244d27..83175e6 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -272,7 +272,7 @@ sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	}
 
 	if (old_link.link_status != current_link.link_status)
-		sfc_info(sa, "Link status is %s",
+		sfc_notice(sa, "Link status is %s",
 			 current_link.link_status ? "UP" : "DOWN");
 
 	return old_link.link_status == current_link.link_status ? 0 : -1;
@@ -945,8 +945,8 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	}
 
 	if (sa->state != SFC_ADAPTER_STARTED) {
-		sfc_info(sa, "the port is not started");
-		sfc_info(sa, "the new MAC address will be set on port start");
+		sfc_notice(sa, "the port is not started");
+		sfc_notice(sa, "the new MAC address will be set on port start");
 
 		goto unlock;
 	}
@@ -1751,7 +1751,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev)
 		goto fail_dp_rx_name;
 	}
 
-	sfc_info(sa, "use %s Rx datapath", sa->dp_rx_name);
+	sfc_notice(sa, "use %s Rx datapath", sa->dp_rx_name);
 
 	dev->rx_pkt_burst = sa->dp_rx->pkt_burst;
 
@@ -1790,7 +1790,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev)
 		goto fail_dp_tx_name;
 	}
 
-	sfc_info(sa, "use %s Tx datapath", sa->dp_tx_name);
+	sfc_notice(sa, "use %s Tx datapath", sa->dp_tx_name);
 
 	dev->tx_pkt_burst = sa->dp_tx->pkt_burst;
 
diff --git a/drivers/net/sfc/sfc_intr.c b/drivers/net/sfc/sfc_intr.c
index d6c8492..fbdc7ee 100644
--- a/drivers/net/sfc/sfc_intr.c
+++ b/drivers/net/sfc/sfc_intr.c
@@ -86,7 +86,7 @@ sfc_intr_line_handler(void *cb_arg)
 
 exit:
 	if (lsc_seq != sa->port.lsc_seq) {
-		sfc_info(sa, "link status change event: link %s",
+		sfc_notice(sa, "link status change event: link %s",
 			 sa->eth_dev->data->dev_link.link_status ?
 			 "UP" : "DOWN");
 		_rte_eth_dev_callback_process(sa->eth_dev,
@@ -130,7 +130,7 @@ sfc_intr_message_handler(void *cb_arg)
 
 exit:
 	if (lsc_seq != sa->port.lsc_seq) {
-		sfc_info(sa, "link status change event");
+		sfc_notice(sa, "link status change event");
 		_rte_eth_dev_callback_process(sa->eth_dev,
 					      RTE_ETH_EVENT_INTR_LSC,
 					      NULL);
@@ -251,7 +251,7 @@ sfc_intr_configure(struct sfc_adapter *sa)
 	intr->handler = NULL;
 	intr->lsc_intr = (sa->eth_dev->data->dev_conf.intr_conf.lsc != 0);
 	if (!intr->lsc_intr) {
-		sfc_info(sa, "LSC tracking using interrupts is disabled");
+		sfc_notice(sa, "LSC tracking using interrupts is disabled");
 		goto done;
 	}
 
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 9d92b8c..895c2f5 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -176,7 +176,7 @@ sfc_mcdi_do_log(const struct sfc_adapter *sa,
 			 * at the end which is required by netlogdecode.
 			 */
 			buffer[position] = '\0';
-			sfc_info(sa, "%s \\", buffer);
+			sfc_notice(sa, "%s \\", buffer);
 			/* Preserve prefix for the next log message */
 			position = pfxsize;
 		}
@@ -201,7 +201,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	if (!sa->mcdi.logging)
 		return;
 
-	/* The format including prefix added by sfc_info() is the format
+	/* The format including prefix added by sfc_notice() is the format
 	 * consumed by the Solarflare netlogdecode tool.
 	 */
 	pfxsize = snprintf(buffer, sizeof(buffer), "MCDI RPC %s:",
@@ -212,7 +212,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	start = sfc_mcdi_do_log(sa, buffer, data, data_size, pfxsize, start);
 	if (start != pfxsize) {
 		buffer[start] = '\0';
-		sfc_info(sa, "%s", buffer);
+		sfc_notice(sa, "%s", buffer);
 	}
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index abc53fb..f7aaab0 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -608,7 +608,7 @@ sfc_rx_qflush(struct sfc_adapter *sa, unsigned int sw_index)
 			sfc_err(sa, "RxQ %u flush failed", sw_index);
 
 		if (rxq->state & SFC_RXQ_FLUSHED)
-			sfc_info(sa, "RxQ %u flushed", sw_index);
+			sfc_notice(sa, "RxQ %u flushed", sw_index);
 	}
 
 	sa->dp_rx->qpurge(rxq->dp);
diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c
index 757b03b..0035c5c 100644
--- a/drivers/net/sfc/sfc_tx.c
+++ b/drivers/net/sfc/sfc_tx.c
@@ -606,7 +606,7 @@ sfc_tx_qstop(struct sfc_adapter *sa, unsigned int sw_index)
 			sfc_err(sa, "TxQ %u flush timed out", sw_index);
 
 		if (txq->state & SFC_TXQ_FLUSHED)
-			sfc_info(sa, "TxQ %u flushed", sw_index);
+			sfc_notice(sa, "TxQ %u flushed", sw_index);
 	}
 
 	sa->dp_tx->qreap(txq->dp);
-- 
2.7.4

[dpdk-dev] [PATCH 5/6] net/sfc: remove dedicated init log parameter

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

The previous patches in the set convert per-port
logging to use NOTICE level and make this level default.
This provides the possibility to remove the dedicated
toggle for init-related messages and merge init logging
with the main log type. In order to keep these logs silent
by default, INFO level should be used.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  |  4 ----
 drivers/net/sfc/sfc.h        |  1 -
 drivers/net/sfc/sfc_ethdev.c |  9 +--------
 drivers/net/sfc/sfc_kvargs.c |  1 -
 drivers/net/sfc/sfc_kvargs.h |  2 --
 drivers/net/sfc/sfc_log.h    | 11 +++++------
 6 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 8d5eb30..0d02f96 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -287,10 +287,6 @@ boolean parameters value.
   **auto** allows NIC firmware to make a choice based on
   installed licences and firmware variant configured using **sfboot**.
 
-- ``debug_init`` [bool] (default **n**)
-
-  Enable extra logging during device initialization and startup.
-
 - ``mcdi_logging`` [bool] (default **n**)
 
   Enable extra logging of the communication with the NIC's management CPU.
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 768578e..378c675 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -171,7 +171,6 @@ struct sfc_adapter {
 	struct rte_eth_dev		*eth_dev;
 	struct rte_kvargs		*kvargs;
 	uint32_t			logtype_main;
-	bool				debug_init;
 	int				socket_id;
 	efsys_bar_t			mem_bar;
 	efx_family_t			family;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 83175e6..f418715 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1944,11 +1944,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	if (rc != 0)
 		goto fail_kvargs_parse;
 
-	rc = sfc_kvargs_process(sa, SFC_KVARG_DEBUG_INIT,
-				sfc_kvarg_bool_handler, &sa->debug_init);
-	if (rc != 0)
-		goto fail_kvarg_debug_init;
-
 	sfc_log_init(sa, "entry");
 
 	dev->data->mac_addrs = rte_zmalloc("sfc", ETHER_ADDR_LEN, 0);
@@ -2002,7 +1997,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	dev->data->mac_addrs = NULL;
 
 fail_mac_addrs:
-fail_kvarg_debug_init:
 	sfc_kvargs_cleanup(sa);
 
 fail_kvargs_parse:
@@ -2085,8 +2079,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_TX_DATAPATH "=" SFC_KVARG_VALUES_TX_DATAPATH " "
 	SFC_KVARG_PERF_PROFILE "=" SFC_KVARG_VALUES_PERF_PROFILE " "
 	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
-	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
-	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
+	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL);
 
 RTE_INIT(sfc_driver_register_logtype);
 static void
diff --git a/drivers/net/sfc/sfc_kvargs.c b/drivers/net/sfc/sfc_kvargs.c
index 8f55e99..860adeb 100644
--- a/drivers/net/sfc/sfc_kvargs.c
+++ b/drivers/net/sfc/sfc_kvargs.c
@@ -23,7 +23,6 @@ sfc_kvargs_parse(struct sfc_adapter *sa)
 	struct rte_devargs *devargs = eth_dev->device->devargs;
 	const char **params = (const char *[]){
 		SFC_KVARG_STATS_UPDATE_PERIOD_MS,
-		SFC_KVARG_DEBUG_INIT,
 		SFC_KVARG_MCDI_LOGGING,
 		SFC_KVARG_PERF_PROFILE,
 		SFC_KVARG_RX_DATAPATH,
diff --git a/drivers/net/sfc/sfc_kvargs.h b/drivers/net/sfc/sfc_kvargs.h
index e7044ca..2c7aaaf 100644
--- a/drivers/net/sfc/sfc_kvargs.h
+++ b/drivers/net/sfc/sfc_kvargs.h
@@ -18,8 +18,6 @@ extern "C" {
 
 #define SFC_KVARG_VALUES_BOOL		"[1|y|yes|on|0|n|no|off]"
 
-#define SFC_KVARG_DEBUG_INIT		"debug_init"
-
 #define SFC_KVARG_MCDI_LOGGING		"mcdi_logging"
 
 #define SFC_KVARG_PERF_PROFILE		"perf_profile"
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index ec903db..6daab1c 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -77,12 +77,11 @@ extern uint32_t sfc_logtype_driver;
 	do {								\
 		const struct sfc_adapter *_sa = (sa);			\
 									\
-		if (_sa->debug_init)					\
-			SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,	\
-				RTE_FMT("%s(): "			\
-					RTE_FMT_HEAD(__VA_ARGS__,),	\
-					__func__,			\
-					RTE_FMT_TAIL(__VA_ARGS__,)));	\
+		SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,		\
+			RTE_FMT("%s(): "				\
+				RTE_FMT_HEAD(__VA_ARGS__ ,),		\
+				__func__,				\
+				RTE_FMT_TAIL(__VA_ARGS__ ,)));		\
 	} while (0)
 
 #endif /* _SFC_LOG_H_ */
-- 
2.7.4

[dpdk-dev] [PATCH 6/6] net/sfc: add dynamic log level for MCDI messages

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  | 12 ++++++------
 drivers/net/sfc/sfc.h        |  2 +-
 drivers/net/sfc/sfc_ethdev.c |  3 +--
 drivers/net/sfc/sfc_kvargs.c |  1 -
 drivers/net/sfc/sfc_kvargs.h |  2 --
 drivers/net/sfc/sfc_log.h    | 15 +++++++++++++++
 drivers/net/sfc/sfc_mcdi.c   | 25 +++++++++++++------------
 7 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 0d02f96..fcda569 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -287,12 +287,6 @@ boolean parameters value.
   **auto** allows NIC firmware to make a choice based on
   installed licences and firmware variant configured using **sfboot**.
 
-- ``mcdi_logging`` [bool] (default **n**)
-
-  Enable extra logging of the communication with the NIC's management CPU.
-  The logging is done using RTE_LOG() with INFO level and PMD type.
-  The format is consumed by the Solarflare netlogdecode cross-platform tool.
-
 - ``stats_update_period_ms`` [long] (default **1000**)
 
   Adjust period in milliseconds to update port hardware statistics.
@@ -326,3 +320,9 @@ SFC EFX PMD provides the following log types available for control:
   Matches a subset of per-port log types registered during runtime.
   A full name for a particular type may be obtained by appending a
   dot and a PCI device identifier (``XXXX:XX:XX.X``) to the prefix.
+
+- ``pmd.net.sfc.mcdi`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Extra logging of the communication with the NIC's management CPU.
+  The format of the log is consumed by the Solarflare netlogdecode
+  cross-platform tool. May be managed per-port, as explained above.
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 378c675..65a4df2 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -104,7 +104,7 @@ struct sfc_mcdi {
 	efsys_mem_t			mem;
 	enum sfc_mcdi_state		state;
 	efx_mcdi_transport_t		transport;
-	bool				logging;
+	uint32_t			logtype;
 	uint32_t			proxy_handle;
 	efx_rc_t			proxy_result;
 };
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f418715..ffe1c45 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -2078,8 +2078,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_RX_DATAPATH "=" SFC_KVARG_VALUES_RX_DATAPATH " "
 	SFC_KVARG_TX_DATAPATH "=" SFC_KVARG_VALUES_TX_DATAPATH " "
 	SFC_KVARG_PERF_PROFILE "=" SFC_KVARG_VALUES_PERF_PROFILE " "
-	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
-	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL);
+	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long>");
 
 RTE_INIT(sfc_driver_register_logtype);
 static void
diff --git a/drivers/net/sfc/sfc_kvargs.c b/drivers/net/sfc/sfc_kvargs.c
index 860adeb..9c08bff 100644
--- a/drivers/net/sfc/sfc_kvargs.c
+++ b/drivers/net/sfc/sfc_kvargs.c
@@ -23,7 +23,6 @@ sfc_kvargs_parse(struct sfc_adapter *sa)
 	struct rte_devargs *devargs = eth_dev->device->devargs;
 	const char **params = (const char *[]){
 		SFC_KVARG_STATS_UPDATE_PERIOD_MS,
-		SFC_KVARG_MCDI_LOGGING,
 		SFC_KVARG_PERF_PROFILE,
 		SFC_KVARG_RX_DATAPATH,
 		SFC_KVARG_TX_DATAPATH,
diff --git a/drivers/net/sfc/sfc_kvargs.h b/drivers/net/sfc/sfc_kvargs.h
index 2c7aaaf..a84dc6b 100644
--- a/drivers/net/sfc/sfc_kvargs.h
+++ b/drivers/net/sfc/sfc_kvargs.h
@@ -18,8 +18,6 @@ extern "C" {
 
 #define SFC_KVARG_VALUES_BOOL		"[1|y|yes|on|0|n|no|off]"
 
-#define SFC_KVARG_MCDI_LOGGING		"mcdi_logging"
-
 #define SFC_KVARG_PERF_PROFILE		"perf_profile"
 
 #define SFC_KVARG_PERF_PROFILE_AUTO		"auto"
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index 6daab1c..d6f3435 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -25,6 +25,12 @@ extern uint32_t sfc_logtype_driver;
 /** Name prefix for the per-device log type used to report basic information */
 #define SFC_LOGTYPE_MAIN_STR	SFC_LOGTYPE_PREFIX "main"
 
+/** Device MCDI log type name prefix */
+#define SFC_LOGTYPE_MCDI_STR	SFC_LOGTYPE_PREFIX "mcdi"
+
+/** Level value used by MCDI log statements */
+#define SFC_LOG_LEVEL_MCDI	RTE_LOG_INFO
+
 /* Log PMD message, automatically add prefix and \n */
 #define SFC_LOG(sa, level, type, ...) \
 	do {								\
@@ -84,4 +90,13 @@ extern uint32_t sfc_logtype_driver;
 				RTE_FMT_TAIL(__VA_ARGS__ ,)));		\
 	} while (0)
 
+#define sfc_log_mcdi(sa, ...) \
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, SFC_LOG_LEVEL_MCDI, _sa->mcdi.logtype,	\
+			__VA_ARGS__);					\
+	} while (0)
+
+
 #endif /* _SFC_LOG_H_ */
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 895c2f5..007506b 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -15,7 +15,6 @@
 
 #include "sfc.h"
 #include "sfc_log.h"
-#include "sfc_kvargs.h"
 #include "sfc_ev.h"
 
 #define SFC_MCDI_POLL_INTERVAL_MIN_US	10		/* 10us in 1us units */
@@ -176,7 +175,7 @@ sfc_mcdi_do_log(const struct sfc_adapter *sa,
 			 * at the end which is required by netlogdecode.
 			 */
 			buffer[position] = '\0';
-			sfc_notice(sa, "%s \\", buffer);
+			sfc_log_mcdi(sa, "%s \\", buffer);
 			/* Preserve prefix for the next log message */
 			position = pfxsize;
 		}
@@ -198,10 +197,17 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	size_t pfxsize;
 	size_t start;
 
-	if (!sa->mcdi.logging)
+	/*
+	 * Unlike the other cases, MCDI logging implies more onerous work
+	 * needed to produce a message. If the dynamic log level prevents
+	 * the end result from being printed, the CPU time will be wasted.
+	 *
+	 * To avoid wasting time, the actual level is examined in advance.
+	 */
+	if (rte_log_get_level(sa->mcdi.logtype) < (int)SFC_LOG_LEVEL_MCDI)
 		return;
 
-	/* The format including prefix added by sfc_notice() is the format
+	/* The format including prefix added by sfc_log_mcdi() is the format
 	 * consumed by the Solarflare netlogdecode tool.
 	 */
 	pfxsize = snprintf(buffer, sizeof(buffer), "MCDI RPC %s:",
@@ -212,7 +218,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	start = sfc_mcdi_do_log(sa, buffer, data, data_size, pfxsize, start);
 	if (start != pfxsize) {
 		buffer[start] = '\0';
-		sfc_notice(sa, "%s", buffer);
+		sfc_log_mcdi(sa, "%s", buffer);
 	}
 }
 
@@ -250,11 +256,8 @@ sfc_mcdi_init(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_dma_alloc;
 
-	/* Convert negative error to positive used in the driver */
-	rc = sfc_kvargs_process(sa, SFC_KVARG_MCDI_LOGGING,
-				sfc_kvarg_bool_handler, &mcdi->logging);
-	if (rc != 0)
-		goto fail_kvargs_process;
+	mcdi->logtype = sfc_register_logtype(sa, SFC_LOGTYPE_MCDI_STR,
+					     RTE_LOG_NOTICE);
 
 	emtp = &mcdi->transport;
 	emtp->emt_context = sa;
@@ -274,8 +277,6 @@ sfc_mcdi_init(struct sfc_adapter *sa)
 
 fail_mcdi_init:
 	memset(emtp, 0, sizeof(*emtp));
-
-fail_kvargs_process:
 	sfc_dma_free(sa, &mcdi->mem);
 
 fail_dma_alloc:
-- 
2.7.4

Re: [dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[Thomas Monjalon]

25/01/2018 18:00, Andrew Rybchenko:
> Unfortunately we're a bit late with dynamic logging implementation.
> So, it can wait for 18.05 release cycle if required.
> 
> The series adds EXPERIMENTAL EAL feature which removes dependency
> on EAL arguments processing and log types registration. It stores
> EAL loglevel arguments in the list and adds API function to register
> a new log type and pick up its value from EAL arguments.
> 
> For us it is important since we would like to be able to control
> per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.

Would it be possible to consider EAL patch for 18.05,
and others for 18.02?

Re: [dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[Andrew Rybchenko]

On 01/26/2018 12:38 AM, Thomas Monjalon wrote:
> 25/01/2018 18:00, Andrew Rybchenko:
>> Unfortunately we're a bit late with dynamic logging implementation.
>> So, it can wait for 18.05 release cycle if required.
>>
>> The series adds EXPERIMENTAL EAL feature which removes dependency
>> on EAL arguments processing and log types registration. It stores
>> EAL loglevel arguments in the list and adds API function to register
>> a new log type and pick up its value from EAL arguments.
>>
>> For us it is important since we would like to be able to control
>> per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.
> Would it be possible to consider EAL patch for 18.05,
> and others for 18.02?

Artefacts of the EAL patch are used in the subsequent patches, so
I think it does not make sense to redo subsequent patches without
the EAL one and, then, change it once again on the next release cycle.

Re: [dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[Thomas Monjalon]

26/01/2018 06:59, Andrew Rybchenko:
> On 01/26/2018 12:38 AM, Thomas Monjalon wrote:
> > 25/01/2018 18:00, Andrew Rybchenko:
> >> Unfortunately we're a bit late with dynamic logging implementation.
> >> So, it can wait for 18.05 release cycle if required.
> >>
> >> The series adds EXPERIMENTAL EAL feature which removes dependency
> >> on EAL arguments processing and log types registration. It stores
> >> EAL loglevel arguments in the list and adds API function to register
> >> a new log type and pick up its value from EAL arguments.
> >>
> >> For us it is important since we would like to be able to control
> >> per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.
> > Would it be possible to consider EAL patch for 18.05,
> > and others for 18.02?
> 
> Artefacts of the EAL patch are used in the subsequent patches, so
> I think it does not make sense to redo subsequent patches without
> the EAL one and, then, change it once again on the next release cycle.

OK, no problem.
We can wait 18.05.

[dpdk-dev] [PATCH v2] eal: register log type and pick level from EAL args

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Dynamic log types are registered on RTE_INIT() step.
This allows one to set log levels by EAL options on
application launch. However, this does not allow to
manage log types if they are created during runtime.

EAL does not store log levels and types passed from
the command line. Thus, they cannot be picked later.
This is an obvious flaw since it would be better to
be able to pick levels for dynamic types registered
for runtime-determined facilities such as NIC ports.

This patch provides a mechanism to store log levels
passed from EAL options and adds an API to register
log types and pick levels from the internal storage.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 lib/librte_eal/common/eal_common_log.c     | 36 ++++++++++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 23 ++++++++++++++
 lib/librte_eal/common/include/rte_log.h    | 48 ++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 4 files changed, 108 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 37b2e20..a271926 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -23,6 +23,10 @@ struct rte_logs rte_logs = {
 	.file = NULL,
 };
 
+/** Global list of valid EAL log level options */
+struct rte_eal_opt_loglevel_list opt_loglevel_list =
+	TAILQ_HEAD_INITIALIZER(opt_loglevel_list);
+
 /* Stream to use for logging if rte_logs.file is NULL */
 static FILE *default_log_stream;
 
@@ -186,6 +190,38 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+/* Register an extended log type and try to pick its level from EAL options */
+int __rte_experimental
+rte_log_register_type_and_pick_level(const char *name, uint32_t level_def)
+{
+	struct rte_eal_opt_loglevel *opt_ll;
+	uint32_t level = level_def;
+	int type;
+
+	type = rte_log_register(name);
+	if (type < 0)
+		return type;
+
+	TAILQ_FOREACH(opt_ll, &opt_loglevel_list, next) {
+		regex_t r;
+
+		if (opt_ll->level > RTE_LOG_DEBUG)
+			continue;
+
+		if (regcomp(&r, opt_ll->re_type, 0) != 0)
+			continue;
+
+		if (regexec(&r, name, 0, NULL, 0) == 0)
+			level = opt_ll->level;
+
+		regfree(&r);
+	}
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return type;
+}
+
 struct logtype {
 	uint32_t log_id;
 	const char *logtype;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d2..d29b85f 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -946,6 +946,29 @@ eal_parse_log_level(const char *arg)
 		printf("cannot set log level %s,%lu\n",
 			type, tmp);
 		goto fail;
+	} else {
+		struct rte_eal_opt_loglevel *opt_ll;
+
+		/*
+		 * Save the type (regexp string) and the loglevel
+		 * in the global storage so that it could be used
+		 * to configure dynamic logtypes which are absent
+		 * at the moment of EAL option processing but may
+		 * be registered during runtime.
+		 */
+		opt_ll = malloc(sizeof(*opt_ll));
+		if (opt_ll == NULL)
+			goto fail;
+
+		opt_ll->re_type = strdup(type);
+		if (opt_ll->re_type == NULL) {
+			free(opt_ll);
+			goto fail;
+		}
+
+		opt_ll->level = tmp;
+
+		TAILQ_INSERT_HEAD(&opt_loglevel_list, opt_ll, next);
 	}
 
 	free(str);
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 9029c78..5f4799e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -20,6 +20,7 @@ extern "C" {
 #include <stdint.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <sys/queue.h>
 
 #include <rte_common.h>
 #include <rte_config.h>
@@ -85,6 +86,32 @@ extern struct rte_logs rte_logs;
 #define RTE_LOG_DEBUG    8U  /**< Debug-level messages.             */
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Entry definition for the storage to keep EAL log level options
+ * which are found to have log type regular expressions specified.
+ */
+struct rte_eal_opt_loglevel {
+	/** Next list entry */
+	TAILQ_ENTRY(rte_eal_opt_loglevel) next;
+	/** Regular expression string obtained from the option */
+	char *re_type;
+	/** Log level value obtained from the option */
+	uint32_t level;
+};
+
+TAILQ_HEAD(rte_eal_opt_loglevel_list, rte_eal_opt_loglevel);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Global list of EAL log level options featuring log type expressions
+ */
+extern struct rte_eal_opt_loglevel_list opt_loglevel_list;
+
+/**
  * Change the stream that will be used by the logging system.
  *
  * This can be done at any time. The f argument represents the stream
@@ -195,6 +222,27 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Register a dynamic log type and try to pick its level from EAL options
+ *
+ * rte_log_register() is called inside. If successful, the function tries
+ * to search for matching regexp in the list of EAL log level options and
+ * pick the level from the last matching entry. If nothing can be applied
+ * from the list, the level will be set to the user-defined default value.
+ *
+ * @param name
+ *    Name for the log type to be registered
+ * @param level_def
+ *    Fallback level to be set if the global list has no matching options
+ * @return
+ *    - >=0: the newly registered log type
+ *    - <0: rte_log_register() error value
+ */
+int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def);
+
+/**
  * Dump log information.
  *
  * Dump the global level and the registered log types.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d123602..f331f54 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -221,6 +221,7 @@ EXPERIMENTAL {
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
 	rte_eal_mbuf_user_pool_ops;
+	rte_log_register_type_and_pick_level;
 	rte_mp_action_register;
 	rte_mp_action_unregister;
 	rte_mp_sendmsg;
-- 
2.7.4

Re: [dpdk-dev] [PATCH 0/6] net/sfc: implement dynamic logging

[Ferruh Yigit]

On 1/25/2018 5:00 PM, Andrew Rybchenko wrote:
> Unfortunately we're a bit late with dynamic logging implementation.
> So, it can wait for 18.05 release cycle if required.
> 
> The series adds EXPERIMENTAL EAL feature which removes dependency
> on EAL arguments processing and log types registration. It stores
> EAL loglevel arguments in the list and adds API function to register
> a new log type and pick up its value from EAL arguments.
> 
> For us it is important since we would like to be able to control
> per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.

It is good idea to have device level granularity in logging,
I believe other devices also would like have this, if only there would be an
easy way to apply this capability to all PMDs.

> 
> The series already follows log type names format defined recently.
> 
> Ivan Malov (6):
>   eal: register log type and pick level from EAL args
>   net/sfc: add support for driver-wide dynamic logging
>   net/sfc: add support for per-port dynamic logging
>   net/sfc: prepare to merge init logs with main log type
>   net/sfc: remove dedicated init log parameter
>   net/sfc: add dynamic log level for MCDI messages

<...>

[dpdk-dev] [PATCH v3 0/6] net/sfc: implement dynamic logging

[Andrew Rybchenko]

The series adds EXPERIMENTAL EAL feature which removes dependency
on EAL arguments processing and log types registration. It stores
EAL loglevel arguments in the list and adds API function to register
a new log type and pick up its value from EAL arguments.

For us it is important since we would like to be able to control
per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.

v3:
 - fix build issue reported by Ferruh

Ivan Malov (6):
  eal: register log type and pick level from EAL args
  net/sfc: add support for driver-wide dynamic logging
  net/sfc: add support for per-port dynamic logging
  net/sfc: prepare to merge init logs with main log type
  net/sfc: remove dedicated init log parameter
  net/sfc: add dynamic log level for MCDI messages

 doc/guides/nics/sfc_efx.rst                | 41 ++++++++++++----
 drivers/net/sfc/Makefile                   |  2 +-
 drivers/net/sfc/efsys.h                    |  3 +-
 drivers/net/sfc/sfc.c                      | 38 +++++++++++++--
 drivers/net/sfc/sfc.h                      |  8 +++-
 drivers/net/sfc/sfc_dp.c                   |  5 +-
 drivers/net/sfc/sfc_dp.h                   |  6 ++-
 drivers/net/sfc/sfc_ef10_rx.c              |  4 +-
 drivers/net/sfc/sfc_ethdev.c               | 39 +++++++++------
 drivers/net/sfc/sfc_intr.c                 |  6 +--
 drivers/net/sfc/sfc_kvargs.c               |  2 -
 drivers/net/sfc/sfc_kvargs.h               |  4 --
 drivers/net/sfc/sfc_log.h                  | 77 ++++++++++++++++++++++++------
 drivers/net/sfc/sfc_mcdi.c                 | 25 +++++-----
 drivers/net/sfc/sfc_rx.c                   |  2 +-
 drivers/net/sfc/sfc_tx.c                   |  2 +-
 lib/librte_eal/common/eal_common_log.c     | 36 ++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 23 +++++++++
 lib/librte_eal/common/include/rte_log.h    | 48 +++++++++++++++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 20 files changed, 297 insertions(+), 75 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v3 1/6] eal: register log type and pick level from EAL args

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Dynamic log types are registered on RTE_INIT() step.
This allows one to set log levels by EAL options on
application launch. However, this does not allow to
manage log types if they are created during runtime.

EAL does not store log levels and types passed from
the command line. Thus, they cannot be picked later.
This is an obvious flaw since it would be better to
be able to pick levels for dynamic types registered
for runtime-determined facilities such as NIC ports.

This patch provides a mechanism to store log levels
passed from EAL options and adds an API to register
log types and pick levels from the internal storage.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 lib/librte_eal/common/eal_common_log.c     | 36 ++++++++++++++++++++++
 lib/librte_eal/common/eal_common_options.c | 23 ++++++++++++++
 lib/librte_eal/common/include/rte_log.h    | 48 ++++++++++++++++++++++++++++++
 lib/librte_eal/rte_eal_version.map         |  1 +
 4 files changed, 108 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index 37b2e20..a271926 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -23,6 +23,10 @@ struct rte_logs rte_logs = {
 	.file = NULL,
 };
 
+/** Global list of valid EAL log level options */
+struct rte_eal_opt_loglevel_list opt_loglevel_list =
+	TAILQ_HEAD_INITIALIZER(opt_loglevel_list);
+
 /* Stream to use for logging if rte_logs.file is NULL */
 static FILE *default_log_stream;
 
@@ -186,6 +190,38 @@ rte_log_register(const char *name)
 	return ret;
 }
 
+/* Register an extended log type and try to pick its level from EAL options */
+int __rte_experimental
+rte_log_register_type_and_pick_level(const char *name, uint32_t level_def)
+{
+	struct rte_eal_opt_loglevel *opt_ll;
+	uint32_t level = level_def;
+	int type;
+
+	type = rte_log_register(name);
+	if (type < 0)
+		return type;
+
+	TAILQ_FOREACH(opt_ll, &opt_loglevel_list, next) {
+		regex_t r;
+
+		if (opt_ll->level > RTE_LOG_DEBUG)
+			continue;
+
+		if (regcomp(&r, opt_ll->re_type, 0) != 0)
+			continue;
+
+		if (regexec(&r, name, 0, NULL, 0) == 0)
+			level = opt_ll->level;
+
+		regfree(&r);
+	}
+
+	rte_logs.dynamic_types[type].loglevel = level;
+
+	return type;
+}
+
 struct logtype {
 	uint32_t log_id;
 	const char *logtype;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d2..d29b85f 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -946,6 +946,29 @@ eal_parse_log_level(const char *arg)
 		printf("cannot set log level %s,%lu\n",
 			type, tmp);
 		goto fail;
+	} else {
+		struct rte_eal_opt_loglevel *opt_ll;
+
+		/*
+		 * Save the type (regexp string) and the loglevel
+		 * in the global storage so that it could be used
+		 * to configure dynamic logtypes which are absent
+		 * at the moment of EAL option processing but may
+		 * be registered during runtime.
+		 */
+		opt_ll = malloc(sizeof(*opt_ll));
+		if (opt_ll == NULL)
+			goto fail;
+
+		opt_ll->re_type = strdup(type);
+		if (opt_ll->re_type == NULL) {
+			free(opt_ll);
+			goto fail;
+		}
+
+		opt_ll->level = tmp;
+
+		TAILQ_INSERT_HEAD(&opt_loglevel_list, opt_ll, next);
 	}
 
 	free(str);
diff --git a/lib/librte_eal/common/include/rte_log.h b/lib/librte_eal/common/include/rte_log.h
index 9029c78..5f4799e 100644
--- a/lib/librte_eal/common/include/rte_log.h
+++ b/lib/librte_eal/common/include/rte_log.h
@@ -20,6 +20,7 @@ extern "C" {
 #include <stdint.h>
 #include <stdio.h>
 #include <stdarg.h>
+#include <sys/queue.h>
 
 #include <rte_common.h>
 #include <rte_config.h>
@@ -85,6 +86,32 @@ extern struct rte_logs rte_logs;
 #define RTE_LOG_DEBUG    8U  /**< Debug-level messages.             */
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Entry definition for the storage to keep EAL log level options
+ * which are found to have log type regular expressions specified.
+ */
+struct rte_eal_opt_loglevel {
+	/** Next list entry */
+	TAILQ_ENTRY(rte_eal_opt_loglevel) next;
+	/** Regular expression string obtained from the option */
+	char *re_type;
+	/** Log level value obtained from the option */
+	uint32_t level;
+};
+
+TAILQ_HEAD(rte_eal_opt_loglevel_list, rte_eal_opt_loglevel);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Global list of EAL log level options featuring log type expressions
+ */
+extern struct rte_eal_opt_loglevel_list opt_loglevel_list;
+
+/**
  * Change the stream that will be used by the logging system.
  *
  * This can be done at any time. The f argument represents the stream
@@ -195,6 +222,27 @@ int rte_log_cur_msg_logtype(void);
 int rte_log_register(const char *name);
 
 /**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Register a dynamic log type and try to pick its level from EAL options
+ *
+ * rte_log_register() is called inside. If successful, the function tries
+ * to search for matching regexp in the list of EAL log level options and
+ * pick the level from the last matching entry. If nothing can be applied
+ * from the list, the level will be set to the user-defined default value.
+ *
+ * @param name
+ *    Name for the log type to be registered
+ * @param level_def
+ *    Fallback level to be set if the global list has no matching options
+ * @return
+ *    - >=0: the newly registered log type
+ *    - <0: rte_log_register() error value
+ */
+int rte_log_register_type_and_pick_level(const char *name, uint32_t level_def);
+
+/**
  * Dump log information.
  *
  * Dump the global level and the registered log types.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d123602..f331f54 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -221,6 +221,7 @@ EXPERIMENTAL {
 	rte_eal_hotplug_add;
 	rte_eal_hotplug_remove;
 	rte_eal_mbuf_user_pool_ops;
+	rte_log_register_type_and_pick_level;
 	rte_mp_action_register;
 	rte_mp_action_unregister;
 	rte_mp_sendmsg;
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3 1/6] eal: register log type and pick level from EAL args

[Ferruh Yigit]

On 3/21/2018 11:28 AM, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Dynamic log types are registered on RTE_INIT() step.
> This allows one to set log levels by EAL options on
> application launch. However, this does not allow to
> manage log types if they are created during runtime.
> 
> EAL does not store log levels and types passed from
> the command line. Thus, they cannot be picked later.
> This is an obvious flaw since it would be better to
> be able to pick levels for dynamic types registered
> for runtime-determined facilities such as NIC ports.
> 
> This patch provides a mechanism to store log levels
> passed from EAL options and adds an API to register
> log types and pick levels from the internal storage.
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> Reviewed-by: Andy Moreton <amoreton@solarflare.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v3 2/6] net/sfc: add support for driver-wide dynamic logging

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst   | 19 +++++++++++++++++++
 drivers/net/sfc/efsys.h       |  3 ++-
 drivers/net/sfc/sfc_dp.c      |  5 +++--
 drivers/net/sfc/sfc_dp.h      |  6 ++++--
 drivers/net/sfc/sfc_ef10_rx.c |  4 ++--
 drivers/net/sfc/sfc_ethdev.c  | 13 +++++++++++++
 drivers/net/sfc/sfc_log.h     | 12 ++++++++++++
 7 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 36e98d3..7162ffd 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -339,3 +339,22 @@ boolean parameters value.
   only possible to set an arbitrary value on SFN8xxx provided that
   firmware version is 6.2.1.1033 or higher, otherwise any positive
   value will select a fixed update period of **1000** milliseconds
+
+
+Dynamic Logging Parameters
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+One may leverage EAL option "--log-level" to change default levels
+for the log types supported by the driver. The option is used with
+an argument typically consisting of two parts separated by a comma.
+
+Level value is the last part which takes an integer greater than 0.
+Log type is the former part which may contain a regular expression.
+Depending on the choice of the expression, the given log level may
+be used either for some specific log type or for a subset of types.
+
+SFC EFX PMD provides the following log types available for control:
+
+- ``pmd.net.sfc.driver`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Affects driver-wide messages unrelated to any particular devices.
diff --git a/drivers/net/sfc/efsys.h b/drivers/net/sfc/efsys.h
index f20b667..d5b03a8 100644
--- a/drivers/net/sfc/efsys.h
+++ b/drivers/net/sfc/efsys.h
@@ -26,6 +26,7 @@
 #include <rte_io.h>
 
 #include "sfc_debug.h"
+#include "sfc_log.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -729,7 +730,7 @@ typedef uint64_t	efsys_stat_t;
 #define EFSYS_ERR(_esip, _code, _dword0, _dword1)			\
 	do {								\
 		(void)(_esip);						\
-		RTE_LOG(ERR, PMD, "FATAL ERROR #%u (0x%08x%08x)\n",	\
+		SFC_GENERIC_LOG(ERR, "FATAL ERROR #%u (0x%08x%08x)",	\
 			(_code), (_dword0), (_dword1));			\
 		_NOTE(CONSTANTCONDITION);				\
 	} while (B_FALSE)
diff --git a/drivers/net/sfc/sfc_dp.c b/drivers/net/sfc/sfc_dp.c
index 9a5ca20..b121dc0 100644
--- a/drivers/net/sfc/sfc_dp.c
+++ b/drivers/net/sfc/sfc_dp.c
@@ -14,6 +14,7 @@
 #include <rte_log.h>
 
 #include "sfc_dp.h"
+#include "sfc_log.h"
 
 void
 sfc_dp_queue_init(struct sfc_dp_queue *dpq, uint16_t port_id, uint16_t queue_id,
@@ -63,8 +64,8 @@ int
 sfc_dp_register(struct sfc_dp_list *head, struct sfc_dp *entry)
 {
 	if (sfc_dp_find_by_name(head, entry->type, entry->name) != NULL) {
-		rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD,
-			"sfc %s dapapath '%s' already registered\n",
+		SFC_GENERIC_LOG(ERR,
+			"sfc %s dapapath '%s' already registered",
 			entry->type == SFC_DP_RX ? "Rx" :
 			entry->type == SFC_DP_TX ? "Tx" :
 			"unknown",
diff --git a/drivers/net/sfc/sfc_dp.h b/drivers/net/sfc/sfc_dp.h
index b142532..26e7195 100644
--- a/drivers/net/sfc/sfc_dp.h
+++ b/drivers/net/sfc/sfc_dp.h
@@ -15,6 +15,8 @@
 
 #include <rte_pci.h>
 
+#include "sfc_log.h"
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -58,10 +60,10 @@ void sfc_dp_queue_init(struct sfc_dp_queue *dpq,
 		const struct sfc_dp_queue *_dpq = (dpq);		\
 		const struct rte_pci_addr *_addr = &(_dpq)->pci_addr;	\
 									\
-		RTE_LOG(level, PMD,					\
+		SFC_GENERIC_LOG(level,					\
 			RTE_FMT("%s " PCI_PRI_FMT			\
 				" #%" PRIu16 ".%" PRIu16 ": "		\
-				RTE_FMT_HEAD(__VA_ARGS__,) "\n",	\
+				RTE_FMT_HEAD(__VA_ARGS__ ,),		\
 				dp_name,				\
 				_addr->domain, _addr->bus,		\
 				_addr->devid, _addr->function,		\
diff --git a/drivers/net/sfc/sfc_ef10_rx.c b/drivers/net/sfc/sfc_ef10_rx.c
index f31a7e0..ad98727 100644
--- a/drivers/net/sfc/sfc_ef10_rx.c
+++ b/drivers/net/sfc/sfc_ef10_rx.c
@@ -597,8 +597,8 @@ sfc_ef10_supported_ptypes_get(uint32_t tunnel_encaps)
 	      1u << EFX_TUNNEL_PROTOCOL_NVGRE):
 		return ef10_overlay_ptypes;
 	default:
-		RTE_LOG(ERR, PMD,
-			"Unexpected set of supported tunnel encapsulations: %#x\n",
+		SFC_GENERIC_LOG(ERR,
+			"Unexpected set of supported tunnel encapsulations: %#x",
 			tunnel_encaps);
 		/* FALLTHROUGH */
 	case 0:
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 7d51db9..341fa46 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -27,6 +27,8 @@
 #include "sfc_dp.h"
 #include "sfc_dp_rx.h"
 
+uint32_t sfc_logtype_driver;
+
 static struct sfc_dp_list sfc_dp_head =
 	TAILQ_HEAD_INITIALIZER(sfc_dp_head);
 
@@ -2068,3 +2070,14 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
 	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
 	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
+
+RTE_INIT(sfc_driver_register_logtype);
+static void
+sfc_driver_register_logtype(void)
+{
+	int ret;
+
+	ret = rte_log_register_type_and_pick_level(SFC_LOGTYPE_PREFIX "driver",
+						   RTE_LOG_NOTICE);
+	sfc_logtype_driver = (ret < 0) ? RTE_LOGTYPE_PMD : ret;
+}
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index a18191e..ecafffc 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -10,6 +10,18 @@
 #ifndef _SFC_LOG_H_
 #define _SFC_LOG_H_
 
+/** Generic driver log type */
+extern uint32_t sfc_logtype_driver;
+
+/** Common log type name prefix */
+#define SFC_LOGTYPE_PREFIX	"pmd.net.sfc."
+
+/** Log PMD generic message, add a prefix and a line break */
+#define SFC_GENERIC_LOG(level, ...) \
+	rte_log(RTE_LOG_ ## level, sfc_logtype_driver,			\
+		RTE_FMT("PMD: " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
+			RTE_FMT_TAIL(__VA_ARGS__ ,)))
+
 /* Log PMD message, automatically add prefix and \n */
 #define SFC_LOG(sa, level, ...) \
 	do {								\
-- 
2.7.4

[dpdk-dev] [PATCH v3 3/6] net/sfc: add support for per-port dynamic logging

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  |  6 ++++++
 drivers/net/sfc/Makefile     |  2 +-
 drivers/net/sfc/sfc.c        | 32 ++++++++++++++++++++++++++++++++
 drivers/net/sfc/sfc.h        |  5 +++++
 drivers/net/sfc/sfc_ethdev.c |  3 +++
 drivers/net/sfc/sfc_log.h    | 41 ++++++++++++++++++++++++++++++++---------
 6 files changed, 79 insertions(+), 10 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index 7162ffd..a580d94 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -358,3 +358,9 @@ SFC EFX PMD provides the following log types available for control:
 - ``pmd.net.sfc.driver`` (default level is **6** - ``RTE_LOG_NOTICE``)
 
   Affects driver-wide messages unrelated to any particular devices.
+
+- ``pmd.net.sfc.main`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Matches a subset of per-port log types registered during runtime.
+  A full name for a particular type may be obtained by appending a
+  dot and a PCI device identifier (``XXXX:XX:XX.X``) to the prefix.
diff --git a/drivers/net/sfc/Makefile b/drivers/net/sfc/Makefile
index d2decd0..d3361d1 100644
--- a/drivers/net/sfc/Makefile
+++ b/drivers/net/sfc/Makefile
@@ -50,7 +50,7 @@ CFLAGS_sfc_ef10_tx.o += -wd3656
 endif
 LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring
 LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs
-LDLIBS += -lrte_bus_pci
+LDLIBS += -lrte_bus_pci -lrte_pci
 
 #
 # List of base driver object files for which
diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index 7eb9305..cb3a581 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -835,3 +835,35 @@ sfc_unprobe(struct sfc_adapter *sa)
 	sfc_flow_fini(sa);
 	sa->state = SFC_ADAPTER_UNINITIALIZED;
 }
+
+uint32_t
+sfc_register_logtype(struct sfc_adapter *sa, const char *lt_prefix_str,
+		     uint32_t ll_default)
+{
+	size_t lt_prefix_str_size = strlen(lt_prefix_str);
+	size_t lt_str_size_max;
+	char *lt_str = NULL;
+	int ret;
+
+	if (SIZE_MAX - PCI_PRI_STR_SIZE - 1 > lt_prefix_str_size) {
+		++lt_prefix_str_size; /* Reserve space for prefix separator */
+		lt_str_size_max = lt_prefix_str_size + PCI_PRI_STR_SIZE + 1;
+	} else {
+		return RTE_LOGTYPE_PMD;
+	}
+
+	lt_str = rte_zmalloc("logtype_str", lt_str_size_max, 0);
+	if (lt_str == NULL)
+		return RTE_LOGTYPE_PMD;
+
+	strncpy(lt_str, lt_prefix_str, lt_prefix_str_size);
+	lt_str[lt_prefix_str_size - 1] = '.';
+	rte_pci_device_name(&sa->pci_addr, lt_str + lt_prefix_str_size,
+			    lt_str_size_max - lt_prefix_str_size);
+	lt_str[lt_str_size_max - 1] = '\0';
+
+	ret = rte_log_register_type_and_pick_level(lt_str, ll_default);
+	rte_free(lt_str);
+
+	return (ret < 0) ? RTE_LOGTYPE_PMD : ret;
+}
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 7557534..768578e 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -170,6 +170,7 @@ struct sfc_adapter {
 	uint16_t			port_id;
 	struct rte_eth_dev		*eth_dev;
 	struct rte_kvargs		*kvargs;
+	uint32_t			logtype_main;
 	bool				debug_init;
 	int				socket_id;
 	efsys_bar_t			mem_bar;
@@ -302,6 +303,10 @@ int sfc_dma_alloc(const struct sfc_adapter *sa, const char *name, uint16_t id,
 		  size_t len, int socket_id, efsys_mem_t *esmp);
 void sfc_dma_free(const struct sfc_adapter *sa, efsys_mem_t *esmp);
 
+uint32_t sfc_register_logtype(struct sfc_adapter *sa,
+			      const char *lt_prefix_str,
+			      uint32_t ll_default);
+
 int sfc_probe(struct sfc_adapter *sa);
 void sfc_unprobe(struct sfc_adapter *sa);
 int sfc_attach(struct sfc_adapter *sa);
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 341fa46..15aee22 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1923,6 +1923,9 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	/* Copy PCI device info to the dev->data */
 	rte_eth_copy_pci_info(dev, pci_dev);
 
+	sa->logtype_main = sfc_register_logtype(sa, SFC_LOGTYPE_MAIN_STR,
+						RTE_LOG_NOTICE);
+
 	rc = sfc_kvargs_parse(sa);
 	if (rc != 0)
 		goto fail_kvargs_parse;
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index ecafffc..ec903db 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -22,14 +22,17 @@ extern uint32_t sfc_logtype_driver;
 		RTE_FMT("PMD: " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
 			RTE_FMT_TAIL(__VA_ARGS__ ,)))
 
+/** Name prefix for the per-device log type used to report basic information */
+#define SFC_LOGTYPE_MAIN_STR	SFC_LOGTYPE_PREFIX "main"
+
 /* Log PMD message, automatically add prefix and \n */
-#define SFC_LOG(sa, level, ...) \
+#define SFC_LOG(sa, level, type, ...) \
 	do {								\
 		const struct sfc_adapter *__sa = (sa);			\
 									\
-		RTE_LOG(level, PMD,					\
-			RTE_FMT("sfc_efx " PCI_PRI_FMT " #%" PRIu8 ": "	\
-				RTE_FMT_HEAD(__VA_ARGS__,) "\n",	\
+		rte_log(level, type,					\
+			RTE_FMT("PMD: sfc_efx " PCI_PRI_FMT " #%" PRIu8	\
+				": " RTE_FMT_HEAD(__VA_ARGS__ ,) "\n",	\
 				__sa->pci_addr.domain,			\
 				__sa->pci_addr.bus,			\
 				__sa->pci_addr.devid,			\
@@ -39,23 +42,43 @@ extern uint32_t sfc_logtype_driver;
 	} while (0)
 
 #define sfc_err(sa, ...) \
-	SFC_LOG(sa, ERR, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_ERR, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_warn(sa, ...) \
-	SFC_LOG(sa, WARNING, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_WARNING, _sa->logtype_main,	\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_notice(sa, ...) \
-	SFC_LOG(sa, NOTICE, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_NOTICE, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_info(sa, ...) \
-	SFC_LOG(sa, INFO, __VA_ARGS__)
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,		\
+			__VA_ARGS__);					\
+	} while (0)
 
 #define sfc_log_init(sa, ...) \
 	do {								\
 		const struct sfc_adapter *_sa = (sa);			\
 									\
 		if (_sa->debug_init)					\
-			SFC_LOG(_sa, INFO,				\
+			SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,	\
 				RTE_FMT("%s(): "			\
 					RTE_FMT_HEAD(__VA_ARGS__,),	\
 					__func__,			\
-- 
2.7.4

[dpdk-dev] [PATCH v3 4/6] net/sfc: prepare to merge init logs with main log type

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Conversion to dynamic logging done in the previous patches
makes it possible to simplify internal controls for init
logging. This patch allows to prepare for such a change.
It makes init-unrelated messages use NOTICE level so that
the following patch will be able to convert init logging
to use INFO level and remain silent by default.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 drivers/net/sfc/sfc.c        |  6 +++---
 drivers/net/sfc/sfc_ethdev.c | 13 +++++++------
 drivers/net/sfc/sfc_intr.c   |  6 +++---
 drivers/net/sfc/sfc_mcdi.c   |  6 +++---
 drivers/net/sfc/sfc_rx.c     |  2 +-
 drivers/net/sfc/sfc_tx.c     |  2 +-
 6 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/sfc/sfc.c b/drivers/net/sfc/sfc.c
index cb3a581..681e117 100644
--- a/drivers/net/sfc/sfc.c
+++ b/drivers/net/sfc/sfc.c
@@ -344,7 +344,7 @@ sfc_start(struct sfc_adapter *sa)
 	case SFC_ADAPTER_CONFIGURED:
 		break;
 	case SFC_ADAPTER_STARTED:
-		sfc_info(sa, "already started");
+		sfc_notice(sa, "already started");
 		return 0;
 	default:
 		rc = EINVAL;
@@ -383,7 +383,7 @@ sfc_stop(struct sfc_adapter *sa)
 	case SFC_ADAPTER_STARTED:
 		break;
 	case SFC_ADAPTER_CONFIGURED:
-		sfc_info(sa, "already stopped");
+		sfc_notice(sa, "already stopped");
 		return;
 	default:
 		sfc_err(sa, "stop in unexpected state %u", sa->state);
@@ -454,7 +454,7 @@ sfc_schedule_restart(struct sfc_adapter *sa)
 	else if (rc != 0)
 		sfc_err(sa, "cannot arm restart alarm (rc=%d)", rc);
 	else
-		sfc_info(sa, "restart scheduled");
+		sfc_notice(sa, "restart scheduled");
 }
 
 int
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 15aee22..f27ce63 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -259,8 +259,9 @@ sfc_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 
 	ret = rte_eth_linkstatus_set(dev, &current_link);
 	if (ret == 0)
-		sfc_info(sa, "Link status is %s",
-			 current_link.link_status ? "UP" : "DOWN");
+		sfc_notice(sa, "Link status is %s",
+			   current_link.link_status ? "UP" : "DOWN");
+
 	return ret;
 }
 
@@ -931,8 +932,8 @@ sfc_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 	}
 
 	if (sa->state != SFC_ADAPTER_STARTED) {
-		sfc_info(sa, "the port is not started");
-		sfc_info(sa, "the new MAC address will be set on port start");
+		sfc_notice(sa, "the port is not started");
+		sfc_notice(sa, "the new MAC address will be set on port start");
 
 		goto unlock;
 	}
@@ -1737,7 +1738,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev)
 		goto fail_dp_rx_name;
 	}
 
-	sfc_info(sa, "use %s Rx datapath", sa->dp_rx_name);
+	sfc_notice(sa, "use %s Rx datapath", sa->dp_rx_name);
 
 	dev->rx_pkt_burst = sa->dp_rx->pkt_burst;
 
@@ -1776,7 +1777,7 @@ sfc_eth_dev_set_ops(struct rte_eth_dev *dev)
 		goto fail_dp_tx_name;
 	}
 
-	sfc_info(sa, "use %s Tx datapath", sa->dp_tx_name);
+	sfc_notice(sa, "use %s Tx datapath", sa->dp_tx_name);
 
 	dev->tx_pkt_burst = sa->dp_tx->pkt_burst;
 
diff --git a/drivers/net/sfc/sfc_intr.c b/drivers/net/sfc/sfc_intr.c
index d6c8492..fbdc7ee 100644
--- a/drivers/net/sfc/sfc_intr.c
+++ b/drivers/net/sfc/sfc_intr.c
@@ -86,7 +86,7 @@ sfc_intr_line_handler(void *cb_arg)
 
 exit:
 	if (lsc_seq != sa->port.lsc_seq) {
-		sfc_info(sa, "link status change event: link %s",
+		sfc_notice(sa, "link status change event: link %s",
 			 sa->eth_dev->data->dev_link.link_status ?
 			 "UP" : "DOWN");
 		_rte_eth_dev_callback_process(sa->eth_dev,
@@ -130,7 +130,7 @@ sfc_intr_message_handler(void *cb_arg)
 
 exit:
 	if (lsc_seq != sa->port.lsc_seq) {
-		sfc_info(sa, "link status change event");
+		sfc_notice(sa, "link status change event");
 		_rte_eth_dev_callback_process(sa->eth_dev,
 					      RTE_ETH_EVENT_INTR_LSC,
 					      NULL);
@@ -251,7 +251,7 @@ sfc_intr_configure(struct sfc_adapter *sa)
 	intr->handler = NULL;
 	intr->lsc_intr = (sa->eth_dev->data->dev_conf.intr_conf.lsc != 0);
 	if (!intr->lsc_intr) {
-		sfc_info(sa, "LSC tracking using interrupts is disabled");
+		sfc_notice(sa, "LSC tracking using interrupts is disabled");
 		goto done;
 	}
 
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 9d92b8c..895c2f5 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -176,7 +176,7 @@ sfc_mcdi_do_log(const struct sfc_adapter *sa,
 			 * at the end which is required by netlogdecode.
 			 */
 			buffer[position] = '\0';
-			sfc_info(sa, "%s \\", buffer);
+			sfc_notice(sa, "%s \\", buffer);
 			/* Preserve prefix for the next log message */
 			position = pfxsize;
 		}
@@ -201,7 +201,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	if (!sa->mcdi.logging)
 		return;
 
-	/* The format including prefix added by sfc_info() is the format
+	/* The format including prefix added by sfc_notice() is the format
 	 * consumed by the Solarflare netlogdecode tool.
 	 */
 	pfxsize = snprintf(buffer, sizeof(buffer), "MCDI RPC %s:",
@@ -212,7 +212,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	start = sfc_mcdi_do_log(sa, buffer, data, data_size, pfxsize, start);
 	if (start != pfxsize) {
 		buffer[start] = '\0';
-		sfc_info(sa, "%s", buffer);
+		sfc_notice(sa, "%s", buffer);
 	}
 }
 
diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c
index abc53fb..f7aaab0 100644
--- a/drivers/net/sfc/sfc_rx.c
+++ b/drivers/net/sfc/sfc_rx.c
@@ -608,7 +608,7 @@ sfc_rx_qflush(struct sfc_adapter *sa, unsigned int sw_index)
 			sfc_err(sa, "RxQ %u flush failed", sw_index);
 
 		if (rxq->state & SFC_RXQ_FLUSHED)
-			sfc_info(sa, "RxQ %u flushed", sw_index);
+			sfc_notice(sa, "RxQ %u flushed", sw_index);
 	}
 
 	sa->dp_rx->qpurge(rxq->dp);
diff --git a/drivers/net/sfc/sfc_tx.c b/drivers/net/sfc/sfc_tx.c
index 757b03b..0035c5c 100644
--- a/drivers/net/sfc/sfc_tx.c
+++ b/drivers/net/sfc/sfc_tx.c
@@ -606,7 +606,7 @@ sfc_tx_qstop(struct sfc_adapter *sa, unsigned int sw_index)
 			sfc_err(sa, "TxQ %u flush timed out", sw_index);
 
 		if (txq->state & SFC_TXQ_FLUSHED)
-			sfc_info(sa, "TxQ %u flushed", sw_index);
+			sfc_notice(sa, "TxQ %u flushed", sw_index);
 	}
 
 	sa->dp_tx->qreap(txq->dp);
-- 
2.7.4

[dpdk-dev] [PATCH v3 5/6] net/sfc: remove dedicated init log parameter

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

The previous patches in the set convert per-port
logging to use NOTICE level and make this level default.
This provides the possibility to remove the dedicated
toggle for init-related messages and merge init logging
with the main log type. In order to keep these logs silent
by default, INFO level should be used.

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  |  4 ----
 drivers/net/sfc/sfc.h        |  1 -
 drivers/net/sfc/sfc_ethdev.c |  9 +--------
 drivers/net/sfc/sfc_kvargs.c |  1 -
 drivers/net/sfc/sfc_kvargs.h |  2 --
 drivers/net/sfc/sfc_log.h    | 11 +++++------
 6 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index a580d94..c0b52d3 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -321,10 +321,6 @@ boolean parameters value.
   **auto** allows NIC firmware to make a choice based on
   installed licences and firmware variant configured using **sfboot**.
 
-- ``debug_init`` [bool] (default **n**)
-
-  Enable extra logging during device initialization and startup.
-
 - ``mcdi_logging`` [bool] (default **n**)
 
   Enable extra logging of the communication with the NIC's management CPU.
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 768578e..378c675 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -171,7 +171,6 @@ struct sfc_adapter {
 	struct rte_eth_dev		*eth_dev;
 	struct rte_kvargs		*kvargs;
 	uint32_t			logtype_main;
-	bool				debug_init;
 	int				socket_id;
 	efsys_bar_t			mem_bar;
 	efx_family_t			family;
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index f27ce63..ebd0a23 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -1931,11 +1931,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	if (rc != 0)
 		goto fail_kvargs_parse;
 
-	rc = sfc_kvargs_process(sa, SFC_KVARG_DEBUG_INIT,
-				sfc_kvarg_bool_handler, &sa->debug_init);
-	if (rc != 0)
-		goto fail_kvarg_debug_init;
-
 	sfc_log_init(sa, "entry");
 
 	dev->data->mac_addrs = rte_zmalloc("sfc", ETHER_ADDR_LEN, 0);
@@ -1989,7 +1984,6 @@ sfc_eth_dev_init(struct rte_eth_dev *dev)
 	dev->data->mac_addrs = NULL;
 
 fail_mac_addrs:
-fail_kvarg_debug_init:
 	sfc_kvargs_cleanup(sa);
 
 fail_kvargs_parse:
@@ -2072,8 +2066,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_TX_DATAPATH "=" SFC_KVARG_VALUES_TX_DATAPATH " "
 	SFC_KVARG_PERF_PROFILE "=" SFC_KVARG_VALUES_PERF_PROFILE " "
 	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
-	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL " "
-	SFC_KVARG_DEBUG_INIT "=" SFC_KVARG_VALUES_BOOL);
+	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL);
 
 RTE_INIT(sfc_driver_register_logtype);
 static void
diff --git a/drivers/net/sfc/sfc_kvargs.c b/drivers/net/sfc/sfc_kvargs.c
index 8f55e99..860adeb 100644
--- a/drivers/net/sfc/sfc_kvargs.c
+++ b/drivers/net/sfc/sfc_kvargs.c
@@ -23,7 +23,6 @@ sfc_kvargs_parse(struct sfc_adapter *sa)
 	struct rte_devargs *devargs = eth_dev->device->devargs;
 	const char **params = (const char *[]){
 		SFC_KVARG_STATS_UPDATE_PERIOD_MS,
-		SFC_KVARG_DEBUG_INIT,
 		SFC_KVARG_MCDI_LOGGING,
 		SFC_KVARG_PERF_PROFILE,
 		SFC_KVARG_RX_DATAPATH,
diff --git a/drivers/net/sfc/sfc_kvargs.h b/drivers/net/sfc/sfc_kvargs.h
index e7044ca..2c7aaaf 100644
--- a/drivers/net/sfc/sfc_kvargs.h
+++ b/drivers/net/sfc/sfc_kvargs.h
@@ -18,8 +18,6 @@ extern "C" {
 
 #define SFC_KVARG_VALUES_BOOL		"[1|y|yes|on|0|n|no|off]"
 
-#define SFC_KVARG_DEBUG_INIT		"debug_init"
-
 #define SFC_KVARG_MCDI_LOGGING		"mcdi_logging"
 
 #define SFC_KVARG_PERF_PROFILE		"perf_profile"
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index ec903db..6daab1c 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -77,12 +77,11 @@ extern uint32_t sfc_logtype_driver;
 	do {								\
 		const struct sfc_adapter *_sa = (sa);			\
 									\
-		if (_sa->debug_init)					\
-			SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,	\
-				RTE_FMT("%s(): "			\
-					RTE_FMT_HEAD(__VA_ARGS__,),	\
-					__func__,			\
-					RTE_FMT_TAIL(__VA_ARGS__,)));	\
+		SFC_LOG(_sa, RTE_LOG_INFO, _sa->logtype_main,		\
+			RTE_FMT("%s(): "				\
+				RTE_FMT_HEAD(__VA_ARGS__ ,),		\
+				__func__,				\
+				RTE_FMT_TAIL(__VA_ARGS__ ,)));		\
 	} while (0)
 
 #endif /* _SFC_LOG_H_ */
-- 
2.7.4

[dpdk-dev] [PATCH v3 6/6] net/sfc: add dynamic log level for MCDI messages

[Andrew Rybchenko]

From: Ivan Malov <ivan.malov@oktetlabs.ru>

Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
Reviewed-by: Andy Moreton <amoreton@solarflare.com>
---
 doc/guides/nics/sfc_efx.rst  | 12 ++++++------
 drivers/net/sfc/sfc.h        |  2 +-
 drivers/net/sfc/sfc_ethdev.c |  3 +--
 drivers/net/sfc/sfc_kvargs.c |  1 -
 drivers/net/sfc/sfc_kvargs.h |  2 --
 drivers/net/sfc/sfc_log.h    | 15 +++++++++++++++
 drivers/net/sfc/sfc_mcdi.c   | 25 +++++++++++++------------
 7 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst
index c0b52d3..2e4c3d8 100644
--- a/doc/guides/nics/sfc_efx.rst
+++ b/doc/guides/nics/sfc_efx.rst
@@ -321,12 +321,6 @@ boolean parameters value.
   **auto** allows NIC firmware to make a choice based on
   installed licences and firmware variant configured using **sfboot**.
 
-- ``mcdi_logging`` [bool] (default **n**)
-
-  Enable extra logging of the communication with the NIC's management CPU.
-  The logging is done using RTE_LOG() with INFO level and PMD type.
-  The format is consumed by the Solarflare netlogdecode cross-platform tool.
-
 - ``stats_update_period_ms`` [long] (default **1000**)
 
   Adjust period in milliseconds to update port hardware statistics.
@@ -360,3 +354,9 @@ SFC EFX PMD provides the following log types available for control:
   Matches a subset of per-port log types registered during runtime.
   A full name for a particular type may be obtained by appending a
   dot and a PCI device identifier (``XXXX:XX:XX.X``) to the prefix.
+
+- ``pmd.net.sfc.mcdi`` (default level is **6** - ``RTE_LOG_NOTICE``)
+
+  Extra logging of the communication with the NIC's management CPU.
+  The format of the log is consumed by the Solarflare netlogdecode
+  cross-platform tool. May be managed per-port, as explained above.
diff --git a/drivers/net/sfc/sfc.h b/drivers/net/sfc/sfc.h
index 378c675..65a4df2 100644
--- a/drivers/net/sfc/sfc.h
+++ b/drivers/net/sfc/sfc.h
@@ -104,7 +104,7 @@ struct sfc_mcdi {
 	efsys_mem_t			mem;
 	enum sfc_mcdi_state		state;
 	efx_mcdi_transport_t		transport;
-	bool				logging;
+	uint32_t			logtype;
 	uint32_t			proxy_handle;
 	efx_rc_t			proxy_result;
 };
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index ebd0a23..f16d520 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -2065,8 +2065,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_sfc_efx,
 	SFC_KVARG_RX_DATAPATH "=" SFC_KVARG_VALUES_RX_DATAPATH " "
 	SFC_KVARG_TX_DATAPATH "=" SFC_KVARG_VALUES_TX_DATAPATH " "
 	SFC_KVARG_PERF_PROFILE "=" SFC_KVARG_VALUES_PERF_PROFILE " "
-	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long> "
-	SFC_KVARG_MCDI_LOGGING "=" SFC_KVARG_VALUES_BOOL);
+	SFC_KVARG_STATS_UPDATE_PERIOD_MS "=<long>");
 
 RTE_INIT(sfc_driver_register_logtype);
 static void
diff --git a/drivers/net/sfc/sfc_kvargs.c b/drivers/net/sfc/sfc_kvargs.c
index 860adeb..9c08bff 100644
--- a/drivers/net/sfc/sfc_kvargs.c
+++ b/drivers/net/sfc/sfc_kvargs.c
@@ -23,7 +23,6 @@ sfc_kvargs_parse(struct sfc_adapter *sa)
 	struct rte_devargs *devargs = eth_dev->device->devargs;
 	const char **params = (const char *[]){
 		SFC_KVARG_STATS_UPDATE_PERIOD_MS,
-		SFC_KVARG_MCDI_LOGGING,
 		SFC_KVARG_PERF_PROFILE,
 		SFC_KVARG_RX_DATAPATH,
 		SFC_KVARG_TX_DATAPATH,
diff --git a/drivers/net/sfc/sfc_kvargs.h b/drivers/net/sfc/sfc_kvargs.h
index 2c7aaaf..a84dc6b 100644
--- a/drivers/net/sfc/sfc_kvargs.h
+++ b/drivers/net/sfc/sfc_kvargs.h
@@ -18,8 +18,6 @@ extern "C" {
 
 #define SFC_KVARG_VALUES_BOOL		"[1|y|yes|on|0|n|no|off]"
 
-#define SFC_KVARG_MCDI_LOGGING		"mcdi_logging"
-
 #define SFC_KVARG_PERF_PROFILE		"perf_profile"
 
 #define SFC_KVARG_PERF_PROFILE_AUTO		"auto"
diff --git a/drivers/net/sfc/sfc_log.h b/drivers/net/sfc/sfc_log.h
index 6daab1c..d6f3435 100644
--- a/drivers/net/sfc/sfc_log.h
+++ b/drivers/net/sfc/sfc_log.h
@@ -25,6 +25,12 @@ extern uint32_t sfc_logtype_driver;
 /** Name prefix for the per-device log type used to report basic information */
 #define SFC_LOGTYPE_MAIN_STR	SFC_LOGTYPE_PREFIX "main"
 
+/** Device MCDI log type name prefix */
+#define SFC_LOGTYPE_MCDI_STR	SFC_LOGTYPE_PREFIX "mcdi"
+
+/** Level value used by MCDI log statements */
+#define SFC_LOG_LEVEL_MCDI	RTE_LOG_INFO
+
 /* Log PMD message, automatically add prefix and \n */
 #define SFC_LOG(sa, level, type, ...) \
 	do {								\
@@ -84,4 +90,13 @@ extern uint32_t sfc_logtype_driver;
 				RTE_FMT_TAIL(__VA_ARGS__ ,)));		\
 	} while (0)
 
+#define sfc_log_mcdi(sa, ...) \
+	do {								\
+		const struct sfc_adapter *_sa = (sa);			\
+									\
+		SFC_LOG(_sa, SFC_LOG_LEVEL_MCDI, _sa->mcdi.logtype,	\
+			__VA_ARGS__);					\
+	} while (0)
+
+
 #endif /* _SFC_LOG_H_ */
diff --git a/drivers/net/sfc/sfc_mcdi.c b/drivers/net/sfc/sfc_mcdi.c
index 895c2f5..007506b 100644
--- a/drivers/net/sfc/sfc_mcdi.c
+++ b/drivers/net/sfc/sfc_mcdi.c
@@ -15,7 +15,6 @@
 
 #include "sfc.h"
 #include "sfc_log.h"
-#include "sfc_kvargs.h"
 #include "sfc_ev.h"
 
 #define SFC_MCDI_POLL_INTERVAL_MIN_US	10		/* 10us in 1us units */
@@ -176,7 +175,7 @@ sfc_mcdi_do_log(const struct sfc_adapter *sa,
 			 * at the end which is required by netlogdecode.
 			 */
 			buffer[position] = '\0';
-			sfc_notice(sa, "%s \\", buffer);
+			sfc_log_mcdi(sa, "%s \\", buffer);
 			/* Preserve prefix for the next log message */
 			position = pfxsize;
 		}
@@ -198,10 +197,17 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	size_t pfxsize;
 	size_t start;
 
-	if (!sa->mcdi.logging)
+	/*
+	 * Unlike the other cases, MCDI logging implies more onerous work
+	 * needed to produce a message. If the dynamic log level prevents
+	 * the end result from being printed, the CPU time will be wasted.
+	 *
+	 * To avoid wasting time, the actual level is examined in advance.
+	 */
+	if (rte_log_get_level(sa->mcdi.logtype) < (int)SFC_LOG_LEVEL_MCDI)
 		return;
 
-	/* The format including prefix added by sfc_notice() is the format
+	/* The format including prefix added by sfc_log_mcdi() is the format
 	 * consumed by the Solarflare netlogdecode tool.
 	 */
 	pfxsize = snprintf(buffer, sizeof(buffer), "MCDI RPC %s:",
@@ -212,7 +218,7 @@ sfc_mcdi_logger(void *arg, efx_log_msg_t type,
 	start = sfc_mcdi_do_log(sa, buffer, data, data_size, pfxsize, start);
 	if (start != pfxsize) {
 		buffer[start] = '\0';
-		sfc_notice(sa, "%s", buffer);
+		sfc_log_mcdi(sa, "%s", buffer);
 	}
 }
 
@@ -250,11 +256,8 @@ sfc_mcdi_init(struct sfc_adapter *sa)
 	if (rc != 0)
 		goto fail_dma_alloc;
 
-	/* Convert negative error to positive used in the driver */
-	rc = sfc_kvargs_process(sa, SFC_KVARG_MCDI_LOGGING,
-				sfc_kvarg_bool_handler, &mcdi->logging);
-	if (rc != 0)
-		goto fail_kvargs_process;
+	mcdi->logtype = sfc_register_logtype(sa, SFC_LOGTYPE_MCDI_STR,
+					     RTE_LOG_NOTICE);
 
 	emtp = &mcdi->transport;
 	emtp->emt_context = sa;
@@ -274,8 +277,6 @@ sfc_mcdi_init(struct sfc_adapter *sa)
 
 fail_mcdi_init:
 	memset(emtp, 0, sizeof(*emtp));
-
-fail_kvargs_process:
 	sfc_dma_free(sa, &mcdi->mem);
 
 fail_dma_alloc:
-- 
2.7.4

Re: [dpdk-dev] [PATCH v3 0/6] net/sfc: implement dynamic logging

[Ferruh Yigit]

On 3/21/2018 11:28 AM, Andrew Rybchenko wrote:
> The series adds EXPERIMENTAL EAL feature which removes dependency
> on EAL arguments processing and log types registration. It stores
> EAL loglevel arguments in the list and adds API function to register
> a new log type and pick up its value from EAL arguments.
> 
> For us it is important since we would like to be able to control
> per-device log level, e.g. pmd.net.sfc.main.0000:01:00.0.
> 
> v3:
>  - fix build issue reported by Ferruh
> 
> Ivan Malov (6):
>   eal: register log type and pick level from EAL args
>   net/sfc: add support for driver-wide dynamic logging
>   net/sfc: add support for per-port dynamic logging
>   net/sfc: prepare to merge init logs with main log type
>   net/sfc: remove dedicated init log parameter
>   net/sfc: add dynamic log level for MCDI messages

Series applied to dpdk-next-net/master, thanks.