[openib-general] [PATCH] osm: partition manager force policy
Eitan Zahavi
eitan at mellanox.co.il
Thu Jun 15 05:19:44 PDT 2006
Sasha Khapyorsky wrote:
> Hi Eitan,
>
> Some comments about the patch.
Thanks for the review.
The major point you bring up is the fact I intentionally impose the result of the
pkey settings on the SMDB and not wait for the GetResp to do that for me.
The idea I had was that once the Pkey Manager calculate the new tables any SA query
that is involving PKey matching would be using the results immediately.
But this actually opens up another bigger bug: What if the setting failed?
The SMDB will not that on next sweep and avoid sending the update.
So I think the best approach is to not set anything and
rely on the receive to perform the setting for me.
I will perform the changes test them and send a new patch.
>
> Personally I'm glad to see that you are using tab instead of spaces as
> identaion character. But it would be nice if next time you will not mix
> the functional changes and identaion fixes in the same patch, but instead
> will provide two different patches. Also it would be nice if your
> identation fixes will cover whole file(s) and not just selected lines.
>
> The same is about massive code moving, the patch separation may simplify
> review.
Yes you are correct about this. I will use this method in the future:
Use first patch with code changes and a second one with ordering and style changes.
>
> The rest is below.
>
> On 15:54 Tue 13 Jun , Eitan Zahavi wrote:
>
>>--text follows this line--
>>Hi Hal
>>
>>This is a second take after debug and cleanup of the partition manager
>>patch I have previously provided. The functionality is the same but
>>this one is after 2 days of testing on the simulator.
>>I also did some code restructuring for clarity.
>>
>>Tests passed were both dedicated pkey enforcements (pkey.*) and
>>stress test (osmStress.*)
>>
>>As I started to test the partition manager code (using ibmgtsim pkey test),
>>I realized the implementation does not really enforces the partition policy
>>on the given fabric. This patch fixes that. It was verified using the
>>simulation test. Several other corner cases were fixed too.
>>
>>Eitan
>>
>>Signed-off-by: Eitan Zahavi <eitan at mellanox.co.il>
>>
>>Index: include/opensm/osm_port.h
>>===================================================================
>>--- include/opensm/osm_port.h (revision 7867)
>>+++ include/opensm/osm_port.h (working copy)
>>@@ -586,6 +586,39 @@ osm_physp_get_pkey_tbl( IN const osm_phy
>> * Port, Physical Port
>> *********/
>>
>>+/****f* OpenSM: Physical Port/osm_physp_get_mod_pkey_tbl
>>+* NAME
>>+* osm_physp_get_mod_pkey_tbl
>>+*
>>+* DESCRIPTION
>>+* Returns a NON CONST pointer to the P_Key table object of the Physical Port object.
>>+*
>>+* SYNOPSIS
>>+*/
>>+static inline osm_pkey_tbl_t *
>>+osm_physp_get_mod_pkey_tbl( IN osm_physp_t* const p_physp )
>>+{
>>+ CL_ASSERT( osm_physp_is_valid( p_physp ) );
>>+ /*
>>+ (14.2.5.7) - the block number valid values are 0-2047, and are further
>>+ limited by the size of the P_Key table specified by the PartitionCap on the node.
>>+ */
>>+ return( &p_physp->pkeys );
>>+};
>>+/*
>>+* PARAMETERS
>>+* p_physp
>>+* [in] Pointer to an osm_physp_t object.
>>+*
>>+* RETURN VALUES
>>+* The pointer to the P_Key table object.
>>+*
>>+* NOTES
>>+*
>>+* SEE ALSO
>>+* Port, Physical Port
>>+*********/
>>+
>
>
> Is not this simpler to remove 'const' from existing
> osm_physp_get_pkey_tbl() function instead of using new one?
There are plenty of const functions using this function internally
so I would have need to fix them too.
>
>
>> /****f* OpenSM: Physical Port/osm_physp_set_slvl_tbl
>> * NAME
>> * osm_physp_set_slvl_tbl
>>Index: include/opensm/osm_pkey.h
>>===================================================================
>>--- include/opensm/osm_pkey.h (revision 7867)
>>+++ include/opensm/osm_pkey.h (working copy)
>>@@ -92,6 +92,9 @@ typedef struct _osm_pkey_tbl
>> cl_ptr_vector_t blocks;
>> cl_ptr_vector_t new_blocks;
>> cl_map_t keys;
>>+ cl_qlist_t pending;
>>+ uint16_t used_blocks;
>>+ uint16_t max_blocks;
>> } osm_pkey_tbl_t;
>> /*
>> * FIELDS
>>@@ -104,6 +107,18 @@ typedef struct _osm_pkey_tbl
>> * keys
>> * A set holding all keys
>> *
>>+* pending
>>+* A list osm_pending_pkey structs that is temporarily set by the
>>+* pkey mgr and used during pkey mgr algorithm only
>>+*
>>+* used_blocks
>>+* Tracks the number of blocks having non-zero pkeys
>>+*
>>+* max_blocks
>>+* The maximal number of blocks this partition table might hold
>>+* this value is based on node_info (for port 0 or CA) or switch_info
>>+* updated on receiving the node_info or switch_info GetResp
>>+*
>> * NOTES
>> * 'blocks' vector should be used to store pkey values obtained from
>> * the port and SM pkey manager should not change it directly, for this
>>@@ -114,6 +129,39 @@ typedef struct _osm_pkey_tbl
>> *
>> *********/
>>
>>+/****s* OpenSM: osm_pending_pkey_t
>>+* NAME
>>+* osm_pending_pkey_t
>>+*
>>+* DESCRIPTION
>>+* This objects stores temporary information on pkeys their target block and index
>>+* during the pkey manager operation
>>+*
>>+* SYNOPSIS
>>+*/
>>+typedef struct _osm_pending_pkey {
>>+ cl_list_item_t list_item;
>>+ uint16_t pkey;
>>+ uint32_t block;
>>+ uint8_t index;
>>+ boolean_t is_new;
>>+} osm_pending_pkey_t;
>>+/*
>>+* FIELDS
>>+* pkey
>>+* The actual P_Key
>>+*
>>+* block
>>+* The block index based on the previous table extracted from the device
>>+*
>>+* index
>>+* The index of the pky within the block
>>+*
>>+* is_new
>>+* TRUE for new P_Keys such that the block and index are invalid in that case
>>+*
>>+*********/
>>+
>> /****f* OpenSM: osm_pkey_tbl_construct
>> * NAME
>> * osm_pkey_tbl_construct
>>@@ -209,8 +257,8 @@ osm_pkey_tbl_get_num_blocks(
>> static inline ib_pkey_table_t *osm_pkey_tbl_block_get(
>> const osm_pkey_tbl_t *p_pkey_tbl, uint16_t block)
>> {
>>- CL_ASSERT(block < cl_ptr_vector_get_size(&p_pkey_tbl->blocks));
>>- return(cl_ptr_vector_get(&p_pkey_tbl->blocks, block));
>>+ return( (block < cl_ptr_vector_get_size(&p_pkey_tbl->blocks)) ?
>>+ cl_ptr_vector_get(&p_pkey_tbl->blocks, block) : NULL);
>> };
>> /*
>> * p_pkey_tbl
>>@@ -244,6 +292,106 @@ static inline ib_pkey_table_t *osm_pkey_
>> /*
>> *********/
>>
>>+
>>+/****f* OpenSM: osm_pkey_tbl_make_block_pair
>>+* NAME
>>+* osm_pkey_tbl_make_block_pair
>>+*
>>+* DESCRIPTION
>>+* Find or create a pair of "old" and "new" blocks for the
>>+* given block index
>>+*
>>+* SYNOPSIS
>>+*/
>>+int osm_pkey_tbl_make_block_pair(
>>+ osm_pkey_tbl_t *p_pkey_tbl,
>>+ uint16_t block_idx,
>>+ ib_pkey_table_t **pp_old_block,
>>+ ib_pkey_table_t **pp_new_block);
>>+/*
>>+* p_pkey_tbl
>>+* [in] Pointer to the PKey table
>>+*
>>+* block_idx
>>+* [in] The block index to use
>>+*
>>+* pp_old_block
>>+* [out] Pointer to the old block pointer arg
>>+*
>>+* pp_new_block
>>+* [out] Pointer to the new block pointer arg
>>+*
>>+* RETURN VALUES
>>+* 0 if OK 1 if failed
>
>
> It is better (conventional) to use -1 as failure return status.
I have seen and used both - depend on the application.
I think I should have used IB_SUCCESS or IB_ERROR but I do not
mind changing that to -1 too.
>
>
>>+*
>>+*********/
>>+
>>+/****f* OpenSM: osm_pkey_tbl_set_new_entry
>>+* NAME
>>+* osm_pkey_tbl_set_new_entry
>>+*
>>+* DESCRIPTION
>>+* stores the given pkey in the "new" blocks array and update
>>+* the "map" to show that on the "old" blocks
>>+*
>>+* SYNOPSIS
>>+*/
>>+int
>>+osm_pkey_tbl_set_new_entry(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ IN uint16_t block_idx,
>>+ IN uint8_t pkey_idx,
>>+ IN uint16_t pkey);
>>+/*
>>+* p_pkey_tbl
>>+* [in] Pointer to the PKey table
>>+*
>>+* block_idx
>>+* [in] The block index to use
>>+*
>>+* pkey_idx
>>+* [in] The index within the block
>>+*
>>+* pkey
>>+* [in] PKey to store
>>+*
>>+* RETURN VALUES
>>+* 0 if OK 1 if failed
>
>
> Ditto
>
>
>>+*
>>+*********/
>>+
>>+/****f* OpenSM: osm_pkey_find_next_free_entry
>>+* NAME
>>+* osm_pkey_find_next_free_entry
>>+*
>>+* DESCRIPTION
>>+* Find the next free entry in the PKey table. Starting at the given
>>+* index and block number. The user should increment pkey_idx before
>>+* next call
>>+* Inspect the "new" blocks array for empty space.
>>+*
>>+* SYNOPSIS
>>+*/
>>+boolean_t
>>+osm_pkey_find_next_free_entry(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ OUT uint16_t *p_block_idx,
>>+ OUT uint8_t *p_pkey_idx);
>>+/*
>>+* p_pkey_tbl
>>+* [in] Pointer to the PKey table
>>+*
>>+* p_block_idx
>>+* [out] The block index to use
>>+*
>>+* p_pkey_idx
>>+* [out] The index within the block to use
>>+*
>>+* RETURN VALUES
>>+* TRUE if found FALSE if did not find
>>+*
>>+*********/
>>+
>> /****f* OpenSM: osm_pkey_tbl_sync_new_blocks
>> * NAME
>> * osm_pkey_tbl_sync_new_blocks
>>@@ -263,9 +411,44 @@ void osm_pkey_tbl_sync_new_blocks(
>> *
>> *********/
>>
>>+/****f* OpenSM: osm_pkey_tbl_get_block_and_idx
>>+* NAME
>>+* osm_pkey_tbl_get_block_and_idx
>>+*
>>+* DESCRIPTION
>>+* set the block index and pkey index the given
>>+* pkey is found in. return 1 if cound not find
>>+* it, 0 if OK
>>+*
>>+* SYNOPSIS
>>+*/
>>+int
>>+osm_pkey_tbl_get_block_and_idx(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ IN uint16_t *p_pkey,
>>+ OUT uint32_t *block_idx,
>>+ OUT uint8_t *pkey_index);
>>+/*
>>+* p_pkey_tbl
>>+* [in] Pointer to osm_pkey_tbl_t object.
>>+*
>>+* p_pkey
>>+* [in] Pointer to the P_Key entry searched
>>+*
>>+* p_block_idx
>>+* [out] Pointer to the block index to be updated
>>+*
>>+* p_pkey_idx
>>+* [out] Pointer to the pkey index (in the block) to be updated
>>+*
>>+*
>>+* NOTES
>>+*
>>+*********/
>>+
>> /****f* OpenSM: osm_pkey_tbl_set
>> * NAME
>> * osm_pkey_tbl_set
>>Index: opensm/osm_pkey.c
>>===================================================================
>>--- opensm/osm_pkey.c (revision 7904)
>>+++ opensm/osm_pkey.c (working copy)
>>@@ -100,6 +100,9 @@ int osm_pkey_tbl_init(
>> cl_ptr_vector_init( &p_pkey_tbl->blocks, 0, 1);
>> cl_ptr_vector_init( &p_pkey_tbl->new_blocks, 0, 1);
>> cl_map_init( &p_pkey_tbl->keys, 1 );
>>+ cl_qlist_init( &p_pkey_tbl->pending );
>>+ p_pkey_tbl->used_blocks = 0;
>>+ p_pkey_tbl->max_blocks = 0;
>> return(IB_SUCCESS);
>> }
>>
>>@@ -118,14 +121,29 @@ void osm_pkey_tbl_sync_new_blocks(
>> p_block = cl_ptr_vector_get(&p_pkey_tbl->blocks, b);
>> if ( b < new_blocks )
>> p_new_block = cl_ptr_vector_get(&p_pkey_tbl->new_blocks, b);
>>- else {
>>+ else
>>+ {
>> p_new_block = (ib_pkey_table_t *)malloc(sizeof(*p_new_block));
>> if (!p_new_block)
>> break;
>>+ cl_ptr_vector_set(&((osm_pkey_tbl_t *)p_pkey_tbl)->new_blocks,
>>+ b, p_new_block);
>>+ }
>>+
>> memset(p_new_block, 0, sizeof(*p_new_block));
>>- cl_ptr_vector_set(&((osm_pkey_tbl_t *)p_pkey_tbl)->new_blocks, b, p_new_block);
>> }
>>- memcpy(p_new_block, p_block, sizeof(*p_new_block));
>>+}
>
>
> You changed this function so it does not do any sync anymore. Should
> function name be changed too?
Yes correct I will change it. Is a better name:
osm_pkey_tbl_init_new_blocks ?
>
>
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+void osm_pkey_tbl_cleanup_pending(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl)
>>+{
>>+ cl_list_item_t *p_item;
>>+ p_item = cl_qlist_remove_head( &p_pkey_tbl->pending );
>>+ while (p_item != cl_qlist_end( &p_pkey_tbl->pending ) )
>>+ {
>>+ free( (osm_pending_pkey_t *)p_item );
>> }
>> }
>>
>>@@ -202,6 +220,138 @@ int osm_pkey_tbl_set(
>>
>> /**********************************************************************
>> **********************************************************************/
>>+int osm_pkey_tbl_make_block_pair(
>>+ osm_pkey_tbl_t *p_pkey_tbl,
>>+ uint16_t block_idx,
>>+ ib_pkey_table_t **pp_old_block,
>>+ ib_pkey_table_t **pp_new_block)
>>+{
>>+ if (block_idx >= p_pkey_tbl->max_blocks) return 1;
>>+
>>+ if (pp_old_block)
>>+ {
>>+ *pp_old_block = osm_pkey_tbl_block_get( p_pkey_tbl, block_idx );
>>+ if (! *pp_old_block)
>>+ {
>>+ *pp_old_block = (ib_pkey_table_t *)malloc(sizeof(ib_pkey_table_t));
>>+ if (!*pp_old_block) return 1;
>>+ memset(*pp_old_block, 0, sizeof(ib_pkey_table_t));
>>+ cl_ptr_vector_set(&p_pkey_tbl->blocks, block_idx, *pp_old_block);
>>+ }
>>+ }
>>+
>>+ if (pp_new_block)
>>+ {
>>+ *pp_new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_idx );
>>+ if (! *pp_new_block)
>>+ {
>>+ *pp_new_block = (ib_pkey_table_t *)malloc(sizeof(ib_pkey_table_t));
>>+ if (!*pp_new_block) return 1;
>>+ memset(*pp_new_block, 0, sizeof(ib_pkey_table_t));
>>+ cl_ptr_vector_set(&p_pkey_tbl->new_blocks, block_idx, *pp_new_block);
>>+ }
>>+ }
>>+ return 0;
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+/*
>>+ store the given pkey in the "new" blocks array and update the "map"
>>+ to show that on the "old" blocks
>>+*/
>>+int
>>+osm_pkey_tbl_set_new_entry(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ IN uint16_t block_idx,
>>+ IN uint8_t pkey_idx,
>>+ IN uint16_t pkey)
>>+{
>>+ ib_pkey_table_t *p_old_block;
>>+ ib_pkey_table_t *p_new_block;
>>+
>>+ if (osm_pkey_tbl_make_block_pair(
>>+ p_pkey_tbl, block_idx, &p_old_block, &p_new_block))
>>+ return 1;
>>+
>>+ cl_map_insert( &p_pkey_tbl->keys,
>>+ ib_pkey_get_base(pkey),
>>+ &(p_old_block->pkey_entry[pkey_idx]));
>
>
> Here you map potentially empty pkey entry. Why? "old block" will be
> remapped anyway on pkey receiving.
The reason I did this was that if the GetResp will fail I still want to represent
the settings in the map.But actually it might be better not to do that so next
time we run we will not find it without a GetResp.
>
> Actually I don't see why you want this pretty tricky and pkey_mgr
> specific procedure as generic function.
I think once the new_blocks was made available through the osm_pkey.h
we actually burden the pkey table object with the full complexity of the
pkey manager. So I think the right place for the functions changing the
pkey table is in the osm_pkey.*
>
>
>>+ p_new_block->pkey_entry[pkey_idx] = pkey;
>>+ if (p_pkey_tbl->used_blocks < block_idx)
>>+ p_pkey_tbl->used_blocks = block_idx;
>>+
>>+ return 0;
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+boolean_t
>>+osm_pkey_find_next_free_entry(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ OUT uint16_t *p_block_idx,
>>+ OUT uint8_t *p_pkey_idx)
>>+{
>>+ ib_pkey_table_t *p_new_block;
>>+
>>+ CL_ASSERT(p_block_idx);
>>+ CL_ASSERT(p_pkey_idx);
>>+
>>+ while ( *p_block_idx < p_pkey_tbl->max_blocks)
>>+ {
>>+ if (*p_pkey_idx > IB_NUM_PKEY_ELEMENTS_IN_BLOCK - 1)
>>+ {
>>+ *p_pkey_idx = 0;
>>+ (*p_block_idx)++;
>>+ if (*p_block_idx >= p_pkey_tbl->max_blocks)
>>+ return FALSE;
>>+ }
>>+
>>+ p_new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, *p_block_idx);
>>+
>>+ if ( !p_new_block ||
>>+ ib_pkey_is_invalid(p_new_block->pkey_entry[*p_pkey_idx]))
>>+ return TRUE;
>>+ else
>>+ (*p_pkey_idx)++;
>>+ }
>>+ return FALSE;
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+int
>>+osm_pkey_tbl_get_block_and_idx(
>>+ IN osm_pkey_tbl_t *p_pkey_tbl,
>>+ IN uint16_t *p_pkey,
>>+ OUT uint32_t *p_block_idx,
>>+ OUT uint8_t *p_pkey_index)
>>+{
>>+ uint32_t num_of_blocks;
>>+ uint32_t block_index;
>>+ ib_pkey_table_t *block;
>>+
>>+ CL_ASSERT( p_pkey_tbl );
>>+ CL_ASSERT( p_block_idx != NULL );
>>+ CL_ASSERT( p_pkey_idx != NULL );
>
>
> Why last two CL_ASSERTs? What should be problem with uninitialized
> pointers here?
>
These are the outputs of the function. It does not make sense to call the functions with
null output pointers (calling by ref) . Anyway instead of putting the check in the free build
I used an assert
>
>>+
>>+ num_of_blocks = cl_ptr_vector_get_size( &p_pkey_tbl->blocks);
>>+ for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>>+ {
>>+ block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index );
>>+ if ( ( block->pkey_entry <= p_pkey ) &&
>>+ ( p_pkey < block->pkey_entry + IB_NUM_PKEY_ELEMENTS_IN_BLOCK))
>>+ {
>>+ *p_block_idx = block_index;
>>+ *p_pkey_index = p_pkey - block->pkey_entry;
>>+ return 0;
>>+ }
>>+ }
>>+ return 1;
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>> static boolean_t __osm_match_pkey (
>> IN const ib_net16_t *pkey1,
>> IN const ib_net16_t *pkey2 ) {
>>@@ -305,7 +455,8 @@ osm_physp_share_pkey(
>> if (cl_is_map_empty(&pkey_tbl1->keys) || cl_is_map_empty(&pkey_tbl2->keys))
>> return TRUE;
>>
>>- return !ib_pkey_is_invalid(osm_physp_find_common_pkey(p_physp_1, p_physp_2));
>>+ return
>>+ !ib_pkey_is_invalid(osm_physp_find_common_pkey(p_physp_1, p_physp_2));
>> }
>>
>> /**********************************************************************
>>@@ -321,7 +472,8 @@ osm_port_share_pkey(
>>
>> OSM_LOG_ENTER( p_log, osm_port_share_pkey );
>>
>>- if (!p_port_1 || !p_port_2) {
>>+ if (!p_port_1 || !p_port_2)
>>+ {
>> ret = FALSE;
>> goto Exit;
>> }
>>@@ -329,7 +481,8 @@ osm_port_share_pkey(
>> p_physp1 = osm_port_get_default_phys_ptr(p_port_1);
>> p_physp2 = osm_port_get_default_phys_ptr(p_port_2);
>>
>>- if (!p_physp1 || !p_physp2) {
>>+ if (!p_physp1 || !p_physp2)
>>+ {
>> ret = FALSE;
>> goto Exit;
>> }
>>Index: opensm/osm_pkey_mgr.c
>>===================================================================
>>--- opensm/osm_pkey_mgr.c (revision 7904)
>>+++ opensm/osm_pkey_mgr.c (working copy)
>>@@ -62,6 +62,139 @@
>>
>> /**********************************************************************
>> **********************************************************************/
>>+/*
>>+ the max number of pkey blocks for a physical port is located in
>>+ different place for switch external ports (SwitchInfo) and the
>>+ rest of the ports (NodeInfo)
>>+*/
>>+static int pkey_mgr_get_physp_max_blocks(
>
>
> I would suggest to add _cap_ to function name. Not too much critical
> since it is static function.
>
>
>>+ IN const osm_subn_t *p_subn,
>>+ IN const osm_physp_t *p_physp)
>>+{
>>+ osm_node_t *p_node = osm_physp_get_node_ptr(p_physp);
>>+ osm_switch_t *p_sw;
>>+ uint16_t num_pkeys = 0;
>>+
>>+ if ( (osm_node_get_type(p_node) != IB_NODE_TYPE_SWITCH) ||
>>+ (osm_physp_get_port_num( p_physp ) == 0))
>>+ num_pkeys = cl_ntoh16( p_node->node_info.partition_cap );
>>+ else
>>+ {
>>+ p_sw = osm_get_switch_by_guid(p_subn, p_node->node_info.node_guid);
>>+ if (p_sw)
>>+ num_pkeys = cl_ntoh16( p_sw->switch_info.enforce_cap );
>>+ }
>>+ return( (num_pkeys + 31) / 32 );
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+/*
>>+ * Insert the new pending pkey entry to the specific port pkey table
>>+ * pending pkeys. new entries are inserted at the back.
>>+ */
>>+static void pkey_mgr_process_physical_port(
>>+ IN osm_log_t *p_log,
>>+ IN const osm_req_t *p_req,
>>+ IN const ib_net16_t pkey,
>>+ IN osm_physp_t *p_physp )
>>+{
>>+ osm_node_t *p_node = osm_physp_get_node_ptr( p_physp );
>>+ osm_pkey_tbl_t *p_pkey_tbl;
>>+ ib_net16_t *p_orig_pkey;
>>+ char *stat = NULL;
>>+ osm_pending_pkey_t *p_pending;
>>+
>>+ p_pkey_tbl = osm_physp_get_mod_pkey_tbl( p_physp );
>>+ if (! p_pkey_tbl)
>
> ^^^^^^^^^^^^^
> Is it possible?
Yes it is ! I run into it during testing. The port did not have any pkey table.
>
>
>>+ {
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_process_physical_port: ERR 0501: "
>>+ "No pkey table found for node "
>>+ "0x%016" PRIx64 " port %u\n",
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>>+ return;
>>+ }
>>+
>>+ p_pending = (osm_pending_pkey_t *)malloc(sizeof(osm_pending_pkey_t));
>>+ if (! p_pending)
>>+ {
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_process_physical_port: ERR 0502: "
>>+ "Fail to allocate new pending pkey entry for node "
>>+ "0x%016" PRIx64 " port %u\n",
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>>+ return;
>>+ }
>>+ p_pending->pkey = pkey;
>>+ p_orig_pkey = cl_map_get( &p_pkey_tbl->keys, ib_pkey_get_base( pkey ) );
>>+ if ( !p_orig_pkey ||
>>+ (ib_pkey_get_base(*p_orig_pkey) != ib_pkey_get_base(pkey) ))
>
>
> There the cases of new pkey and updated pkey membership is mixed. Why?
I am not following your question.
The specific case I am trying to catch is the one that for some reason the map points to
a pkey entry that was modified somehow and is different then the one you would expect by
the map.
>
>
>>+ {
>>+ p_pending->is_new = TRUE;
>>+ cl_qlist_insert_tail(&p_pkey_tbl->pending, (cl_list_item_t*)p_pending);
>>+ stat = "inserted";
>>+ }
>>+ else
>>+ {
>>+ p_pending->is_new = FALSE;
>>+ if (osm_pkey_tbl_get_block_and_idx(p_pkey_tbl, p_orig_pkey,
>>+ &p_pending->block, &p_pending->index))
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> AFAIK in this function there were CL_ASSERTs which check for uinitialized
> pointers.
True. So the asserts are not required in this case.
>
>
>>+ {
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_process_physical_port: ERR 0503: "
>>+ "Fail to obtain P_Key 0x%04x block and index for node "
>>+ "0x%016" PRIx64 " port %u\n",
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>>+ return;
>>+ }
>>+ cl_qlist_insert_head(&p_pkey_tbl->pending, (cl_list_item_t*)p_pending);
>>+ stat = "updated";
>
>
> Is it will be updated? It is likely "already there" case. No?
>
> Also in this case you can already put the pkey in new_block instead of
> holding it in pending list. Then later you will only need to add new
> pkeys. This may simplify the flow and even save some mem.
True but in my mind it does not simplify - on the contrary it makes the partition between
populating each port pending list and actually setting the pkey tables mixed.
I do not think the memory impact deserves this mix of staging
>
>
>>+ }
>>+
>>+ osm_log( p_log, OSM_LOG_DEBUG,
>>+ "pkey_mgr_process_physical_port: "
>>+ "pkey 0x%04x was %s for node 0x%016" PRIx64
>>+ " port %u\n",
>>+ cl_ntoh16( pkey ), stat,
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>>+static void
>>+pkey_mgr_process_partition_table(
>>+ osm_log_t *p_log,
>>+ const osm_req_t *p_req,
>>+ const osm_prtn_t *p_prtn,
>>+ const boolean_t full )
>>+{
>>+ const cl_map_t *p_tbl = full ?
>>+ &p_prtn->full_guid_tbl : &p_prtn->part_guid_tbl;
>>+ cl_map_iterator_t i, i_next;
>>+ ib_net16_t pkey = p_prtn->pkey;
>>+ osm_physp_t *p_physp;
>>+
>>+ if ( full )
>>+ pkey = cl_hton16( cl_ntoh16( pkey ) | 0x8000 );
>>+
>>+ i_next = cl_map_head( p_tbl );
>>+ while ( i_next != cl_map_end( p_tbl ) )
>>+ {
>>+ i = i_next;
>>+ i_next = cl_map_next( i );
>>+ p_physp = cl_map_obj( i );
>>+ if ( p_physp && osm_physp_is_valid( p_physp ) )
>>+ pkey_mgr_process_physical_port( p_log, p_req, pkey, p_physp );
>>+ }
>>+}
>>+
>>+/**********************************************************************
>>+ **********************************************************************/
>> static ib_api_status_t
>> pkey_mgr_update_pkey_entry(
>> IN const osm_req_t *p_req,
>>@@ -114,7 +247,8 @@ pkey_mgr_enforce_partition(
>> p_pi->state_info2 = 0;
>> ib_port_info_set_port_state( p_pi, IB_LINK_NO_CHANGE );
>>
>>- context.pi_context.node_guid = osm_node_get_node_guid( osm_physp_get_node_ptr( p_physp ) );
>>+ context.pi_context.node_guid =
>>+ osm_node_get_node_guid( osm_physp_get_node_ptr( p_physp ) );
>> context.pi_context.port_guid = osm_physp_get_port_guid( p_physp );
>> context.pi_context.set_method = TRUE;
>> context.pi_context.update_master_sm_base_lid = FALSE;
>>@@ -131,80 +265,132 @@ pkey_mgr_enforce_partition(
>>
>> /**********************************************************************
>> **********************************************************************/
>>-/*
>>- * Prepare a new entry for the pkey table for this port when this pkey
>>- * does not exist. Update existed entry when membership was changed.
>>- */
>>-static void pkey_mgr_process_physical_port(
>>- IN osm_log_t *p_log,
>>- IN const osm_req_t *p_req,
>>- IN const ib_net16_t pkey,
>>- IN osm_physp_t *p_physp )
>>+static boolean_t pkey_mgr_update_port(
>>+ osm_log_t *p_log,
>>+ osm_req_t *p_req,
>>+ const osm_port_t * const p_port )
>> {
>>- osm_node_t *p_node = osm_physp_get_node_ptr( p_physp );
>>- ib_pkey_table_t *block;
>>+ osm_physp_t *p_physp;
>>+ osm_node_t *p_node;
>>+ ib_pkey_table_t *block, *new_block;
>>+ osm_pkey_tbl_t *p_pkey_tbl;
>> uint16_t block_index;
>>+ uint8_t pkey_index;
>>+ uint16_t last_free_block_index = 0;
>>+ uint16_t last_free_pkey_index = 0;
>> uint16_t num_of_blocks;
>>- const osm_pkey_tbl_t *p_pkey_tbl;
>>- ib_net16_t *p_orig_pkey;
>>- char *stat = NULL;
>>- uint32_t i;
>>+ uint16_t max_num_of_blocks;
>>
>>- p_pkey_tbl = osm_physp_get_pkey_tbl( p_physp );
>>- num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl );
>>+ ib_api_status_t status;
>>+ boolean_t ret_val = FALSE;
>>+ osm_pending_pkey_t *p_pending;
>>+ boolean_t found;
>>
>>- p_orig_pkey = cl_map_get( &p_pkey_tbl->keys, ib_pkey_get_base( pkey ) );
>>+ p_physp = osm_port_get_default_phys_ptr( p_port );
>>+ if ( !osm_physp_is_valid( p_physp ) )
>>+ return FALSE;
>>
>>- if ( !p_orig_pkey )
>>- {
>>- for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>>+ p_pkey_tbl = osm_physp_get_mod_pkey_tbl( p_physp );
>>+ num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl );
>>+ max_num_of_blocks = pkey_mgr_get_physp_max_blocks( p_req->p_subn, p_physp );
>>+ if ( p_pkey_tbl->max_blocks > max_num_of_blocks )
>> {
>>- block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index );
>>- for ( i = 0; i < IB_NUM_PKEY_ELEMENTS_IN_BLOCK; i++ )
>>+ osm_log( p_log, OSM_LOG_INFO,
>>+ "pkey_mgr_update_port: "
>>+ "Max number of blocks reduced from %u to %u "
>>+ "for node 0x%016" PRIx64 " port %u\n",
>>+ p_pkey_tbl->max_blocks, max_num_of_blocks,
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>>+ }
>>+ p_pkey_tbl->max_blocks = max_num_of_blocks;
>>+
>>+ osm_pkey_tbl_sync_new_blocks( p_pkey_tbl );
>>+ cl_map_remove_all( &p_pkey_tbl->keys );
>
>
> What is the reason to drop map here? AFAIK it will be reinitialized later
> anyway when pkey blocks will be received.
What if it is not received?
>
>
>>+ p_pkey_tbl->used_blocks = 0;
>>+
>>+ /*
>>+ process every pending pkey in order -
>>+ first must be "updated" last are "new"
>>+ */
>>+ p_pending =
>>+ (osm_pending_pkey_t *)cl_qlist_remove_head( &p_pkey_tbl->pending );
>>+ while (p_pending !=
>>+ (osm_pending_pkey_t *)cl_qlist_end( &p_pkey_tbl->pending ) )
>>+ {
>>+ if (p_pending->is_new == FALSE)
>>+ {
>>+ block_index = p_pending->block;
>>+ pkey_index = p_pending->index;
>>+ found = TRUE;
>>+ }
>>+ else
>> {
>>- if ( ib_pkey_is_invalid( block->pkey_entry[i] ) )
>>+ found = osm_pkey_find_next_free_entry(p_pkey_tbl,
>>+ &last_free_block_index,
>>+ &last_free_pkey_index);
>
>
> There should be warning: expected third arg is uint8_t*
True. I will fix the variable declaration to uint8_t
>
>
>>+ if ( !found )
>> {
>>- block->pkey_entry[i] = pkey;
>>- stat = "inserted";
>>- goto _done;
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_update_port: ERR 0504: "
>>+ "failed to find empty space for new pkey 0x%04x "
>>+ "of node 0x%016" PRIx64 " port %u\n",
>>+ cl_ntoh16(p_pending->pkey),
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( p_physp ) );
>> }
>>+ else
>>+ {
>>+ block_index = last_free_block_index;
>>+ pkey_index = last_free_pkey_index++;
>> }
>> }
>>+
>>+ if (found)
>>+ {
>>+ if (osm_pkey_tbl_set_new_entry(
>>+ p_pkey_tbl, block_index, pkey_index, p_pending->pkey) )
>>+ {
>> osm_log( p_log, OSM_LOG_ERROR,
>>- "pkey_mgr_process_physical_port: ERR 0501: "
>>- "No empty pkey entry was found to insert 0x%04x for node "
>>- "0x%016" PRIx64 " port %u\n",
>>- cl_ntoh16( pkey ),
>>+ "pkey_mgr_update_port: ERR 0505: "
>>+ "failed to set PKey 0x%04x in block %u idx %u "
>>+ "of node 0x%016" PRIx64 " port %u\n",
>>+ p_pending->pkey, block_index, pkey_index,
>> cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>> osm_physp_get_port_num( p_physp ) );
>> }
>>- else if ( *p_orig_pkey != pkey )
>>- {
>>+ }
>>+
>>+ free( p_pending );
>>+ p_pending =
>>+ (osm_pending_pkey_t *)cl_qlist_remove_head( &p_pkey_tbl->pending );
>>+ }
>>+
>>+ /* now look for changes and store */
>> for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>> {
>>- /* we need real block (not just new_block) in order
>>- * to resolve block/pkey indices */
>> block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index );
>>- i = p_orig_pkey - block->pkey_entry;
>>- if (i < IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
>>- block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index );
>>- block->pkey_entry[i] = pkey;
>>- stat = "updated";
>>- goto _done;
>>- }
>>- }
>>- }
>>+ new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index );
>>
>>- _done:
>>- if (stat) {
>>- osm_log( p_log, OSM_LOG_VERBOSE,
>>- "pkey_mgr_process_physical_port: "
>>- "pkey 0x%04x was %s for node 0x%016" PRIx64
>>- " port %u\n",
>>- cl_ntoh16( pkey ), stat,
>>+ if (block &&
>>+ (!new_block || !memcmp( new_block, block, sizeof( *block ) )) )
>>+ continue;
>>+
>>+ status = pkey_mgr_update_pkey_entry(
>>+ p_req, p_physp , new_block, block_index );
>>+ if (status == IB_SUCCESS)
>>+ ret_val = TRUE;
>>+ else
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_update_port: ERR 0506: "
>>+ "pkey_mgr_update_pkey_entry() failed to update "
>>+ "pkey table block %d for node 0x%016" PRIx64 " port %u\n",
>>+ block_index,
>> cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>> osm_physp_get_port_num( p_physp ) );
>> }
>>+
>>+ return ret_val;
>> }
>>
>> /**********************************************************************
>>@@ -217,21 +403,23 @@ pkey_mgr_update_peer_port(
>> const osm_port_t * const p_port,
>> boolean_t enforce )
>> {
>>- osm_physp_t *p, *peer;
>>+ osm_physp_t *p_physp, *peer;
>> osm_node_t *p_node;
>> ib_pkey_table_t *block, *peer_block;
>>- const osm_pkey_tbl_t *p_pkey_tbl, *p_peer_pkey_tbl;
>>+ const osm_pkey_tbl_t *p_pkey_tbl;
>>+ osm_pkey_tbl_t *p_peer_pkey_tbl;
>> osm_switch_t *p_sw;
>> ib_switch_info_t *p_si;
>> uint16_t block_index;
>> uint16_t num_of_blocks;
>>+ uint16_t peer_max_blocks;
>> ib_api_status_t status = IB_SUCCESS;
>> boolean_t ret_val = FALSE;
>>
>>- p = osm_port_get_default_phys_ptr( p_port );
>>- if ( !osm_physp_is_valid( p ) )
>>+ p_physp = osm_port_get_default_phys_ptr( p_port );
>>+ if ( !osm_physp_is_valid( p_physp ) )
>> return FALSE;
>>- peer = osm_physp_get_remote( p );
>>+ peer = osm_physp_get_remote( p_physp );
>> if ( !peer || !osm_physp_is_valid( peer ) )
>> return FALSE;
>> p_node = osm_physp_get_node_ptr( peer );
>>@@ -245,7 +433,7 @@ pkey_mgr_update_peer_port(
>> if (pkey_mgr_enforce_partition( p_req, peer, enforce ) != IB_SUCCESS)
>> {
>> osm_log( p_log, OSM_LOG_ERROR,
>>- "pkey_mgr_update_peer_port: ERR 0502: "
>>+ "pkey_mgr_update_peer_port: ERR 0507: "
>> "pkey_mgr_enforce_partition() failed to update "
>> "node 0x%016" PRIx64 " port %u\n",
>> cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>@@ -255,24 +443,36 @@ pkey_mgr_update_peer_port(
>> if (enforce == FALSE)
>> return FALSE;
>>
>>- p_pkey_tbl = osm_physp_get_pkey_tbl( p );
>>- p_peer_pkey_tbl = osm_physp_get_pkey_tbl( peer );
>>+ p_pkey_tbl = osm_physp_get_pkey_tbl( p_physp );
>>+ p_peer_pkey_tbl = osm_physp_get_mod_pkey_tbl( peer );
>> num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl );
>>- if ( num_of_blocks > osm_pkey_tbl_get_num_blocks( p_peer_pkey_tbl ) )
>>- num_of_blocks = osm_pkey_tbl_get_num_blocks( p_peer_pkey_tbl );
>>+ peer_max_blocks = pkey_mgr_get_physp_max_blocks( p_subn, peer );
>>+ if (peer_max_blocks < p_pkey_tbl->used_blocks)
>>+ {
>>+ osm_log( p_log, OSM_LOG_ERROR,
>>+ "pkey_mgr_update_peer_port: ERR 0508: "
>>+ "not enough entries (%u < %u) on switch 0x%016" PRIx64
>>+ " port %u\n",
>>+ peer_max_blocks, num_of_blocks,
>>+ cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>+ osm_physp_get_port_num( peer ) );
>>+ return FALSE;
>
>
> Do you think it is the best way, just to skip update - partitions are
> enforced already on the switch. May be better to truncate pkey tables
> in order to meet peer's capabilities?
You are right about that - Its a bug!
I think the best approach here is to turn off the enforcement on the switch.
If we truncate the table we actually impact connectivity of the fabric.
I prefer a softer approach - an error in the log.
>
>
>>+ }
>>
>>- for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>>+ p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
>>+ for ( block_index = 0; block_index < p_pkey_tbl->used_blocks; block_index++)
>> {
>> block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index );
>> peer_block = osm_pkey_tbl_block_get( p_peer_pkey_tbl, block_index );
>> if ( memcmp( peer_block, block, sizeof( *peer_block ) ) )
>> {
>>+ osm_pkey_tbl_set(p_peer_pkey_tbl, block_index, block);
>
>
> Why this (osm_pkey_tbl_set())? This will be called by receiver.
Same as the above note about updating the map
I wanted to avoid to wait for the GetResp.
I think it is a mistake and we can actually remove it.
>
>
>> status = pkey_mgr_update_pkey_entry( p_req, peer, block, block_index );
>> if ( status == IB_SUCCESS )
>> ret_val = TRUE;
>> else
>> osm_log( p_log, OSM_LOG_ERROR,
>>- "pkey_mgr_update_peer_port: ERR 0503: "
>>+ "pkey_mgr_update_peer_port: ERR 0509: "
>> "pkey_mgr_update_pkey_entry() failed to update "
>> "pkey table block %d for node 0x%016" PRIx64
>> " port %u\n",
>>@@ -282,10 +482,10 @@ pkey_mgr_update_peer_port(
>> }
>> }
>>
>>- if ( ret_val == TRUE &&
>>- osm_log_is_active( p_log, OSM_LOG_VERBOSE ) )
>>+ if ( (ret_val == TRUE) &&
>>+ osm_log_is_active( p_log, OSM_LOG_DEBUG ) )
>> {
>>- osm_log( p_log, OSM_LOG_VERBOSE,
>>+ osm_log( p_log, OSM_LOG_DEBUG,
>> "pkey_mgr_update_peer_port: "
>> "pkey table was updated for node 0x%016" PRIx64
>> " port %u\n",
>>@@ -298,82 +498,6 @@ pkey_mgr_update_peer_port(
>>
>> /**********************************************************************
>> **********************************************************************/
>>-static boolean_t pkey_mgr_update_port(
>>- osm_log_t *p_log,
>>- osm_req_t *p_req,
>>- const osm_port_t * const p_port )
>>-{
>>- osm_physp_t *p;
>>- osm_node_t *p_node;
>>- ib_pkey_table_t *block, *new_block;
>>- const osm_pkey_tbl_t *p_pkey_tbl;
>>- uint16_t block_index;
>>- uint16_t num_of_blocks;
>>- ib_api_status_t status;
>>- boolean_t ret_val = FALSE;
>>-
>>- p = osm_port_get_default_phys_ptr( p_port );
>>- if ( !osm_physp_is_valid( p ) )
>>- return FALSE;
>>-
>>- p_pkey_tbl = osm_physp_get_pkey_tbl(p);
>>- num_of_blocks = osm_pkey_tbl_get_num_blocks( p_pkey_tbl );
>>-
>>- for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>>- {
>>- block = osm_pkey_tbl_block_get( p_pkey_tbl, block_index );
>>- new_block = osm_pkey_tbl_new_block_get( p_pkey_tbl, block_index );
>>-
>>- if (!new_block || !memcmp( new_block, block, sizeof( *block ) ) )
>>- continue;
>>-
>>- status = pkey_mgr_update_pkey_entry( p_req, p, new_block, block_index );
>>- if (status == IB_SUCCESS)
>>- ret_val = TRUE;
>>- else
>>- osm_log( p_log, OSM_LOG_ERROR,
>>- "pkey_mgr_update_port: ERR 0504: "
>>- "pkey_mgr_update_pkey_entry() failed to update "
>>- "pkey table block %d for node 0x%016" PRIx64 " port %u\n",
>>- block_index,
>>- cl_ntoh64( osm_node_get_node_guid( p_node ) ),
>>- osm_physp_get_port_num( p ) );
>>- }
>>-
>>- return ret_val;
>>-}
>>-
>>-/**********************************************************************
>>- **********************************************************************/
>>-static void
>>-pkey_mgr_process_partition_table(
>>- osm_log_t *p_log,
>>- const osm_req_t *p_req,
>>- const osm_prtn_t *p_prtn,
>>- const boolean_t full )
>>-{
>>- const cl_map_t *p_tbl = full ?
>>- &p_prtn->full_guid_tbl : &p_prtn->part_guid_tbl;
>>- cl_map_iterator_t i, i_next;
>>- ib_net16_t pkey = p_prtn->pkey;
>>- osm_physp_t *p_physp;
>>-
>>- if ( full )
>>- pkey = cl_hton16( cl_ntoh16( pkey ) | 0x8000 );
>>-
>>- i_next = cl_map_head( p_tbl );
>>- while ( i_next != cl_map_end( p_tbl ) )
>>- {
>>- i = i_next;
>>- i_next = cl_map_next( i );
>>- p_physp = cl_map_obj( i );
>>- if ( p_physp && osm_physp_is_valid( p_physp ) )
>>- pkey_mgr_process_physical_port( p_log, p_req, pkey, p_physp );
>>- }
>>-}
>>-
>>-/**********************************************************************
>>- **********************************************************************/
>> osm_signal_t
>> osm_pkey_mgr_process(
>> IN osm_opensm_t *p_osm )
>>@@ -383,8 +507,7 @@ osm_pkey_mgr_process(
>> osm_prtn_t *p_prtn;
>> osm_port_t *p_port;
>> osm_signal_t signal = OSM_SIGNAL_DONE;
>>- osm_physp_t *p_physp;
>>-
>>+ osm_node_t *p_node;
>> CL_ASSERT( p_osm );
>>
>> OSM_LOG_ENTER( &p_osm->log, osm_pkey_mgr_process );
>>@@ -394,32 +517,25 @@ osm_pkey_mgr_process(
>> if ( osm_prtn_make_partitions( &p_osm->log, &p_osm->subn ) != IB_SUCCESS )
>> {
>> osm_log( &p_osm->log, OSM_LOG_ERROR,
>>- "osm_pkey_mgr_process: ERR 0505: "
>>+ "osm_pkey_mgr_process: ERR 0510: "
>> "osm_prtn_make_partitions() failed\n" );
>> goto _err;
>> }
>>
>>- p_tbl = &p_osm->subn.port_guid_tbl;
>>- p_next = cl_qmap_head( p_tbl );
>>- while ( p_next != cl_qmap_end( p_tbl ) )
>>- {
>>- p_port = ( osm_port_t * ) p_next;
>>- p_next = cl_qmap_next( p_next );
>>- p_physp = osm_port_get_default_phys_ptr( p_port );
>>- if ( osm_physp_is_valid( p_physp ) )
>>- osm_pkey_tbl_sync_new_blocks( osm_physp_get_pkey_tbl( p_physp ) );
>>- }
>>-
>>+ /* populate the pending pkey entries by scanning all partitions */
>> p_tbl = &p_osm->subn.prtn_pkey_tbl;
>> p_next = cl_qmap_head( p_tbl );
>> while ( p_next != cl_qmap_end( p_tbl ) )
>> {
>> p_prtn = ( osm_prtn_t * ) p_next;
>> p_next = cl_qmap_next( p_next );
>>- pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, FALSE );
>>- pkey_mgr_process_partition_table( &p_osm->log, &p_osm->sm.req, p_prtn, TRUE );
>>+ pkey_mgr_process_partition_table(
>>+ &p_osm->log, &p_osm->sm.req, p_prtn, FALSE );
>>+ pkey_mgr_process_partition_table(
>>+ &p_osm->log, &p_osm->sm.req, p_prtn, TRUE );
>> }
>>
>>+ /* calculate new pkey tables and set */
>> p_tbl = &p_osm->subn.port_guid_tbl;
>> p_next = cl_qmap_head( p_tbl );
>> while ( p_next != cl_qmap_end( p_tbl ) )
>>@@ -428,8 +544,10 @@ osm_pkey_mgr_process(
>> p_next = cl_qmap_next( p_next );
>> if ( pkey_mgr_update_port( &p_osm->log, &p_osm->sm.req, p_port ) )
>> signal = OSM_SIGNAL_DONE_PENDING;
>>- if ( osm_node_get_type( osm_port_get_parent_node( p_port ) ) != IB_NODE_TYPE_SWITCH &&
>>- pkey_mgr_update_peer_port( &p_osm->log, &p_osm->sm.req,
>>+ p_node = osm_port_get_parent_node( p_port );
>>+ if ( ( osm_node_get_type( p_node ) != IB_NODE_TYPE_SWITCH ) &&
>>+ pkey_mgr_update_peer_port(
>>+ &p_osm->log, &p_osm->sm.req,
>> &p_osm->subn, p_port,
>> !p_osm->subn.opt.no_partition_enforcement ) )
>> signal = OSM_SIGNAL_DONE_PENDING;
>>
>>
>
>
> Thanks,
> Sasha
More information about the general
mailing list