diff mbox series

[1/4] cifs: fix DFS failover

Message ID 20210224235924.29931-1-pc@cjr.nz
State New
Headers show
Series [1/4] cifs: fix DFS failover | expand

Commit Message

Paulo Alcantara Feb. 24, 2021, 11:59 p.m. UTC
In do_dfs_failover(), the mount_get_conns() function requires the full
fs context in order to get new connection to server, so clone the
original context and change it accordingly when retrying the DFS
targets in the referral.

If failover was successful, then update original context with the new
UNC, prefix path and ip address.

Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
 fs/cifs/connect.c | 123 ++++++++++++++++++++++------------------------
 1 file changed, 59 insertions(+), 64 deletions(-)

Comments

Steve French Feb. 25, 2021, 1:17 a.m. UTC | #1
is this series of 4 identical with
   https://git.cjr.nz/linux.git/commit/?h=dfs-fixes-v2

On Wed, Feb 24, 2021 at 6:00 PM Paulo Alcantara <pc@cjr.nz> wrote:
>
> In do_dfs_failover(), the mount_get_conns() function requires the full
> fs context in order to get new connection to server, so clone the
> original context and change it accordingly when retrying the DFS
> targets in the referral.
>
> If failover was successful, then update original context with the new
> UNC, prefix path and ip address.
>
> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/connect.c | 123 ++++++++++++++++++++++------------------------
>  1 file changed, 59 insertions(+), 64 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index cd6dbeaf2166..a800e055c614 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -3045,96 +3045,91 @@ static int update_vol_info(const struct dfs_cache_tgt_iterator *tgt_it,
>         return 0;
>  }
>
> -static int setup_dfs_tgt_conn(const char *path, const char *full_path,
> -                             const struct dfs_cache_tgt_iterator *tgt_it,
> -                             struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx,
> -                             unsigned int *xid, struct TCP_Server_Info **server,
> -                             struct cifs_ses **ses, struct cifs_tcon **tcon)
> -{
> -       int rc;
> -       struct dfs_info3_param ref = {0};
> -       char *mdata = NULL;
> -       struct smb3_fs_context fake_ctx = {NULL};
> -       char *fake_devname = NULL;
> -
> -       cifs_dbg(FYI, "%s: dfs path: %s\n", __func__, path);
> -
> -       rc = dfs_cache_get_tgt_referral(path, tgt_it, &ref);
> -       if (rc)
> -               return rc;
> -
> -       mdata = cifs_compose_mount_options(cifs_sb->ctx->mount_options,
> -                                          full_path + 1, &ref,
> -                                          &fake_devname);
> -       free_dfs_info_param(&ref);
> -
> -       if (IS_ERR(mdata)) {
> -               rc = PTR_ERR(mdata);
> -               mdata = NULL;
> -       } else
> -               rc = cifs_setup_volume_info(&fake_ctx, mdata, fake_devname);
> -
> -       kfree(mdata);
> -       kfree(fake_devname);
> -
> -       if (!rc) {
> -               /*
> -                * We use a 'fake_ctx' here because we need pass it down to the
> -                * mount_{get,put} functions to test connection against new DFS
> -                * targets.
> -                */
> -               mount_put_conns(cifs_sb, *xid, *server, *ses, *tcon);
> -               rc = mount_get_conns(&fake_ctx, cifs_sb, xid, server, ses,
> -                                    tcon);
> -               if (!rc || (*server && *ses)) {
> -                       /*
> -                        * We were able to connect to new target server.
> -                        * Update current context with new target server.
> -                        */
> -                       rc = update_vol_info(tgt_it, &fake_ctx, ctx);
> -               }
> -       }
> -       smb3_cleanup_fs_context_contents(&fake_ctx);
> -       return rc;
> -}
> -
>  static int do_dfs_failover(const char *path, const char *full_path, struct cifs_sb_info *cifs_sb,
>                            struct smb3_fs_context *ctx, struct cifs_ses *root_ses,
>                            unsigned int *xid, struct TCP_Server_Info **server,
>                            struct cifs_ses **ses, struct cifs_tcon **tcon)
>  {
>         int rc;
> -       struct dfs_cache_tgt_list tgt_list;
> +       struct dfs_cache_tgt_list tgt_list = {0};
>         struct dfs_cache_tgt_iterator *tgt_it = NULL;
> +       struct smb3_fs_context tmp_ctx = {NULL};
>
>         if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)
>                 return -EOPNOTSUPP;
>
> +       cifs_dbg(FYI, "%s: path=%s full_path=%s\n", __func__, path, full_path);
> +
>         rc = dfs_cache_noreq_find(path, NULL, &tgt_list);
>         if (rc)
>                 return rc;
> +       /*
> +        * We use a 'tmp_ctx' here because we need pass it down to the mount_{get,put} functions to
> +        * test connection against new DFS targets.
> +        */
> +       rc = smb3_fs_context_dup(&tmp_ctx, ctx);
> +       if (rc)
> +               goto out;
>
>         for (;;) {
> +               struct dfs_info3_param ref = {0};
> +               char *fake_devname = NULL, *mdata = NULL;
> +
>                 /* Get next DFS target server - if any */
>                 rc = get_next_dfs_tgt(path, &tgt_list, &tgt_it);
>                 if (rc)
>                         break;
> -               /* Connect to next DFS target */
> -               rc = setup_dfs_tgt_conn(path, full_path, tgt_it, cifs_sb, ctx, xid, server, ses,
> -                                       tcon);
> -               if (!rc || (*server && *ses))
> +
> +               rc = dfs_cache_get_tgt_referral(path, tgt_it, &ref);
> +               if (rc)
>                         break;
> +
> +               cifs_dbg(FYI, "%s: old ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
> +                        tmp_ctx.prepath);
> +
> +               mdata = cifs_compose_mount_options(cifs_sb->ctx->mount_options, full_path + 1, &ref,
> +                                                  &fake_devname);
> +               free_dfs_info_param(&ref);
> +
> +               if (IS_ERR(mdata)) {
> +                       rc = PTR_ERR(mdata);
> +                       mdata = NULL;
> +               } else
> +                       rc = cifs_setup_volume_info(&tmp_ctx, mdata, fake_devname);
> +
> +               kfree(mdata);
> +               kfree(fake_devname);
> +
> +               if (rc)
> +                       break;
> +
> +               cifs_dbg(FYI, "%s: new ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
> +                        tmp_ctx.prepath);
> +
> +               mount_put_conns(cifs_sb, *xid, *server, *ses, *tcon);
> +               rc = mount_get_conns(&tmp_ctx, cifs_sb, xid, server, ses, tcon);
> +               if (!rc || (*server && *ses)) {
> +                       /*
> +                        * We were able to connect to new target server. Update current context with
> +                        * new target server.
> +                        */
> +                       rc = update_vol_info(tgt_it, &tmp_ctx, ctx);
> +                       break;
> +               }
>         }
>         if (!rc) {
> +               cifs_dbg(FYI, "%s: final ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
> +                        tmp_ctx.prepath);
>                 /*
> -                * Update DFS target hint in DFS referral cache with the target
> -                * server we successfully reconnected to.
> +                * Update DFS target hint in DFS referral cache with the target server we
> +                * successfully reconnected to.
>                  */
> -               rc = dfs_cache_update_tgthint(*xid, root_ses ? root_ses : *ses,
> -                                             cifs_sb->local_nls,
> -                                             cifs_remap(cifs_sb), path,
> -                                             tgt_it);
> +               rc = dfs_cache_update_tgthint(*xid, root_ses ? root_ses : *ses, cifs_sb->local_nls,
> +                                             cifs_remap(cifs_sb), path, tgt_it);
>         }
> +
> +out:
> +       smb3_cleanup_fs_context_contents(&tmp_ctx);
>         dfs_cache_free_tgts(&tgt_list);
>         return rc;
>  }
> --
> 2.30.1
>
Paulo Alcantara Feb. 25, 2021, 1:23 a.m. UTC | #2
Steve French <smfrench@gmail.com> writes:

> is this series of 4 identical with
>    https://git.cjr.nz/linux.git/commit/?h=dfs-fixes-v2

Nope.  Please pull from git://git.cjr.nz/linux.git dfs-fixes
Steve French Feb. 25, 2021, 3:34 a.m. UTC | #3
git diff dfs-fixes..dfs-fixes-v2      showed no differences between
the two branches

And should we cc: Stable #5.10

On Wed, Feb 24, 2021 at 7:23 PM Paulo Alcantara <pc@cjr.nz> wrote:
>
> Steve French <smfrench@gmail.com> writes:
>
> > is this series of 4 identical with
> >    https://git.cjr.nz/linux.git/commit/?h=dfs-fixes-v2
>
> Nope.  Please pull from git://git.cjr.nz/linux.git dfs-fixes
Steve French Feb. 25, 2021, 6:25 p.m. UTC | #4
I pushed the 4 in your email attachment to cifs-2.6 for-next

They differed only in patch 3 from the one in dfs-fixes-v2

Let me know if the correct version is in for-next

On Wed, Feb 24, 2021 at 9:34 PM Steve French <smfrench@gmail.com> wrote:
>
> git diff dfs-fixes..dfs-fixes-v2      showed no differences between
> the two branches
>
> And should we cc: Stable #5.10
>
> On Wed, Feb 24, 2021 at 7:23 PM Paulo Alcantara <pc@cjr.nz> wrote:
> >
> > Steve French <smfrench@gmail.com> writes:
> >
> > > is this series of 4 identical with
> > >    https://git.cjr.nz/linux.git/commit/?h=dfs-fixes-v2
> >
> > Nope.  Please pull from git://git.cjr.nz/linux.git dfs-fixes
>
>
>
> --
> Thanks,
>
> Steve
Steve French Feb. 25, 2021, 6:25 p.m. UTC | #5
I also added cc: stable # 5.11

On Thu, Feb 25, 2021 at 12:25 PM Steve French <smfrench@gmail.com> wrote:
>
> I pushed the 4 in your email attachment to cifs-2.6 for-next
>
> They differed only in patch 3 from the one in dfs-fixes-v2
>
> Let me know if the correct version is in for-next
>
> On Wed, Feb 24, 2021 at 9:34 PM Steve French <smfrench@gmail.com> wrote:
> >
> > git diff dfs-fixes..dfs-fixes-v2      showed no differences between
> > the two branches
> >
> > And should we cc: Stable #5.10
> >
> > On Wed, Feb 24, 2021 at 7:23 PM Paulo Alcantara <pc@cjr.nz> wrote:
> > >
> > > Steve French <smfrench@gmail.com> writes:
> > >
> > > > is this series of 4 identical with
> > > >    https://git.cjr.nz/linux.git/commit/?h=dfs-fixes-v2
> > >
> > > Nope.  Please pull from git://git.cjr.nz/linux.git dfs-fixes
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve
diff mbox series

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index cd6dbeaf2166..a800e055c614 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -3045,96 +3045,91 @@  static int update_vol_info(const struct dfs_cache_tgt_iterator *tgt_it,
 	return 0;
 }
 
-static int setup_dfs_tgt_conn(const char *path, const char *full_path,
-			      const struct dfs_cache_tgt_iterator *tgt_it,
-			      struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx,
-			      unsigned int *xid, struct TCP_Server_Info **server,
-			      struct cifs_ses **ses, struct cifs_tcon **tcon)
-{
-	int rc;
-	struct dfs_info3_param ref = {0};
-	char *mdata = NULL;
-	struct smb3_fs_context fake_ctx = {NULL};
-	char *fake_devname = NULL;
-
-	cifs_dbg(FYI, "%s: dfs path: %s\n", __func__, path);
-
-	rc = dfs_cache_get_tgt_referral(path, tgt_it, &ref);
-	if (rc)
-		return rc;
-
-	mdata = cifs_compose_mount_options(cifs_sb->ctx->mount_options,
-					   full_path + 1, &ref,
-					   &fake_devname);
-	free_dfs_info_param(&ref);
-
-	if (IS_ERR(mdata)) {
-		rc = PTR_ERR(mdata);
-		mdata = NULL;
-	} else
-		rc = cifs_setup_volume_info(&fake_ctx, mdata, fake_devname);
-
-	kfree(mdata);
-	kfree(fake_devname);
-
-	if (!rc) {
-		/*
-		 * We use a 'fake_ctx' here because we need pass it down to the
-		 * mount_{get,put} functions to test connection against new DFS
-		 * targets.
-		 */
-		mount_put_conns(cifs_sb, *xid, *server, *ses, *tcon);
-		rc = mount_get_conns(&fake_ctx, cifs_sb, xid, server, ses,
-				     tcon);
-		if (!rc || (*server && *ses)) {
-			/*
-			 * We were able to connect to new target server.
-			 * Update current context with new target server.
-			 */
-			rc = update_vol_info(tgt_it, &fake_ctx, ctx);
-		}
-	}
-	smb3_cleanup_fs_context_contents(&fake_ctx);
-	return rc;
-}
-
 static int do_dfs_failover(const char *path, const char *full_path, struct cifs_sb_info *cifs_sb,
 			   struct smb3_fs_context *ctx, struct cifs_ses *root_ses,
 			   unsigned int *xid, struct TCP_Server_Info **server,
 			   struct cifs_ses **ses, struct cifs_tcon **tcon)
 {
 	int rc;
-	struct dfs_cache_tgt_list tgt_list;
+	struct dfs_cache_tgt_list tgt_list = {0};
 	struct dfs_cache_tgt_iterator *tgt_it = NULL;
+	struct smb3_fs_context tmp_ctx = {NULL};
 
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)
 		return -EOPNOTSUPP;
 
+	cifs_dbg(FYI, "%s: path=%s full_path=%s\n", __func__, path, full_path);
+
 	rc = dfs_cache_noreq_find(path, NULL, &tgt_list);
 	if (rc)
 		return rc;
+	/*
+	 * We use a 'tmp_ctx' here because we need pass it down to the mount_{get,put} functions to
+	 * test connection against new DFS targets.
+	 */
+	rc = smb3_fs_context_dup(&tmp_ctx, ctx);
+	if (rc)
+		goto out;
 
 	for (;;) {
+		struct dfs_info3_param ref = {0};
+		char *fake_devname = NULL, *mdata = NULL;
+
 		/* Get next DFS target server - if any */
 		rc = get_next_dfs_tgt(path, &tgt_list, &tgt_it);
 		if (rc)
 			break;
-		/* Connect to next DFS target */
-		rc = setup_dfs_tgt_conn(path, full_path, tgt_it, cifs_sb, ctx, xid, server, ses,
-					tcon);
-		if (!rc || (*server && *ses))
+
+		rc = dfs_cache_get_tgt_referral(path, tgt_it, &ref);
+		if (rc)
 			break;
+
+		cifs_dbg(FYI, "%s: old ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
+			 tmp_ctx.prepath);
+
+		mdata = cifs_compose_mount_options(cifs_sb->ctx->mount_options, full_path + 1, &ref,
+						   &fake_devname);
+		free_dfs_info_param(&ref);
+
+		if (IS_ERR(mdata)) {
+			rc = PTR_ERR(mdata);
+			mdata = NULL;
+		} else
+			rc = cifs_setup_volume_info(&tmp_ctx, mdata, fake_devname);
+
+		kfree(mdata);
+		kfree(fake_devname);
+
+		if (rc)
+			break;
+
+		cifs_dbg(FYI, "%s: new ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
+			 tmp_ctx.prepath);
+
+		mount_put_conns(cifs_sb, *xid, *server, *ses, *tcon);
+		rc = mount_get_conns(&tmp_ctx, cifs_sb, xid, server, ses, tcon);
+		if (!rc || (*server && *ses)) {
+			/*
+			 * We were able to connect to new target server. Update current context with
+			 * new target server.
+			 */
+			rc = update_vol_info(tgt_it, &tmp_ctx, ctx);
+			break;
+		}
 	}
 	if (!rc) {
+		cifs_dbg(FYI, "%s: final ctx: UNC=%s prepath=%s\n", __func__, tmp_ctx.UNC,
+			 tmp_ctx.prepath);
 		/*
-		 * Update DFS target hint in DFS referral cache with the target
-		 * server we successfully reconnected to.
+		 * Update DFS target hint in DFS referral cache with the target server we
+		 * successfully reconnected to.
 		 */
-		rc = dfs_cache_update_tgthint(*xid, root_ses ? root_ses : *ses,
-					      cifs_sb->local_nls,
-					      cifs_remap(cifs_sb), path,
-					      tgt_it);
+		rc = dfs_cache_update_tgthint(*xid, root_ses ? root_ses : *ses, cifs_sb->local_nls,
+					      cifs_remap(cifs_sb), path, tgt_it);
 	}
+
+out:
+	smb3_cleanup_fs_context_contents(&tmp_ctx);
 	dfs_cache_free_tgts(&tgt_list);
 	return rc;
 }