Hi Sasha,

On Fri, Oct 30, 2009 at 5:17 PM, Sasha Khapyorsky <[email protected]> wrote:
>
> Hi Hal,
>
> On 09:27 Wed 23 Sep     , Hal Rosenstock wrote:
>>
>> Fix seg fault which occurs when get_osm_switch_from_port is
>> called with NULL port (which in this case was caused by calling
>> cl_ptr_vector_get on port LID table with LID 0)
>
> Would be really useful to describe when and why this (LID = 0) can
> happen.

Thought we resolved this in "osm_link_mgr.c:link_mgr_get_smsl question" thread.

>>
>> Signed-off-by: Hal Rosenstock <[email protected]>
>
> What about simpler change:
>
>
> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> index 4d4be56..217f51e 100644
> --- a/opensm/opensm/osm_link_mgr.c
> +++ b/opensm/opensm/osm_link_mgr.c
> @@ -68,7 +68,8 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> osm_physp_t * p_physp)
>
>          OSM_LOG_ENTER(sm->p_log);
>
> -        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH) {
> +        if (p_osm->routing_engine_used != OSM_ROUTING_ENGINE_TYPE_LASH
> +            || !(slid = osm_physp_get_base_lid(p_physp))) {
>                  /* Use default SL if lash routing is not used */
>                  OSM_LOG_EXIT(sm->p_log);
>                  return (sm->p_subn->opt.sm_sl);
> @@ -80,7 +81,6 @@ static uint8_t link_mgr_get_smsl(IN osm_sm_t * sm, IN
> osm_physp_t * p_physp)
>              cl_ptr_vector_get(&sm->p_subn->port_lid_tbl, cl_ntoh16(smlid));
>
>          /* Find osm_port of the source = p_physp */
> -        slid = osm_physp_get_base_lid(p_physp);
>          p_src_port =
>              cl_ptr_vector_get(&sm->p_subn->port_lid_tbl, cl_ntoh16(slid));
>
>
> Wouldn't it be the same functionally?

Yes,

Do you want an updated patch ?

-- Hal

> Sasha
>
>> ---
>> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
>> index c9bdfee..35f83e2 100644
>> --- a/opensm/opensm/osm_link_mgr.c
>> +++ b/opensm/opensm/osm_link_mgr.c
>> @@ -131,27 +131,32 @@ static int link_mgr_set_physp_pi(osm_sm_t * sm, IN
>> osm_physp_t * p_physp,
>>                  if
>> (ib_switch_info_is_enhanced_port0(&p_node->sw->switch_info)
>>                      == FALSE) {
>>
>> -                        /* Even for base port 0 we might have to set smsl
>> -                           (if we are using lash routing) */
>> -                        smsl = link_mgr_get_smsl(sm, p_physp);
>> -                        if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> -                                send_set = TRUE;
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Setting SMSL to %d on port 0
>> GUID 0x%016"
>> -                                        PRIx64 "\n", smsl,
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)));
>> -                        } else {
>> -                                /* This means the switch doesn't support
>> -                                   enhanced port 0 and we don't need to
>> -                                   change SMSL. Can skip it. */
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Skipping port 0, GUID 0x%016"
>> PRIx64
>> -                                        "\n",
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)));
>> -                                goto Exit;
>> +                        /* Make sure LID is valid prior to calling
>> link_mgr_get_smsl */
>> +                        if (osm_physp_get_base_lid(p_physp)) {
>> +
>> +                                /* Even for base port 0 we might have to
>> set
>> +                                   smsl (if we are using lash routing) */
>> +                                smsl = link_mgr_get_smsl(sm, p_physp);
>> +                                if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> +                                        send_set = TRUE;
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Setting SMSL to %d on
>> port 0 "
>> +                                                "GUID 0x%016" PRIx64
>> "\n", smsl,
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)));
>> +                                } else {
>> +                                        /* This means the switch doesn't
>> support
>> +                                           enhanced port 0 and we don't
>> need to
>> +                                           change SMSL. Can skip it. */
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Skipping port 0, GUID
>> 0x%016"
>> +                                                PRIx64 "\n",
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)));
>> +                                        goto Exit;
>> +                                }
>>                          }
>> +
>>                  } else
>>                          esp0 = TRUE;
>>          }
>> @@ -217,18 +222,22 @@ static int link_mgr_set_physp_pi(osm_sm_t * sm, IN
>> osm_physp_t * p_physp,
>>                                     sizeof(p_pi->master_sm_base_lid)))
>>                                  send_set = TRUE;
>>
>> -                        smsl = link_mgr_get_smsl(sm, p_physp);
>> -                        if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>> +                        /* Make sure LID is valid prior to calling
>> link_mgr_get_smsl */
>> +                        if (osm_physp_get_base_lid(p_physp)) {
>> +                                smsl = link_mgr_get_smsl(sm, p_physp);
>> +                                if (smsl !=
>> ib_port_info_get_master_smsl(p_old_pi)) {
>>
>> -                                ib_port_info_set_master_smsl(p_pi, smsl);
>>
>> +                                        ib_port_info_set_master_smsl(p_pi,
>> smsl);
>>
>> -                                OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> -                                        "Setting SMSL to %d on GUID
>> 0x%016"
>> -                                        PRIx64 ", port %d\n", smsl,
>> -                                        cl_ntoh64(osm_physp_get_port_guid
>> -                                                  (p_physp)), port_num);
>> +                                        OSM_LOG(sm->p_log, OSM_LOG_DEBUG,
>> +                                                "Setting SMSL to %d on
>> GUID "
>> +                                                "0x%016" PRIx64 ", port
>> %d\n",
>> +                                                smsl,
>>
>> +                                                cl_ntoh64(osm_physp_get_port_guid
>> +                                                          (p_physp)),
>> port_num);
>>
>> -                                send_set = TRUE;
>> +                                        send_set = TRUE;
>> +                                }
>>                          }
>>
>>                          p_pi->m_key_lease_period =
>>
>

Reply via email to