Improved node descriptions

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.

---

Michael Heinz (2):
Improved node descriptions
Making it easier to diagnose fabric problems by improving the node descriptions.

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

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

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.

---
 drivers/infiniband/core/mad.c |   18 ++++++++++++++++++
 include/rdma/ib_mad.h         |    8 ++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

index 822cfdc..8e4ac68 100644
@@ -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,23 @@ int ib_get_mad_data_offset(u8 mgmt_class)
 }
 EXPORT_SYMBOL(ib_get_mad_data_offset);
 
+void ib_build_node_desc(char *dest, char *src)
+{
+	int i;
+	for (i = 0; i < 64;) {
+		if (*src == '@') {
+			char *name = init_utsname()->nodename;
+			for (; *name && (*name != '.') && (i < 64); ++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) ||

index d3b9401..5916617 100644
@@ -637,6 +637,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(char *dest, 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Adds support for ib_build_node_desc() to the HCAs.

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

index ceb98ee..6da4750 100644
@@ -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((char *)smp->data, ibdev->node_desc);
 
 	return reply(smp);
 }

index 57ffa50..6b35d49 100644
@@ -196,7 +196,8 @@ 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((char *)((struct ib_smp *) mad)->data,
+							dev->node_desc);
 		spin_unlock(&to_mdev(dev)->sm_lock);
 	}
 }

index 03a5953..7662097 100644
@@ -153,7 +153,8 @@ 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((char *)((struct ib_smp *) mad)->data,
+							dev->node_desc);
 		mutex_unlock(&to_mdev(dev)->cap_mask_mutex);
 	}
 }

index 5ad224e..59d7cbb 100644
@@ -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((char *)smp->data, ibdev->node_desc);
 
 	return reply(smp);
 }

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

This looks like a reasonable approach to me, although of course the SM has no way of knowing it should update a port's node description if a hostname changes.

Aside from some minor quibbles
- next time please use different subjects for each patch in the thread
- the prototype of ib_build_node_desc() seems to force every call site to have a cast; maybe the function should take a pointer to struct ib_smp instead?
- the internals of ib_build_node_desc() look a bit ugly, is there any way to make it a little cleaner?
I do like this. Does anyone have any feelings about applying this for 2.6.39? Is this shipping in OFED?

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

It does; There's an enhanced trap for this now.

I need a little time to review this.

-- Hal

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

Right; sorry for being unclear but I meant that this patch does not have a mechanism for generating this trap if the hostname changes or if userspace changes the node description.

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

SGFsLA0KDQpJJ20gaW4gdGhlIHByb2Nlc3Mgb2Ygc21va2UtdGVzdGluZyBjaGFuZ2VzIHRvIG1l ZXQgUm9sYW5kJ3MgInF1aWJibGVzIiAtIEknbSBnb2luZyB0byBjaGFuZ2UgdGhlIGNvZGUgdG8g cGFzcyB0aGUgc3RydWN0dXJlIGluc3RlYWQgb2YgYSBjaGFyKi4NCg0KQWxzbywgSSBqdXN0IHdh bnQgdG8gcG9pbnQgb3V0IHRoYXQgZXZlbiBpbiB0aGUgY2FzZSB3aGVyZSBubyBlbmhhbmNlZCB0 cmFwIGlzIHVzZWQsIGFueSBjaGFuZ2Ugd2lsbCBzdGlsbCBiZSBzZWVuIGJ5IHRoZSBTTSB0aGUg bmV4dCB0aW1lIGl0IHN3ZWVwcy4gVGhlcmUgd2lsbCBzdGlsbCBiZSBsYWcgaW52b2x2ZWQsIGJ1 dCBJIGRvbid0IHRoaW5rIGl0J3MgaW5zdXJtb3VudGFibGUuDQoNCi0tLS0tT3JpZ2luYWwgTWVz c2FnZS0tLS0tDQpGcm9tOiBIYWwgUm9zZW5zdG9jayBbbWFpbHRvOmhhbC5yb3NlbnN0b2NrQGdt YWlsLmNvbV0NClNlbnQ6IFRodXJzZGF5LCBGZWJydWFyeSAxNywgMjAxMSAxMToxOSBQTQ0KVG86 IFJvbGFuZCBEcmVpZXINCkNjOiBNaWtlIEhlaW56OyBsaW51eC1yZG1hQHZnZXIua2VybmVsLm9y Zw0KU3ViamVjdDogUmU6IFtQQVRDSCAwLzJdIEltcHJvdmVkIG5vZGUgZGVzY3JpcHRpb25zDQoN Ck9uIFRodSwgRmViIDE3LCAyMDExIGF0IDY6MjAgUE0sIFJvbGFuZCBEcmVpZXIgPHJvbGFuZEBr ZXJuZWwub3JnPiB3cm90ZToNCj4gT24gVGh1LCBGZWIgMTcsIDIwMTEgYXQgMTozMCBQTSwgTWlj aGFlbCBIZWlueiA8bWljaGFlbC5oZWluekBxbG9naWMuY29tPiB3cm90ZToNCj4+IFRoaXMgcGF0 Y2ggYWRkcmVzc2VzIHRoZSBwcm9ibGVtIGJ5IHByb3ZpZGluZyBhIGZ1bmN0aW9uIHRvIGJ1aWxk IHRoZSBub2RlDQo+PiBkZXNjcmlwdGlvbi4gSWYgdGhlIHByb3ZpZGVkIHNvdXJjZSBzdHJpbmcg Zm9yIHRoZSBkZXNjcmlwdGlvbiBjb250YWlucyBhbg0KPj4gJ0AnIGNoYXJhY3RlciwgdGhlIGZ1 bmN0aW9uIHdpbGwgc3Vic3RpdHV0ZSB0aGUgY3VycmVudCB1dHNuYW1lLg0KPj4NCj4+IFRoaXMg ZW5zdXJlcyB0aGF0IGV2ZW4gYWZ0ZXIgYSBmYWJyaWMgaGFzIGJlZW4gY29tcGxldGVseSBpbml0 aWFsaXplZCwgaWYNCj4+IGEgbm9kZSdzIGhvc3RuYW1lIGNoYW5nZXMsIHRoYXQgY2hhbmdlIHdp bGwgYmUgcmVmbGVjdGVkIGluIHRoZSBuZXh0IHN3ZWVwDQo+PiBvZiB0aGUgU00sIGJ1dCBhbHNv IG1haW50YWlucyBjb21wYXRpYmlsaXR5IHdpdGggZXhpc3RpbmcgY29kZSBzaW5jZSB0aGUNCj4+ IGJlaGF2aW9yIGlzIHVuY2hhbmdlZCBpZiB0aGUgZGVzY3JpcHRpb24gc3RyaW5nIGRvZXMgbm90 IGNvbnRhaW4gYW4gJ0AnDQo+PiBjaGFyYWN0ZXIuDQo+DQo+IFRoaXMgbG9va3MgbGlrZSBhIHJl YXNvbmFibGUgYXBwcm9hY2ggdG8gbWUsIGFsdGhvdWdoIG9mIGNvdXJzZSB0aGUgU00NCj4gaGFz IG5vIHdheSBvZiBrbm93aW5nIGl0IHNob3VsZCB1cGRhdGUgYSBwb3J0J3Mgbm9kZSBkZXNjcmlw dGlvbiBpZiBhDQo+IGhvc3RuYW1lIGNoYW5nZXMuDQoNCkl0IGRvZXM7IFRoZXJlJ3MgYW4gZW5o YW5jZWQgdHJhcCBmb3IgdGhpcyBub3cuDQoNCj4gQXNpZGUgZnJvbSBzb21lIG1pbm9yIHF1aWJi bGVzDQo+ICAtIG5leHQgdGltZSBwbGVhc2UgdXNlIGRpZmZlcmVudCBzdWJqZWN0cyBmb3IgZWFj aCBwYXRjaCBpbiB0aGUgdGhyZWFkDQo+ICAtIHRoZSBwcm90b3R5cGUgb2YgaWJfYnVpbGRfbm9k ZV9kZXNjKCkgc2VlbXMgdG8gZm9yY2UgZXZlcnkgY2FsbA0KPiAgIHNpdGUgdG8gaGF2ZSBhIGNh c3Q7IG1heWJlIHRoZSBmdW5jdGlvbiBzaG91bGQgdGFrZSBhIHBvaW50ZXIgdG8NCj4gICBzdHJ1 Y3QgaWJfc21wIGluc3RlYWQ/DQo+ICAtIHRoZSBpbnRlcm5hbHMgb2YgaWJfYnVpbGRfbm9kZV9k ZXNjKCkgbG9vayBhIGJpdCB1Z2x5LCBpcyB0aGVyZSBhbnkNCj4gICB3YXkgdG8gbWFrZSBpdCBh IGxpdHRsZSBjbGVhbmVyPw0KPiBJIGRvIGxpa2UgdGhpcy4gIERvZXMgYW55b25lIGhhdmUgYW55 IGZlZWxpbmdzIGFib3V0IGFwcGx5aW5nIHRoaXMNCj4gZm9yIDIuNi4zOT8gIElzIHRoaXMgc2hp cHBpbmcgaW4gT0ZFRD8NCg0KSSBuZWVkIGEgbGl0dGxlIHRpbWUgdG8gcmV2aWV3IHRoaXMuDQoN Ci0tIEhhbA0KDQo+ICAtIFIuDQo+IC0tDQo+IFRvIHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0 OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1yZG1hIiBpbg0KPiB0aGUgYm9keSBv ZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiBNb3JlIG1ham9yZG9t byBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCj4N Cg0KDQpUaGlzIG1lc3NhZ2UgYW5kIGFueSBhdHRhY2hlZCBkb2N1bWVudHMgY29udGFpbiBpbmZv cm1hdGlvbiBmcm9tIFFMb2dpYyBDb3Jwb3JhdGlvbiBvciBpdHMgd2hvbGx5LW93bmVkIHN1YnNp ZGlhcmllcyB0aGF0IG1heSBiZSBjb25maWRlbnRpYWwuIElmIHlvdSBhcmUgbm90IHRoZSBpbnRl bmRlZCByZWNpcGllbnQsIHlvdSBtYXkgbm90IHJlYWQsIGNvcHksIGRpc3RyaWJ1dGUsIG9yIHVz ZSB0aGlzIGluZm9ybWF0aW9uLiBJZiB5b3UgaGF2ZSByZWNlaXZlZCB0aGlzIHRyYW5zbWlzc2lv biBpbiBlcnJvciwgcGxlYXNlIG5vdGlmeSB0aGUgc2VuZGVyIGltbWVkaWF0ZWx5IGJ5IHJlcGx5 IGUtbWFpbCBhbmQgdGhlbiBkZWxldGUgdGhpcyBtZXNzYWdlLg0K

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

Mike,

That's not true for all SMs. It's true for a "heavy" but not a "light" sweep. There's no requirement to rediscover NodeDescriptions all the time periodically.

I should have my comments (on your now updated patches) by COB Tuesday as Monday is a holiday.

-- Hal

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

Roland,

Thanks for the positive feedback - I'll make the changes ASAP. Sorry about the subject lines, I thought I was doing the "right thing" by making them match.

The change is not in OFED yet, but QLogic has been using it in our own release for a while and works extremely well on large fabrics.
________________________________________
From: [email protected] [[email protected]] On Behalf Of Roland Dreier [[email protected]]
Sent: Thursday, February 17, 2011 6:20 PM
To: Mike Heinz
Cc: [email protected]
Subject: Re: [PATCH 0/2] Improved node descriptions

This looks like a reasonable approach to me, although of course the SM has no way of knowing it should update a port's node description if a hostname changes.

Aside from some minor quibbles
- next time please use different subjects for each patch in the thread
- the prototype of ib_build_node_desc() seems to force every call site to have a cast; maybe the function should take a pointer to struct ib_smp instead?
- the internals of ib_build_node_desc() look a bit ugly, is there any way to make it a little cleaner?
I do like this. Does anyone have any feelings about applying this for 2.6.39? Is this shipping in OFED?

- R.

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

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

What about the problem of multiple HCA's of the same type on a single host? Won't all of them get the identical node description?
Mike, can you add something to handle this case?

(See my original feedback:
http://www.mail-archive.com/[email protected]/msg04413.html
and see Mike's response:
http://www.mail-archive.com/[email protected]/msg04414.html

Mike, I agree that doing something for the upstream is better than doing nothing, but I would still like to see the multiple-HCA case handled. I think this can be done by adding a query to the low-level driver to distinguish between HCAs).

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

Jack,

The way we handle that right now is in the openibd start script. What we do is this:

ib_set_node_desc()

{
        # Add node description to sysfs
      ibsysdir="/sys/class/infiniband"
      if [ -d ${ibsysdir} ]; then
          declare -i hca_id=1
          for hca in ${ibsysdir}/*
          do
              if [ -e ${hca}/node_desc ]; then
                  logger -i "Set node_desc for $(basename $hca)"
                  echo -n "@ HCA-${hca_id}" >> ${hca}/node_desc
              fi
              let hca_id++
          done
      fi
}

Once the kernel modification is accepted, I plan to submit a patch for the openibd script.

-----Original Message-----
From: Jack Morgenstein [mailto:[email protected]]
Sent: Saturday, February 19, 2011 2:23 AM
To: Roland Dreier
Cc: Mike Heinz; [email protected]; Hal Rosenstock
Subject: Re: [PATCH 0/2] Improved node descriptions

What about the problem of multiple HCA's of the same type on a single host? Won't all of them get the identical node description?
Mike, can you add something to handle this case?

(See my original feedback:
http://www.mail-archive.com/[email protected]/msg04413.html
and see Mike's response:
http://www.mail-archive.com/[email protected]/msg04414.html

Mike, I agree that doing something for the upstream is better than doing nothing, but I would still like to see the multiple-HCA case handled. I think this can be done by adding a query to the low-level driver to distinguish between HCAs).

-Jack

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

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

If the main concern is DHCP what is the problem with using /etc/dhcp/dhclient-enter-hooks.d/ or alike?

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

The biggest problem with that is that patching existing boot scripts is always going to vary from distro to distro and is always going to have problems when dealing with files that were already edited for site-specific reasons.

Ipoib is a good example of this, I frequently see failures trying to install the ipoib rpm because it failed to successfully patch the ifup- scripts. I'd rather not add another such dependency.

For example, there are is /etc/dhcp/dhclient-enter-hooks.d in SLES11 or on Redhat 6 - redhat 6 uses different files for different hooks and uses a single file (which does not exist by default) called /etc/dhcp/dhclient-enter-hook, but SLES11 uses a single script for all dhcp hooks called /etc/dhcpcd.sh.

Trying to create a single RPM capable of hacking into a wide variety of different dhcp mechanisms could be a huge headache. Using something that relies on the openibd script and a kernel-expansion of the hostname is much less prone to problems.

-----Original Message-----
From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Saturday, February 19, 2011 2:25 PM
To: Roland Dreier
Cc: Mike Heinz; [email protected]
Subject: Re: [PATCH 0/2] Improved node descriptions

If the main concern is DHCP what is the problem with using /etc/dhcp/dhclient-enter-hooks.d/ or alike?

Jason

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

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

This is the wrong way to look at it. Really it would make sense in the long term to add required support to native distros -- piling hack on hack in OFED is clearly a long-term disaster. So I don't think saying "it's hard to make a single RPM" is a very good argument.

With that said I think we should look at where it makes sense to implement this sort of thing. Just because we can do something with scripts in userspace doesn't necessarily mean we have to do it there -- if it's much simpler or more robust in the kernel, then we can implement it there.

In this case I do think it makes sense to add this support to the kernel, since the kernel handling is so simple. In fact based on Jack's question it might make sense to go further and have more flexible expansion... what if we do something like adding primitive format expansion, ie

    %h --> expands to current hostname
    %d --> expands to ib_device->name

Then one could have a trivial script that just does 'for every IB device, prepend "%h/%d: " to the node description.' And I guess we could even talk about making that the default kernel policy.

Not sure what Jason or others thinks about this opinion...
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Indeed, considering that OFEDs entire purpose is to manage packaging IB stuff for distributions this doesn't seem like a good argument.. Nor do I think OFED should even try to have one RPM for all distros, good packaging isn't like that.

Doing it in userspace makes generating the node description changed trap simpler?

What about other drivers? I didn't see ehca in Mike's patch..

I just wonder if this is a big pain to do right, what about charsets, IDN, and ugly details like that?

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

Hmm... how many places can do sethostname()? Seems easier to catch in the kernel than hook every place in userspace. (Although there's no kernel hook right now) I do agree it would be good to have some idea of how we could generate the "node desc changed" trap at appropriate times.

ehca doesn't implement modify_device --> can't set node desc anyway.

Does anyone expect to care about non-ASCII node descs? What can we sensibly do except take what we're given?

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

Wasn't thinking about 100% perfection, just if DHCP is the concern it shouldn't be hard to hook that one place.

node desc is UTF-8, hostname is IDNA, a conversion is required, see RFC 3490.

Does anyone care? Who knows, but very pedantically it is wrong to just copy the host name byte by byte. I only mention it to point out that it is trivial to do what Mike did, somewhat harder to do % escaping like you suggest and solve the multiple HCA problem, harder still to trap sethostname() and generate a trap, and extra special hard to correctly handle character sets on top of all that. :)

So, is it still trivial to do it in the kernel?

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

I like the idea of having a way to tie node description to the hostname. It just makes sense. The question in my mind is will upstream accept a hook in sethostname? Generally it seems like a good idea, perhaps other subsystems would want to know about it. But it does seem like it might not get accepted.

For what it is worth none of the diags or OpenSM support UTF-8. I'll add it to the list of things to do. ;-)

I don't know much about IDNA but it seems that ASCII is a subset of IDNA as well as UTF-8. Perhaps a check for ASCII in the hostname would suffice to at least make this work for ASCII hostnames only. (That should cover a significant portion of the hosts out there, if not all of them.)

Looking at the latest code it seems that setting the node descriptor for mlx4 and qib will result in a trap 144 being set. A hook in sethostname which results in the regeneration of the node description and the generation of the trap seems pretty easy. So, from a technical side I think the answer is "yes" it is pretty easy.

That said getting that hook into sethostname might be harder to get upstream than just writing the code. I can't comment on that because I don't have any idea how hard that would be. :-(

Ira

Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
[email protected]
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jason,

While I do think supporting the trap would be a great idea going forward, I don't see any reason why it couldn't be combined with this patch when we figure out how to detect the hostname change.

For example, it would be great for me as a developer to be able to flag a group of machines as being dedicated to me, by setting their node descriptions to "%h: %d Mike's MPI Testbed" or similar.

I could echo that into the node description at boot time and, if the host name changes afterwards, still have a trap triggered (somehow) to inform the SM of the change.

-----Original Message-----
From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
Sent: Wednesday, February 23, 2011 12:42 AM
To: Roland Dreier
Cc: Mike Heinz; [email protected]
Subject: Re: [PATCH 0/2] Improved node descriptions

Wasn't thinking about 100% perfection, just if DHCP is the concern it shouldn't be hard to hook that one place.

node desc is UTF-8, hostname is IDNA, a conversion is required, see RFC 3490.

Does anyone care? Who knows, but very pedantically it is wrong to just copy the host name byte by byte. I only mention it to point out that it is trivial to do what Mike did, somewhat harder to do % escaping like you suggest and solve the multiple HCA problem, harder still to trap sethostname() and generate a trap, and extra special hard to correctly handle character sets on top of all that. :)

So, is it still trivial to do it in the kernel?

Jason

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

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

I had actually submitted a patch back in June of last year that did set the default policy to be the hostname & device but looking back through my records I can't find the reason it was never explicitly rejected - it got limited feedback and just wasn't ever added to Linux-rdma. (I'm not sure how to learn the final disposition of proposed patches.) Maybe it was a coding style or formatting issue?

Right now the HCAs default to a description of the HCA model. I can certainly create a patch that sets the default descriptions to "%h: %d (device description)" if that's what you want. We certainly need something that's widely applicable, not distribution specific and lets large clusters manage their nodes effectively.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Roland Dreier
Sent: Tuesday, February 22, 2011 6:14 PM
To: Mike Heinz
Cc: Jason Gunthorpe; [email protected]
Subject: Re: [PATCH 0/2] Improved node descriptions

This is the wrong way to look at it. Really it would make sense in the long term to add required support to native distros -- piling hack on hack in OFED is clearly a long-term disaster. So I don't think saying "it's hard to make a single RPM" is a very good argument.

With that said I think we should look at where it makes sense to implement this sort of thing. Just because we can do something with scripts in userspace doesn't necessarily mean we have to do it there -- if it's much simpler or more robust in the kernel, then we can implement it there.

In this case I do think it makes sense to add this support to the kernel, since the kernel handling is so simple. In fact based on Jack's question it might make sense to go further and have more flexible expansion... what if we do something like adding primitive format expansion, ie

    %h --> expands to current hostname
    %d --> expands to ib_device->name

Then one could have a trivial script that just does 'for every IB device, prepend "%h/%d: " to the node description.' And I guess we could even talk about making that the default kernel policy.

Not sure what Jason or others thinks about this opinion...

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

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