Skip to contentSkip to conversation

IB 64 bit counter support V2

PATCH In review Christoph Lameter 4 participants 17 messages Dec 17, 2015 – Jan 4, 2016 13 messages not in the archive InfiniBand

lore
Share & tools
lore

Christoph Lameter

author
#0 lore raw

cover IB 64 bit counter support V2

V1->V2 Add detection of the capability for 64 bit counter support and lots
of improvements as a result of suggestions by Hal Rosenstock.

Currently we only use 32 bits for the packet and byte counters. There have been extended countes available for some time but we have no support for those yet upstream. We keep having issues with 32 bit counters wrapping. Especially the byte counter can wrap frequently (as in multiple times per minute)

This patch adds 4 new counters and updates 4 32 bit counters to use the 64 bit sizes so that they no longer wrap.

Should the device not support 64 bit counters then only the original 32 bit counters will be visible.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#1 lore raw

PATCH 1/3 IB Core: Create get_perf_mad function in sysfs.c

Create a new function to retrieve performance management data from the existing code in get_pma_counter().

Signed-off-by Christoph Lameter <[email protected]>
drivers/infiniband/core/sysfs.c +40 -22
Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
@@ -317,21 +317,21 @@ struct port_table_attribute port_pma_att
 	.index = (_offset) | ((_width) << 16) | ((_counter) << 24)	\
 }
 
-static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
-				char *buf)
+/*
+ * Get a Perfmgmt MAD block of data.
+ * Returns error code or the number of bytes retrieved.
+ */
+static int get_perf_mad(struct ib_device *dev, int port_num, int attr,
+		void *data, int offset, size_t size)
 {
-	struct port_table_attribute *tab_attr =
-		container_of(attr, struct port_table_attribute, attr);
-	int offset = tab_attr->index & 0xffff;
-	int width  = (tab_attr->index >> 16) & 0xff;
-	struct ib_mad *in_mad  = NULL;
-	struct ib_mad *out_mad = NULL;
+	struct ib_mad *in_mad;
+	struct ib_mad *out_mad;
 	size_t mad_size = sizeof(*out_mad);
 	u16 out_mad_pkey_index = 0;
 	ssize_t ret;
 
-	if (!p->ibdev->process_mad)
-		return sprintf(buf, "N/A (no PMA)\n");
+	if (!dev->process_mad)
+		return -ENOSYS;
 
 	in_mad  = kzalloc(sizeof *in_mad, GFP_KERNEL);
 	out_mad = kmalloc(sizeof *out_mad, GFP_KERNEL);

@@ -344,12 +344,12 @@ static ssize_t show_pma_counter(struct i
 	in_mad->mad_hdr.mgmt_class    = IB_MGMT_CLASS_PERF_MGMT;
 	in_mad->mad_hdr.class_version = 1;
 	in_mad->mad_hdr.method        = IB_MGMT_METHOD_GET;
-	in_mad->mad_hdr.attr_id       = cpu_to_be16(0x12); /* PortCounters */
+	in_mad->mad_hdr.attr_id       = attr;
 
-	in_mad->data[41] = p->port_num;	/* PortSelect field */
+	in_mad->data[41] = port_num;	/* PortSelect field */
 
-	if ((p->ibdev->process_mad(p->ibdev, IB_MAD_IGNORE_MKEY,
-		 p->port_num, NULL, NULL,
+	if ((dev->process_mad(dev, IB_MAD_IGNORE_MKEY,
+		 port_num, NULL, NULL,
 		 (const struct ib_mad_hdr *)in_mad, mad_size,
 		 (struct ib_mad_hdr *)out_mad, &mad_size,
 		 &out_mad_pkey_index) &

@@ -358,31 +358,49 @@ static ssize_t show_pma_counter(struct i
 		ret = -EINVAL;
 		goto out;
 	}
+	memcpy(data, out_mad->data + offset, size);
+	ret = size;
+out:
+	kfree(in_mad);
+	kfree(out_mad);
+	return ret;
+}
+
+static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
+				char *buf)
+{
+	struct port_table_attribute *tab_attr =
+		container_of(attr, struct port_table_attribute, attr);
+	int offset = tab_attr->index & 0xffff;
+	int width  = (tab_attr->index >> 16) & 0xff;
+	ssize_t ret;
+	u8 data[8];
+
+	ret = get_perf_mad(p->ibdev, p->port_num, cpu_to_be16(0x12), &data,
+			40 + offset / 8, sizeof(data));
+	if (ret < 0)
+		return sprintf(buf, "N/A (no PMA)\n");
 
 	switch (width) {
 	case 4:
-		ret = sprintf(buf, "%u\n", (out_mad->data[40 + offset / 8] >>
+		ret = sprintf(buf, "%u\n", (*data >>
 					    (4 - (offset % 8))) & 0xf);
 		break;
 	case 8:
-		ret = sprintf(buf, "%u\n", out_mad->data[40 + offset / 8]);
+		ret = sprintf(buf, "%u\n", *data);
 		break;
 	case 16:
 		ret = sprintf(buf, "%u\n",
-			      be16_to_cpup((__be16 *)(out_mad->data + 40 + offset / 8)));
+			      be16_to_cpup((__be16 *)data));
 		break;
 	case 32:
 		ret = sprintf(buf, "%u\n",
-			      be32_to_cpup((__be32 *)(out_mad->data + 40 + offset / 8)));
+			      be32_to_cpup((__be32 *)data));
 		break;
 	default:
 		ret = 0;
 	}
 
-out:
-	kfree(in_mad);
-	kfree(out_mad);
-
 	return ret;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by
#3 lore raw

Re: [PATCH 1/3] IB Core: Create get_perf_mad function in sysfs.c

Quoted 4 lines · Christoph Lameter

Create a new function to retrieve performance management data from the existing code in get_pma_counter().

Signed-off-by Christoph Lameter <[email protected]>
Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

Re: [PATCH 1/3] IB Core: Create get_perf_mad function in sysfs.c

Quoted 9 lines · Bart Van Assche

drivers/infiniband/core/sysfs.c static ssize_t show_pma_counter(struct i · line 347

Quoted 2 lines · Christoph Lameter
drivers/infiniband/core/sysfs.c +1 -1
-	in_mad->mad_hdr.attr_id       = cpu_to_be16(0x12); /* PortCounters */
+	in_mad->mad_hdr.attr_id       = attr;

Hello Christoph,

sparse reports an endianness mismatch for this and similar assignments (make M=drivers/infiniband/core C=2 CF=-D__CHECK_ENDIAN__). Can you have a look at this ?

quick patch sent:

https://www.mail-archive.com/[email protected]/msg31257.html

Ira

Quoted 4 lines

Thanks,

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#7 lore raw

PATCH 2/3 IB core counters: Specify attribute_id in port_table_attribute

In reply to Christoph Lameter

Add the attr_id on port_table_attribute since we will have to add a different port_table_attribute for the extended attribute soon.

Signed-off-by Christoph Lameter <[email protected]>
drivers/infiniband/core/sysfs.c +5 -2
Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
@@ -39,6 +39,7 @@
 #include <linux/string.h>
 
 #include <rdma/ib_mad.h>
+#include <rdma/ib_pma.h>
 
 struct ib_port {
 	struct kobject         kobj;

@@ -65,6 +66,7 @@ struct port_table_attribute {
 	struct port_attribute	attr;
 	char			name[8];
 	int			index;
+	int			attr_id;
 };
 
 static ssize_t port_attr_show(struct kobject *kobj,

@@ -314,7 +316,8 @@ static ssize_t show_port_pkey(struct ib_
 #define PORT_PMA_ATTR(_name, _counter, _width, _offset)			\
 struct port_table_attribute port_pma_attr_##_name = {			\
 	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
-	.index = (_offset) | ((_width) << 16) | ((_counter) << 24)	\
+	.index = (_offset) | ((_width) << 16) | ((_counter) << 24),	\
+	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
 /*

@@ -376,7 +379,7 @@ static ssize_t show_pma_counter(struct i
 	ssize_t ret;
 	u8 data[8];
 
-	ret = get_perf_mad(p->ibdev, p->port_num, cpu_to_be16(0x12), &data,
+	ret = get_perf_mad(p->ibdev, p->port_num, tab_attr->attr_id, &data,
 			40 + offset / 8, sizeof(data));
 	if (ret < 0)
 		return sprintf(buf, "N/A (no PMA)\n");

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Reviewed-by
#9 lore raw

Re: [PATCH 2/3] IB core counters: Specify attribute_id in port_table_attribute

Quoted 4 lines · Christoph Lameter

Add the attr_id on port_table_attribute since we will have to add a different port_table_attribute for the extended attribute soon.

Signed-off-by Christoph Lameter <[email protected]>
Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#10 lore raw

PATCH 3/3 bject: IB Core: Display extended counter set if available

In reply to Christoph Lameter

Check if the extended counters are available and if so create the proper extended and additional counters.

Signed-off-by Christoph Lameter <[email protected]>
drivers/infiniband/core/sysfs.c +80 -2
Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_att
 	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)			\
+struct port_table_attribute port_pma_attr_ext_##_name = {		\
+	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
+	.index = (_offset) | ((_width) << 16),				\
+	.attr_id = IB_PMA_PORT_COUNTERS_EXT ,				\
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -349,7 +356,8 @@ static int get_perf_mad(struct ib_device
 	in_mad->mad_hdr.method        = IB_MGMT_METHOD_GET;
 	in_mad->mad_hdr.attr_id       = attr;
 
-	in_mad->data[41] = port_num;	/* PortSelect field */
+	if (port_num)
+		in_mad->data[41] = port_num;	/* PortSelect field */
 
 	if ((dev->process_mad(dev, IB_MAD_IGNORE_MKEY,
 		 port_num, NULL, NULL,

@@ -400,6 +408,11 @@ static ssize_t show_pma_counter(struct i
 		ret = sprintf(buf, "%u\n",
 			      be32_to_cpup((__be32 *)data));
 		break;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)data));
+		break;
+
 	default:
 		ret = 0;
 	}

@@ -424,6 +437,18 @@ static PORT_PMA_ATTR(port_rcv_data
 static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data		    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data		    , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets	    , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets	    , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets	    , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets	    , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets	    , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets	    , 64, 512);
+
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,
 	&port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +469,40 @@ static struct attribute *pma_attrs[] = {
 	NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+	&port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+	.name  = "counters",
+	.attrs  = pma_attrs_ext
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +575,26 @@ err:
 	return NULL;
 }
 
+/*
+ * Check if the port supports the Extended Counters.
+ * Return error code of 0 for success
+ */
+static int port_check_extended_counters(struct ib_device *dev)
+{
+	int ret = 0;
+	struct ib_class_port_info cpi;
+
+	ret = get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));
+
+	if (ret >= 0) {
+		if (!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) &&
+			!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF))
+			ret = -ENOSYS;
+	}
+
+	return ret;
+}
+
 static int add_port(struct ib_device *device, int port_num,
 		    int (*port_callback)(struct ib_device *,
 					 u8, struct kobject *))

@@ -549,7 +623,11 @@ static int add_port(struct ib_device *de
 		return ret;
 	}
 
-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		port_check_extended_counters(device) ?
+			&pma_group_ext :
+			&pma_group);
+
 	if (ret)
 		goto err_put;
include/rdma/ib_pma.h +1 -0
Index: linux/include/rdma/ib_pma.h
===================================================================
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 
 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#12 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 133 lines · Christoph Lameter

Check if the extended counters are available and if so create the proper extended and additional counters.

Signed-off-by Christoph Lameter <[email protected]>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================

drivers/infiniband/core/sysfs.c +75 -1
@@ -320,6 +320,13 @@ struct port_table_attribute port_pma_att
 	.attr_id = IB_PMA_PORT_COUNTERS ,				\
 }
 
+#define PORT_PMA_ATTR_EXT(_name, _width, _offset)			\
+struct port_table_attribute port_pma_attr_ext_##_name = {		\
+	.attr  = __ATTR(_name, S_IRUGO, show_pma_counter, NULL),	\
+	.index = (_offset) | ((_width) << 16),				\
+	.attr_id = IB_PMA_PORT_COUNTERS_EXT ,				\
+}
+
 /*
  * Get a Perfmgmt MAD block of data.
  * Returns error code or the number of bytes retrieved.

@@ -349,7 +356,8 @@ static int get_perf_mad(struct ib_device
 	in_mad->mad_hdr.method        = IB_MGMT_METHOD_GET;
 	in_mad->mad_hdr.attr_id       = attr;
 
-	in_mad->data[41] = port_num;	/* PortSelect field */
+	if (port_num)
+		in_mad->data[41] = port_num;	/* PortSelect field */
 
 	if ((dev->process_mad(dev, IB_MAD_IGNORE_MKEY,
 		 port_num, NULL, NULL,

@@ -400,6 +408,11 @@ static ssize_t show_pma_counter(struct i
 		ret = sprintf(buf, "%u\n",
 			      be32_to_cpup((__be32 *)data));
 		break;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)data));
+		break;
+
 	default:
 		ret = 0;
 	}

@@ -424,6 +437,18 @@ static PORT_PMA_ATTR(port_rcv_data
 static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256);
 static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288);
 
+/*
+ * Counters added by extended set
+ */
+static PORT_PMA_ATTR_EXT(port_xmit_data		    , 64,  64);
+static PORT_PMA_ATTR_EXT(port_rcv_data		    , 64, 128);
+static PORT_PMA_ATTR_EXT(port_xmit_packets	    , 64, 192);
+static PORT_PMA_ATTR_EXT(port_rcv_packets	    , 64, 256);
+static PORT_PMA_ATTR_EXT(unicast_xmit_packets	    , 64, 320);
+static PORT_PMA_ATTR_EXT(unicast_rcv_packets	    , 64, 384);
+static PORT_PMA_ATTR_EXT(multicast_xmit_packets	    , 64, 448);
+static PORT_PMA_ATTR_EXT(multicast_rcv_packets	    , 64, 512);
+
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,
 	&port_pma_attr_link_error_recovery.attr.attr,

@@ -444,11 +469,40 @@ static struct attribute *pma_attrs[] = {
 	NULL
 };
 
+static struct attribute *pma_attrs_ext[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_unicast_xmit_packets.attr.attr,
+	&port_pma_attr_ext_multicast_rcv_packets.attr.attr,
+	&port_pma_attr_ext_multicast_xmit_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs
 };
 
+static struct attribute_group pma_group_ext = {
+	.name  = "counters",
+	.attrs  = pma_attrs_ext
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -521,6 +575,26 @@ err:
 	return NULL;
 }
 
+/*
+ * Check if the port supports the Extended Counters.
+ * Return error code of 0 for success
+ */
+static int port_check_extended_counters(struct ib_device *dev)
+{
+	int ret = 0;
+	struct ib_class_port_info cpi;
+
+	ret = get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));
+
+	if (ret >= 0) {
+		if (!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) &&
+			!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF))
+			ret = -ENOSYS;
+	}
+
+	return ret;
+}
+
 static int add_port(struct ib_device *device, int port_num,
 		    int (*port_callback)(struct ib_device *,
 					 u8, struct kobject *))

drivers/infiniband/core/sysfs.c static int add_port(struct ib_device *de · line 628

Quoted 7 lines
drivers/infiniband/core/sysfs.c +3 -1
@@ -549,7 +623,11 @@ static int add_port(struct ib_device *de
 		return ret;
 	}un
 
-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		port_check_extended_counters(device) ?
+			&pma_group_ext :

Would be nice to populate 2 different groups based on whether PMA supports full extended counters or extended counters without the IETF ones (no [uni mcast] [rcv xmit] pkt counters) in sysfs.

-- Hal

Quoted 11 lines
drivers/infiniband/core/sysfs.c +2 -0
+			&pma_group);
+
 	if (ret)
 		goto err_put;

Index: linux/include/rdma/ib_pma.h
===================================================================

include/rdma/ib_pma.h line 48

Quoted 7 lines
include/rdma/ib_pma.h +1 -0
@@ -42,6 +42,7 @@
  */
 #define IB_PMA_CLASS_CAP_ALLPORTSELECT  cpu_to_be16(1 << 8)
 #define IB_PMA_CLASS_CAP_EXT_WIDTH      cpu_to_be16(1 << 9)
+#define IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF cpu_to_be16(1 << 10)
 #define IB_PMA_CLASS_CAP_XMIT_WAIT      cpu_to_be16(1 << 12)
 
 #define IB_PMA_CLASS_PORT_INFO          cpu_to_be16(0x0001)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 567316F7.9030707-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

author
#14 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 8 lines · Hal Rosenstock

drivers/infiniband/core/sysfs.c static int add_port(struct ib_device *de · line 628

Quoted 4 lines
drivers/infiniband/core/sysfs.c +3 -1
-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		port_check_extended_counters(device) ?
+			&pma_group_ext :

Would be nice to populate 2 different groups based on whether PMA supports full extended counters or extended counters without the IETF ones (no [uni mcast] [rcv xmit] pkt counters) in sysfs.

So port_check_extended_counters need to return another value for this. The IETF ones are the uni/mcast xxx counters?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): alpine.DEB.2.20.1512171527060.22782-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org

#16 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 2 lines · Christoph Lameter

So port_check_extended_counters need to return another value for this. The IETF ones are the uni/mcast xxx counters?

Yes
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 56732D97.2080303-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

author
#18 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 5 lines · Hal Rosenstock
Quoted 2 lines · Christoph Lameter

So port_check_extended_counters need to return another value for this. The IETF ones are the uni/mcast xxx counters?

Yes

Ok. Then this patch on top of the last one should give us all of what you want:

Subject: IB core counters: Support noietf extended counters

Detect if we have extended counters but not IETF counters. For that we need a special table and create a function that returns the table address.

Signed-off-by Christoph Lameter <[email protected]>
drivers/infiniband/core/sysfs.c +43 -13
Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
@@ -493,6 +493,26 @@ static struct attribute *pma_attrs_ext[]
 	NULL
 };
 
+static struct attribute *pma_attrs_noietf[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs

@@ -503,6 +523,11 @@ static struct attribute_group pma_group_
 	.attrs  = pma_attrs_ext
 };
 
+static struct attribute_group pma_group_noietf = {
+	.name  = "counters",
+	.attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -576,23 +601,32 @@ err:
 }
 
 /*
- * Check if the port supports the Extended Counters.
- * Return error code of 0 for success
+ * Figure out which counter table to use depending on
+ * the device capabilities.
  */
-static int port_check_extended_counters(struct ib_device *dev)
+static struct attribute_group *get_counter_table(struct ib_device *dev)
 {
 	int ret = 0;
 	struct ib_class_port_info cpi;
 
 	ret = get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));
 
-	if (ret >= 0) {
-		if (!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) &&
-			!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF))
-			ret = -ENOSYS;
+	if (ret < 0)
+		/* Fall back to normal counters */
+		return &pma_group;
+
+
+	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
+		/* We have extended counters */
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;
+
+		return &pma_group_ext;
 	}
 
-	return ret;
+	return &pma_group;
 }
 
 static int add_port(struct ib_device *device, int port_num,

@@ -623,11 +657,7 @@ static int add_port(struct ib_device *de
 		return ret;
 	}
 
-	ret = sysfs_create_group(&p->kobj,
-		port_check_extended_counters(device) ?
-			&pma_group_ext :
-			&pma_group);
-
+	ret = sysfs_create_group(&p->kobj, get_counter_table(device));
 	if (ret)
 		goto err_put;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): alpine.DEB.2.20.1512171649070.22782-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org

#20 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 86 lines · Christoph Lameter
Quoted 5 lines · Hal Rosenstock
Quoted 2 lines · Christoph Lameter

So port_check_extended_counters need to return another value for this. The IETF ones are the uni/mcast xxx counters?

Yes

Ok. Then this patch on top of the last one should give us all of what you want:

Subject: IB core counters: Support noietf extended counters

Detect if we have extended counters but not IETF counters. For that we need a special table and create a function that returns the table address.

Signed-off-by Christoph Lameter <[email protected]>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================

drivers/infiniband/core/sysfs.c +29 -7
@@ -493,6 +493,26 @@ static struct attribute *pma_attrs_ext[]
 	NULL
 };
 
+static struct attribute *pma_attrs_noietf[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs

@@ -503,6 +523,11 @@ static struct attribute_group pma_group_
 	.attrs  = pma_attrs_ext
 };
 
+static struct attribute_group pma_group_noietf = {
+	.name  = "counters",
+	.attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -576,23 +601,32 @@ err:
 }
 
 /*
- * Check if the port supports the Extended Counters.
- * Return error code of 0 for success
+ * Figure out which counter table to use depending on
+ * the device capabilities.
  */
-static int port_check_extended_counters(struct ib_device *dev)
+static struct attribute_group *get_counter_table(struct ib_device *dev)
 {
 	int ret = 0;
 	struct ib_class_port_info cpi;
 
 	ret = get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));
 
-	if (ret >= 0) {
-		if (!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) &&
-			!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF))
-			ret = -ENOSYS;
+	if (ret < 0)

drivers/infiniband/core/sysfs.c err: · line 624

Quoted 10 lines
drivers/infiniband/core/sysfs.c +10 -0
+		/* Fall back to normal counters */
+		return &pma_group;
+
+
+	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
+		/* We have extended counters */
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;

These 2 capability bits are mutually exclusive so I think it should be:

	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
		/* We have extended counters */
		return &pma_group_ext;

}

	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
		/* But not the IETF ones */
		return &pma_group_noietf;
	}

	return &pma_group;
Quoted 13 lines
drivers/infiniband/core/sysfs.c +3 -1
+
+		return &pma_group_ext;
 	}
 
-	return ret;
+	return &pma_group;
 }
 
 static int add_port(struct ib_device *device, int port_num,

@@ -623,11 +657,7 @@ static int add_port(struct ib_device *de
 		return ret;
 	}

drivers/infiniband/core/sysfs.c static int add_port(struct ib_device *de · line 663

Quoted 10 lines
drivers/infiniband/core/sysfs.c +1 -5
 
-	ret = sysfs_create_group(&p->kobj,
-		port_check_extended_counters(device) ?
-			&pma_group_ext :
-			&pma_group);
-
+	ret = sysfs_create_group(&p->kobj, get_counter_table(device));
 	if (ret)
 		goto err_put;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 567342F5.5090000-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

author
#22 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 9 lines · Hal Rosenstock

drivers/infiniband/core/sysfs.c err: · line 624

Quoted 6 lines
drivers/infiniband/core/sysfs.c +6 -0
+	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
+		/* We have extended counters */
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;

These 2 capability bits are mutually exclusive so I think it should be:

Quoted 8 lines
	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
		/* We have extended counters */
		return &pma_group_ext;

}

	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
		/* But not the IETF ones */
		return &pma_group_noietf;

This case would then use the 64 bit counters despite of the IB_PMA_CLASS_CAP_EXT_WIDTH not being set.

Quoted 5 lines
	}

	return &pma_group;
Quoted 1 line

+

The tables contain all the counters each. So we would need another table of counters that has the ietf counters but not the 64 bit extended ones?

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): alpine.DEB.2.20.1512180737150.32514-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org

#24 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 16 lines · Christoph Lameter
Quoted 14 lines · Hal Rosenstock

drivers/infiniband/core/sysfs.c err: · line 624

Quoted 6 lines
drivers/infiniband/core/sysfs.c +6 -0
+	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
+		/* We have extended counters */
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;

These 2 capability bits are mutually exclusive so I think it should be:

	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) {
		/* We have extended counters */
		return &pma_group_ext;

}

Quoted 6 lines
Quoted 3 lines
	if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
		/* But not the IETF ones */
		return &pma_group_noietf;

This case would then use the 64 bit counters despite of the IB_PMA_CLASS_CAP_EXT_WIDTH not being set.

Yes, IB_PMA_CLASS_CAP_EXT_WIDTH means all extended counters including IETF ones whereas IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF means extended counters without IETF ones ([uni multi]cast [rcv xmit] pkts).

Quoted 9 lines
Quoted 6 lines
	}

	return &pma_group;
Quoted 1 line

+

The tables contain all the counters each. So we would need another table of counters that has the ietf counters but not the 64 bit extended ones?

I'm not following what you mean. I thought pma_group_noietf and pma_group_ext take care of the 2 different options for extended counters (with the error counters needed from the mandatory PortCounters) and pma_group is when neither of the extended counter capabilities is indicated.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 56740F8C.4060703-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org

author
#26 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 6 lines · Hal Rosenstock
Quoted 2 lines

This case would then use the 64 bit counters despite of the IB_PMA_CLASS_CAP_EXT_WIDTH not being set.

Yes, IB_PMA_CLASS_CAP_EXT_WIDTH means all extended counters including IETF ones whereas IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF means extended counters without IETF ones ([uni multi]cast [rcv xmit] pkts).

Ok so I updated the add on patch to the following. Doug: Is this enough or do you want another rollup?

From: Christoph Lameter <[email protected]>
Subject: IB core counters: Support noietf extended counters V2

V1-V2: Fix logic to detect when 64 bit counter are available
based on Hal's suggestions.

Detect if we have extended counters but not IETF counters. For that we need a special table and create a function that returns the table address.

Signed-off-by Christoph Lameter <[email protected]>
drivers/infiniband/core/sysfs.c +39 -12
Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
@@ -493,6 +493,26 @@ static struct attribute *pma_attrs_ext[]
 	NULL
 };
 
+static struct attribute *pma_attrs_noietf[] = {
+	&port_pma_attr_symbol_error.attr.attr,
+	&port_pma_attr_link_error_recovery.attr.attr,
+	&port_pma_attr_link_downed.attr.attr,
+	&port_pma_attr_port_rcv_errors.attr.attr,
+	&port_pma_attr_port_rcv_remote_physical_errors.attr.attr,
+	&port_pma_attr_port_rcv_switch_relay_errors.attr.attr,
+	&port_pma_attr_port_xmit_discards.attr.attr,
+	&port_pma_attr_port_xmit_constraint_errors.attr.attr,
+	&port_pma_attr_port_rcv_constraint_errors.attr.attr,
+	&port_pma_attr_local_link_integrity_errors.attr.attr,
+	&port_pma_attr_excessive_buffer_overrun_errors.attr.attr,
+	&port_pma_attr_VL15_dropped.attr.attr,
+	&port_pma_attr_ext_port_xmit_data.attr.attr,
+	&port_pma_attr_ext_port_rcv_data.attr.attr,
+	&port_pma_attr_ext_port_xmit_packets.attr.attr,
+	&port_pma_attr_ext_port_rcv_packets.attr.attr,
+	NULL
+};
+
 static struct attribute_group pma_group = {
 	.name  = "counters",
 	.attrs  = pma_attrs

@@ -503,6 +523,11 @@ static struct attribute_group pma_group_
 	.attrs  = pma_attrs_ext
 };
 
+static struct attribute_group pma_group_noietf = {
+	.name  = "counters",
+	.attrs  = pma_attrs_noietf
+};
+
 static void ib_port_release(struct kobject *kobj)
 {
 	struct ib_port *p = container_of(kobj, struct ib_port, kobj);

@@ -576,10 +601,10 @@ err:
 }
 
 /*
- * Check if the port supports the Extended Counters.
- * Return error code of 0 for success
+ * Figure out which counter table to use depending on
+ * the device capabilities.
  */
-static int port_check_extended_counters(struct ib_device *dev)
+static struct attribute_group *get_counter_table(struct ib_device *dev)
 {
 	int ret = 0;
 	struct ib_class_port_info cpi;

@@ -587,12 +612,18 @@ static int port_check_extended_counters(
 	ret = get_perf_mad(dev, 0, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));
 
 	if (ret >= 0) {
-		if (!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH) &&
-			!(cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF))
-			ret = -ENOSYS;
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH)
+			/* We have extended counters */
+			return &pma_group_ext;
+
+		if (cpi.capability_mask && IB_PMA_CLASS_CAP_EXT_WIDTH_NOIETF)
+			/* But not the IETF ones */
+			return &pma_group_noietf;
 	}
 
-	return ret;
+	/* Fall back to normal counters */
+	return &pma_group;
 }
 
 static int add_port(struct ib_device *device, int port_num,

@@ -623,11 +654,7 @@ static int add_port(struct ib_device *de
 		return ret;
 	}
 
-	ret = sysfs_create_group(&p->kobj,
-		port_check_extended_counters(device) ?
-			&pma_group_ext :
-			&pma_group);
-
+	ret = sysfs_create_group(&p->kobj, get_counter_table(device));
 	if (ret)
 		goto err_put;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): alpine.DEB.2.20.1512180944540.1506-wcBtFHqTun5QOdAKl3ChDw@public.gmane.org

Reviewed-by
#28 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

Quoted 10 lines · Christoph Lameter

Subject: IB core counters: Support noietf extended counters V2

V1-V2: Fix logic to detect when 64 bit counter are available
based on Hal's suggestions.

Detect if we have extended counters but not IETF counters. For that we need a special table and create a function that returns the table address.

Signed-off-by Christoph Lameter <[email protected]>
Reviewed-by Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/[email protected]>

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#29 lore raw

Re: [PATCH 3/3] bject: IB Core: Display extended counter set if available

In reply to a message not in the archive

There are number of minor issues:
Subject: Re: [PATCH 3/3] bject: IB Core: Display extended counter set if

                        ^^^^^^^

drivers/infiniband/core/sysfs.c static int get_perf_mad(struct ib_device · line 360

Quoted 3 lines · Christoph Lameter
drivers/infiniband/core/sysfs.c +2 -1
-	in_mad->data[41] = port_num;	/* PortSelect field */
+	if (port_num)
+		in_mad->data[41] = port_num;	/* PortSelect field */

in_mad was created with kzalloc with all fields zeroed by default, so you can drop if(port_num) condition.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html