Bug 254560 - NFSv4.1 server broken when mounted by Linux using nconnects
Summary: NFSv4.1 server broken when mounted by Linux using nconnects
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 12.2-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: Rick Macklem
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-03-25 21:59 UTC by Rick Macklem
Modified: 2022-06-25 14:31 UTC (History)
5 users (show)

See Also:
rmacklem: mfc-stable13+
rmacklem: mfc-stable12+


Attachments
fix NFSv4.1 server so that it does not bind a new conn to backchannel (2.29 KB, patch)
2021-03-25 22:09 UTC, Rick Macklem
no flags Details | Diff
fix bindconnectiontosession so that it clears LCL_CBDOWN (462 bytes, patch)
2021-04-05 00:45 UTC, Rick Macklem
no flags Details | Diff
make server retry CB_RECALL every couple of seconds (3.08 KB, patch)
2021-04-05 01:25 UTC, Rick Macklem
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Macklem freebsd_committer freebsd_triage 2021-03-25 21:59:02 UTC

    
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2021-03-25 22:09:43 UTC
Created attachment 223584 [details]
fix NFSv4.1 server so that it does not bind a new conn to backchannel

The NFSv4.1 (and 4.2 on 13) server incorrectly binds
a new TCP connection to the backchannel when first
used by an RPC with Sequence. RFC5661 specified that
it should only bind to the fore channel.

This was done because early clients (including FreeBSD)
did not do the required BindConnectionToSession RPC.

Unfortunately, this breaks the Linux client when the
"nconnects" mount option is used, since the server
may do a callback on the incorrect TCP connection.

This patch converts the server behaviour to that
required by the RFC.
Until this patch is applied to the server, the
"nconnects" mount option should not be used.
Comment 2 Juraj Lutter freebsd_committer freebsd_triage 2021-03-25 22:20:30 UTC
Could this also solve a situation where a NFSv4.2 mount on a Linux client will stall, recv-q on server will increase, requiring nfsd to restart? NFSv4.1 client mounts are working OK.
Comment 3 Rick Macklem freebsd_committer freebsd_triage 2021-03-26 04:51:27 UTC
Unless you are using "nconnects" no, it is not
related. There is another issue that can happen
on both NFSv4.1 and NFSv4.2 mounts (NFSv4.2 is
only supported on main/13), where the TCP
connection ends up partially shut down.
(netstat -a shows it in CLOSE_WAIT.)
I have a patch that might fix this, but
that is a separate issue.
I'll be creating a PR for that one soon.
Comment 4 commit-hook freebsd_committer freebsd_triage 2021-03-30 22:32:52 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=01ae8969a9eed652fbd894faa5b31b1593079ed8

commit 01ae8969a9eed652fbd894faa5b31b1593079ed8
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-03-30 21:31:05 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-03-30 21:31:05 +0000

    nfsd: do not implicitly bind the back channel for NFSv4.1/4.2 mounts

    The NFSv4.1 (and 4.2 on 13) server incorrectly binds
    a new TCP connection to the back channel when first
    used by an RPC with a Sequence op in it (almost all of them).
    RFC5661 specifies that only the fore channel should be bound.

    This was done because early clients (including FreeBSD)
    did not do the required BindConnectionToSession RPC.

    Unfortunately, this breaks the Linux client when the
    "nconnects" mount option is used, since the server
    may do a callback on the incorrect TCP connection.

    This patch converts the server behaviour to that
    required by the RFC.  It also makes the server test/indicate
    failure of the back channel more aggressively.

    Until this patch is applied to the server, the
    "nconnects" mount option is not recommended for a Linux
    NFSv4.1/4.2 client mount to the FreeBSD server.

    Reported by:    bcodding@redhat.com
    Tested by:      bcodding@redhat.com
    PR:             254560
    MFC after:      1 week

 sys/fs/nfsserver/nfs_nfsdstate.c | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)
Comment 5 Rick Macklem freebsd_committer freebsd_triage 2021-03-30 22:49:57 UTC
The patch has now been committed to main and will be MFC'd
in a week.

I might request an errata update for this from re@.

I will not close this PR until the patch is in releases,
so that it can more easily found.
Comment 6 Rick Macklem freebsd_committer freebsd_triage 2021-04-05 00:45:45 UTC
Created attachment 223813 [details]
fix bindconnectiontosession so that it clears LCL_CBDOWN

The back channel is needed for two cases for NFSv4.1/4.2:
- the NFS server has delegations enabled
- the NFS server is a pNFS configuration
--> Neither of these are common, so most don't need
    this patch.
    It is also only needed for Linux NFSv4.1/4.2 mounts.

After a network partitioning that has caused a TCP connection
reset, the server will not be able to do callbacks and will
indicate this via the NFSV4SEQ_CBPATHDOWN reply flag on Sequence.
--> This triggers the Linux client to do a BindConnectionToSession,
    but without this patch, the client will repeat this until an
    Open notices that the back channel is again working.

This patch clears LCL_CBDOWN, assuming that the BindConnectionToSession
worked, until tested at the next Open, avoiding making the
Linux client do many BindConnectionToSession RPCs.
Comment 7 Rick Macklem freebsd_committer freebsd_triage 2021-04-05 01:25:10 UTC
Created attachment 223814 [details]
make server retry CB_RECALL every couple of seconds

The Linux NFS client will do a
BindConnectionToSession when it sees NFSV4SEQ_CBPATHDOWN
set in a sequence reply. This will fix the back channel, but the
first attempt at a callback like CB_RECALL will already have
failed. Without this patch, a CB_RECALL will not be retried
and that can result in a 5 minute delay until the delegation
times out.

This patch modifies the code so that it will retry the
CB_RECALL every couple of seconds, often avoiding the
5 minute delay.

This is not critical for correct behaviour, but avoids
the 5 minute delay for the case where the Linux client
re-binds the back channel via BindConnectionToSession.
Comment 8 commit-hook freebsd_committer freebsd_triage 2021-04-11 22:46:49 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5fb6cfadcd9dbc850a018a3690a2b775c01fff8f

commit 5fb6cfadcd9dbc850a018a3690a2b775c01fff8f
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-03-30 21:31:05 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-04-11 22:43:20 +0000

    nfsd: do not implicitly bind the back channel for NFSv4.1/4.2 mounts

    The NFSv4.1 (and 4.2 on 13) server incorrectly binds
    a new TCP connection to the back channel when first
    used by an RPC with a Sequence op in it (almost all of them).
    RFC5661 specifies that only the fore channel should be bound.

    This was done because early clients (including FreeBSD)
    did not do the required BindConnectionToSession RPC.

    Unfortunately, this breaks the Linux client when the
    "nconnects" mount option is used, since the server
    may do a callback on the incorrect TCP connection.

    This patch converts the server behaviour to that
    required by the RFC.  It also makes the server test/indicate
    failure of the back channel more aggressively.

    Until this patch is applied to the server, the
    "nconnects" mount option is not recommended for a Linux
    NFSv4.1/4.2 client mount to the FreeBSD server.

    PR:             254560
    (cherry picked from commit 01ae8969a9eed652fbd894faa5b31b1593079ed8)

 sys/fs/nfsserver/nfs_nfsdstate.c | 33 ++++-----------------------------
 1 file changed, 4 insertions(+), 29 deletions(-)
Comment 9 Rick Macklem freebsd_committer freebsd_triage 2022-06-25 14:31:34 UTC
Patch is now in FreeBSD 12.3 and 13.1, as well as
being MFC'd to stable/12 and stable/13.