[ofa-general] Re: [PATCH] opensm: Bug in coding of VL Arbitration tables

[Hal Rosenstock]

Hi Eitan,

On 7/18/07, Eitan Zahavi <eitan at mellanox.co.il> wrote:
>
> Hi Sasha
>
> Discovered a bug in coding of the VL Arbitration table "index".
> According to spec should be:
> 1 for low part of low table
> 2 for high part of low table
> 3 for low part of high table
> 4 for high part of high table
>
> the patch below fixes it:
>
> Eitan
>
> Signed-off-by:  Eitan Zahavi <eitan at mellanox.co.il>
>
> 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
> @@ -116,14 +116,14 @@ static ib_api_status_t vlarb_update(osm_req_t *
> p_req,
>                     p_pi->vl_arb_low_cap :
> IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK;
>                 if ((status = vlarb_update_table_block(p_req, p, port_num,
>
> &qcfg->vlarb_low[0],
> -                                                      len, 0)) !=
> IB_SUCCESS)
> +                                                      len, 1)) !=
> IB_SUCCESS)
>                         return status;
>         }
>         if (p_pi->vl_arb_low_cap > IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK) {
>                 len = p_pi->vl_arb_low_cap %
> IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK;
>                 if ((status = vlarb_update_table_block(p_req, p, port_num,
>
> &qcfg->vlarb_low[1],
> -                                                      len, 1)) !=
> IB_SUCCESS)
> +                                                      len, 2)) !=
> IB_SUCCESS)
>                         return status;
>         }
>         if (p_pi->vl_arb_high_cap > 0) {
> @@ -131,14 +131,14 @@ static ib_api_status_t vlarb_update(osm_req_t *
> p_req,
>                     p_pi->vl_arb_high_cap :
> IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK;
>                 if ((status = vlarb_update_table_block(p_req, p, port_num,
>
> &qcfg->vlarb_high[0],
> -                                                      len, 2)) !=
> IB_SUCCESS)
> +                                                      len, 3)) !=
> IB_SUCCESS)
>                         return status;
>         }
>         if (p_pi->vl_arb_high_cap > IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK) {
>                 len = p_pi->vl_arb_high_cap %
> IB_NUM_VL_ARB_ELEMENTS_IN_BLOCK;
>                 if ((status = vlarb_update_table_block(p_req, p, port_num,
>
> &qcfg->vlarb_high[1],
> -                                                      len, 3)) !=
> IB_SUCCESS)
> +                                                      len, 4)) !=
> IB_SUCCESS)
>                         return status;
>         }



Are you sure ? It looks to me like this is already handled in
> vlarb_update_table_block as follows:
>

        if (!memcmp(&p->vl_arb[block_num], &block,
                     block_length * sizeof(block.vl_entry[0])))
                return IB_SUCCESS;

but

        attr_mod = ((block_num + 1) << 16) | port_num;

        return osm_req_set(p_req, osm_physp_get_dr_path_ptr(p),
                           (uint8_t *) & block, sizeof(block),
                           IB_MAD_ATTR_VL_ARBITRATION,
                           cl_hton32(attr_mod), CL_DISP_MSGID_NONE,
&context);

-- Hal
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openfabrics.org/pipermail/general/attachments/20070718/343e3180/attachment.html>