On Wed, May 14, 2014 at 04:44:50PM +0300, Or Gerlitz wrote: > From: Matan Barak <[email protected]> > > The receive flow steering API didn't conform with the extension > verbs: > 1. It introduced VERBS_CONTEXT_CREATE_FLOW and > VERBS_CONTEXT_DESTROY_FLOW, even though the structures > aren't allocated by the provider.
"even though the structures aren't allocated by the provider" is not the reason.. 1. VERBS_CONTEXT_CREATE_FLOW/VERBS_CONTEXT_DESTROY_FLOW mis uses 'has_comp_mask' which is supposed to indicate if a legacy structure is extended or not. It does not indicate the presence of a function call BTW, I see now that libmlx4 was never updated to set these values (???) so I think we can actually just drop them instead of adding RESERVED, since they were never written or read. > struct verbs_context *vctx = verbs_get_ctx_op(qp->context, > - lib_ibv_create_flow); > - if (!vctx || !vctx->lib_ibv_create_flow) > + create_flow); > + if (!vctx) > return NULL; Right, that 2nd test for lib_ibv_create flow was redundant. Should errno be set here? I've forgotton what we settled on :| No matter what, errno will be set by the kernel and leak out of the provider call. It feels broken that there are multiple failure modes to the call and now way for the user to tell them apart. .. and the man page should talk about errno if it is actually part of the API. > /** > diff --git a/man/ibv_create_flow.3 b/man/ibv_create_flow.3 > new file mode 100644 > index 0000000..f7c767e > +++ b/man/ibv_create_flow.3 > @@ -0,0 +1,86 @@ > +.TH IBV_CREATE_FLOW 3 2013-08-21 libibverbs "Libibverbs > Programmer's Manual" Christoph: can you, as a user of this API, give the man page a quick read to check that it captures everything? Otherwise, this looks good to me. Reviewd-by: Jason Gunthorpe <[email protected]> Thanks for taking care of this Or Jason -- 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
