[ofa-general] RE: [PATCH] opensm: Bug in coding trying to set vl_arb_high_limit

[Sasha Khapyorsky]

On Thu, 2007-07-19 at 07:51 +0300, Eitan Zahavi wrote:
> Ohh your right. The Enh0 should get an update. 
> I thought I got it right. Do you want me to provide an updated patch?

I can update on my side - I think we could remove VLHighLimit update
from osm_lid_mgr and have one only in osm_link_mgr.

Sasha

> 
> Eitan Zahavi
> Senior Engineering Director, Software Architect
> Mellanox Technologies LTD
> Tel:+972-4-9097208
> Fax:+972-4-9593245
> P.O. Box 586 Yokneam 20692 ISRAEL
> 
>  
> 
> > -----Original Message-----
> > From: Sasha Khapyorsky [mailto:sashak at voltaire.com] 
> > Sent: Wednesday, July 18, 2007 10:22 PM
> > To: Eitan Zahavi
> > Cc: OPENIB; hal.rosenstock at gmail.com; Yevgeny Kliteynik
> > Subject: Re: [PATCH] opensm: Bug in coding trying to set 
> > vl_arb_high_limit
> > 
> > Hi Eitan,
> > 
> > On 19:31 Wed 18 Jul     , Eitan Zahavi wrote:
> > > Hi Sasha
> > > 
> > > When QoS setup is done the code was trying to send updates of 
> > > vl_arb_high_limit by req_set of PORT_INFO with the new data.
> > > However, at that stage the SM still did not assign LIDs to 
> > the ports.
> > > So the sent PortInfo.base_lid was still zero. The 
> > specification does 
> > > not allow for such LIDs (they are considered ilegal).
> > > 
> > > the patch below fixes this by storing the calculated value 
> > and later 
> > > using it in link and lid managers.
> > 
> > Good, Thanks (and this also saves one PortInfo update MAD). 
> > One question below:
> > 
> > 
> > > 
> > > Eitan
> > > 
> > > Signed-off-by:  Eitan Zahavi <eitan at mellanox.co.il>
> > > 
> > 
> > [snip...]
> > 
> > > diff --git a/opensm/opensm/osm_lid_mgr.c 
> > b/opensm/opensm/osm_lid_mgr.c 
> > > index bc3f8b3..ed76382 100644
> > > --- a/opensm/opensm/osm_lid_mgr.c
> > > +++ b/opensm/opensm/osm_lid_mgr.c
> > > @@ -1182,6 +1182,14 @@ __osm_lid_mgr_set_physp_pi(
> > >             ib_port_info_get_port_state(p_old_pi) )
> > >          send_set = TRUE;
> > >      }
> > > +
> > > +	 /* provide the vl_high_limit from the qos mgr */
> > > +	 if (p_mgr->p_subn->opt.no_qos == FALSE) 
> > > +		 if (p_physp->vl_high_limit != p_old_pi->vl_high_limit)
> > > +		 {
> > > +			 send_set = TRUE;
> > > +			 p_pi->vl_high_limit = p_physp->vl_high_limit;
> > > +		 }
> > 
> > This part of code is for port_num != 0, so VLHighLimit setup 
> > will be skipped for switch enhanced port 0. Is it something 
> > expected? If so why?
> > 
> > Sasha
> >