cifs: set up next DFS target before generic_ip_connect()
authorPaulo Alcantara <pc@cjr.nz>
Tue, 19 May 2020 18:38:27 +0000 (15:38 -0300)
committerSteve French <stfrench@microsoft.com>
Mon, 1 Jun 2020 05:10:18 +0000 (00:10 -0500)
If we mount a very specific DFS link

    \\FS0.FOO.COM\dfs\link -> \FS0\share1, \FS1\share2

where its target list contains NB names ("FS0" & "FS1") rather than
FQDN ones ("FS0.FOO.COM" & "FS1.FOO.COM"), we end up connecting to
\FOO\share1 but server->hostname will have "FOO.COM".  The reason is
because both "FS0" and "FS0.FOO.COM" resolve to same IP address and
they share same TCP server connection, but "FS0.FOO.COM" was the first
hostname set -- which is OK.

However, if the echo thread timeouts and we still have a good
connection to "FS0", in cifs_reconnect()

    rc = generic_ip_connect(server) -> success
    if (rc) {
            ...
            reconn_inval_dfs_target(server, cifs_sb, &tgt_list,
                            &tgt_it);
            ...
     }
     ...

it successfully reconnects to "FS0" server but does not set up next
DFS target - which should be the same target server "\FS0\share1" -
and server->hostname remains set to "FS0.FOO.COM" rather than "FS0",
as reconn_inval_dfs_target() would have it set to "FS0" if called
earlier.

Finally, in __smb2_reconnect(), the reconnect of tcons would fail
because tcon->ses->server->hostname (FS0.FOO.COM) does not match DFS
target's hostname (FS0).

Fix that by calling reconn_inval_dfs_target() before
generic_ip_connect() so server->hostname will get updated correctly
prior to reconnecting its tcons in __smb2_reconnect().

With "cifs: handle hostnames that resolve to same ip in failover"
patch

    - The above problem would not occur.
    - We could save an DNS query to find out that they both resolve to
      the same ip address.

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
fs/cifs/connect.c

index 62503fb..aa1173f 100644 (file)
@@ -572,26 +572,26 @@ cifs_reconnect(struct TCP_Server_Info *server)
                try_to_freeze();
 
                mutex_lock(&server->srv_mutex);
+#ifdef CONFIG_CIFS_DFS_UPCALL
                /*
                 * Set up next DFS target server (if any) for reconnect. If DFS
                 * feature is disabled, then we will retry last server we
                 * connected to before.
                 */
+               reconn_inval_dfs_target(server, cifs_sb, &tgt_list, &tgt_it);
+#endif
+               rc = reconn_set_ipaddr(server);
+               if (rc) {
+                       cifs_dbg(FYI, "%s: failed to resolve hostname: %d\n",
+                                __func__, rc);
+               }
+
                if (cifs_rdma_enabled(server))
                        rc = smbd_reconnect(server);
                else
                        rc = generic_ip_connect(server);
                if (rc) {
                        cifs_dbg(FYI, "reconnect error %d\n", rc);
-#ifdef CONFIG_CIFS_DFS_UPCALL
-                       reconn_inval_dfs_target(server, cifs_sb, &tgt_list,
-                                               &tgt_it);
-#endif
-                       rc = reconn_set_ipaddr(server);
-                       if (rc) {
-                               cifs_dbg(FYI, "%s: failed to resolve hostname: %d\n",
-                                        __func__, rc);
-                       }
                        mutex_unlock(&server->srv_mutex);
                        msleep(3000);
                } else {