prov/efa: Refactor AV to split it for efa/efa-direct

libfabric.vcxproj

@@ -873,7 +873,7 @@
     <ClCompile Include="prov\efa\src\windows\efawin.c" />
     <ClCompile Include="prov\efa\src\efa_av.c" />
     <ClCompile Include="prov\efa\src\efa_ah.c" />
-    <ClCompile Include="prov\efa\src\efa_conn.c" />
+    <ClCompile Include="prov\efa\src\rdm\efa_proto_av.c" />
     <ClCompile Include="prov\efa\src\efa_hmem.c" />
     <ClCompile Include="prov\efa\src\efa_device.c" />
     <ClCompile Include="prov\efa\src\efa_domain.c" />
@@ -1018,7 +1018,7 @@
     <ClInclude Include="prov\efa\src\efa.h" />
     <ClInclude Include="prov\efa\src\efa_av.h" />
     <ClInclude Include="prov\efa\src\efa_ah.h" />
-    <ClInclude Include="prov\efa\src\efa_conn.h" />
+    <ClInclude Include="prov\efa\src\rdm\efa_proto_av.h" />
     <ClInclude Include="prov\efa\src\efa_base_ep.h" />
     <ClInclude Include="prov\efa\src\efa_cntr.h" />
     <ClInclude Include="prov\efa\src\rdm\efa_rdm_ep.h" />

prov/efa/Makefile.include

@@ -36,7 +36,7 @@ _efa_files = \
 	prov/efa/src/efa_shm.c \
 	prov/efa/src/efa_av.c \
 	prov/efa/src/efa_ah.c \
-	prov/efa/src/efa_conn.c \
+	prov/efa/src/rdm/efa_proto_av.c \
 	prov/efa/src/efa_domain.c \
 	prov/efa/src/efa_fabric.c \
 	prov/efa/src/efa_mr.c \
@@ -89,7 +89,6 @@ _efa_headers = \
 	prov/efa/src/efa.h \
 	prov/efa/src/efa_av.h \
 	prov/efa/src/efa_ah.h \
-	prov/efa/src/efa_conn.h \
 	prov/efa/src/efa_mr.h \
 	prov/efa/src/efa_shm.h \
 	prov/efa/src/efa_hmem.h \
@@ -117,6 +116,7 @@ _efa_headers = \
 	prov/efa/src/efa_data_path_direct_internal.h \
 	prov/efa/src/efa_mmio.h \
 	prov/efa/src/rdm/efa_rdm_peer.h	\
+	prov/efa/src/rdm/efa_proto_av.h \
 	prov/efa/src/rdm/efa_rdm_cq.h \
 	prov/efa/src/rdm/efa_rdm_cntr.h \
 	prov/efa/src/rdm/efa_rdm_ep.h \
@@ -159,6 +159,7 @@ nodist_prov_efa_test_efa_unit_test_SOURCES = \
 	prov/efa/test/efa_unit_test_domain.c \
 	prov/efa/test/efa_unit_test_ep.c \
 	prov/efa/test/efa_unit_test_av.c \
+	prov/efa/test/efa_unit_test_proto_av.c \
 	prov/efa/test/efa_unit_test_cq.c \
 	prov/efa/test/efa_unit_test_cntr.c \
 	prov/efa/test/efa_unit_test_device.c \

prov/efa/src/efa.h

@@ -52,6 +52,7 @@
 #include "rdm/efa_rdm_ope.h"
 #include "rdm/efa_rdm_pke.h"
 #include "rdm/efa_rdm_peer.h"
+#include "rdm/efa_proto_av.h"
 #include "rdm/efa_rdm_util.h"
 #include "fi_ext_efa.h"
 

prov/efa/src/efa_ah.c

@@ -5,75 +5,18 @@
 
 #include "efa.h"
 #include "efa_ah.h"
-#include "efa_conn.h"
 #include <infiniband/efadv.h>
 
-void efa_ah_destroy_ah(struct efa_domain *domain, struct efa_ah *ah);
-
 /**
- * @brief Move the AH to the end of the LRU list to indicate that it is the
- * most recently used entry
+ * @brief Emit a detailed warning for ibv_create_ah EINVAL.
  *
- * This function is not called in the efa_rdm_ep_get_peer so that we don't add
- * extra latency to the critical path with explicit AV insertion. We use the LRU
- * list to remove AH entries with only implicit AV entries, so it is OK to do
- * that.
+ * The most common reasons for EINVAL are cross-AZ addressing, invalid
+ * remote GID, and invalid PD. Log both local and remote GIDs plus the
+ * PD pointer to help operators diagnose failures from logs alone.
  *
- * @param[in]	av	efa address vector
- * @param[in]	conn	efa conn to be added to the LRU list
+ * @param[in]	domain	efa domain (for local GID and PD)
+ * @param[in]	gid	remote GID that failed
  */
-void efa_ah_implicit_av_lru_ah_move(struct efa_domain *domain,
-					struct efa_ah *ah)
-{
-	assert(ah->implicit_refcnt > 0 || ah->explicit_refcnt > 0);
-	assert(dlist_entry_in_list(&domain->ah_lru_list,
-				   &ah->domain_lru_ah_list_entry));
-
-	dlist_remove(&ah->domain_lru_ah_list_entry);
-	dlist_insert_tail(&ah->domain_lru_ah_list_entry,
-			  &domain->ah_lru_list);
-}
-
-static inline int efa_ah_implicit_av_evict_ah(struct efa_domain *domain) {
-	struct efa_conn *conn_to_release;
-	struct efa_ah *ah_tmp, *ah_to_release = NULL;
-	struct dlist_entry *tmp;
-
-	dlist_foreach_container (&domain->ah_lru_list, struct efa_ah, ah_tmp,
-				 domain_lru_ah_list_entry) {
-		if (ah_tmp->explicit_refcnt == 0) {
-			ah_to_release = ah_tmp;
-			break;
-		}
-	}
-
-	if (!ah_to_release) {
-		EFA_WARN(FI_LOG_AV,
-			 "AH creation for implicit AV entry failed with ENOMEM "
-			 "but no AH entries available to evict\n");
-		return -FI_ENOMEM;
-	}
-
-	assert(ah_to_release->implicit_refcnt > 0);
-
-	dlist_foreach_container_safe(&ah_to_release->implicit_conn_list,
-				      struct efa_conn, conn_to_release,
-				      ah_implicit_conn_list_entry, tmp) {
-
-		assert(conn_to_release->implicit_fi_addr != FI_ADDR_NOTAVAIL &&
-		       conn_to_release->fi_addr == FI_ADDR_NOTAVAIL);
-
-		efa_conn_release_ah_unsafe(conn_to_release->av, conn_to_release, true);
-	}
-
-	if (ah_to_release->implicit_refcnt == 0 &&
-	    ah_to_release->explicit_refcnt == 0) {
-		efa_ah_destroy_ah(domain, ah_to_release);
-	}
-
-	return FI_SUCCESS;
-}
-
 static void efa_ah_warn_create_einval(struct efa_domain *domain, const uint8_t *gid)
 {
 	char remote_gid_str[INET6_ADDRSTRLEN] = {0};
@@ -95,37 +38,44 @@ static void efa_ah_warn_create_einval(struct efa_domain *domain, const uint8_t *
 }
 
 /**
- * @brief allocate an ibv_ah object from GID.
- * This function use a hash map to store GID to ibv_ah map,
- * and re-use ibv_ah for same GID
+ * @brief allocate an ibv_ah from GID, reusing existing AH if possible
+ *
+ * Uses a hash map to store GID to ibv_ah mapping and reuses ibv_ah for
+ * the same GID. If ibv_create_ah fails, returns NULL with errno set.
+ * The caller is responsible for handling ENOMEM (e.g. by evicting AH
+ * entries and retrying).
  *
- * @param[in]	domain	efa_domain
- * @param[in]	gid	GID
+ * @param[in]	domain		efa domain
+ * @param[in]	gid		GID
+ * @param[in]	alloc_size	size to allocate (sizeof(efa_ah) or larger for protocol wrapper)
+ * @return	pointer to efa_ah on success, NULL on failure (errno set)
  */
 struct efa_ah *efa_ah_alloc(struct efa_domain *domain, const uint8_t *gid,
-			    bool insert_implicit_av)
+			    size_t alloc_size)
 {
 	struct ibv_pd *ibv_pd = domain->ibv_pd;
 	struct efa_ah *efa_ah;
 	struct ibv_ah_attr ibv_ah_attr = { 0 };
 	struct efadv_ah_attr efa_ah_attr = { 0 };
 	int err;
 
+	assert(alloc_size >= sizeof(struct efa_ah));
+
 	efa_ah = NULL;
 
 	ofi_genlock_lock(&domain->util_domain.lock);
 	HASH_FIND(hh, domain->ah_map, gid, EFA_GID_LEN, efa_ah);
 	if (efa_ah) {
-		insert_implicit_av ? efa_ah->implicit_refcnt++ : efa_ah->explicit_refcnt++;
-		efa_ah_implicit_av_lru_ah_move(domain, efa_ah);
+		efa_ah->refcnt++;
 		ofi_genlock_unlock(&domain->util_domain.lock);
 		return efa_ah;
 	}
 
-	efa_ah = malloc(sizeof(struct efa_ah));
+	efa_ah = calloc(1, alloc_size);
 	if (!efa_ah) {
 		errno = FI_ENOMEM;
 		EFA_WARN(FI_LOG_AV, "cannot allocate memory for efa_ah\n");
+		ofi_genlock_unlock(&domain->util_domain.lock);
 		return NULL;
 	}
 
@@ -134,39 +84,13 @@ struct efa_ah *efa_ah_alloc(struct efa_domain *domain, const uint8_t *gid,
 	memcpy(ibv_ah_attr.grh.dgid.raw, gid, EFA_GID_LEN);
 	efa_ah->ibv_ah = ibv_create_ah(ibv_pd, &ibv_ah_attr);
 	if (!efa_ah->ibv_ah) {
-		/* If the failure is because we have too many AH entries, try to
-		 * evict an AH entry with no explicit AV entries and try AH
-		 * creation again */
-		if (errno == FI_ENOMEM) {
-			EFA_INFO(
-				FI_LOG_AV,
-				"ibv_create_ah failed with ENOMEM for implicit "
-				"AV insertion. Attempting to evict AH entry\n");
-
-			err = efa_ah_implicit_av_evict_ah(domain);
-			if (err)
-				goto err_free_efa_ah;
-
-			efa_ah->ibv_ah = ibv_create_ah(ibv_pd, &ibv_ah_attr);
-			if (!efa_ah->ibv_ah) {
-				if (errno == EINVAL) {
-					efa_ah_warn_create_einval(domain, gid);
-				} else {
-					EFA_WARN(FI_LOG_AV,
-						 "ibv_create_ah failed for implicit AV "
-						 "insertion! errno: %d\n",
-						 errno);
-				}
-				goto err_free_efa_ah;
-			}
-		} else if (errno == EINVAL) {
+		if (errno == EINVAL) {
 			efa_ah_warn_create_einval(domain, gid);
-			goto err_free_efa_ah;
 		} else {
 			EFA_WARN(FI_LOG_AV,
-				 "ibv_create_ah failed! errno: %s\n", strerror(errno));
-			goto err_free_efa_ah;
+				 "ibv_create_ah failed! errno: %d\n", errno);
 		}
+		goto err_free;
 	}
 
 	err = efadv_query_ah(efa_ah->ibv_ah, &efa_ah_attr, sizeof(efa_ah_attr));
@@ -176,11 +100,7 @@ struct efa_ah *efa_ah_alloc(struct efa_domain *domain, const uint8_t *gid,
 		goto err_destroy_ibv_ah;
 	}
 
-	dlist_init(&efa_ah->implicit_conn_list);
-	dlist_insert_tail(&efa_ah->domain_lru_ah_list_entry, &domain->ah_lru_list);
-	efa_ah->implicit_refcnt = 0;
-	efa_ah->explicit_refcnt = 0;
-	insert_implicit_av ? efa_ah->implicit_refcnt++ : efa_ah->explicit_refcnt++;
+	efa_ah->refcnt = 1;
 	efa_ah->ahn = efa_ah_attr.ahn;
 	memcpy(efa_ah->gid, gid, EFA_GID_LEN);
 	HASH_ADD(hh, domain->ah_map, gid, EFA_GID_LEN, efa_ah);
@@ -189,21 +109,27 @@ struct efa_ah *efa_ah_alloc(struct efa_domain *domain, const uint8_t *gid,
 
 err_destroy_ibv_ah:
 	ibv_destroy_ah(efa_ah->ibv_ah);
-err_free_efa_ah:
+err_free:
 	free(efa_ah);
 	ofi_genlock_unlock(&domain->util_domain.lock);
 	return NULL;
 }
 
-void efa_ah_destroy_ah(struct efa_domain *domain, struct efa_ah *ah)
+/**
+ * @brief destroy an efa_ah (remove from hash, destroy ibv_ah, free)
+ *
+ * Caller must hold util_domain.lock.
+ *
+ * @param[in]	domain	efa domain
+ * @param[in]	ah	efa_ah to destroy
+ */
+void efa_ah_destroy(struct efa_domain *domain, struct efa_ah *ah)
 {
 	int err;
 
-	assert(ah->implicit_refcnt == 0 && ah->explicit_refcnt == 0);
-	assert(dlist_empty(&ah->implicit_conn_list));
+	assert(ah->refcnt == 0);
 
 	EFA_INFO(FI_LOG_AV, "Destroying AH for ahn %d\n", ah->ahn);
-	dlist_remove(&ah->domain_lru_ah_list_entry);
 	HASH_DEL(domain->ah_map, ah);
 
 	err = ibv_destroy_ah(ah->ibv_ah);
@@ -213,29 +139,20 @@ void efa_ah_destroy_ah(struct efa_domain *domain, struct efa_ah *ah)
 }
 
 /**
- * @brief release an efa_ah object after acquiring the util domain lock
+ * @brief release an efa_ah, destroying it when refcount reaches zero
  *
- * @param[in]	domain	efa_domain
- * @param[in]	ah	efa_ah object pointer
+ * @param[in]	domain	efa domain
+ * @param[in]	ah	efa_ah to release
  */
-void efa_ah_release(struct efa_domain *domain, struct efa_ah *ah,
-		    bool release_from_implicit_av)
+void efa_ah_release(struct efa_domain *domain, struct efa_ah *ah)
 {
 	ofi_genlock_lock(&domain->util_domain.lock);
-#if ENABLE_DEBUG
-	struct efa_ah *tmp;
 
-	HASH_FIND(hh, domain->ah_map, ah->gid, EFA_GID_LEN, tmp);
-	assert(tmp == ah);
-#endif
-	assert((release_from_implicit_av && ah->implicit_refcnt > 0) ||
-	       (!release_from_implicit_av && ah->explicit_refcnt > 0));
+	assert(ah->refcnt > 0);
+	ah->refcnt--;
 
-	release_from_implicit_av ? ah->implicit_refcnt-- :
-				   ah->explicit_refcnt--;
+	if (ah->refcnt == 0)
+		efa_ah_destroy(domain, ah);
 
-	if (ah->implicit_refcnt == 0 && ah->explicit_refcnt == 0) {
-		efa_ah_destroy_ah(domain, ah);
-	}
 	ofi_genlock_unlock(&domain->util_domain.lock);
 }