netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality
@ 2017-04-13  7:48 Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 1/8] net/ncsi: Disable HWA mode when no channels are found Gavin Shan
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

This series supports NCSI debugging infrastructure by adding several
debugfs files. It was inspired by the reported issues: No available
package and channel are probed successfully. Obviously, we don't
have a debugging infrastructure for NCSI stack yet.

The first 3 patches, fixing some issues, aren't relevant to the
subject. I included them because I expect they can be merged beofre
the code for debugging infrastructure. PATCH[4,5,6/8] adds debugfs 
directories and files to support the debugging infrastructure for
several purposes: presenting the NCSI topology; statistics on sent
and received NCSI packets; generate NCSI command packet manually.
PATCH[7,8/8] fixes two issues found from the debugging functionality.

Changelog
=========
v2:
   * Use debugfs instead of procfs (Joe Perches).

Gavin Shan (8):
  net/ncsi: Disable HWA mode when no channels are found
  net/ncsi: Properly track channel monitor timer state
  net/ncsi: Enforce failover on link monitor timeout
  net/ncsi: Add debugging infrastructurre
  net/ncsi: Dump NCSI packet statistics
  net/ncsi: Support NCSI packet generation
  net/ncsi: No error report on DP response to non-existing package
  net/ncsi: Fix length of GVI response packet

 net/ncsi/Kconfig       |   9 +
 net/ncsi/Makefile      |   1 +
 net/ncsi/internal.h    |  68 ++++
 net/ncsi/ncsi-aen.c    |  15 +-
 net/ncsi/ncsi-cmd.c    |  23 +-
 net/ncsi/ncsi-debug.c  | 962 +++++++++++++++++++++++++++++++++++++++++++++++++
 net/ncsi/ncsi-manage.c |  63 +++-
 net/ncsi/ncsi-rsp.c    |  37 +-
 8 files changed, 1166 insertions(+), 12 deletions(-)
 create mode 100644 net/ncsi/ncsi-debug.c

-- 
2.7.4

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

* [PATCH v2 net-next 1/8] net/ncsi: Disable HWA mode when no channels are found
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 2/8] net/ncsi: Properly track channel monitor timer state Gavin Shan
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

When there are no NCSI channels probed, HWA (Hardware Arbitration)
mode is enabled. It's not correct because HWA depends on the fact:
NCSI channels exist and all of them support HWA mode. This disables
HWA when no channels are probed.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-manage.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index a3bd5fa..5073e15 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -839,12 +839,15 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
 	struct ncsi_package *np;
 	struct ncsi_channel *nc;
 	unsigned int cap;
+	bool has_channel = false;
 
 	/* The hardware arbitration is disabled if any one channel
 	 * doesn't support explicitly.
 	 */
 	NCSI_FOR_EACH_PACKAGE(ndp, np) {
 		NCSI_FOR_EACH_CHANNEL(np, nc) {
+			has_channel = true;
+
 			cap = nc->caps[NCSI_CAP_GENERIC].cap;
 			if (!(cap & NCSI_CAP_GENERIC_HWA) ||
 			    (cap & NCSI_CAP_GENERIC_HWA_MASK) !=
@@ -855,8 +858,13 @@ static bool ncsi_check_hwa(struct ncsi_dev_priv *ndp)
 		}
 	}
 
-	ndp->flags |= NCSI_DEV_HWA;
-	return true;
+	if (has_channel) {
+		ndp->flags |= NCSI_DEV_HWA;
+		return true;
+	}
+
+	ndp->flags &= ~NCSI_DEV_HWA;
+	return false;
 }
 
 static int ncsi_enable_hwa(struct ncsi_dev_priv *ndp)
-- 
2.7.4

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

* [PATCH v2 net-next 2/8] net/ncsi: Properly track channel monitor timer state
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 1/8] net/ncsi: Disable HWA mode when no channels are found Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 3/8] net/ncsi: Enforce failover on link monitor timeout Gavin Shan
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

The field @monitor.enabled in the NCSI channel descriptor is used
to track the state of channel monitor timer. It indicates the timer's
state (pending or not). We could not start the timer again in its
handler. In that case, We missed to update @monitor.enabled to false.
It leads to below warning printed by WARN_ON_ONCE() when the monitor
is restarted afterwards.

   ------------[ cut here ]------------
   WARNING: CPU: 0 PID: 411 at /var/lib/jenkins/workspace/openbmc-build \
   /distro/ubuntu/target/palmetto/openbmc/build/tmp/work-shared/palmetto \
   net/ncsi/ncsi-manage.c:240 ncsi_start_channel_monitor+0x44/0x7c
   CPU: 0 PID: 411 Comm: kworker/0:3 Not tainted \
   4.7.10-f26558191540830589fe03932d05577957670b8d #1
   Hardware name: ASpeed SoC
   Workqueue: events ncsi_dev_work
   [<c0106cbc>] (unwind_backtrace) from [<c0104f04>] (show_stack+0x10/0x14)
   [<c0104f04>] (show_stack) from [<c010eef8>] (__warn+0xc4/0xf0)
   [<c010eef8>] (__warn) from [<c010f018>] (warn_slowpath_null+0x1c/0x24)
   [<c010f018>] (warn_slowpath_null) from [<c03e1d10>] (ncsi_start_channel_monitor+0x44/0x7c)
   [<c03e1d10>] (ncsi_start_channel_monitor) from [<c03e29c4>] (ncsi_configure_channel+0x27c/0x2dc)
   [<c03e29c4>] (ncsi_configure_channel) from [<c03e31d0>] (ncsi_dev_work+0x39c/0x3e8)
   [<c03e31d0>] (ncsi_dev_work) from [<c0120288>] (process_one_work+0x1b8/0x2fc)
   [<c0120288>] (process_one_work) from [<c01206bc>] (worker_thread+0x2c0/0x3f8)
   [<c01206bc>] (worker_thread) from [<c0124b10>] (kthread+0xd0/0xe8)
   [<c0124b10>] (kthread) from [<c0102250>] (ret_from_fork+0x14/0x24)
   ---[ end trace 110cccf2b038c44d ]---

This fixes the issue by updating @monitor.enabled to false if needed.

Reported-by: Sridevi Ramesh <sridevra@in.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-manage.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 5073e15..c71a3a5 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -183,11 +183,16 @@ static void ncsi_channel_monitor(unsigned long data)
 	monitor_state = nc->monitor.state;
 	spin_unlock_irqrestore(&nc->lock, flags);
 
-	if (!enabled || chained)
+	if (!enabled || chained) {
+		ncsi_stop_channel_monitor(nc);
 		return;
+	}
+
 	if (state != NCSI_CHANNEL_INACTIVE &&
-	    state != NCSI_CHANNEL_ACTIVE)
+	    state != NCSI_CHANNEL_ACTIVE) {
+		ncsi_stop_channel_monitor(nc);
 		return;
+	}
 
 	switch (monitor_state) {
 	case NCSI_CHANNEL_MONITOR_START:
@@ -199,6 +204,7 @@ static void ncsi_channel_monitor(unsigned long data)
 		nca.req_flags = 0;
 		ret = ncsi_xmit_cmd(&nca);
 		if (ret) {
+			ncsi_stop_channel_monitor(nc);
 			netdev_err(ndp->ndev.dev, "Error %d sending GLS\n",
 				   ret);
 			return;
@@ -218,6 +224,8 @@ static void ncsi_channel_monitor(unsigned long data)
 		nc->state = NCSI_CHANNEL_INVISIBLE;
 		spin_unlock_irqrestore(&nc->lock, flags);
 
+		ncsi_stop_channel_monitor(nc);
+
 		spin_lock_irqsave(&ndp->lock, flags);
 		nc->state = NCSI_CHANNEL_INACTIVE;
 		list_add_tail_rcu(&nc->link, &ndp->channel_queue);
@@ -257,6 +265,10 @@ void ncsi_stop_channel_monitor(struct ncsi_channel *nc)
 	nc->monitor.enabled = false;
 	spin_unlock_irqrestore(&nc->lock, flags);
 
+	/* The timer isn't in pending state if we're deleting the timer
+	 * in its handler. del_timer_sync() can detect it and just does
+	 * nothing.
+	 */
 	del_timer_sync(&nc->monitor.timer);
 }
 
-- 
2.7.4

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

* [PATCH v2 net-next 3/8] net/ncsi: Enforce failover on link monitor timeout
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 1/8] net/ncsi: Disable HWA mode when no channels are found Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 2/8] net/ncsi: Properly track channel monitor timer state Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre Gavin Shan
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

The NCSI channel has been configured to provide service if its link
monitor timer is enabled, regardless of its state (inactive or active).
So the timeout event on the link monitor indicates the out-of-service
on that channel, for which a failover is needed.

This sets NCSI_DEV_RESHUFFLE flag to enforce failover on link monitor
timeout, regardless the channel's original state (inactive or active).
Also, the link is put into "down" state to give the failing channel
lowest priority when selecting for the active channel. The state of
failing channel should be set to active in order for deinitialization
and failover to be done.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-manage.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index c71a3a5..13ad1f26 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -170,6 +170,7 @@ static void ncsi_channel_monitor(unsigned long data)
 	struct ncsi_channel *nc = (struct ncsi_channel *)data;
 	struct ncsi_package *np = nc->package;
 	struct ncsi_dev_priv *ndp = np->ndp;
+	struct ncsi_channel_mode *ncm;
 	struct ncsi_cmd_arg nca;
 	bool enabled, chained;
 	unsigned int monitor_state;
@@ -214,20 +215,21 @@ static void ncsi_channel_monitor(unsigned long data)
 	case NCSI_CHANNEL_MONITOR_WAIT ... NCSI_CHANNEL_MONITOR_WAIT_MAX:
 		break;
 	default:
-		if (!(ndp->flags & NCSI_DEV_HWA) &&
-		    state == NCSI_CHANNEL_ACTIVE) {
+		if (!(ndp->flags & NCSI_DEV_HWA)) {
 			ncsi_report_link(ndp, true);
 			ndp->flags |= NCSI_DEV_RESHUFFLE;
 		}
 
+		ncm = &nc->modes[NCSI_MODE_LINK];
 		spin_lock_irqsave(&nc->lock, flags);
 		nc->state = NCSI_CHANNEL_INVISIBLE;
+		ncm->data[2] &= ~0x1;
 		spin_unlock_irqrestore(&nc->lock, flags);
 
 		ncsi_stop_channel_monitor(nc);
 
 		spin_lock_irqsave(&ndp->lock, flags);
-		nc->state = NCSI_CHANNEL_INACTIVE;
+		nc->state = NCSI_CHANNEL_ACTIVE;
 		list_add_tail_rcu(&nc->link, &ndp->channel_queue);
 		spin_unlock_irqrestore(&ndp->lock, flags);
 		ncsi_process_next_channel(ndp);
-- 
2.7.4

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

* [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
                   ` (2 preceding siblings ...)
  2017-04-13  7:48 ` [PATCH v2 net-next 3/8] net/ncsi: Enforce failover on link monitor timeout Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13 10:41   ` Joe Perches
  2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

This creates debugfs directories as NCSI debugging infrastructure.
With the patch applied, We will see below debugfs directories. Every
NCSI package and channel has one corresponding directory. Other than
presenting the NCSI topology, No real function has been achieved
through these debugfs directories so far.

     /sys/kernel/debug/ncsi/eth0
     /sys/kernel/debug/ncsi/eth0/p0
     /sys/kernel/debug/ncsi/eth0/p0/c0
     /sys/kernel/debug/ncsi/eth0/p0/c1

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/Kconfig       |  9 +++++
 net/ncsi/Makefile      |  1 +
 net/ncsi/internal.h    | 45 ++++++++++++++++++++++++
 net/ncsi/ncsi-debug.c  | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++
 net/ncsi/ncsi-manage.c | 16 +++++++++
 5 files changed, 164 insertions(+)
 create mode 100644 net/ncsi/ncsi-debug.c

diff --git a/net/ncsi/Kconfig b/net/ncsi/Kconfig
index 08a8a60..baa42501 100644
--- a/net/ncsi/Kconfig
+++ b/net/ncsi/Kconfig
@@ -10,3 +10,12 @@ config NET_NCSI
 	  support. Enable this only if your system connects to a network
 	  device via NCSI and the ethernet driver you're using supports
 	  the protocol explicitly.
+
+config NET_NCSI_DEBUG
+	bool "Enable NCSI debugging"
+	depends on NET_NCSI && DEBUG_FS
+	default n
+	---help---
+	  This enables the interfaces (e.g. debugfs) for NCSI debugging purpose.
+
+	  If unsure, say Y.
diff --git a/net/ncsi/Makefile b/net/ncsi/Makefile
index dd12b56..2897fa0 100644
--- a/net/ncsi/Makefile
+++ b/net/ncsi/Makefile
@@ -2,3 +2,4 @@
 # Makefile for NCSI API
 #
 obj-$(CONFIG_NET_NCSI) += ncsi-cmd.o ncsi-rsp.o ncsi-aen.o ncsi-manage.o
+obj-$(CONFIG_NET_NCSI_DEBUG) += ncsi-debug.o
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 1308a56..e9ede4f 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -198,6 +198,9 @@ struct ncsi_channel {
 	} monitor;
 	struct list_head            node;
 	struct list_head            link;
+#ifdef CONFIG_NET_NCSI_DEBUG
+	struct dentry               *dentry;    /* Debugfs directory    */
+#endif
 };
 
 struct ncsi_package {
@@ -208,6 +211,9 @@ struct ncsi_package {
 	unsigned int         channel_num; /* Number of channels     */
 	struct list_head     channels;    /* List of chanels        */
 	struct list_head     node;        /* Form list of packages  */
+#ifdef CONFIG_NET_NCSI_DEBUG
+	struct dentry        *dentry;     /* Debugfs directory       */
+#endif
 };
 
 struct ncsi_request {
@@ -276,6 +282,9 @@ struct ncsi_dev_priv {
 	struct work_struct  work;            /* For channel management     */
 	struct packet_type  ptype;           /* NCSI packet Rx handler     */
 	struct list_head    node;            /* Form NCSI device list      */
+#ifdef CONFIG_NET_NCSI_DEBUG
+	struct dentry       *dentry;         /* Procfs directory           */
+#endif
 };
 
 struct ncsi_cmd_arg {
@@ -337,4 +346,40 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 		 struct packet_type *pt, struct net_device *orig_dev);
 int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb);
 
+/* Debugging functionality */
+#ifdef CONFIG_NET_NCSI_DEBUG
+int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp);
+void ncsi_dev_release_debug(struct ncsi_dev_priv *ndp);
+int ncsi_package_init_debug(struct ncsi_package *np);
+void ncsi_package_release_debug(struct ncsi_package *np);
+int ncsi_channel_init_debug(struct ncsi_channel *nc);
+void ncsi_channel_release_debug(struct ncsi_channel *nc);
+#else
+static inline int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
+{
+	return -ENOTTY;
+}
+
+static inline void ncsi_dev_release_debug(struct ncsi_dev_priv *ndp)
+{
+}
+
+static inline int ncsi_package_init_debug(struct ncsi_package *np)
+{
+	return -ENOTTY;
+}
+
+static inline void ncsi_package_release_debug(struct ncsi_package *np)
+{
+}
+
+static inline int ncsi_channel_init_debug(struct ncsi_channel *nc)
+{
+	return -ENOTTY;
+}
+
+static inline void ncsi_channel_release_debug(struct ncsi_channel *nc)
+{
+}
+#endif /* CONFIG_NET_NCSI_DEBUG */
 #endif /* __NCSI_INTERNAL_H__ */
diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
new file mode 100644
index 0000000..6c00e9b
--- /dev/null
+++ b/net/ncsi/ncsi-debug.c
@@ -0,0 +1,93 @@
+/*
+ * Copyright Gavin Shan, IBM Corporation 2017.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/atomic.h>
+#include <linux/netdevice.h>
+#include <linux/debugfs.h>
+#include <linux/skbuff.h>
+
+#include <net/ncsi.h>
+#include <net/net_namespace.h>
+#include <net/sock.h>
+
+#include "internal.h"
+#include "ncsi-pkt.h"
+
+static struct dentry *ncsi_dentry;
+
+int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
+{
+	if (WARN_ON_ONCE(ndp->dentry))
+		return 0;
+
+	if (!ncsi_dentry) {
+		ncsi_dentry = debugfs_create_dir("ncsi", NULL);
+		if (!ncsi_dentry) {
+			pr_warn("NCSI: Cannot create /sys/kernel/debug/ncsi\n");
+			return -ENOENT;
+		}
+	}
+
+	ndp->dentry = debugfs_create_dir(netdev_name(ndp->ndev.dev),
+					 ncsi_dentry);
+	if (!ndp->dentry)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void ncsi_dev_release_debug(struct ncsi_dev_priv *ndp)
+{
+	debugfs_remove(ndp->dentry);
+}
+
+int ncsi_package_init_debug(struct ncsi_package *np)
+{
+	struct ncsi_dev_priv *ndp = np->ndp;
+	char name[4];
+
+	if (!ndp->dentry)
+		return -ENOENT;
+
+	sprintf(name, "p%d", np->id);
+	np->dentry = debugfs_create_dir(name, ndp->dentry);
+	if (!np->dentry)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void ncsi_package_release_debug(struct ncsi_package *np)
+{
+	debugfs_remove(np->dentry);
+}
+
+int ncsi_channel_init_debug(struct ncsi_channel *nc)
+{
+	struct ncsi_package *np = nc->package;
+	char name[3];
+
+	if (!np->dentry)
+		return -ENOENT;
+
+	sprintf(name, "c%d", nc->id);
+	nc->dentry = debugfs_create_dir(name, np->dentry);
+	if (!nc->dentry)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void ncsi_channel_release_debug(struct ncsi_channel *nc)
+{
+	debugfs_remove(nc->dentry);
+}
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 13ad1f26..84f1405 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -322,6 +322,8 @@ struct ncsi_channel *ncsi_add_channel(struct ncsi_package *np, unsigned char id)
 	np->channel_num++;
 	spin_unlock_irqrestore(&np->lock, flags);
 
+	ncsi_channel_init_debug(nc);
+
 	return nc;
 }
 
@@ -332,6 +334,8 @@ static void ncsi_remove_channel(struct ncsi_channel *nc)
 	unsigned long flags;
 	int i;
 
+	ncsi_channel_release_debug(nc);
+
 	/* Release filters */
 	spin_lock_irqsave(&nc->lock, flags);
 	for (i = 0; i < NCSI_FILTER_MAX; i++) {
@@ -396,6 +400,8 @@ struct ncsi_package *ncsi_add_package(struct ncsi_dev_priv *ndp,
 	ndp->package_num++;
 	spin_unlock_irqrestore(&ndp->lock, flags);
 
+	ncsi_package_init_debug(np);
+
 	return np;
 }
 
@@ -409,6 +415,8 @@ void ncsi_remove_package(struct ncsi_package *np)
 	list_for_each_entry_safe(nc, tmp, &np->channels, node)
 		ncsi_remove_channel(nc);
 
+	ncsi_package_release_debug(np);
+
 	/* Remove and free package */
 	spin_lock_irqsave(&ndp->lock, flags);
 	list_del_rcu(&np->node);
@@ -1280,6 +1288,13 @@ int ncsi_start_dev(struct ncsi_dev *nd)
 		return -ENOTTY;
 
 	if (!(ndp->flags & NCSI_DEV_PROBED)) {
+		/* The debugging functionality should have been initialized
+		 * when registerring the NCSI device. As the network device
+		 * name isn't available that time, we have to delay the work
+		 * to here.
+		 */
+		ncsi_dev_init_debug(ndp);
+
 		nd->state = ncsi_dev_state_probe;
 		schedule_work(&ndp->work);
 		return 0;
@@ -1329,6 +1344,7 @@ void ncsi_unregister_dev(struct ncsi_dev *nd)
 	struct ncsi_package *np, *tmp;
 	unsigned long flags;
 
+	ncsi_dev_release_debug(ndp);
 	dev_remove_pack(&ndp->ptype);
 
 	list_for_each_entry_safe(np, tmp, &ndp->packages, node)
-- 
2.7.4

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

* [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
                   ` (3 preceding siblings ...)
  2017-04-13  7:48 ` [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13 10:50   ` Joe Perches
                     ` (2 more replies)
  2017-04-13  7:48 ` [PATCH v2 net-next 6/8] net/ncsi: Support NCSI packet generation Gavin Shan
                   ` (2 subsequent siblings)
  7 siblings, 3 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
packets sent and received over all packages and channels. It's useful
to diagnose NCSI problems, especially when NCSI packages and channels
aren't probed properly. The statistics can be gained from debugfs file
as below:

 # cat /sys/kernel/debug/ncsi/eth0/stats

 CMD          OK       TIMEOUT  ERROR
 =======================================
 CIS          32       29       0
 SP           10       7        0
 DP           17       14       0
 EC           1        0        0
 ECNT         1        0        0
 AE           1        0        0
 GLS          11       0        0
 SMA          1        0        0
 EBF          1        0        0
 GVI          2        0        0
 GC           2        0        0

 RSP          OK       TIMEOUT  ERROR
 =======================================
 CIS          3        0        0
 SP           3        0        0
 DP           2        0        1
 EC           1        0        0
 ECNT         1        0        0
 AE           1        0        0
 GLS          11       0        0
 SMA          1        0        0
 EBF          1        0        0
 GVI          0        0        2
 GC           2        0        0

 AEN          OK       TIMEOUT  ERROR
 =======================================

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/internal.h    |  10 +++
 net/ncsi/ncsi-aen.c    |  15 +++-
 net/ncsi/ncsi-cmd.c    |  18 +++-
 net/ncsi/ncsi-debug.c  | 235 +++++++++++++++++++++++++++++++++++++++++++++++++
 net/ncsi/ncsi-manage.c |   8 ++
 net/ncsi/ncsi-rsp.c    |  21 ++++-
 6 files changed, 304 insertions(+), 3 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index e9ede4f..009c41d 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -283,6 +283,16 @@ struct ncsi_dev_priv {
 	struct packet_type  ptype;           /* NCSI packet Rx handler     */
 	struct list_head    node;            /* Form NCSI device list      */
 #ifdef CONFIG_NET_NCSI_DEBUG
+	struct {
+		struct dentry  *dentry;
+#define NCSI_PKT_STAT_OK	0
+#define NCSI_PKT_STAT_TIMEOUT	1
+#define NCSI_PKT_STAT_ERROR	2
+#define NCSI_PKT_STAT_MAX	3
+		unsigned long  cmd[128][NCSI_PKT_STAT_MAX];
+		unsigned long  rsp[128][NCSI_PKT_STAT_MAX];
+		unsigned long  aen[256][NCSI_PKT_STAT_MAX];
+	} stats;
 	struct dentry       *dentry;         /* Procfs directory           */
 #endif
 };
diff --git a/net/ncsi/ncsi-aen.c b/net/ncsi/ncsi-aen.c
index 6898e72..adcaa56 100644
--- a/net/ncsi/ncsi-aen.c
+++ b/net/ncsi/ncsi-aen.c
@@ -206,16 +206,29 @@ int ncsi_aen_handler(struct ncsi_dev_priv *ndp, struct sk_buff *skb)
 	}
 
 	if (!nah) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.aen[h->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		netdev_warn(ndp->ndev.dev, "Invalid AEN (0x%x) received\n",
 			    h->type);
 		return -ENOENT;
 	}
 
 	ret = ncsi_validate_aen_pkt(h, nah->payload);
-	if (ret)
+	if (ret) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.aen[h->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		goto out;
+	}
 
 	ret = nah->handler(ndp, h);
+#ifdef CONFIG_NET_NCSI_DEBUG
+	if (ret)
+		ndp->stats.aen[h->type][NCSI_PKT_STAT_ERROR]++;
+	else
+		ndp->stats.aen[h->type][NCSI_PKT_STAT_OK]++;
+#endif
 out:
 	consume_skb(skb);
 	return ret;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index db7083b..c6b2bc5 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -323,6 +323,9 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 	}
 
 	if (!nch) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		nca->ndp->stats.cmd[nca->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		netdev_err(nca->ndp->ndev.dev,
 			   "Cannot send packet with type 0x%02x\n", nca->type);
 		return -ENOENT;
@@ -331,13 +334,20 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 	/* Get packet payload length and allocate the request */
 	nca->payload = nch->payload;
 	nr = ncsi_alloc_command(nca);
-	if (!nr)
+	if (!nr) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		nca->ndp->stats.cmd[nca->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		return -ENOMEM;
+	}
 
 	/* Prepare the packet */
 	nca->id = nr->id;
 	ret = nch->handler(nr->cmd, nca);
 	if (ret) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		nca->ndp->stats.cmd[nca->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		ncsi_free_request(nr);
 		return ret;
 	}
@@ -359,9 +369,15 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 	skb_get(nr->cmd);
 	ret = dev_queue_xmit(nr->cmd);
 	if (ret < 0) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		nca->ndp->stats.cmd[nca->type][NCSI_PKT_STAT_ERROR]++;
+#endif
 		ncsi_free_request(nr);
 		return ret;
 	}
 
+#ifdef CONFIG_NET_NCSI_DEBUG
+	nca->ndp->stats.cmd[nca->type][NCSI_PKT_STAT_OK]++;
+#endif
 	return 0;
 }
diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
index 6c00e9b..29c233c 100644
--- a/net/ncsi/ncsi-debug.c
+++ b/net/ncsi/ncsi-debug.c
@@ -23,6 +23,235 @@
 #include "ncsi-pkt.h"
 
 static struct dentry *ncsi_dentry;
+static struct ncsi_pkt_handler {
+	unsigned char	type;
+	const char	*name;
+} ncsi_pkt_handlers[] = {
+	{ NCSI_PKT_CMD_CIS,    "CIS"    },
+	{ NCSI_PKT_CMD_SP,     "SP"     },
+	{ NCSI_PKT_CMD_DP,     "DP"     },
+	{ NCSI_PKT_CMD_EC,     "EC"     },
+	{ NCSI_PKT_CMD_DC,     "DC"     },
+	{ NCSI_PKT_CMD_RC,     "RC"     },
+	{ NCSI_PKT_CMD_ECNT,   "ECNT"   },
+	{ NCSI_PKT_CMD_DCNT,   "DCNT"   },
+	{ NCSI_PKT_CMD_AE,     "AE"     },
+	{ NCSI_PKT_CMD_SL,     "SL"     },
+	{ NCSI_PKT_CMD_GLS,    "GLS"    },
+	{ NCSI_PKT_CMD_SVF,    "SVF"    },
+	{ NCSI_PKT_CMD_EV,     "EV"     },
+	{ NCSI_PKT_CMD_DV,     "DV"     },
+	{ NCSI_PKT_CMD_SMA,    "SMA"    },
+	{ NCSI_PKT_CMD_EBF,    "EBF"    },
+	{ NCSI_PKT_CMD_DBF,    "DBF"    },
+	{ NCSI_PKT_CMD_EGMF,   "EGMF"   },
+	{ NCSI_PKT_CMD_DGMF,   "DGMF"   },
+	{ NCSI_PKT_CMD_SNFC,   "SNFC"   },
+	{ NCSI_PKT_CMD_GVI,    "GVI"    },
+	{ NCSI_PKT_CMD_GC,     "GC"     },
+	{ NCSI_PKT_CMD_GP,     "GP"     },
+	{ NCSI_PKT_CMD_GCPS,   "GCPS"   },
+	{ NCSI_PKT_CMD_GNS,    "GNS"    },
+	{ NCSI_PKT_CMD_GNPTS,  "GNPTS"  },
+	{ NCSI_PKT_CMD_GPS,    "GPS"    },
+	{ NCSI_PKT_CMD_OEM,    "OEM"    },
+	{ NCSI_PKT_CMD_PLDM,   "PLDM"   },
+	{ NCSI_PKT_CMD_GPUUID, "GPUUID" },
+};
+
+static bool ncsi_dev_stats_index(struct ncsi_dev_priv *ndp, loff_t pos,
+				 unsigned long *type, unsigned long *index,
+				 unsigned long *entries)
+{
+	int i;
+	unsigned long ranges[3][2] = {
+		{ 1,
+		  ARRAY_SIZE(ndp->stats.cmd) - 1		},
+		{ ranges[0][1] + 2,
+		  ranges[1][0] + ARRAY_SIZE(ndp->stats.rsp) - 1	},
+		{ ranges[1][1] + 2,
+		  ranges[2][0] + ARRAY_SIZE(ndp->stats.aen) - 1 }
+	};
+
+	for (i = 0; i < 3; i++) {
+		if (pos == (ranges[i][0] - 1)) {
+			*index = i;
+			*entries = 0;
+			return true;
+		}
+
+		if (pos >= ranges[i][0] && pos <= ranges[i][1]) {
+			*type = i;
+			*index = (pos - ranges[i][0]);
+			*entries = NCSI_PKT_STAT_MAX;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static void *ncsi_dev_stats_data(struct ncsi_dev_priv *ndp, loff_t pos)
+{
+	unsigned long type, index, entries;
+	bool valid;
+
+	valid = ncsi_dev_stats_index(ndp, pos, &type, &index, &entries);
+	if (!valid)
+		return NULL;
+
+	/* The bits in return value are assigned as below:
+	 *
+	 * Bit[7:0]:   Number of ulong entries
+	 * Bit[23:8]:  Offset to that specific data entry
+	 * Bit[30:24]: Type of packet statistics
+	 * Bit[31]:    0x1 as valid flag.
+	 */
+	if (!entries)
+		index += ((unsigned long)SEQ_START_TOKEN);
+	else
+		index = (1 << 31) | (type << 24) | (index << 8) | entries;
+
+	return (void *)index;
+}
+
+static void *ncsi_dev_stats_seq_start(struct seq_file *seq, loff_t *pos)
+{
+	struct ncsi_dev_priv *ndp = seq->private;
+	void *data;
+
+	data = ncsi_dev_stats_data(ndp, *pos);
+	++(*pos);
+
+	return data;
+}
+
+static void *ncsi_dev_stats_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+{
+	struct ncsi_dev_priv *ndp = seq->private;
+	void *data;
+
+	data = ncsi_dev_stats_data(ndp, *pos);
+	++(*pos);
+	return data;
+}
+
+static void ncsi_dev_stats_seq_stop(struct seq_file *seq, void *v)
+{
+}
+
+static const char *ncsi_pkt_type_name(unsigned int type)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ncsi_pkt_handlers); i++) {
+		if (ncsi_pkt_handlers[i].type == type)
+			return ncsi_pkt_handlers[i].name;
+	}
+
+	return "N/A";
+}
+
+static const char *ncsi_dev_stats_pkt_name(unsigned long type,
+					   unsigned long index)
+{
+	switch (type) {
+	case 0: /* Command */
+	case 1: /* Response */
+		return ncsi_pkt_type_name(index);
+	case 2: /* AEN */
+		switch (index) {
+		case NCSI_PKT_AEN_LSC:
+			return "LSC";
+		case NCSI_PKT_AEN_CR:
+			return "CR";
+		case NCSI_PKT_AEN_HNCDSC:
+			return "HNCDSC";
+		default:
+			return "N/A";
+		}
+	}
+
+	return "N/A";
+}
+
+static int ncsi_dev_stats_seq_show(struct seq_file *seq, void *v)
+{
+	struct ncsi_dev_priv *ndp = seq->private;
+	unsigned long type, index, entries, *data;
+	const char *name;
+
+	if (v >= SEQ_START_TOKEN && v <= (SEQ_START_TOKEN + 2)) {
+		static const char * const header[] = { "CMD", "RSP", "AEN" };
+
+		seq_puts(seq, "\n");
+		seq_printf(seq, "%-12s %-8s %-8s %-8s\n",
+			   header[v - SEQ_START_TOKEN],
+			   "OK", "TIMEOUT", "ERROR");
+		seq_puts(seq, "=======================================\n");
+		return 0;
+	}
+
+	index = (unsigned long)v;
+	type = (index >> 24) & 0x7F;
+	entries = (index & 0xFF);
+	index = ((index >> 8) & 0xFFFF);
+	name = ncsi_dev_stats_pkt_name(type, index);
+	if (WARN_ON_ONCE(entries != NCSI_PKT_STAT_MAX))
+		return 0;
+
+	switch (type) {
+	case 0: /* Command */
+		data = &ndp->stats.cmd[0][0];
+		break;
+	case 1: /* Response */
+		data = &ndp->stats.rsp[0][0];
+		break;
+	case 2: /* AEN */
+		data = &ndp->stats.aen[0][0];
+		break;
+	default:
+		pr_warn("%s: Unsupported type %ld\n", __func__, type);
+		return 0;
+	}
+
+	data += (index * NCSI_PKT_STAT_MAX);
+	if (*data || *(data + 1) || *(data + 2)) {
+		seq_printf(seq, "%-12s %-8ld %-8ld %-8ld\n",
+			   name, *data, *(data + 1), *(data + 2));
+	}
+
+	return 0;
+}
+
+static const struct seq_operations ncsi_dev_stats_seq_ops = {
+	.start = ncsi_dev_stats_seq_start,
+	.next  = ncsi_dev_stats_seq_next,
+	.stop  = ncsi_dev_stats_seq_stop,
+	.show  = ncsi_dev_stats_seq_show,
+};
+
+static int ncsi_dev_stats_seq_open(struct inode *inode, struct file *file)
+{
+	struct seq_file *sf;
+	int ret;
+
+	ret = seq_open(file, &ncsi_dev_stats_seq_ops);
+	if (!ret) {
+		sf = file->private_data;
+		sf->private = inode->i_private;
+	}
+
+	return ret;
+}
+
+static const struct file_operations ncsi_dev_stats_fops = {
+	.owner   = THIS_MODULE,
+	.open    = ncsi_dev_stats_seq_open,
+	.read    = seq_read,
+	.llseek  = seq_lseek,
+	.release = seq_release,
+};
 
 int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
 {
@@ -42,11 +271,17 @@ int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
 	if (!ndp->dentry)
 		return -ENOMEM;
 
+	ndp->stats.dentry = debugfs_create_file("stats", 0400, ndp->dentry,
+						ndp, &ncsi_dev_stats_fops);
+	if (!ndp->stats.dentry)
+		return -ENOMEM;
+
 	return 0;
 }
 
 void ncsi_dev_release_debug(struct ncsi_dev_priv *ndp)
 {
+	debugfs_remove(ndp->stats.dentry);
 	debugfs_remove(ndp->dentry);
 }
 
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 84f1405..29063d5 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -530,6 +530,9 @@ static void ncsi_request_timeout(unsigned long data)
 	struct ncsi_request *nr = (struct ncsi_request *)data;
 	struct ncsi_dev_priv *ndp = nr->ndp;
 	unsigned long flags;
+#ifdef CONFIG_NET_NCSI_DEBUG
+	struct ncsi_pkt_hdr *hdr;
+#endif
 
 	/* If the request already had associated response,
 	 * let the response handler to release it.
@@ -542,6 +545,11 @@ static void ncsi_request_timeout(unsigned long data)
 	}
 	spin_unlock_irqrestore(&ndp->lock, flags);
 
+#ifdef CONFIG_NET_NCSI_DEBUG
+	hdr = (struct ncsi_pkt_hdr *)skb_network_header(nr->cmd);
+	ndp->stats.cmd[hdr->type][NCSI_PKT_STAT_TIMEOUT]++;
+#endif
+
 	/* Release the request */
 	ncsi_free_request(nr);
 }
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 087db77..30bd97e 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -998,6 +998,9 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	if (!nrh) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_ERROR]++;
+#endif
 		netdev_err(nd->dev, "Received unrecognized packet (0x%x)\n",
 			   hdr->type);
 		return -ENOENT;
@@ -1007,12 +1010,18 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 	spin_lock_irqsave(&ndp->lock, flags);
 	nr = &ndp->requests[hdr->id];
 	if (!nr->used) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_TIMEOUT]++;
+#endif
 		spin_unlock_irqrestore(&ndp->lock, flags);
 		return -ENODEV;
 	}
 
 	nr->rsp = skb;
 	if (!nr->enabled) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_TIMEOUT]++;
+#endif
 		spin_unlock_irqrestore(&ndp->lock, flags);
 		ret = -ENOENT;
 		goto out;
@@ -1024,11 +1033,21 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 	if (payload < 0)
 		payload = ntohs(hdr->length);
 	ret = ncsi_validate_rsp_pkt(nr, payload);
-	if (ret)
+	if (ret) {
+#ifdef CONFIG_NET_NCSI_DEBUG
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_ERROR]++;
+#endif
 		goto out;
+	}
 
 	/* Process the packet */
 	ret = nrh->handler(nr);
+#ifdef CONFIG_NET_NCSI_DEBUG
+	if (ret)
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_ERROR]++;
+	else
+		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_OK]++;
+#endif
 out:
 	ncsi_free_request(nr);
 	return ret;
-- 
2.7.4

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

* [PATCH v2 net-next 6/8] net/ncsi: Support NCSI packet generation
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
                   ` (4 preceding siblings ...)
  2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 7/8] net/ncsi: No error report on DP response to non-existing package Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 8/8] net/ncsi: Fix length of GVI response packet Gavin Shan
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

This introduces /sys/kernel/debug/ncsi/eth0/pkt. The debugfs entry
can accept parameters to produce NCSI command packet. The received
NCSI response packet is dumped on read. Below is an example to send
CIS command and dump its response.

   # echo CIS,0,0 > /sys/kernel/debug/ncsi/eth0/pkt
   # cat /sys/kernel/debug/ncsi/eth0/pkt
   NCSI response [CIS] packet received

   00 01 dd 80 00 0004 0000 0000

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/internal.h    |  13 +
 net/ncsi/ncsi-cmd.c    |   5 +
 net/ncsi/ncsi-debug.c  | 694 ++++++++++++++++++++++++++++++++++++++++++++++---
 net/ncsi/ncsi-manage.c |   3 +
 net/ncsi/ncsi-rsp.c    |  14 +-
 5 files changed, 698 insertions(+), 31 deletions(-)

diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 009c41d..3dbca46 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -221,6 +221,9 @@ struct ncsi_request {
 	bool                 used;    /* Request that has been assigned  */
 	unsigned int         flags;   /* NCSI request property           */
 #define NCSI_REQ_FLAG_EVENT_DRIVEN	1
+#ifdef CONFIG_NET_NCSI_DEBUG
+#define NCSI_REQ_FLAG_DEBUG		2
+#endif
 	struct ncsi_dev_priv *ndp;    /* Associated NCSI device          */
 	struct sk_buff       *cmd;    /* Associated NCSI command packet  */
 	struct sk_buff       *rsp;    /* Associated NCSI response packet */
@@ -285,6 +288,14 @@ struct ncsi_dev_priv {
 #ifdef CONFIG_NET_NCSI_DEBUG
 	struct {
 		struct dentry  *dentry;
+		unsigned int   req;
+#define NCSI_PKT_REQ_FREE      0
+#define NCSI_PKT_REQ_BUSY      0xFFFFFFFF
+		int            errno;
+		struct sk_buff *rsp;
+	} pkt;
+	struct {
+		struct dentry  *dentry;
 #define NCSI_PKT_STAT_OK	0
 #define NCSI_PKT_STAT_TIMEOUT	1
 #define NCSI_PKT_STAT_ERROR	2
@@ -364,6 +375,8 @@ int ncsi_package_init_debug(struct ncsi_package *np);
 void ncsi_package_release_debug(struct ncsi_package *np);
 int ncsi_channel_init_debug(struct ncsi_channel *nc);
 void ncsi_channel_release_debug(struct ncsi_channel *nc);
+void ncsi_dev_reset_pkt_debug(struct ncsi_dev_priv *ndp,
+			      struct sk_buff *skb, int errno);
 #else
 static inline int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
 {
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index c6b2bc5..3455465 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -365,6 +365,11 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
 	nr->enabled = true;
 	mod_timer(&nr->timer, jiffies + 1 * HZ);
 
+#ifdef CONFIG_NET_NCSI_DEBUG
+	if (nr->flags & NCSI_REQ_FLAG_DEBUG)
+		nca->ndp->pkt.req = nr->id;
+#endif
+
 	/* Send NCSI packet */
 	skb_get(nr->cmd);
 	ret = dev_queue_xmit(nr->cmd);
diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
index 29c233c..a0fcc58 100644
--- a/net/ncsi/ncsi-debug.c
+++ b/net/ncsi/ncsi-debug.c
@@ -23,40 +23,668 @@
 #include "ncsi-pkt.h"
 
 static struct dentry *ncsi_dentry;
+
+static const char *ncsi_pkt_type_name(unsigned int type);
+
+static int ncsi_pkt_input_default(struct ncsi_dev_priv *ndp,
+				  struct ncsi_cmd_arg *nca, char *buf)
+{
+	return 0;
+}
+
+static int ncsi_pkt_input_params(char *buf, int *outval, int count)
+{
+	int num, i;
+
+	for (i = 0; i < count; i++, outval++) {
+		if (sscanf(buf, "%x%n", outval, &num) != 1)
+			return -EINVAL;
+
+		if (buf[num] == ',')
+			buf += (count + 1);
+		else
+			buf += count;
+	}
+
+	return 0;
+}
+
+static int ncsi_pkt_input_sp(struct ncsi_dev_priv *ndp,
+			     struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* The hardware arbitration will be configured according
+	 * to the NCSI's capability if it's not specified.
+	 */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (!ret && param != 0 && param != 1)
+		return -EINVAL;
+	else if (ret)
+		param = (ndp->flags & NCSI_DEV_HWA) ? 1 : 0;
+
+	nca->bytes[0] = param;
+
+	return 0;
+}
+
+static int ncsi_pkt_input_dc(struct ncsi_dev_priv *ndp,
+			     struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* Allow link down will be disallowed if it's not specified */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (!ret && param != 0 && param != 1)
+		return -EINVAL;
+	else if (ret)
+		param = 0;
+
+	nca->bytes[0] = param;
+
+	return 0;
+}
+
+static int ncsi_pkt_input_ae(struct ncsi_dev_priv *ndp,
+			     struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param[2], ret;
+
+	/* MC ID and AE mode are mandatory */
+	ret = ncsi_pkt_input_params(buf, param, 2);
+	if (ret)
+		return -EINVAL;
+
+	nca->bytes[0] = param[0];
+	nca->dwords[1] = param[1];
+
+	return 0;
+}
+
+static int ncsi_pkt_input_sl(struct ncsi_dev_priv *ndp,
+			     struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param[2], ret;
+
+	/* Link mode and OEM mode are mandatory */
+	ret = ncsi_pkt_input_params(buf, param, 2);
+	if (ret)
+		return -EINVAL;
+
+	nca->dwords[0] = param[0];
+	nca->dwords[1] = param[1];
+
+	return 0;
+}
+
+static int ncsi_pkt_input_svf(struct ncsi_dev_priv *ndp,
+			      struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param[3], ret;
+
+	/* VLAN ID, table index and enable */
+	ret = ncsi_pkt_input_params(buf, param, 3);
+	if (ret)
+		return -EINVAL;
+
+	if (param[2] != 0 && param[2] != 1)
+		return -EINVAL;
+
+	nca->words[0] = param[0];
+	nca->bytes[2] = param[1];
+	nca->bytes[3] = param[2];
+
+	return 0;
+}
+
+static int ncsi_pkt_input_ev(struct ncsi_dev_priv *ndp,
+			     struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* VLAN filter mode */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (ret)
+		return -EINVAL;
+
+	nca->bytes[0] = param;
+
+	return 0;
+}
+
+static int ncsi_pkt_input_sma(struct ncsi_dev_priv *ndp,
+			      struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param[8], ret;
+
+	/* MAC address, MAC table index, Address type and operation */
+	ret = ncsi_pkt_input_params(buf, param, 8);
+	if (ret)
+		return -EINVAL;
+
+	if (param[7] & ~0x9)
+		return -EINVAL;
+
+	nca->bytes[0] = param[0];
+	nca->bytes[1] = param[1];
+	nca->bytes[2] = param[2];
+	nca->bytes[3] = param[3];
+	nca->bytes[4] = param[4];
+	nca->bytes[5] = param[5];
+
+	nca->bytes[6] = param[6];
+	nca->bytes[7] = param[7];
+
+	return 0;
+}
+
+static int ncsi_pkt_input_ebf(struct ncsi_dev_priv *ndp,
+			      struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* Broadcast filter mode */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (ret)
+		return -EINVAL;
+
+	nca->dwords[0] = param;
+
+	return 0;
+}
+
+static int ncsi_pkt_input_egmf(struct ncsi_dev_priv *ndp,
+			       struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* Global multicast filter mode */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (ret)
+		return -EINVAL;
+
+	nca->dwords[0] = param;
+
+	return 0;
+}
+
+static int ncsi_pkt_input_snfc(struct ncsi_dev_priv *ndp,
+			       struct ncsi_cmd_arg *nca, char *buf)
+{
+	int param, ret;
+
+	/* NCSI flow control mode */
+	ret = ncsi_pkt_input_params(buf, &param, 1);
+	if (ret)
+		return -EINVAL;
+
+	nca->bytes[0] = param;
+
+	return 0;
+}
+
+static void ncsi_pkt_output_header(struct ncsi_dev_priv *ndp,
+				   struct seq_file *seq,
+				   struct ncsi_rsp_pkt_hdr *h)
+{
+	seq_printf(seq, "NCSI response [%s] packet received\n\n",
+		   ncsi_pkt_type_name(h->common.type - 0x80));
+	seq_printf(seq, "%02x %02x %02x %02x %02x %04x %04x %04x\n",
+		   h->common.mc_id, h->common.revision, h->common.id,
+		   h->common.type, h->common.channel, ntohs(h->common.length),
+		   ntohs(h->code), ntohs(h->reason));
+}
+
+static int ncsi_pkt_output_default(struct ncsi_dev_priv *ndp,
+				   struct seq_file *seq,
+				   struct sk_buff *skb)
+{
+	struct ncsi_rsp_pkt_hdr *hdr;
+
+	hdr = (struct ncsi_rsp_pkt_hdr *)skb_network_header(skb);
+	ncsi_pkt_output_header(ndp, seq, hdr);
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gls(struct ncsi_dev_priv *ndp,
+			       struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gls_pkt *gls;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gls = (struct ncsi_rsp_gls_pkt *)skb_network_header(skb);
+	seq_printf(seq, "Status: %08x Other: %08x OEM: %08x\n",
+		   ntohl(gls->status), ntohl(gls->other),
+		   ntohl(gls->oem_status));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gvi(struct ncsi_dev_priv *ndp,
+			       struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gvi_pkt *gvi;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gvi = (struct ncsi_rsp_gvi_pkt *)skb_network_header(skb);
+	seq_printf(seq, "NCSI Version: %08x Alpha2: %02x\n",
+		   ntohl(gvi->ncsi_version), gvi->alpha2);
+	seq_printf(seq, "Firmware: %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x Version: %08x\n",
+		   gvi->fw_name[0], gvi->fw_name[1], gvi->fw_name[2],
+		   gvi->fw_name[3], gvi->fw_name[4], gvi->fw_name[5],
+		   gvi->fw_name[6], gvi->fw_name[7], gvi->fw_name[8],
+		   gvi->fw_name[9], gvi->fw_name[10], gvi->fw_name[11],
+		   ntohl(gvi->fw_version));
+	seq_printf(seq, "PCI: %04x %04x %04x %04x Manufacture ID: %08x\n",
+		   ntohs(gvi->pci_ids[0]), ntohs(gvi->pci_ids[1]),
+		   ntohs(gvi->pci_ids[2]), ntohs(gvi->pci_ids[3]),
+		   ntohl(gvi->mf_id));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gc(struct ncsi_dev_priv *ndp,
+			      struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gc_pkt *gc;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gc = (struct ncsi_rsp_gc_pkt *)skb_network_header(skb);
+	seq_printf(seq, "Cap: %08x BC: %08x MC: %08x Buf: %08x AEN: %08x\n",
+		   ntohl(gc->cap), ntohl(gc->bc_cap), ntohl(gc->mc_cap),
+		   ntohl(gc->buf_cap), ntohl(gc->aen_cap));
+	seq_printf(seq, "VLAN: %02x Mixed: %02x MC: %02x UC: %02x\n",
+		   gc->vlan_cnt, gc->mixed_cnt, gc->mc_cnt, gc->uc_cnt);
+	seq_printf(seq, "VLAN Mode: %02x Channels: %02x\n",
+		   gc->vlan_mode, gc->channel_cnt);
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gp(struct ncsi_dev_priv *ndp,
+			      struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gp_pkt *gp;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gp = (struct ncsi_rsp_gp_pkt *)skb_network_header(skb);
+	seq_printf(seq, "MAC: %02x %02x VLAN: %02x %04x\n",
+		   gp->mac_cnt, gp->mac_enable, gp->vlan_cnt,
+		   ntohs(gp->vlan_enable));
+	seq_printf(seq, "Link: %08x BC: %08x Valid: %08x\n",
+		   ntohl(gp->link_mode), ntohl(gp->bc_mode),
+		   ntohl(gp->valid_modes));
+	seq_printf(seq, "VLAN: %02x FC: %02x AEN: %08x\n",
+		   gp->vlan_mode, gp->fc_mode, ntohl(gp->aen_mode));
+	seq_printf(seq, "MAC: %02x %02x %02x %02x %02x %02x VLAN: %04x\n",
+		   gp->mac[5], gp->mac[4], gp->mac[3], gp->mac[2],
+		   gp->mac[1], gp->mac[0], ntohs(gp->vlan));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gcps(struct ncsi_dev_priv *ndp,
+				struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gcps_pkt *gcps;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gcps = (struct ncsi_rsp_gcps_pkt *)skb_network_header(skb);
+	seq_printf(seq, "cnt_hi: %08x cnt_lo: %08x rx_bytes: %08x\n",
+		   ntohl(gcps->cnt_hi), ntohl(gcps->cnt_lo),
+		   ntohl(gcps->rx_bytes));
+	seq_printf(seq, "tx_bytes: %08x rx_uc_pkts: %08x rx_mc_pkts: %08x\n",
+		   ntohl(gcps->tx_bytes), ntohl(gcps->rx_uc_pkts),
+		   ntohl(gcps->rx_mc_pkts));
+	seq_printf(seq, "rx_bc_pkts: %08x tx_uc_pkts: %08x tx_mc_pkts: %08x\n",
+		   ntohl(gcps->rx_bc_pkts), ntohl(gcps->tx_uc_pkts),
+		   ntohl(gcps->tx_mc_pkts));
+	seq_printf(seq, "tx_bc_pkts: %08x fcs_err: %08x align_err: %08x\n",
+		   ntohl(gcps->tx_bc_pkts), ntohl(gcps->fcs_err),
+		   ntohl(gcps->align_err));
+	seq_printf(seq, "false_carrier: %08x runt_pkts: %08x jabber_pkts: %08x\n",
+		   ntohl(gcps->false_carrier), ntohl(gcps->runt_pkts),
+		   ntohl(gcps->jabber_pkts));
+	seq_printf(seq, "rx_pause_xon: %08x rx_pause_xoff: %08x tx_pause_xon: %08x",
+		   ntohl(gcps->rx_pause_xon), ntohl(gcps->rx_pause_xoff),
+		   ntohl(gcps->tx_pause_xon));
+	seq_printf(seq, "tx_pause_xoff: %08x tx_s_collision: %08x tx_m_collision: %08x\n",
+		   ntohl(gcps->tx_pause_xoff), ntohl(gcps->tx_s_collision),
+		   ntohl(gcps->tx_m_collision));
+	seq_printf(seq, "l_collision: %08x e_collision: %08x rx_ctl_frames: %08x\n",
+		   ntohl(gcps->l_collision), ntohl(gcps->e_collision),
+		   ntohl(gcps->rx_ctl_frames));
+	seq_printf(seq, "rx_64_frames: %08x rx_127_frames: %08x rx_255_frames: %08x\n",
+		   ntohl(gcps->rx_64_frames), ntohl(gcps->rx_127_frames),
+		   ntohl(gcps->rx_255_frames));
+	seq_printf(seq, "rx_511_frames: %08x rx_1023_frames: %08x rx_1522_frames: %08x\n",
+		   ntohl(gcps->rx_511_frames), ntohl(gcps->rx_1023_frames),
+		   ntohl(gcps->rx_1522_frames));
+	seq_printf(seq, "rx_9022_frames: %08x tx_64_frames: %08x tx_127_frames: %08x\n",
+		   ntohl(gcps->rx_9022_frames), ntohl(gcps->tx_64_frames),
+		   ntohl(gcps->tx_127_frames));
+	seq_printf(seq, "tx_255_frames: %08x tx_511_frames: %08x tx_1023_frames: %08x\n",
+		   ntohl(gcps->tx_255_frames), ntohl(gcps->tx_511_frames),
+		   ntohl(gcps->tx_1023_frames));
+	seq_printf(seq, "tx_1522_frames: %08x tx_9022_frames: %08x rx_valid_bytes: %08x\n",
+		   ntohl(gcps->tx_1522_frames), ntohl(gcps->tx_9022_frames),
+		   ntohl(gcps->rx_valid_bytes));
+	seq_printf(seq, "rx_runt_pkts: %08x rx_jabber_pkts: %08x\n",
+		   ntohl(gcps->rx_runt_pkts), ntohl(gcps->rx_jabber_pkts));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gns(struct ncsi_dev_priv *ndp,
+			       struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gns_pkt *gns;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gns = (struct ncsi_rsp_gns_pkt *)skb_network_header(skb);
+	seq_printf(seq, "rx_cmds: %08x dropped_cmds: %08x cmd_type_errs: %08x\n",
+		   ntohl(gns->rx_cmds), ntohl(gns->dropped_cmds),
+		   ntohl(gns->cmd_type_errs));
+	seq_printf(seq, "cmd_csum_errs: %08x rx_pkts: %08x tx_pkts: %08x\n",
+		   ntohl(gns->cmd_csum_errs), ntohl(gns->rx_pkts),
+		   ntohl(gns->tx_pkts));
+	seq_printf(seq, "tx_aen_pkts: %08x\n",
+		   ntohl(gns->tx_aen_pkts));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gnpts(struct ncsi_dev_priv *ndp,
+				 struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gnpts_pkt *gnpts;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gnpts = (struct ncsi_rsp_gnpts_pkt *)skb_network_header(skb);
+	seq_printf(seq, "tx_pkts: %08x tx_dropped: %08x tx_channel_err: %08x\n",
+		   ntohl(gnpts->tx_pkts), ntohl(gnpts->tx_dropped),
+		   ntohl(gnpts->tx_channel_err));
+	seq_printf(seq, "tx_us_err: %08x rx_pkts: %08x rx_dropped: %08x\n",
+		   ntohl(gnpts->tx_us_err), ntohl(gnpts->rx_pkts),
+		   ntohl(gnpts->rx_dropped));
+	seq_printf(seq, "rx_channel_err: %08x rx_us_err: %08x rx_os_err: %08x\n",
+		   ntohl(gnpts->rx_channel_err), ntohl(gnpts->rx_us_err),
+		   ntohl(gnpts->rx_os_err));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gps(struct ncsi_dev_priv *ndp,
+			       struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gps_pkt *gps;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gps = (struct ncsi_rsp_gps_pkt *)skb_network_header(skb);
+	seq_printf(seq, "Status: %08x\n", ntohl(gps->status));
+
+	return 0;
+}
+
+static int ncsi_pkt_output_gpuuid(struct ncsi_dev_priv *ndp,
+				  struct seq_file *seq, struct sk_buff *skb)
+{
+	struct ncsi_rsp_gpuuid_pkt *gpuuid;
+
+	ncsi_pkt_output_default(ndp, seq, skb);
+
+	gpuuid = (struct ncsi_rsp_gpuuid_pkt *)skb_network_header(skb);
+	seq_printf(seq, "UUID: %02x %02x %02x %02x %02x %02x %02x %02x\n",
+		   gpuuid->uuid[15], gpuuid->uuid[14], gpuuid->uuid[13],
+		   gpuuid->uuid[12], gpuuid->uuid[11], gpuuid->uuid[10],
+		   gpuuid->uuid[9],  gpuuid->uuid[8]);
+	seq_printf(seq, "      %02x %02x %02x %02x %02x %02x %02x %02x\n",
+		   gpuuid->uuid[7], gpuuid->uuid[6], gpuuid->uuid[5],
+		   gpuuid->uuid[4], gpuuid->uuid[3], gpuuid->uuid[2],
+		   gpuuid->uuid[1], gpuuid->uuid[0]);
+
+	return 0;
+}
+
 static struct ncsi_pkt_handler {
 	unsigned char	type;
 	const char	*name;
+	int		(*input)(struct ncsi_dev_priv *ndp,
+				 struct ncsi_cmd_arg *nca, char *buf);
+	int		(*output)(struct ncsi_dev_priv *ndp,
+				  struct seq_file *seq, struct sk_buff *skb);
 } ncsi_pkt_handlers[] = {
-	{ NCSI_PKT_CMD_CIS,    "CIS"    },
-	{ NCSI_PKT_CMD_SP,     "SP"     },
-	{ NCSI_PKT_CMD_DP,     "DP"     },
-	{ NCSI_PKT_CMD_EC,     "EC"     },
-	{ NCSI_PKT_CMD_DC,     "DC"     },
-	{ NCSI_PKT_CMD_RC,     "RC"     },
-	{ NCSI_PKT_CMD_ECNT,   "ECNT"   },
-	{ NCSI_PKT_CMD_DCNT,   "DCNT"   },
-	{ NCSI_PKT_CMD_AE,     "AE"     },
-	{ NCSI_PKT_CMD_SL,     "SL"     },
-	{ NCSI_PKT_CMD_GLS,    "GLS"    },
-	{ NCSI_PKT_CMD_SVF,    "SVF"    },
-	{ NCSI_PKT_CMD_EV,     "EV"     },
-	{ NCSI_PKT_CMD_DV,     "DV"     },
-	{ NCSI_PKT_CMD_SMA,    "SMA"    },
-	{ NCSI_PKT_CMD_EBF,    "EBF"    },
-	{ NCSI_PKT_CMD_DBF,    "DBF"    },
-	{ NCSI_PKT_CMD_EGMF,   "EGMF"   },
-	{ NCSI_PKT_CMD_DGMF,   "DGMF"   },
-	{ NCSI_PKT_CMD_SNFC,   "SNFC"   },
-	{ NCSI_PKT_CMD_GVI,    "GVI"    },
-	{ NCSI_PKT_CMD_GC,     "GC"     },
-	{ NCSI_PKT_CMD_GP,     "GP"     },
-	{ NCSI_PKT_CMD_GCPS,   "GCPS"   },
-	{ NCSI_PKT_CMD_GNS,    "GNS"    },
-	{ NCSI_PKT_CMD_GNPTS,  "GNPTS"  },
-	{ NCSI_PKT_CMD_GPS,    "GPS"    },
-	{ NCSI_PKT_CMD_OEM,    "OEM"    },
-	{ NCSI_PKT_CMD_PLDM,   "PLDM"   },
-	{ NCSI_PKT_CMD_GPUUID, "GPUUID" },
+	{ NCSI_PKT_CMD_CIS,    "CIS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_SP,     "SP",
+	  ncsi_pkt_input_sp,      ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DP,     "DP",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_EC,     "EC",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DC,     "DC",
+	  ncsi_pkt_input_dc,      ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_RC,     "RC",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_ECNT,   "ECNT",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DCNT,   "DCNT",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_AE,     "AE",
+	  ncsi_pkt_input_ae,      ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_SL,     "SL",
+	  ncsi_pkt_input_sl,      ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_GLS,    "GLS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gls     },
+	{ NCSI_PKT_CMD_SVF,    "SVF",
+	  ncsi_pkt_input_svf,     ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_EV,     "EV",
+	  ncsi_pkt_input_ev,      ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DV,     "DV",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_SMA,    "SMA",
+	  ncsi_pkt_input_sma,     ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_EBF,    "EBF",
+	  ncsi_pkt_input_ebf,     ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DBF,    "DBF",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_EGMF,   "EGMF",
+	  ncsi_pkt_input_egmf,    ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_DGMF,   "DGMF",
+	  ncsi_pkt_input_default, ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_SNFC,   "SNFC",
+	  ncsi_pkt_input_snfc,    ncsi_pkt_output_default },
+	{ NCSI_PKT_CMD_GVI,    "GVI",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gvi     },
+	{ NCSI_PKT_CMD_GC,     "GC",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gc      },
+	{ NCSI_PKT_CMD_GP,     "GP",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gp      },
+	{ NCSI_PKT_CMD_GCPS,   "GCPS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gcps    },
+	{ NCSI_PKT_CMD_GNS,    "GNS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gns     },
+	{ NCSI_PKT_CMD_GNPTS,  "GNPTS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gnpts   },
+	{ NCSI_PKT_CMD_GPS,    "GPS",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gps     },
+	{ NCSI_PKT_CMD_OEM,    "OEM",
+	  NULL,                   NULL                    },
+	{ NCSI_PKT_CMD_PLDM,   "PLDM",
+	  NULL,                   NULL                    },
+	{ NCSI_PKT_CMD_GPUUID, "GPUUID",
+	  ncsi_pkt_input_default, ncsi_pkt_output_gpuuid  },
+};
+
+static int ncsi_dev_pkt_seq_show(struct seq_file *seq, void *v)
+{
+	struct ncsi_dev_priv *ndp = seq->private;
+	struct ncsi_pkt_handler *h = NULL;
+	struct ncsi_pkt_hdr *hdr;
+	struct sk_buff *skb;
+	int i, errno;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ndp->lock, flags);
+	errno = ndp->pkt.errno;
+	skb = ndp->pkt.rsp;
+	ndp->pkt.rsp = NULL;
+	spin_unlock_irqrestore(&ndp->lock, flags);
+	ncsi_dev_reset_pkt_debug(ndp, NULL, 0);
+
+	if (errno) {
+		WARN_ON(skb);
+		seq_printf(seq, "Error %d receiving response packet\n", errno);
+		return 0;
+	} else if (!skb) {
+		seq_puts(seq, "No available response packet\n");
+		return 0;
+	}
+
+	hdr = (struct ncsi_pkt_hdr *)skb_network_header(skb);
+	for (i = 0; i < ARRAY_SIZE(ncsi_pkt_handlers); i++) {
+		if (ncsi_pkt_handlers[i].type == (hdr->type - 0x80)) {
+			h = &ncsi_pkt_handlers[i];
+			break;
+		}
+	}
+
+	if (!h || !h->output) {
+		consume_skb(skb);
+		return 0;
+	}
+
+	h->output(ndp, seq, skb);
+	return 0;
+}
+
+static int ncsi_dev_pkt_seq_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, ncsi_dev_pkt_seq_show, inode->i_private);
+}
+
+static ssize_t ncsi_dev_pkt_seq_write(struct file *file,
+				      const char __user *buffer,
+				      size_t count, loff_t *pos)
+{
+	struct seq_file *seq = file->private_data;
+	struct ncsi_dev_priv *ndp = seq->private;
+	struct ncsi_cmd_arg nca;
+	char buf[64], name[64], *pbuf;
+	struct ncsi_pkt_handler *h;
+	int num, package, channel, i, ret;
+	unsigned long flags;
+
+	if (count >= sizeof(buf))
+		return -EINVAL;
+
+	/* Copy the buffer from user space. Currently we have 64 bytes as
+	 * the length limitation. It should be enough as there are no bunch
+	 * of parameters to be specified when sending NCSI command packet.
+	 */
+	memset(buf, 0, sizeof(buf));
+	if (copy_from_user(buf, buffer, count))
+		return -EFAULT;
+
+	/* Extract the specified command */
+	memset(name, 0, sizeof(name));
+	pbuf = strchr(buf, ',');
+	if (!pbuf)
+		return -EINVAL;
+	memcpy(name, buf, pbuf - buf);
+	pbuf++;
+
+	/* Extract mandatory parameters: package and channel ID */
+	memset(&nca, 0, sizeof(struct ncsi_cmd_arg));
+	if (sscanf(pbuf, "%x,%x%n", &package, &channel, &num) != 2)
+		return -EINVAL;
+	if (package < 0 || package >= 8 ||
+	    channel < 0 || channel > NCSI_RESERVED_CHANNEL)
+		return -EINVAL;
+
+	nca.package = package;
+	nca.channel = channel;
+	if (pbuf[num] == ',')
+		pbuf += (count + 1);
+	else
+		pbuf += count;
+
+	/* Search for handler */
+	h = NULL;
+	for (i = 0; i < ARRAY_SIZE(ncsi_pkt_handlers); i++) {
+		if (!strcmp(ncsi_pkt_handlers[i].name, name)) {
+		h = &ncsi_pkt_handlers[i];
+			nca.type = h->type;
+			break;
+		}
+	}
+
+	if (!h || !h->input)
+		return -ERANGE;
+
+	/* Sort out additional parameters */
+	nca.ndp = ndp;
+	nca.req_flags = NCSI_REQ_FLAG_DEBUG;
+	ret = h->input(ndp, &nca, pbuf);
+	if (ret)
+		return ret;
+
+	/* This interface works in serialized fashion, meaning new command
+	 * cannot be sent until previous one has been finalized.
+	 */
+	spin_lock_irqsave(&ndp->lock, flags);
+	if (ndp->pkt.req != NCSI_PKT_REQ_FREE) {
+		spin_unlock_irqrestore(&ndp->lock, flags);
+		return -EBUSY;
+	}
+
+	ndp->pkt.req = NCSI_PKT_REQ_BUSY;
+	spin_unlock_irqrestore(&ndp->lock, flags);
+
+	ret = ncsi_xmit_cmd(&nca);
+	if (ret) {
+		spin_lock_irqsave(&ndp->lock, flags);
+		ndp->pkt.req = NCSI_PKT_REQ_FREE;
+		spin_unlock_irqrestore(&ndp->lock, flags);
+		return ret;
+	}
+
+	return count;
+}
+
+void ncsi_dev_reset_pkt_debug(struct ncsi_dev_priv *ndp,
+			      struct sk_buff *skb, int errno)
+{
+	unsigned long flags;
+	struct sk_buff *old;
+
+	spin_lock_irqsave(&ndp->lock, flags);
+	ndp->pkt.req = NCSI_PKT_REQ_FREE;
+	ndp->pkt.errno = errno;
+
+	old = ndp->pkt.rsp;
+	ndp->pkt.rsp = skb;
+	spin_unlock_irqrestore(&ndp->lock, flags);
+
+	consume_skb(old);
+}
+
+static const struct file_operations ncsi_dev_pkt_fops = {
+	.owner   = THIS_MODULE,
+	.open    = ncsi_dev_pkt_seq_open,
+	.read    = seq_read,
+	.write   = ncsi_dev_pkt_seq_write,
+	.llseek  = seq_lseek,
+	.release = seq_release,
 };
 
 static bool ncsi_dev_stats_index(struct ncsi_dev_priv *ndp, loff_t pos,
@@ -276,11 +904,17 @@ int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
 	if (!ndp->stats.dentry)
 		return -ENOMEM;
 
+	ndp->pkt.dentry = debugfs_create_file("pkt", 0600, ndp->dentry,
+					      ndp, &ncsi_dev_pkt_fops);
+	if (!ndp->pkt.dentry)
+		return -ENOMEM;
+
 	return 0;
 }
 
 void ncsi_dev_release_debug(struct ncsi_dev_priv *ndp)
 {
+	debugfs_remove(ndp->pkt.dentry);
 	debugfs_remove(ndp->stats.dentry);
 	debugfs_remove(ndp->dentry);
 }
diff --git a/net/ncsi/ncsi-manage.c b/net/ncsi/ncsi-manage.c
index 29063d5..823c7dc 100644
--- a/net/ncsi/ncsi-manage.c
+++ b/net/ncsi/ncsi-manage.c
@@ -548,6 +548,9 @@ static void ncsi_request_timeout(unsigned long data)
 #ifdef CONFIG_NET_NCSI_DEBUG
 	hdr = (struct ncsi_pkt_hdr *)skb_network_header(nr->cmd);
 	ndp->stats.cmd[hdr->type][NCSI_PKT_STAT_TIMEOUT]++;
+
+	if ((nr->flags & NCSI_REQ_FLAG_DEBUG) && ndp->pkt.req == nr->id)
+		ncsi_dev_reset_pkt_debug(ndp, NULL, -ETIMEDOUT);
 #endif
 
 	/* Release the request */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 30bd97e..93ebe0f 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -1036,17 +1036,29 @@ int ncsi_rcv_rsp(struct sk_buff *skb, struct net_device *dev,
 	if (ret) {
 #ifdef CONFIG_NET_NCSI_DEBUG
 		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_ERROR]++;
+		if ((nr->flags & NCSI_REQ_FLAG_DEBUG) &&
+		    (nr->id == ndp->pkt.req))
+			ncsi_dev_reset_pkt_debug(ndp, NULL, -EINVAL);
 #endif
 		goto out;
 	}
 
 	/* Process the packet */
-	ret = nrh->handler(nr);
 #ifdef CONFIG_NET_NCSI_DEBUG
+	if ((nr->flags & NCSI_REQ_FLAG_DEBUG) &&
+	    (nr->id == ndp->pkt.req)) {
+		ncsi_dev_reset_pkt_debug(ndp, nr->rsp, 0);
+		nr->rsp = NULL;
+		goto out;
+	}
+
+	ret = nrh->handler(nr);
 	if (ret)
 		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_ERROR]++;
 	else
 		ndp->stats.rsp[hdr->type - 0x80][NCSI_PKT_STAT_OK]++;
+#else
+	ret = nrh->handler(nr);
 #endif
 out:
 	ncsi_free_request(nr);
-- 
2.7.4

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

* [PATCH v2 net-next 7/8] net/ncsi: No error report on DP response to non-existing package
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
                   ` (5 preceding siblings ...)
  2017-04-13  7:48 ` [PATCH v2 net-next 6/8] net/ncsi: Support NCSI packet generation Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  2017-04-13  7:48 ` [PATCH v2 net-next 8/8] net/ncsi: Fix length of GVI response packet Gavin Shan
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

The issue was found from /sys/kernel/debug/ncsi/eth0/stats. The
first step in NCSI package/channel enumeration is deselect all
packages by sending DP (Deselect Package) commands. The remote
NIC replies with response while the corresponding package isn't
populated yet and it is treated as an error wrongly.

 # cat /sys/kernel/debug/ncsi/eth0/stats
 :
 RSP          OK       TIMEOUT  ERROR
 =======================================
 CIS          3        0        0
 SP           3        0        0
 DP           2        0        1

This fixes the issue by ignoring the error in DP response handler,
when the corresponding package isn't existing. With this applied,
no error reported from DP response packets.

 # cat /sys/kernel/debug/ncsi/eth0/stats
 :
 RSP          OK       TIMEOUT  ERROR
 =======================================
 CIS          3        0        0
 SP           3        0        0
 DP           3        0        0

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-rsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 93ebe0f..095726f 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -118,7 +118,7 @@ static int ncsi_rsp_handler_dp(struct ncsi_request *nr)
 	ncsi_find_package_and_channel(ndp, rsp->rsp.common.channel,
 				      &np, NULL);
 	if (!np)
-		return -ENODEV;
+		return 0;
 
 	/* Change state of all channels attached to the package */
 	NCSI_FOR_EACH_CHANNEL(np, nc) {
-- 
2.7.4

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

* [PATCH v2 net-next 8/8] net/ncsi: Fix length of GVI response packet
  2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
                   ` (6 preceding siblings ...)
  2017-04-13  7:48 ` [PATCH v2 net-next 7/8] net/ncsi: No error report on DP response to non-existing package Gavin Shan
@ 2017-04-13  7:48 ` Gavin Shan
  7 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-13  7:48 UTC (permalink / raw)
  To: netdev; +Cc: joe, davem, Gavin Shan

The length of GVI (GetVersionInfo) response packet should be 40 instead
of 36. This issue was found from /sys/kernel/debug/ncsi/eth0/stats.

 # cat /sys/kernel/debug/ncsi/eth0/stats
 :
 RSP          OK       TIMEOUT  ERROR
 =======================================
 GVI          0        0        2

With this applied, no error reported on GVI response packets:

 # cat /sys/kernel/debug/ncsi/eth0/stats
 :
 RSP          OK       TIMEOUT  ERROR
 =======================================
 GVI          2        0        0

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 net/ncsi/ncsi-rsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 095726f..67260f3 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -951,7 +951,7 @@ static struct ncsi_rsp_handler {
 	{ NCSI_PKT_RSP_EGMF,    4, ncsi_rsp_handler_egmf    },
 	{ NCSI_PKT_RSP_DGMF,    4, ncsi_rsp_handler_dgmf    },
 	{ NCSI_PKT_RSP_SNFC,    4, ncsi_rsp_handler_snfc    },
-	{ NCSI_PKT_RSP_GVI,    36, ncsi_rsp_handler_gvi     },
+	{ NCSI_PKT_RSP_GVI,    40, ncsi_rsp_handler_gvi     },
 	{ NCSI_PKT_RSP_GC,     32, ncsi_rsp_handler_gc      },
 	{ NCSI_PKT_RSP_GP,     -1, ncsi_rsp_handler_gp      },
 	{ NCSI_PKT_RSP_GCPS,  172, ncsi_rsp_handler_gcps    },
-- 
2.7.4

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

* Re: [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre
  2017-04-13  7:48 ` [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre Gavin Shan
@ 2017-04-13 10:41   ` Joe Perches
  2017-04-17 23:22     ` Gavin Shan
  0 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2017-04-13 10:41 UTC (permalink / raw)
  To: Gavin Shan, netdev; +Cc: davem

On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
> This creates debugfs directories as NCSI debugging infrastructure.
> With the patch applied, We will see below debugfs directories. Every
> NCSI package and channel has one corresponding directory. Other than
> presenting the NCSI topology, No real function has been achieved
> through these debugfs directories so far.
> 
>      /sys/kernel/debug/ncsi/eth0
>      /sys/kernel/debug/ncsi/eth0/p0
>      /sys/kernel/debug/ncsi/eth0/p0/c0
>      /sys/kernel/debug/ncsi/eth0/p0/c1

[]

> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
[]
> +int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
> +{
> +	if (WARN_ON_ONCE(ndp->dentry))
> +		return 0;
> +
> +	if (!ncsi_dentry) {
> +		ncsi_dentry = debugfs_create_dir("ncsi", NULL);
> +		if (!ncsi_dentry) {
> +			pr_warn("NCSI: Cannot create /sys/kernel/debug/ncsi\n");
> +			return -ENOENT;

debugfs does not have a fixed path.

Most error messages for this just use something like
	pr_<level>("Failed to create debugfs directory '%s'\n", foo)
And most failures don't emit any error message at all.

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
@ 2017-04-13 10:50   ` Joe Perches
  2017-04-17 23:23     ` Gavin Shan
  2017-04-14  1:50   ` Jakub Kicinski
  2017-04-14  2:30   ` Joe Perches
  2 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2017-04-13 10:50 UTC (permalink / raw)
  To: Gavin Shan, netdev; +Cc: davem

On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
> packets sent and received over all packages and channels. It's useful
> to diagnose NCSI problems, especially when NCSI packages and channels
> aren't probed properly.

trivia:

> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
> index 6c00e9b..29c233c 100644
> --- a/net/ncsi/ncsi-debug.c
> +++ b/net/ncsi/ncsi-debug.c
> @@ -23,6 +23,235 @@
>  #include "ncsi-pkt.h"
>  
>  static struct dentry *ncsi_dentry;
> +static struct ncsi_pkt_handler {

static const struct etc...

> +	unsigned char	type;
> +	const char	*name;
> +} ncsi_pkt_handlers[] = {
> +	{ NCSI_PKT_CMD_CIS,    "CIS"    },
[]
> +static bool ncsi_dev_stats_index(struct ncsi_dev_priv *ndp, loff_t pos,
> +				 unsigned long *type, unsigned long *index,
> +				 unsigned long *entries)
> +{
> +	int i;
> +	unsigned long ranges[3][2] = {
> +		{ 1,
> +		  ARRAY_SIZE(ndp->stats.cmd) - 1		},
> +		{ ranges[0][1] + 2,
> +		  ranges[1][0] + ARRAY_SIZE(ndp->stats.rsp) - 1	},
> +		{ ranges[1][1] + 2,
> +		  ranges[2][0] + ARRAY_SIZE(ndp->stats.aen) - 1 }
> +	};

const?

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
  2017-04-13 10:50   ` Joe Perches
@ 2017-04-14  1:50   ` Jakub Kicinski
  2017-04-18  0:14     ` Gavin Shan
  2017-04-14  2:30   ` Joe Perches
  2 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2017-04-14  1:50 UTC (permalink / raw)
  To: Gavin Shan; +Cc: netdev, joe, davem

Hi!

On Thu, 13 Apr 2017 17:48:18 +1000, Gavin Shan wrote:
> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
> packets sent and received over all packages and channels. It's useful
> to diagnose NCSI problems, especially when NCSI packages and channels
> aren't probed properly. The statistics can be gained from debugfs file
> as below:
> 
>  # cat /sys/kernel/debug/ncsi/eth0/stats
> 
>  CMD          OK       TIMEOUT  ERROR
>  =======================================
>  CIS          32       29       0
>  SP           10       7        0
>  DP           17       14       0
>  EC           1        0        0
>  ECNT         1        0        0
>  AE           1        0        0
>  GLS          11       0        0
>  SMA          1        0        0
>  EBF          1        0        0
>  GVI          2        0        0
>  GC           2        0        0
> 
>  RSP          OK       TIMEOUT  ERROR
>  =======================================
>  CIS          3        0        0
>  SP           3        0        0
>  DP           2        0        1
>  EC           1        0        0
>  ECNT         1        0        0
>  AE           1        0        0
>  GLS          11       0        0
>  SMA          1        0        0
>  EBF          1        0        0
>  GVI          0        0        2
>  GC           2        0        0
> 
>  AEN          OK       TIMEOUT  ERROR
>  =======================================
> 
> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

I'm not familiar with NC-SI but these look like some standard stats.
Would it make sense to provide a proper netlink API for them?

[...]
> +#ifdef CONFIG_NET_NCSI_DEBUG
> +		ndp->stats.aen[h->type][NCSI_PKT_STAT_ERROR]++;
> +#endif

In any case, did you consider creating a macro or inline helper to
limit the number of #ifdefs?

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
  2017-04-13 10:50   ` Joe Perches
  2017-04-14  1:50   ` Jakub Kicinski
@ 2017-04-14  2:30   ` Joe Perches
  2017-04-18  0:22     ` Gavin Shan
  2 siblings, 1 reply; 17+ messages in thread
From: Joe Perches @ 2017-04-14  2:30 UTC (permalink / raw)
  To: Gavin Shan, netdev; +Cc: davem

On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
> packets sent and received over all packages and channels. It's useful
> to diagnose NCSI problems, especially when NCSI packages and channels
> aren't probed properly. The statistics can be gained from debugfs file
> as below:
> 
>  # cat /sys/kernel/debug/ncsi/eth0/stats
> 
>  CMD          OK       TIMEOUT  ERROR
>  =======================================
>  CIS          32       29       0
>  SP           10       7        0
>  DP           17       14       0
>  EC           1        0        0
>  ECNT         1        0        0
>  AE           1        0        0
>  GLS          11       0        0
>  SMA          1        0        0
>  EBF          1        0        0
>  GVI          2        0        0
>  GC           2        0        0

more trivia:

> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
[]
> @@ -23,6 +23,235 @@
>  #include "ncsi-pkt.h"
>  
>  static struct dentry *ncsi_dentry;
> +static struct ncsi_pkt_handler {
> +	unsigned char	type;
> +	const char	*name;
> +} ncsi_pkt_handlers[] = {
> +	{ NCSI_PKT_CMD_CIS,    "CIS"    },
> +	{ NCSI_PKT_CMD_SP,     "SP"     },
> +	{ NCSI_PKT_CMD_DP,     "DP"     },
> +	{ NCSI_PKT_CMD_EC,     "EC"     },
> +	{ NCSI_PKT_CMD_DC,     "DC"     },
> +	{ NCSI_PKT_CMD_RC,     "RC"     },
> +	{ NCSI_PKT_CMD_ECNT,   "ECNT"   },
> +	{ NCSI_PKT_CMD_DCNT,   "DCNT"   },
> +	{ NCSI_PKT_CMD_AE,     "AE"     },
> +	{ NCSI_PKT_CMD_SL,     "SL"     },
> +	{ NCSI_PKT_CMD_GLS,    "GLS"    },
> +	{ NCSI_PKT_CMD_SVF,    "SVF"    },
> +	{ NCSI_PKT_CMD_EV,     "EV"     },
> +	{ NCSI_PKT_CMD_DV,     "DV"     },
> +	{ NCSI_PKT_CMD_SMA,    "SMA"    },
> +	{ NCSI_PKT_CMD_EBF,    "EBF"    },
> +	{ NCSI_PKT_CMD_DBF,    "DBF"    },
> +	{ NCSI_PKT_CMD_EGMF,   "EGMF"   },
> +	{ NCSI_PKT_CMD_DGMF,   "DGMF"   },
> +	{ NCSI_PKT_CMD_SNFC,   "SNFC"   },
> +	{ NCSI_PKT_CMD_GVI,    "GVI"    },
> +	{ NCSI_PKT_CMD_GC,     "GC"     },
> +	{ NCSI_PKT_CMD_GP,     "GP"     },
> +	{ NCSI_PKT_CMD_GCPS,   "GCPS"   },
> +	{ NCSI_PKT_CMD_GNS,    "GNS"    },
> +	{ NCSI_PKT_CMD_GNPTS,  "GNPTS"  },
> +	{ NCSI_PKT_CMD_GPS,    "GPS"    },
> +	{ NCSI_PKT_CMD_OEM,    "OEM"    },
> +	{ NCSI_PKT_CMD_PLDM,   "PLDM"   },
> +	{ NCSI_PKT_CMD_GPUUID, "GPUUID" },

I don't know how common these are and how
intelligible these acronyms are to knowledgeable
developer/users, but maybe it'd be better to
spell out what these are instead of having to
look up what the acronyms stand for

	CIS - Clear Initial State
	SP - Select Package
	etc...

Maybe copy the descriptions from the ncsi-pkt.h file

#define NCSI_PKT_CMD_CIS	0x00 /* Clear Initial State              */
#define NCSI_PKT_CMD_SP		0x01 /* Select Package                   */
#define NCSI_PKT_CMD_DP		0x02 /* Deselect Package                 */
#define NCSI_PKT_CMD_EC		0x03 /* Enable Channel                   */
#define NCSI_PKT_CMD_DC		0x04 /* Disable Channel                  */
#define NCSI_PKT_CMD_RC		0x05 /* Reset Channel                    */
#define NCSI_PKT_CMD_ECNT	0x06 /* Enable Channel Network Tx        */
#define NCSI_PKT_CMD_DCNT	0x07 /* Disable Channel Network Tx       */
#define NCSI_PKT_CMD_AE		0x08 /* AEN Enable                       */
#define NCSI_PKT_CMD_SL		0x09 /* Set Link                         */
#define NCSI_PKT_CMD_GLS	0x0a /* Get Link                         */
#define NCSI_PKT_CMD_SVF	0x0b /* Set VLAN Filter                  */
#define NCSI_PKT_CMD_EV		0x0c /* Enable VLAN                      */
#define NCSI_PKT_CMD_DV		0x0d /* Disable VLAN                     */
#define NCSI_PKT_CMD_SMA	0x0e /* Set MAC address                  */
#define NCSI_PKT_CMD_EBF	0x10 /* Enable Broadcast Filter          */
#define NCSI_PKT_CMD_DBF	0x11 /* Disable Broadcast Filter         */
#define NCSI_PKT_CMD_EGMF	0x12 /* Enable Global Multicast Filter   */
#define NCSI_PKT_CMD_DGMF	0x13 /* Disable Global Multicast Filter  */
#define NCSI_PKT_CMD_SNFC	0x14 /* Set NCSI Flow Control            */
#define NCSI_PKT_CMD_GVI	0x15 /* Get Version ID                   */
#define NCSI_PKT_CMD_GC		0x16 /* Get Capabilities                 */
#define NCSI_PKT_CMD_GP		0x17 /* Get Parameters                   */
#define NCSI_PKT_CMD_GCPS	0x18 /* Get Controller Packet Statistics */
#define NCSI_PKT_CMD_GNS	0x19 /* Get NCSI Statistics              */
#define NCSI_PKT_CMD_GNPTS	0x1a /* Get NCSI Pass-throu Statistics   */
#define NCSI_PKT_CMD_GPS	0x1b /* Get package status               */
#define NCSI_PKT_CMD_OEM	0x50 /* OEM                              */
#define NCSI_PKT_CMD_PLDM	0x51 /* PLDM request over NCSI over RBT  */
#define NCSI_PKT_CMD_GPUUID	0x52 /* Get package UUID                 */

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

* Re: [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre
  2017-04-13 10:41   ` Joe Perches
@ 2017-04-17 23:22     ` Gavin Shan
  0 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-17 23:22 UTC (permalink / raw)
  To: Joe Perches; +Cc: Gavin Shan, netdev, davem

On Thu, Apr 13, 2017 at 03:41:46AM -0700, Joe Perches wrote:
>On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
>> This creates debugfs directories as NCSI debugging infrastructure.
>> With the patch applied, We will see below debugfs directories. Every
>> NCSI package and channel has one corresponding directory. Other than
>> presenting the NCSI topology, No real function has been achieved
>> through these debugfs directories so far.
>> 
>>      /sys/kernel/debug/ncsi/eth0
>>      /sys/kernel/debug/ncsi/eth0/p0
>>      /sys/kernel/debug/ncsi/eth0/p0/c0
>>      /sys/kernel/debug/ncsi/eth0/p0/c1
>
>[]
>
>> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
>[]
>> +int ncsi_dev_init_debug(struct ncsi_dev_priv *ndp)
>> +{
>> +	if (WARN_ON_ONCE(ndp->dentry))
>> +		return 0;
>> +
>> +	if (!ncsi_dentry) {
>> +		ncsi_dentry = debugfs_create_dir("ncsi", NULL);
>> +		if (!ncsi_dentry) {
>> +			pr_warn("NCSI: Cannot create /sys/kernel/debug/ncsi\n");
>> +			return -ENOENT;
>
>debugfs does not have a fixed path.
>
>Most error messages for this just use something like
>	pr_<level>("Failed to create debugfs directory '%s'\n", foo)
>And most failures don't emit any error message at all.
>

Thanks, Joe. I will correct in next respin.

Thanks,
Gavin

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-13 10:50   ` Joe Perches
@ 2017-04-17 23:23     ` Gavin Shan
  0 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-17 23:23 UTC (permalink / raw)
  To: Joe Perches; +Cc: Gavin Shan, netdev, davem

On Thu, Apr 13, 2017 at 03:50:40AM -0700, Joe Perches wrote:
>On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
>> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
>> packets sent and received over all packages and channels. It's useful
>> to diagnose NCSI problems, especially when NCSI packages and channels
>> aren't probed properly.
>
>trivia:
>
>> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
>> index 6c00e9b..29c233c 100644
>> --- a/net/ncsi/ncsi-debug.c
>> +++ b/net/ncsi/ncsi-debug.c
>> @@ -23,6 +23,235 @@
>>  #include "ncsi-pkt.h"
>>  
>>  static struct dentry *ncsi_dentry;
>> +static struct ncsi_pkt_handler {
>
>static const struct etc...
>

Yes.

>> +	unsigned char	type;
>> +	const char	*name;
>> +} ncsi_pkt_handlers[] = {
>> +	{ NCSI_PKT_CMD_CIS,    "CIS"    },
>[]
>> +static bool ncsi_dev_stats_index(struct ncsi_dev_priv *ndp, loff_t pos,
>> +				 unsigned long *type, unsigned long *index,
>> +				 unsigned long *entries)
>> +{
>> +	int i;
>> +	unsigned long ranges[3][2] = {
>> +		{ 1,
>> +		  ARRAY_SIZE(ndp->stats.cmd) - 1		},
>> +		{ ranges[0][1] + 2,
>> +		  ranges[1][0] + ARRAY_SIZE(ndp->stats.rsp) - 1	},
>> +		{ ranges[1][1] + 2,
>> +		  ranges[2][0] + ARRAY_SIZE(ndp->stats.aen) - 1 }
>> +	};
>
>const?
>

Yes. I will modify in next respin.

Thanks,
Gavin

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-14  1:50   ` Jakub Kicinski
@ 2017-04-18  0:14     ` Gavin Shan
  0 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-18  0:14 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Gavin Shan, netdev, joe, davem

On Thu, Apr 13, 2017 at 06:50:42PM -0700, Jakub Kicinski wrote:
>On Thu, 13 Apr 2017 17:48:18 +1000, Gavin Shan wrote:
>> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
>> packets sent and received over all packages and channels. It's useful
>> to diagnose NCSI problems, especially when NCSI packages and channels
>> aren't probed properly. The statistics can be gained from debugfs file
>> as below:
>> 
>>  # cat /sys/kernel/debug/ncsi/eth0/stats
>> 
>>  CMD          OK       TIMEOUT  ERROR
>>  =======================================
>>  CIS          32       29       0
>>  SP           10       7        0
>>  DP           17       14       0
>>  EC           1        0        0
>>  ECNT         1        0        0
>>  AE           1        0        0
>>  GLS          11       0        0
>>  SMA          1        0        0
>>  EBF          1        0        0
>>  GVI          2        0        0
>>  GC           2        0        0
>> 
>>  RSP          OK       TIMEOUT  ERROR
>>  =======================================
>>  CIS          3        0        0
>>  SP           3        0        0
>>  DP           2        0        1
>>  EC           1        0        0
>>  ECNT         1        0        0
>>  AE           1        0        0
>>  GLS          11       0        0
>>  SMA          1        0        0
>>  EBF          1        0        0
>>  GVI          0        0        2
>>  GC           2        0        0
>> 
>>  AEN          OK       TIMEOUT  ERROR
>>  =======================================
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>
>I'm not familiar with NC-SI but these look like some standard stats.
>Would it make sense to provide a proper netlink API for them?
>
>[...]
>> +#ifdef CONFIG_NET_NCSI_DEBUG
>> +		ndp->stats.aen[h->type][NCSI_PKT_STAT_ERROR]++;
>> +#endif
>
>In any case, did you consider creating a macro or inline helper to
>limit the number of #ifdefs?
>

Jakub, thanks for the comments. NCSI does have standard statistics
about the packets passed to peer (NIC) or NCSI packets handled by
hardware. I have some patches (not posted yet and won't post in
this merge window) to create debugfs file ncsi/eth0/p0/c0/stats
and dump them there.

This debugfs tracks NCSI packets sent and received by software
in order to see if the software has obvious bugs.

Yeah, it's definitely worthy to eliminate the #ifdef's. I'll do
in next respin.

Thanks,
Gavin

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

* Re: [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics
  2017-04-14  2:30   ` Joe Perches
@ 2017-04-18  0:22     ` Gavin Shan
  0 siblings, 0 replies; 17+ messages in thread
From: Gavin Shan @ 2017-04-18  0:22 UTC (permalink / raw)
  To: Joe Perches; +Cc: Gavin Shan, netdev, davem

On Thu, Apr 13, 2017 at 07:30:45PM -0700, Joe Perches wrote:
>On Thu, 2017-04-13 at 17:48 +1000, Gavin Shan wrote:
>> This creates /sys/kernel/debug/ncsi/<eth0>/stats to dump the NCSI
>> packets sent and received over all packages and channels. It's useful
>> to diagnose NCSI problems, especially when NCSI packages and channels
>> aren't probed properly. The statistics can be gained from debugfs file
>> as below:
>> 
>>  # cat /sys/kernel/debug/ncsi/eth0/stats
>> 
>>  CMD          OK       TIMEOUT  ERROR
>>  =======================================
>>  CIS          32       29       0
>>  SP           10       7        0
>>  DP           17       14       0
>>  EC           1        0        0
>>  ECNT         1        0        0
>>  AE           1        0        0
>>  GLS          11       0        0
>>  SMA          1        0        0
>>  EBF          1        0        0
>>  GVI          2        0        0
>>  GC           2        0        0
>
>more trivia:
>
>> diff --git a/net/ncsi/ncsi-debug.c b/net/ncsi/ncsi-debug.c
>[]
>> @@ -23,6 +23,235 @@
>>  #include "ncsi-pkt.h"
>>  
>>  static struct dentry *ncsi_dentry;
>> +static struct ncsi_pkt_handler {
>> +	unsigned char	type;
>> +	const char	*name;
>> +} ncsi_pkt_handlers[] = {
>> +	{ NCSI_PKT_CMD_CIS,    "CIS"    },
>> +	{ NCSI_PKT_CMD_SP,     "SP"     },
>> +	{ NCSI_PKT_CMD_DP,     "DP"     },
>> +	{ NCSI_PKT_CMD_EC,     "EC"     },
>> +	{ NCSI_PKT_CMD_DC,     "DC"     },
>> +	{ NCSI_PKT_CMD_RC,     "RC"     },
>> +	{ NCSI_PKT_CMD_ECNT,   "ECNT"   },
>> +	{ NCSI_PKT_CMD_DCNT,   "DCNT"   },
>> +	{ NCSI_PKT_CMD_AE,     "AE"     },
>> +	{ NCSI_PKT_CMD_SL,     "SL"     },
>> +	{ NCSI_PKT_CMD_GLS,    "GLS"    },
>> +	{ NCSI_PKT_CMD_SVF,    "SVF"    },
>> +	{ NCSI_PKT_CMD_EV,     "EV"     },
>> +	{ NCSI_PKT_CMD_DV,     "DV"     },
>> +	{ NCSI_PKT_CMD_SMA,    "SMA"    },
>> +	{ NCSI_PKT_CMD_EBF,    "EBF"    },
>> +	{ NCSI_PKT_CMD_DBF,    "DBF"    },
>> +	{ NCSI_PKT_CMD_EGMF,   "EGMF"   },
>> +	{ NCSI_PKT_CMD_DGMF,   "DGMF"   },
>> +	{ NCSI_PKT_CMD_SNFC,   "SNFC"   },
>> +	{ NCSI_PKT_CMD_GVI,    "GVI"    },
>> +	{ NCSI_PKT_CMD_GC,     "GC"     },
>> +	{ NCSI_PKT_CMD_GP,     "GP"     },
>> +	{ NCSI_PKT_CMD_GCPS,   "GCPS"   },
>> +	{ NCSI_PKT_CMD_GNS,    "GNS"    },
>> +	{ NCSI_PKT_CMD_GNPTS,  "GNPTS"  },
>> +	{ NCSI_PKT_CMD_GPS,    "GPS"    },
>> +	{ NCSI_PKT_CMD_OEM,    "OEM"    },
>> +	{ NCSI_PKT_CMD_PLDM,   "PLDM"   },
>> +	{ NCSI_PKT_CMD_GPUUID, "GPUUID" },
>
>I don't know how common these are and how
>intelligible these acronyms are to knowledgeable
>developer/users, but maybe it'd be better to
>spell out what these are instead of having to
>look up what the acronyms stand for
>
>	CIS - Clear Initial State
>	SP - Select Package
>	etc...
>
>Maybe copy the descriptions from the ncsi-pkt.h file
>

Joe, good question. As these decriptive strings are part of
the output from ncsi/eth0/stats and input to ncsi/eth0/pkt,
I intended to keep them short enough. Also, this debugging
interface would service developers who knows NCSI protocol
and perhaps know the meanings of these acronyms.

Thanks,
Gavin

>#define NCSI_PKT_CMD_CIS	0x00 /* Clear Initial State              */
>#define NCSI_PKT_CMD_SP		0x01 /* Select Package                   */
>#define NCSI_PKT_CMD_DP		0x02 /* Deselect Package                 */
>#define NCSI_PKT_CMD_EC		0x03 /* Enable Channel                   */
>#define NCSI_PKT_CMD_DC		0x04 /* Disable Channel                  */
>#define NCSI_PKT_CMD_RC		0x05 /* Reset Channel                    */
>#define NCSI_PKT_CMD_ECNT	0x06 /* Enable Channel Network Tx        */
>#define NCSI_PKT_CMD_DCNT	0x07 /* Disable Channel Network Tx       */
>#define NCSI_PKT_CMD_AE		0x08 /* AEN Enable                       */
>#define NCSI_PKT_CMD_SL		0x09 /* Set Link                         */
>#define NCSI_PKT_CMD_GLS	0x0a /* Get Link                         */
>#define NCSI_PKT_CMD_SVF	0x0b /* Set VLAN Filter                  */
>#define NCSI_PKT_CMD_EV		0x0c /* Enable VLAN                      */
>#define NCSI_PKT_CMD_DV		0x0d /* Disable VLAN                     */
>#define NCSI_PKT_CMD_SMA	0x0e /* Set MAC address                  */
>#define NCSI_PKT_CMD_EBF	0x10 /* Enable Broadcast Filter          */
>#define NCSI_PKT_CMD_DBF	0x11 /* Disable Broadcast Filter         */
>#define NCSI_PKT_CMD_EGMF	0x12 /* Enable Global Multicast Filter   */
>#define NCSI_PKT_CMD_DGMF	0x13 /* Disable Global Multicast Filter  */
>#define NCSI_PKT_CMD_SNFC	0x14 /* Set NCSI Flow Control            */
>#define NCSI_PKT_CMD_GVI	0x15 /* Get Version ID                   */
>#define NCSI_PKT_CMD_GC		0x16 /* Get Capabilities                 */
>#define NCSI_PKT_CMD_GP		0x17 /* Get Parameters                   */
>#define NCSI_PKT_CMD_GCPS	0x18 /* Get Controller Packet Statistics */
>#define NCSI_PKT_CMD_GNS	0x19 /* Get NCSI Statistics              */
>#define NCSI_PKT_CMD_GNPTS	0x1a /* Get NCSI Pass-throu Statistics   */
>#define NCSI_PKT_CMD_GPS	0x1b /* Get package status               */
>#define NCSI_PKT_CMD_OEM	0x50 /* OEM                              */
>#define NCSI_PKT_CMD_PLDM	0x51 /* PLDM request over NCSI over RBT  */
>#define NCSI_PKT_CMD_GPUUID	0x52 /* Get package UUID                 */
>

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

end of thread, other threads:[~2017-04-18  0:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-13  7:48 [PATCH v2 net-next 0/8] net/ncsi: Add debugging functionality Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 1/8] net/ncsi: Disable HWA mode when no channels are found Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 2/8] net/ncsi: Properly track channel monitor timer state Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 3/8] net/ncsi: Enforce failover on link monitor timeout Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 4/8] net/ncsi: Add debugging infrastructurre Gavin Shan
2017-04-13 10:41   ` Joe Perches
2017-04-17 23:22     ` Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 5/8] net/ncsi: Dump NCSI packet statistics Gavin Shan
2017-04-13 10:50   ` Joe Perches
2017-04-17 23:23     ` Gavin Shan
2017-04-14  1:50   ` Jakub Kicinski
2017-04-18  0:14     ` Gavin Shan
2017-04-14  2:30   ` Joe Perches
2017-04-18  0:22     ` Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 6/8] net/ncsi: Support NCSI packet generation Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 7/8] net/ncsi: No error report on DP response to non-existing package Gavin Shan
2017-04-13  7:48 ` [PATCH v2 net-next 8/8] net/ncsi: Fix length of GVI response packet Gavin Shan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).