[ofa-general] Re: [PATCH] opensm: Bug in coding trying to set vl_arb_high_limit

[Hal Rosenstock]

Hi again Eitan,

On 7/18/07, Eitan Zahavi <eitan at mellanox.co.il> wrote:
>
> Hi Sasha
>
> When QoS setup is done the code was trying to send updates of
> vl_arb_high_limit by req_set of PORT_INFO with the new data.
> However, at that stage the SM still did not assign LIDs to the ports.
> So the sent PortInfo.base_lid was still zero. The specification does not
> allow for such LIDs (they are considered ilegal).


 Doesn't that really depend on the PortState ? The LID (and SMLID) needs to
be set by ARMED/ACTIVE.

the patch below fixes this by storing the calculated value and later
> using it in link and lid managers.


It's probably better to defer the setting as this patch appears to do.

-- Hal

Eitan
>
> Signed-off-by:  Eitan Zahavi <eitan at mellanox.co.il>
>
> diff --git a/opensm/include/opensm/osm_port.h
> b/opensm/include/opensm/osm_port.h
> index 54ebcfc..5032b1b 100644
> --- a/opensm/include/opensm/osm_port.h
> +++ b/opensm/include/opensm/osm_port.h
> @@ -117,6 +117,7 @@ typedef struct _osm_physp
>    struct _osm_node             *p_node;
>    struct _osm_physp            *p_remote_physp;
>    boolean_t                      healthy;
> +  uint8_t                vl_high_limit;
>    osm_dr_path_t                         dr_path;
>    osm_pkey_tbl_t                 pkeys;
>    ib_vl_arb_table_t              vl_arb[4];
> diff --git a/opensm/opensm/osm_lid_mgr.c b/opensm/opensm/osm_lid_mgr.c
> index bc3f8b3..ed76382 100644
> --- a/opensm/opensm/osm_lid_mgr.c
> +++ b/opensm/opensm/osm_lid_mgr.c
> @@ -1182,6 +1182,14 @@ __osm_lid_mgr_set_physp_pi(
>             ib_port_info_get_port_state(p_old_pi) )
>          send_set = TRUE;
>      }
> +
> +        /* provide the vl_high_limit from the qos mgr */
> +        if (p_mgr->p_subn->opt.no_qos == FALSE)
> +                if (p_physp->vl_high_limit != p_old_pi->vl_high_limit)
> +                {
> +                        send_set = TRUE;
> +                        p_pi->vl_high_limit = p_physp->vl_high_limit;
> +                }
>    }
>    else
>    {
> diff --git a/opensm/opensm/osm_link_mgr.c b/opensm/opensm/osm_link_mgr.c
> index 25f0fc3..3781fd2 100644
> --- a/opensm/opensm/osm_link_mgr.c
> +++ b/opensm/opensm/osm_link_mgr.c
> @@ -354,6 +354,15 @@ __osm_link_mgr_set_physp_pi(
>        context.pi_context.active_transition = FALSE;
>    }
>
> +  /* provide the vl_high_limit from the qos mgr */
> +  if (p_mgr->p_subn->opt.no_qos == FALSE)
> +         if (p_physp->vl_high_limit != p_old_pi->vl_high_limit)
> +         {
> +                 send_set = TRUE;
> +                 p_pi->vl_high_limit = p_physp->vl_high_limit;
> +         }
> +
> +
>    context.pi_context.node_guid = osm_node_get_node_guid( p_node );
>    context.pi_context.port_guid = osm_physp_get_port_guid( p_physp );
>    context.pi_context.set_method = TRUE;
> diff --git a/opensm/opensm/osm_qos.c b/opensm/opensm/osm_qos.c
> index bbb1608..413e200 100644
> --- a/opensm/opensm/osm_qos.c
> +++ b/opensm/opensm/osm_qos.c
> @@ -216,42 +216,6 @@ static ib_api_status_t sl2vl_update(osm_req_t *
> p_req, osm_port_t * p_port,
>         return IB_SUCCESS;
> }
>
> -static ib_api_status_t vl_high_limit_update(osm_req_t * p_req,
> -                                           osm_physp_t * p,
> -                                           const struct qos_config *qcfg)
> -{
> -       uint8_t payload[IB_SMP_DATA_SIZE];
> -       osm_madw_context_t context;
> -       ib_port_info_t *p_pi;
> -
> -       p_pi = &p->port_info;
> -
> -       if (p_pi->vl_high_limit == qcfg->vl_high_limit)
> -               return IB_SUCCESS;
> -
> -       memset(payload, 0, IB_SMP_DATA_SIZE);
> -       memcpy(payload, p_pi, sizeof(ib_port_info_t));
> -
> -       p_pi = (ib_port_info_t *) payload;
> -       ib_port_info_set_state_no_change(p_pi);
> -
> -       p_pi->vl_high_limit = qcfg->vl_high_limit;
> -
> -       context.pi_context.node_guid =
> -           osm_node_get_node_guid(osm_physp_get_node_ptr(p));
> -       context.pi_context.port_guid = osm_physp_get_port_guid(p);
> -       context.pi_context.set_method = TRUE;
> -       context.pi_context.update_master_sm_base_lid = FALSE;
> -       context.pi_context.ignore_errors = FALSE;
> -       context.pi_context.light_sweep = FALSE;
> -       context.pi_context.active_transition = FALSE;
> -
> -       return osm_req_set(p_req, osm_physp_get_dr_path_ptr(p),
> -                          payload, sizeof(payload),
> IB_MAD_ATTR_PORT_INFO,
> -                          cl_hton32(osm_physp_get_port_num(p)),
> -                          CL_DISP_MSGID_NONE, &context);
> -}
> -
> static ib_api_status_t qos_physp_setup(osm_log_t * p_log, osm_req_t *
> p_req,
>                                        osm_port_t * p_port, osm_physp_t *
> p,
>                                        uint8_t port_num,
> @@ -261,16 +225,8 @@ static ib_api_status_t qos_physp_setup(osm_log_t *
> p_log, osm_req_t * p_req,
>
>         /* OpVLs should be ok at this moment - just use it */
>
> -       /* setup VL high limit */
> -       status = vl_high_limit_update(p_req, p, qcfg);
> -       if (status != IB_SUCCESS) {
> -               osm_log(p_log, OSM_LOG_ERROR,
> -                       "qos_physp_setup: ERR 6201 : "
> -                       "failed to update VLHighLimit "
> -                       "for port %" PRIx64 " #%d\n",
> -                       cl_ntoh64(p->port_guid), port_num);
> -               return status;
> -       }
> +       /* setup VL high limit on the physp later to be updated by
> lid/link mgrs */
> +       p->vl_high_limit = qcfg->vl_high_limit;
>
>         /* setup VLArbitration */
>         status = vlarb_update(p_req, p, port_num, qcfg);
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20070718/2aed6cba/attachment.html>