Discovery walk follow-ups: entry limit and leaf-priority ordering
Two follow-ups from the discovery walk rewrite review (PR #3837), deliberately left out of that PR since neither is spec-mandated and both need more discussion. Filed as one issue since they're related (both about how _nvmf_discover() spends its time/budget), but separable in implementation.
Note: referral depth interpretation is settled
Not a topic for this issue, recorded here for reference since it came up in the same PR discussion: the implemented 8-level referral depth cap (NVMF_MAX_REFERRAL_DEPTH) treats the primary DC as level 0, allowing up to 8 further levels of referrals past it — i.e., 8 hops past the primary, not 8 total nodes including the primary. We confirmed this reading directly with Erik Smith, a contributor to the NVMe base specs. Worth having on record, to avoid misinterpretation.
1. Configurable limit on total Discovery Log Page entries processed
Nothing today bounds how many entries a single Discovery Log Page can report, or how many get processed across a whole walk. numrec (struct nvmf_discovery_log) is a 64-bit field, and each entry (struct nvmf_disc_log_entry) is a fixed 1024 bytes — so a misconfigured or malicious DC can hand back an arbitrarily large page. The existing 8-level referral depth cap bounds how deep the walk goes, but says nothing about how wide a single DC's own page can be.
Open questions, not yet settled:
- Where does the limit apply? Per-DC (cap how many entries of one DC's own page get processed) and/or cumulative across the whole walk (cap the running total before recursing into the next referral, the same point
NVMF_MAX_REFERRAL_DEPTH is already checked). A per-DC-only cap doesn't bound total fan-out; a cumulative-only cap doesn't stop one huge page from being fully parsed before the check ever fires. Likely needs both, sharing one counter.
- Pre, in, or post enforcement relative to the fetch? Cap the number of entries actually requested from the DC (via
LPOL/LPOU offset addressing — a partial read is spec-legal), cap how many of an already-fetched page get iterated/connected, or some combination.
- Configurable how? A CLI flag, an INI key, or both. Whether it needs a sensible non-zero default or defaults to "unlimited" (today's behavior) for backward compatibility.
2. Prioritize a DC's own leaf entries over its own referrals
_nvmf_discover()'s pass-2 loop (fabrics.c) processes one DC's DLP entries in raw array order. Since a referral entry recurses immediately and doesn't return until that whole referral subtree (up to 8 levels) is walked, a referral entry that happens to sit before some I/O controller entries in the same page delays connecting that DC's own, more directly relevant controllers behind however deep the referral chain goes.
Proposal: within one DC's own DLP, process leaf (NVME_NQN_NVME) entries before referral (NVME_NQN_DISC) entries, regardless of array order — a DC's own self-reported controllers are first-hand information; a referral is telling you about a DC you haven't talked to yet, one hop more indirect. A two-pass split of the existing pass-2 loop, no new data structures, no change to the depth cap or dc_visited.
This is deliberately narrower than a full BFS rewrite of the walk (raised in an offline discussion between Daniel and Martin). Switching the whole walk from DFS to BFS is a bigger, separate question — different recursion model, different interaction with parent_eflags fallback and dc_visited ordering — and not needed to fix the specific starvation problem above.
Relevant code
Discovery walk follow-ups: entry limit and leaf-priority ordering
Two follow-ups from the discovery walk rewrite review (PR #3837), deliberately left out of that PR since neither is spec-mandated and both need more discussion. Filed as one issue since they're related (both about how
_nvmf_discover()spends its time/budget), but separable in implementation.Note: referral depth interpretation is settled
Not a topic for this issue, recorded here for reference since it came up in the same PR discussion: the implemented 8-level referral depth cap (
NVMF_MAX_REFERRAL_DEPTH) treats the primary DC as level 0, allowing up to 8 further levels of referrals past it — i.e., 8 hops past the primary, not 8 total nodes including the primary. We confirmed this reading directly with Erik Smith, a contributor to the NVMe base specs. Worth having on record, to avoid misinterpretation.1. Configurable limit on total Discovery Log Page entries processed
Nothing today bounds how many entries a single Discovery Log Page can report, or how many get processed across a whole walk.
numrec(struct nvmf_discovery_log) is a 64-bit field, and each entry (struct nvmf_disc_log_entry) is a fixed 1024 bytes — so a misconfigured or malicious DC can hand back an arbitrarily large page. The existing 8-level referral depth cap bounds how deep the walk goes, but says nothing about how wide a single DC's own page can be.Open questions, not yet settled:
NVMF_MAX_REFERRAL_DEPTHis already checked). A per-DC-only cap doesn't bound total fan-out; a cumulative-only cap doesn't stop one huge page from being fully parsed before the check ever fires. Likely needs both, sharing one counter.LPOL/LPOUoffset addressing — a partial read is spec-legal), cap how many of an already-fetched page get iterated/connected, or some combination.2. Prioritize a DC's own leaf entries over its own referrals
_nvmf_discover()'s pass-2 loop (fabrics.c) processes one DC's DLP entries in raw array order. Since a referral entry recurses immediately and doesn't return until that whole referral subtree (up to 8 levels) is walked, a referral entry that happens to sit before some I/O controller entries in the same page delays connecting that DC's own, more directly relevant controllers behind however deep the referral chain goes.Proposal: within one DC's own DLP, process leaf (
NVME_NQN_NVME) entries before referral (NVME_NQN_DISC) entries, regardless of array order — a DC's own self-reported controllers are first-hand information; a referral is telling you about a DC you haven't talked to yet, one hop more indirect. A two-pass split of the existing pass-2 loop, no new data structures, no change to the depth cap ordc_visited.This is deliberately narrower than a full BFS rewrite of the walk (raised in an offline discussion between Daniel and Martin). Switching the whole walk from DFS to BFS is a bigger, separate question — different recursion model, different interaction with
parent_eflagsfallback anddc_visitedordering — and not needed to fix the specific starvation problem above.Relevant code
libnvme/src/nvme/fabrics.c:_nvmf_discover(),dc_walk_referral(),NVMF_MAX_REFERRAL_DEPTH.