Skip to content

prov/efa: Fail efa_rdm_ep_open when user passes FI_RX_CQ_DATA #11668

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
wants to merge 2 commits into
base: main
Choose a base branch
from
+5 −21
Open

prov/efa: Fail efa_rdm_ep_open when user passes FI_RX_CQ_DATA #11668

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91f08ff
prov/efa: Fail efa_rdm_ep_open when user passes FI_RX_CQ_DATA
jiaxiyan Nov 25, 2025
070fafe
prov/efa: Remove unused function efa_rdm_ep_should_write_rnr_completion
jiaxiyan Nov 25, 2025
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
21 changes: 0 additions & 21 deletions prov/efa/src/rdm/efa_rdm_ep.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,27 +273,6 @@ void efa_rdm_ep_post_internal_rx_pkts(struct efa_rdm_ep *ep);

int efa_rdm_ep_bulk_post_internal_rx_pkts(struct efa_rdm_ep *ep);

/**
* @brief return whether this endpoint should write error cq entry for RNR.
*
* For an endpoint to write RNR completion, two conditions must be met:
*
* First, the end point must be able to receive RNR completion from rdma-core,
* which means rnr_etry must be less then EFA_RNR_INFINITE_RETRY.
*
* Second, the app need to request this feature when opening endpoint
* (by setting info->domain_attr->resource_mgmt to FI_RM_DISABLED).
* The setting was saved as efa_rdm_ep->handle_resource_management.
*
* @param[in] ep endpoint
*/
static inline
bool efa_rdm_ep_should_write_rnr_completion(struct efa_rdm_ep *ep)
{
return (ep->base_ep.rnr_retry < EFA_RNR_INFINITE_RETRY) &&
(ep->handle_resource_management == FI_RM_DISABLED);
}

/*
* @brief: check whether we should use p2p for this transaction
*
Expand Down
5 changes: 5 additions & 0 deletions prov/efa/src/rdm/efa_rdm_ep_fiops.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,11 @@ int efa_rdm_ep_open(struct fid_domain *domain, struct fi_info *info,
int ret, retv, i;
enum fi_hmem_iface iface;

if (info->mode & FI_RX_CQ_DATA) {
Copy link
Contributor

@shijin-aws shijin-aws Nov 26, 2025

Choose a reason for hiding this comment

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

We shouldn't fail, we should ignore it

Copy link
Contributor

@bwbarrett bwbarrett Nov 26, 2025

Choose a reason for hiding this comment

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

Are you sure we should ignore it? We should definitely clear it on return from getinfo(). But at ep_open() point, it seems like the upper layer needs to know one way or the other and silently ignoring it seems wrong. But the man page is unhelpfully unclear here.

EFA_WARN(FI_LOG_EP_CTRL, "FI_RX_CQ_DATA is not supported\n");
return -FI_EINVAL;
}

efa_rdm_ep = calloc(1, sizeof(*efa_rdm_ep));
if (!efa_rdm_ep)
return -FI_ENOMEM;
Expand Down