[ofa-general] Re: [PATCH] opensm/opensm/osm_state_mgr.c: Add check for valid physical port before using pointer.

[Ira Weiny]

On Mon, 10 Nov 2008 22:13:33 +0200
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> On 09:57 Tue 04 Nov     , Ira Weiny wrote:
> > From 567c3893f24f4dc25ef5f4e74ef9deeb8ae541ad Mon Sep 17 00:00:00 2001
> > From: Ira Weiny <weiny2 at llnl.gov>
> > Date: Mon, 3 Nov 2008 14:47:50 -0800
> > Subject: [PATCH] opensm/opensm/osm_state_mgr.c: Add check for valid physical port before using
> >  pointer.
> > 
> >    There are times when PortInfo fails which leaves osm_node_t with invalid
> >    osm_physp_t pointers.  In this case do not use an invalid pointer.
> > 
> > Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
> 
> Applied. Thanks.
> 
> However some note is below.
> 
> > ---
> >  opensm/opensm/osm_state_mgr.c |    6 ++++++
> >  1 files changed, 6 insertions(+), 0 deletions(-)
> > 
> > diff --git a/opensm/opensm/osm_state_mgr.c b/opensm/opensm/osm_state_mgr.c
> > index ba3b6bf..841438c 100644
> > --- a/opensm/opensm/osm_state_mgr.c
> > +++ b/opensm/opensm/osm_state_mgr.c
> > @@ -542,6 +542,12 @@ static void __osm_state_mgr_get_node_desc(IN cl_map_item_t * const p_object,
> >  
> >  	/* get a physp to request from. */
> >  	p_physp = osm_node_get_any_physp_ptr(p_node);
> > +	if (!osm_physp_is_valid(p_physp)) {
> > +		OSM_LOG(sm->p_log, OSM_LOG_ERROR,
> > +			"__osm_state_mgr_get_node_desc: ERR 331C: "
> > +			"Failed to get valid physical port object\n");
> > +		goto exit;
> > +	}
> 
> Actually it can be a valid case. For example when node was first time
> discovered via port A, when this port was disconnected and the same node
> was discovered via port B - it is not a new node and node_info (where
> port number for osm_node_get_any_physp_ptr() is stored) will not be
> updated.

Ah, good point, I just happened to see it when PortInfo failed.

> 
> Obviously the patch is fine. But probably we need more general fix, for
> example to redo osm_node_get_any_physp_ptr() so that it will not return
> invalid ports. Need to review other osm_node_get_any_physp_ptr() usages.

I was wondering if it would return invalid ports ever.  It would be easy for it
to return only valid ports but perhaps that should be another function to
preserve functionality?

Ira