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

[Eitan Zahavi]

> On Sun, 2005-12-18 at 03:53, Eitan Zahavi wrote:
> 
> [snip...]
> 
> > > This seems a little inconsistent to me. It seems like NeighborMTU
would
> > > be the equivalent of OperationalVLs, rather than MTUCap (which is
RO).
> 
> > Yes I think we should have checked the NeighborMTU and not the
MTUCap.
> 
> OK. I'll fix this.
[EZ] Thanks. I have seen the patch. It is fine.
> 
> > > 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. 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.
> 
> > > 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.
> 
> -- Hal