15100 WPTS BVT_SMB2Basic_ChangeNotify_... (write cases)

Reviewed by: Andy Stormont <astormont@racktopsystems.com>
Reviewed by: Matt Barden <mbarden@tintri.com>
Approved by: Robert Mustacchi <rm@fingolfin.org>

7 files changed, 84 insertions, 37 deletions

diff --git a/usr/src/uts/common/fs/smbsrv/smb2_write.c b/usr/src/uts/common/fs/smbsrv/smb2_write.c
index 8f10f67d49..fdea70b31a 100644
--- a/usr/src/uts/common/fs/smbsrv/smb2_write.c
+++ b/usr/src/uts/common/fs/smbsrv/smb2_write.c
@@ -11,6 +11,7 @@
 
 /*
  * Copyright 2020 Tintri by DDN, Inc. All rights reserved.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 
 /*
@@ -150,6 +151,17 @@ smb2_write(smb_request_t *sr)
 			break;
 		/* This revokes read cache delegations. */
 		(void) smb_oplock_break_WRITE(of->f_node, of);
+		/*
+		 * Don't want the performance cost of generating
+		 * change notify events on every write.  Instead:
+		 * Keep track of the fact that we have written
+		 * data via this handle, and do change notify
+		 * work on the first write, and during close.
+		 */
+		if (of->f_written == B_FALSE) {
+			of->f_written = B_TRUE;
+			smb_node_notify_modified(of->f_node);
+		}
 		break;
 
 	case STYPE_IPC:
diff --git a/usr/src/uts/common/fs/smbsrv/smb_fem.c b/usr/src/uts/common/fs/smbsrv/smb_fem.c
index 4204830afd..458a9e35f6 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_fem.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_fem.c
@@ -22,6 +22,7 @@
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2020 Tintri by DDN, Inc.  All rights reserved.
  * Copyright 2015 Joyent, Inc.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 
 #include <smbsrv/smb_kproto.h>
@@ -206,6 +207,16 @@ smb_fem_oplock_uninstall(smb_node_t *node)
  *
  * The FCN monitors intercept the respective VOP_* call regardless
  * of whether the call originates from CIFS, NFS, or a local process.
+ *
+ * Here we're only interested in operations that change the list of
+ * names contained in the directory.  SMB clients can also ask to be
+ * notified about events where a file in this directory has had its
+ * meta-data changed (size, times, etc) but that's outside of the
+ * design intent for these FEM hooks.  Those meta-data events DO
+ * happen when caused by SMB clients (via smb_node_notify_modified)
+ * but not by other FS activity because we don't have a good way to
+ * place all the FEM hooks that would be required for that, and if
+ * we did, the performance cost could be severe.
  */
 
 /*
diff --git a/usr/src/uts/common/fs/smbsrv/smb_node.c b/usr/src/uts/common/fs/smbsrv/smb_node.c
index fd37c2b328..542a646bbf 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_node.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_node.c
@@ -21,7 +21,7 @@
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2020 Tintri by DDN, Inc. All rights reserved.
- * Copyright 2021 RackTop Systems, Inc.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 /*
  * SMB Node State Machine
@@ -976,6 +976,32 @@ smb_node_notify_change(smb_node_t *node, uint_t action, const char *name)
 }
 
 /*
+ * Change notify modified differs for stream vs regular file.
+ * Changes to a stream give a notification on the "unnamed" node,
+ * which is the parent object of the stream.
+ */
+void
+smb_node_notify_modified(smb_node_t *node)
+{
+	smb_node_t *u_node;
+
+	u_node = SMB_IS_STREAM(node);
+	if (u_node != NULL) {
+		/* This is a named stream */
+		if (u_node->n_dnode != NULL) {
+			smb_node_notify_change(u_node->n_dnode,
+			    FILE_ACTION_MODIFIED_STREAM, u_node->od_name);
+		}
+	} else {
+		/* regular file or directory */
+		if (node->n_dnode != NULL) {
+			smb_node_notify_change(node->n_dnode,
+			    FILE_ACTION_MODIFIED, node->od_name);
+		}
+	}
+}
+
+/*
  * smb_node_start_crit()
  *
  * Enter critical region for share reservations.
@@ -1482,17 +1508,6 @@ smb_node_file_is_readonly(smb_node_t *node)
  * normal time stamp updates, such as updating the mtime after a
  * write, and ctime after an attribute change.
  *
- * Dos Attributes are stored persistently, but with a twist:
- * In Windows, when you set the "read-only" bit on some file,
- * existing writable handles to that file continue to have
- * write access.  (because access check happens at open)
- * If we were to set the read-only bit directly, we would
- * cause errors in subsequent writes on any of our open
- * (and writable) file handles.  So here too, we have to
- * simulate the Windows behavior.  We keep the read-only
- * bit "pending" in the smb_node (so it will be visible in
- * any new opens of the file) and apply it on close.
- *
  * File allocation size is also simulated, and not persistent.
  * When the file allocation size is set it is first rounded up
  * to block size. If the file size is smaller than the allocation
@@ -1505,7 +1520,6 @@ smb_node_setattr(smb_request_t *sr, smb_node_t *node,
 	int rc;
 	uint_t times_mask;
 	smb_attr_t tmp_attr;
-	smb_node_t *unnamed_node;
 
 	SMB_NODE_VALID(node);
 
@@ -1626,22 +1640,15 @@ smb_node_setattr(smb_request_t *sr, smb_node_t *node,
 	}
 
 	rc = smb_fsop_setattr(sr, cr, node, attr);
-	if (rc != 0)
-		return (rc);
-
-	if (node->n_dnode != NULL) {
-		smb_node_notify_change(node->n_dnode,
-		    FILE_ACTION_MODIFIED, node->od_name);
-	}
 
-	if ((unnamed_node = SMB_IS_STREAM(node)) != NULL) {
-		ASSERT(unnamed_node->n_magic == SMB_NODE_MAGIC);
-		ASSERT(unnamed_node->n_state != SMB_NODE_STATE_DESTROYING);
-		smb_node_notify_change(node->n_dnode,
-		    FILE_ACTION_MODIFIED_STREAM, node->od_name);
-	}
+	/*
+	 * Only generate change notify events for client requests.
+	 * Internal operations use sr=NULL
+	 */
+	if (rc == 0 && sr != NULL)
+		smb_node_notify_modified(node);
 
-	return (0);
+	return (rc);
 }
 
 /*
@@ -1650,11 +1657,6 @@ smb_node_setattr(smb_request_t *sr, smb_node_t *node,
  * Get attributes from the file system and apply any smb-specific
  * overrides for size, dos attributes and timestamps
  *
- * When node->n_pending_readonly is set on a node, pretend that
- * we've already set this node readonly at the filesystem level.
- * We can't actually do that until all writable handles are closed
- * or those writable handles would suddenly loose their access.
- *
  * Returns: errno
  */
 int
diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 59a3900513..9c275f9527 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -23,7 +23,7 @@
  * Copyright 2011-2020 Tintri by DDN, Inc. All rights reserved.
  * Copyright 2016 Syneto S.R.L. All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
- * Copyright 2021 RackTop Systems, Inc.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 
 /*
@@ -581,6 +581,16 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 			(void) smb_node_setattr(NULL, of->f_node,
 			    of->f_cr, NULL, pa);
 		}
+		/*
+		 * Don't want the performance cost of generating
+		 * change notify events on every write.  Instead:
+		 * Keep track of the fact that we have written
+		 * data via this handle, and do change notify
+		 * work on the first write, and during close.
+		 */
+		if (of->f_written) {
+			smb_node_notify_modified(of->f_node);
+		}
 
 		smb_server_dec_files(of->f_server);
 		break;
diff --git a/usr/src/uts/common/fs/smbsrv/smb_write.c b/usr/src/uts/common/fs/smbsrv/smb_write.c
index fbf85da282..a5d4c22904 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_write.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_write.c
@@ -22,6 +22,7 @@
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2020 Tintri by DDN, Inc. All rights reserved.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 
 #include <sys/sdt.h>
@@ -513,14 +514,22 @@ smb_common_write(smb_request_t *sr, smb_rw_param_t *param)
 
 		rc = smb_fsop_write(sr, sr->user_cr, node, ofile,
 		    &param->rw_vdb.vdb_uio, &lcount, stability);
-
 		if (rc)
 			return (rc);
-
+		param->rw_count = lcount;
 		/* This revokes read cache delegations. */
 		(void) smb_oplock_break_WRITE(node, ofile);
-
-		param->rw_count = lcount;
+		/*
+		 * Don't want the performance cost of generating
+		 * change notify events on every write.  Instead:
+		 * Keep track of the fact that we have written
+		 * data via this handle, and do change notify
+		 * work on the first write, and during close.
+		 */
+		if (ofile->f_written == B_FALSE) {
+			ofile->f_written = B_TRUE;
+			smb_node_notify_modified(node);
+		}
 		break;
 
 	case STYPE_IPC:
diff --git a/usr/src/uts/common/smbsrv/smb_kproto.h b/usr/src/uts/common/smbsrv/smb_kproto.h
index 76f88c06cb..3ffc10f15e 100644
--- a/usr/src/uts/common/smbsrv/smb_kproto.h
+++ b/usr/src/uts/common/smbsrv/smb_kproto.h
@@ -23,6 +23,7 @@
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2016 Syneto S.R.L.  All rights reserved.
  * Copyright 2011-2022 Tintri by DDN, Inc. All rights reserved.
+ * Copyright 2022 RackTop Systems, Inc.
  */
 
 /*
@@ -537,6 +538,7 @@ boolean_t smb_node_share_check(smb_node_t *);
 void smb_node_fcn_subscribe(smb_node_t *);
 void smb_node_fcn_unsubscribe(smb_node_t *);
 void smb_node_notify_change(smb_node_t *, uint_t, const char *);
+void smb_node_notify_modified(smb_node_t *);
 
 int smb_node_getattr(smb_request_t *, smb_node_t *, cred_t *,
     smb_ofile_t *, smb_attr_t *);
diff --git a/usr/src/uts/common/smbsrv/smb_ktypes.h b/usr/src/uts/common/smbsrv/smb_ktypes.h
index 9896a1baf2..f29a5b24c0 100644
--- a/usr/src/uts/common/smbsrv/smb_ktypes.h
+++ b/usr/src/uts/common/smbsrv/smb_ktypes.h
@@ -1417,6 +1417,7 @@ typedef struct smb_ofile {
 	cred_t			*f_cr;
 	pid_t			f_pid;
 	smb_attr_t		f_pending_attr;
+	boolean_t		f_written;
 	smb_oplock_grant_t	f_oplock;
 	boolean_t		f_oplock_closing;
 	uint8_t			TargetOplockKey[SMB_LEASE_KEY_SZ];