***SPAM*** [ofa-general] [PATCH] opensm/osm_mcast_tbl.c wrong max mcast lid cause the sm to set invalid MFT block.

[Eli Dorfman]

Hal Rosenstock wrote:
> Hi Eli,
> 
> On Thu, Nov 13, 2008 at 10:51 AM, Eli Dorfman <dorfman.eli at gmail.com> wrote:
>> wrong max mcast lid cause the sm to set invalid MFT block.
>> when mcmember tries to set mcast lid beyond mcast capability (e.g. 0xc400),
>> the sm accepts this and tries to set invalid block.
> 
> Good find (and nice test case).
> 
> Do the switch SMA's reject those invalid sets ? I'm hoping that's the case.

yes it is rejected as invalid.

> 
> See below for minor question on the patch.
> 
>> Signed-off-by: Eli Dorfman <elid at voltaire.com>
>>
>> ---
>>  opensm/opensm/osm_mcast_tbl.c          |    6 +++---
>>  opensm/opensm/osm_sa_mcmember_record.c |    2 +-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/opensm/opensm/osm_mcast_tbl.c b/opensm/opensm/osm_mcast_tbl.c
>> index 92fbb63..17fb69c 100644
>> --- a/opensm/opensm/osm_mcast_tbl.c
>> +++ b/opensm/opensm/osm_mcast_tbl.c
>> @@ -81,7 +81,7 @@ osm_mcast_tbl_init(IN osm_mcast_tbl_t * const p_tbl,
>>                                                IB_MCAST_BLOCK_SIZE) /
>>                                        IB_MCAST_BLOCK_SIZE) - 1);
>>
>> -       p_tbl->max_mlid_ho = (uint16_t) (IB_LID_MCAST_START_HO + capacity);
>> +       p_tbl->max_mlid_ho = (uint16_t) (IB_LID_MCAST_START_HO + capacity - 1);
>>
>>        /*
>>           The number of bytes needed in the mask table is:
>> @@ -216,7 +216,7 @@ osm_mcast_tbl_set_block(IN osm_mcast_tbl_t * const p_tbl,
>>
>>        mlid_start_ho = (uint16_t) (block_num * IB_MCAST_BLOCK_SIZE);
>>
>> -       if (mlid_start_ho + IB_MCAST_BLOCK_SIZE > p_tbl->max_mlid_ho)
>> +       if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
>>                return (IB_INVALID_PARAMETER);
>>
>>        for (i = 0; i < IB_MCAST_BLOCK_SIZE; i++)
>> @@ -274,7 +274,7 @@ osm_mcast_tbl_get_block(IN osm_mcast_tbl_t * const p_tbl,
>>
>>        mlid_start_ho = (uint16_t) (block_num * IB_MCAST_BLOCK_SIZE);
>>
>> -       if (mlid_start_ho + IB_MCAST_BLOCK_SIZE > p_tbl->max_mlid_ho)
>> +       if (mlid_start_ho + IB_MCAST_BLOCK_SIZE - 1 > p_tbl->max_mlid_ho)
>>                return (IB_INVALID_PARAMETER);
>>
>>        for (i = 0; i < IB_MCAST_BLOCK_SIZE; i++)
>> diff --git a/opensm/opensm/osm_sa_mcmember_record.c b/opensm/opensm/osm_sa_mcmember_record.c
>> index 5dd286a..6007b06 100644
>> --- a/opensm/opensm/osm_sa_mcmember_record.c
>> +++ b/opensm/opensm/osm_sa_mcmember_record.c
>> @@ -846,7 +846,7 @@ osm_mcmr_rcv_create_new_mgrp(IN osm_sa_t * sa,
>>        mlid = __get_new_mlid(sa, mcm_rec.mlid);
>>        if (mlid == 0) {
>>                OSM_LOG(sa->p_log, OSM_LOG_ERROR, "ERR 1B19: "
>> -                       "__get_new_mlid failed\n");
>> +                       "__get_new_mlid failed request mlid 0x%04x\n", mcm_rec.mlid);
> 
>                             ^^^^^^^^^^^^^^^^
> Should this be cl_ntoh16(mcm_rec.mlid) ?

yes, i'll fix the patch.

Thanks,
Eli