On 13:14 Wed 29 Oct , Yevgeny Kliteynik wrote:
> Sasha Khapyorsky wrote:
>> 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 <[EMAIL PROTECTED]>
>>> ---
>> [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.
>
> Do we care that the lin_cap of the switch claims to support more
> than IB_LID_UCAST_END_HO? Don't think so, so I agree - removing this.
It means buggy switch, we can drop a warning in run-time, but this is
not a reason for OpenSM to crash with CL_ASSERT().
>>> +
>>> + 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()?
>
> Well, I can really simplify this whole function to
> something like this:
>
> boolean_t
> osm_switch_get_lft_block(IN const osm_switch_t * const p_sw,
> IN const uint16_t block_id,
> OUT uint8_t * const p_block)
> {
> uint16_t base_lid_ho = block_id * IB_SMP_DATA_SIZE;
> CL_ASSERT(p_sw);
> CL_ASSERT(p_block);
> if (base_lid_ho > p_sw->max_lid_ho)
> return FALSE;
> memcpy(p_block, &(p_sw->lft[base_lid_ho]), IB_SMP_DATA_SIZE);
> return TRUE;
> }
Good.
> Patch shortly.
Thanks.
Sasha
_______________________________________________
general mailing list
general@lists.openfabrics.org
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
