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: introduce FI_EFA_IFACE to restrict visible NICs #10210

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

wenduwan
Copy link
Contributor

This patch introduces an environment variable to limit which NICs are visible to the application via fi_getinfo.

@j-xiong
Copy link
Contributor

j-xiong commented Jul 23, 2024

Could it be named FI_EFA_IFACE to be consistent with other providers?

@shijin-aws
Copy link
Contributor

Could it be named FI_EFA_IFACE to be consistent with other providers?

Could you share a link to the implementation of other providers?

@j-xiong
Copy link
Contributor

j-xiong commented Jul 23, 2024

@wenduwan wenduwan changed the title prov/efa: introduce FI_EFA_NIC_NAMES to restrict visible NICs prov/efa: introduce FI_EFA_IFACE to restrict visible NICs Jul 23, 2024
@wenduwan
Copy link
Contributor Author

@j-xiong Thank you! PR is updated accordingly.

@wenduwan wenduwan requested a review from a team July 24, 2024 00:49
@wenduwan
Copy link
Contributor Author

@zachdworkin I imagine the intel CI failure is unrelated, correct?

@zachdworkin
Copy link
Contributor

@zachdworkin I imagine the intel CI failure is unrelated, correct?

It is unrelated. The opt-out path should have been taken since all of the changes in this PR are under prov/efa. If you want Intel-CI to opt-out in the future please rebase so that only the changes in your PR are the ones in the changeset in the comparison between the base branch you are pull requesting against and your changes.

@shijin-aws
Copy link
Contributor

Should we advertise this env in man page?

@j-xiong
Copy link
Contributor

j-xiong commented Jul 24, 2024

Yes, I think so.

This patch introduces an environment variable to limit NICs that are
visible to the application via fi_getinfo.

FI_EFA_IFACE accepts the following values:
- all(default): All NICs are visible to the application
- dev0[,dev1,...]: Only devices whose name exactly matches one of the
  provided values are visible to the application.

Signed-off-by: Wenduo Wang <[email protected]>
@wenduwan
Copy link
Contributor Author

Should we advertise this env in man page?

Good point. Added documentation.

@wenduwan wenduwan requested a review from shijin-aws July 25, 2024 18:35
if ((match > efa_env.iface && *(match - 1) != ',') ||
((match + strlen(name)) < end && *(match + strlen(name)) != ',')) {
/* Skip partial match */
match += strlen(name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Clarified with @wenduwan offline. But this logic is not intuitive to new readers.

@shijin-aws
Copy link
Contributor

bot:aws:retest

@shijin-aws shijin-aws merged commit fb5bc87 into ofiwg:main Jul 26, 2024
13 checks passed
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.

4 participants