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

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

On 16:51 Fri 30 Oct     , Hal Rosenstock wrote:
> On Fri, Oct 30, 2009 at 4:17 PM, Sasha Khapyorsky <sashak-smomgflXvObQFizaE/u3fw@public.gmane.orgm> 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.

I see, you are trying to protect old OpenSM <-> new vendor case.

Such combination will not work for many reasons (we changed OpenSM/vendor
interaction couple of times in past few years, including configuration)
and it is better to avoid such usage.

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

Old vendor will not work with '-1'. OTOH this enforces all vendor users
to set all parameter explicitly, and ignores those values when a value
other than '-1' was used in vendor creation. In fact it enforces using
'-1' and effectively makes it to be mandatory part of vendor API. Seems
too messy for me.

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