From patchwork Wed Aug 22 05:15:03 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 1359151 Return-Path: X-Original-To: patchwork-linux-wireless@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id B736B3FD40 for ; Wed, 22 Aug 2012 05:15:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754091Ab2HVFPK (ORCPT ); Wed, 22 Aug 2012 01:15:10 -0400 Received: from mail-wg0-f44.google.com ([74.125.82.44]:58297 "EHLO mail-wg0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753091Ab2HVFPJ (ORCPT ); Wed, 22 Aug 2012 01:15:09 -0400 Received: by wgbdr13 with SMTP id dr13so496604wgb.1 for ; Tue, 21 Aug 2012 22:15:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; bh=jAOJupbI80lLvA8xkYh3GIJxamMDTkZq0X/jlzbXvoE=; b=0t4K1MHm66f26QsIQZCMFV1FvnSn9pUVDBY+IuRw8Na0b5+K+0FTZDdaJw17O8dG0p 0xhfYqEgJG7y3qgNrASsE9ChvnnaM1/+AUCkMJcJVivg4eoBO3BCjcHbGDgIaEPSRGp9 EkbkFJ006sH5f2SRUzVKfdgvzITm+nrtp4z5ri8O6D0ZlKAZFjTQQxlSiOcFwKlxUk/Y 475Xuu7nSbpFMRugxfxV766kBIdlL36242n3FjQTX42895Bbd0ofbJ7DWhvaNfpfZv+l KN24lFLbfBGnH5XgadBLJ+aF/+bLRs5wlnT9VG0uk9af3LwIuxmX5ixlzvmi88FQKLAK kw6A== Received: by 10.216.139.145 with SMTP id c17mr10231588wej.175.1345612507384; Tue, 21 Aug 2012 22:15:07 -0700 (PDT) Received: from [172.28.91.92] ([74.125.122.49]) by mx.google.com with ESMTPS id ep14sm38120870wid.0.2012.08.21.22.15.05 (version=SSLv3 cipher=OTHER); Tue, 21 Aug 2012 22:15:06 -0700 (PDT) Subject: Re: Regression associated with commit c8628155ece3 - "tcp: reduce out_of_order memory use" From: Eric Dumazet To: Larry Finger Cc: Neal Cardwell , "David S. Miller" , John W Linville , linux-wireless , LKML In-Reply-To: <50345B12.1050600@lwfinger.net> References: <50345B12.1050600@lwfinger.net> Date: Wed, 22 Aug 2012 07:15:03 +0200 Message-ID: <1345612503.5158.566.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Tue, 2012-08-21 at 23:07 -0500, Larry Finger wrote: > Hi, > > The commit entitled "tcp: reduce out_of_order memory use" turns out to cause > problems with a number of USB drivers. > > The first one called to my attention was for staging/r8712u. For this driver, > there are problems with SSL communications as reported in > https://bugzilla.redhat.com/show_bug.cgi?id=847525. > > Other drivers including rtl8187, rtl8192cu and rt2800usb cannot connect to a > WPA1- or WEP-encrypted AP, but work fine with WPA2. The rtl8192cu problem is > reported in https://bugzilla.kernel.org/show_bug.cgi?id=46171. > > I find it hard to understand why this patch should cause these effects; however, > I have verified that a kernel generated from "git checkout e86b2919" works fine. > This commit is immediately before the patch in question. > > Note, this patch was applied during the merge between 3.3 and 3.4. The > regression has been undiscovered for some time. > > Any help on this problem will be appreciated. > > Larry This particular commit is the start of a patches batch that ended in the generic TCP coalescing mechanism. It is known to have problem on drivers doing skb_clone() in their rx path. Current kernels should be ok, because coalescing doesnt happen if the destination skb is cloned (skb_cloned(to) in skb_try_coalesce()) For 3.4 kernel, I guess I need to backport this skb_cloned(to) check fo stable 3.4 kernel But these skb_clone() in various USB drivers should be killed for good, they really can kill the box because of skb->truesize lies. Please test following patch (for 3.4 kernels) --- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 257b617..c45ac2d 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4496,7 +4496,9 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb) * to avoid future tcp_collapse_ofo_queue(), * probably the most expensive function in tcp stack. */ - if (skb->len <= skb_tailroom(skb1) && !tcp_hdr(skb)->fin) { + if (skb->len <= skb_tailroom(skb1) && + !skb_cloned(skb1) && + !tcp_hdr(skb)->fin) { NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPRCVCOALESCE); BUG_ON(skb_copy_bits(skb, 0,