In D58181#1334837, @fuz wrote:

This would have been nicer if the flag was named "noucache" with inverse
semantics as with nodump. (so "ucache" to turn it off)
nodontcache is a weird double negation.

Maybe this can still be revised?

In the NFS game there are two things...

• The RFCs. They don't define exactly when an NFS server should return ESTALE, so I don't think either the current behaviour or the behaviour with this patch is incorrect.
• Linux. It has become the "defacto standard" for NFS. (Solaris once was the "defacto standard" but??)

So, if this patch results in behaviour consistent with
Linux, I think it is reasonable to do.

I also updated the comment in kgss_load().

In D57681#1322466, @kib wrote:

In D57681#1322456, @rmacklem wrote:

This looks ok to me, although I'll admit I am not
conversant with UFS semantics, so I'll leave that
to others.

All existing callers pass tsa == NULL, so the intent is that the semantic does not change.
It is the question whether your case is served when tsa != NULL is provided.

I guess it does. I don't know UFS, but it does seem "inefficient" to first do
UFS_INODE_SET_FLAG_SHARED(), which locks/unlocks the vnode interlock and
then call ufs_itimes(), which locks the vnode interlock again and calls ufs_itimes_locked().

This looks ok to me, although I'll admit I am not
conversant with UFS semantics, so I'll leave that
to others.

In D57681#1322365, @kib wrote:

What is not clear to me, should ufs_itimes() only use the passed timespec for atime update, or it shall be used for all times as I did in the initial version of the patch.
Perhaps this is the question to Rick, since I believe the only caller with tsa != NULL would be nfs server.

Made changes as suggested by kib@.

Changed return value to EBUSY when the nfsd
are running and got rid of the locking around
reading the list head, as suggested by markj@.

Add vfs_netexport_reset() as suggested by markj@.
Also, add KASSERT()s to check ne_ref > 0.

In D57555#1320278, @glebius wrote:

Mark, again thanks a lot for fixing my bug that I was too slow (extremely slow!) to fix. I feel shame for that. Rick, very sorry for delaying looking into the problem for two months.

In D57554#1320166, @kib wrote:

Do you need a kernel thread and code to create the placeholder files? Could it be done in userspace?

Updated with changes recommended by markj@
and kib@. Mostly removal of unnecessary locking
calls, plus a couple of important typo fixes.

Added vfs_netexport_acquire() as suggested by markj@
and renamed vfs_netexport_free() to vfs_netexport_release(),
since that seemed more consistent.

This looks fine to me and seemed to behave when I
tested with lotsa printf()s.

Ok, so I finally think I remember how this works;-)
Your patch tests out well, but I think it needs something like the following:

• sys/rpc/rpcsec_tls/rpctls_impl.c.yyy 2026-06-14 09:08:45.910463000 -0700

+++ sys/rpc/rpcsec_tls/rpctls_impl.c 2026-06-14 09:01:06.722976000 -0700
@@ -298,8 +298,18 @@ printf("rpctls_connect soref so=%p\n", so);

	stat = rpctlscd_connect_2(&arg, &res, rpctls_connect_handle);
	if (stat == RPC_SUCCESS)
		*reterr = res.reterr;

• else

+ else {
+ struct ct_data *ct = (struct ct_data *)newclient->cl_private;
+
+printf("clnt at rpctls_rpc_failed so=%p\n", so);

		rpctls_rpc_failed(&ups, so);

+ /*
+ * Since rpctls_rpc_failed() will do a soclose() if the
+ * daemon is not running, set ct_tlsstate to avoid doing
+ * a close in clnt_vc_destroy().
+ */
+ ct->ct_tlsstate = RPCTLS_INHANDSHAKE;
+ }

In D57555#1319373, @rmacklem wrote:

Ok, so now I see what you've done. You are now taking the
soref() when it is inserted in the RB tree instead of when it
is taken out.

I suspect the "else" case in rpctls_rpc_failed() needs a
sorele() along with the soshutdown(), to get rid of the
reference.

Now that I am testing it, this does not seem to be needed.
(You get to the "else" case if rpc.tlsservd is running on the
server and you use your little tls_trigger.c test against it.)

Once I've put it through some testing, I'll update the patch.

Ok, so now I see what you've done. You are now taking the
soref() when it is inserted in the RB tree instead of when it
is taken out.

Fix the memset() so that it only zeros out
part of the structure, as spotted by markj@.

I don't see why adding another soref() stops a socket
from being leaked?

Use refcount_init() as suggested by kib@.

Modified patch to use a local static bool instead
of kgss_gssd_cl. I also moved the svc_nl_create()
and svc_reg() function calls in gssd.c to before
the daemon() call, to avoid any race with nfsd
startup.
As suggested by glebius@.

Do you need me to commit it or do you have a
commit bit for src?

Thanks for the comments.

In D57455#1317682, @glebius wrote:

I think this change also creates a potential process level race with gssd. The rc(8) can run nfsd before gssd instantiated the RPC server. Together with this change in the gssd the daemon(3) call needs to be moved down after svc_nl_create() and svc_reg().

In D57455#1317681, @glebius wrote:

But why do you need a second pointer? You may have a static bool that marks the kgss_gssd_handle has been successfully checked with a NULL RPC. I would actually suggest not to store a second pointer neither a bool. Make kgss_gssd_client() always return a checked client.

In D57455#1317599, @glebius wrote:

Sorry, can't understand why do we need a second CLIENT * for this NULL RPC.

I'll just leave things in their current state.

In D57180#1310528, @pen_lysator.liu.se wrote:

Hmm.. Yeah, I didn't want to change that part of the code. But now when I read the fine print in the man page for strtoul() and strtol() I think we should just get rid of that extra check and just use "*endp != '\0'"...

Hmm.. Shouldn't we use strtoumax() and strtoimax() while we're at it...? Perhaps something like:

intmax_t iv;
uintmax_t uv;
uid_t id;
...
if (*name == -) {
        iv = strtoimax(name,&endp,0);
} else {
        uv = strtoumax(name,&endp,0);
}
if (name == endp || *endp != '\0') {
        errno = EINVAL; /* No or invalid number */
        return (-1);
}
if (*name == '-') {
        id = iv;
        if (int32_t) id != iv) {
               errno = EINVAL; /* Overflow */
               return (-1);
       }
} else {
        id = uv;
        if (id != uv) {
                errno = EINVAL; /* Overflow */
                return (-1);
        }
}

Hmm.. Not sure (or I'm too tired right now) how to write a better and fully portably check for under/overflow in the negative case that works if we get 64 bit uid_t/gid_t. :-)