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

[Sasha Khapyorsky]

Hi Eitan,

On Thu, 2007-07-19 at 15:24 +0300, Eitan Zahavi wrote:
> Hi Sasha,
> 
> I was not sure if there might be a case where the Link manager will not
> touch the port.

It should, at least with IB_LINK_NO_CHANGE call. So I moved VLHighLimit
setup under this condition too (where most PortInfo fields are handled).
Will push soon. Thanks for the patch.

Sasha

> So I placed it on both sides. Can't remember now if it is possible or
> not.
> Thanks for taking care of it.
> 
> 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: Thursday, July 19, 2007 3:14 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
> > 
> > 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
> > > > 
> >