[PATCH 0/3] IB 64 bit counter support

[PATCH 0/3] IB 64 bit counter support

[Christoph Lameter]

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.


--
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: Allow specification of attr_id in PORT_PMA_ATTR macro

[Christoph Lameter]

[-- Attachment #1: 0001-IB-core-Allow-specification-of-attr_id-in-PORT_PMA_A.patch --]
[-- Type: text/plain, Size: 4620 bytes --]

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

Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/sysfs.c | 41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index b1f37d4..1c8716f 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/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,
@@ -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,
-- 
2.5.0


--
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: Support 64 bit values in the port counters

[Christoph Lameter]

[-- Attachment #1: 0002-IB-core-Support-64-bit-values-in-the-port-counters.patch --]
[-- Type: text/plain, Size: 971 bytes --]

Add a branch to display 64 bit values

Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/sysfs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 1c8716f..0083a4f 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -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;
 	}
-- 
2.5.0


--
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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

[-- Attachment #1: 0003-IB-core-Display-64-bit-counters-from-the-extended-se.patch --]
[-- Type: text/plain, Size: 2842 bytes --]

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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index 0083a4f..f7f2954 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -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


--
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: Allow specification of attr_id in PORT_PMA_ATTR macro

[ira.weiny]

On Fri, Dec 11, 2015 at 12:25:33PM -0600, Christoph Lameter wrote:
> This is necessary to support the extended attributes which involves
> a different attribute id.
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/infiniband/core/sysfs.c | 41 ++++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index b1f37d4..1c8716f 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/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,
> @@ -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,
> -- 
> 2.5.0
> 
> 
> --
> 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
--
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: Support 64 bit values in the port counters

[ira.weiny]

On Fri, Dec 11, 2015 at 12:25:34PM -0600, Christoph Lameter wrote:
> Add a branch to display 64 bit values
> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>

Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> ---
>  drivers/infiniband/core/sysfs.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 1c8716f..0083a4f 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -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;
>  	}
> -- 
> 2.5.0
> 
> 
> --
> 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
--
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] IB core: Display 64 bit counters from the extended set

[ira.weiny]

On Fri, Dec 11, 2015 at 12:25:35PM -0600, Christoph Lameter wrote:
> 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

> 
> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 0083a4f..f7f2954 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -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
> 
> 
> --
> 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
--
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] IB core: Display 64 bit counters from the extended set

[Jason Gunthorpe]

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

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.

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

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[ira.weiny]

On Fri, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote:
> 
> 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.

Frankly for IB/OPA one should be using the central PM.

Ira

--
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] IB core: Display 64 bit counters from the extended set

[Jason Gunthorpe]

On Fri, Dec 11, 2015 at 07:23:13PM -0500, ira.weiny wrote:
> On Fri, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote:
> > 
> > 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'

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

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

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[ira.weiny]

On Fri, Dec 11, 2015 at 05:47:15PM -0700, Jason Gunthorpe wrote:
> On Fri, Dec 11, 2015 at 07:23:13PM -0500, ira.weiny wrote:
> > On Fri, Dec 11, 2015 at 05:00:47PM -0700, Jason Gunthorpe wrote:
> > > 
> > > 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'

It's been a while since I have looked at OpenSMs PM but a 64 bit counter does
not get reset very often.

For OPA with 64bit counters the same is true.

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

Well...  That would be changing the behavior that everyone is expecting.  I
know your argument; these are useless and no one is using them.  But that is
not the point.  They were there as a check to see the PMA counters directly.
Any change now is breaking the existing ABI.

Don't get me wrong.  I'm not against keeping running counters for devices.
But, I would rather see a more flexible interface.  Something like netlink
perhaps.

And I will again mention that both IB and OPA have PMs which keep running
counters.

For now Christophs series is a welcome improvement.

Ira

--
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] IB core: Display 64 bit counters from the extended set

[Matan Barak]

On Fri, Dec 11, 2015 at 8:25 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> 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 <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 0083a4f..f7f2954 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -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);
>

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

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

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Mon, 14 Dec 2015, Matan Barak wrote:

> > +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.
--
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] IB core: Display 64 bit counters from the extended set

[Matan Barak]

On Mon, Dec 14, 2015 at 4:55 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, 14 Dec 2015, Matan Barak wrote:
>
>> > +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.

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?
--
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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Mon, 14 Dec 2015, Matan Barak wrote:

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

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

--
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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/11/2015 7:00 PM, Jason Gunthorpe wrote:
>> qib, mlx4 are fine.  mlx5 should be as well I would think (I don't have that
>> hardware.)

I'm not 100% sure but I don't think that mthca supports the
PortCountersExtended attribute.

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

I have seen this too :-(

> Not sure exactly which HW though.
> 
> Mellanox should really confirm this for their hardware matrix.

I am trying to get definitive answer to this.

-- Hal
--
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] IB core: Display 64 bit counters from the extended set

[Devesh Sharma]

Hello all,

On Sat, Dec 12, 2015 at 5:26 AM, ira.weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Dec 11, 2015 at 12:25:35PM -0600, Christoph Lameter wrote:
>> 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.

Yes, today we report 32 bit counters and to support this change a
simple patch is needed to replace those cpu_to_be32() with
cpu_to_be64(). Internally we already have 64bit countes.

>
> 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
>
>>
>> Signed-off-by: Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
>> ---
>>  drivers/infiniband/core/sysfs.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
>> index 0083a4f..f7f2954 100644
>> --- a/drivers/infiniband/core/sysfs.c
>> +++ b/drivers/infiniband/core/sysfs.c
>> @@ -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
>>
>>
>> --
>> 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
> --
> 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
--
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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Mon, 14 Dec 2015, Hal Rosenstock wrote:

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

--
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] IB core: Display 64 bit counters from the extended set

[Jason Gunthorpe]

On Tue, Dec 15, 2015 at 01:51:35PM -0600, Christoph Lameter wrote:
> On Mon, 14 Dec 2015, Hal Rosenstock wrote:
> 
> > > 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.

.. 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
--
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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/15/2015 2:55 PM, Jason Gunthorpe wrote:
> On Tue, Dec 15, 2015 at 01:51:35PM -0600, Christoph Lameter wrote:
>> On Mon, 14 Dec 2015, Hal Rosenstock wrote:
>>
>>>> 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.
> 
> .. and every field is valid?
> 
> 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.

> But not necessarily the others.

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

-- Hal

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

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[Jason Gunthorpe]

On Tue, Dec 15, 2015 at 03:01:03PM -0500, Hal Rosenstock wrote:
> > 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.

Can't remember

> > But not necessarily the others.
> 
> 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.

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

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/15/2015 4:20 PM, Jason Gunthorpe wrote:
>> 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.

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 ?
--
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] IB core: Display 64 bit counters from the extended set

[Doug Ledford]

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

On 12/15/2015 04:42 PM, Hal Rosenstock wrote:
> On 12/15/2015 4:20 PM, Jason Gunthorpe wrote:
>>> 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.
> 
> 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 ?

Yes.


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Tue, 15 Dec 2015, Doug Ledford wrote:

> On 12/15/2015 04:42 PM, Hal Rosenstock wrote:
> > On 12/15/2015 4:20 PM, Jason Gunthorpe wrote:
> >>> 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.
> >
> > 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 ?
>
> Yes.

We can return ENOSYS for the counters not supported.

Or simply not create the sysfs files when the device is instantiated as
well as fall back to the 32 bit counters on instantiation for those
devices not supporting the extended set.



--
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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Tue, 15 Dec 2015, Jason Gunthorpe wrote:

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

So this is in struct ib_class_port_info the capability_mask? This does not
seem to be used anywhere in the IB core.

Here is a draft patch to change the counters depending on a bit (which I
do not know how to determine). So this would hopefully work if someone
would insert the proper check. Note that this patch no longer needs the
earlier 2 patches.

>From Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org>
Subject: IB Core: Display extended counter set if available

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

DRAFT: This is missing the check if this device supports
extended 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
@@ -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,15 @@ 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 ,				\
+}
+
+#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 ,				\
 }

 static ssize_t show_pma_counter(struct ib_port *p, struct port_attribute *attr,
@@ -344,7 +354,7 @@ 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       = tab_attr->attr_id;

 	in_mad->data[41] = p->port_num;	/* PortSelect field */

@@ -375,6 +385,11 @@ static ssize_t show_pma_counter(struct i
 		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;
 	}
@@ -403,6 +418,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 +450,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);
@@ -528,7 +584,9 @@ static int add_port(struct ib_device *de
 		return ret;
 	}

-	ret = sysfs_create_group(&p->kobj, &pma_group);
+	ret = sysfs_create_group(&p->kobj,
+		*** Extended counters *** ? &pma_group_ext : &pma_group);
+
 	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] IB core: Display 64 bit counters from the extended set

[Doug Ledford]

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

On 12/15/2015 04:46 PM, Doug Ledford wrote:
> On 12/15/2015 04:42 PM, Hal Rosenstock wrote:
>> On 12/15/2015 4:20 PM, Jason Gunthorpe wrote:
>>>> 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.
>>
>> 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 ?
> 
> Yes.
> 
> 

From my mthca machine:

[root@rdma-dev-04 ~]$ lspci | grep Mellanox
01:00.0 InfiniBand: Mellanox Technologies MT25208 InfiniHost III Ex
(Tavor compatibility mode) (rev 20)
[root@rdma-dev-04 ~]$ perfquery
# Port counters: Lid 36 port 1 (CapMask: 0x00)
PortSelect:......................1
CounterSelect:...................0x0000
SymbolErrorCounter:..............0
LinkErrorRecoveryCounter:........0
LinkDownedCounter:...............0
PortRcvErrors:...................0
PortRcvRemotePhysicalErrors:.....0
PortRcvSwitchRelayErrors:........0
PortXmitDiscards:................0
PortXmitConstraintErrors:........0
PortRcvConstraintErrors:.........0
CounterSelect2:..................0x00
LocalLinkIntegrityErrors:........0
ExcessiveBufferOverrunErrors:....0
VL15Dropped:.....................1
PortXmitData:....................2470620192
PortRcvData:.....................2401094563
PortXmitPkts:....................6363544
PortRcvPkts:.....................6321251
[root@rdma-dev-04 ~]$ perfquery -x
ibwarn: [29831] dump_perfcounters: PerfMgt ClassPortInfo 0x0; No
extended counter support indicated

perfquery: iberror: failed: perfextquery

So, no extended counters on this device.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH 3/3] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Wed, 16 Dec 2015, Christoph Lameter wrote:

> 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 <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,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)
+#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 MAD block of data.
+ * Returns error code or the number of bytes retrieved.
+ */
+static int get_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 +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;
 }

+/*
+ * 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 port)
+{
+	int ret = 0;
+	struct ib_class_port_info cpi;
+
+	ret = get_mad(dev, port, 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 *))
@@ -528,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, port_num) ?
+			&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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/16/2015 2:34 PM, Christoph Lameter wrote:
> On Wed, 16 Dec 2015, Christoph Lameter wrote:
> 
>> 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.

Looks mostly good to me with some minor comments below.

> 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,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)
> +#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 MAD block of data.

Nit: Get PerfMgt MAD block of data

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

> +		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;
>  }
> 
> +/*
> + * 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 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.

> +
> +	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 *))
> @@ -528,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, 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.

-- Hal

> +
>  	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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Thu, 17 Dec 2015, Hal Rosenstock wrote:

> > + * Get a MAD block of data.
>
> Nit: Get PerfMgt MAD block of data

Ok.

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

Ok.

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

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?

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

--
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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/17/2015 1:54 PM, Christoph Lameter wrote:
> On Thu, 17 Dec 2015, Hal Rosenstock wrote:

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

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

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

Good; I missed that; sorry.

-- Hal
--
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] IB core: Display 64 bit counters from the extended set

[Christoph Lameter]

On Thu, 17 Dec 2015, Hal Rosenstock wrote:

> > 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?
--
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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/17/2015 2:21 PM, Christoph Lameter wrote:
> On Thu, 17 Dec 2015, Hal Rosenstock wrote:
> 
>>> 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?

It would be queried for each end port LID but should provide same
response on each port. Note that switch only has port 0 as end port.

It looks to me that in the query that the port supplied overwrites
ClassVersion field in supplied ClassPortInfo attribute which is MAD data
offset 41 where PortSelect goes. ClassPortInfo.ClassVersion is a RO
field and should be ignored by the PMA in the query and set properly in
the response so yes, you're right that it should not matter.
--
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] IB core: Display 64 bit counters from the extended set

[Matan Barak]

On Mon, Dec 14, 2015 at 6:06 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
> On Mon, 14 Dec 2015, Matan Barak wrote:
>
>> > 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?
>
> 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.
--
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] IB core: Display 64 bit counters from the extended set

[Hal Rosenstock]

On 12/20/2015 5:10 AM, Matan Barak wrote:
> On Mon, Dec 14, 2015 at 6:06 PM, Christoph Lameter <cl-vYTEC60ixJUAvxtiuMwx3w@public.gmane.org> wrote:
>> On Mon, 14 Dec 2015, Matan Barak wrote:
>>
>>>> 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?
>>
>> 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.

counter number is gone in Christoph's latest patches in terms of the new
port extended counters but still present for the original port 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