[ofa-general] Re: [PATCH 1/2] opensm: replace switch's fwd_tbl with simple LFT

[Sasha Khapyorsky]

Hi Yevgeny,

On 23:36 Thu 16 Oct     , Yevgeny Kliteynik wrote:
> Replace the unnecessarily complex switch's forwarding table
> implementation with a simple LFT that is implemented as plain
> uint8_t array.
> 
> Signed-off-by: Yevgeny Kliteynik <kliteyn at dev.mellanox.co.il>
> ---

[snip...]

> diff --git a/opensm/opensm/osm_switch.c b/opensm/opensm/osm_switch.c
> index 9bf76e0..bdfc7d0 100644
> --- a/opensm/opensm/osm_switch.c
> +++ b/opensm/opensm/osm_switch.c
> @@ -97,9 +97,26 @@ osm_switch_init(IN osm_switch_t * const p_sw,
>  	p_sw->num_ports = num_ports;
>  	p_sw->need_update = 2;
> 
> -	status = osm_fwd_tbl_init(&p_sw->fwd_tbl, p_si);
> -	if (status != IB_SUCCESS)
> +	/* Initiate the linear forwarding table */
> +
> +	if (!p_si->lin_cap) {
> +		/* This switch does not support linear forwarding tables */
> +		status = IB_UNSUPPORTED;
>  		goto Exit;
> +	}
> +
> +	/* The capacity reported by the switch includes LID 0,
> +	   so add 1 to the end of the range here for this assert. */
> +        CL_ASSERT(cl_ntoh16(p_si->lin_cap) <= IB_LID_UCAST_END_HO + 1);

Maybe there should be run-time check (not sure since lin_cap is not
really used in other places in the code), but not assertion - any bogus
data received from network should not crash OpenSM. I'm removing this.

> +
> +	p_sw->lft = malloc(IB_LID_UCAST_END_HO + 1);
> +	if (!p_sw->lft) {
> +		status = IB_INSUFFICIENT_MEMORY;
> +		goto Exit;
> +	}
> +
> +	/* Initialize the table to OSM_NO_PATH, which is "invalid port" */
> +	memset(p_sw->lft, OSM_NO_PATH, IB_LID_UCAST_END_HO + 1);
> 
>  	p_sw->lft_buf = malloc(IB_LID_UCAST_END_HO + 1);
>  	if (!p_sw->lft_buf) {
> @@ -138,7 +155,8 @@ void osm_switch_delete(IN OUT osm_switch_t ** const pp_sw)
> 
>  	osm_mcast_tbl_destroy(&p_sw->mcast_tbl);
>  	free(p_sw->p_prof);
> -	osm_fwd_tbl_destroy(&p_sw->fwd_tbl);
> +	if (p_sw->lft)
> +		free(p_sw->lft);
>  	if (p_sw->lft_buf)
>  		free(p_sw->lft_buf);
>  	if (p_sw->hops) {
> @@ -176,44 +194,36 @@ osm_switch_t *osm_switch_new(IN osm_node_t * const p_node,
>  /**********************************************************************
>   **********************************************************************/
>  boolean_t
> -osm_switch_get_fwd_tbl_block(IN const osm_switch_t * const p_sw,
> -			     IN const uint32_t block_id,
> -			     OUT uint8_t * const p_block)
> +osm_switch_get_lft_block(IN const osm_switch_t * const p_sw,
> +			 IN const uint32_t block_id,
> +			 OUT uint8_t * const p_block)
>  {
>  	uint16_t base_lid_ho;
> -	uint16_t max_lid_ho;
> -	uint16_t lid_ho;
>  	uint16_t block_top_lid_ho;
> -	uint32_t lids_per_block;
> -	osm_fwd_tbl_t *p_tbl;
>  	boolean_t return_flag = FALSE;
> 
>  	CL_ASSERT(p_sw);
>  	CL_ASSERT(p_block);
> 
> -	p_tbl = osm_switch_get_fwd_tbl_ptr(p_sw);
> -	max_lid_ho = p_sw->max_lid_ho;
> -	lids_per_block = osm_fwd_tbl_get_lids_per_block(&p_sw->fwd_tbl);
> -	base_lid_ho = (uint16_t) (block_id * lids_per_block);
> +	base_lid_ho = (uint16_t) (block_id * IB_SMP_DATA_SIZE);
> 
> -	if (base_lid_ho <= max_lid_ho) {
> +	if (base_lid_ho <= p_sw->max_lid_ho) {
>  		/* Initialize LIDs in block to invalid port number. */
>  		memset(p_block, OSM_NO_PATH, IB_SMP_DATA_SIZE);
>  		/*
>  		   Determine the range of LIDs we can return with this block.
>  		 */
>  		block_top_lid_ho =
> -		    (uint16_t) (base_lid_ho + lids_per_block - 1);
> -		if (block_top_lid_ho > max_lid_ho)
> -			block_top_lid_ho = max_lid_ho;
> +		    (uint16_t) (base_lid_ho + IB_SMP_DATA_SIZE - 1);
> +		if (block_top_lid_ho > p_sw->max_lid_ho)
> +			block_top_lid_ho = p_sw->max_lid_ho;
> 
>  		/*
>  		   Configure the forwarding table with the routing
>  		   information for the specified block of LIDs.
>  		 */
> -		for (lid_ho = base_lid_ho; lid_ho <= block_top_lid_ho; lid_ho++)
> -			p_block[lid_ho - base_lid_ho] =
> -			    osm_fwd_tbl_get(p_tbl, lid_ho);
> +		memcpy(p_block, &(p_sw->lft[base_lid_ho]),
> +		       block_top_lid_ho - base_lid_ho + 1);

Hmm, why not just

	memcpy(p_block, &p_sw->lft[base_lid_ho], 64);

? And then no need initial memset()?

Sasha