From patchwork Sat Dec 17 15:20:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 13075848 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7D409C4332F for ; Sat, 17 Dec 2022 15:20:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229549AbiLQPUY (ORCPT ); Sat, 17 Dec 2022 10:20:24 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58878 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229480AbiLQPUX (ORCPT ); Sat, 17 Dec 2022 10:20:23 -0500 Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 75BE4B1D for ; Sat, 17 Dec 2022 07:20:21 -0800 (PST) Received: by mail-lj1-x229.google.com with SMTP id n1so5042021ljg.3 for ; Sat, 17 Dec 2022 07:20:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=UOtJn9Yz7icGdvVFn5jUjXLEOlGGrbnIb/5xrDm5AGA=; b=ElchUrnCDHHtrGko4IXBGoIoGOVgmMiwth0QWEpM47IJ+c/rtXTOBTJuplR/IZ5upp kpsd2rjVCCc/7jJj68xvvQLV7uqq2KAt1gYStKwksusOUv/IHVrmOhD2Y6woCG8behvi 0JnweOPvbfT9ZXbpVyiu2YHF7pnog0nt8oHz8OkfBhM2qb8vx3HECDPu1SJLyh3qz+kV hNurVxYJqyw187Mc7T4GcoUKoQJkWPGbqMxLWLG0YH040ToMRs80WyVw3NjEMhnC84KG ozHKulBk5h/X2DvC+2h28GyHZAL2vPp/zjbi0wIj5jEtODvi86AXeP/qYkY1RARGuD1U rEXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=UOtJn9Yz7icGdvVFn5jUjXLEOlGGrbnIb/5xrDm5AGA=; b=HzttVjzS4zbFZw3zfjxH/jMIJ79Fv9EKVfd2z/e3rf2qAop6g5AMQLkalTa3mcUafG M57J8srY8P5M3RRkjbK6bmWsbyCyK2uuCm5Pjq2bwyaW5conZO53/2Vqce/BTtD+3LS2 /RLuJAT1W27FjDvCh1KG//13iRW4ReRXcZlSvOl+xCg0fAICJHiTtLGad48B+IaI4xfV vU4th2vk9w8pQ2GVQGfUuYaLWw/tzKNzrajifb0i11xZ2tG0U5jSpsyv73Y+9nFeI4e7 xD2kHu0w59J095IJLB+orTY/WjYYNWUz6L4O9BV6S8ni3GLipOCwz0Wl3a7o6GT7fAjn bsFQ== X-Gm-Message-State: ANoB5pnTCctA0JHB6liM9XH2ejRMkcXhG1cvNMpPBYOdBg9z3P+uN/Dr 54F+LBLi28iqsdXlu0ZRMYRyJpF42lDQhrGO5By2wc9ishQ= X-Google-Smtp-Source: AA0mqf7h3kCs/6VIUPwuhSNmnJD4G5GRmc2g3K6pkJ0MhSlNhyq1ueF5IS2U9DZa98sWtPKPV1K3A14XyQihFtTWu+A= X-Received: by 2002:a2e:86d2:0:b0:279:df97:e895 with SMTP id n18-20020a2e86d2000000b00279df97e895mr9128839ljj.226.1671290419238; Sat, 17 Dec 2022 07:20:19 -0800 (PST) MIME-Version: 1.0 From: Steve French Date: Sat, 17 Dec 2022 09:20:08 -0600 Message-ID: Subject: [PATCH][SMB3 client] reduce roundrips on create query_info To: CIFS Cc: Paulo Alcantara Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org Patch attached. "cifs: reduce roundtrips on create/qinfo requests" To work around some Window servers that return STATUS_OBJECT_NAME_INVALID on query infos under DFS namespaces that contain non-ASCII characters, we started checking for -ENOENT on every file open, and if so, then send additional requests to figure out whether it is a DFS link or not. It means that all those requests will be sent to every non-existing file. So, in order to reduce the number of roundtrips, check earlier whether status code is STATUS_OBJECT_NAME_INVALID and tcon supports dfs, and if so, then map -ENOENT to -EREMOTE so mount or automount will take care of chasing the DFS link -- if it isn't an DFS link, then -ENOENT will be returned appropriately. Before patch SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 228 Ioctl Request FSCTL_DFS_GET_REFERRALS, File: \ada.test\dfs\foo SMB2 143 Ioctl Response, Error: STATUS_OBJECT_PATH_NOT_FOUND SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 228 Ioctl Request FSCTL_DFS_GET_REFERRALS, File: \ada.test\dfs\foo SMB2 143 Ioctl Response, Error: STATUS_OBJECT_PATH_NOT_FOUND After patch SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... From c831fdbb88cabd13c187c0e6929b427836e4354a Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Mon, 12 Dec 2022 23:39:37 -0300 Subject: [PATCH 01/15] cifs: reduce roundtrips on create/qinfo requests To work around some Window servers that return STATUS_OBJECT_NAME_INVALID on query infos under DFS namespaces that contain non-ASCII characters, we started checking for -ENOENT on every file open, and if so, then send additional requests to figure out whether it is a DFS link or not. It means that all those requests will be sent to every non-existing file. So, in order to reduce the number of roundtrips, check earlier whether status code is STATUS_OBJECT_NAME_INVALID and tcon supports dfs, and if so, then map -ENOENT to -EREMOTE so mount or automount will take care of chasing the DFS link -- if it isn't an DFS link, then -ENOENT will be returned appropriately. Before patch SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 228 Ioctl Request FSCTL_DFS_GET_REFERRALS, File: \ada.test\dfs\foo SMB2 143 Ioctl Response, Error: STATUS_OBJECT_PATH_NOT_FOUND SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 228 Ioctl Request FSCTL_DFS_GET_REFERRALS, File: \ada.test\dfs\foo SMB2 143 Ioctl Response, Error: STATUS_OBJECT_PATH_NOT_FOUND After patch SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... SMB2 438 Create Request File: ada.test\dfs\foo;GetInfo Request... SMB2 310 Create Response, Error: STATUS_OBJECT_NAME_NOT_FOUND;... Signed-off-by: Paulo Alcantara (SUSE) Signed-off-by: Steve French --- fs/cifs/connect.c | 16 ---------------- fs/cifs/inode.c | 6 ------ fs/cifs/misc.c | 45 -------------------------------------------- fs/cifs/smb2inode.c | 46 ++++++++++++++++++++++++++++++++------------- fs/cifs/smb2ops.c | 28 +++++++++++++++++++++++---- 5 files changed, 57 insertions(+), 84 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index f51715d3e2f2..b04706835e02 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -3547,9 +3547,6 @@ static int is_path_remote(struct mount_ctx *mnt_ctx) struct cifs_tcon *tcon = mnt_ctx->tcon; struct smb3_fs_context *ctx = mnt_ctx->fs_ctx; char *full_path; -#ifdef CONFIG_CIFS_DFS_UPCALL - bool nodfs = cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS; -#endif if (!server->ops->is_path_accessible) return -EOPNOTSUPP; @@ -3566,19 +3563,6 @@ static int is_path_remote(struct mount_ctx *mnt_ctx) rc = server->ops->is_path_accessible(xid, tcon, cifs_sb, full_path); -#ifdef CONFIG_CIFS_DFS_UPCALL - if (nodfs) { - if (rc == -EREMOTE) - rc = -EOPNOTSUPP; - goto out; - } - - /* path *might* exist with non-ASCII characters in DFS root - * try again with full path (only if nodfs is not set) */ - if (rc == -ENOENT && is_tcon_dfs(tcon)) - rc = cifs_dfs_query_info_nonascii_quirk(xid, tcon, cifs_sb, - full_path); -#endif if (rc != 0 && rc != -EREMOTE) goto out; diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 286a5400b94e..f145a59af89b 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -993,12 +993,6 @@ int cifs_get_inode_info(struct inode **inode, const char *full_path, } rc = server->ops->query_path_info(xid, tcon, cifs_sb, full_path, &tmp_data, &adjust_tz, &is_reparse_point); -#ifdef CONFIG_CIFS_DFS_UPCALL - if (rc == -ENOENT && is_tcon_dfs(tcon)) - rc = cifs_dfs_query_info_nonascii_quirk(xid, tcon, - cifs_sb, - full_path); -#endif data = &tmp_data; } diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c index 1cbecd64d697..062175994e87 100644 --- a/fs/cifs/misc.c +++ b/fs/cifs/misc.c @@ -1314,49 +1314,4 @@ int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix) cifs_sb->mnt_cifs_flags |= CIFS_MOUNT_USE_PREFIX_PATH; return 0; } - -/** cifs_dfs_query_info_nonascii_quirk - * Handle weird Windows SMB server behaviour. It responds with - * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request - * for "\\\" DFS reference, - * where contains non-ASCII unicode symbols. - * - * Check such DFS reference. - */ -int cifs_dfs_query_info_nonascii_quirk(const unsigned int xid, - struct cifs_tcon *tcon, - struct cifs_sb_info *cifs_sb, - const char *linkpath) -{ - char *treename, *dfspath, sep; - int treenamelen, linkpathlen, rc; - - treename = tcon->tree_name; - /* MS-DFSC: All paths in REQ_GET_DFS_REFERRAL and RESP_GET_DFS_REFERRAL - * messages MUST be encoded with exactly one leading backslash, not two - * leading backslashes. - */ - sep = CIFS_DIR_SEP(cifs_sb); - if (treename[0] == sep && treename[1] == sep) - treename++; - linkpathlen = strlen(linkpath); - treenamelen = strnlen(treename, MAX_TREE_SIZE + 1); - dfspath = kzalloc(treenamelen + linkpathlen + 1, GFP_KERNEL); - if (!dfspath) - return -ENOMEM; - if (treenamelen) - memcpy(dfspath, treename, treenamelen); - memcpy(dfspath + treenamelen, linkpath, linkpathlen); - rc = dfs_cache_find(xid, tcon->ses, cifs_sb->local_nls, - cifs_remap(cifs_sb), dfspath, NULL, NULL); - if (rc == 0) { - cifs_dbg(FYI, "DFS ref '%s' is found, emulate -EREMOTE\n", - dfspath); - rc = -EREMOTE; - } else { - cifs_dbg(FYI, "%s: dfs_cache_find returned %d\n", __func__, rc); - } - kfree(dfspath); - return rc; -} #endif diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c index fbd46db1023a..25a2da0934b3 100644 --- a/fs/cifs/smb2inode.c +++ b/fs/cifs/smb2inode.c @@ -556,22 +556,42 @@ int smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, FILE_OPEN, create_options, ACL_NO_MODE, data, SMB2_OP_QUERY_INFO, cfile, NULL, NULL, err_iov, err_buftype); - if (rc == -EOPNOTSUPP) { - if (err_iov[0].iov_base && err_buftype[0] != CIFS_NO_BUFFER && - ((struct smb2_hdr *)err_iov[0].iov_base)->Command == SMB2_CREATE && - ((struct smb2_hdr *)err_iov[0].iov_base)->Status == STATUS_STOPPED_ON_SYMLINK) { - rc = smb2_parse_symlink_response(cifs_sb, err_iov, &data->symlink_target); + if (rc) { + struct smb2_hdr *hdr = err_iov[0].iov_base; + + if (unlikely(!hdr || err_buftype[0] == CIFS_NO_BUFFER)) + rc = -ENOMEM; + else if (rc == -EOPNOTSUPP && hdr->Command == SMB2_CREATE && + hdr->Status == STATUS_STOPPED_ON_SYMLINK) { + rc = smb2_parse_symlink_response(cifs_sb, err_iov, + &data->symlink_target); if (rc) goto out; - } - *reparse = true; - create_options |= OPEN_REPARSE_POINT; - /* Failed on a symbolic link - query a reparse point info */ - cifs_get_readable_path(tcon, full_path, &cfile); - rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, FILE_READ_ATTRIBUTES, - FILE_OPEN, create_options, ACL_NO_MODE, data, - SMB2_OP_QUERY_INFO, cfile, NULL, NULL, NULL, NULL); + *reparse = true; + create_options |= OPEN_REPARSE_POINT; + + /* Failed on a symbolic link - query a reparse point info */ + cifs_get_readable_path(tcon, full_path, &cfile); + rc = smb2_compound_op(xid, tcon, cifs_sb, full_path, + FILE_READ_ATTRIBUTES, FILE_OPEN, + create_options, ACL_NO_MODE, data, + SMB2_OP_QUERY_INFO, cfile, NULL, NULL, + NULL, NULL); + goto out; + } else if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && + hdr->Status == STATUS_OBJECT_NAME_INVALID) { + /* + * Handle weird Windows SMB server behaviour. It responds with + * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request + * for "\\\" DFS reference, + * where contains non-ASCII unicode symbols. + */ + rc = -EREMOTE; + } + if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb && + (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)) + rc = -EOPNOTSUPP; } out: diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index d33b00ac37de..1f11e5ad06a8 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -796,7 +796,9 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, int rc; __le16 *utf16_path; __u8 oplock = SMB2_OPLOCK_LEVEL_NONE; + int err_buftype = CIFS_NO_BUFFER; struct cifs_open_parms oparms; + struct kvec err_iov = {}; struct cifs_fid fid; struct cached_fid *cfid; @@ -820,14 +822,32 @@ smb2_is_path_accessible(const unsigned int xid, struct cifs_tcon *tcon, oparms.fid = &fid; oparms.reconnect = false; - rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, NULL, - NULL); + rc = SMB2_open(xid, &oparms, utf16_path, &oplock, NULL, NULL, + &err_iov, &err_buftype); if (rc) { - kfree(utf16_path); - return rc; + struct smb2_hdr *hdr = err_iov.iov_base; + + if (unlikely(!hdr || err_buftype == CIFS_NO_BUFFER)) + rc = -ENOMEM; + /* + * Handle weird Windows SMB server behaviour. It responds with + * STATUS_OBJECT_NAME_INVALID code to SMB2 QUERY_INFO request + * for "\\\" DFS reference, + * where contains non-ASCII unicode symbols. + */ + else if (rc != -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && + hdr->Status == STATUS_OBJECT_NAME_INVALID) + rc = -EREMOTE; + if (rc == -EREMOTE && IS_ENABLED(CONFIG_CIFS_DFS_UPCALL) && cifs_sb && + (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_DFS)) + rc = -EOPNOTSUPP; + goto out; } rc = SMB2_close(xid, tcon, fid.persistent_fid, fid.volatile_fid); + +out: + free_rsp_buf(err_buftype, err_iov.iov_base); kfree(utf16_path); return rc; } -- 2.34.1