[PATCH 0/2] updated patch for improving node descriptions.

[PATCH 0/2] updated patch for improving node descriptions.

[Michael Heinz]

Updated the code as per Roland's requests, except for one part:

Roland, I tried to rework the main function to be cleaner, because of the
need to split the hostname from the domain name and the need to not
modify the original source string, using library functions made things
worse than the existing pointer math.

---

Michael Heinz (2):
      Function for improved node descriptions
      Add support for ib_build_node_desc() to the HCAs.


 drivers/infiniband/core/mad.c           |   19 +++++++++++++++++++
 drivers/infiniband/hw/ipath/ipath_mad.c |    2 +-
 drivers/infiniband/hw/mlx4/mad.c        |    2 +-
 drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
 drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
 include/rdma/ib_mad.h                   |    9 +++++++++
 6 files changed, 32 insertions(+), 4 deletions(-)

-- 
Signed-off-by: Michael Heinz <michael.heinz-h88ZbnxC6KDQT0dZR+AlfA@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 1/2] Function for improved node descriptions

[Michael Heinz]

The common practice in IB fabrics is to set the description of an HCA to be
the hostname of the machine plus a description (i.e., "myhost hca-1",
"myhost hca-2", etc..)

This has a limitation, however.  The first is that if the machine's
hostname is set via DHCP, the HCA description may be set before the hostname
is, leading to an incorrect description. This can also occur if the machine's
hostname changes for some other reason after boot.

This can cause difficulties and confusion when trying to maintain a large
fabric - if all your nodes are described as "localhost HCA-1" it can be very
difficult to figure out which node is suffering from symbol errors.

This patch addresses the problem by providing a function to build the node
description. If the provided source string for the description contains an
'@' character, the function will substitute the current utsname.

This ensures that even after a fabric has been completely initialized, if
a node's hostname changes, that change will be reflected in the next sweep
of the SM, but also maintains compatibility with existing code since the
behavior is unchanged if the description string does not contain an '@'
character.

Signed-off-by: Michael Heinz <michael.heinz-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>

---
 drivers/infiniband/core/mad.c |   19 +++++++++++++++++++
 include/rdma/ib_mad.h         |    9 +++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
index 822cfdc..7ea8556 100644
--- a/drivers/infiniband/core/mad.c
+++ b/drivers/infiniband/core/mad.c
@@ -41,6 +41,7 @@
 #include "mad_rmpp.h"
 #include "smi.h"
 #include "agent.h"
+#include "linux/utsname.h"
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("kernel IB MAD API");
@@ -932,6 +933,24 @@ int ib_get_mad_data_offset(u8 mgmt_class)
 }
 EXPORT_SYMBOL(ib_get_mad_data_offset);
 
+void ib_build_node_desc(struct ib_smp *smp, char *src)
+{
+	char *dest = smp->data;
+	int i;
+	for (i = 0; i < IB_SMP_DATA_SIZE;) {
+		if (*src == '@') {
+			char *name = init_utsname()->nodename;
+			for (; *name && (*name != '.') && (i < IB_SMP_DATA_SIZE); ++i)
+				*dest++ = *name++;
+			src++;
+		} else {
+			*dest++ = *src++;
+			i++;
+		}
+	}
+}
+EXPORT_SYMBOL(ib_build_node_desc);
+
 int ib_is_mad_class_rmpp(u8 mgmt_class)
 {
 	if ((mgmt_class == IB_MGMT_CLASS_SUBN_ADM) ||
diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
index d3b9401..e2ed2db 100644
--- a/include/rdma/ib_mad.h
+++ b/include/rdma/ib_mad.h
@@ -40,6 +40,7 @@
 #include <linux/list.h>
 
 #include <rdma/ib_verbs.h>
+#include <rdma/ib_smi.h>
 
 /* Management base version */
 #define IB_MGMT_BASE_VERSION			1
@@ -637,6 +638,14 @@ int ib_is_mad_class_rmpp(u8 mgmt_class);
 int ib_get_mad_data_offset(u8 mgmt_class);
 
 /**
+ * ib_build_node_desc - copies the node description and replaces
+ * any @ markers with the present system node name.
+ * @dest: destination
+ * @src: source
+ */
+void ib_build_node_desc(struct ib_smp *smp, char *src);
+
+/**
  * ib_get_rmpp_segment - returns the data buffer for a given RMPP segment.
  * @send_buf: Previously allocated send data buffer.
  * @seg_num: number of segment to return

--
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/2] Function for improved node descriptions

[Hal Rosenstock]

On Fri, Feb 18, 2011 at 4:29 PM, Michael Heinz <michael.heinz-h88ZbnxC6KBhl2p70BpVqQ@public.gmane.orgm> wrote:
> The common practice in IB fabrics is to set the description of an HCA to be
> the hostname of the machine plus a description (i.e., "myhost hca-1",
> "myhost hca-2", etc..)
>
> This has a limitation, however.  The first is that if the machine's
> hostname is set via DHCP, the HCA description may be set before the hostname
> is, leading to an incorrect description. This can also occur if the machine's
> hostname changes for some other reason after boot.
>
> This can cause difficulties and confusion when trying to maintain a large
> fabric - if all your nodes are described as "localhost HCA-1" it can be very
> difficult to figure out which node is suffering from symbol errors.
>
> This patch addresses the problem by providing a function to build the node
> description. If the provided source string for the description contains an
> '@' character, the function will substitute the current utsname.
>
> This ensures that even after a fabric has been completely initialized, if
> a node's hostname changes, that change will be reflected in the next sweep
> of the SM,

This is when the SM reads NodeDescriptions during those sweeps (which
is not a requirement nor intended to be a requirement for SMs in
general). A better way than periodic polling is to only poll it when
there's a trap issued for it changing. I think this is (somewhat)
orthogonal to the patch if this is referring to NodeDescription in the
SMAs in the subnet and not from the perspective of the SM.

As has been mentioned before, issuing the trap for NodeDescription
changed would make this better in terms of SMs in general.

I would also like to see how multiple HCAs in the same host are
handled just to be sure but I think I see what you intend in the
script for that.

-- Hal

> but also maintains compatibility with existing code since the
> behavior is unchanged if the description string does not contain an '@'
> character.
>
> Signed-off-by: Michael Heinz <michael.heinz-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>
>
> ---
>  drivers/infiniband/core/mad.c |   19 +++++++++++++++++++
>  include/rdma/ib_mad.h         |    9 +++++++++
>  2 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
> index 822cfdc..7ea8556 100644
> --- a/drivers/infiniband/core/mad.c
> +++ b/drivers/infiniband/core/mad.c
> @@ -41,6 +41,7 @@
>  #include "mad_rmpp.h"
>  #include "smi.h"
>  #include "agent.h"
> +#include "linux/utsname.h"
>
>  MODULE_LICENSE("Dual BSD/GPL");
>  MODULE_DESCRIPTION("kernel IB MAD API");
> @@ -932,6 +933,24 @@ int ib_get_mad_data_offset(u8 mgmt_class)
>  }
>  EXPORT_SYMBOL(ib_get_mad_data_offset);
>
> +void ib_build_node_desc(struct ib_smp *smp, char *src)
> +{
> +       char *dest = smp->data;
> +       int i;
> +       for (i = 0; i < IB_SMP_DATA_SIZE;) {
> +               if (*src == '@') {
> +                       char *name = init_utsname()->nodename;
> +                       for (; *name && (*name != '.') && (i < IB_SMP_DATA_SIZE); ++i)
> +                               *dest++ = *name++;
> +                       src++;
> +               } else {
> +                       *dest++ = *src++;
> +                       i++;
> +               }
> +       }
> +}
> +EXPORT_SYMBOL(ib_build_node_desc);
> +
>  int ib_is_mad_class_rmpp(u8 mgmt_class)
>  {
>        if ((mgmt_class == IB_MGMT_CLASS_SUBN_ADM) ||
> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
> index d3b9401..e2ed2db 100644
> --- a/include/rdma/ib_mad.h
> +++ b/include/rdma/ib_mad.h
> @@ -40,6 +40,7 @@
>  #include <linux/list.h>
>
>  #include <rdma/ib_verbs.h>
> +#include <rdma/ib_smi.h>
>
>  /* Management base version */
>  #define IB_MGMT_BASE_VERSION                   1
> @@ -637,6 +638,14 @@ int ib_is_mad_class_rmpp(u8 mgmt_class);
>  int ib_get_mad_data_offset(u8 mgmt_class);
>
>  /**
> + * ib_build_node_desc - copies the node description and replaces
> + * any @ markers with the present system node name.
> + * @dest: destination
> + * @src: source
> + */
> +void ib_build_node_desc(struct ib_smp *smp, char *src);
> +
> +/**
>  * ib_get_rmpp_segment - returns the data buffer for a given RMPP segment.
>  * @send_buf: Previously allocated send data buffer.
>  * @seg_num: number of segment to return
>
> --
> 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

[PATCH 2/2] Add support for ib_build_node_desc() to the HCAs.

[Michael Heinz]

Adds support for ib_build_node_desc() to the HCAs.

Signed-off-by: Michael Heinz <michael.heinz-h88ZbnxC6KDQT0dZR+AlfA@public.gmane.org>

---
 drivers/infiniband/hw/ipath/ipath_mad.c |    2 +-
 drivers/infiniband/hw/mlx4/mad.c        |    2 +-
 drivers/infiniband/hw/mthca/mthca_mad.c |    2 +-
 drivers/infiniband/hw/qib/qib_mad.c     |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/ipath/ipath_mad.c b/drivers/infiniband/hw/ipath/ipath_mad.c
index ceb98ee..bd1a112 100644
--- a/drivers/infiniband/hw/ipath/ipath_mad.c
+++ b/drivers/infiniband/hw/ipath/ipath_mad.c
@@ -60,7 +60,7 @@ static int recv_subn_get_nodedescription(struct ib_smp *smp,
 	if (smp->attr_mod)
 		smp->status |= IB_SMP_INVALID_FIELD;
 
-	memcpy(smp->data, ibdev->node_desc, sizeof(smp->data));
+	ib_build_node_desc(smp, ibdev->node_desc);
 
 	return reply(smp);
 }
diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c
index 57ffa50..6a89c3c 100644
--- a/drivers/infiniband/hw/mlx4/mad.c
+++ b/drivers/infiniband/hw/mlx4/mad.c
@@ -196,7 +196,7 @@ static void node_desc_override(struct ib_device *dev,
 	    mad->mad_hdr.method == IB_MGMT_METHOD_GET_RESP &&
 	    mad->mad_hdr.attr_id == IB_SMP_ATTR_NODE_DESC) {
 		spin_lock(&to_mdev(dev)->sm_lock);
-		memcpy(((struct ib_smp *) mad)->data, dev->node_desc, 64);
+		ib_build_node_desc((struct ib_smp *) mad, dev->node_desc);
 		spin_unlock(&to_mdev(dev)->sm_lock);
 	}
 }
diff --git a/drivers/infiniband/hw/mthca/mthca_mad.c b/drivers/infiniband/hw/mthca/mthca_mad.c
index 03a5953..b442028 100644
--- a/drivers/infiniband/hw/mthca/mthca_mad.c
+++ b/drivers/infiniband/hw/mthca/mthca_mad.c
@@ -153,7 +153,7 @@ static void node_desc_override(struct ib_device *dev,
 	    mad->mad_hdr.method == IB_MGMT_METHOD_GET_RESP &&
 	    mad->mad_hdr.attr_id == IB_SMP_ATTR_NODE_DESC) {
 		mutex_lock(&to_mdev(dev)->cap_mask_mutex);
-		memcpy(((struct ib_smp *) mad)->data, dev->node_desc, 64);
+		ib_build_node_desc((struct ib_smp *) mad, dev->node_desc);
 		mutex_unlock(&to_mdev(dev)->cap_mask_mutex);
 	}
 }
diff --git a/drivers/infiniband/hw/qib/qib_mad.c b/drivers/infiniband/hw/qib/qib_mad.c
index 5ad224e..1033112 100644
--- a/drivers/infiniband/hw/qib/qib_mad.c
+++ b/drivers/infiniband/hw/qib/qib_mad.c
@@ -260,7 +260,7 @@ static int subn_get_nodedescription(struct ib_smp *smp,
 	if (smp->attr_mod)
 		smp->status |= IB_SMP_INVALID_FIELD;
 
-	memcpy(smp->data, ibdev->node_desc, sizeof(smp->data));
+	ib_build_node_desc(smp, ibdev->node_desc);
 
 	return reply(smp);
 }

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