sfc: add MCDI tracing

PATCH In review Edward Cree 2 participants 5 messages May 27, 2015

Subsystems (3)

Networking Network drivers Sfc network driver

lore

Share & tools

lore

Edward Cree

author

2015-05-27 12:12 UTC #0 lore raw

v4 cover sfc: add MCDI tracing

This patchset adds support for logging MCDI (Management-Controller-to-
Driver Interface) interactions between the sfc driver and a bound device, to aid in debugging.
Solarflare has a tool to decode the resulting traces and will look to
open-source this if there is any external interest, but the protocol is already detailed in drivers/net/ethernet/sfc/mcdi_pcol.h.
The logging buffer we allocate per MCDI context is a work area for
constructing each individual message before logging it with netif_info.
The reason the buffer is long-lived is simply to avoid the overhead of
allocating and freeing it every MCDI call, since MCDIs are already known to be serialised for other reasons.

--
v4: remove patch #4, which has already been applied via sshah
v3: add some explanations to cover letter and patch #4
v2: avoid long lines in cover letter; fix multiline comment style

Edward Cree (3):
sfc: add tracing of MCDI commands
sfc: add sysfs entry to control MCDI tracing
sfc: add module parameter to enable MCDI logging on new functions

 drivers/net/ethernet/sfc/Kconfig |   9 ++++
 drivers/net/ethernet/sfc/ef10.c  |   3 ++
 drivers/net/ethernet/sfc/efx.c   |  49 ++++++++++++++++---
 drivers/net/ethernet/sfc/mcdi.c  | 100 +++++++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/sfc/mcdi.h  |   6 +++
 5 files changed, 156 insertions(+), 11 deletions(-)

Edward Cree

author

2015-05-27 12:14 UTC #1 lore raw

PATCH v4 1/3 sfc: add tracing of MCDI commands

MCDI tracing is conditional on CONFIG_SFC_MCDI_LOGGING, which is enabled
by default.

Each MCDI command will produce a console line like
sfc dom:bus:dev:fn ifname: MCDI RPC REQ: xxxxxxxx [yyyyyyyy...]
where xxxxxxxx etc. are the raw MCDI payload in 32-bit hex chunks. The response will then produce a similar line with "RESP" instead of "REQ",
and containing the MCDI response payload (if any).

Signed-off-by Edward Cree <[email protected]>

---
 drivers/net/ethernet/sfc/Kconfig |  8 ++++
 drivers/net/ethernet/sfc/ef10.c  |  3 ++
 drivers/net/ethernet/sfc/mcdi.c  | 91 ++++++++++++++++++++++++++++++++++++++--
 drivers/net/ethernet/sfc/mcdi.h  |  4 ++
 4 files changed, 102 insertions(+), 4 deletions(-)

drivers/net/ethernet/sfc/Kconfig +8 -0

index 0889212..c4ba42b 100644
@@ -36,3 +36,11 @@ config SFC_SRIOV
 	  This enables support for the SFC9000 I/O Virtualization
 	  features, allowing accelerated network performance in
 	  virtualized environments.
+config SFC_MCDI_LOGGING
+	bool "Solarflare SFC9000/SFC9100-family MCDI logging support"
+	depends on SFC
+	default y
+	---help---
+	  This enables support for tracing of MCDI (Management-Controller-to-
+	  Driver-Interface) commands and responses, allowing debugging of
+	  driver/firmware interaction.

drivers/net/ethernet/sfc/ef10.c +3 -0

index a547ceb..4eb6ab7 100644
@@ -267,6 +267,9 @@ static int efx_ef10_probe(struct efx_nic *efx)
 		return -ENOMEM;
 	efx->nic_data = nic_data;
 
+	/* we assume later that we can copy from this buffer in dwords */
+	BUILD_BUG_ON(MCDI_CTL_SDU_LEN_MAX_V2 % 4);
+
 	rc = efx_nic_alloc_buffer(efx, &nic_data->mcdi_buf,
 				  8 + MCDI_CTL_SDU_LEN_MAX_V2, GFP_KERNEL);
 	if (rc)

drivers/net/ethernet/sfc/mcdi.c +87 -4

151 lines not on this page · raw message

drivers/net/ethernet/sfc/mcdi.h +4 -0

index 7afab2f..b783a2d 100644
@@ -58,6 +58,7 @@ enum efx_mcdi_mode {
  *	enabled
  * @async_list: Queue of asynchronous requests
  * @async_timer: Timer for asynchronous request timeout
+ * @logging_buffer: buffer that may be used to build MCDI tracing messages
  */
 struct efx_mcdi_iface {
 	struct efx_nic *efx;

@@ -74,6 +75,9 @@ struct efx_mcdi_iface {
 	spinlock_t async_lock;
 	struct list_head async_list;
 	struct timer_list async_timer;
+#ifdef CONFIG_SFC_MCDI_LOGGING
+	char *logging_buffer;
+#endif
 };
 
 struct efx_mcdi_mon {

Edward Cree

author

2015-05-27 12:14 UTC #2 lore raw

PATCH v4 2/3 sfc: add sysfs entry to control MCDI tracing

In reply to Edward Cree

MCDI tracing is enabled per-function with a sysfs file
/sys/class/net/<NET_DEV>/device/mcdi_logging

Signed-off-by Edward Cree <[email protected]>

---
 drivers/net/ethernet/sfc/Kconfig |  3 ++-
 drivers/net/ethernet/sfc/efx.c   | 49 ++++++++++++++++++++++++++++++++++------
 drivers/net/ethernet/sfc/mcdi.c  |  4 ++--
 drivers/net/ethernet/sfc/mcdi.h  |  2 ++
 4 files changed, 48 insertions(+), 10 deletions(-)

drivers/net/ethernet/sfc/Kconfig +2 -1

index c4ba42b..4dd92b7 100644
@@ -43,4 +43,5 @@ config SFC_MCDI_LOGGING
 	---help---
 	  This enables support for tracing of MCDI (Management-Controller-to-
 	  Driver-Interface) commands and responses, allowing debugging of
-	  driver/firmware interaction.
+	  driver/firmware interaction.  The tracing is actually enabled by
+	  a sysfs file 'mcdi_logging' under the PCI device.

drivers/net/ethernet/sfc/efx.c +42 -7

70 lines not on this page · raw message

drivers/net/ethernet/sfc/mcdi.c +2 -2

index 31eda32..dde7f90 100644
@@ -188,7 +188,7 @@ static void efx_mcdi_send_request(struct efx_nic *efx, unsigned cmd,
 	}
 
 #ifdef CONFIG_SFC_MCDI_LOGGING
-	if (!WARN_ON_ONCE(!buf)) {
+	if (mcdi->logging_enabled && !WARN_ON_ONCE(!buf)) {
 		int bytes = 0;
 		int i;
 		/* Lengths should always be a whole number of dwords, so scream

@@ -274,7 +274,7 @@ static void efx_mcdi_read_response_header(struct efx_nic *efx)
 	}
 
 #ifdef CONFIG_SFC_MCDI_LOGGING
-	if (!WARN_ON_ONCE(!buf)) {
+	if (mcdi->logging_enabled && !WARN_ON_ONCE(!buf)) {
 		size_t hdr_len, data_len;
 		int bytes = 0;
 		int i;

drivers/net/ethernet/sfc/mcdi.h +2 -0

index b783a2d..1838afe 100644
@@ -59,6 +59,7 @@ enum efx_mcdi_mode {
  * @async_list: Queue of asynchronous requests
  * @async_timer: Timer for asynchronous request timeout
  * @logging_buffer: buffer that may be used to build MCDI tracing messages
+ * @logging_enabled: whether to trace MCDI
  */
 struct efx_mcdi_iface {
 	struct efx_nic *efx;

@@ -77,6 +78,7 @@ struct efx_mcdi_iface {
 	struct timer_list async_timer;
 #ifdef CONFIG_SFC_MCDI_LOGGING
 	char *logging_buffer;
+	bool logging_enabled;
 #endif
 };

Edward Cree

author

2015-05-27 12:14 UTC #3 lore raw

PATCH v4 3/3 sfc: add module parameter to enable MCDI logging on new functions

In reply to Edward Cree

As many issues are encountered at probe time, where MCDI logging can't be
enabled through the sysfs node, this change adds a module parameter 'mcdi_logging_default', which defaults to false. When set to true, newly- probed functions will have MCDI logging enabled. The setting can subsequently be changed as normal through the sysfs node.

Signed-off-by Edward Cree <[email protected]>

---
 drivers/net/ethernet/sfc/mcdi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/net/ethernet/sfc/mcdi.c +9 -0

index dde7f90..81640f8 100644
@@ -8,6 +8,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/moduleparam.h>
 #include <asm/cmpxchg.h>
 #include "net_driver.h"
 #include "nic.h"

@@ -54,6 +55,13 @@ static int efx_mcdi_drv_attach(struct efx_nic *efx, bool driver_operating,
 static bool efx_mcdi_poll_once(struct efx_nic *efx);
 static void efx_mcdi_abandon(struct efx_nic *efx);
 
+#ifdef CONFIG_SFC_MCDI_LOGGING
+static bool mcdi_logging_default;
+module_param(mcdi_logging_default, bool, 0644);
+MODULE_PARM_DESC(mcdi_logging_default,
+		 "Enable MCDI logging on newly-probed functions");
+#endif
+
 int efx_mcdi_init(struct efx_nic *efx)
 {
 	struct efx_mcdi_iface *mcdi;

@@ -71,6 +79,7 @@ int efx_mcdi_init(struct efx_nic *efx)
 	mcdi->logging_buffer = (char *)__get_free_page(GFP_KERNEL);
 	if (!mcdi->logging_buffer)
 		goto fail1;
+	mcdi->logging_enabled = mcdi_logging_default;
 #endif
 	init_waitqueue_head(&mcdi->wq);
 	spin_lock_init(&mcdi->iface_lock);

David Miller

maintainer

2015-05-27 17:55 UTC #4 lore raw

In reply to Edward Cree

From: Edward Cree <[email protected]>
Date: Wed, 27 May 2015 13:12:31 +0100

Quoted 12 lines

This patchset adds support for logging MCDI (Management-Controller-to-
Driver Interface) interactions between the sfc driver and a bound device, to aid in debugging.
Solarflare has a tool to decode the resulting traces and will look to
open-source this if there is any external interest, but the protocol is already detailed in drivers/net/ethernet/sfc/mcdi_pcol.h.
The logging buffer we allocate per MCDI context is a work area for
constructing each individual message before logging it with netif_info.
The reason the buffer is long-lived is simply to avoid the overhead of
allocating and freeing it every MCDI call, since MCDIs are already known to be serialised for other reasons.

Quoted 4 lines

--
v4: remove patch #4, which has already been applied via sshah
v3: add some explanations to cover letter and patch #4
v2: avoid long lines in cover letter; fix multiline comment style

Series applied, thanks.