[ofa-general] Re: [RFC] [PATCH 2/2] for 2.6.23: ib/sa - add local path record caching

[Sean Hefty]

Michael S. Tsirkin wrote:
> It seems that below you try to get 0x7F paths to each dest:

This is the maximum number that a PR can request.  Note that you only 
get that many if that many exist.  I would expect most subnets to only 
have a couple of paths between each destination.

> But here you seem to bypass cache for multi-path queries:
> 
> +int ib_sa_path_rec_get(struct ib_sa_client *client,
> +		       struct ib_device *device, u8 port_num,
> +		       struct ib_sa_path_rec *rec,
> +		       ib_sa_comp_mask comp_mask,
> +		       int timeout_ms, gfp_t gfp_mask,
> +		       void (*callback)(int status,
> +					struct ib_sa_path_rec *resp,
> +					void *context),
> +		       void *context,
> +		       struct ib_sa_query **sa_query)


This is the existing ib_sa API, which only returns one path.  You could 
change the behavior to return an array of paths, but I did not do that 
at this time.

> +{
> +	struct sa_path_request *req;
> +	struct ib_sa_attr_iter iter;
> +	struct ib_sa_path_rec *path_rec;
> +	int ret;
> +
> +	if (!paths_per_dest)
> +		goto query_sa;
> +
> +	if (!(comp_mask & IB_SA_PATH_REC_DGID) ||
> +	    !(comp_mask & IB_SA_PATH_REC_NUMB_PATH) || rec->numb_path != 1)
> +		goto query_sa;
> 
> how are multiple paths used?

The cache returns paths using one of two algorithms.  Paths are either 
returned in a round robin fashion or randomly.  See further down in this 
same function:

+	if (lookup_method == SA_DB_LOOKUP_RANDOM)
+		path_rec = get_random_path(&iter, rec, comp_mask);
+	else
+		path_rec = get_next_path(&iter, rec, comp_mask);


The check for rec->numb_path != 1 should probably return a failure, 
since neither the API nor the underlying sa_query code supports it.

- Sean