[PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Dennis Dalessandro]

Hi Doug,

Here are the other two patches for opa_vnic. This adds a debugfs interface and
that is why I'm sending it as a separate series apart from the rest of our
submissions. The goal is to support view/modify of the VESW port config.

Patches can can also be found in my GitHub repo at:
https://github.com/ddalessa/kernel/tree/for-4.15

---

Niranjana Vishwanathapura (2):
      IB/opa_vnic: Add debugfs support to OPA VNIC
      IB/opa_vnic: Support adding OPA VNIC vports through debug interface


 drivers/infiniband/ulp/opa_vnic/Makefile           |    1 
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c |  667 ++++++++++++++++++++
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h |   70 ++
 .../infiniband/ulp/opa_vnic/opa_vnic_internal.h    |   36 +
 drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c    |   47 +
 5 files changed, 789 insertions(+), 32 deletions(-)
 create mode 100644 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
 create mode 100644 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h

--
-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 1/2] IB/opa_vnic: Add debugfs support to OPA VNIC

[Dennis Dalessandro]

From: Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Add debugfs support to view/modify the VESW port configuration information
including MAC to DLID translation table.

Reviewed-by: Sudeep Dutt <sudeep.dutt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Scott Franco <safranco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/ulp/opa_vnic/Makefile           |    1 
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c |  620 ++++++++++++++++++++
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h |   66 ++
 .../infiniband/ulp/opa_vnic/opa_vnic_internal.h    |    6 
 drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c    |   11 
 5 files changed, 700 insertions(+), 4 deletions(-)
 create mode 100644 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
 create mode 100644 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h

diff --git a/drivers/infiniband/ulp/opa_vnic/Makefile b/drivers/infiniband/ulp/opa_vnic/Makefile
index 8061b28..8d1929c 100644
--- a/drivers/infiniband/ulp/opa_vnic/Makefile
+++ b/drivers/infiniband/ulp/opa_vnic/Makefile
@@ -5,3 +5,4 @@ obj-$(CONFIG_INFINIBAND_OPA_VNIC) += opa_vnic.o
 
 opa_vnic-y := opa_vnic_netdev.o opa_vnic_encap.o opa_vnic_ethtool.o \
               opa_vnic_vema.o opa_vnic_vema_iface.o
+opa_vnic-$(CONFIG_DEBUG_FS) += opa_vnic_debugfs.o
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
new file mode 100644
index 0000000..f5565bd
--- /dev/null
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
@@ -0,0 +1,620 @@
+/*
+ * Copyright(c) 2017 Intel Corporation.
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/*
+ * This file contains HFI VNIC debug interface
+ */
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/list_sort.h>
+#include <linux/module.h>
+
+#include "opa_vnic_internal.h"
+#include "opa_vnic_debugfs.h"
+
+/* MAC table string size; 64K is enough for the whole table */
+#define OPA_VNIC_MACTBL_STR_SIZE  SZ_64K
+
+/* EEPH string size */
+#define OPA_VNIC_EEPH_STR_SIZE    SZ_1K
+
+/* Vnic stats string size */
+#define OPA_VNIC_STATS_STR_SIZE   SZ_1K
+
+enum {
+	OPA_VNIC_DBG_FABRIC_ID,
+	OPA_VNIC_DBG_VESW_ID,
+	OPA_VNIC_DBG_PKEY,
+	OPA_VNIC_DBG_ETH_LINK_STATUS,
+	OPA_VNIC_DBG_BASE_MAC_ADDR,
+	OPA_VNIC_DBG_CFG_STATE,
+	OPA_VNIC_DBG_OPER_STATE,
+	OPA_VNIC_DBG_ENCAP_SLID,
+	OPA_VNIC_DBG_DEF_PORT_MASK,
+	OPA_VNIC_DBG_UC_DLID,
+	OPA_VNIC_DBG_MC_DLID,
+	OPA_VNIC_DBG_SC_UC,
+	OPA_VNIC_DBG_SC_MC,
+	OPA_VNIC_DBG_PCP_SC_UC,
+	OPA_VNIC_DBG_PCP_SC_MC,
+	OPA_VNIC_DBG_VL_UC,
+	OPA_VNIC_DBG_VL_MC,
+	OPA_VNIC_DBG_PCP_VL_UC,
+	OPA_VNIC_DBG_PCP_VL_MC,
+	OPA_VNIC_DBG_ETH_MTU,
+	OPA_VNIC_DBG_ENCAP_RC,
+	OPA_VNIC_DBG_RESET,
+	OPA_VNIC_DBG_MACTBL_DIGEST,
+	OPA_VNIC_DBG_NUM_ATTR,
+};
+
+static struct dentry *opa_vnic_dbg_root;
+
+#define DEBUGFS_SEQ_FILE_OPS(name) \
+static const struct seq_operations _##name##_seq_ops = { \
+	.start = _##name##_seq_start, \
+	.next  = _##name##_seq_next, \
+	.stop  = _##name##_seq_stop, \
+	.show  = _##name##_seq_show \
+}
+
+#define DEBUGFS_SEQ_FILE_OPEN(name) \
+static int _##name##_open(struct inode *inode, struct file *file) \
+{ \
+	struct seq_file *seq; \
+	int ret; \
+	ret =  seq_open(file, &_##name##_seq_ops); \
+	if (ret) \
+		return ret; \
+	seq = file->private_data; \
+	seq->private = inode->i_private; \
+	return 0; \
+}
+
+#define DEBUGFS_FILE_OPS(name) \
+static const struct file_operations _##name##_file_ops = { \
+	.owner   = THIS_MODULE, \
+	.open    = _##name##_open, \
+	.write   = _##name##_write, \
+	.read    = seq_read, \
+	.llseek  = seq_lseek, \
+	.release = seq_release \
+}
+
+#define DEBUGFS_FILE_CREATE(name, parent, data, ops, mode)	\
+do { \
+	struct dentry *ent; \
+	ent = debugfs_create_file(name, mode, parent, data, ops); \
+	if (!ent) \
+		pr_warn("create of %s failed\n", name); \
+} while (0)
+
+#define DEBUGFS_SEQ_FILE_CREATE(name, parent, data) \
+	DEBUGFS_FILE_CREATE(#name, parent, data, &_##name##_file_ops, 0644)
+
+static void *_vport_state_seq_start(struct seq_file *s, loff_t *pos)
+__acquires(RCU)
+{
+	rcu_read_lock();
+	if (*pos >= OPA_VNIC_DBG_NUM_ATTR)
+		return NULL;
+	return pos;
+}
+
+static void *_vport_state_seq_next(struct seq_file *s, void *v, loff_t *pos)
+{
+	++*pos;
+	if (*pos >= OPA_VNIC_DBG_NUM_ATTR)
+		return NULL;
+	return pos;
+}
+
+static void _vport_state_seq_stop(struct seq_file *s, void *v)
+__releases(RCU)
+{
+	rcu_read_unlock();
+}
+
+static int _vport_state_seq_show(struct seq_file *s, void *v)
+{
+	struct opa_vnic_adapter *adapter =
+				(struct opa_vnic_adapter *)s->private;
+	struct __opa_veswport_info *info = &adapter->info;
+	loff_t *spos = v;
+	int stat;
+	char *str;
+	u8 i, *val;
+
+	if (v == SEQ_START_TOKEN)
+		return 0;
+
+	stat = *spos;
+	switch (stat) {
+	case OPA_VNIC_DBG_FABRIC_ID:
+		seq_printf(s, "fabric_id (fabric id): 0x%x\n",
+			   info->vesw.fabric_id);
+		break;
+	case OPA_VNIC_DBG_VESW_ID:
+		seq_printf(s, "vesw_id (virtual eth switch id): 0x%x\n",
+			   info->vesw.vesw_id);
+		break;
+	case OPA_VNIC_DBG_PKEY:
+		seq_printf(s, "pkey (partition key): 0x%x\n",
+			   info->vesw.pkey);
+		break;
+	case OPA_VNIC_DBG_ETH_LINK_STATUS:
+		seq_printf(s, "eth_link_status (0=unknown 1=up 2=down): %u\n",
+			   info->vport.eth_link_status);
+		break;
+	case OPA_VNIC_DBG_BASE_MAC_ADDR:
+		val = info->vport.base_mac_addr;
+		str = "%s: %02x:%02x:%02x:%02x:%02x:%02x\n";
+		seq_printf(s, str, "base_mac_addr (mac address)",
+			   val[0], val[1], val[2], val[3], val[4], val[5]);
+		break;
+	case OPA_VNIC_DBG_CFG_STATE:
+		seq_printf(s, "config_state (0=nop 1=drop 2=init 3=fwd): %u\n",
+			   info->vport.config_state);
+		break;
+	case OPA_VNIC_DBG_OPER_STATE:
+		seq_printf(s, "oper_state (0=nop 1=drop 2=init 3=fwd): %u\n",
+			   info->vport.oper_state);
+		break;
+	case OPA_VNIC_DBG_ENCAP_SLID:
+		seq_printf(s, "encap_slid (source lid): 0x%x\n",
+			   info->vport.encap_slid);
+		break;
+	case OPA_VNIC_DBG_DEF_PORT_MASK:
+		seq_printf(s, "def_port_mask (default port mask): 0x%04x\n",
+			   info->vesw.def_port_mask);
+		break;
+	case OPA_VNIC_DBG_UC_DLID:
+		seq_puts(s, "u_ucast_dlid (unknown ucast dlid):");
+		for (i = 0; i < OPA_VESW_MAX_NUM_DEF_PORT; i++)
+			seq_printf(s, " 0x%x", info->vesw.u_ucast_dlid[i]);
+		seq_puts(s, "\n");
+		break;
+	case OPA_VNIC_DBG_MC_DLID:
+		seq_printf(s, "u_mcast_dlid (unknown mcast dlid): 0x%x\n",
+			   info->vesw.u_mcast_dlid);
+		break;
+	case OPA_VNIC_DBG_ETH_MTU:
+		seq_printf(s, "eth_mtu (ethernet mtu): %u\n",
+			   info->vesw.eth_mtu);
+		break;
+	case OPA_VNIC_DBG_SC_UC:
+		seq_printf(s, "non_vlan_sc_uc (non-vlan ucast sc): 0x%x\n",
+			   info->vport.non_vlan_sc_uc);
+		break;
+	case OPA_VNIC_DBG_SC_MC:
+		seq_printf(s, "non_vlan_sc_mc (non-vlan mcast sc): 0x%x\n",
+			   info->vport.non_vlan_sc_mc);
+		break;
+	case OPA_VNIC_DBG_PCP_SC_UC:
+		val = info->vport.pcp_to_sc_uc;
+		str = "%s: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n";
+		seq_printf(s, str, "pcp_to_sc_uc (vlan ucast sc)",
+			   val[0], val[1], val[2], val[3],
+			   val[4], val[5], val[6], val[7]);
+		break;
+	case OPA_VNIC_DBG_PCP_SC_MC:
+		val = info->vport.pcp_to_sc_mc;
+		str = "%s: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n";
+		seq_printf(s, str, "pcp_to_sc_mc (vlan mcast sc)",
+			   val[0], val[1], val[2], val[3],
+			   val[4], val[5], val[6], val[7]);
+		break;
+	case OPA_VNIC_DBG_VL_UC:
+		seq_printf(s, "non_vlan_vl_uc (non-vlan ucast vl): 0x%x\n",
+			   info->vport.non_vlan_vl_uc);
+		break;
+	case OPA_VNIC_DBG_VL_MC:
+		seq_printf(s, "non_vlan_vl_mc (non-vlan mcast vl): 0x%x\n",
+			   info->vport.non_vlan_vl_mc);
+		break;
+	case OPA_VNIC_DBG_PCP_VL_UC:
+		val = info->vport.pcp_to_vl_uc;
+		str = "%s: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n";
+		seq_printf(s, str, "pcp_to_vl_uc (vlan ucast vl)",
+			   val[0], val[1], val[2], val[3],
+			   val[4], val[5], val[6], val[7]);
+		break;
+	case OPA_VNIC_DBG_PCP_VL_MC:
+		val = info->vport.pcp_to_vl_mc;
+		str = "%s: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x\n";
+		seq_printf(s, str, "pcp_to_vl_mc (vlan mcast vl)",
+			   val[0], val[1], val[2], val[3],
+			   val[4], val[5], val[6], val[7]);
+		break;
+	case OPA_VNIC_DBG_ENCAP_RC:
+		seq_printf(s, "encap_rc (routing control): 0x%08x\n",
+			   info->vesw.rc);
+		break;
+	case OPA_VNIC_DBG_MACTBL_DIGEST:
+		seq_printf(s, "MAC Table Digest: %u\n",
+			   info->vport.mac_tbl_digest);
+		break;
+	default:
+		return SEQ_SKIP;
+	}
+
+	return 0;
+}
+
+static ssize_t _vport_state_write(struct file *file, const char __user *buf,
+				  size_t count, loff_t *ppos)
+{
+	struct seq_file *s = (struct seq_file *)file->private_data;
+	struct opa_vnic_adapter *adapter =
+				(struct opa_vnic_adapter *)s->private;
+	struct __opa_veswport_info *info = &adapter->info;
+	char debug_buf[256];
+	ssize_t len;
+	u32 value;
+	int cnt;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= sizeof(debug_buf))
+		return -ENOSPC;
+
+	len = simple_write_to_buffer(debug_buf, sizeof(debug_buf) - 1,
+				     ppos, buf, count);
+	if (len < 0)
+		return len;
+
+	debug_buf[len] = '\0';
+	if (strncmp(debug_buf, "fabric_id", 9) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[9]), 0, &value);
+		if (!cnt)
+			info->vesw.fabric_id = value;
+	} else if (strncmp(debug_buf, "vesw_id", 7) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[7]), 0, &value);
+		if (!cnt)
+			info->vesw.vesw_id = value;
+	} else if (strncmp(debug_buf, "pkey", 4) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[4]), 0, &value);
+		if (!cnt)
+			info->vesw.pkey = value;
+	} else if (strncmp(debug_buf, "base_mac_addr", 13) == 0) {
+		u8 i, mac[6];
+
+		cnt = sscanf(&debug_buf[13], "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
+			     &mac[0], &mac[1], &mac[2],
+			     &mac[3], &mac[4], &mac[5]);
+		if (cnt == 6)
+			for (i = 0; i < 6; i++)
+				info->vport.base_mac_addr[i] = mac[i];
+	} else if (strncmp(debug_buf, "config_state", 12) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[12]), 0, &value);
+		if (!cnt)
+			info->vport.config_state = value;
+	} else if (strncmp(debug_buf, "encap_slid", 10) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[10]), 0, &value);
+		if (!cnt)
+			info->vport.encap_slid = value;
+	} else if (strncmp(debug_buf, "def_port_mask", 13) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[13]), 16, &value);
+		if (!cnt)
+			info->vesw.def_port_mask = value;
+	} else if (strncmp(debug_buf, "u_ucast_dlid", 12) == 0) {
+		u32 idx;
+		int val;
+
+		cnt = sscanf(&debug_buf[12], "%u %i", &idx, &val);
+		if ((cnt == 2) && (idx < OPA_VESW_MAX_NUM_DEF_PORT))
+			info->vesw.u_ucast_dlid[idx] = (u32)val;
+	} else if (strncmp(debug_buf, "u_mcast_dlid", 12) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[12]), 0, &value);
+		if (!cnt)
+			info->vesw.u_mcast_dlid = value;
+	} else if (strncmp(debug_buf, "eth_mtu", 7) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[7]), 0, &value);
+		if (!cnt)
+			info->vesw.eth_mtu = value;
+	} else if (strncmp(debug_buf, "non_vlan_sc_uc", 14) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[14]), 0, &value);
+		if (!cnt)
+			info->vport.non_vlan_sc_uc = value;
+	} else if (strncmp(debug_buf, "non_vlan_sc_mc", 14) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[14]), 0, &value);
+		if (!cnt)
+			info->vport.non_vlan_sc_mc = value;
+	} else if (strncmp(debug_buf, "pcp_to_sc_uc", 12) == 0) {
+		int i, sc[8];
+
+		cnt = sscanf(&debug_buf[12], "%i %i %i %i %i %i %i %i",
+			     &sc[0], &sc[1], &sc[2], &sc[3],
+			     &sc[4], &sc[5], &sc[6], &sc[7]);
+		if (cnt == 8)
+			for (i = 0; i < 8; i++)
+				info->vport.pcp_to_sc_uc[i] = sc[i];
+	} else if (strncmp(debug_buf, "pcp_to_sc_mc", 12) == 0) {
+		int i, sc[8];
+
+		cnt = sscanf(&debug_buf[12], "%i %i %i %i %i %i %i %i",
+			     &sc[0], &sc[1], &sc[2], &sc[3],
+			     &sc[4], &sc[5], &sc[6], &sc[7]);
+		if (cnt == 8)
+			for (i = 0; i < 8; i++)
+				info->vport.pcp_to_sc_mc[i] = sc[i];
+	} else if (strncmp(debug_buf, "non_vlan_vl_uc", 14) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[14]), 0, &value);
+		if (!cnt)
+			info->vport.non_vlan_vl_uc = value;
+	} else if (strncmp(debug_buf, "non_vlan_vl_mc", 14) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[14]), 0, &value);
+		if (!cnt)
+			info->vport.non_vlan_vl_mc = value;
+	} else if (strncmp(debug_buf, "pcp_to_vl_uc", 12) == 0) {
+		int i, vl[8];
+
+		cnt = sscanf(&debug_buf[12], "%i %i %i %i %i %i %i %i",
+			     &vl[0], &vl[1], &vl[2], &vl[3],
+			     &vl[4], &vl[5], &vl[6], &vl[7]);
+		if (cnt == 8)
+			for (i = 0; i < 8; i++)
+				info->vport.pcp_to_vl_uc[i] = vl[i];
+	} else if (strncmp(debug_buf, "pcp_to_vl_mc", 12) == 0) {
+		int i, vl[8];
+
+		cnt = sscanf(&debug_buf[12], "%i %i %i %i %i %i %i %i",
+			     &vl[0], &vl[1], &vl[2], &vl[3],
+			     &vl[4], &vl[5], &vl[6], &vl[7]);
+		if (cnt == 8)
+			for (i = 0; i < 8; i++)
+				info->vport.pcp_to_vl_mc[i] = vl[i];
+	} else if (strncmp(debug_buf, "encap_rc", 8) == 0) {
+		cnt = kstrtouint(strim(&debug_buf[8]), 16, &value);
+		if (!cnt)
+			info->vesw.rc = value;
+	} else if (strncmp(debug_buf, "reset", 5) == 0) {
+		struct opa_veswport_info port_info;
+
+		vema_get_pod_values(&port_info);
+		opa_vnic_set_vesw_info(adapter, &port_info.vesw);
+		opa_vnic_set_per_veswport_info(adapter, &port_info.vport);
+		opa_vnic_release_mac_tbl(adapter);
+	}
+
+	/* process the new config settings */
+	opa_vnic_process_vema_config(adapter);
+	return count;
+}
+
+DEBUGFS_SEQ_FILE_OPS(vport_state);
+DEBUGFS_SEQ_FILE_OPEN(vport_state)
+DEBUGFS_FILE_OPS(vport_state);
+
+static ssize_t mac_tbl_read(struct file *file, char __user *buf,
+			    size_t count, loff_t *ppos)
+{
+	struct opa_vnic_adapter *adapter = file_inode(file)->i_private;
+	struct opa_veswport_mactable *tbl;
+	char *debug_buf;
+	ssize_t len = 0;
+	int i, extra, rc = 0;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count < OPA_VNIC_MACTBL_STR_SIZE)
+		return -ENOSPC;
+
+	debug_buf = kmalloc(OPA_VNIC_MACTBL_STR_SIZE, GFP_KERNEL);
+	if (!debug_buf)
+		return -ENOMEM;
+
+	/* allocate veswport mac table */
+	extra = sizeof(struct opa_veswport_mactable_entry) *
+					OPA_VNIC_MAC_TBL_MAX_ENTRIES;
+	tbl = kzalloc(sizeof(*tbl) + extra, GFP_KERNEL);
+	if (!tbl) {
+		rc = -ENOMEM;
+		goto read_done;
+	}
+
+	/* get the whole table */
+	tbl->offset = cpu_to_be16(0);
+	tbl->num_entries = cpu_to_be16(OPA_VNIC_MAC_TBL_MAX_ENTRIES);
+
+	opa_vnic_query_mac_tbl(adapter, tbl);
+	len += scnprintf(debug_buf + len, OPA_VNIC_MACTBL_STR_SIZE - len,
+			 "Mac Table Digest: %u\n",
+			 be32_to_cpu(tbl->mac_tbl_digest));
+	for (i = 0; i < OPA_VNIC_MAC_TBL_MAX_ENTRIES; i++) {
+		struct opa_veswport_mactable_entry *entry =
+							&tbl->tbl_entries[i];
+		u8 *mac_addr = entry->mac_addr;
+		u8 empty_mac[ETH_ALEN] = { 0 };
+
+		/* if the entry is not there (null), skip */
+		if (!memcmp(mac_addr, empty_mac, ARRAY_SIZE(empty_mac)))
+			continue;
+
+		len += scnprintf(debug_buf + len,
+				 OPA_VNIC_MACTBL_STR_SIZE - len,
+				 "%4d: %02x:%02x:%02x:%02x:%02x:%02x  0x%x\n",
+				 i, mac_addr[0], mac_addr[1], mac_addr[2],
+				 mac_addr[3], mac_addr[4], mac_addr[5],
+				 be32_to_cpu(entry->dlid_sd));
+	}
+	kfree(tbl);
+
+	rc = simple_read_from_buffer(buf, count, ppos, debug_buf, len);
+read_done:
+	kfree(debug_buf);
+	return rc;
+}
+
+static ssize_t mac_tbl_write(struct file *file, const char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	struct opa_vnic_adapter *adapter = file_inode(file)->i_private;
+	struct opa_veswport_mactable *tbl;
+	int i, extra, cnt, rc, num_bytes;
+	u32 offset, num_entries, digest;
+	char *debug_buf, *buf_ptr;
+	ssize_t len;
+
+	if (*ppos != 0)
+		return 0;
+
+	if (count >= OPA_VNIC_MACTBL_STR_SIZE)
+		return -ENOSPC;
+
+	debug_buf = kmalloc(count + 1, GFP_KERNEL);
+	if (!debug_buf)
+		return -ENOMEM;
+
+	rc = simple_write_to_buffer(debug_buf, count, ppos, buf, count);
+	if (rc < 0)
+		goto write_err;
+
+	len = rc;
+	debug_buf[len] = '\0';
+
+	/* read offset and number of entries */
+	buf_ptr = debug_buf;
+	cnt = sscanf(buf_ptr, "%u %u %u %n", &digest, &offset, &num_entries,
+		     &num_bytes);
+	if ((cnt != 3) || !num_entries ||
+	    ((offset + num_entries) > OPA_VNIC_MAC_TBL_MAX_ENTRIES)) {
+		v_err("Invalid input\n");
+		rc = -EINVAL;
+		goto write_err;
+	}
+
+	/* allocate veswport mac table */
+	extra = sizeof(struct opa_veswport_mactable_entry) * num_entries;
+	tbl = kzalloc(sizeof(*tbl) + extra, GFP_KERNEL);
+	if (!tbl) {
+		rc = -ENOMEM;
+		goto write_err;
+	}
+
+	/* build the veswport mac table */
+	tbl->mac_tbl_digest = cpu_to_be32(digest);
+	tbl->offset = cpu_to_be16(offset);
+	tbl->num_entries = cpu_to_be16(num_entries);
+	for (i = 0; i < num_entries; i++) {
+		struct opa_veswport_mactable_entry *entry =
+							&tbl->tbl_entries[i];
+		u8 *mac_addr = entry->mac_addr;
+		u32 dlid_sd;
+
+		buf_ptr += num_bytes;
+		cnt = sscanf(buf_ptr, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx  %x%n",
+			     &mac_addr[0], &mac_addr[1], &mac_addr[2],
+			     &mac_addr[3], &mac_addr[4], &mac_addr[5],
+			     &dlid_sd, &num_bytes);
+		if (cnt != 7)  {
+			v_err("Invalid input\n");
+			rc = -EINVAL;
+			goto write_invalid;
+		}
+		entry->dlid_sd = cpu_to_be32(dlid_sd);
+	}
+
+	/* update mac table */
+	rc = opa_vnic_update_mac_tbl(adapter, tbl);
+write_invalid:
+	kfree(tbl);
+	rc = rc ? : count;
+write_err:
+	kfree(debug_buf);
+	return rc;
+}
+
+static const struct file_operations mac_tbl_file_ops = {
+	.owner   = THIS_MODULE,
+	.write   = mac_tbl_write,
+	.read    = mac_tbl_read,
+};
+
+void opa_vnic_dbg_vport_init(struct opa_vnic_adapter *adapter)
+{
+	struct net_device *dev = adapter->netdev;
+
+	if (!opa_vnic_dbg_root)
+		return;
+
+	adapter->dentry  = debugfs_create_dir(dev->name,
+					      opa_vnic_dbg_root);
+	if (!adapter->dentry) {
+		pr_warn("init of opa vnic debugfs failed\n");
+		return;
+	}
+
+	DEBUGFS_SEQ_FILE_CREATE(vport_state, adapter->dentry, adapter);
+	DEBUGFS_FILE_CREATE("mac_tbl", adapter->dentry, adapter,
+			    &mac_tbl_file_ops, 0644);
+}
+
+void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter)
+{
+	debugfs_remove_recursive(adapter->dentry);
+}
+
+void opa_vnic_dbg_init(void)
+{
+	opa_vnic_dbg_root = debugfs_create_dir(opa_vnic_driver_name, NULL);
+	if (IS_ERR(opa_vnic_dbg_root))
+		opa_vnic_dbg_root = NULL;
+	if (!opa_vnic_dbg_root)
+		pr_warn("init of hfi vnic debugfs failed\n");
+}
+
+void opa_vnic_dbg_exit(void)
+{
+	debugfs_remove_recursive(opa_vnic_dbg_root);
+	opa_vnic_dbg_root = NULL;
+}
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h
new file mode 100644
index 0000000..9583a34
--- /dev/null
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h
@@ -0,0 +1,66 @@
+#ifndef _OPA_VNIC_DEBUGFS_H
+#define _OPA_VNIC_DEBUGFS_H
+/*
+ * Copyright(c) 2017 Intel Corporation.
+ *
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GPL LICENSE SUMMARY
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * BSD LICENSE
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ *  - Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  - Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *  - Neither the name of Intel Corporation nor the names of its
+ *    contributors may be used to endorse or promote products derived
+ *    from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/*
+ * This file contains OPA VNIC Debug interface declarations
+ */
+
+#ifdef CONFIG_DEBUG_FS
+void opa_vnic_dbg_vport_init(struct opa_vnic_adapter *adapter);
+void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter);
+void opa_vnic_dbg_init(void);
+void opa_vnic_dbg_exit(void);
+#else
+static inline void opa_vnic_dbg_vport_init(struct opa_vnic_adapter *adapter) {}
+static inline void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter) {}
+static inline void opa_vnic_dbg_init(void) {}
+static inline void opa_vnic_dbg_exit(void) {}
+#endif
+
+#endif /* _OPA_VNIC_DEBUGFS_H */
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
index afd95f4..4c82c55 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
@@ -219,8 +219,9 @@ struct opa_vnic_adapter {
 
 	u8 flow_tbl[OPA_VNIC_FLOW_TBL_SIZE];
 
-	unsigned long trap_timeout;
-	u8            trap_count;
+	unsigned long  trap_timeout;
+	u8             trap_count;
+	struct dentry *dentry;
 };
 
 /* Same as opa_veswport_mactable_entry, but without bitwise attribute */
@@ -326,5 +327,6 @@ void opa_vnic_set_per_veswport_info(struct opa_vnic_adapter *adapter,
 void opa_vnic_set_ethtool_ops(struct net_device *netdev);
 void opa_vnic_vema_send_trap(struct opa_vnic_adapter *adapter,
 			     struct __opa_veswport_trap *data, u32 lid);
+void vema_get_pod_values(struct opa_veswport_info *port_info);
 
 #endif /* _OPA_VNIC_INTERNAL_H */
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
index 4b615c1..c6cccbb 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
@@ -57,6 +57,7 @@
 #include <rdma/opa_port_info.h>
 
 #include "opa_vnic_internal.h"
+#include "opa_vnic_debugfs.h"
 
 #define DRV_VERSION "1.0"
 char opa_vnic_driver_name[] = "opa_vnic";
@@ -177,7 +178,7 @@ static inline bool vema_mac_tbl_req_ok(struct opa_veswport_mactable *mac_tbl)
  * Return the power on default values in the port info structure
  * in big endian format as required by MAD.
  */
-static inline void vema_get_pod_values(struct opa_veswport_info *port_info)
+void vema_get_pod_values(struct opa_veswport_info *port_info)
 {
 	memset(port_info, 0, sizeof(*port_info));
 	port_info->vport.max_mac_tbl_ent =
@@ -213,6 +214,7 @@ static inline void vema_get_pod_values(struct opa_veswport_info *port_info)
 			opa_vnic_rem_netdev(adapter);
 			adapter = ERR_PTR(rc);
 		}
+		opa_vnic_dbg_vport_init(adapter);
 	}
 
 	return adapter;
@@ -857,6 +859,7 @@ static int vema_rem_vport(int id, void *p, void *data)
 {
 	struct opa_vnic_adapter *adapter = p;
 
+	opa_vnic_dbg_vport_exit(adapter);
 	opa_vnic_rem_netdev(adapter);
 	return 0;
 }
@@ -1057,9 +1060,12 @@ static int __init opa_vnic_init(void)
 	pr_info("OPA Virtual Network Driver - v%s\n",
 		opa_vnic_driver_version);
 
+	opa_vnic_dbg_init();
 	rc = ib_register_client(&opa_vnic_client);
-	if (rc)
+	if (rc) {
 		pr_err("VNIC driver register failed %d\n", rc);
+		opa_vnic_dbg_exit();
+	}
 
 	return rc;
 }
@@ -1068,6 +1074,7 @@ static int __init opa_vnic_init(void)
 static void opa_vnic_deinit(void)
 {
 	ib_unregister_client(&opa_vnic_client);
+	opa_vnic_dbg_exit();
 }
 module_exit(opa_vnic_deinit);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 2/2] IB/opa_vnic: Support adding OPA VNIC vports through debug interface

[Dennis Dalessandro]

From: Niranjana Vishwanathapura <niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Adds the ability to create vnic vports manually from command line using
the debugfs interface.

Reviewed-by: Sadanand Warrier <sadanand.warrier-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Niranjana Vishwanathapura <Niranjana.Vishwanathapura-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Scott Franco <safranco-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c |   47 ++++++++++++++++++++
 drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h |    4 ++
 .../infiniband/ulp/opa_vnic/opa_vnic_internal.h    |   30 +++++++++++++
 drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c    |   38 ++++------------
 4 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
index f5565bd..39cd410 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.c
@@ -604,6 +604,53 @@ void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter)
 	debugfs_remove_recursive(adapter->dentry);
 }
 
+static int ctrl_add_vport_set(void *data, u64 val)
+{
+	struct opa_vnic_vema_port *port = data;
+	struct opa_vnic_adapter *adapter;
+	u8 vport = (u8)val;
+
+	adapter = vema_add_vport(port, vport);
+	return IS_ERR(adapter);
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(ctrl_add_vport, NULL, ctrl_add_vport_set, "%llu\n");
+
+void opa_vnic_dbg_ctrl_init(struct opa_vnic_ctrl_port *cport)
+{
+	struct opa_vnic_vema_port *port;
+	int i;
+
+	if (!opa_vnic_dbg_root)
+		return;
+
+	for (i = 1; i <= cport->num_ports; i++) {
+		char name[255];
+
+		port = vema_get_port(cport, i);
+		if (port->dentry)
+			continue;
+		snprintf(name, sizeof(name), "%s.%02x",
+			 dev_name(&cport->ibdev->dev), i);
+		port->dentry = debugfs_create_dir(name, opa_vnic_dbg_root);
+		if (port->dentry)
+			DEBUGFS_FILE_CREATE("add_vport", port->dentry, port,
+					    &ctrl_add_vport, 0200);
+	}
+}
+
+void opa_vnic_dbg_ctrl_exit(struct opa_vnic_ctrl_port *cport)
+{
+	struct opa_vnic_vema_port *port;
+	int i;
+
+	for (i = 1; i <= cport->num_ports; i++) {
+		port = vema_get_port(cport, i);
+		debugfs_remove_recursive(port->dentry);
+		port->dentry = NULL;
+	}
+}
+
 void opa_vnic_dbg_init(void)
 {
 	opa_vnic_dbg_root = debugfs_create_dir(opa_vnic_driver_name, NULL);
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h
index 9583a34..90fafcb 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_debugfs.h
@@ -52,11 +52,15 @@
  */
 
 #ifdef CONFIG_DEBUG_FS
+void opa_vnic_dbg_ctrl_init(struct opa_vnic_ctrl_port *cport);
+void opa_vnic_dbg_ctrl_exit(struct opa_vnic_ctrl_port *cport);
 void opa_vnic_dbg_vport_init(struct opa_vnic_adapter *adapter);
 void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter);
 void opa_vnic_dbg_init(void);
 void opa_vnic_dbg_exit(void);
 #else
+static void opa_vnic_dbg_ctrl_init(struct opa_vnic_ctrl_port *cport) {}
+static void opa_vnic_dbg_ctrl_exit(struct opa_vnic_ctrl_port *cport) {}
 static inline void opa_vnic_dbg_vport_init(struct opa_vnic_adapter *adapter) {}
 static inline void opa_vnic_dbg_vport_exit(struct opa_vnic_adapter *adapter) {}
 static inline void opa_vnic_dbg_init(void) {}
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
index 4c82c55..884d6ec 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_internal.h
@@ -243,6 +243,32 @@ struct opa_vnic_mac_tbl_node {
 	struct __opa_vnic_mactable_entry     entry;
 };
 
+/**
+ * struct opa_vnic_vema_port -- VNIC VEMA port details
+ * @cport: pointer to port
+ * @mad_agent: pointer to mad agent for port
+ * @class_port_info: Class port info information.
+ * @tid: Transaction id
+ * @port_num: OPA port number
+ * @vport_idr: vnic ports idr
+ * @event_handler: ib event handler
+ * @dentry: Pointer to port debugfs node
+ * @lock: adapter interface lock
+ */
+struct opa_vnic_vema_port {
+	struct opa_vnic_ctrl_port      *cport;
+	struct ib_mad_agent            *mad_agent;
+	struct opa_class_port_info      class_port_info;
+	u64                             tid;
+	u8                              port_num;
+	struct idr                      vport_idr;
+	struct ib_event_handler         event_handler;
+	struct dentry                  *dentry;
+
+	/* Lock to query/update network adapter */
+	struct mutex                    lock;
+};
+
 #define v_dbg(format, arg...) \
 	netdev_dbg(adapter->netdev, format, ## arg)
 #define v_err(format, arg...) \
@@ -328,5 +354,9 @@ void opa_vnic_set_per_veswport_info(struct opa_vnic_adapter *adapter,
 void opa_vnic_vema_send_trap(struct opa_vnic_adapter *adapter,
 			     struct __opa_veswport_trap *data, u32 lid);
 void vema_get_pod_values(struct opa_veswport_info *port_info);
+struct opa_vnic_vema_port *vema_get_port(struct opa_vnic_ctrl_port *cport,
+					 u8 port_num);
+struct opa_vnic_adapter *vema_add_vport(struct opa_vnic_vema_port *port,
+					u8 vport_num);
 
 #endif /* _OPA_VNIC_INTERNAL_H */
diff --git a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
index c6cccbb..137dc36 100644
--- a/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
+++ b/drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c
@@ -91,30 +91,6 @@
 /* Maximum number of VNIC ports supported */
 #define OPA_VNIC_MAX_NUM_VPORT    255
 
-/**
- * struct opa_vnic_vema_port -- VNIC VEMA port details
- * @cport: pointer to port
- * @mad_agent: pointer to mad agent for port
- * @class_port_info: Class port info information.
- * @tid: Transaction id
- * @port_num: OPA port number
- * @vport_idr: vnic ports idr
- * @event_handler: ib event handler
- * @lock: adapter interface lock
- */
-struct opa_vnic_vema_port {
-	struct opa_vnic_ctrl_port      *cport;
-	struct ib_mad_agent            *mad_agent;
-	struct opa_class_port_info      class_port_info;
-	u64                             tid;
-	u8                              port_num;
-	struct idr                      vport_idr;
-	struct ib_event_handler         event_handler;
-
-	/* Lock to query/update network adapter */
-	struct mutex                    lock;
-};
-
 static void opa_vnic_vema_add_one(struct ib_device *device);
 static void opa_vnic_vema_rem_one(struct ib_device *device,
 				  void *client_data);
@@ -197,8 +173,8 @@ void vema_get_pod_values(struct opa_veswport_info *port_info)
  *
  * Return a pointer to the vnic adapter structure
  */
-static struct opa_vnic_adapter *vema_add_vport(struct opa_vnic_vema_port *port,
-					       u8 vport_num)
+struct opa_vnic_adapter *vema_add_vport(struct opa_vnic_vema_port *port,
+					u8 vport_num)
 {
 	struct opa_vnic_ctrl_port *cport = port->cport;
 	struct opa_vnic_adapter *adapter;
@@ -213,8 +189,9 @@ void vema_get_pod_values(struct opa_veswport_info *port_info)
 		if (rc < 0) {
 			opa_vnic_rem_netdev(adapter);
 			adapter = ERR_PTR(rc);
+		} else {
+			opa_vnic_dbg_vport_init(adapter);
 		}
-		opa_vnic_dbg_vport_init(adapter);
 	}
 
 	return adapter;
@@ -699,7 +676,7 @@ static void vema_recv(struct ib_mad_agent *mad_agent,
  * Return: ptr to requested opa_vnic_vema_port strucure
  *         if success, NULL if not
  */
-static struct opa_vnic_vema_port *
+struct opa_vnic_vema_port *
 vema_get_port(struct opa_vnic_ctrl_port *cport, u8 port_num)
 {
 	struct opa_vnic_vema_port *port = (void *)cport + sizeof(*cport);
@@ -1025,8 +1002,10 @@ static void opa_vnic_vema_add_one(struct ib_device *device)
 
 	/* Initialize opa vnic management agent (vema) */
 	rc = vema_register(cport);
-	if (!rc)
+	if (!rc) {
+		opa_vnic_dbg_ctrl_init(cport);
 		c_info("VNIC client initialized\n");
+	}
 
 	ib_set_client_data(device, &opa_vnic_client, cport);
 	opa_vnic_ctrl_config_dev(cport, true);
@@ -1049,6 +1028,7 @@ static void opa_vnic_vema_rem_one(struct ib_device *device,
 
 	c_info("removing VNIC client\n");
 	opa_vnic_ctrl_config_dev(cport, false);
+	opa_vnic_dbg_ctrl_exit(cport);
 	vema_unregister(cport);
 	kfree(cport);
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

On Tue, Sep 26, 2017 at 07:08:37AM -0700, Dennis Dalessandro wrote:
> Hi Doug,
>
> Here are the other two patches for opa_vnic. This adds a debugfs interface and
> that is why I'm sending it as a separate series apart from the rest of our
> submissions. The goal is to support view/modify of the VESW port config.
>
> Patches can can also be found in my GitHub repo at:
> https://github.com/ddalessa/kernel/tree/for-4.15
>
> ---

Can you elaborate more on the reasons why debugfs was chosen over more
generic configuration tool?

+DEBUGFS_FILE_OPS(vport_state);

Maybe, I'm too biased with rdmatool, but I don't see many reasons to
introduce debugfs for the ULP.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

On Tue, Sep 26, 2017 at 08:49:46PM +0300, Leon Romanovsky wrote:
> On Tue, Sep 26, 2017 at 07:08:37AM -0700, Dennis Dalessandro wrote:
> > Hi Doug,
> >
> > Here are the other two patches for opa_vnic. This adds a debugfs interface and
> > that is why I'm sending it as a separate series apart from the rest of our
> > submissions. The goal is to support view/modify of the VESW port config.
> >
> > Patches can can also be found in my GitHub repo at:
> > https://github.com/ddalessa/kernel/tree/for-4.15
> >
> > ---
>
> Can you elaborate more on the reasons why debugfs was chosen over more
> generic configuration tool?
>
> +DEBUGFS_FILE_OPS(vport_state);
>
> Maybe, I'm too biased with rdmatool, but I don't see many reasons to
> introduce debugfs for the ULP.

And honestly, netdevices have so much tools to configure them (ethtool,
iproute2, e.t.c.) so I completely don't understand the rationale behind
debugfs for OPA-VNIC.

Thanks

>
> Thanks



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Vishwanathapura, Niranjana]

On Tue, Sep 26, 2017 at 08:55:24PM +0300, Leon Romanovsky wrote:
>On Tue, Sep 26, 2017 at 08:49:46PM +0300, Leon Romanovsky wrote:
>> On Tue, Sep 26, 2017 at 07:08:37AM -0700, Dennis Dalessandro wrote:
>> > Hi Doug,
>> >
>> > Here are the other two patches for opa_vnic. This adds a debugfs interface and
>> > that is why I'm sending it as a separate series apart from the rest of our
>> > submissions. The goal is to support view/modify of the VESW port config.
>> >
>> > Patches can can also be found in my GitHub repo at:
>> > https://github.com/ddalessa/kernel/tree/for-4.15
>> >
>> > ---
>>
>> Can you elaborate more on the reasons why debugfs was chosen over more
>> generic configuration tool?
>>
>> +DEBUGFS_FILE_OPS(vport_state);
>>
>> Maybe, I'm too biased with rdmatool, but I don't see many reasons to
>> introduce debugfs for the ULP.
>
>And honestly, netdevices have so much tools to configure them (ethtool,
>iproute2, e.t.c.) so I completely don't understand the rationale behind
>debugfs for OPA-VNIC.
>
>Thanks
>
Hi,

The debugfs interface here is for VNIC port encapsulation configuration.
They are not related to netdevice functionality.

They are for debug purpose only and not the supported configuration mechanism.
Hence, I think they probably shouldn't be in a tool like rdmatool.

Niranjana

>>
>> Thanks
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

On Tue, Sep 26, 2017 at 11:12:16AM -0700, Vishwanathapura, Niranjana wrote:
> On Tue, Sep 26, 2017 at 08:55:24PM +0300, Leon Romanovsky wrote:
> > On Tue, Sep 26, 2017 at 08:49:46PM +0300, Leon Romanovsky wrote:
> > > On Tue, Sep 26, 2017 at 07:08:37AM -0700, Dennis Dalessandro wrote:
> > > > Hi Doug,
> > > >
> > > > Here are the other two patches for opa_vnic. This adds a debugfs interface and
> > > > that is why I'm sending it as a separate series apart from the rest of our
> > > > submissions. The goal is to support view/modify of the VESW port config.
> > > >
> > > > Patches can can also be found in my GitHub repo at:
> > > > https://github.com/ddalessa/kernel/tree/for-4.15
> > > >
> > > > ---
> > >
> > > Can you elaborate more on the reasons why debugfs was chosen over more
> > > generic configuration tool?
> > >
> > > +DEBUGFS_FILE_OPS(vport_state);
> > >
> > > Maybe, I'm too biased with rdmatool, but I don't see many reasons to
> > > introduce debugfs for the ULP.
> >
> > And honestly, netdevices have so much tools to configure them (ethtool,
> > iproute2, e.t.c.) so I completely don't understand the rationale behind
> > debugfs for OPA-VNIC.
> >
> > Thanks
> >
> Hi,
>
> The debugfs interface here is for VNIC port encapsulation configuration.
> They are not related to netdevice functionality.
>
> They are for debug purpose only and not the supported configuration mechanism.
> Hence, I think they probably shouldn't be in a tool like rdmatool.

Usually, there are two types of "debug":
 * One time during development and such thing shouldn't be part of upstream code.
 * During the daily work to understand misconfigured system and this is
   usable for all users of that feature.

There are exceptions to this rule, but they are not relevant for the
general ULP which is supposed to work on all OPA-capable devices.

Thanks

>
> Niranjana
>
> > >
> > > Thanks
> >
> >
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Doug Ledford]

On Tue, 2017-09-26 at 11:12 -0700, Vishwanathapura, Niranjana wrote:
> The debugfs interface here is for VNIC port encapsulation
> configuration.
> They are not related to netdevice functionality.
> 
> They are for debug purpose only and not the supported configuration
> mechanism.

OK, if this isn't the supported configuration mechanism, then what is
the supported mechanism?

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Vishwanathapura, Niranjana]

On Wed, Sep 27, 2017 at 11:26:17AM -0400, Doug Ledford wrote:
>On Tue, 2017-09-26 at 11:12 -0700, Vishwanathapura, Niranjana wrote:
>> The debugfs interface here is for VNIC port encapsulation
>> configuration.
>> They are not related to netdevice functionality.
>>
>> They are for debug purpose only and not the supported configuration
>> mechanism.
>
>OK, if this isn't the supported configuration mechanism, then what is
>the supported mechanism?
>

The supported mechanism is the Ethernet Manager providing the encapsulation 
configuration (as in the opa_vnic.txt documentation)
This is just a debug interface to it.

Niranjana

>-- >Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>    GPG KeyID: B826A3330E572FDD
>    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On Thu, Sep 28, 2017 at 11:47:28AM -0700, Vishwanathapura, Niranjana wrote:
> On Wed, Sep 27, 2017 at 11:26:17AM -0400, Doug Ledford wrote:
> > On Tue, 2017-09-26 at 11:12 -0700, Vishwanathapura, Niranjana wrote:
> > > The debugfs interface here is for VNIC port encapsulation
> > > configuration.
> > > They are not related to netdevice functionality.
> > >
> > > They are for debug purpose only and not the supported configuration
> > > mechanism.
> >
> > OK, if this isn't the supported configuration mechanism, then what is
> > the supported mechanism?
> >
>
> The supported mechanism is the Ethernet Manager providing the encapsulation
> configuration (as in the opa_vnic.txt documentation)
> This is just a debug interface to it.

Add tracepoint for that.

>
> Niranjana
>
> > -- >Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> >    GPG KeyID: B826A3330E572FDD
> >    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
> >

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Vishwanathapura, Niranjana]

On Thu, Sep 28, 2017 at 09:57:53PM +0300, Leon Romanovsky wrote:
>On Thu, Sep 28, 2017 at 11:47:28AM -0700, Vishwanathapura, Niranjana wrote:
>> On Wed, Sep 27, 2017 at 11:26:17AM -0400, Doug Ledford wrote:
>> > On Tue, 2017-09-26 at 11:12 -0700, Vishwanathapura, Niranjana wrote:
>> > > The debugfs interface here is for VNIC port encapsulation
>> > > configuration.
>> > > They are not related to netdevice functionality.
>> > >
>> > > They are for debug purpose only and not the supported configuration
>> > > mechanism.
>> >
>> > OK, if this isn't the supported configuration mechanism, then what is
>> > the supported mechanism?
>> >
>>
>> The supported mechanism is the Ethernet Manager providing the encapsulation
>> configuration (as in the opa_vnic.txt documentation)
>> This is just a debug interface to it.
>
>Add tracepoint for that.
>

For some reason, I couldn't find Leon's earlier response on my mutt! Will 
repond here.

It is not really for tracing, but to have a simple debug read/write access to 
the encapsution configuration which allows us to triage issues.

We find it useful on an ongoing basis, more like some standard NICs providing 
debugfs interface to read/write from/to some of its registers.

It is much easier to have it upstreamed and available instead of maintaining 
these patches separately.

Niranjana

>>
>> Niranjana
>>
>> > -- >Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> >    GPG KeyID: B826A3330E572FDD
>> >    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>> >


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

On Thu, Sep 28, 2017 at 12:05:02PM -0700, Vishwanathapura, Niranjana wrote:
> On Thu, Sep 28, 2017 at 09:57:53PM +0300, Leon Romanovsky wrote:
> > On Thu, Sep 28, 2017 at 11:47:28AM -0700, Vishwanathapura, Niranjana wrote:
> > > On Wed, Sep 27, 2017 at 11:26:17AM -0400, Doug Ledford wrote:
> > > > On Tue, 2017-09-26 at 11:12 -0700, Vishwanathapura, Niranjana wrote:
> > > > > The debugfs interface here is for VNIC port encapsulation
> > > > > configuration.
> > > > > They are not related to netdevice functionality.
> > > > >
> > > > > They are for debug purpose only and not the supported configuration
> > > > > mechanism.
> > > >
> > > > OK, if this isn't the supported configuration mechanism, then what is
> > > > the supported mechanism?
> > > >
> > >
> > > The supported mechanism is the Ethernet Manager providing the encapsulation
> > > configuration (as in the opa_vnic.txt documentation)
> > > This is just a debug interface to it.
> >
> > Add tracepoint for that.
> >
>
> For some reason, I couldn't find Leon's earlier response on my mutt! Will
> repond here.
>
> It is not really for tracing, but to have a simple debug read/write access
> to the encapsution configuration which allows us to triage issues.
>
> We find it useful on an ongoing basis, more like some standard NICs
> providing debugfs interface to read/write from/to some of its registers.

Usually if such interface is spotted during submission, it will be requested
to drop it. Ask Salil, who lately upstreamed new ETH driver and was asked
to remove it.

>
> It is much easier to have it upstreamed and available instead of maintaining
> these patches separately.

See, I didn't hear anyone here against adding such ability, but if you
want to hear such, you can always CC netdev :).

The question, we are asking is related to the interface chosen to do so - debugfs, and IMHO it is a mistake.

Thanks

>
> Niranjana
>
> > >
> > > Niranjana
> > >
> > > > -- >Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > >    GPG KeyID: B826A3330E572FDD
> > > >    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
> > > >
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Jason Gunthorpe]

On Thu, Sep 28, 2017 at 10:37:40PM +0300, Leon Romanovsky wrote:

> > We find it useful on an ongoing basis, more like some standard NICs
> > providing debugfs interface to read/write from/to some of its registers.
> 
> Usually if such interface is spotted during submission, it will be requested
> to drop it. Ask Salil, who lately upstreamed new ETH driver and was asked
> to remove it.

To be fair Salil's was dropped more or less for security reasons..

I think debugfs to dump truely internal state that is not intended to
be API stable is reasonable.

Promise you will not be tooling on top of this, or use it as an
interface in your manager or something crazy like that. You must be
happy in all cases if your distro chooses to disable debugfs.

The problem I have with reviewing all the vnic patches is that I don't
have a spec for vnic or anything, and I don't really know what the
data being dumped is.. eg I would have been much happier if ipoib
dump'd its debugfs stuff through netlink connected to neighbor objects
- I don't know if this is something similar...

As Leon says, you should really think carefully if this needs to be
something that admins are going to be told to look at when vnic isn't
working properly, as the ipoib debufs is. In that case netlink is a
stable API and more suitable.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

On Thu, Sep 28, 2017 at 02:02:58PM -0600, Jason Gunthorpe wrote:
> On Thu, Sep 28, 2017 at 10:37:40PM +0300, Leon Romanovsky wrote:
>
> > > We find it useful on an ongoing basis, more like some standard NICs
> > > providing debugfs interface to read/write from/to some of its registers.
> >
> > Usually if such interface is spotted during submission, it will be requested
> > to drop it. Ask Salil, who lately upstreamed new ETH driver and was asked
> > to remove it.
>
> To be fair Salil's was dropped more or less for security reasons..
>
> I think debugfs to dump truely internal state that is not intended to
> be API stable is reasonable.

Yes, it can be ripped in any point of life, despite the screaming.

>
> Promise you will not be tooling on top of this, or use it as an
> interface in your manager or something crazy like that. You must be
> happy in all cases if your distro chooses to disable debugfs.

Honestly, I don't know if we can trust to Intel's promises.

Among other promises, they wrote that OmniPath Ethernet Management part
will be open sourced too. It seems like it wasn't at the end.
My google searches with words "ethernet manager omni-path" revealed nothing.

>
> The problem I have with reviewing all the vnic patches is that I don't
> have a spec for vnic or anything, and I don't really know what the
> data being dumped is..

My problem with those patches that they don't show whole picture and the
more important you have no way to see it, if you want it.
Right now, it is legal code-sharing and not truly open-source which I would like to see.

> eg I would have been much happier if ipoib
> dump'd its debugfs stuff through netlink connected to neighbor objects
> - I don't know if this is something similar...
>
> As Leon says, you should really think carefully if this needs to be
> something that admins are going to be told to look at when vnic isn't
> working properly, as the ipoib debufs is. In that case netlink is a
> stable API and more suitable.
>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Dennis Dalessandro]

On 9/29/2017 1:49 AM, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 02:02:58PM -0600, Jason Gunthorpe wrote:
>> On Thu, Sep 28, 2017 at 10:37:40PM +0300, Leon Romanovsky wrote:
>>
>>>> We find it useful on an ongoing basis, more like some standard NICs
>>>> providing debugfs interface to read/write from/to some of its registers.
>>>
>>> Usually if such interface is spotted during submission, it will be requested
>>> to drop it. Ask Salil, who lately upstreamed new ETH driver and was asked
>>> to remove it.
>>
>> To be fair Salil's was dropped more or less for security reasons..
>>
>> I think debugfs to dump truely internal state that is not intended to
>> be API stable is reasonable.
> 
> Yes, it can be ripped in any point of life, despite the screaming.

We also can't count on debugfs always being available. To me that's the 
main problem with it.

>> Promise you will not be tooling on top of this, or use it as an
>> interface in your manager or something crazy like that. You must be
>> happy in all cases if your distro chooses to disable debugfs.
> 
> Honestly, I don't know if we can trust to Intel's promises.
 >
> 
> Among other promises, they wrote that OmniPath Ethernet Management part
> will be open sourced too. It seems like it wasn't at the end.
> My google searches with words "ethernet manager omni-path" revealed nothing.

I don't think we are "at the end" yet. The EM is a work in progress and 
*will* be open sourced. I don't work on that bit of code so I can't tell 
you when exactly.

All of this vnic stuff is a work in progress and we are incrementally 
pushing things out. Iterative development, involving the community and all.

>> The problem I have with reviewing all the vnic patches is that I don't
>> have a spec for vnic or anything, and I don't really know what the
>> data being dumped is..
> 
> My problem with those patches that they don't show whole picture and the
> more important you have no way to see it, if you want it.
> Right now, it is legal code-sharing and not truly open-source which I would like to see.
> 
>> eg I would have been much happier if ipoib
>> dump'd its debugfs stuff through netlink connected to neighbor objects
>> - I don't know if this is something similar...

Sounds like a good idea to me.

>> As Leon says, you should really think carefully if this needs to be
>> something that admins are going to be told to look at when vnic isn't
>> working properly, as the ipoib debufs is. In that case netlink is a
>> stable API and more suitable.

Suffice to say, I think we agree on that. Leon and I have talked 
recently and I made it clear my intention is to lessen our reliance on 
debugfs and rely more on netlink by way of rdma tool.

-Denny

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

On Fri, Sep 29, 2017 at 10:46:51AM -0400, Dennis Dalessandro wrote:
>
> All of this vnic stuff is a work in progress and we are incrementally
> pushing things out. Iterative development, involving the community and all.

Can we get OPA-VNIC specification? Can we get OPA specification?

The Documentation/infiniband/opa_vnic.txt is not enough to really
implement anything on top of that or communicate with that.

For example, what are the values and relations for routing control? How
is entropy calculated?

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Vishwanathapura, Niranjana]

On Fri, Sep 29, 2017 at 05:59:50PM +0300, Leon Romanovsky wrote:
>On Fri, Sep 29, 2017 at 10:46:51AM -0400, Dennis Dalessandro wrote:
>>
>> All of this vnic stuff is a work in progress and we are incrementally
>> pushing things out. Iterative development, involving the community and all.
>
>Can we get OPA-VNIC specification? Can we get OPA specification?
>
>The Documentation/infiniband/opa_vnic.txt is not enough to really
>implement anything on top of that or communicate with that.
>
>For example, what are the values and relations for routing control? How
>is entropy calculated?
>

You are putting a wrong interpretation to this patch.
EM provides the encapsulation configuration as in the opa_vnic documentation.
EM will be opensourced when it is ready (VNIC is development in progress).

As mentioned, this patch is a simple *debug* hook to that encapsulation 
configuration and no more.
That is why it is put under DEBUGFS.

I do see valid reason in Jason's and your earlier comments that it should a 
formal support to the admins.
But this patch was not mainly for that. I don't expect DEBUGFS to be turned on 
in all deployments.
I understand the question whether or not this patch should be in the kernel 
tree.
But it is a useful debug hook for debugging/triaging issues (similar to NIC 
having debug hook read/write some register), and convenient to have it 
available in the kernel than maintaining it separately.

Niranjana

>Thanks


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]

On Fri, Sep 29, 2017 at 08:27:08AM -0700, Vishwanathapura, Niranjana wrote:
> You are putting a wrong interpretation to this patch.
> EM provides the encapsulation configuration as in the opa_vnic documentation.
> EM will be opensourced when it is ready (VNIC is development in progress).
>
> As mentioned, this patch is a simple *debug* hook to that encapsulation
> configuration and no more.
> That is why it is put under DEBUGFS.
>
> I do see valid reason in Jason's and your earlier comments that it should a
> formal support to the admins.
> But this patch was not mainly for that. I don't expect DEBUGFS to be turned
> on in all deployments.
> I understand the question whether or not this patch should be in the kernel
> tree.
> But it is a useful debug hook for debugging/triaging issues (similar to NIC
> having debug hook read/write some register), and convenient to have it
> available in the kernel than maintaining it separately.

I want to summarize the responses:
1. If you want to know (read) which vport is configured by EM, the
tracepoints/prints will be enough to achieve it and no need to provide
any debugfs.
2. If you want to debug and configure (write) the vport, you should answer on two
questions.
2.a. Will it be local change and won't be propagated to EM?
2.b. Will this change be propagated to EM?

Upstream is not a development playground and you should submit your code once
you think that it is ready. So we are assuming that VNIC is working and
you are interested to debug your EM and such code doesn't belong to the kernel.

If you still insist on 2.a, the solution should be in your company: add
debugfs locally, write tests, find and fix bugs and submit them to
upstream.

There is no need to add "one-time" interface to clean the code.

Thanks

>
> Niranjana
>
> > Thanks
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Dennis Dalessandro]

On 10/2/2017 12:23 AM, Leon Romanovsky wrote:
> Upstream is not a development playground and you should submit your code once
> you think that it is ready. So we are assuming that VNIC is working and
> you are interested to debug your EM and such code doesn't belong to the kernel.

This patch aside, I do have to take some issue with this statement. 
While upstream is not a "playground" we should be submitting code early. 
I keep harping on iterative development, show your work. Granted that 
doesn't mean write broken code and toss it over the fence to kernel.org, 
but pieces that can be broken up and tested on their own are best.

> If you still insist on 2.a, the solution should be in your company: add
> debugfs locally, write tests, find and fix bugs and submit them to
> upstream.

Now back to this patch series. debugfs vs NetLink, I don't think it 
really matters if the rationale for having the patch in the first place 
is wrong. In other words you would have still opposed this even if it 
were NetLink I assume? We are discussing internally but for now I think 
this series can be dropped.

> There is no need to add "one-time" interface to clean the code.

That I would agree to.

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

On Mon, Oct 02, 2017 at 08:47:08PM -0400, Dennis Dalessandro wrote:
> On 10/2/2017 12:23 AM, Leon Romanovsky wrote:
> > Upstream is not a development playground and you should submit your code once
> > you think that it is ready. So we are assuming that VNIC is working and
> > you are interested to debug your EM and such code doesn't belong to the kernel.
>
> This patch aside, I do have to take some issue with this statement. While
> upstream is not a "playground" we should be submitting code early. I keep
> harping on iterative development, show your work. Granted that doesn't mean
> write broken code and toss it over the fence to kernel.org, but pieces that
> can be broken up and tested on their own are best.

We are talking about the same. Large feature (VNIC) was divided to
sub-tasks like patches and maybe sub-features. Those sub-features were
submitted early (iterative development), which is good and sounds right
for me too.

But the expectation is that these building blocks are tested and working
and you don't need to debug them.

For sure, there are always exceptions to the statements above and
various counters, states, dumps e.t.c are good examples for
such exceptions, because they add visibility to the system, but they
don't change the system.

>
> > If you still insist on 2.a, the solution should be in your company: add
> > debugfs locally, write tests, find and fix bugs and submit them to
> > upstream.
>
> Now back to this patch series. debugfs vs NetLink, I don't think it really
> matters if the rationale for having the patch in the first place is wrong.
> In other words you would have still opposed this even if it were NetLink I
> assume? We are discussing internally but for now I think this series can be
> dropped.

I am more than happy to see OPA-VNIC configurations in NetLink, but
don't forget that it will be part of ABI and users will use it, so you
need to ensure that the OPA-VNIC network won't die after one of the
nodes will change vport value.

Before you are rushing to implement it via netlink, you should ask
yourself WHY this functionality is needed for the users.

>
> > There is no need to add "one-time" interface to clean the code.
>
> That I would agree to.
>
> -Denny

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH for-next 0/2] IB/opa_vnic: Add debugfs interface

[Vishwanathapura, Niranjana]

On Tue, Oct 03, 2017 at 08:15:21AM +0300, Leon Romanovsky wrote:
>On Mon, Oct 02, 2017 at 08:47:08PM -0400, Dennis Dalessandro wrote:
>> On 10/2/2017 12:23 AM, Leon Romanovsky wrote:
>> > Upstream is not a development playground and you should submit your code once
>> > you think that it is ready. So we are assuming that VNIC is working and
>> > you are interested to debug your EM and such code doesn't belong to the kernel.
>>
>> This patch aside, I do have to take some issue with this statement. While
>> upstream is not a "playground" we should be submitting code early. I keep
>> harping on iterative development, show your work. Granted that doesn't mean
>> write broken code and toss it over the fence to kernel.org, but pieces that
>> can be broken up and tested on their own are best.
>
>We are talking about the same. Large feature (VNIC) was divided to
>sub-tasks like patches and maybe sub-features. Those sub-features were
>submitted early (iterative development), which is good and sounds right
>for me too.
>
>But the expectation is that these building blocks are tested and working
>and you don't need to debug them.
>
>For sure, there are always exceptions to the statements above and
>various counters, states, dumps e.t.c are good examples for
>such exceptions, because they add visibility to the system, but they
>don't change the system.
>
>>
>> > If you still insist on 2.a, the solution should be in your company: add
>> > debugfs locally, write tests, find and fix bugs and submit them to
>> > upstream.
>>
>> Now back to this patch series. debugfs vs NetLink, I don't think it really
>> matters if the rationale for having the patch in the first place is wrong.
>> In other words you would have still opposed this even if it were NetLink I
>> assume? We are discussing internally but for now I think this series can be
>> dropped.
>
>I am more than happy to see OPA-VNIC configurations in NetLink, but
>don't forget that it will be part of ABI and users will use it, so you
>need to ensure that the OPA-VNIC network won't die after one of the
>nodes will change vport value.
>
>Before you are rushing to implement it via netlink, you should ask
>yourself WHY this functionality is needed for the users.
>

Alright, let us drop this VNIC debugfs patch series.

Niranjana

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html