[ofa-general] [PATCH 11/10] libibmad:infiniband-diags: deprecate madrpc_set_[retries|timeout] WAS: [PATCH 1/10] libibmad: Clean up "new" interface

[Ira Weiny]

On Sun, 1 Mar 2009 20:23:59 -0500
Hal Rosenstock <hal.rosenstock at gmail.com> wrote:

> On Sun, Mar 1, 2009 at 2:26 AM, Sasha Khapyorsky <sashak at voltaire.com> wrote:
> 
> [snip...]
> 
> > Now you need to duplicate this single call over all tools. For me it
> > looks like an overkill.
> 
> See below.
> 
> > Probably it would be simpler to just read global
> > ibd_timeout variable in rpc.c?
> 
> [snip...]
> 
> > Same with retries - it is hard for me to believe that any multithreaded
> > application will try to setup different retry values per port, for
> > different threads, "on the fly"....
> 
> For timeouts and retries, per port makes sense to me as ports may be
> connected to different fabrics with different timeouts. Also, it may
> depend on the operation (e.g. restricted to a single subnet or across
> multiple subnets).

I see this as well.  Right now libibmad is designed around the "run and exit"
diag model but we are moving it toward a library which can be used in more
complex applications.  We should push to do this once and as correct as
possible.

> 
> > (rpc.c with all its limited functionality will not be sufficient for such flexibility
> > level anyway :)).
> 
> This would be an implementation issue to fix if the more flexible
> timeout/retries is to be implemented.

Agreed.  I have found further places where the timeout needs to be cleaned up.
<sigh> Just search for timeout in mad.h...  In addition to the *_set_timeout
calls; there are many functions which specify it directly; either through a
struct (ib_rpc_t, ib_vendor_call_t) or via parameter (mad_receive*,
smp_[query|set], sa_call, sa_rcp_call, ib_resolve*, pma_query_via,
performance_reset_via).

Retries are a bit more straight forward.

I will think on this some more,
Ira

-- 
Ira Weiny
Math Programer/Computer Scientist
Larence Livermore National Lab
weiny2 at llnl.gov