[ofw][IBAL] patch for pool corruption

Tue Mar 6 09:42:06 PST 2007

Hi Folks,

There is a chassis slot and an IOC slot.  The IOC slot is known because
the DM MADs (for service entries and IOC profile) are directed at the
IOC slots - the list of valid slots is returned in the IOU profile in
the ControllerList field.

The chassis slot should come from a ChassisInfo BM MAD.  I don't know if
the QLogic IOU and IOCs populate the system GUID with the chassis GUID -
Alex should be able to tell us.  In any case, if you get the ChassisInfo
to get the chassis slot you also get the chassis GUID.

I do know that when I wrote the code, the full location information was
put in the node description, so there is code there to take the
information from the node description and use it for formatting the
location information.  The device description is then substituted based
on the device ID to reflect the type of device, ignoring the node info.

I don't see anything wrong with leaving this behavior for now.  There's
precedent for having device-specific logic in the drivers - for example,
the WSD provider will change the MTU to 1K if it detects that it's using
a Tavor HCA.  As long as the device-specific logic is robust there's no
harm in having it, and the robustness is what Alex's patch was trying to
address.  It probably makes sense to separate the bug fix from the
design change discussion - apply the fix if it is correct, and continue
to discuss how to more generically handle the chassis GUID and slot
number.

The current logic should be removed once the code issues the ChassisInfo
query.

-Fab



-----Original Message-----
From: ofw-bounces at lists.openfabrics.org
[mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Yossi Leybovich
Sent: Tuesday, March 06, 2007 4:35 AM
To: Alex Estrin
Cc: ofw at lists.openfabrics.org
Subject: RE: [ofw][IBAL] patch for pool corruption

 

> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org 
> [mailto:ofw-bounces at lists.openfabrics.org] On Behalf Of Alex Estrin
> Sent: Monday, March 05, 2007 6:31 PM
> To: Yossi Leybovich
> Cc: ofw at lists.openfabrics.org
> Subject: RE: [ofw][IBAL] patch for pool corruption
> 
> 
> 
> > -----Original Message-----
> > From: ofw-bounces at lists.openfabrics.org [mailto:ofw- 
> > bounces at lists.openfabrics.org] On Behalf Of Alex Estrin
> > Sent: Monday, March 05, 2007 9:36 AM
> > To: Yossi Leybovich
> > Cc: ofw at lists.openfabrics.org
> > Subject: RE: [ofw][IBAL] patch for pool corruption
> > 
> > Hi,
> > 
> > > -----Original Message-----
> > > From: Yossi Leybovich [mailto:sleybo at mellanox.co.il]
> > > Sent: Sunday, March 04, 2007 2:25 AM
> > > To: Alex Estrin
> > > Cc: ofw at lists.openfabrics.org
> > > Subject: RE: [ofw][IBAL] patch for pool corruption
> > >
> > > Hi
> > >
> > > Do you know why ven_id 0x66a ( former SilverStorm ) need special 
> > > treatment for the location query?
> > No, I don't know yet.
> It seem passing through id_string is the only way to provide actual IO

> Unit geographical position(slot number) within chassis architectures 
> accepting multiple IOUnits.

I don't understand .

1. In the IB articture the slot number is mantion only regarding to IOC.
Do you need the IOU slot or the IOC slot in the IOU ?
(The only place where the is IOU slot is in the Appendix A1.3.1.3.1
which related to the BM mads)

2. I think that there is bug in the building of the IOU info structure
as it does not set the chassis guid and the chassis slot so the
inforamtion does not penetrate to the IOC structure.
I can add the chassis guid (which should be the system guid from the
node record query.
I don't know from where I can get the IOU slot number ....

The default location string is : Chassis 0x%016I64x, Slot %d, IOC %d You
should get the chassis guid and iou slot from the default query without
the id_string Can you check if this patch helps you 

Index: al/kernel/al_ioc_pnp.c
===================================================================

--- al/kernel/al_ioc_pnp.c	(revision 1935)
+++ al/kernel/al_ioc_pnp.c	(working copy)
@@ -883,6 +883,7 @@
 
 	p_iou->ca_guid = ca_guid;
 	p_iou->guid = p_node_rec->node_info.node_guid;
+	p_iou->chassis_guid = p_node_rec->node_info.sys_guid;
 	p_iou->vend_id = ib_node_info_get_vendor_id(
&p_node_rec->node_info );
 	p_iou->dev_id = p_node_rec->node_info.device_id;
 	p_iou->revision = p_node_rec->node_info.revision;

Any way I will add the your patch 



> > > Can you use id_string in the query_description ?
> > >
> > > Can we just use the same code for all IOCs
> > I guess we can.
> > > Yossi
> > >
> > >
> > > > -----Original Message-----
> > > > From: Alex Estrin [mailto:alex.estrin at qlogic.com]
> > > > Sent: Friday, March 02, 2007 5:30 PM
> > > > To: Yossi Leybovich
> > > > Cc: ofw at lists.openfabrics.org
> > > > Subject: [ofw][IBAL] patch for pool corruption
> > > >
> > > > Hi Yossi,
> > > >
> > > > This patch should fix potential pool corruption.
> > > > Please review.
> > > >
> > > > Thanks,
> > > > Alex
> > > >
> > > > Index: iou_ioc_mgr.c
> > > >
> ===================================================================
> > > > --- iou_ioc_mgr.c	(revision 599)
> > > > +++ iou_ioc_mgr.c	(working copy)
> > > > @@ -1142,7 +1142,9 @@
> > > >  		return STATUS_NO_SUCH_DEVICE;
> > > >  	}
> > > >
> > > > -	p_string = ExAllocatePool( PagedPool, 
> IOC_LOCATION_SIZE );
> > > > +	p_string = ExAllocatePool( PagedPool,
> > > > +		max( IOC_LOCATION_SIZE,
> > > > +		sizeof( WCHAR ) * ( sizeof(
> > > > p_ext->info.profile.id_string ) + 1 )));
> > > >  	if( !p_string )
> > > >  	{
> > > >  		IOU_PRINT_EXIT( TRACE_LEVEL_ERROR,
> IOU_DBG_ERROR,
> > > >
> > > >
> > _______________________________________________
> > ofw mailing list
> > ofw at lists.openfabrics.org
> > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
> _______________________________________________
> ofw mailing list
> ofw at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/ofw
> 

