Skip to content
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.

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

prov/efa: Make DGRAM provider use new av_entry struct #10706

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

a-szegel
Copy link
Contributor

Update the DGRAM provider to use the new efa_base_av_entry structure. This change splits the AV logic for the DGRAM and the RDM providers. Splitting the av logic for DGRAM makes DGRAM's AV logic simpler and faster in a few ways.

  1. This change removes the efa_con struct from the av_entry because the DGRAM protocol does not maintain an idea of a connection to the endpoint receiving the messages. It does not matter if the remote endpoint we were previously communicating with exits, and a new QP opens with the same AH (NIC), and QPN (Endpoint). As long as someone is there to post recv buffers for our messages, DGRAM provider is happy.

  2. This changes gets rid of the current/prev av reverse lookup map (ahn/qpn -> av_entry), and creates a single reverse look up map because the dgram provider does not care about old connections.

@a-szegel a-szegel force-pushed the modify-efa-dgram-av-path branch from c686ee3 to d7a715b Compare January 16, 2025 16:35
@a-szegel
Copy link
Contributor Author

This needs unit testing, but it should be ready to review.

@a-szegel a-szegel marked this pull request as ready for review January 16, 2025 16:36
@a-szegel a-szegel requested review from sunkuamzn and a team January 16, 2025 16:36
struct efa_ep_addr is used in both the DGRAM and RDM providers, so the
structure definition should be in a common file, and not an RDM only
file.

Signed-off-by: Seth Zegelstein <[email protected]>
Update the DGRAM provider to use the new efa_base_av_entry structure.
This change splits the AV logic for the DGRAM and the RDM providers.
Splitting the av logic for DGRAM makes DGRAM's AV logic simpler and
faster in a few ways.

1. This change removes the efa_con struct from the av_entry because the
   DGRAM protocol does not maintain an idea of a connection to the
   endpoint receiving the messages. It does not matter if the remote
   endpoint we were previously communicating with exits, and a new QP
   opens with the same AH (NIC), and QPN (Endpoint). As long as someone
   is there to post recv buffers for our messages, DGRAM provider is
   happy.

2. This changes gets rid of the current/prev av reverse lookup map
   (ahn/qpn -> av_entry), and creates a single reverse look up map
   because the dgram provider does not care about old connections.

Signed-off-by: Seth Zegelstein <[email protected]>
@a-szegel a-szegel force-pushed the modify-efa-dgram-av-path branch from d7a715b to c3d1cd4 Compare January 16, 2025 16:48
@shijin-aws
Copy link
Contributor

A high level comment is that I want to get rid of dgram. I prefer to use efa_av prefix for the code path that doesn't involve the rdm protocols. And use efa_rdm_av prefix for the protocol path.

@shijin-aws
Copy link
Contributor

I am wondering if it is cleaner to decouple rdm_peer with efa_conn. As far as I read the code you only access peer from coon during the av insert and remove path. You don't need rdm peer for reverse look up. It should be much less changes if you do in this way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants