On 21/5/2014 11:31 PM, Jason Gunthorpe wrote:
On Sun, May 18, 2014 at 12:38:47PM +0300, Or Gerlitz wrote:struct ibv_ah *__ibv_create_ah(struct ibv_pd *pd, struct ibv_ah_attr *attr) { - struct ibv_ah *ah = pd->context->ops.create_ah(pd, attr); + int err; + struct ibv_ah *ah = NULL; +#ifndef NRESOLVE_NEIGH + struct ibv_port_attr port_attr; + int dst_family; + int src_family; + int oif; + struct get_neigh_handler neigh_handler; + union ibv_gid sgid; + struct ibv_ah_attr_ex attr_ex; + int ether_len; + struct verbs_context *vctx = verbs_get_ctx_op(pd->context, + create_ah_ex); + struct peer_address src; + struct peer_address dst; + + if (!vctx) { +#endif + ah = pd->context->ops.create_ah(pd, attr); +#ifndef NRESOLVE_NEIGH + goto return_ah; + } + + err = ibv_query_port(pd->context, attr->port_num, &port_attr);It feels like a regression to force this overhead. Many HCAs have no possibility to support anything but IB, or Ethernet and don't need this. This whole arrangement seems strange. create_ah_ex should be a full fledged user callable function, not a buried driver entry point. It is also very unusual for verbs to have all this generic code in a driver wrapper. I suspect the answer here is to have the driver call into helper functions from verbs to do this addressing work. 'get_ethernet_l2_from_ah' or something. If you do that then we don't even need create_ah_ex and query_port_ex - those function seem to only be required to support this wrapper technique. So please rethink how this flows.. Maybe wrapping is not the best choice?? Jason
We could use a libibverbs API call in order to resolve the IP based GID into a MAC, but I think it could cause multiple vendors to have some code duplication. We hope that in the future, more products will use RoCE with IP based GIDs. All those providers will have to supply similar code that checks if the link layer is Ethernet and IP based GID is used, they'll have to use the libibverbs utility function.
A possible future create_ah_ex *libibverbs* could be a full fledged verb that uses the new provider's create_ah_ex. Currently, it's hidden inside the good old create_ah call, but we shouldn't limit ourselves to keep it that way.
Anyway, I have no objection to use a utility function instead of this method, but I do think that the current code has some advantages.
Matan -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
