IB 64 bit counter support

This is necessary to support the extended attributes which involves a different attribute id.

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

---
 drivers/infiniband/core/sysfs.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

drivers/infiniband/core/sysfs.c +22 -19

index b1f37d4..1c8716f 100644
@@ -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,

@@ -311,10 +313,11 @@ static ssize_t show_port_pkey(struct ib_port *p, struct port_attribute *attr,
 	return sprintf(buf, "0x%04x\n", pkey);
 }
 
-#define PORT_PMA_ATTR(_name, _counter, _width, _offset)			\
+#define PORT_PMA_ATTR(_name, _counter, _width, _offset, _attr_id)	\
 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 = _attr_id ,						\
 }
 
 static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,

@@ -344,7 +347,7 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 	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       = tab_attr->attr_id;
 
 	in_mad->data[41] = p->port_num;	/* PortSelect field */
 

@@ -386,22 +389,22 @@ out:
 	return ret;
 }
 
-static PORT_PMA_ATTR(symbol_error		    ,  0, 16,  32);
-static PORT_PMA_ATTR(link_error_recovery	    ,  1,  8,  48);
-static PORT_PMA_ATTR(link_downed		    ,  2,  8,  56);
-static PORT_PMA_ATTR(port_rcv_errors		    ,  3, 16,  64);
-static PORT_PMA_ATTR(port_rcv_remote_physical_errors,  4, 16,  80);
-static PORT_PMA_ATTR(port_rcv_switch_relay_errors   ,  5, 16,  96);
-static PORT_PMA_ATTR(port_xmit_discards		    ,  6, 16, 112);
-static PORT_PMA_ATTR(port_xmit_constraint_errors    ,  7,  8, 128);
-static PORT_PMA_ATTR(port_rcv_constraint_errors	    ,  8,  8, 136);
-static PORT_PMA_ATTR(local_link_integrity_errors    ,  9,  4, 152);
-static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10,  4, 156);
-static PORT_PMA_ATTR(VL15_dropped		    , 11, 16, 176);
-static PORT_PMA_ATTR(port_xmit_data		    , 12, 32, 192);
-static PORT_PMA_ATTR(port_rcv_data		    , 13, 32, 224);
-static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256);
-static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288);
+static PORT_PMA_ATTR(symbol_error		    ,  0, 16,  32, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(link_error_recovery	    ,  1,  8,  48, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(link_downed		    ,  2,  8,  56, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_errors		    ,  3, 16,  64, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_remote_physical_errors,  4, 16,  80, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_switch_relay_errors   ,  5, 16,  96, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_discards		    ,  6, 16, 112, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_constraint_errors    ,  7,  8, 128, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_constraint_errors	    ,  8,  8, 136, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(local_link_integrity_errors    ,  9,  4, 152, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10,  4, 156, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(VL15_dropped		    , 11, 16, 176, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_data		    , 12, 32, 192, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_data		    , 13, 32, 224, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288, IB_PMA_PORT_COUNTERS);
 
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,

Signature · 7 lines

2.5.0


--
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

Add a branch to display 64 bit values

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

---
 drivers/infiniband/core/sysfs.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/infiniband/core/sysfs.c +5 -0

index 1c8716f..0083a4f 100644
@@ -378,6 +378,11 @@ static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
 		ret = sprintf(buf, "%u\n",
 			      be32_to_cpup((__be32 *)(out_mad->data + 40 + offset / 8)));
 		break;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)(out_mad->data + 40 + offset / 8)));
+		break;
+
 	default:
 		ret = 0;
 	}

Signature · 7 lines

2.5.0


--
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

Display the additional 64 bit counters available through the extended set and replace the existing 32 bit counters if there is a 64 bit alternative available.

Note: This requires universal support of extended counters in the devices. If there are still devices around that do not support extended counters then we will have to add some fallback technique here.

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

---
 drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

drivers/infiniband/core/sysfs.c +12 -4

index 0083a4f..f7f2954 100644
@@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors	    ,  8,  8, 136, IB_PMA_PORT_C
 static PORT_PMA_ATTR(local_link_integrity_errors    ,  9,  4, 152, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10,  4, 156, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(VL15_dropped		    , 11, 16, 176, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_data		    , 12, 32, 192, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_rcv_data		    , 13, 32, 224, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_packets		    , 14, 32, 256, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_rcv_packets		    , 15, 32, 288, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_data		    ,  0, 64,  64, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_rcv_data		    ,  0, 64, 128, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_xmit_packets		    ,  0, 64, 192, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_rcv_packets		    ,  0, 64, 256, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(unicast_xmit_packets	    ,  0, 64, 320, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(unicast_rcv_packets	    ,  0, 64, 384, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_xmit_packets	    ,  0, 64, 448, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_rcv_packets	    ,  0, 64, 512, IB_PMA_PORT_COUNTERS_EXT);
 
 static struct attribute *pma_attrs[] = {
 	&port_pma_attr_symbol_error.attr.attr,

@@ -428,6 +432,10 @@ static struct attribute *pma_attrs[] = {
 	&port_pma_attr_port_rcv_data.attr.attr,
 	&port_pma_attr_port_xmit_packets.attr.attr,
 	&port_pma_attr_port_rcv_packets.attr.attr,
+	&port_pma_attr_unicast_rcv_packets.attr.attr,
+	&port_pma_attr_unicast_xmit_packets.attr.attr,
+	&port_pma_attr_multicast_rcv_packets.attr.attr,
+	&port_pma_attr_multicast_xmit_packets.attr.attr,
 	NULL
 };
 

Signature · 7 lines

2.5.0


--
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

qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that hardware.)

FWIW, I also hate the sysfs counters that reflect the PMA, these would be much better are free running, wrapping, non-resetting counters unrelated to the PMA. Something that doesn't zero after the SM samples it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO.

Quoted 5 lines · Jason Gunthorpe

FWIW, I also hate the sysfs counters that reflect the PMA, these would be much better are free running, wrapping, non-resetting counters unrelated to the PMA. Something that doesn't zero after the SM samples it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO.

To be fair with a 64bit counter these are not going to get reset very often.

Furthermore, I don't think we can change the behavior now.

Quoted 8 lines · ira.weiny

Quoted 5 lines · Jason Gunthorpe

FWIW, I also hate the sysfs counters that reflect the PMA, these would be much better are free running, wrapping, non-resetting counters unrelated to the PMA. Something that doesn't zero after the SM samples it. Sounds like qib, hif, rdmavt can trivially fix this, and should, IMHO.

To be fair with a 64bit counter these are not going to get reset very often.

It resets when ever the SM sends a reset packet, so 'whenever'

Quoted 2 lines

Furthermore, I don't think we can change the behavior now.

Sure we can, the restting is really a bug, to the point that nothing can actually use the sysfs counters reliably.

Quoted 2 lines

qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that hardware.)

I have no specifics to add, but I keep running into systems, even today, where the 64 bit counters don't work. The MAD might be there, but several counters are wired to 0.

Not sure exactly which HW though.

Mellanox should really confirm this for their hardware matrix.

Quoted 1 line

Mellanox should really confirm this for their hardware matrix.

I am trying to get definitive answer to this.

Quoted 3 lines · Hal Rosenstock

Quoted 1 line

Mellanox should really confirm this for their hardware matrix.

I am trying to get definitive answer to this.

I was told today on a conf call with a couple of Mellanox employees that extended counters are always available.

Quoted 6 lines · Christoph Lameter

Quoted 3 lines · Hal Rosenstock

Quoted 1 line

Mellanox should really confirm this for their hardware matrix.

I am trying to get definitive answer to this.

Quoted 2 lines

I was told today on a conf call with a couple of Mellanox employees that extended counters are always available.

.. and every field is valid?

I would agree, from my observation, that the two main byte counters are always available.

But not necessarily the others.

Jason

Quoted 2 lines

I would agree, from my observation, that the two main byte counters are always available.

The extended packet counts work but I thought there was a PMA with one of the extended byte counts wired to 0.

Quoted 1 line

But not necessarily the others.

The unicast/multicast extended counters are not always supported - depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Quoted 3 lines

The unicast/multicast extended counters are not always supported -

Quoted 2 lines

depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case.

Quoted 6 lines · Jason Gunthorpe

Quoted 3 lines

The unicast/multicast extended counters are not always supported -

Quoted 2 lines

depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case.

There are no 32 bit equivalents of those 4 "IETF" counters ([uni multi]cast [xmit rcv] pkts).

When not supported, perhaps it is best not to populate these counters in sysfs so one can discern between counter not supported and 0 value.

I'm still working on definitive mthca answer but think the attribute is not supported there. Does anyone out there have an mthca setup where they can try this ?

Quoted 11 lines · Hal Rosenstock

Quoted 6 lines · Jason Gunthorpe

Quoted 3 lines

The unicast/multicast extended counters are not always supported -

Quoted 2 lines

depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case.

There are no 32 bit equivalents of those 4 "IETF" counters ([uni multi]cast [xmit rcv] pkts).

Quoted 6 lines

When not supported, perhaps it is best not to populate these counters in sysfs so one can discern between counter not supported and 0 value.

I'm still working on definitive mthca answer but think the attribute is not supported there. Does anyone out there have an mthca setup where they can try this ?

Yes.

Quoted 11 lines · Hal Rosenstock

Quoted 6 lines · Jason Gunthorpe

Quoted 3 lines

The unicast/multicast extended counters are not always supported -

Quoted 2 lines

depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case.

There are no 32 bit equivalents of those 4 "IETF" counters ([uni multi]cast [xmit rcv] pkts).

Quoted 6 lines

When not supported, perhaps it is best not to populate these counters in sysfs so one can discern between counter not supported and 0 value.

I'm still working on definitive mthca answer but think the attribute is not supported there. Does anyone out there have an mthca setup where they can try this ?

Yes.

Quoted 3 lines

The unicast/multicast extended counters are not always supported - depends on setting of PerfMgt ClassPortInfo CapabilityMask.IsExtendedWidthSupportedNoIETF (bit 10).

Yes.. certainly this proposed patch needs to account for that and continue to use the 32 bit ones in that case.

From Christoph Lameter <[email protected]>

DRAFT: This is missing the check if this device supports extended counters.

Quoted 2 lines · Christoph Lameter

DRAFT: This is missing the check if this device supports extended counters.

Found some time and here is the patch with the detection of the extended attribute through sending a mad request. Untested. Got the info on how to do the proper mad request from an earlier patch by Or in 2011.

Subject: IB Core: Display extended counter set if available V2

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 +4 -3

@@ -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,24 +316,33 @@ 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 ,				\
 }
 
-static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
-				char *buf)

drivers/infiniband/core/sysfs.c +81 -20

+		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 +355,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 +369,54 @@ 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_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");
 
 	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;
+	case 64:
+		ret = sprintf(buf, "%llu\n",
+				be64_to_cpup((__be64 *)data));
 		break;
+
 	default:
 		ret = 0;
 	}
 
-out:
-	kfree(in_mad);
-	kfree(out_mad);
-
 	return ret;
 }
 

@@ -403,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,

@@ -423,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);

@@ -500,6 +575,26 @@ err:
 	return NULL;
 }
 

drivers/infiniband/core/sysfs.c +10 -0

+
+	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 +1 -0

+
 	if (ret)
 		goto err_put;
 

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

Quoted 1 line

+ * Get a MAD block of data.

Nit: Get PerfMgt MAD block of data

Quoted 3 lines

+ * Returns error code or the number of bytes retrieved.
+ */
+static int get_mad(struct ib_device *dev, int port_num, int attr,

Nit: Maybe this is too verbose but better name might be get_perf_mad

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

Quoted 6 lines

drivers/infiniband/core/sysfs.c +6 -0

+static int port_check_extended_counters(struct ib_device *dev, int port)
+{
+	int ret = 0;
+	struct ib_class_port_info cpi;
+
+	ret = get_mad(dev, port, IB_PMA_CLASS_PORT_INFO, &cpi, 40, sizeof(cpi));

ClassPortInfo is per class not per class per port so need to indicate to get_mad whether a port is supplied or not or conditionalize based on attr ID.

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

Quoted 5 lines

drivers/infiniband/core/sysfs.c +4 -1

-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		port_check_extended_counters(device, port_num) ?
+			&pma_group_ext :
+			&pma_group);

PortExtendedCounters does not have all the error counters in PortCounters so this isn't an either or. When extended port counters are supported should still include the original port counters with the exception of the [xmit rcv] [pkts data] which should come from the extended counters.

Quoted 3 lines · Hal Rosenstock

ClassPortInfo is per class not per class per port so need to indicate to get_mad whether a port is supplied or not or conditionalize based on attr ID.

I thought a port is always supplied since we get the info for a particular port and the directory only exists if there is a port?

Quoted 11 lines

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

Quoted 5 lines

drivers/infiniband/core/sysfs.c +4 -1

-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		port_check_extended_counters(device, port_num) ?
+			&pma_group_ext :
+			&pma_group);

PortExtendedCounters does not have all the error counters in PortCounters so this isn't an either or. When extended port counters are supported should still include the original port counters with the exception of the [xmit rcv] [pkts data] which should come from the extended counters.

The original port counters are still included. The _ext table refers to both extended and regular counters.

Quoted 2 lines

I thought a port is always supplied since we get the info for a particular port and the directory only exists if there is a port?

Yes, but there is no port (PortSelect) field in ClassPortInfo attribute unlike the PortCounters and PortExtendedCounters attributes.

Quoted 5 lines · Hal Rosenstock

Quoted 2 lines

I thought a port is always supplied since we get the info for a particular port and the directory only exists if there is a port?

Yes, but there is no port (PortSelect) field in ClassPortInfo attribute unlike the PortCounters and PortExtendedCounters attributes.

Ok but its valid for all ports on that class right? Then this does not matter?

Quoted 8 lines · Christoph Lameter

Display the additional 64 bit counters available through the extended set and replace the existing 32 bit counters if there is a 64 bit alternative available.

Note: This requires universal support of extended counters in the devices. If there are still devices around that do not support extended counters then we will have to add some fallback technique here.

Looks like ocrdma will break here.

I'm not sure about mthca.

qib, mlx4 are fine. mlx5 should be as well I would think (I don't have that hardware.)

hfi1 did not process these MADs previously as all the hardware counters are 64 bits. But with this patch series we would add it.

ehca, amso1100, and ipath are all gone so they don't matter.

I can whip up a patch for hfi1 and we have to wait for Doug to take over that driver anyway to make sure that the patch would apply. So I think you can ignore it.

ocrdma seems like it could be a quick patch pre this one.

Ira

Quoted 44 lines

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

---
 drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

drivers/infiniband/core/sysfs.c +12 -5

index 0083a4f..f7f2954 100644
@@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors     ,  8,  8, 136, IB_PMA_PORT_C
 static PORT_PMA_ATTR(local_link_integrity_errors    ,  9,  4, 152, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10,  4, 156, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(VL15_dropped                , 11, 16, 176, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_data              , 12, 32, 192, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_rcv_data               , 13, 32, 224, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_packets                   , 14, 32, 256, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_rcv_packets                    , 15, 32, 288, IB_PMA_PORT_COUNTERS);
+static PORT_PMA_ATTR(port_xmit_data              ,  0, 64,  64, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_rcv_data               ,  0, 64, 128, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_xmit_packets                   ,  0, 64, 192, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(port_rcv_packets                    ,  0, 64, 256, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(unicast_xmit_packets        ,  0, 64, 320, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(unicast_rcv_packets         ,  0, 64, 384, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_xmit_packets      ,  0, 64, 448, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_rcv_packets       ,  0, 64, 512, IB_PMA_PORT_COUNTERS_EXT);
 
 static struct attribute *pma_attrs[] = {
      &port_pma_attr_symbol_error.attr.attr,

@@ -428,6 +432,10 @@ static struct attribute *pma_attrs[] = {
      &port_pma_attr_port_rcv_data.attr.attr,
      &port_pma_attr_port_xmit_packets.attr.attr,
      &port_pma_attr_port_rcv_packets.attr.attr,
+     &port_pma_attr_unicast_rcv_packets.attr.attr,
+     &port_pma_attr_unicast_xmit_packets.attr.attr,
+     &port_pma_attr_multicast_rcv_packets.attr.attr,
+     &port_pma_attr_multicast_xmit_packets.attr.attr,
      NULL
 };
 
--

2.5.0

Quoted 4 lines

--
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

--
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

Display the additional 64 bit counters available through the extended set and replace the existing 32 bit counters if there is a 64 bit alternative available.

Note: This requires universal support of extended counters in the devices. If there are still devices around that do not support extended counters then we will have to add some fallback technique here.

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

---
 drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

drivers/infiniband/core/sysfs.c +0 -3

index 0083a4f..f7f2954 100644
@@ -406,10 +406,14 @@ static PORT_PMA_ATTR(port_rcv_constraint_errors       ,  8,  8, 136, IB_PMA_PORT_C
 static PORT_PMA_ATTR(local_link_integrity_errors    ,  9,  4, 152, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(excessive_buffer_overrun_errors, 10,  4, 156, IB_PMA_PORT_COUNTERS);
 static PORT_PMA_ATTR(VL15_dropped                  , 11, 16, 176, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_data                , 12, 32, 192, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_rcv_data                 , 13, 32, 224, IB_PMA_PORT_COUNTERS);
-static PORT_PMA_ATTR(port_xmit_packets             , 14, 32, 256, IB_PMA_PORT_COUNTERS);

drivers/infiniband/core/sysfs.c +4 -1

 static struct attribute *pma_attrs[] = {
        &port_pma_attr_symbol_error.attr.attr,

@@ -428,6 +432,10 @@ static struct attribute *pma_attrs[] = {
        &port_pma_attr_port_rcv_data.attr.attr,
        &port_pma_attr_port_xmit_packets.attr.attr,
        &port_pma_attr_port_rcv_packets.attr.attr,
+       &port_pma_attr_unicast_rcv_packets.attr.attr,
+       &port_pma_attr_unicast_xmit_packets.attr.attr,
+       &port_pma_attr_multicast_rcv_packets.attr.attr,
+       &port_pma_attr_multicast_xmit_packets.attr.attr,
        NULL
 };
 
--

2.5.0

--
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

drivers/infiniband/core/sysfs.c static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C · line 416

Quoted 4 lines

drivers/infiniband/core/sysfs.c +3 -0

+static PORT_PMA_ATTR(unicast_rcv_packets           ,  0, 64, 384, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_xmit_packets        ,  0, 64, 448, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_rcv_packets         ,  0, 64, 512, IB_PMA_PORT_COUNTERS_EXT);

Why do we use 0 as the counter argument for all EXT counters?

Quoted 6 lines · Matan Barak

drivers/infiniband/core/sysfs.c static PORT_PMA_ATTR(port_rcv_constraint_errors , 8, 8, 136, IB_PMA_PORT_C · line 416

Quoted 4 lines

drivers/infiniband/core/sysfs.c +3 -0

+static PORT_PMA_ATTR(unicast_rcv_packets           ,  0, 64, 384, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_xmit_packets        ,  0, 64, 448, IB_PMA_PORT_COUNTERS_EXT);
+static PORT_PMA_ATTR(multicast_rcv_packets         ,  0, 64, 512, IB_PMA_PORT_COUNTERS_EXT);

Why do we use 0 as the counter argument for all EXT counters?

No idea what the counter is doing. Saw another EXT counter implementation use 0 so I thought that was fine.

Quoted 2 lines

No idea what the counter is doing. Saw another EXT counter implementation use 0 so I thought that was fine.

It seems like a counter index, but I might be wrong though. If it is, don't we want to preserve the existing non-EXT schema for the new counters too?

Quoted 3 lines · Matan Barak

Quoted 2 lines

No idea what the counter is doing. Saw another EXT counter implementation use 0 so I thought that was fine.

Quoted 3 lines

It seems like a counter index, but I might be wrong though. If it is, don't we want to preserve the existing non-EXT schema for the new counters too?

I do not see any use of that field so I am not sure what to put in there. Could it be obsolete?

Quoted 9 lines · Christoph Lameter

Quoted 6 lines · Matan Barak

Quoted 2 lines

No idea what the counter is doing. Saw another EXT counter implementation use 0 so I thought that was fine.

It seems like a counter index, but I might be wrong though. If it is, don't we want to preserve the existing non-EXT schema for the new counters too?

Quoted 3 lines

I do not see any use of that field so I am not sure what to put in there. Could it be obsolete?

I don't see any usage to that field either, but I think 0 is a bit misleading. So maybe it's more appropriate to delete this field.