Skip to content

nfs4: allow to disable open delegations per export - #177

Merged
kofemann merged 2 commits into
dCache:masterfrom
iterate-ch:nfs4-export-deleg-option
Sep 3, 2026
Merged

kofemann merged 2 commits into
dCache:masterfrom
iterate-ch:nfs4-export-deleg-option

Conversation

@ylangisc

Copy link
Copy Markdown
Contributor

Implements the export-option approach suggested in #176.

A new export option deleg/nodeleg (enabled by default) controls whether the server may hand out open delegations to clients matching that export:

/export  10.0.0.0/24(rw,deleg)  192.168.1.5(rw,nodeleg)

OperationOPEN consults the export the same way OperationLAYOUTGET consults pnfs, and passes the result to FileTracker#addOpen. The existing 5-argument addOpen signature is kept and defaults to allowing delegations, so nothing changes for existing deployments or for embedders calling it directly. Only the granting of new delegations is gated; recall, return and revocation are untouched.

Tests cover parsing of deleg / nodeleg / no_deleg and the default, plus the gating itself: with a callback channel present and OPEN4_SHARE_ACCESS_WANT_READ_DELEG set, no delegation is granted when the export disallows it.

Two points I would like your opinion on:

  • isDelegationAllowed() returns true when the context has no export table at all, which is a supported configuration for a pNFS data server (NFSServerV41 sets the DS role when _exportTable == null). isPnfsAllowed() can dereference the table unconditionally because LAYOUTGET is an MDS operation; OPEN is not, hence the extra check. Let me know if you would rather handle that differently.
  • Should the tracker resolve the export itself (e.g. by being constructed with the ExportTable) instead of having OperationOPEN pass the decision in? I kept it in the operation because that is where the other export options are evaluated, and because the export lookup then uses the address of the current connection rather than the one recorded at client creation.

Not addressed here: when a client explicitly asks for a delegation on a nodeleg export, the reply still reports WND4_NOT_WANTED / WND4_RESOURCE. Happy to add a more accurate why_no_delegation4 in a follow-up if you have a preference which one fits.

Motivation:
Since the adaptive delegation heuristic was added, the server offers read
delegations whenever a client has a working callback channel, and there is
no way for an administrator or embedder to turn that off. Not all clients
cope well with delegation recalls, and as delegations are voluntary on the
server side (rfc8881 10.2), whether to offer them is a deployment
decision. As it depends on the client, it belongs into the export
definition rather than into a global switch.

Modification:
add a 'deleg'/'nodeleg' export option, enabled by default, and consult it
in OperationOPEN the same way OperationLAYOUTGET consults 'pnfs'.
FileTracker#addOpen gets an overload taking the export's decision; the
existing signature is kept and defaults to allowing delegations, so the
behaviour of existing deployments and embedders is unchanged. Add tests
for parsing the option and for the gating in FileTracker.

Result:
open delegations can be disabled for selected clients, e.g.

    /export  10.0.0.0/24(rw,deleg)  192.168.1.5(rw,nodeleg)

Fixes: dCache#176
Target: master
Signed-off-by: Yves Langisch <yla@iterate.ch>
Comment thread core/src/main/java/org/dcache/nfs/v4/FileTracker.java Outdated
Motivation:
The previous commit kept the old five argument version of FileTracker#addOpen to stay backward compatible. As pointed out in the review, it has no callers outside of the tests, and the public API is explicitly unstable between minor releases.

Modification:
remove the old method and update the remaining callers in the unit tests and in FileTrackerBenchmark. Record the signature change in API-changes.md.

Result:
one way to register an open, no unused code path.

Suggested-by: Tigran Mkrtchyan
Target: master
Signed-off-by: Yves Langisch <yla@iterate.ch>
@kofemann
kofemann merged commit 981331d into dCache:master Sep 3, 2026
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