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?
> >> @@ -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?
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
