[PATCH 0/3] IB 64 bit counter support V2

[PATCH 0/3] IB 64 bit counter support V2

[Christoph Lameter]

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christoph Lameter]

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

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

Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
--- linux.orig/drivers/infiniband/core/sysfs.c
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/17/2015 2:52 PM, Christoph Lameter wrote:
> Create a new function to retrieve performance management
> data from the existing code in get_pma_counter().
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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 1/3] IB Core: Create get_perf_mad function in sysfs.c

[ira.weiny]

On Thu, Dec 31, 2015 at 08:58:31AM +0100, Bart Van Assche wrote:
> On 12/17/2015 08:52 PM, Christoph Lameter wrote:
> >-	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/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg31257.html

Ira

> 
> Thanks,
> 
> Bart.
--
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 2/3] IB core counters: Specify attribute_id in port_table_attribute

[Christoph Lameter]

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

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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
--- linux.orig/drivers/infiniband/core/sysfs.c
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/17/2015 2:52 PM, Christoph Lameter wrote:
> 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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 3/3] bject: IB Core: Display extended counter set if available

[Christoph Lameter]

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

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

Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
--- linux.orig/drivers/infiniband/core/sysfs.c
+++ 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;
 
Index: linux/include/rdma/ib_pma.h
===================================================================
--- linux.orig/include/rdma/ib_pma.h
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/17/2015 2:52 PM, Christoph Lameter wrote:
> Check if the extended counters are available and if so
> create the proper extended and additional counters.
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> 
> Index: linux/drivers/infiniband/core/sysfs.c
> ===================================================================
> --- linux.orig/drivers/infiniband/core/sysfs.c
> +++ 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;
>  	}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

> +			&pma_group);
> +
>  	if (ret)
>  		goto err_put;
>  
> Index: linux/include/rdma/ib_pma.h
> ===================================================================
> --- linux.orig/include/rdma/ib_pma.h
> +++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christoph Lameter]

On Thu, 17 Dec 2015, Hal Rosenstock wrote:

> > -	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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/17/2015 4:28 PM, Christoph Lameter wrote:
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christoph Lameter]

On Thu, 17 Dec 2015, Hal Rosenstock wrote:

> On 12/17/2015 4:28 PM, Christoph Lameter wrote:
> > 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
--- linux.orig/drivers/infiniband/core/sysfs.c
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/17/2015 5:50 PM, Christoph Lameter wrote:
> On Thu, 17 Dec 2015, Hal Rosenstock wrote:
> 
>> On 12/17/2015 4:28 PM, Christoph Lameter wrote:
>>> 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> 
> Index: linux/drivers/infiniband/core/sysfs.c
> ===================================================================
> --- linux.orig/drivers/infiniband/core/sysfs.c
> +++ 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;

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;

> +
> +		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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christoph Lameter]

On Thu, 17 Dec 2015, Hal Rosenstock wrote:

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

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

> 	}
>
> 	return &pma_group;
>
> > +

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/18/2015 8:39 AM, Christoph Lameter wrote:
> On Thu, 17 Dec 2015, Hal Rosenstock wrote:
> 
>>> +	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;
> 
> 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).

> 
>> 	}
>>
>> 	return &pma_group;
>>
>>> +
> 
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Christoph Lameter]

On Fri, 18 Dec 2015, Hal Rosenstock wrote:

> > 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Index: linux/drivers/infiniband/core/sysfs.c
===================================================================
--- linux.orig/drivers/infiniband/core/sysfs.c
+++ 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hal Rosenstock]

On 12/18/2015 10:46 AM, Christoph Lameter wrote:
> 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Hal Rosenstock <hal-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
--
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 3/3] bject: IB Core: Display extended counter set if available

[Leon Romanovsky]

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

On Thu, Dec 17, 2015 at 01:52:58PM -0600, Christoph Lameter wrote:
> -	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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html