nfscl: Simplify the deleg argument for nfscl_deleg()

The deleg argument to nfscl_deleg() is a "struct nfscldeleg **"
although the returned pointer value is never used by callers.

This patch changes

nfscl: Simplify the deleg argument for nfscl_deleg()

The deleg argument to nfscl_deleg() is a "struct nfscldeleg **"
although the returned pointer value is never used by callers.

This patch changes the argument to "struct nfscldeleg *" to
simplify the call and avoid any confusion w.r.t. use of the
returned value.

This patch should not create any NFS semantics change.

show more ...

nfscl: Add support for read delegations and atomic upgrade

For NFSv4.1/4.2, an atomic upgrade of a delegation from a
read delegation to a write delegation is allowed and can
result in significantly

nfscl: Add support for read delegations and atomic upgrade

For NFSv4.1/4.2, an atomic upgrade of a delegation from a
read delegation to a write delegation is allowed and can
result in significantly improved performance.
This patch adds this upgrade to the NFSv4.1/4.2 client and
enables use of read delegations.

For a test case of building a FreeBSD kernel (sources and
output objects) over a NFSv4.2 mount, these changes reduced
the elapsed time by 30% and included a reduction of 80% for
RPC counts when delegations were enabled. As such, with this
patch there are at least certain cases where enabling
delegations seems to be worth the increased complexity they
bring.

This patch should only affect the NFSv4.1/4.2 behaviour
when delegations are enabled, which is not the default.

MFC after: 1 month

show more ...

nfscl: Add a check for VREG for delegations

Since delegations are only issued for regular files, check
v_type to see if the query is for a regular file. This is
a simple optimization for the non-VR

nfscl: Add a check for VREG for delegations

Since delegations are only issued for regular files, check
v_type to see if the query is for a regular file. This is
a simple optimization for the non-VREG case.
While here, fix a couple of global variable declarations.

This patch should only affect the NFSv4.1/4.2 behaviour
when delegations are enabled, which is not the default.

MFC after: 1 month

show more ...

nfscl: Revert part of commit 196787f79e67

Commit 196787f79e67 erroneously assumed that the client code for
Open/Claim_deleg_cur_FH was broken, but it was not.
It was actually wireshark that was brok

nfscl: Revert part of commit 196787f79e67

Commit 196787f79e67 erroneously assumed that the client code for
Open/Claim_deleg_cur_FH was broken, but it was not.
It was actually wireshark that was broken and indicated
that the correct XDR was bogus.

This reverts the part of 196787f79e67 that changed the arguments for
Open/Claim_deleg_cur_FH.

Found during the IETF bakeathon testing event this week.

MFC after: 3 days

show more ...

nfscl: newnfs_copycred() cannot be called when a mutex is held

Since newnfs_copycred() calls crsetgroups() which in turn calls
crextend() which might do a malloc(M_WAITOK), newnfs_copycred()
cannot

nfscl: newnfs_copycred() cannot be called when a mutex is held

Since newnfs_copycred() calls crsetgroups() which in turn calls
crextend() which might do a malloc(M_WAITOK), newnfs_copycred()
cannot be called with a mutex held. Fortunately, the malloc()
call is rarely done, since XU_GROUPS is 16 and the NFS client
uses a maximum of 17 (only 17 groups will cause the malloc() to
be called). Further, it is only a problem if the malloc() tries
to sleep(). As such, this bug does not seem to have caused
problems in practice.

This patch fixes the one place in the NFS client where
newnfs_copycred() is called while a mutex is held by moving the
call to after where the mutex is released.

Found by inspection while working on an experimental patch.

MFC after: 2 weeks

show more ...

nfscl: Use Claim_Null_FH and Claim_Deleg_Cur_FH

For NFSv4.1/4.2, there are two new options for the Open operation.
These two options use the file handle for the file instead of the
file handle for t

nfscl: Use Claim_Null_FH and Claim_Deleg_Cur_FH

For NFSv4.1/4.2, there are two new options for the Open operation.
These two options use the file handle for the file instead of the
file handle for the directory plus a file name. By doing so, the
client code is simplified (it no longer needs the "nfsv4node" structure
attached to the NFS vnode). It also avoids problems caused by another
NFS client (or process running locally in the NFS server) doing a
rename or remove of the file name between the Lookup and Open.

Unfortunately, there was a bug (fixed recently by commit X)
in the NFS server which mis-parsed the Claim_Deleg_Cur_FH
arguments. To allow this patch to work with the broken FreeBSD
NFSv4.1/4.2 server, NFSMNTP_BUGGYFBSDSRV is defined and is set
when a correctly formatted Claim_Deleg_Cur_FH fails with NFSERR_EXPIRED.
(This is what the old, broken NFS server does, since it erroneously
uses the Getattr arguments as a stateID.) Once this flag is set,
the client fills in a stateID, to make the broken NFS server happy.

Tested at a recent IETF NFSv4 Bakeathon.

MFC after: 1 month

show more ...

sys: Remove $FreeBSD$: one-line .c pattern

Remove /^[\s*]*__FBSDID\("\$FreeBSD\$"\);?\s*\n/

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of

spdx: The BSD-2-Clause-FreeBSD identifier is obsolete, drop -FreeBSD

The SPDX folks have obsoleted the BSD-2-Clause-FreeBSD identifier. Catch
up to that fact and revert to their recommended match of BSD-2-Clause.

Discussed with: pfg
MFC After: 3 days
Sponsored by: Netflix

show more ...

nfscl: Move nfsrpc_destroysession into nfscommon

This patch moves nfsrpc_destroysession() into nfscommon.ko
and also modifies its arguments slightly. This will allow
the function to be called from

nfscl: Move nfsrpc_destroysession into nfscommon

This patch moves nfsrpc_destroysession() into nfscommon.ko
and also modifies its arguments slightly. This will allow
the function to be called from nfsv4_sequencelookup() in
a future commit.

This patch should not result in a semantics change.

PR: 260011
MFC after: 2 weeks

show more ...

nfscl: Clean up the code by not using the vnode_vtype() macro

The vnode_vtype() macro was used to make the code compatible
with Mac OSX, for the Mac OSX port.
For FreeBSD, this macro just obscured t

nfscl: Clean up the code by not using the vnode_vtype() macro

The vnode_vtype() macro was used to make the code compatible
with Mac OSX, for the Mac OSX port.
For FreeBSD, this macro just obscured the code, so
avoid using it to clean up the code.

This commit should not result in a semantics change.

show more ...

nfscl: Clean up the code by removing unused arguments

The "void *stuff" (also called fstuff and dstuff) argument
was used by the Mac OSX port. For FreeBSD, this argument
is always NULL, so remove i

nfscl: Clean up the code by removing unused arguments

The "void *stuff" (also called fstuff and dstuff) argument
was used by the Mac OSX port. For FreeBSD, this argument
is always NULL, so remove it to clean up the code.

This commit gets rid of "stuff" for assorted functions
defined in nfs_clrpcops.c and called in nfs_clvnops.c and
nfs_clstate.c.

This commit should not result in a semantics change.

show more ...

nfscl: Do not flush when a write delegation is held

When a NFSv4 byte range write lock is unlocked, all
data modifications need to be flushed to the server
to satisfy the coherency requirements for

nfscl: Do not flush when a write delegation is held

When a NFSv4 byte range write lock is unlocked, all
data modifications need to be flushed to the server
to satisfy the coherency requirements for byte range
locking. However, if a write delegation for the
file is held by the client, flushing is not required,
since no other NFSv4 client can have the file NFSv4
Opened.

Found by inspection as suggested by a similar change
that was done to the Linux NFSv4 client.

show more ...

nfscl: Do not handle NFSERR_BADSESSION in operation code

The NFSERR_BADSESSION reply from a NFSv4.1/4.2 server
is handled by newnfs_request(). It should not be handled
separately after newnfs_reque

nfscl: Do not handle NFSERR_BADSESSION in operation code

The NFSERR_BADSESSION reply from a NFSv4.1/4.2 server
is handled by newnfs_request(). It should not be handled
separately after newnfs_request() has returned.

These two cases were spotted during code inspection.
One of them should only redo what newnfs_request() already
did by the same "nfscl" thread. The other might have
resulted in recovery being done twice, but the code is
only used for "pnfs" mounts, so that would be rare.
Also, since NFSERR_BADSESSION should only be replied by
a server after the server reboots, this would be extremely
rare.

MFC after: 2 weeks

show more ...

nfscl: Fix a use after free in nfscl_cleanupkext()

ler@, markj@ reported a use after free in nfscl_cleanupkext().
They also provided two possible causes:
- In nfscl_cleanup_common(), "own" is the ow

nfscl: Fix a use after free in nfscl_cleanupkext()

ler@, markj@ reported a use after free in nfscl_cleanupkext().
They also provided two possible causes:
- In nfscl_cleanup_common(), "own" is the owner string
owp->nfsow_owner. If we free that particular
owner structure, than in subsequent comparisons
"own" will point to freed memory.
- nfscl_cleanup_common() can free more than one owner, so the use
of LIST_FOREACH_SAFE() in nfscl_cleanupkext() is not sufficient.

I also believe there is a 3rd:
- If nfscl_freeopenowner() or nfscl_freelockowner() is called
without the NFSCLSTATE mutex held, this could race with
nfscl_cleanupkext().
This could happen when the exclusive lock is held
on the client, such as when delegations are being returned
or when recovering from NFSERR_EXPIRED.

This patch fixes them as follows:
1 - Copy the owner string to a local variable before the
nfscl_cleanup_common() call.
2 - Modify nfscl_cleanup_common() so that it will never free more
than the first matching element. Normally there should only
be one element in each list with a matching open/lock owner
anyhow (but there might be a bug that results in a duplicate).
This should guarantee that the FOREACH_SAFE loops in
nfscl_cleanupkext() are adequate.
3 - Acquire the NFSCLSTATE mutex in nfscl_freeopenowner()
and nfscl_freelockowner(), if it is not already held.
This serializes all of these calls with the ones done in
nfscl_cleanup_common().

Reported by: ler
Reviewed by: markj
Tested by: cy
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D34334

show more ...

Revert "nfscl: Fix a use after free in nfscl_cleanupkext()"

This reverts commit dd08b84e35b6fdee0df5fd0e0533cd361dec71cb.

cy@ reported a problem caused by this patch. He will be
testing an alterna

Revert "nfscl: Fix a use after free in nfscl_cleanupkext()"

This reverts commit dd08b84e35b6fdee0df5fd0e0533cd361dec71cb.

cy@ reported a problem caused by this patch. He will be
testing an alternate patch, but I'm reverting this one.

show more ...

nfscl: Fix a use after free in nfscl_cleanupkext()

ler@, markj@ reported a use after free in nfscl_cleanupkext().
They also provided two possible causes:
- In nfscl_cleanup_common(), "own" is the ow

nfscl: Fix a use after free in nfscl_cleanupkext()

ler@, markj@ reported a use after free in nfscl_cleanupkext().
They also provided two possible causes:
- In nfscl_cleanup_common(), "own" is the owner string
owp->nfsow_owner. If we free that particular
owner structure, than in subsequent comparisons
"own" will point to freed memory.
- nfscl_cleanup_common() can free more than one owner, so the use
of LIST_FOREACH_SAFE() in nfscl_cleanupkext() is not sufficient.

I also believe there is a 3rd:
- If nfscl_freeopenowner() or nfscl_freelockowner() is called
without the NFSCLSTATE mutex held, this could race with
nfscl_cleanupkext().
This could happen when the exclusive lock is held
on the client, such as when delegations are being returned.

This patch fixes them as follows:
1 - Copy the owner string to a local variable before the
nfscl_cleanup_common() call.
2 - Modify nfscl_cleanup_common() to return whether or not a
free was done.
When a free was done, do a goto to restart the loop, instead
of using FOREACH_SAFE, which was not safe in this case.
3 - Acquire the NFSCLSTATE mutex in nfscl_freeopenowner()
and nfscl_freelockowner(), if it not already held.
This serializes all of these calls with the ones done in
nfscl_cleanup_common().

Reported by: ler
Reviewed by: markj
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D34334

show more ...

nfscl: Handle CB_SEQUENCE not first op correctly

The check for "not first operation" in CB_SEQUENCE
was done after the slot, etc. was updated. This patch
moves the check to the beginning of CB_SEQUE

nfscl: Handle CB_SEQUENCE not first op correctly

The check for "not first operation" in CB_SEQUENCE
was done after the slot, etc. was updated. This patch
moves the check to the beginning of CB_SEQUENCE
processing.

While here, also fix the check for "no CB_SEQUENCE operation first"
by moving the check to the beginning of callback operation parsing,
since the check was in a couple of the other operations, but
not all of them.

Reported by: rtm@lcs.mit.edu
Tested by: rtm@lcs.mit.edu
PR: 260412
MFC after: 2 weeks

show more ...

nfscl: Sanity check the callback tag length

The sanity check for tag length in a callback request
was broken in two ways:

It checked for a negative value, but not a large positive
value.

It did no

nfscl: Sanity check the callback tag length

The sanity check for tag length in a callback request
was broken in two ways:

It checked for a negative value, but not a large positive
value.

It did not set taglen to -1, to indicate to the code that
it should not be used.

This patch fixes both of these issues.

Reported by: rtm@lcs.mit.edu
Tested by: rtm@lcs.mit.edu
PR: 260266
MFC after: 2 weeks

show more ...

pNFS: Add nfsstats counters for number of Layouts

For pNFS, Layouts are issued by the server to indicate
where a file's data resides on the DS(s). This patch
adds counters for how many layouts are

pNFS: Add nfsstats counters for number of Layouts

For pNFS, Layouts are issued by the server to indicate
where a file's data resides on the DS(s). This patch
adds counters for how many layouts are allocated to
the nfsstatsv1 structure, using two reserved fields.

MFC after: 2 weeks

show more ...

nfscl: Fix two more cases for forced dismount

Although I was not able to cause a failure during testing, there
are places in nfscl_removedeleg() and nfscl_renamedeleg() where
I think a forced dismou

nfscl: Fix two more cases for forced dismount

Although I was not able to cause a failure during testing, there
are places in nfscl_removedeleg() and nfscl_renamedeleg() where
I think a forced dismount could get hung. This patch fixes those.

This patch only affects forced dismount and only if the NFSv4
server is issuing delegations to the client.

Found by code inspection.

MFC after: 2 weeks

show more ...

nfscl: Fix use after free for forced dismount

When a forced dismount is done and delegations are being
issued by the server (disabled by default for FreeBSD
servers), the delegation structure is fre

nfscl: Fix use after free for forced dismount

When a forced dismount is done and delegations are being
issued by the server (disabled by default for FreeBSD
servers), the delegation structure is free'd before the
loop calling vflush(). This could result in a use after
free of the delegation structure.

This patch changes the code so that the delegation
structures are not free'd until after the vflush()
loop for forced dismounts.

Found during a recent IETF NFSv4 working group testing event.

MFC after: 2 weeks

show more ...

nfscl: Check for a forced dismount in nfscl_getref()

The nfscl_getref() function is called within nfscl_doiods() when
the NFSv4.1/4.2 pNFS client is doing I/O on a DS. As such,
nfscl_getref() needs

nfscl: Check for a forced dismount in nfscl_getref()

The nfscl_getref() function is called within nfscl_doiods() when
the NFSv4.1/4.2 pNFS client is doing I/O on a DS. As such,
nfscl_getref() needs to check for a forced dismount.
This patch adds that check.

Found during a recent IETF NFSv4 working group testing event.

MFC after: 2 weeks

show more ...

nfscl: Do pNFS layout return_on_close synchronously

For pNFS servers that specify that Layouts are to be returned
upon close, they may expect that LayoutReturn to happen before
the associated Close.

nfscl: Do pNFS layout return_on_close synchronously

For pNFS servers that specify that Layouts are to be returned
upon close, they may expect that LayoutReturn to happen before
the associated Close.

This patch modifies the NFSv4.1/4.2 pNFS client so that this
is done. This only affects a pNFS mount against a non-FreeBSD
NFSv4.1/4.2 server that specifies return_on_close in LayoutGet
replies.

Found during a recent IETF NFSv4 working group testing event.

MFC after: 2 weeks

show more ...

nfscl: Use NFSMNTP_DELEGISSUED in two more functions

Commit 5e5ca4c8fc53 added a NFSMNTP_DELEGISSUED flag to indicate when
a delegation has been issued to the mount. For the common case
where an NF

nfscl: Use NFSMNTP_DELEGISSUED in two more functions

Commit 5e5ca4c8fc53 added a NFSMNTP_DELEGISSUED flag to indicate when
a delegation has been issued to the mount. For the common case
where an NFSv4 server is not issuing delegations, this flag
can be checked to avoid acquisition of the NFSCLSTATEMUTEX.

This patch adds checks for NFSMNTP_DELEGISSUED being set
to two more functions.

This change appears to be performance neutral for a small number
of opens, but should reduce lock contention for a large number of opens
for the common case where server is not issuing delegations.

MFC after: 2 week

show more ...

nfscl: Handle NFSv4.1/4.2 Close RPC NFSERR_DELAY replies better

Without this patch, if a NFSv4.1/4.2 server replies NFSERR_DELAY to
a Close operation, the client loops retrying the Close while holdi

nfscl: Handle NFSv4.1/4.2 Close RPC NFSERR_DELAY replies better

Without this patch, if a NFSv4.1/4.2 server replies NFSERR_DELAY to
a Close operation, the client loops retrying the Close while holding
a shared lock on the clientID. This shared lock blocks returns of
delegations, even though the server has issued a CB_RECALL to request
the delegation return.

This patch delays doing a retry of a Close that received a reply of
NFSERR_DELAY until after the shared lock on the clientID is released,
for NFSv4.1/4.2. To fix this for NFSv4.0 would be very difficult and
since the only known NFSv4 server to reply NFSERR_DELAY to Close only
does NFSv4.1/4.2, this fix is hoped to be sufficient.

This problem was detected during a recent IETF working group NFSv4
testing event.

MFC after: 2 week

show more ...