Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
a-szegel wants to merge 16 commits into
base: main
Choose a base branch
from
+3,830 −2,203
Open

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
73b570d
prov/efa: add efa_av_is_valid_address helper function
a-szegel Apr 13, 2026
6b89ad2
prov/efa: add efa_proto_av header with struct definitions
a-szegel Apr 9, 2026
12f00c7
prov/efa: add context_len parameter to efa_av_init_util_av
a-szegel May 5, 2026
e66e642
prov/efa: add efa_av_addr_to_entry and efa_av_entry_ep_addr helpers
a-szegel May 5, 2026
bd5b66b
prov/efa: migrate base path callers to efa_av_addr_to_entry
a-szegel May 5, 2026
0eb01d7
prov/efa: change efa_av_reverse_av_add/_remove signature to take efa_…
a-szegel May 5, 2026
c4743cf
prov/efa: add efa_proto_av implementation
a-szegel May 5, 2026
5059ae0
prov/efa: use FI_ADDR_NOTAVAIL for empty fi_addr slot check
a-szegel May 5, 2026
9c35b0c
prov/efa: wire efa_proto_av into RDM, switch peer backing to efa_prot…
a-szegel May 5, 2026
027f6ca
prov/efa: release util_domain.lock on malloc failure in efa_ah_alloc
a-szegel May 6, 2026
65f7055
prov/efa: fix AH layering — move eviction to efa_proto_av.c
a-szegel May 6, 2026
5415fed
prov/efa: split efa_ah into base and protocol layers
a-szegel May 6, 2026
451a3a7
prov/efa: strip efa_av to base-only
a-szegel May 6, 2026
7af9f8a
prov/efa: delete efa_conn.c and efa_conn.h
a-szegel May 6, 2026
2b8e742
prov/efa/test: split AV tests into base and protocol files
a-szegel Apr 10, 2026
3e8d130
prov/efa/test: add new unit tests for AV coverage
a-szegel Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions libfabric.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,7 @@
<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" />
Expand Down Expand Up @@ -1019,6 +1020,7 @@
<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" />
Expand Down
2 changes: 2 additions & 0 deletions prov/efa/Makefile.include
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ _efa_files = \
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 \
Expand Down Expand Up @@ -117,6 +118,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 \
Expand Down
1 change: 1 addition & 0 deletions prov/efa/src/efa.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
103 changes: 60 additions & 43 deletions prov/efa/src/efa_av.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,29 @@ struct efa_conn *efa_av_addr_to_conn_implicit(struct efa_av *av, fi_addr_t fi_ad
return efa_av_addr_to_conn_impl(&av->util_av_implicit, fi_addr);
}

/**
* @brief Look up an efa_av_entry by fi_addr in the base (explicit) AV
*
* Wrapper around efa_av_addr_to_conn that returns the containing
* efa_av_entry via container_of. Exposed as the base-layer lookup
* primitive for callers that need to work with efa_av_entry rather
* than the embedded efa_conn.
*
* @param[in] av address vector
* @param[in] fi_addr libfabric address
* @return pointer to efa_av_entry, or NULL if not found
*/
struct efa_av_entry *efa_av_addr_to_entry(struct efa_av *av, fi_addr_t fi_addr)
{
struct efa_conn *conn;

conn = efa_av_addr_to_conn(av, fi_addr);
if (!conn)
return NULL;

return container_of(conn, struct efa_av_entry, conn);
}

/**
* @brief find fi_addr for efa endpoint
*
Expand All @@ -76,7 +99,7 @@ fi_addr_t efa_av_reverse_lookup(struct efa_av *av, uint16_t ahn, uint16_t qpn)
cur_key.qpn = qpn;
HASH_FIND(hh, av->cur_reverse_av, &cur_key, sizeof(cur_key), cur_entry);

return (OFI_LIKELY(!!cur_entry)) ? cur_entry->conn->fi_addr : FI_ADDR_NOTAVAIL;
return (OFI_LIKELY(!!cur_entry)) ? cur_entry->av_entry->conn.fi_addr : FI_ADDR_NOTAVAIL;
}

static inline struct efa_conn *
Expand Down Expand Up @@ -106,7 +129,7 @@ efa_av_reverse_lookup_rdm_conn(struct efa_cur_reverse_av **cur_reverse_av,
* the pkt_entry is allocated from a buffer user posted that
* doesn't expect any pkt hdr.
*/
return cur_entry->conn;
return &cur_entry->av_entry->conn;
}

connid = efa_rdm_pke_connid_ptr(pkt_entry);
Expand All @@ -119,11 +142,11 @@ efa_av_reverse_lookup_rdm_conn(struct efa_cur_reverse_av **cur_reverse_av,
"The communication can continue but it is "
"encouraged to use\n"
"a newer version of libfabric\n");
return cur_entry->conn;
return &cur_entry->av_entry->conn;
}

if (OFI_LIKELY(*connid == cur_entry->conn->ep_addr->qkey))
return cur_entry->conn;
if (OFI_LIKELY(*connid == efa_av_entry_ep_addr(cur_entry->av_entry)->qkey))
return &cur_entry->av_entry->conn;

/* the packet is from a previous peer, look for its address from the
* prv_reverse_av */
Expand All @@ -132,7 +155,7 @@ efa_av_reverse_lookup_rdm_conn(struct efa_cur_reverse_av **cur_reverse_av,
prv_key.connid = *connid;
HASH_FIND(hh, *prv_reverse_av, &prv_key, sizeof(prv_key), prv_entry);

return OFI_LIKELY(!!prv_entry) ? prv_entry->conn : NULL;
return OFI_LIKELY(!!prv_entry) ? &prv_entry->av_entry->conn : NULL;
};

/**
Expand Down Expand Up @@ -189,13 +212,6 @@ fi_addr_t efa_av_reverse_lookup_rdm_implicit(struct efa_av *av, uint16_t ahn,
return FI_ADDR_NOTAVAIL;
}

static inline int efa_av_is_valid_address(struct efa_ep_addr *addr)
{
struct efa_ep_addr all_zeros = { 0 };

return memcmp(addr->raw, all_zeros.raw, sizeof(addr->raw));
}

/**
* @brief Move the conn to the front of the LRU list to indicate that it is the
* most recently used entry
Expand Down Expand Up @@ -224,22 +240,22 @@ void efa_av_implicit_av_lru_conn_move(struct efa_av *av,
* @param[in] av EFA AV object
* @param[in,out] cur_reverse_av Reverse AV with AHN and QPN as key
* @param[in,out] prv_reverse_av Reverse AV with AHN, QPN and QKEY as key
* @param[in] conn efa_conn object
* @param[in] av_entry efa_av_entry object
* @return On success, return 0.
* Otherwise, return a negative libfabric error code
*/
int efa_av_reverse_av_add(struct efa_av *av,
struct efa_cur_reverse_av **cur_reverse_av,
struct efa_prv_reverse_av **prv_reverse_av,
struct efa_conn *conn)
struct efa_av_entry *av_entry)
{
struct efa_cur_reverse_av *cur_entry;
struct efa_prv_reverse_av *prv_entry;
struct efa_cur_reverse_av_key cur_key;

memset(&cur_key, 0, sizeof(cur_key));
cur_key.ahn = conn->ah->ahn;
cur_key.qpn = conn->ep_addr->qpn;
cur_key.ahn = av_entry->conn.ah->ahn;
cur_key.qpn = efa_av_entry_ep_addr(av_entry)->qpn;
cur_entry = NULL;

HASH_FIND(hh, *cur_reverse_av, &cur_key, sizeof(cur_key), cur_entry);
Expand All @@ -252,7 +268,7 @@ int efa_av_reverse_av_add(struct efa_av *av,

cur_entry->key.ahn = cur_key.ahn;
cur_entry->key.qpn = cur_key.qpn;
cur_entry->conn = conn;
cur_entry->av_entry = av_entry;
HASH_ADD(hh, *cur_reverse_av, key, sizeof(cur_key), cur_entry);

return 0;
Expand All @@ -270,11 +286,11 @@ int efa_av_reverse_av_add(struct efa_av *av,

prv_entry->key.ahn = cur_key.ahn;
prv_entry->key.qpn = cur_key.qpn;
prv_entry->key.connid = cur_entry->conn->ep_addr->qkey;
prv_entry->conn = cur_entry->conn;
prv_entry->key.connid = efa_av_entry_ep_addr(cur_entry->av_entry)->qkey;
prv_entry->av_entry = cur_entry->av_entry;
HASH_ADD(hh, *prv_reverse_av, key, sizeof(prv_entry->key), prv_entry);

cur_entry->conn = conn;
cur_entry->av_entry = av_entry;
return 0;
}

Expand All @@ -285,39 +301,36 @@ int efa_av_reverse_av_add(struct efa_av *av,
* cur_reverse_av. Keeping the address in prv_reverse_av helps avoid QPN
* collisions.
*
* @param[in] av EFA AV object
* @param[in,out] cur_reverse_av Reverse AV with AHN and QPN as key
* @param[in,out] prv_reverse_av Reverse AV with AHN, QPN and QKEY as key
* @param[in] conn efa_conn object
* @return On success, return 0.
* Otherwise, return a negative libfabric error code
* @param[in] av_entry efa_av_entry object
*/
void efa_av_reverse_av_remove(struct efa_cur_reverse_av **cur_reverse_av,
struct efa_prv_reverse_av **prv_reverse_av,
struct efa_conn *conn)
struct efa_av_entry *av_entry)
{
struct efa_cur_reverse_av *cur_reverse_av_entry;
struct efa_prv_reverse_av *prv_reverse_av_entry;
struct efa_cur_reverse_av_key cur_key;
struct efa_prv_reverse_av_key prv_key;

memset(&cur_key, 0, sizeof(cur_key));
cur_key.ahn = conn->ah->ahn;
cur_key.qpn = conn->ep_addr->qpn;
cur_key.ahn = av_entry->conn.ah->ahn;
cur_key.qpn = efa_av_entry_ep_addr(av_entry)->qpn;
HASH_FIND(hh, *cur_reverse_av, &cur_key, sizeof(cur_key),
cur_reverse_av_entry);
if (cur_reverse_av_entry && cur_reverse_av_entry->conn == conn) {
if (cur_reverse_av_entry && cur_reverse_av_entry->av_entry == av_entry) {
HASH_DEL(*cur_reverse_av, cur_reverse_av_entry);
free(cur_reverse_av_entry);
} else {
memset(&prv_key, 0, sizeof(prv_key));
prv_key.ahn = conn->ah->ahn;
prv_key.qpn = conn->ep_addr->qpn;
prv_key.connid = conn->ep_addr->qkey;
prv_key.ahn = av_entry->conn.ah->ahn;
prv_key.qpn = efa_av_entry_ep_addr(av_entry)->qpn;
prv_key.connid = efa_av_entry_ep_addr(av_entry)->qkey;
HASH_FIND(hh, *prv_reverse_av, &prv_key, sizeof(prv_key),
prv_reverse_av_entry);
assert(prv_reverse_av_entry &&
prv_reverse_av_entry->conn == conn);
prv_reverse_av_entry->av_entry == av_entry);
HASH_DEL(*prv_reverse_av, prv_reverse_av_entry);
free(prv_reverse_av_entry);
}
Expand Down Expand Up @@ -407,7 +420,7 @@ static int efa_conn_implicit_to_explicit(struct efa_av *av,

/* Handle reverse AV and AV ref counts */
efa_av_reverse_av_remove(&av->cur_reverse_av_implicit,
&av->prv_reverse_av_implicit, implicit_conn);
&av->prv_reverse_av_implicit, implicit_av_entry);

dlist_remove(&implicit_av_entry->conn.implicit_av_lru_entry);

Expand All @@ -423,7 +436,7 @@ static int efa_conn_implicit_to_explicit(struct efa_av *av,
av->used_implicit--;

err = efa_av_reverse_av_add(av, &av->cur_reverse_av, &av->prv_reverse_av,
explicit_conn);
explicit_av_entry);
if (err)
return err;

Expand Down Expand Up @@ -751,23 +764,23 @@ static void efa_av_close_reverse_av(struct efa_av *av)
ofi_genlock_lock(&av->util_av.lock);

HASH_ITER(hh, av->cur_reverse_av, cur_entry, curtmp) {
efa_conn_release(av, cur_entry->conn, false);
efa_conn_release(av, &cur_entry->av_entry->conn, false);
}

HASH_ITER(hh, av->prv_reverse_av, prv_entry, prvtmp) {
efa_conn_release(av, prv_entry->conn, false);
efa_conn_release(av, &prv_entry->av_entry->conn, false);
}

ofi_genlock_unlock(&av->util_av.lock);

ofi_genlock_lock(&av->util_av_implicit.lock);

HASH_ITER(hh, av->cur_reverse_av_implicit, cur_entry, curtmp) {
efa_conn_release(av, cur_entry->conn, true);
efa_conn_release(av, &cur_entry->av_entry->conn, true);
}

HASH_ITER(hh, av->prv_reverse_av_implicit, prv_entry, prvtmp) {
efa_conn_release(av, prv_entry->conn, true);
efa_conn_release(av, &prv_entry->av_entry->conn, true);
}

ofi_genlock_unlock(&av->util_av_implicit.lock);
Expand Down Expand Up @@ -833,18 +846,20 @@ static struct fi_ops efa_av_fi_ops = {
* @param[in] attr AV attr application passed to fi_av_open
* @param[out] util_av util_av field in efa_av
* @param[in] context contexted application passed to fi_av_open
* @param[in] context_len size of provider-specific context per AV entry
* @return On success, return 0.
* On failure, return a negative libfabric error code.
*/
int efa_av_init_util_av(struct efa_domain *efa_domain,
struct fi_av_attr *attr,
struct util_av *util_av,
void *context)
void *context,
size_t context_len)
{
struct util_av_attr util_attr;

util_attr.addrlen = EFA_EP_ADDR_LEN;
util_attr.context_len = sizeof(struct efa_av_entry) - EFA_EP_ADDR_LEN;
util_attr.context_len = context_len;
util_attr.flags = 0;
return ofi_av_init(&efa_domain->util_domain, attr, &util_attr,
util_av, context);
Expand Down Expand Up @@ -894,11 +909,13 @@ int efa_av_open(struct fid_domain *domain_fid, struct fi_av_attr *attr,
&universe_size) == FI_SUCCESS)
attr->count = MAX(attr->count, universe_size);

ret = efa_av_init_util_av(efa_domain, attr, &av->util_av_implicit, context);
ret = efa_av_init_util_av(efa_domain, attr, &av->util_av_implicit, context,
sizeof(struct efa_av_entry) - EFA_EP_ADDR_LEN);
if (ret)
goto err;

ret = efa_av_init_util_av(efa_domain, attr, &av->util_av, context);
ret = efa_av_init_util_av(efa_domain, attr, &av->util_av, context,
sizeof(struct efa_av_entry) - EFA_EP_ADDR_LEN);
if (ret)
goto err_close_util_av_implicit;

Expand Down
42 changes: 37 additions & 5 deletions prov/efa/src/efa_av.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,38 @@ struct efa_av_entry {
struct efa_conn conn;
};

/**
* @brief typed accessor for the ep_addr field of an AV entry
*
* @param[in] entry AV entry
* @return pointer to the efa_ep_addr embedded in the entry
*/
static inline struct efa_ep_addr *efa_av_entry_ep_addr(struct efa_av_entry *entry)
{
return (struct efa_ep_addr *)entry->ep_addr;
}

/**
* @brief check if an efa_ep_addr has a non-zero GID
*
* @param[in] addr address to check
* @return non-zero if valid, 0 if all-zeros
*/
static inline int efa_av_is_valid_address(struct efa_ep_addr *addr)
Copy link
Copy Markdown
Contributor

@sunkuamzn sunkuamzn May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message says "add" but it's really being moved

{
struct efa_ep_addr all_zeros = { 0 };

return memcmp(addr->raw, all_zeros.raw, sizeof(addr->raw));
}

struct efa_cur_reverse_av_key {
uint16_t ahn;
uint16_t qpn;
};

struct efa_cur_reverse_av {
struct efa_cur_reverse_av_key key;
struct efa_conn *conn;
struct efa_av_entry *av_entry;
UT_hash_handle hh;
};

Expand All @@ -54,7 +78,7 @@ struct efa_prv_reverse_av_key {

struct efa_prv_reverse_av {
struct efa_prv_reverse_av_key key;
struct efa_conn *conn;
struct efa_av_entry *av_entry;
UT_hash_handle hh;
};

Expand Down Expand Up @@ -108,13 +132,21 @@ fi_addr_t efa_av_reverse_lookup(struct efa_av *av, uint16_t ahn, uint16_t qpn);
int efa_av_reverse_av_add(struct efa_av *av,
struct efa_cur_reverse_av **cur_reverse_av,
struct efa_prv_reverse_av **prv_reverse_av,
struct efa_conn *conn);
struct efa_av_entry *av_entry);

void efa_av_reverse_av_remove(struct efa_cur_reverse_av **cur_reverse_av,
struct efa_prv_reverse_av **prv_reverse_av,
struct efa_conn *conn);
struct efa_prv_reverse_av **prv_reverse_av,
struct efa_av_entry *av_entry);

void efa_av_implicit_av_lru_conn_move(struct efa_av *av,
struct efa_conn *conn);

struct efa_av_entry *efa_av_addr_to_entry(struct efa_av *av, fi_addr_t fi_addr);

int efa_av_init_util_av(struct efa_domain *efa_domain,
struct fi_av_attr *attr,
struct util_av *util_av,
void *context,
size_t context_len);

#endif
Loading