[openib-general] RE: A couple of questions about osm_lid_mgr.c::__osm_lid_mgr_set_physp_pi

[Hal Rosenstock]

Hi Eitan,

On Sun, 2005-12-18 at 14:20, Eitan Zahavi wrote:
> [EZ] Thanks. I have seen the patch. It is fine.

Thanks. I just committed it.

> > > > Also, why does changing the MTU require that the link be taken
> down ?
> > 
> > > The behavior of the link when a neighbor MTU is changes is not very
> well defined.
> > > So the best way to handle that is to force it down.
> > 
> > NeighborMTU is not involved with the link negotiation nor is there a
> > comment in the description like OperationalVLs. What behavior are you
> > referring to ?

> [EZ] I actually do not see any spec note about modifying neighbor MTU
> during link up.

Yes, that was what I was saying.

>  However, I remember we had to add this functionality. I
> try to dig this up in the old bit keeper and found the first occurrence
> of the setting of the port down in version 1.7. But the log does not say
> why.

Thanks for looking. This seems mysterious to me but I would hestitate to
remove it even though I don't think it should be required or if it is
some spec comment should be made. I would like to close the loop on this
but don't see how.

> > > > I also noticed a nit in the same function:
> > > >
> > > >   p_pi->m_key_lease_period =
> p_mgr->p_subn->opt.m_key_lease_period;
> > > >   /* Check to see if the value we are setting is different than
> > > >      the value in the port_info. If it is - turn on send_set flag
> */
> > > >   if (cl_memcmp( &p_pi->m_key_lease_period,
> > > > &p_old_pi->m_key_lease_period,
> > > >                  sizeof(p_pi->m_key_lease_period) ))
> > > >     send_set = TRUE;
> > > >
> > > > Should that be only when the Mkey is non 0 ?
> > 
> > > Well, I know the lease is not relevant when MKey = 0. But for code
> clarity I
> > > propose to ignore that fact. The effect is only when someone set
> lease period but
> > MKey = 0
> > > which IMO does not make any sense anyway.
> > 
> > I agree it does not make sense but could happen (is it prevented
> somehow
> > ?) so my take is to minimize the need for sets. As I said this is a
> nit.
> [EZ] We could avoid that but I do not think this is required.

I agree that it is not required. It's a trivial "optimization".

-- Hal