[openib-general] [PATCHv3] osm: partition manager force policy

[Eitan Zahavi]

Hi Sasha,

Thanks!
These two are real bugs.
I am sending PATCHv4...

Sasha Khapyorsky wrote:
> Hi Eitan,
> 
> On 14:46 Sun 18 Jun     , Eitan Zahavi wrote:
> 
>>This is a third take after incorporating Sasha's comments to the 
>>partition manager patch I have previously provided. 
> 
> 
> Two small comments below.
> 
> 
>> /**********************************************************************
>>  **********************************************************************/
>>-/*
>>- * 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;
> 
> 
> p_node is uninitialized and used in osm_log() later,
Thanks. I wonder how I missed this one.
> 
> 
>>+	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;
>>+	uint8_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 ) );				
>>+	}
> 
> 
> 
>>@@ -255,13 +450,8 @@ 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 );
>>-   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 );
>>-
>>-   for ( block_index = 0; block_index < num_of_blocks; block_index++ )
>>+	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
> 
> 
> Peer's pkey table blocks may be not initialized yet, and then
> 
> 
>>+	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 );
> 
> 
> peer_block can be NULL.
> 
> Later in the code (not in this patch) there is
> 'if (memcmp(peer_block, ...))', should be changed to
> 'if (!peer_block || memcmp(peer_block, ...))'.
> 
> 
> Sasha
>