Message ID | CAH2r5mvS6_AXjbK8sY_dEWUbmtRjodSYEtxeNz_NST9+EyC96A@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [smb3,client] log less confusing message on multichannel mounts to Samba when no interfaces returned | expand |
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 >
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 > >
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 >>> > > >
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 > >>> > > > > > >
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
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 > > >
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 > > > > > >
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
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