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

[Eli Dorfman (Voltaire)]

Hal Rosenstock wrote:
> 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 ?

no, but there are other fields in portinfo that are not validated.
for example link_speed_enabled (which allows 0 value only on Set as well).
also if a node returns op_vl=0 how do you know it supports op_vl=1?


Eli