Re: [PATCH] opensm: Add configurable retries for transactions

[Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>]

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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html