diff mbox

[1/2] opensm/osm_pkey_mgr.c: Pack switch peer port PKey tables

Message ID 4DA358F8.6070203@dev.mellanox.co.il (mailing list archive)
State Accepted
Delegated to: Alex Netes
Headers show

Commit Message

Hal Rosenstock April 11, 2011, 7:39 p.m. UTC
In pkey_mgr_update_peer_port, rather than mirror the end port
PKey table, pack the peer switch's port PKey table to
eliminate any holes.

Signed-off-by: Hal Rosenstock <hal@mellanox.com>
---
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Alex Netes May 2, 2011, 12:12 p.m. UTC | #1
Hi Hal,


On 15:39 Mon 11 Apr     , Hal Rosenstock wrote:
> 
> In pkey_mgr_update_peer_port, rather than mirror the end port
> PKey table, pack the peer switch's port PKey table to
> eliminate any holes.
> 

Why don't you want to pack port's pkey table itself and mirror it packed to
the peer's port?

> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
> ---
> diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
> index f6bc9d1..06d9b1e 100644
> --- a/opensm/osm_pkey_mgr.c
> +++ b/opensm/osm_pkey_mgr.c
> @@ -398,6 +398,35 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
>  	return index;
>  }
>  
> +static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,
> +			     osm_physp_t * peer,
> +			     osm_pkey_tbl_t * p_peer_pkey_tbl,
> +			     ib_pkey_table_t * new_peer_block,
> +			     uint16_t peer_block_idx, osm_node_t * p_node)
> +{
> +	int ret = 0;
> +	ib_pkey_table_t *peer_block;
> +
> +	peer_block = osm_pkey_tbl_block_get(p_peer_pkey_tbl, peer_block_idx);
> +	if (!peer_block ||
> +	    memcmp(peer_block, new_peer_block, sizeof(*peer_block))) {
> +		if (pkey_mgr_update_pkey_entry(sm, peer, new_peer_block,
> +					       peer_block_idx) != IB_SUCCESS) {
> +			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
> +				"pkey_mgr_update_pkey_entry() failed to update "
> +				"pkey table block %d for node 0x%016"
> +				PRIx64 " port %u (%s)\n",
> +				peer_block_idx,
> +				cl_ntoh64(osm_node_get_node_guid(p_node)),
> +				osm_physp_get_port_num(peer),
> +				p_node->print_desc);
> +			ret = -1;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
>  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>  				     const osm_subn_t * p_subn,
>  				     const osm_port_t * const p_port,
> @@ -405,15 +434,16 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>  {
>  	osm_physp_t *p_physp, *peer;
>  	osm_node_t *p_node;
> -	ib_pkey_table_t *block, *peer_block;
> +	ib_pkey_table_t *block;
>  	const osm_pkey_tbl_t *p_pkey_tbl;
>  	osm_pkey_tbl_t *p_peer_pkey_tbl;
> -	uint16_t block_index;
> +	uint16_t block_index, peer_block_idx;
>  	uint16_t peer_max_blocks;
>  	uint16_t last_index;
> -	ib_api_status_t status = IB_SUCCESS;
> -	ib_pkey_table_t empty_block;
> -	int ret = 0;
> +	ib_pkey_table_t new_peer_block;
> +	uint16_t pkey_idx, peer_pkey_idx;
> +	ib_net16_t pkey;
> +	int ret = 0, loop_exit = 0;
>  
>  	p_physp = p_port->p_physp;
>  	if (!p_physp)
> @@ -425,70 +455,80 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>  	if (!p_node->sw || !p_node->sw->switch_info.enforce_cap)
>  		return 0;
>  
> -	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
> -	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
> -	if (peer_max_blocks < p_pkey_tbl->used_blocks) {
> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0508: "
> -			"Not enough pkey blocks (%u < %u used) on switch 0x%016"
> -			PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> -			peer_max_blocks, p_pkey_tbl->used_blocks,
> -			cl_ntoh64(osm_node_get_node_guid(p_node)),
> -			osm_physp_get_port_num(peer),
> -			p_node->print_desc);
> -		enforce = FALSE;
> -		ret = -1;
> -	} else if (peer_max_blocks == p_pkey_tbl->used_blocks) {
> -		/* Is last used pkey index beyond switch peer port capacity ? */
> -		last_index = (peer_max_blocks - 1) * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
> -			     last_used_pkey_index(p_port, p_pkey_tbl);
> -		if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
> -			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
> -				"Not enough pkey entries (%u <= %u) on switch 0x%016"
> -				PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> -				cl_ntoh16(p_node->sw->switch_info.enforce_cap),
> -				last_index,
> -				cl_ntoh64(osm_node_get_node_guid(p_node)),
> -				osm_physp_get_port_num(peer),
> -				p_node->print_desc);
> -			enforce = FALSE;
> -			ret = -1;
> -		}
> +	if (enforce == FALSE) {
> +		pkey_mgr_enforce_partition(p_log, sm, peer, FALSE);
> +		return -1;

Though it doesn't affect anything, why do you return -1 here?

>  	}
>  
> -	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
> -		ret = -1;
> -
> -	if (enforce == FALSE)
> -		return ret;
> -
> -	memset(&empty_block, 0, sizeof(ib_pkey_table_t));
> -
> +	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
> +	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
>  	p_peer_pkey_tbl = &peer->pkeys;
> -	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
> +	peer_block_idx = 0;
> +	peer_pkey_idx = 0;
>  	for (block_index = 0; block_index < p_pkey_tbl->used_blocks;
>  	     block_index++) {
> +		if (loop_exit)
> +			break;
>  		block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
>  		if (!block)
> -			block = &empty_block;
> -
> -		peer_block =
> -		    osm_pkey_tbl_block_get(p_peer_pkey_tbl, block_index);
> -		if (!peer_block
> -		    || memcmp(peer_block, block, sizeof(*peer_block))) {
> -			status = pkey_mgr_update_pkey_entry(sm, peer, block,
> -							    block_index);
> -			if (status != IB_SUCCESS) {
> -				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
> -					"pkey_mgr_update_pkey_entry() failed to update "
> -					"pkey table block %d for node 0x%016"
> -					PRIx64 " port %u (%s)\n", block_index,
> -					cl_ntoh64(osm_node_get_node_guid
> -						  (p_node)),
> +			continue;
> +		for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
> +		     pkey_idx++) {
> +			pkey = block->pkey_entry[pkey_idx];
> +			if (ib_pkey_is_invalid(pkey))
> +				continue;
> +			new_peer_block.pkey_entry[peer_pkey_idx] = pkey;
> +			if (peer_block_idx >= peer_max_blocks) {
> +				loop_exit = 1;
> +				break;
> +			}
> +			if (++peer_pkey_idx == IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
> +				if (update_peer_block(p_log, sm, peer,
> +						      p_peer_pkey_tbl,
> +						      &new_peer_block,
> +						      peer_block_idx, p_node))
> +					ret = -1;
> +				peer_pkey_idx = 0;
> +				peer_block_idx++;
> +			}
> +		}
> +	}
> +
> +	if (peer_block_idx < peer_max_blocks) {
> +		if (peer_pkey_idx) {
> +			/* Handle partial last block */
> +			for (; peer_pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
> +			     peer_pkey_idx++)
> +				new_peer_block.pkey_entry[peer_pkey_idx] = 0;
> +			if (update_peer_block(p_log, sm, peer, p_peer_pkey_tbl,
> +					      &new_peer_block, peer_block_idx,
> +					      p_node))
> +				ret = -1;
> +		} else
> +			peer_block_idx--;
> +
> +		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
> +		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
> +			/* Is last used pkey index beyond switch peer port capacity ? */
> +			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
> +				     last_used_pkey_index(p_port,
> +							  p_peer_pkey_tbl);
> +			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
> +				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
> +					"Not enough pkey entries (%u <= %u) on switch 0x%016"
> +					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> +					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
> +					last_index,
> +					cl_ntoh64(osm_node_get_node_guid(p_node)),
>  					osm_physp_get_port_num(peer),
>  					p_node->print_desc);
> +				enforce = FALSE;
>  				ret = -1;
>  			}
>  		}
> +	} else {
> +		p_peer_pkey_tbl->used_blocks = peer_max_blocks;
> +		enforce = FALSE;
>  	}
>  
>  	if (!ret)

Only if ret = 0,  "Pkey table was updated for node..." message is printed,
however Pkey table might be changed even if ret = -1.

> @@ -498,6 +538,9 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>  			cl_ntoh64(osm_node_get_node_guid(p_node)),
>  			osm_physp_get_port_num(peer), p_node->print_desc);
>  
> +	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
> +		ret = -1;
> +
>  	return ret;
>  }
>  
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hal Rosenstock May 2, 2011, 11:45 p.m. UTC | #2
Hi Alex,

On 5/2/2011 8:12 AM, Alex Netes wrote:
> Hi Hal,
> 
> 
> On 15:39 Mon 11 Apr     , Hal Rosenstock wrote:
>>
>> In pkey_mgr_update_peer_port, rather than mirror the end port
>> PKey table, pack the peer switch's port PKey table to
>> eliminate any holes.
>>
> 
> Why don't you want to pack port's pkey table itself and mirror it packed to
> the peer's port?

It's disruptive to change end port pkey indices whereas that's not the
case on switch external ports and it has benefit there.

> 
>> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
>> ---
>> diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
>> index f6bc9d1..06d9b1e 100644
>> --- a/opensm/osm_pkey_mgr.c
>> +++ b/opensm/osm_pkey_mgr.c
>> @@ -398,6 +398,35 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
>>  	return index;
>>  }
>>  
>> +static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,
>> +			     osm_physp_t * peer,
>> +			     osm_pkey_tbl_t * p_peer_pkey_tbl,
>> +			     ib_pkey_table_t * new_peer_block,
>> +			     uint16_t peer_block_idx, osm_node_t * p_node)
>> +{
>> +	int ret = 0;
>> +	ib_pkey_table_t *peer_block;
>> +
>> +	peer_block = osm_pkey_tbl_block_get(p_peer_pkey_tbl, peer_block_idx);
>> +	if (!peer_block ||
>> +	    memcmp(peer_block, new_peer_block, sizeof(*peer_block))) {
>> +		if (pkey_mgr_update_pkey_entry(sm, peer, new_peer_block,
>> +					       peer_block_idx) != IB_SUCCESS) {
>> +			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
>> +				"pkey_mgr_update_pkey_entry() failed to update "
>> +				"pkey table block %d for node 0x%016"
>> +				PRIx64 " port %u (%s)\n",
>> +				peer_block_idx,
>> +				cl_ntoh64(osm_node_get_node_guid(p_node)),
>> +				osm_physp_get_port_num(peer),
>> +				p_node->print_desc);
>> +			ret = -1;
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>>  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>  				     const osm_subn_t * p_subn,
>>  				     const osm_port_t * const p_port,
>> @@ -405,15 +434,16 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>  {
>>  	osm_physp_t *p_physp, *peer;
>>  	osm_node_t *p_node;
>> -	ib_pkey_table_t *block, *peer_block;
>> +	ib_pkey_table_t *block;
>>  	const osm_pkey_tbl_t *p_pkey_tbl;
>>  	osm_pkey_tbl_t *p_peer_pkey_tbl;
>> -	uint16_t block_index;
>> +	uint16_t block_index, peer_block_idx;
>>  	uint16_t peer_max_blocks;
>>  	uint16_t last_index;
>> -	ib_api_status_t status = IB_SUCCESS;
>> -	ib_pkey_table_t empty_block;
>> -	int ret = 0;
>> +	ib_pkey_table_t new_peer_block;
>> +	uint16_t pkey_idx, peer_pkey_idx;
>> +	ib_net16_t pkey;
>> +	int ret = 0, loop_exit = 0;
>>  
>>  	p_physp = p_port->p_physp;
>>  	if (!p_physp)
>> @@ -425,70 +455,80 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>  	if (!p_node->sw || !p_node->sw->switch_info.enforce_cap)
>>  		return 0;
>>  
>> -	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
>> -	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
>> -	if (peer_max_blocks < p_pkey_tbl->used_blocks) {
>> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0508: "
>> -			"Not enough pkey blocks (%u < %u used) on switch 0x%016"
>> -			PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>> -			peer_max_blocks, p_pkey_tbl->used_blocks,
>> -			cl_ntoh64(osm_node_get_node_guid(p_node)),
>> -			osm_physp_get_port_num(peer),
>> -			p_node->print_desc);
>> -		enforce = FALSE;
>> -		ret = -1;
>> -	} else if (peer_max_blocks == p_pkey_tbl->used_blocks) {
>> -		/* Is last used pkey index beyond switch peer port capacity ? */
>> -		last_index = (peer_max_blocks - 1) * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
>> -			     last_used_pkey_index(p_port, p_pkey_tbl);
>> -		if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
>> -			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
>> -				"Not enough pkey entries (%u <= %u) on switch 0x%016"
>> -				PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>> -				cl_ntoh16(p_node->sw->switch_info.enforce_cap),
>> -				last_index,
>> -				cl_ntoh64(osm_node_get_node_guid(p_node)),
>> -				osm_physp_get_port_num(peer),
>> -				p_node->print_desc);
>> -			enforce = FALSE;
>> -			ret = -1;
>> -		}
>> +	if (enforce == FALSE) {
>> +		pkey_mgr_enforce_partition(p_log, sm, peer, FALSE);
>> +		return -1;
> 
> Though it doesn't affect anything, why do you return -1 here?

I don't recall and it doesn't look right. It should probably just be
return ret there.

>>  	}
>>  
>> -	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
>> -		ret = -1;
>> -
>> -	if (enforce == FALSE)
>> -		return ret;
>> -
>> -	memset(&empty_block, 0, sizeof(ib_pkey_table_t));
>> -
>> +	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
>> +	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
>>  	p_peer_pkey_tbl = &peer->pkeys;
>> -	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
>> +	peer_block_idx = 0;
>> +	peer_pkey_idx = 0;
>>  	for (block_index = 0; block_index < p_pkey_tbl->used_blocks;
>>  	     block_index++) {
>> +		if (loop_exit)
>> +			break;
>>  		block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
>>  		if (!block)
>> -			block = &empty_block;
>> -
>> -		peer_block =
>> -		    osm_pkey_tbl_block_get(p_peer_pkey_tbl, block_index);
>> -		if (!peer_block
>> -		    || memcmp(peer_block, block, sizeof(*peer_block))) {
>> -			status = pkey_mgr_update_pkey_entry(sm, peer, block,
>> -							    block_index);
>> -			if (status != IB_SUCCESS) {
>> -				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
>> -					"pkey_mgr_update_pkey_entry() failed to update "
>> -					"pkey table block %d for node 0x%016"
>> -					PRIx64 " port %u (%s)\n", block_index,
>> -					cl_ntoh64(osm_node_get_node_guid
>> -						  (p_node)),
>> +			continue;
>> +		for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
>> +		     pkey_idx++) {
>> +			pkey = block->pkey_entry[pkey_idx];
>> +			if (ib_pkey_is_invalid(pkey))
>> +				continue;
>> +			new_peer_block.pkey_entry[peer_pkey_idx] = pkey;
>> +			if (peer_block_idx >= peer_max_blocks) {
>> +				loop_exit = 1;
>> +				break;
>> +			}
>> +			if (++peer_pkey_idx == IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
>> +				if (update_peer_block(p_log, sm, peer,
>> +						      p_peer_pkey_tbl,
>> +						      &new_peer_block,
>> +						      peer_block_idx, p_node))
>> +					ret = -1;
>> +				peer_pkey_idx = 0;
>> +				peer_block_idx++;
>> +			}
>> +		}
>> +	}
>> +
>> +	if (peer_block_idx < peer_max_blocks) {
>> +		if (peer_pkey_idx) {
>> +			/* Handle partial last block */
>> +			for (; peer_pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
>> +			     peer_pkey_idx++)
>> +				new_peer_block.pkey_entry[peer_pkey_idx] = 0;
>> +			if (update_peer_block(p_log, sm, peer, p_peer_pkey_tbl,
>> +					      &new_peer_block, peer_block_idx,
>> +					      p_node))
>> +				ret = -1;
>> +		} else
>> +			peer_block_idx--;
>> +
>> +		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
>> +		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
>> +			/* Is last used pkey index beyond switch peer port capacity ? */
>> +			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
>> +				     last_used_pkey_index(p_port,
>> +							  p_peer_pkey_tbl);
>> +			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
>> +				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
>> +					"Not enough pkey entries (%u <= %u) on switch 0x%016"
>> +					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>> +					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
>> +					last_index,
>> +					cl_ntoh64(osm_node_get_node_guid(p_node)),
>>  					osm_physp_get_port_num(peer),
>>  					p_node->print_desc);
>> +				enforce = FALSE;
>>  				ret = -1;
>>  			}
>>  		}
>> +	} else {
>> +		p_peer_pkey_tbl->used_blocks = peer_max_blocks;
>> +		enforce = FALSE;
>>  	}
>>  
>>  	if (!ret)
> 
> Only if ret = 0,  "Pkey table was updated for node..." message is printed,
> however Pkey table might be changed even if ret = -1.

This is related to your comment above, right ? Or are there additional
places where pkey table is updated and ret is -1 ?

-- Hal

>> @@ -498,6 +538,9 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>  			cl_ntoh64(osm_node_get_node_guid(p_node)),
>>  			osm_physp_get_port_num(peer), p_node->print_desc);
>>  
>> +	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
>> +		ret = -1;
>> +
>>  	return ret;
>>  }
>>  

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alex Netes May 16, 2011, 2:56 p.m. UTC | #3
Hi Hal,

On 19:45 Mon 02 May     , Hal Rosenstock wrote:
> Hi Alex,
> 
> On 5/2/2011 8:12 AM, Alex Netes wrote:
> > Hi Hal,
> > 
> > 
> > On 15:39 Mon 11 Apr     , Hal Rosenstock wrote:
> >>
> >> In pkey_mgr_update_peer_port, rather than mirror the end port
> >> PKey table, pack the peer switch's port PKey table to
> >> eliminate any holes.
> >>
> > 
> > Why don't you want to pack port's pkey table itself and mirror it packed to
> > the peer's port?
> 
> It's disruptive to change end port pkey indices whereas that's not the
> case on switch external ports and it has benefit there.
> 
> > 
> >> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
> >> ---
> >> diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
> >> index f6bc9d1..06d9b1e 100644
> >> --- a/opensm/osm_pkey_mgr.c
> >> +++ b/opensm/osm_pkey_mgr.c
> >> @@ -398,6 +398,35 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
> >>  	return index;
> >>  }
> >>  
> >> +static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,
> >> +			     osm_physp_t * peer,
> >> +			     osm_pkey_tbl_t * p_peer_pkey_tbl,
> >> +			     ib_pkey_table_t * new_peer_block,
> >> +			     uint16_t peer_block_idx, osm_node_t * p_node)
> >> +{
> >> +	int ret = 0;
> >> +	ib_pkey_table_t *peer_block;
> >> +
> >> +	peer_block = osm_pkey_tbl_block_get(p_peer_pkey_tbl, peer_block_idx);
> >> +	if (!peer_block ||
> >> +	    memcmp(peer_block, new_peer_block, sizeof(*peer_block))) {
> >> +		if (pkey_mgr_update_pkey_entry(sm, peer, new_peer_block,
> >> +					       peer_block_idx) != IB_SUCCESS) {
> >> +			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
> >> +				"pkey_mgr_update_pkey_entry() failed to update "
> >> +				"pkey table block %d for node 0x%016"
> >> +				PRIx64 " port %u (%s)\n",
> >> +				peer_block_idx,
> >> +				cl_ntoh64(osm_node_get_node_guid(p_node)),
> >> +				osm_physp_get_port_num(peer),
> >> +				p_node->print_desc);
> >> +			ret = -1;
> >> +		}
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >>  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
> >>  				     const osm_subn_t * p_subn,
> >>  				     const osm_port_t * const p_port,
> >> @@ -405,15 +434,16 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
> >>  {
> >>  	osm_physp_t *p_physp, *peer;
> >>  	osm_node_t *p_node;
> >> -	ib_pkey_table_t *block, *peer_block;
> >> +	ib_pkey_table_t *block;
> >>  	const osm_pkey_tbl_t *p_pkey_tbl;
> >>  	osm_pkey_tbl_t *p_peer_pkey_tbl;
> >> -	uint16_t block_index;
> >> +	uint16_t block_index, peer_block_idx;
> >>  	uint16_t peer_max_blocks;
> >>  	uint16_t last_index;
> >> -	ib_api_status_t status = IB_SUCCESS;
> >> -	ib_pkey_table_t empty_block;
> >> -	int ret = 0;
> >> +	ib_pkey_table_t new_peer_block;
> >> +	uint16_t pkey_idx, peer_pkey_idx;
> >> +	ib_net16_t pkey;
> >> +	int ret = 0, loop_exit = 0;
> >>  
> >>  	p_physp = p_port->p_physp;
> >>  	if (!p_physp)
> >> @@ -425,70 +455,80 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
> >>  	if (!p_node->sw || !p_node->sw->switch_info.enforce_cap)
> >>  		return 0;
> >>  
> >> -	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
> >> -	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
> >> -	if (peer_max_blocks < p_pkey_tbl->used_blocks) {
> >> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0508: "
> >> -			"Not enough pkey blocks (%u < %u used) on switch 0x%016"
> >> -			PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> >> -			peer_max_blocks, p_pkey_tbl->used_blocks,
> >> -			cl_ntoh64(osm_node_get_node_guid(p_node)),
> >> -			osm_physp_get_port_num(peer),
> >> -			p_node->print_desc);
> >> -		enforce = FALSE;
> >> -		ret = -1;
> >> -	} else if (peer_max_blocks == p_pkey_tbl->used_blocks) {
> >> -		/* Is last used pkey index beyond switch peer port capacity ? */
> >> -		last_index = (peer_max_blocks - 1) * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
> >> -			     last_used_pkey_index(p_port, p_pkey_tbl);
> >> -		if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
> >> -			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
> >> -				"Not enough pkey entries (%u <= %u) on switch 0x%016"
> >> -				PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> >> -				cl_ntoh16(p_node->sw->switch_info.enforce_cap),
> >> -				last_index,
> >> -				cl_ntoh64(osm_node_get_node_guid(p_node)),
> >> -				osm_physp_get_port_num(peer),
> >> -				p_node->print_desc);
> >> -			enforce = FALSE;
> >> -			ret = -1;
> >> -		}
> >> +	if (enforce == FALSE) {
> >> +		pkey_mgr_enforce_partition(p_log, sm, peer, FALSE);
> >> +		return -1;
> > 
> > Though it doesn't affect anything, why do you return -1 here?
> 
> I don't recall and it doesn't look right. It should probably just be
> return ret there.
> 
> >>  	}
> >>  
> >> -	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
> >> -		ret = -1;
> >> -
> >> -	if (enforce == FALSE)
> >> -		return ret;
> >> -
> >> -	memset(&empty_block, 0, sizeof(ib_pkey_table_t));
> >> -
> >> +	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
> >> +	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
> >>  	p_peer_pkey_tbl = &peer->pkeys;
> >> -	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
> >> +	peer_block_idx = 0;
> >> +	peer_pkey_idx = 0;
> >>  	for (block_index = 0; block_index < p_pkey_tbl->used_blocks;
> >>  	     block_index++) {
> >> +		if (loop_exit)
> >> +			break;
> >>  		block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
> >>  		if (!block)
> >> -			block = &empty_block;
> >> -
> >> -		peer_block =
> >> -		    osm_pkey_tbl_block_get(p_peer_pkey_tbl, block_index);
> >> -		if (!peer_block
> >> -		    || memcmp(peer_block, block, sizeof(*peer_block))) {
> >> -			status = pkey_mgr_update_pkey_entry(sm, peer, block,
> >> -							    block_index);
> >> -			if (status != IB_SUCCESS) {
> >> -				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
> >> -					"pkey_mgr_update_pkey_entry() failed to update "
> >> -					"pkey table block %d for node 0x%016"
> >> -					PRIx64 " port %u (%s)\n", block_index,
> >> -					cl_ntoh64(osm_node_get_node_guid
> >> -						  (p_node)),
> >> +			continue;
> >> +		for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
> >> +		     pkey_idx++) {
> >> +			pkey = block->pkey_entry[pkey_idx];
> >> +			if (ib_pkey_is_invalid(pkey))
> >> +				continue;
> >> +			new_peer_block.pkey_entry[peer_pkey_idx] = pkey;
> >> +			if (peer_block_idx >= peer_max_blocks) {
> >> +				loop_exit = 1;
> >> +				break;
> >> +			}
> >> +			if (++peer_pkey_idx == IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
> >> +				if (update_peer_block(p_log, sm, peer,
> >> +						      p_peer_pkey_tbl,
> >> +						      &new_peer_block,
> >> +						      peer_block_idx, p_node))
> >> +					ret = -1;
> >> +				peer_pkey_idx = 0;
> >> +				peer_block_idx++;
> >> +			}
> >> +		}
> >> +	}
> >> +
> >> +	if (peer_block_idx < peer_max_blocks) {
> >> +		if (peer_pkey_idx) {
> >> +			/* Handle partial last block */
> >> +			for (; peer_pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
> >> +			     peer_pkey_idx++)
> >> +				new_peer_block.pkey_entry[peer_pkey_idx] = 0;
> >> +			if (update_peer_block(p_log, sm, peer, p_peer_pkey_tbl,
> >> +					      &new_peer_block, peer_block_idx,
> >> +					      p_node))
> >> +				ret = -1;
> >> +		} else
> >> +			peer_block_idx--;
> >> +
> >> +		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
> >> +		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
> >> +			/* Is last used pkey index beyond switch peer port capacity ? */
> >> +			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
> >> +				     last_used_pkey_index(p_port,
> >> +							  p_peer_pkey_tbl);
> >> +			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
> >> +				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
> >> +					"Not enough pkey entries (%u <= %u) on switch 0x%016"
> >> +					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
> >> +					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
> >> +					last_index,
> >> +					cl_ntoh64(osm_node_get_node_guid(p_node)),
> >>  					osm_physp_get_port_num(peer),
> >>  					p_node->print_desc);
> >> +				enforce = FALSE;
> >>  				ret = -1;
> >>  			}
> >>  		}
> >> +	} else {
> >> +		p_peer_pkey_tbl->used_blocks = peer_max_blocks;
> >> +		enforce = FALSE;
> >>  	}
> >>  
> >>  	if (!ret)
> > 
> > Only if ret = 0,  "Pkey table was updated for node..." message is printed,
> > however Pkey table might be changed even if ret = -1.
> 
> This is related to your comment above, right ? Or are there additional
> places where pkey table is updated and ret is -1 ?
> 

All the places.

> -- Hal
> 
> >> @@ -498,6 +538,9 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
> >>  			cl_ntoh64(osm_node_get_node_guid(p_node)),
> >>  			osm_physp_get_port_num(peer), p_node->print_desc);
> >>  
> >> +	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
> >> +		ret = -1;
> >> +
> >>  	return ret;
> >>  }
> >>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hal Rosenstock May 20, 2011, 6:08 p.m. UTC | #4
Hi Alex,

On 5/16/2011 10:56 AM, Alex Netes wrote:
> Hi Hal,
> 
> On 19:45 Mon 02 May     , Hal Rosenstock wrote:
>> Hi Alex,
>>
>> On 5/2/2011 8:12 AM, Alex Netes wrote:
>>> Hi Hal,
>>>
>>>
>>> On 15:39 Mon 11 Apr     , Hal Rosenstock wrote:
>>>>
>>>> In pkey_mgr_update_peer_port, rather than mirror the end port
>>>> PKey table, pack the peer switch's port PKey table to
>>>> eliminate any holes.
>>>>
>>>
>>> Why don't you want to pack port's pkey table itself and mirror it packed to
>>> the peer's port?
>>
>> It's disruptive to change end port pkey indices whereas that's not the
>> case on switch external ports and it has benefit there.

I added text to this effect in the patch description in v2 of this patch.

>>>
>>>> Signed-off-by: Hal Rosenstock <hal@mellanox.com>
>>>> ---
>>>> diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
>>>> index f6bc9d1..06d9b1e 100644
>>>> --- a/opensm/osm_pkey_mgr.c
>>>> +++ b/opensm/osm_pkey_mgr.c
>>>> @@ -398,6 +398,35 @@ static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
>>>>  	return index;
>>>>  }
>>>>  
>>>> +static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,
>>>> +			     osm_physp_t * peer,
>>>> +			     osm_pkey_tbl_t * p_peer_pkey_tbl,
>>>> +			     ib_pkey_table_t * new_peer_block,
>>>> +			     uint16_t peer_block_idx, osm_node_t * p_node)
>>>> +{
>>>> +	int ret = 0;
>>>> +	ib_pkey_table_t *peer_block;
>>>> +
>>>> +	peer_block = osm_pkey_tbl_block_get(p_peer_pkey_tbl, peer_block_idx);
>>>> +	if (!peer_block ||
>>>> +	    memcmp(peer_block, new_peer_block, sizeof(*peer_block))) {
>>>> +		if (pkey_mgr_update_pkey_entry(sm, peer, new_peer_block,
>>>> +					       peer_block_idx) != IB_SUCCESS) {
>>>> +			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
>>>> +				"pkey_mgr_update_pkey_entry() failed to update "
>>>> +				"pkey table block %d for node 0x%016"
>>>> +				PRIx64 " port %u (%s)\n",
>>>> +				peer_block_idx,
>>>> +				cl_ntoh64(osm_node_get_node_guid(p_node)),
>>>> +				osm_physp_get_port_num(peer),
>>>> +				p_node->print_desc);
>>>> +			ret = -1;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>>  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>>>  				     const osm_subn_t * p_subn,
>>>>  				     const osm_port_t * const p_port,
>>>> @@ -405,15 +434,16 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>>>  {
>>>>  	osm_physp_t *p_physp, *peer;
>>>>  	osm_node_t *p_node;
>>>> -	ib_pkey_table_t *block, *peer_block;
>>>> +	ib_pkey_table_t *block;
>>>>  	const osm_pkey_tbl_t *p_pkey_tbl;
>>>>  	osm_pkey_tbl_t *p_peer_pkey_tbl;
>>>> -	uint16_t block_index;
>>>> +	uint16_t block_index, peer_block_idx;
>>>>  	uint16_t peer_max_blocks;
>>>>  	uint16_t last_index;
>>>> -	ib_api_status_t status = IB_SUCCESS;
>>>> -	ib_pkey_table_t empty_block;
>>>> -	int ret = 0;
>>>> +	ib_pkey_table_t new_peer_block;
>>>> +	uint16_t pkey_idx, peer_pkey_idx;
>>>> +	ib_net16_t pkey;
>>>> +	int ret = 0, loop_exit = 0;
>>>>  
>>>>  	p_physp = p_port->p_physp;
>>>>  	if (!p_physp)
>>>> @@ -425,70 +455,80 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>>>  	if (!p_node->sw || !p_node->sw->switch_info.enforce_cap)
>>>>  		return 0;
>>>>  
>>>> -	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
>>>> -	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
>>>> -	if (peer_max_blocks < p_pkey_tbl->used_blocks) {
>>>> -		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0508: "
>>>> -			"Not enough pkey blocks (%u < %u used) on switch 0x%016"
>>>> -			PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>>>> -			peer_max_blocks, p_pkey_tbl->used_blocks,
>>>> -			cl_ntoh64(osm_node_get_node_guid(p_node)),
>>>> -			osm_physp_get_port_num(peer),
>>>> -			p_node->print_desc);
>>>> -		enforce = FALSE;
>>>> -		ret = -1;
>>>> -	} else if (peer_max_blocks == p_pkey_tbl->used_blocks) {
>>>> -		/* Is last used pkey index beyond switch peer port capacity ? */
>>>> -		last_index = (peer_max_blocks - 1) * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
>>>> -			     last_used_pkey_index(p_port, p_pkey_tbl);
>>>> -		if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
>>>> -			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
>>>> -				"Not enough pkey entries (%u <= %u) on switch 0x%016"
>>>> -				PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>>>> -				cl_ntoh16(p_node->sw->switch_info.enforce_cap),
>>>> -				last_index,
>>>> -				cl_ntoh64(osm_node_get_node_guid(p_node)),
>>>> -				osm_physp_get_port_num(peer),
>>>> -				p_node->print_desc);
>>>> -			enforce = FALSE;
>>>> -			ret = -1;
>>>> -		}
>>>> +	if (enforce == FALSE) {
>>>> +		pkey_mgr_enforce_partition(p_log, sm, peer, FALSE);
>>>> +		return -1;
>>>
>>> Though it doesn't affect anything, why do you return -1 here?
>>
>> I don't recall and it doesn't look right. It should probably just be
>> return ret there.

I changed this to return ret in v2 of this patch.

>>>>  	}
>>>>  
>>>> -	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
>>>> -		ret = -1;
>>>> -
>>>> -	if (enforce == FALSE)
>>>> -		return ret;
>>>> -
>>>> -	memset(&empty_block, 0, sizeof(ib_pkey_table_t));
>>>> -
>>>> +	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
>>>> +	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
>>>>  	p_peer_pkey_tbl = &peer->pkeys;
>>>> -	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
>>>> +	peer_block_idx = 0;
>>>> +	peer_pkey_idx = 0;
>>>>  	for (block_index = 0; block_index < p_pkey_tbl->used_blocks;
>>>>  	     block_index++) {
>>>> +		if (loop_exit)
>>>> +			break;
>>>>  		block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
>>>>  		if (!block)
>>>> -			block = &empty_block;
>>>> -
>>>> -		peer_block =
>>>> -		    osm_pkey_tbl_block_get(p_peer_pkey_tbl, block_index);
>>>> -		if (!peer_block
>>>> -		    || memcmp(peer_block, block, sizeof(*peer_block))) {
>>>> -			status = pkey_mgr_update_pkey_entry(sm, peer, block,
>>>> -							    block_index);
>>>> -			if (status != IB_SUCCESS) {
>>>> -				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
>>>> -					"pkey_mgr_update_pkey_entry() failed to update "
>>>> -					"pkey table block %d for node 0x%016"
>>>> -					PRIx64 " port %u (%s)\n", block_index,
>>>> -					cl_ntoh64(osm_node_get_node_guid
>>>> -						  (p_node)),
>>>> +			continue;
>>>> +		for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
>>>> +		     pkey_idx++) {
>>>> +			pkey = block->pkey_entry[pkey_idx];
>>>> +			if (ib_pkey_is_invalid(pkey))
>>>> +				continue;
>>>> +			new_peer_block.pkey_entry[peer_pkey_idx] = pkey;
>>>> +			if (peer_block_idx >= peer_max_blocks) {
>>>> +				loop_exit = 1;
>>>> +				break;
>>>> +			}
>>>> +			if (++peer_pkey_idx == IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
>>>> +				if (update_peer_block(p_log, sm, peer,
>>>> +						      p_peer_pkey_tbl,
>>>> +						      &new_peer_block,
>>>> +						      peer_block_idx, p_node))
>>>> +					ret = -1;
>>>> +				peer_pkey_idx = 0;
>>>> +				peer_block_idx++;
>>>> +			}
>>>> +		}
>>>> +	}
>>>> +
>>>> +	if (peer_block_idx < peer_max_blocks) {
>>>> +		if (peer_pkey_idx) {
>>>> +			/* Handle partial last block */
>>>> +			for (; peer_pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
>>>> +			     peer_pkey_idx++)
>>>> +				new_peer_block.pkey_entry[peer_pkey_idx] = 0;
>>>> +			if (update_peer_block(p_log, sm, peer, p_peer_pkey_tbl,
>>>> +					      &new_peer_block, peer_block_idx,
>>>> +					      p_node))
>>>> +				ret = -1;
>>>> +		} else
>>>> +			peer_block_idx--;
>>>> +
>>>> +		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
>>>> +		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
>>>> +			/* Is last used pkey index beyond switch peer port capacity ? */
>>>> +			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
>>>> +				     last_used_pkey_index(p_port,
>>>> +							  p_peer_pkey_tbl);
>>>> +			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
>>>> +				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
>>>> +					"Not enough pkey entries (%u <= %u) on switch 0x%016"
>>>> +					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
>>>> +					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
>>>> +					last_index,
>>>> +					cl_ntoh64(osm_node_get_node_guid(p_node)),
>>>>  					osm_physp_get_port_num(peer),
>>>>  					p_node->print_desc);
>>>> +				enforce = FALSE;
>>>>  				ret = -1;
>>>>  			}
>>>>  		}
>>>> +	} else {
>>>> +		p_peer_pkey_tbl->used_blocks = peer_max_blocks;
>>>> +		enforce = FALSE;
>>>>  	}
>>>>  
>>>>  	if (!ret)
>>>
>>> Only if ret = 0,  "Pkey table was updated for node..." message is printed,
>>> however Pkey table might be changed even if ret = -1.
>>
>> This is related to your comment above, right ? Or are there additional
>> places where pkey table is updated and ret is -1 ?
>>
> 
> All the places.

I modified the log message to indicate that the PKey table was
successfully updated for the node. ret of -1 means there was some
failure (and things are partially updated).

-- Hal

>> -- Hal
>>
>>>> @@ -498,6 +538,9 @@ static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
>>>>  			cl_ntoh64(osm_node_get_node_guid(p_node)),
>>>>  			osm_physp_get_port_num(peer), p_node->print_desc);
>>>>  
>>>> +	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
>>>> +		ret = -1;
>>>> +
>>>>  	return ret;
>>>>  }
>>>>  
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/opensm/osm_pkey_mgr.c b/opensm/osm_pkey_mgr.c
index f6bc9d1..06d9b1e 100644
--- a/opensm/osm_pkey_mgr.c
+++ b/opensm/osm_pkey_mgr.c
@@ -398,6 +398,35 @@  static uint16_t last_used_pkey_index(const osm_port_t * const p_port,
 	return index;
 }
 
+static int update_peer_block(osm_log_t * p_log, osm_sm_t * sm,
+			     osm_physp_t * peer,
+			     osm_pkey_tbl_t * p_peer_pkey_tbl,
+			     ib_pkey_table_t * new_peer_block,
+			     uint16_t peer_block_idx, osm_node_t * p_node)
+{
+	int ret = 0;
+	ib_pkey_table_t *peer_block;
+
+	peer_block = osm_pkey_tbl_block_get(p_peer_pkey_tbl, peer_block_idx);
+	if (!peer_block ||
+	    memcmp(peer_block, new_peer_block, sizeof(*peer_block))) {
+		if (pkey_mgr_update_pkey_entry(sm, peer, new_peer_block,
+					       peer_block_idx) != IB_SUCCESS) {
+			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
+				"pkey_mgr_update_pkey_entry() failed to update "
+				"pkey table block %d for node 0x%016"
+				PRIx64 " port %u (%s)\n",
+				peer_block_idx,
+				cl_ntoh64(osm_node_get_node_guid(p_node)),
+				osm_physp_get_port_num(peer),
+				p_node->print_desc);
+			ret = -1;
+		}
+	}
+
+	return ret;
+}
+
 static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
 				     const osm_subn_t * p_subn,
 				     const osm_port_t * const p_port,
@@ -405,15 +434,16 @@  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
 {
 	osm_physp_t *p_physp, *peer;
 	osm_node_t *p_node;
-	ib_pkey_table_t *block, *peer_block;
+	ib_pkey_table_t *block;
 	const osm_pkey_tbl_t *p_pkey_tbl;
 	osm_pkey_tbl_t *p_peer_pkey_tbl;
-	uint16_t block_index;
+	uint16_t block_index, peer_block_idx;
 	uint16_t peer_max_blocks;
 	uint16_t last_index;
-	ib_api_status_t status = IB_SUCCESS;
-	ib_pkey_table_t empty_block;
-	int ret = 0;
+	ib_pkey_table_t new_peer_block;
+	uint16_t pkey_idx, peer_pkey_idx;
+	ib_net16_t pkey;
+	int ret = 0, loop_exit = 0;
 
 	p_physp = p_port->p_physp;
 	if (!p_physp)
@@ -425,70 +455,80 @@  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
 	if (!p_node->sw || !p_node->sw->switch_info.enforce_cap)
 		return 0;
 
-	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
-	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
-	if (peer_max_blocks < p_pkey_tbl->used_blocks) {
-		OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0508: "
-			"Not enough pkey blocks (%u < %u used) on switch 0x%016"
-			PRIx64 " port %u (%s). Clearing Enforcement bit\n",
-			peer_max_blocks, p_pkey_tbl->used_blocks,
-			cl_ntoh64(osm_node_get_node_guid(p_node)),
-			osm_physp_get_port_num(peer),
-			p_node->print_desc);
-		enforce = FALSE;
-		ret = -1;
-	} else if (peer_max_blocks == p_pkey_tbl->used_blocks) {
-		/* Is last used pkey index beyond switch peer port capacity ? */
-		last_index = (peer_max_blocks - 1) * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
-			     last_used_pkey_index(p_port, p_pkey_tbl);
-		if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
-			OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
-				"Not enough pkey entries (%u <= %u) on switch 0x%016"
-				PRIx64 " port %u (%s). Clearing Enforcement bit\n",
-				cl_ntoh16(p_node->sw->switch_info.enforce_cap),
-				last_index,
-				cl_ntoh64(osm_node_get_node_guid(p_node)),
-				osm_physp_get_port_num(peer),
-				p_node->print_desc);
-			enforce = FALSE;
-			ret = -1;
-		}
+	if (enforce == FALSE) {
+		pkey_mgr_enforce_partition(p_log, sm, peer, FALSE);
+		return -1;
 	}
 
-	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
-		ret = -1;
-
-	if (enforce == FALSE)
-		return ret;
-
-	memset(&empty_block, 0, sizeof(ib_pkey_table_t));
-
+	p_pkey_tbl = osm_physp_get_pkey_tbl(p_physp);
+	peer_max_blocks = pkey_mgr_get_physp_max_blocks(peer);
 	p_peer_pkey_tbl = &peer->pkeys;
-	p_peer_pkey_tbl->used_blocks = p_pkey_tbl->used_blocks;
+	peer_block_idx = 0;
+	peer_pkey_idx = 0;
 	for (block_index = 0; block_index < p_pkey_tbl->used_blocks;
 	     block_index++) {
+		if (loop_exit)
+			break;
 		block = osm_pkey_tbl_new_block_get(p_pkey_tbl, block_index);
 		if (!block)
-			block = &empty_block;
-
-		peer_block =
-		    osm_pkey_tbl_block_get(p_peer_pkey_tbl, block_index);
-		if (!peer_block
-		    || memcmp(peer_block, block, sizeof(*peer_block))) {
-			status = pkey_mgr_update_pkey_entry(sm, peer, block,
-							    block_index);
-			if (status != IB_SUCCESS) {
-				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0509: "
-					"pkey_mgr_update_pkey_entry() failed to update "
-					"pkey table block %d for node 0x%016"
-					PRIx64 " port %u (%s)\n", block_index,
-					cl_ntoh64(osm_node_get_node_guid
-						  (p_node)),
+			continue;
+		for (pkey_idx = 0; pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
+		     pkey_idx++) {
+			pkey = block->pkey_entry[pkey_idx];
+			if (ib_pkey_is_invalid(pkey))
+				continue;
+			new_peer_block.pkey_entry[peer_pkey_idx] = pkey;
+			if (peer_block_idx >= peer_max_blocks) {
+				loop_exit = 1;
+				break;
+			}
+			if (++peer_pkey_idx == IB_NUM_PKEY_ELEMENTS_IN_BLOCK) {
+				if (update_peer_block(p_log, sm, peer,
+						      p_peer_pkey_tbl,
+						      &new_peer_block,
+						      peer_block_idx, p_node))
+					ret = -1;
+				peer_pkey_idx = 0;
+				peer_block_idx++;
+			}
+		}
+	}
+
+	if (peer_block_idx < peer_max_blocks) {
+		if (peer_pkey_idx) {
+			/* Handle partial last block */
+			for (; peer_pkey_idx < IB_NUM_PKEY_ELEMENTS_IN_BLOCK;
+			     peer_pkey_idx++)
+				new_peer_block.pkey_entry[peer_pkey_idx] = 0;
+			if (update_peer_block(p_log, sm, peer, p_peer_pkey_tbl,
+					      &new_peer_block, peer_block_idx,
+					      p_node))
+				ret = -1;
+		} else
+			peer_block_idx--;
+
+		p_peer_pkey_tbl->used_blocks = peer_block_idx + 1;
+		if (p_peer_pkey_tbl->used_blocks == peer_max_blocks) {
+			/* Is last used pkey index beyond switch peer port capacity ? */
+			last_index = peer_block_idx * IB_NUM_PKEY_ELEMENTS_IN_BLOCK +
+				     last_used_pkey_index(p_port,
+							  p_peer_pkey_tbl);
+			if (cl_ntoh16(p_node->sw->switch_info.enforce_cap) <= last_index) {
+				OSM_LOG(p_log, OSM_LOG_ERROR, "ERR 0507: "
+					"Not enough pkey entries (%u <= %u) on switch 0x%016"
+					PRIx64 " port %u (%s). Clearing Enforcement bit\n",
+					cl_ntoh16(p_node->sw->switch_info.enforce_cap),
+					last_index,
+					cl_ntoh64(osm_node_get_node_guid(p_node)),
 					osm_physp_get_port_num(peer),
 					p_node->print_desc);
+				enforce = FALSE;
 				ret = -1;
 			}
 		}
+	} else {
+		p_peer_pkey_tbl->used_blocks = peer_max_blocks;
+		enforce = FALSE;
 	}
 
 	if (!ret)
@@ -498,6 +538,9 @@  static int pkey_mgr_update_peer_port(osm_log_t * p_log, osm_sm_t * sm,
 			cl_ntoh64(osm_node_get_node_guid(p_node)),
 			osm_physp_get_port_num(peer), p_node->print_desc);
 
+	if (pkey_mgr_enforce_partition(p_log, sm, peer, enforce))
+		ret = -1;
+
 	return ret;
 }