[ofa-general] Re: [PATCHv2] ibnetdiscover: Support Xsigo chassis grouping

Sasha Khapyorsky sashak at voltaire.com
Mon Sep 17 15:40:00 PDT 2007


On 15:08 Mon 17 Sep     , Hal Rosenstock wrote:
> Hi Sasha,
> 
> On Mon, 2007-09-17 at 23:51 +0200, Sasha Khapyorsky wrote:
> > Hi Hal,
> > 
> > On 17:36 Thu 13 Sep     , Hal Rosenstock wrote:
> > > ibnetdiscover: Support Xsigo chassis grouping
> > > 
> > > I think this also fixes a bug with grouping of multiple non Voltaire
> > > chassis as well.
> > 
> > Could you provide more details about this bug. 
> 
> I found it because the Xsigo grouping is similar to the non Voltaire
> grouping and tested a multiple chassis case which did not work.

But what the bug is?

> > Should this be a separate patch?
> 
> Is this really needed ? I have no way of testing this independently of
> the (other) Xsigo changes.
> 
> > > Note: this patch is against OFED 1.2
> > 
> > Hal, you know - the patches for master should be against master (I spent
> > some time).
> 
> Thanks. As you know, we are working with OFED 1.2.

But this patch targets master, not OFED 1.2. It is not something new -
the patches should be generated against branch they are targeted.

> 
> > Some comments are below.
> > 
> > > 
> > > Signed-off-by: Hal Rosenstock <hal at xsigo.com>
> > > 
> > > diff --git a/diags/include/grouping.h b/diags/include/grouping.h
> > > index 4666935..3ba872c 100644
> > > --- a/diags/include/grouping.h
> > > +++ b/diags/include/grouping.h
> > > @@ -1,5 +1,6 @@
> > >  /*
> > >   * Copyright (c) 2004-2007 Voltaire Inc.  All rights reserved.
> > > + * Copyright (c) 2007 Xsigo Systems Inc.  All rights reserved.
> > >   *
> > >   * This software is available to you under a choice of one of two
> > >   * licenses.  You may choose to be licensed under the terms of the GNU
> > > @@ -104,4 +105,8 @@ char *get_chassis_type(unsigned char chassistype);
> > >  char *get_chassis_slot(unsigned char chassisslot);
> > >  uint64_t get_chassis_guid(unsigned char chassisnum);
> > >  
> > > +int is_xsigo_guid(uint64_t guid);
> > > +int is_xsigo_tca(uint64_t guid);
> > > +int is_xsigo_hca(uint64_t guid);
> > > +
> > >  #endif	/* _GROUPING_H_ */
> > > diff --git a/diags/include/ibnetdiscover.h b/diags/include/ibnetdiscover.h
> > > index d13a666..bfbe7f5 100644
> > > --- a/diags/include/ibnetdiscover.h
> > > +++ b/diags/include/ibnetdiscover.h
> > > @@ -1,5 +1,6 @@
> > >  /*
> > >   * Copyright (c) 2004-2006 Voltaire Inc.  All rights reserved.
> > > + * Copyright (c) 2007 Xsigo Systems Inc.  All rights reserved. 
> > >   *
> > >   * This software is available to you under a choice of one of two
> > >   * licenses.  You may choose to be licensed under the terms of the GNU
> > > @@ -44,6 +45,7 @@
> > >  #define VTR_VENDOR_ID			0x8f1	/* Voltaire */
> > >  #define TS_VENDOR_ID			0x5ad	/* Cisco */
> > >  #define SS_VENDOR_ID			0x66a	/* InfiniCon */
> > > +#define XS_VENDOR_ID			0x1397	/* Xsigo */
> > >  
> > >  
> > >  typedef struct Port Port;
> > > diff --git a/diags/src/grouping.c b/diags/src/grouping.c
> > > index 0e5bd78..6602f26 100644
> > > --- a/diags/src/grouping.c
> > > +++ b/diags/src/grouping.c
> > > @@ -1,5 +1,6 @@
> > >  /*
> > >   * Copyright (c) 2004-2007 Voltaire Inc.  All rights reserved.
> > > + * Copyright (c) 2007 Xsigo Systems Inc.  All rights reserved.
> > >   *
> > >   * This software is available to you under a choice of one of two
> > >   * licenses.  You may choose to be licensed under the terms of the GNU
> > > @@ -96,20 +97,91 @@ static uint64_t topspin_chassisguid(uint64_t guid)
> > >  	return guid & 0xffffffff00ffffffULL;
> > >  }
> > >  
> > > -static uint64_t get_chassisguid(uint64_t guid, uint32_t vendid)
> > > +int is_xsigo_guid(uint64_t guid)
> > >  {
> > > -	if (vendid == TS_VENDOR_ID || vendid == SS_VENDOR_ID)
> > > -		return topspin_chassisguid(guid);
> > > +	if ((guid & 0xffffff0000000000ULL) == 0x0013970000000000ULL)
> > > +		return 1;
> > >  	else
> > > -		return guid;
> > > +		return 0;
> > > +}
> > > +
> > > +static int is_xsigo_leafone(uint64_t guid)
> > > +{
> > > +	if ((guid & 0xffffffffff000000ULL) == 0x0013970102000000ULL)
> > > +		return 1;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +int is_xsigo_hca(uint64_t guid)
> > > +{
> > > +	/* NodeType 2 is HCA */
> > > +	if ((guid & 0xffffffff00000000ULL) == 0x0013970200000000ULL)
> > > +		return 1;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +int is_xsigo_tca(uint64_t guid)
> > > +{
> > > +	/* NodeType 3 is TCA */
> > > +	if ((guid & 0xffffffff00000000ULL) == 0x0013970300000000ULL)
> > > +		return 1;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +static int is_xsigo_ca(uint64_t guid)
> > > +{
> > > +	if (is_xsigo_hca(guid) || is_xsigo_tca(guid))
> > > +		return 1;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +static int is_xsigo_switch(uint64_t guid)
> > > +{
> > > +	if ((guid & 0xffffffff00000000ULL) == 0x0013970100000000ULL)
> > > +		return 1;
> > > +	else
> > > +		return 0;
> > > +}
> > > +
> > > +static uint64_t xsigo_chassisguid(Node *node)
> > > +{
> > > +	if (!is_xsigo_ca(node->sysimgguid)) { 
> > > +		/* Byte 3 is NodeType and byte 4 is PortType */
> > > +		/* If NodeType is 1 (switch), PortType is masked */
> > > +		if (is_xsigo_switch(node->sysimgguid))
> > > +			return node->sysimgguid & 0xffffffff00ffffffULL;
> > > +		else
> > > +			return node->sysimgguid;
> > > +	} else {
> > > +		/* If peer port is Leaf 1, use its chassis GUID */
> > > +		if (is_xsigo_leafone(node->ports->remoteport->node->sysimgguid))
> > > +			return node->ports->remoteport->node->sysimgguid &
> > > +			       0xffffffff00ffffffULL;
> > > +		else
> > > +			return node->sysimgguid;
> > > +	}
> > >  }
> > >  
> > > -static struct ChassisList *find_chassisguid(uint64_t guid, uint32_t vendid)
> > > +static uint64_t get_chassisguid(Node *node)
> > > +{
> > > +	if (node->vendid == TS_VENDOR_ID || node->vendid == SS_VENDOR_ID)
> > > +		return topspin_chassisguid(node->sysimgguid);
> > > +	else if (node->vendid == XS_VENDOR_ID || is_xsigo_guid(node->sysimgguid))
> > > +		return xsigo_chassisguid(node);
> > > +	else
> > > +		return node->sysimgguid;
> > > +}
> > > +
> > > +static struct ChassisList *find_chassisguid(Node *node)
> > >  {
> > >  	ChassisList *current;
> > >  	uint64_t chguid;
> > >  
> > > -	chguid = get_chassisguid(guid, vendid);
> > > +	chguid = get_chassisguid(node);
> > >  	for (current = mylist.first; current; current = current->next) {
> > >  		if (current->chassisguid == chguid)
> > >  			return current;
> > > @@ -668,14 +740,13 @@ ChassisList *group_nodes()
> > >  			if (node->vendid == VTR_VENDOR_ID)
> > >  				continue;
> > >  			if (node->sysimgguid) {
> > > -				chassis = find_chassisguid(node->sysimgguid,
> > > -							   node->vendid);
> > > +				chassis = find_chassisguid(node);
> > >  				if (chassis)
> > >  					chassis->nodecount++;
> > >  				else {
> > >  					/* Possible new chassis */
> > >  					add_chassislist();
> > > -					mylist.current->chassisguid = get_chassisguid(node->sysimgguid, node->vendid);
> > > +					mylist.current->chassisguid = get_chassisguid(node);
> > >  					mylist.current->nodecount = 1;
> > >  				}
> > >  			}
> > > @@ -684,13 +755,12 @@ ChassisList *group_nodes()
> > >  
> > >  	/* now, make another pass to see which nodes are part of chassis */
> > >  	/* (defined as chassis->nodecount > 1) */
> > > -	for (dist = 0; dist <= maxhops_discovered; dist++) {
> > > +	for (dist = 0; dist <= MAXHOPS; ) {
> > >  		for (node = nodesdist[dist]; node; node = node->dnext) {
> > >  			if (node->vendid == VTR_VENDOR_ID)
> > >  				continue;
> > >  			if (node->sysimgguid) {
> > > -				chassis = find_chassisguid(node->sysimgguid,
> > > -							   node->vendid);
> > > +				chassis = find_chassisguid(node);
> > >  				if (chassis && chassis->nodecount > 1) {
> > >  					if (!chassis->chassisnum)
> > >  						chassis->chassisnum = ++chassisnum;
> > > @@ -702,6 +772,10 @@ ChassisList *group_nodes()
> > >  				}
> > >  			}
> > >  		}
> > > +		if (dist == maxhops_discovered)
> > > +			dist = MAXHOPS;	/* skip to CAs */
> > > +		else
> > > +			dist++;
> > >  	}
> > >  
> > >  	return (mylist.first);
> > > diff --git a/diags/src/ibnetdiscover.c b/diags/src/ibnetdiscover.c
> > > index cb62c44..2cff87e 100644
> > > --- a/diags/src/ibnetdiscover.c
> > > +++ b/diags/src/ibnetdiscover.c
> > > @@ -1,5 +1,6 @@
> > >  /*
> > >   * Copyright (c) 2004-2007 Voltaire Inc.  All rights reserved.
> > > + * Copyright (c) 2007 Xsigo Systems Inc.  All rights reserved.
> > >   *
> > >   * This software is available to you under a choice of one of two
> > >   * licenses.  You may choose to be licensed under the terms of the GNU
> > > @@ -450,14 +451,26 @@ list_node(Node *node)
> > >  }
> > >  
> > >  void
> > > -out_ids(Node *node)
> > > +out_ids(Node *node, int group, char *chname)
> > >  {
> > >  	fprintf(f, "\nvendid=0x%x\ndevid=0x%x\n", node->vendid, node->devid);
> > >  	if (node->sysimgguid)
> > > -		fprintf(f, "sysimgguid=0x%" PRIx64 "\n", node->sysimgguid);
> > > +		fprintf(f, "sysimgguid=0x%" PRIx64, node->sysimgguid);
> > > +	if (group)
> > > +		if (node->chrecord)
> > > +			if (node->chrecord->chassisnum) {
> > > +				fprintf(f, "\t\t# Chassis %d", node->chrecord->chassisnum);
> > > +				if (chname)
> > > +					fprintf(f, " (%s)", clean_nodedesc(chname));
> > > +				if (is_xsigo_tca(node->nodeguid)) {
> > > +					if (node->ports->remoteport)
> > > +						fprintf(f, " slot %d", node->ports->remoteport->portnum);
> > > +				}
> > > +			}
> > > +	fprintf(f, "\n");
> > >  }
> > >  
> > > -void
> > > +uint64_t
> > >  out_chassis(int chassisnum)
> > >  {
> > >  	uint64_t guid;
> > > @@ -467,20 +480,20 @@ out_chassis(int chassisnum)
> > >  	if (guid)
> > >  		fprintf(f, " (guid 0x%" PRIx64 ")", guid);
> > >  	fprintf(f, "\n");
> > > +	return guid;
> > >  }
> > >  
> > >  void
> > > -out_switch(Node *node, int group)
> > > +out_switch(Node *node, int group, char *chname)
> > >  {
> > >  	char *str;
> > >  	char *nodename = NULL;
> > >  
> > > -	out_ids(node);
> > > +	out_ids(node, group, chname);
> > >  	fprintf(f, "switchguid=0x%" PRIx64, node->nodeguid);
> > >  	if (group) {
> > >  		if (node->chrecord) {
> > >  			if (node->chrecord->chassisnum) {
> > > -				fprintf(f, "\t\t# Chassis %d ", node->chrecord->chassisnum);
> > >  				/* Currently, only if Voltaire chassis */
> > >  				if (node->vendid == VTR_VENDOR_ID) {
> > >  					str = get_chassis_type(node->chrecord->chassistype);
> > > @@ -510,12 +523,12 @@ out_switch(Node *node, int group)
> > >  }
> > >  
> > >  void
> > > -out_ca(Node *node)
> > > +out_ca(Node *node, int group, char *chname)
> > >  {
> > >  	char *node_type;
> > >  	char *node_type2;
> > >  
> > > -	out_ids(node);
> > > +	out_ids(node, group, chname);
> > >  	switch(node->type) {
> > >  	case CA_NODE:
> > >  		node_type = "ca";
> > > @@ -532,9 +545,12 @@ out_ca(Node *node)
> > >  	}
> > >  
> > >  	fprintf(f, "%sguid=0x%" PRIx64 "\n", node_type, node->nodeguid);
> > > -	fprintf(f, "%s\t%d %s\t\t# \"%s\"\n",
> > > +	fprintf(f, "%s\t%d %s\t\t# \"%s\"",
> > >  		node_type2, node->numports, node_name(node),
> > >  		clean_nodedesc(node->nodedesc));
> > > +	if (group && is_xsigo_hca(node->nodeguid))
> > > +		fprintf(f, " (scp)");
> > > +	fprintf(f, "\n");
> > >  }
> > >  
> > >  static char *
> > > @@ -572,12 +588,17 @@ out_switch_port(Port *port, int group)
> > >  		rem_nodename = clean_nodedesc(port->remoteport->node->nodedesc);
> > >  
> > >  	ext_port_str = out_ext_port(port->remoteport, group);
> > > -	fprintf(f, "\t%s[%d]%s\t\t# \"%s\" lid %d\n",
> > > +	fprintf(f, "\t%s[%d]%s\t\t# \"%s\" lid %d",
> > >  		node_name(port->remoteport->node),
> > >  		port->remoteport->portnum,
> > >  		ext_port_str ? ext_port_str : "",
> > >  		rem_nodename,
> > >  		port->remoteport->node->type == SWITCH_NODE ? port->remoteport->node->smalid : port->remoteport->lid);
> > > +	if (is_xsigo_tca(port->remoteport->portguid))
> > > +		fprintf(f, " slot %d", port->portnum);
> > > +	else if (is_xsigo_hca(port->remoteport->portguid))
> > > +		fprintf(f, " (scp)");
> > > +	fprintf(f, "\n");
> > >  
> > >  	if (rem_nodename && (port->remoteport->node->type == SWITCH_NODE))
> > >  		free(rem_nodename);
> > > @@ -616,6 +637,8 @@ dump_topology(int listtype, int group)
> > >  	Port *port;
> > >  	int i = 0, dist = 0;
> > >  	time_t t = time(0);
> > > +	uint64_t chguid;
> > > +	char *chname = NULL;
> > >  
> > >  	if (!listtype) {
> > >  		fprintf(f, "#\n# Topology file: generated on %s#\n", ctime(&t));
> > > @@ -633,11 +656,31 @@ dump_topology(int listtype, int group)
> > >  
> > >  			if (!ch->chassisnum)
> > >  				continue;
> > > -			out_chassis(ch->chassisnum);
> > > +			chguid = out_chassis(ch->chassisnum);
> > > +			chname = NULL;
> > > +			if (is_xsigo_guid(chguid)) {
> > > +				/* !!! */
> > > +				for (node = nodesdist[MAXHOPS]; node; node = node->dnext) {
> > > +					if (node->chrecord) {
> > > +						if (!node->chrecord->chassisnum)
> > > +							continue;
> > > +					} else
> > > +						continue;
> > > +
> > > +					if (node->chrecord->chassisnum != ch->chassisnum)
> > > +						continue;
> > > +
> > > +					if (is_xsigo_hca(node->nodeguid)) {
> > > +						chname = node->nodedesc;
> > > +						fprintf(f, "Hostname: %s\n", clean_nodedesc(node->nodedesc));
> > > +					}
> > > +				}
> > > +			}
> > > +
> > 
> > Not sure I understand this code correctly, but is it Xsigo only? I mean
> > where is_xsigo_hca() is used.
> 
> Yes, this is specific to Xsigo.
> 
> > Anyway why to not hide all this section inside out_chassis()?
> 
> It looks like it could be done as you suggest but it is currently done
> similar to other code slightly lower down which loop in a similar manner
> (Chassis Switches, Chassis CAs). 

Ok.

Sasha



More information about the general mailing list