[ofa-general] [PATCH] osm_port.c: do not force max_op_vls = 0 to 1

[Hal Rosenstock]

On Tue, May 5, 2009 at 9:48 AM, Eli Dorfman (Voltaire)
<dorfman.eli at gmail.com> wrote:
> Hal Rosenstock wrote:
>> On Tue, May 5, 2009 at 9:00 AM, Doron Shoham <dorons at voltaire.com> wrote:
>>> when setting max_op_vls = 0
>>> do not force it to 1.
>>> 0 is valid value which means "No change"
>>>
>>> Signed-off-by: Doron Shoham <dorons at voltaire.com>
>>> ---
>>>  opensm/opensm/osm_port.c   |    6 ------
>>>  opensm/opensm/osm_subnet.c |    8 ++++++++
>>>  2 files changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/opensm/opensm/osm_port.c b/opensm/opensm/osm_port.c
>>> index 2e6c642..db0c27e 100644
>>> --- a/opensm/opensm/osm_port.c
>>> +++ b/opensm/opensm/osm_port.c
>>> @@ -380,12 +380,6 @@ uint8_t osm_physp_calc_link_op_vls(IN osm_log_t * p_log,
>>>        if (op_vls > p_subn->opt.max_op_vls)
>>>                op_vls = p_subn->opt.max_op_vls;
>>>
>>> -       if (op_vls == 0) {
>>> -               OSM_LOG(p_log, OSM_LOG_DEBUG, "ERR 4102: "
>>> -                       "Invalid OP_VLS = 0. Forcing correction to 1 (VL0)\n");
>>> -               op_vls = 1;
>>> -       }
>>> -
>>
>> Should that only be done when max_op_vls is 0 ?
>>
>> Something like:
>>            if (op_vls > p_subn->opt.max_op_vls)
>>                 op_vls = p_subn->opt.max_op_vls;
>>            else if (op_vls == 0) {
>>                OSM_LOG(p_log, OSM_LOG_DEBUG, "ERR 4102: "
>>                        "Invalid OP_VLS = 0. Forcing correction to 1 (VL0)\n");
>>                op_vls = 1;
>>           }
>
> why do you suggest a special case for op_vls=0 (and not for other portinfo fields)?

> is there a firmware bug that reports op_vls=0?

There were (still are ?) implementations which returned op_vls 0 which
is why the words "valid on Set()" were added to the IBA spec and why I
don't feel safe removing the code as originally proposed but think my
alternative is safe and accomplishes the stated goal. Is there a
problem with my alternative proposal ?

-- Hal

> Eli
>
>
>