[openib-general] Re: [PATCH] opensm: observe PartitionEnforcementCap of zero

[Hal Rosenstock]

On Thu, 2006-03-30 at 11:26, Sasha Khapyorsky wrote:
> Hello Hal,
> 
> On 06:41 Thu 30 Mar     , Hal Rosenstock wrote:
> > On Wed, 2006-03-29 at 18:54, Rolf Manderscheid wrote:
> > > Hi Hal,
> > > 
> > > opensm attempts to set pkey table entries on external switch ports
> > > even if the switch declares a PartitionEnforcementCap of zero.  The
> > > consequence is ERR 4108.
> > 
> > I presume this is with the trunk version, right ?
> > 
> > > The decision to set the block is based on the size of the
> > > pkeys->blocks vector, which is initialized to one.  There is a
> > > comment that claims there must be a pre-allocated block in said vector
> > > "for the sake of empty table test", but I can't see why it's
> > > necessary.  Is this comment wrong or am I missing something?
> > > 
> > > The vector size grows, if necessary, when processing the response from
> > > a SubnGet(PKeyTable).  The query happens during a sweep after
> > > obtaining the portinfo, and the query code is careful to observe the
> > > partition cap.  All this means that the vector size can still be used
> > > to decide whether to do the set provided the vector size starts out at
> > > zero.  So, the patch below just initializes the vector to size zero
> > > and removes the code that inserts the pre-allocated block.
> > 
> > This appears like it would get past the ERR 4108 but wouldn't OpenSM
> > then insert the block several lines down in
> > osm_port.c::osm_physp_set_pkey_tbl where it calls osm_pkey_tbl_set ?
> 
> For me Rolf's patch looks fine. I don't see any good reason for pkey
> dummy table preallocation (probably historical reasons?).
> 
> > Also, is PortInfo:PartitionEnforcementInbound/Outbound set properly (0)
> > for these ports which don't support partition enforcement ? (That might
> > be another issue).
> 
> Partititon manager will try to update PortInfo in this case too. I don't
> know could it hurt anything or not, but it is obviously unnecessary
> action. There is the patch.
> 
> Sasha.
> 
> 
> Check Partition Enfocement capabilities before switch external ports'
> pkey tables update and partiton enforcement.
> 
> Signed-off-by: Sasha Khapyorsky <sashak at voltaire.com>
> ---
> 
>  osm/opensm/osm_pkey_mgr.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/osm/opensm/osm_pkey_mgr.c b/osm/opensm/osm_pkey_mgr.c
> index 28b475c..577fcc6 100644
> --- a/osm/opensm/osm_pkey_mgr.c
> +++ b/osm/opensm/osm_pkey_mgr.c
> @@ -55,6 +55,7 @@
>  #include <complib/cl_qmap.h>
>  #include <complib/cl_debug.h>
>  #include <opensm/osm_node.h>
> +#include <opensm/osm_switch.h>
>  #include <opensm/osm_pkey_mgr.h>
>  #include <opensm/osm_partition.h>
>  
> @@ -325,6 +326,15 @@ osm_pkey_mgr_update_peer_port(
>     p_node = osm_physp_get_node_ptr( peer );
>     if ( osm_node_get_type( p_node ) == IB_NODE_TYPE_CA )
>        return;
> +   else if ( osm_node_get_type( p_node ) == IB_NODE_TYPE_SWITCH ) {
> +      osm_switch_t *p_sw;
> +      ib_switch_info_t *p_si;
> +      if (!(p_sw = osm_get_switch_by_guid( p_mgr->p_subn,
> +                                           osm_node_get_node_guid( p_node ))) ||
> +	  !(p_si = osm_switch_get_si_ptr( p_sw )) ||
> +	  !p_si->enforce_cap)

Should switch port 0 be excepted from this ?

-- Hal

> +	  return;
> +   }
>  
>     if (p_mgr->p_subn->opt.no_partition_enforcement == TRUE)
>        goto _enforce_port;