From patchwork Sat Feb 27 09:31:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Shilovsky X-Patchwork-Id: 8444111 Return-Path: X-Original-To: patchwork-cifs-client@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B96399F372 for ; Sat, 27 Feb 2016 09:31:26 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4F3EA203B4 for ; Sat, 27 Feb 2016 09:31:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 665E7203A1 for ; Sat, 27 Feb 2016 09:31:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752411AbcB0JbW (ORCPT ); Sat, 27 Feb 2016 04:31:22 -0500 Received: from mail-io0-f172.google.com ([209.85.223.172]:36543 "EHLO mail-io0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751696AbcB0JbW (ORCPT ); Sat, 27 Feb 2016 04:31:22 -0500 Received: by mail-io0-f172.google.com with SMTP id l127so144520772iof.3 for ; Sat, 27 Feb 2016 01:31:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=HF3lU68hVNLQ1RqXtlSZ5aKjjm2MAFvrFj/Po/1rKw4=; b=C98BPYRvMkt0u57t8QU6YZhq6Cx45hni1m5j/Gf+ukchiHeHnmNCN9MsNt2PGZhr9X IhpJHgmdd2hyJ7gXx2yEz/g1zbKYB2C6JEnIuOZmqfID4MAqKn2gnsnST/Fz5Iu0Lqlj 37ZU8U/pQn6AV89HLsuKKV19ieW+//1siVT1GC2+VZwE+rBYoMDzXFHr+MAZSPN0HRlT fsCfKFw1EmMTyueCU7Fm+fMOUKp2VQOlsVytCWqyJda04EbBLqLIy8RkGHgLSbS5F76o Ed4ohcH+NwOJCxtBP6ZzXvMqN3i78qXoA3V48KANLw6OWZ0uPWDXlruwWcX4xNqMghjP /+OA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=HF3lU68hVNLQ1RqXtlSZ5aKjjm2MAFvrFj/Po/1rKw4=; b=Rl62x7v/ZR7nuij5hpsxmPcon4BvhbnRw26gWabkbOEALotNnDcBnqN4k7TwqhWurf OB44+iON4d3HtdxoBnyZfhpgcWBBZ3qRtOk0YRyAh+uM8pNapJPrvpA8SIxXuPZolw7W xEX7KPW1XUsZQ26O39EYcOqFCpU2Co9uyWy0jfcvo1IJ8OFcZbDW89PCQ4r6Uy3wqlDC v41+Z1LdzcMz3BuAqp5BlwwJj7Y+srx20zYGyaPWBarCF3SEyjVnaVmzxGPRSIRBtwiE 7E+wYcTHidfz62ca5I1zDdYaa1anVZEkKRNalO+hr+SegRPxezHaxHfh8mgr/vjTG9Nx 22XA== X-Gm-Message-State: AG10YORF8jl6bKaJDfMbgSL+dLFgcL+b3ooaky8LMhPHUpD7/LDhPUbtaaBqUEIZZg6yCrsyuQYFvA57OsmPAg== MIME-Version: 1.0 X-Received: by 10.107.164.15 with SMTP id n15mr10792582ioe.148.1456565481297; Sat, 27 Feb 2016 01:31:21 -0800 (PST) Received: by 10.64.81.107 with HTTP; Sat, 27 Feb 2016 01:31:21 -0800 (PST) In-Reply-To: References: Date: Sat, 27 Feb 2016 12:31:21 +0300 Message-ID: Subject: Re: Interop Issue: SMB2+ async replies, and the kernel, Samba side fix enclosed. From: Pavel Shilovsky To: Ira Cooper Cc: Samba Technical , linux-cifs , sfrench Sender: linux-cifs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-cifs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, T_TVD_MIME_EPI,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 2016-02-27 12:11 GMT+03:00 Pavel Shilovsky : > 2016-02-23 15:55 GMT+03:00 Ira Cooper : >> >> If the server sends an interim response, then the real response, the real >> response, is handled by standard_receive3() in the kernel, instead of the >> proper function, and this causes a disconnect. If there isn't a >> disconnect, I believe the reply will just be discarded if I understand the >> code correctly. (That is a big if here ;) ) >> >> I've written a patch for Samba to stop sending interim replies on >> SMB2_READ >> and SMB2_WRITE, when non-compounded to stop the impact of this issue. We >> may want to add SMB2_CREATE to the list of ops we don't send async replies >> for non-compounded, but I'm not sold either way, I know we CAN go async >> there! I want some opinions here. >> >> This is not hidden behind a flag because compatibility issues that don't >> impact protocol correctness usually aren't. >> >> Setting req->async_te = talloc_new(NULL); is just ugly, though it works. >> If you have a cleaner approach, I welcome it. >> >> I request you please ASK me before pushing this one, yes, that means you >> jra! >> >> For those interested in reproduction: I'd suggest using a server that's >> rebuilt with a lower timeout set in smb2_read.c, though we've hit it with >> vfs_glusterfs straight up, in our testing. >> >> Thanks, >> >> -Ira / ira@(samba.org|redhat.com|wakeful.net) > > > > Thank you for catching this! > > It is the issue in the kernel client - a check for interim responses is > missed for SMB2_READ command. I created a patch that should fix the problem. > > Could you please test it? > > -- > Best regards, > Pavel Shilovsky CC'ing @samba-technical. Sorry for the spam. Tested-by: Shirish Pargaonkar From 09904bf0c6a1ecc7f61d4755db33b762b53176d6 Mon Sep 17 00:00:00 2001 From: Pavel Shilovsky Date: Sat, 27 Feb 2016 11:58:18 +0300 Subject: [PATCH] CIFS: Fix SMB2+ interim response processing for read requests For interim responses we only need to parse a header and update a number credits. Now it is done for all SMB2+ command except SMB2_READ which is wrong. Fix this by adding such processing. Signed-off-by: Pavel Shilovsky --- fs/cifs/cifssmb.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c index 90b4f9f..76fcb50 100644 --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -1396,11 +1396,10 @@ openRetry: * current bigbuf. */ static int -cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid) +discard_remaining_data(struct TCP_Server_Info *server) { unsigned int rfclen = get_rfc1002_length(server->smallbuf); int remaining = rfclen + 4 - server->total_read; - struct cifs_readdata *rdata = mid->callback_data; while (remaining > 0) { int length; @@ -1414,10 +1413,20 @@ cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid) remaining -= length; } - dequeue_mid(mid, rdata->result); return 0; } +static int +cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid) +{ + int length; + struct cifs_readdata *rdata = mid->callback_data; + + length = discard_remaining_data(server); + dequeue_mid(mid, rdata->result); + return length; +} + int cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid) { @@ -1446,6 +1455,12 @@ cifs_readv_receive(struct TCP_Server_Info *server, struct mid_q_entry *mid) return length; server->total_read += length; + if (server->ops->is_status_pending && + server->ops->is_status_pending(buf, server, 0)) { + discard_remaining_data(server); + return -1; + } + /* Was the SMB read successful? */ rdata->result = server->ops->map_error(buf, false); if (rdata->result != 0) { -- 2.1.4