[PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Steve French]

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

Some servers can return an empty network interface list so, unless
multichannel is requested, no need to log an error for this, and
when multichannel is requested on mount but no interfaces, log
something less confusing.  For this case change
   parse_server_interfaces: malformed interface info
to
   empty network interface list returned by server

Cc: <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

See attached patch

-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-do-not-log-confusing-message-when-server-return.patch --]
[-- Type: text/x-patch, Size: 1480 bytes --]

From 96a0af2c50ac5e454d1e896a9877a51ed100312b Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sat, 1 Oct 2022 11:44:08 -0500
Subject: [PATCH] smb3: do not log confusing message when server returns no
 network interfaces

Some servers can return an empty network interface list so, unless
multichannel is requested, no need to log an error for this, and
when multichannel is requested on mount but no interfaces, log
something less confusing.  For this case change
   parse_server_interfaces: malformed interface info
to
   empty network interface list returned by server

Cc: <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2ops.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 5b0600939206..88cbf2890f6a 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -543,6 +543,17 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 	}
 	spin_unlock(&ses->iface_lock);
 
+	/*
+	 * Samba server e.g. can return an empty interface list in some cases,
+	 * which would only be a problem if we were requesting multichannel
+	 */
+	if (bytes_left == 0) {
+		if (ses->chan_max > 1)
+			cifs_dbg(VFS, "empty network interface list returned by server\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
 	while (bytes_left >= sizeof(*p)) {
 		memset(&tmp_iface, 0, sizeof(tmp_iface));
 		tmp_iface.speed = le64_to_cpu(p->LinkSpeed);
-- 
2.34.1

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Tom Talpey]

On 10/1/2022 12:54 PM, Steve French wrote:
> Some servers can return an empty network interface list so, unless
> multichannel is requested, no need to log an error for this, and
> when multichannel is requested on mount but no interfaces, log
> something less confusing.  For this case change
>     parse_server_interfaces: malformed interface info
> to
>     empty network interface list returned by server

Will this spam the log if it happens on every MC refresh (10 mins)?
It might be helpful to identify the servername, too.

Tom.


> Cc: <stable@vger.kernel.org>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> 
> See attached patch
> 

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Steve French]

On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
>
> On 10/1/2022 12:54 PM, Steve French wrote:
> > Some servers can return an empty network interface list so, unless
> > multichannel is requested, no need to log an error for this, and
> > when multichannel is requested on mount but no interfaces, log
> > something less confusing.  For this case change
> >     parse_server_interfaces: malformed interface info
> > to
> >     empty network interface list returned by server
>
> Will this spam the log if it happens on every MC refresh (10 mins)?
> It might be helpful to identify the servername, too.

Yes - I just noticed that in this case (multichannel mount to Samba
where no valid interfaces) we log it every ten minutes.
Maybe best way to fix this is to change it to a log once error (with
server name is fine with me) since it is probably legal to return an
empty list (so not serious enough to be worth logging every ten
minutes) and in theory server could fix its interfaces later.



> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Steve French <stfrench@microsoft.com>
> >
> > See attached patch
> >



-- 
Thanks,

Steve

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Tom Talpey]

On 10/3/2022 12:38 AM, Steve French wrote:
> On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
>>
>> On 10/1/2022 12:54 PM, Steve French wrote:
>>> Some servers can return an empty network interface list so, unless
>>> multichannel is requested, no need to log an error for this, and
>>> when multichannel is requested on mount but no interfaces, log
>>> something less confusing.  For this case change
>>>      parse_server_interfaces: malformed interface info
>>> to
>>>      empty network interface list returned by server
>>
>> Will this spam the log if it happens on every MC refresh (10 mins)?
>> It might be helpful to identify the servername, too.
> 
> Yes - I just noticed that in this case (multichannel mount to Samba
> where no valid interfaces) we log it every ten minutes.
> Maybe best way to fix this is to change it to a log once error (with
> server name is fine with me) since it is probably legal to return an
> empty list (so not serious enough to be worth logging every ten
> minutes) and in theory server could fix its interfaces later.

Ten minutes is the default recommended polling interval in the doc.

While it's odd, it's not prevented by the protocol. I could guess
that a server running in a namespace might return strange things
as devices came and went, for example.

It's not an error, so the message is purely informational. It is
useful though. Is it possible to suppress the logging if the
message *doesn't* change, but otherwise emit new ones? That might
require some per-server fiddling to avoid multiple servers flipping
the message.

A boolean or bit in the server struct? A little ugly for the purpose,
but surfacing multichannel events - especially ones that prevent it
from happening - seems worthwhile.

Tom.


Tom.


>>> Cc: <stable@vger.kernel.org>
>>> Signed-off-by: Steve French <stfrench@microsoft.com>
>>>
>>> See attached patch
>>>
> 
> 
> 

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Steve French]

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

updated patch to:
1) log the server name for this message
2) only log on mount (not every ten minutes)

See attached

On Mon, Oct 3, 2022 at 9:21 AM Tom Talpey <tom@talpey.com> wrote:
>
> On 10/3/2022 12:38 AM, Steve French wrote:
> > On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
> >>
> >> On 10/1/2022 12:54 PM, Steve French wrote:
> >>> Some servers can return an empty network interface list so, unless
> >>> multichannel is requested, no need to log an error for this, and
> >>> when multichannel is requested on mount but no interfaces, log
> >>> something less confusing.  For this case change
> >>>      parse_server_interfaces: malformed interface info
> >>> to
> >>>      empty network interface list returned by server
> >>
> >> Will this spam the log if it happens on every MC refresh (10 mins)?
> >> It might be helpful to identify the servername, too.
> >
> > Yes - I just noticed that in this case (multichannel mount to Samba
> > where no valid interfaces) we log it every ten minutes.
> > Maybe best way to fix this is to change it to a log once error (with
> > server name is fine with me) since it is probably legal to return an
> > empty list (so not serious enough to be worth logging every ten
> > minutes) and in theory server could fix its interfaces later.
>
> Ten minutes is the default recommended polling interval in the doc.
>
> While it's odd, it's not prevented by the protocol. I could guess
> that a server running in a namespace might return strange things
> as devices came and went, for example.
>
> It's not an error, so the message is purely informational. It is
> useful though. Is it possible to suppress the logging if the
> message *doesn't* change, but otherwise emit new ones? That might
> require some per-server fiddling to avoid multiple servers flipping
> the message.
>
> A boolean or bit in the server struct? A little ugly for the purpose,
> but surfacing multichannel events - especially ones that prevent it
> from happening - seems worthwhile.
>
> Tom.
>
>
> Tom.
>
>
> >>> Cc: <stable@vger.kernel.org>
> >>> Signed-off-by: Steve French <stfrench@microsoft.com>
> >>>
> >>> See attached patch
> >>>
> >
> >
> >



-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-do-not-log-confusing-message-when-server-return.patch --]
[-- Type: text/x-patch, Size: 3933 bytes --]

From 952672f57f3d94a631ca890e96bf70740c36758b Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Mon, 3 Oct 2022 17:25:41 -0500
Subject: [PATCH] smb3: do not log confusing message when server returns no 
 network interfaces

Some servers can return an empty network interface list so, unless
multichannel is requested, no need to log an error for this, and
when multichannel is requested on mount but no interfaces, log
something less confusing.  For this case change
   parse_server_interfaces: malformed interface info
to
   empty network interface list returned by server localhost

Also do not relog this error every ten minutes (only log on mount, once)

Cc: <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifsproto.h |  2 +-
 fs/cifs/connect.c   |  2 +-
 fs/cifs/smb2ops.c   | 16 +++++++++-------
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index f5adcb8ea04d..84ec71bdfacd 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -638,7 +638,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
 int
 cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
 int
-SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon);
+SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
 
 void extract_unc_hostname(const char *unc, const char **h, size_t *len);
 int copy_path_name(char *dst, const char *src);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ad81d7d43eaf..93e59b3b36c7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -155,7 +155,7 @@ static void smb2_query_server_interfaces(struct work_struct *work)
 	/*
 	 * query server network interfaces, in case they change
 	 */
-	rc = SMB3_request_interfaces(0, tcon);
+	rc = SMB3_request_interfaces(0, tcon, false);
 	if (rc) {
 		cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n",
 				__func__, rc);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index ef88f95302f6..b0d93188631e 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -512,8 +512,7 @@ smb3_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx)
 
 static int
 parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
-			size_t buf_len,
-			struct cifs_ses *ses)
+			size_t buf_len, struct cifs_ses *ses, bool in_mount)
 {
 	struct network_interface_info_ioctl_rsp *p;
 	struct sockaddr_in *addr4;
@@ -548,8 +547,11 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 	 * which would only be a problem if we were requesting multichannel
 	 */
 	if (bytes_left == 0) {
-		if (ses->chan_max > 1)
-			cifs_dbg(VFS, "empty network interface list returned by server\n");
+		/* avoid spamming logs every 10 minutes, so log only in mount */
+		if ((ses->chan_max > 1) && in_mount)
+			cifs_dbg(VFS,
+				 "empty network interface list returned by server %s\n",
+				 ses->server->hostname);
 		rc = -EINVAL;
 		goto out;
 	}
@@ -684,7 +686,7 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 }
 
 int
-SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
+SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount)
 {
 	int rc;
 	unsigned int ret_data_len = 0;
@@ -704,7 +706,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
 		goto out;
 	}
 
-	rc = parse_server_interfaces(out_buf, ret_data_len, ses);
+	rc = parse_server_interfaces(out_buf, ret_data_len, ses, in_mount);
 	if (rc)
 		goto out;
 
@@ -740,7 +742,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon,
 	if (rc)
 		return;
 
-	SMB3_request_interfaces(xid, tcon);
+	SMB3_request_interfaces(xid, tcon, true /* called during  mount */);
 
 	SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid,
 			FS_ATTRIBUTE_INFORMATION);
-- 
2.34.1

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Steve French]

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

attached wrong patch - resending


On Mon, Oct 3, 2022 at 5:32 PM Steve French <smfrench@gmail.com> wrote:
>
> updated patch to:
> 1) log the server name for this message
> 2) only log on mount (not every ten minutes)
>
> See attached
>
> On Mon, Oct 3, 2022 at 9:21 AM Tom Talpey <tom@talpey.com> wrote:
> >
> > On 10/3/2022 12:38 AM, Steve French wrote:
> > > On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
> > >>
> > >> On 10/1/2022 12:54 PM, Steve French wrote:
> > >>> Some servers can return an empty network interface list so, unless
> > >>> multichannel is requested, no need to log an error for this, and
> > >>> when multichannel is requested on mount but no interfaces, log
> > >>> something less confusing.  For this case change
> > >>>      parse_server_interfaces: malformed interface info
> > >>> to
> > >>>      empty network interface list returned by server
> > >>
> > >> Will this spam the log if it happens on every MC refresh (10 mins)?
> > >> It might be helpful to identify the servername, too.
> > >
> > > Yes - I just noticed that in this case (multichannel mount to Samba
> > > where no valid interfaces) we log it every ten minutes.
> > > Maybe best way to fix this is to change it to a log once error (with
> > > server name is fine with me) since it is probably legal to return an
> > > empty list (so not serious enough to be worth logging every ten
> > > minutes) and in theory server could fix its interfaces later.
> >
> > Ten minutes is the default recommended polling interval in the doc.
> >
> > While it's odd, it's not prevented by the protocol. I could guess
> > that a server running in a namespace might return strange things
> > as devices came and went, for example.
> >
> > It's not an error, so the message is purely informational. It is
> > useful though. Is it possible to suppress the logging if the
> > message *doesn't* change, but otherwise emit new ones? That might
> > require some per-server fiddling to avoid multiple servers flipping
> > the message.
> >
> > A boolean or bit in the server struct? A little ugly for the purpose,
> > but surfacing multichannel events - especially ones that prevent it
> > from happening - seems worthwhile.
> >
> > Tom.
> >
> >
> > Tom.
> >
> >
> > >>> Cc: <stable@vger.kernel.org>
> > >>> Signed-off-by: Steve French <stfrench@microsoft.com>
> > >>>
> > >>> See attached patch
> > >>>
> > >
> > >
> > >
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-do-not-log-confusing-message-when-server-return.patch --]
[-- Type: text/x-patch, Size: 4090 bytes --]

From cf82a4c17840a6971fee8cb0e4c2ad060f0283a4 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Sat, 1 Oct 2022 11:44:08 -0500
Subject: [PATCH] smb3: do not log confusing message when server returns no
 network interfaces

Some servers can return an empty network interface list so, unless
multichannel is requested, no need to log an error for this, and
when multichannel is requested on mount but no interfaces, log
something less confusing.  For this case change
   parse_server_interfaces: malformed interface info
to
   empty network interface list returned by server localhost

Also do not relog this error every ten minutes (only log on mount, once)

Cc: <stable@vger.kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifsproto.h |  2 +-
 fs/cifs/connect.c   |  2 +-
 fs/cifs/smb2ops.c   | 23 ++++++++++++++++++-----
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index f5adcb8ea04d..84ec71bdfacd 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -638,7 +638,7 @@ cifs_chan_is_iface_active(struct cifs_ses *ses,
 int
 cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server);
 int
-SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon);
+SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount);
 
 void extract_unc_hostname(const char *unc, const char **h, size_t *len);
 int copy_path_name(char *dst, const char *src);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ad81d7d43eaf..93e59b3b36c7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -155,7 +155,7 @@ static void smb2_query_server_interfaces(struct work_struct *work)
 	/*
 	 * query server network interfaces, in case they change
 	 */
-	rc = SMB3_request_interfaces(0, tcon);
+	rc = SMB3_request_interfaces(0, tcon, false);
 	if (rc) {
 		cifs_dbg(FYI, "%s: failed to query server interfaces: %d\n",
 				__func__, rc);
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index c82b0871341c..b0d93188631e 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -512,8 +512,7 @@ smb3_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx)
 
 static int
 parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
-			size_t buf_len,
-			struct cifs_ses *ses)
+			size_t buf_len, struct cifs_ses *ses, bool in_mount)
 {
 	struct network_interface_info_ioctl_rsp *p;
 	struct sockaddr_in *addr4;
@@ -543,6 +542,20 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 	}
 	spin_unlock(&ses->iface_lock);
 
+	/*
+	 * Samba server e.g. can return an empty interface list in some cases,
+	 * which would only be a problem if we were requesting multichannel
+	 */
+	if (bytes_left == 0) {
+		/* avoid spamming logs every 10 minutes, so log only in mount */
+		if ((ses->chan_max > 1) && in_mount)
+			cifs_dbg(VFS,
+				 "empty network interface list returned by server %s\n",
+				 ses->server->hostname);
+		rc = -EINVAL;
+		goto out;
+	}
+
 	while (bytes_left >= sizeof(*p)) {
 		memset(&tmp_iface, 0, sizeof(tmp_iface));
 		tmp_iface.speed = le64_to_cpu(p->LinkSpeed);
@@ -673,7 +686,7 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 }
 
 int
-SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
+SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon, bool in_mount)
 {
 	int rc;
 	unsigned int ret_data_len = 0;
@@ -693,7 +706,7 @@ SMB3_request_interfaces(const unsigned int xid, struct cifs_tcon *tcon)
 		goto out;
 	}
 
-	rc = parse_server_interfaces(out_buf, ret_data_len, ses);
+	rc = parse_server_interfaces(out_buf, ret_data_len, ses, in_mount);
 	if (rc)
 		goto out;
 
@@ -729,7 +742,7 @@ smb3_qfs_tcon(const unsigned int xid, struct cifs_tcon *tcon,
 	if (rc)
 		return;
 
-	SMB3_request_interfaces(xid, tcon);
+	SMB3_request_interfaces(xid, tcon, true /* called during  mount */);
 
 	SMB2_QFS_attr(xid, tcon, fid.persistent_fid, fid.volatile_fid,
 			FS_ATTRIBUTE_INFORMATION);
-- 
2.34.1

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Tom Talpey]

In the patch:

> +	/*
> +	 * Samba server e.g. can return an empty interface list in some cases,
> +	 * which would only be a problem if we were requesting multichannel
> +	 */
> +	if (bytes_left == 0) {
> +		/* avoid spamming logs every 10 minutes, so log only in mount */
> +		if ((ses->chan_max > 1) && in_mount)
> +			cifs_dbg(VFS,
> +				 "empty network interface list returned by server %s\n",
> +				 ses->server->hostname);
> +		rc = -EINVAL;
> +		goto out;
> +	}



This logs the server name, but it might be confusing to the
admin since the mount does not actually fail. Perhaps add some
words to the effect of "multichannel not available"?

Acked-by: Tom Talpey <tom@talpey.com>

On 10/3/2022 6:36 PM, Steve French wrote:
> attached wrong patch - resending
> 
> 
> On Mon, Oct 3, 2022 at 5:32 PM Steve French <smfrench@gmail.com> wrote:
>>
>> updated patch to:
>> 1) log the server name for this message
>> 2) only log on mount (not every ten minutes)
>>
>> See attached
>>
>> On Mon, Oct 3, 2022 at 9:21 AM Tom Talpey <tom@talpey.com> wrote:
>>>
>>> On 10/3/2022 12:38 AM, Steve French wrote:
>>>> On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
>>>>>
>>>>> On 10/1/2022 12:54 PM, Steve French wrote:
>>>>>> Some servers can return an empty network interface list so, unless
>>>>>> multichannel is requested, no need to log an error for this, and
>>>>>> when multichannel is requested on mount but no interfaces, log
>>>>>> something less confusing.  For this case change
>>>>>>       parse_server_interfaces: malformed interface info
>>>>>> to
>>>>>>       empty network interface list returned by server
>>>>>
>>>>> Will this spam the log if it happens on every MC refresh (10 mins)?
>>>>> It might be helpful to identify the servername, too.
>>>>
>>>> Yes - I just noticed that in this case (multichannel mount to Samba
>>>> where no valid interfaces) we log it every ten minutes.
>>>> Maybe best way to fix this is to change it to a log once error (with
>>>> server name is fine with me) since it is probably legal to return an
>>>> empty list (so not serious enough to be worth logging every ten
>>>> minutes) and in theory server could fix its interfaces later.
>>>
>>> Ten minutes is the default recommended polling interval in the doc.
>>>
>>> While it's odd, it's not prevented by the protocol. I could guess
>>> that a server running in a namespace might return strange things
>>> as devices came and went, for example.
>>>
>>> It's not an error, so the message is purely informational. It is
>>> useful though. Is it possible to suppress the logging if the
>>> message *doesn't* change, but otherwise emit new ones? That might
>>> require some per-server fiddling to avoid multiple servers flipping
>>> the message.
>>>
>>> A boolean or bit in the server struct? A little ugly for the purpose,
>>> but surfacing multichannel events - especially ones that prevent it
>>> from happening - seems worthwhile.
>>>
>>> Tom.
>>>
>>>
>>> Tom.
>>>
>>>
>>>>>> Cc: <stable@vger.kernel.org>
>>>>>> Signed-off-by: Steve French <stfrench@microsoft.com>
>>>>>>
>>>>>> See attached patch
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
>> --
>> Thanks,
>>
>> Steve
> 
> 
> 

Re: [PATCH][smb3 client] log less confusing message on multichannel mounts to Samba when no interfaces returned

[Steve French]

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

message updated as suggested

    multichannel not available
    empty network interface returned by server localhost

See attached

On Tue, Oct 11, 2022 at 2:40 PM Tom Talpey <tom@talpey.com> wrote:
>
> In the patch:
>
> > +     /*
> > +      * Samba server e.g. can return an empty interface list in some cases,
> > +      * which would only be a problem if we were requesting multichannel
> > +      */
> > +     if (bytes_left == 0) {
> > +             /* avoid spamming logs every 10 minutes, so log only in mount */
> > +             if ((ses->chan_max > 1) && in_mount)
> > +                     cifs_dbg(VFS,
> > +                              "empty network interface list returned by server %s\n",
> > +                              ses->server->hostname);
> > +             rc = -EINVAL;
> > +             goto out;
> > +     }
>
>
>
> This logs the server name, but it might be confusing to the
> admin since the mount does not actually fail. Perhaps add some
> words to the effect of "multichannel not available"?
>
> Acked-by: Tom Talpey <tom@talpey.com>
>
> On 10/3/2022 6:36 PM, Steve French wrote:
> > attached wrong patch - resending
> >
> >
> > On Mon, Oct 3, 2022 at 5:32 PM Steve French <smfrench@gmail.com> wrote:
> >>
> >> updated patch to:
> >> 1) log the server name for this message
> >> 2) only log on mount (not every ten minutes)
> >>
> >> See attached
> >>
> >> On Mon, Oct 3, 2022 at 9:21 AM Tom Talpey <tom@talpey.com> wrote:
> >>>
> >>> On 10/3/2022 12:38 AM, Steve French wrote:
> >>>> On Sat, Oct 1, 2022 at 6:22 PM Tom Talpey <tom@talpey.com> wrote:
> >>>>>
> >>>>> On 10/1/2022 12:54 PM, Steve French wrote:
> >>>>>> Some servers can return an empty network interface list so, unless
> >>>>>> multichannel is requested, no need to log an error for this, and
> >>>>>> when multichannel is requested on mount but no interfaces, log
> >>>>>> something less confusing.  For this case change
> >>>>>>       parse_server_interfaces: malformed interface info
> >>>>>> to
> >>>>>>       empty network interface list returned by server
> >>>>>
> >>>>> Will this spam the log if it happens on every MC refresh (10 mins)?
> >>>>> It might be helpful to identify the servername, too.
> >>>>
> >>>> Yes - I just noticed that in this case (multichannel mount to Samba
> >>>> where no valid interfaces) we log it every ten minutes.
> >>>> Maybe best way to fix this is to change it to a log once error (with
> >>>> server name is fine with me) since it is probably legal to return an
> >>>> empty list (so not serious enough to be worth logging every ten
> >>>> minutes) and in theory server could fix its interfaces later.
> >>>
> >>> Ten minutes is the default recommended polling interval in the doc.
> >>>
> >>> While it's odd, it's not prevented by the protocol. I could guess
> >>> that a server running in a namespace might return strange things
> >>> as devices came and went, for example.
> >>>
> >>> It's not an error, so the message is purely informational. It is
> >>> useful though. Is it possible to suppress the logging if the
> >>> message *doesn't* change, but otherwise emit new ones? That might
> >>> require some per-server fiddling to avoid multiple servers flipping
> >>> the message.
> >>>
> >>> A boolean or bit in the server struct? A little ugly for the purpose,
> >>> but surfacing multichannel events - especially ones that prevent it
> >>> from happening - seems worthwhile.
> >>>
> >>> Tom.
> >>>
> >>>
> >>> Tom.
> >>>
> >>>
> >>>>>> Cc: <stable@vger.kernel.org>
> >>>>>> Signed-off-by: Steve French <stfrench@microsoft.com>
> >>>>>>
> >>>>>> See attached patch
> >>>>>>
> >>>>
> >>>>
> >>>>
> >>
> >>
> >>
> >> --
> >> Thanks,
> >>
> >> Steve
> >
> >
> >



-- 
Thanks,

Steve

[-- Attachment #2: 0001-smb3-clarify-multichannel-warning.patch --]
[-- Type: text/x-patch, Size: 1147 bytes --]

From 6c03a4dd8073c213d113c39c839aa3295a209af6 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Tue, 11 Oct 2022 23:26:33 -0500
Subject: [PATCH] smb3: clarify multichannel warning

When server does not return network interfaces, clarify the
message to indicate that "multichannel not available" not just
that "empty network interface returned by server ..."

Suggested-by: Tom Talpey <tom@talpey.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/smb2ops.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 5187250c5f66..9ed4daef6770 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -550,7 +550,8 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 		/* avoid spamming logs every 10 minutes, so log only in mount */
 		if ((ses->chan_max > 1) && in_mount)
 			cifs_dbg(VFS,
-				 "empty network interface list returned by server %s\n",
+				 "multichannel not available\n"
+				 "Empty network interface list returned by server %s\n",
 				 ses->server->hostname);
 		rc = -EINVAL;
 		goto out;
-- 
2.34.1