Repository navigation
libfabric: move provider-specific settings into a provider table - #15
Draft
wpower-work wants to merge 2 commits into
Draft
wpower-work wants to merge 2 commits into
wpower-work wants to merge 2 commits into
Conversation
Provider-specific behaviour was spread across device discovery, rail setup and topology discovery as `provider == "..."` comparisons. Collect it in one table, nixlLibfabricProviderInfo, returned by knownProviders() in preference order (cxi, efa, tcp, sockets, unchanged). Each entry holds the rail hints (extra caps, mr_mode, mr_key_size), whether the provider gets full topology discovery, and whether it uses a single domain. A provider without an entry gets the efa defaults, as before. Provider selection is factored into selectProvider(), so it can be unit tested against synthetic fi_getinfo results. Device discovery now keeps each selected domain's PCIe address and link speed, and the topology's PCIe mapping uses that instead of a second fi_getinfo() with different hints. Small behaviour changes: - has_pcie_devices_ is set only when a NIC's PCIe address resolves. - The missing-link-speed warning fires when the speed is 0, not only when nic->link_attr is null. - A provider that does topology-based rail selection but reports no PCIe addresses now logs a warning, since every accelerator then uses every rail. Adds libfabric_provider_test for baseProviderName(), the provider table and selectProvider().
|
👋 Hi wpower-work! Thank you for contributing to intel-staging/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
There was a problem hiding this comment.
🟡 Changes recommended
The domain-only cache key can mix metadata from different providers, causing topology to associate a selected rail with the wrong NIC or bandwidth.
1 open issue
What changed in this PR
This PR centralizes libfabric provider settings to simplify support for additional GPU vendors while preserving provider preference order.
Changes:
- Moves provider selection, rail hints, and topology flags into a shared provider table.
- Reuses discovered PCIe addresses and link speeds for topology mapping.
- Adds provider-selection tests and documents how to add providers.
| File | Description |
|---|---|
test/unit/utils/libfabric/meson.build |
Registers the provider unit test. |
test/unit/utils/libfabric/libfabric_provider_test.cpp |
Tests provider names, settings, and selection. |
src/utils/libfabric/libfabric_topology.h |
Stores discovered domain metadata. |
src/utils/libfabric/libfabric_topology.cpp |
Uses cached metadata and table-driven topology flags. |
src/utils/libfabric/libfabric_rail.h |
Clarifies domain and provider field comments. |
src/utils/libfabric/libfabric_rail.cpp |
Reads rail hints from the provider table. |
src/utils/libfabric/libfabric_common.h |
Declares provider settings and device metadata APIs. |
src/utils/libfabric/libfabric_common.cpp |
Implements the table, selection helpers, and metadata capture. |
src/plugins/libfabric/README.md |
Documents provider settings and extension steps. |
Review effort: Balanced
provider name in cache lookup key.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Provider-specific behaviur was spread across device discovery, rail setup and topology discovery as
provider == "..."comparisons. This PR refactors the provider-specific functionality into a common lookup table.Why?
This change is needed to simplify adding support for additional GPU vendors. Intel will be adding support for their GPU in a future PR that uses the new lookup table.
How?
Provider-specific behavior was spread across device discovery, rail setup and topology discovery as
provider == "..."comparisons. Collect it in one table, nixlLibfabricProviderInfo, returned by knownProviders() in preference order (cxi, efa, tcp, sockets, unchanged). Each entry holds the rail hints (extra caps, mr_mode, mr_key_size), whether the provider gets full topology discovery, and whether it uses a single domain. A provider without an entry gets the efa defaults, as before.Provider selection is factored into selectProvider(), so it can be unit tested against synthetic fi_getinfo results. Device discovery now keeps each selected domain's PCIe address and link speed, and the topology's PCIe mapping uses that instead of a second fi_getinfo() with different hints.
Small behaviour changes:
Adds libfabric_provider_test for baseProviderName(), the provider table and selectProvider().