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