On Fri, Oct 30, 2009 at 4:17 PM, Sasha Khapyorsky <[email protected]> wrote:
> On 11:48 Fri 30 Oct     , Hal Rosenstock wrote:
>> >> @@ -819,6 +821,13 @@ osm_vendor_bind(IN osm_vendor_t * const p_vend,
>> >>       p_bind->send_err_callback = send_err_callback;
>> >>       p_bind->p_mad_pool = p_mad_pool;
>> >>       p_bind->port_guid = port_guid;
>> >> +     if (p_vend->timeout == -1) {
>> >> +             p_bind->timeout = p_user_bind->timeout;
>> >> +             p_bind->max_retries = p_user_bind->retries;
>> >> +     } else {
>> >> +             p_bind->timeout = p_vend->timeout;
>> >> +             p_bind->max_retries = p_vend->max_retries;
>> >> +     }
>> >
>> > Hmm, shouldn't we respect user requested data? Something like:
>> >
>> >        p_bind->timeout = p_user_bind->timeout ? p_user_bind->timeout :
>> >                                p_vend->timeout;
>> >        p_bind->retries = p_user_bind->retries ? p_user_bind->retries :
>> >                                p_vend->retries;
>> >
>> > ?
>>
>> The -1 is for the new ABI.
>
> Could you elaborate?

In order to be able to deal with combinations of old/new opensm/vendor layer.

>> >> @@ -983,6 +989,11 @@ int main(int argc, char *argv[])
>> >>                       opt.sm_sl = (uint8_t) temp;
>> >>                       printf(" SMSL = %d\n", opt.sm_sl);
>> >>                       break;
>> >> +             case 8:
>> >> +                     opt.transaction_retries = strtol(optarg, NULL, 0);
>> >
>> > Please use strtoul().
>>
>> OK. What about transaction_timeout ?
>
> Yes, the same is applicable here.
>
>> >> @@ -392,8 +393,7 @@ ib_api_status_t osm_opensm_init(IN osm_opensm_t * 
>> >> p_osm,
>> >>       if (status != IB_SUCCESS)
>> >>               goto Exit;
>> >>
>> >> -     p_osm->p_vendor =
>> >> -         osm_vendor_new(&p_osm->log, p_opt->transaction_timeout);
>> >> +     p_osm->p_vendor = osm_vendor_new(&p_osm->log, -1);
>> >
>> > Why to not make p_opt->transaction_timeout as default for newly
>> > allocated vendor object?
>>
>> To support old and new ABI.
>
> What do you mean? OpenSM will run against older vendor?
>
> Even if so how '-1' is helpful? Likely it will break things, no?

-1 is not a valid timeout so can't this be used in this manner ? How
will it break things ?

-- Hal

> Sasha
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to