[PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 718 bytes --]

[PATCH] smb3: allow debugging session and tcon info to improve stats
 analysis and debugging

When multiple mounts are to the same share from the same client it was not
possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
correspond to that mount.  In some recent examples this turned out to  be
a significant problem when trying to analyze performance problems - since
there are many cases where unless we know the tree id and session id we
can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
the total time they take, which is slowest, how many fail etc.) apply to
which mount.

Add an ioctl to return tid, session id, tree connect count and tcon flags.

-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-allow-debugging-session-and-tcon-info-to-improv.patch --]
[-- Type: text/x-patch, Size: 3262 bytes --]

From 54796a555a55c40ff61b14f574a139c912cffded Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 9 Nov 2023 15:28:12 -0600
Subject: [PATCH] smb3: allow debugging session and tcon info to improve stats
 analysis and debugging

When multiple mounts are to the same share from the same client it was not
possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
correspond to that mount.  In some recent examples this turned out to  be
a significant problem when trying to analyze performance problems - since
there are many cases where unless we know the tree id and session id we
can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
the total time they take, which is slowest, how many fail etc.) apply to
which mount.

Add an ioctl to return tid, session id, tree connect count and tcon flags.

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/smb/client/cifs_ioctl.h |  8 ++++++++
 fs/smb/client/ioctl.c      | 26 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h
index 332588e77c31..2eb0a747b6c3 100644
--- a/fs/smb/client/cifs_ioctl.h
+++ b/fs/smb/client/cifs_ioctl.h
@@ -26,6 +26,13 @@ struct smb_mnt_fs_info {
 	__u64   cifs_posix_caps;
 } __packed;
 
+struct smb_mnt_tcon_info {
+	__u32	tid;
+	__u64	session_id;
+	int	tc_count;
+	__u16	flags;
+} __packed;
+
 struct smb_snapshot_array {
 	__u32	number_of_snapshots;
 	__u32	number_of_snapshots_returned;
@@ -108,6 +115,7 @@ struct smb3_notify_info {
 #define CIFS_IOC_NOTIFY _IOW(CIFS_IOCTL_MAGIC, 9, struct smb3_notify)
 #define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info)
 #define CIFS_IOC_NOTIFY_INFO _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
+#define CIFS_IOC_GET_TCON_INFO _IOR(CIFS_IOCTL_MAGIC, 12, struct smb_mnt_tcon_info)
 #define CIFS_IOC_SHUTDOWN _IOR('X', 125, __u32)
 
 /*
diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
index f7160003e0ed..8000433407df 100644
--- a/fs/smb/client/ioctl.c
+++ b/fs/smb/client/ioctl.c
@@ -117,6 +117,22 @@ static long cifs_ioctl_copychunk(unsigned int xid, struct file *dst_file,
 	return rc;
 }
 
+static long smb_mnt_get_tcon_info(struct cifs_tcon *tcon, void __user *arg)
+{
+	int rc = 0;
+	struct smb_mnt_tcon_info tcon_inf;
+
+	tcon_inf.tid = tcon->tid;
+	tcon_inf.session_id = tcon->ses->Suid;
+	tcon_inf.tc_count = tcon->tc_count;
+	tcon_inf.flags = tcon->Flags;
+
+	if (copy_to_user(arg, &tcon_inf, sizeof(struct smb_mnt_tcon_info)))
+		rc = -EFAULT;
+
+	return rc;
+}
+
 static long smb_mnt_get_fsinfo(unsigned int xid, struct cifs_tcon *tcon,
 				void __user *arg)
 {
@@ -414,6 +430,16 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 			tcon = tlink_tcon(pSMBFile->tlink);
 			rc = smb_mnt_get_fsinfo(xid, tcon, (void __user *)arg);
 			break;
+		case CIFS_IOC_GET_TCON_INFO:
+			cifs_sb = CIFS_SB(inode->i_sb);
+			tlink = cifs_sb_tlink(cifs_sb);
+			if (IS_ERR(tlink)) {
+				rc = PTR_ERR(tlink);
+				break;
+			}
+			tcon = tlink_tcon(tlink);
+			rc = smb_mnt_get_tcon_info(tcon, (void __user *)arg);
+			break;
 		case CIFS_ENUMERATE_SNAPSHOTS:
 			if (pSMBFile == NULL)
 				break;
-- 
2.39.2

Re: [PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

Updated patch to remove dumping of flags (tcon->Flags was already
supposed to be dumped via
the other ioctl (CIFS_IOC_GET_MNT_INFO) but it had a minor bug - will
send followon patch for that)


On Thu, Nov 9, 2023 at 3:51 PM Steve French <smfrench@gmail.com> wrote:
>
> [PATCH] smb3: allow debugging session and tcon info to improve stats
>  analysis and debugging
>
> When multiple mounts are to the same share from the same client it was not
> possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
> correspond to that mount.  In some recent examples this turned out to  be
> a significant problem when trying to analyze performance problems - since
> there are many cases where unless we know the tree id and session id we
> can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
> the total time they take, which is slowest, how many fail etc.) apply to
> which mount.
>
> Add an ioctl to return tid, session id, tree connect count and tcon flags.
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-allow-dumping-session-and-tcon-id-to-improve-st.patch --]
[-- Type: text/x-patch, Size: 3441 bytes --]

From 003ffa326df481486de61d761855126d968184ac Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Thu, 9 Nov 2023 15:28:12 -0600
Subject: [PATCH] smb3: allow dumping session and tcon id to improve stats
 analysis and debugging

When multiple mounts are to the same share from the same client it was not
possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
correspond to that mount.  In some recent examples this turned out to  be
a significant problem when trying to analyze performance data - since
there are many cases where unless we know the tree id and session id we
can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
the total time they take, which is slowest, how many fail etc.) apply to
which mount. The only existing loosely related ioctl CIFS_IOC_GET_MNT_INFO
does not return the information needed to uniquely identify which tcon
is which mount although it does return various flags and device info.

Add a cifs.ko ioctl CIFS_IOC_GET_TCON_INFO (0x8010cf0c) to return tid,
session id, tree connect count.

Signed-off-by: Steve French <stfrench@microsoft.com>

foo
---
 fs/smb/client/cifs_ioctl.h |  7 +++++++
 fs/smb/client/ioctl.c      | 25 +++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/fs/smb/client/cifs_ioctl.h b/fs/smb/client/cifs_ioctl.h
index 332588e77c31..71bdce7ea60c 100644
--- a/fs/smb/client/cifs_ioctl.h
+++ b/fs/smb/client/cifs_ioctl.h
@@ -26,6 +26,12 @@ struct smb_mnt_fs_info {
 	__u64   cifs_posix_caps;
 } __packed;
 
+struct smb_mnt_tcon_info {
+	__u32	tid;
+	__u64	session_id;
+	int	tc_count;
+} __packed;
+
 struct smb_snapshot_array {
 	__u32	number_of_snapshots;
 	__u32	number_of_snapshots_returned;
@@ -108,6 +114,7 @@ struct smb3_notify_info {
 #define CIFS_IOC_NOTIFY _IOW(CIFS_IOCTL_MAGIC, 9, struct smb3_notify)
 #define CIFS_DUMP_FULL_KEY _IOWR(CIFS_IOCTL_MAGIC, 10, struct smb3_full_key_debug_info)
 #define CIFS_IOC_NOTIFY_INFO _IOWR(CIFS_IOCTL_MAGIC, 11, struct smb3_notify_info)
+#define CIFS_IOC_GET_TCON_INFO _IOR(CIFS_IOCTL_MAGIC, 12, struct smb_mnt_tcon_info)
 #define CIFS_IOC_SHUTDOWN _IOR('X', 125, __u32)
 
 /*
diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c
index f7160003e0ed..f13730750e84 100644
--- a/fs/smb/client/ioctl.c
+++ b/fs/smb/client/ioctl.c
@@ -117,6 +117,21 @@ static long cifs_ioctl_copychunk(unsigned int xid, struct file *dst_file,
 	return rc;
 }
 
+static long smb_mnt_get_tcon_info(struct cifs_tcon *tcon, void __user *arg)
+{
+	int rc = 0;
+	struct smb_mnt_tcon_info tcon_inf;
+
+	tcon_inf.tid = tcon->tid;
+	tcon_inf.session_id = tcon->ses->Suid;
+	tcon_inf.tc_count = tcon->tc_count;
+
+	if (copy_to_user(arg, &tcon_inf, sizeof(struct smb_mnt_tcon_info)))
+		rc = -EFAULT;
+
+	return rc;
+}
+
 static long smb_mnt_get_fsinfo(unsigned int xid, struct cifs_tcon *tcon,
 				void __user *arg)
 {
@@ -414,6 +429,16 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg)
 			tcon = tlink_tcon(pSMBFile->tlink);
 			rc = smb_mnt_get_fsinfo(xid, tcon, (void __user *)arg);
 			break;
+		case CIFS_IOC_GET_TCON_INFO:
+			cifs_sb = CIFS_SB(inode->i_sb);
+			tlink = cifs_sb_tlink(cifs_sb);
+			if (IS_ERR(tlink)) {
+				rc = PTR_ERR(tlink);
+				break;
+			}
+			tcon = tlink_tcon(tlink);
+			rc = smb_mnt_get_tcon_info(tcon, (void __user *)arg);
+			break;
 		case CIFS_ENUMERATE_SNAPSHOTS:
 			if (pSMBFile == NULL)
 				break;
-- 
2.39.2

Re: [PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[Shyam Prasad N]

On Fri, Nov 10, 2023 at 8:21 AM Steve French <smfrench@gmail.com> wrote:
>
> Updated patch to remove dumping of flags (tcon->Flags was already
> supposed to be dumped via
> the other ioctl (CIFS_IOC_GET_MNT_INFO) but it had a minor bug - will
> send followon patch for that)
>
>
> On Thu, Nov 9, 2023 at 3:51 PM Steve French <smfrench@gmail.com> wrote:
> >
> > [PATCH] smb3: allow debugging session and tcon info to improve stats
> >  analysis and debugging
> >
> > When multiple mounts are to the same share from the same client it was not
> > possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
> > correspond to that mount.  In some recent examples this turned out to  be
> > a significant problem when trying to analyze performance problems - since
> > there are many cases where unless we know the tree id and session id we
> > can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
> > the total time they take, which is slowest, how many fail etc.) apply to
> > which mount.
> >
> > Add an ioctl to return tid, session id, tree connect count and tcon flags.
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve

IMO, tc_count is not necessary here. We should keep this ioctl very
specific to this use case.
Moreover, DebugData already prints tc_count.

-- 
Regards,
Shyam

Re: [PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

Fixed one minor bug (missing cifs_put_tlink call to free reference on
tlink) and updated cifs-2.6.git for-next

See attached (updated patch)


On Thu, Nov 9, 2023 at 8:51 PM Steve French <smfrench@gmail.com> wrote:
>
> Updated patch to remove dumping of flags (tcon->Flags was already
> supposed to be dumped via
> the other ioctl (CIFS_IOC_GET_MNT_INFO) but it had a minor bug - will
> send followon patch for that)
>
>
> On Thu, Nov 9, 2023 at 3:51 PM Steve French <smfrench@gmail.com> wrote:
> >
> > [PATCH] smb3: allow debugging session and tcon info to improve stats
> >  analysis and debugging
> >
> > When multiple mounts are to the same share from the same client it was not
> > possible to determine which section of /proc/fs/cifs/Stats (and DebugData)
> > correspond to that mount.  In some recent examples this turned out to  be
> > a significant problem when trying to analyze performance problems - since
> > there are many cases where unless we know the tree id and session id we
> > can't figure out which stats (e.g. number of SMB3.1.1 requests by type,
> > the total time they take, which is slowest, how many fail etc.) apply to
> > which mount.
> >
> > Add an ioctl to return tid, session id, tree connect count and tcon flags.
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve



--
Thanks,

Steve

[-- Attachment #2: 0001-smb3-allow-dumping-session-and-tcon-id-to-improve-st.patch --]
[-- Type: application/x-patch, Size: 3435 bytes --]

Re: [PATCH][smb3 client] allow debugging session and tcon info to improve stats analysis and debugging

[Steve French]

[-- Attachment #1.1: Type: text/plain, Size: 767 bytes --]

Attached is a sample program (that uses the new cifs.ko IOCTL) to dump the
tree id for a mount to make it easier to look at traces, wireshark,
/proc/fs/cifs/Stats, /proc/fs/cifs/DebugData if you have more than one
mount (would be useful to add something similar to debugging tools and/or
smb-info or something new in cifs-utils).

e.g.

# ./get-tcon-inf /mnt2
ioctl completed. tid 0x47b3d0e2 session id: 0xf6cde60a

# cat /proc/fs/cifs/DebugData | grep "tid: 0x47b3d0e2" -C3
2) \\localhost\test Mounts: 1 DevInfo: 0x20 Attributes: 0x801007f
PathComponentMax: 255 Status: 1 type: DISK Serial Number: 0xaab31952
Share Capabilities: None Aligned, Partition Aligned, Share Flags: 0x0
tid: 0x47b3d0e2 Optimal sector size: 0x200 Maximal Access: 0x1f01ff
-- 
Thanks,

Steve

[-- Attachment #1.2: Type: text/html, Size: 1035 bytes --]

[-- Attachment #2: get-tcon-inf.c --]
[-- Type: text/x-csrc, Size: 811 bytes --]

#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <errno.h>
#include <stdbool.h>
#include <fcntl.h>
#include <string.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <unistd.h>


struct __attribute__((__packed__))smb_mnt_tcon_info {
       uint32_t   tid;
       uint64_t   session_id;
} __packed;


#define CIFS_IOC_GET_TCON_INFO 0x800ccf0c
int main(int argc, char **argv)
{
	struct smb_mnt_tcon_info mnt_info;
	int f;

	if ((f = open(argv[1], O_RDONLY)) < 0) {
		fprintf(stderr, "Failed to open %s\n", argv[1]);
		exit(1);
	}

	if (ioctl(f, CIFS_IOC_GET_TCON_INFO, &mnt_info) < 0)
		printf("Error %d returned from ioctl\n", errno);
	else {
		printf("ioctl completed. tid 0x%x session id: 0x%lx\n", mnt_info.tid, mnt_info.session_id);
	}
}