From patchwork Mon Dec 5 20:37:13 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Ricardo Leitner X-Patchwork-Id: 702871 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3tXc5J4rCXz9svs for ; Tue, 6 Dec 2016 07:37:40 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="xaYsANfr"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752087AbcLEUhh (ORCPT ); Mon, 5 Dec 2016 15:37:37 -0500 Received: from mail-qt0-f196.google.com ([209.85.216.196]:34302 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751902AbcLEUhg (ORCPT ); Mon, 5 Dec 2016 15:37:36 -0500 Received: by mail-qt0-f196.google.com with SMTP id l20so37532221qta.1 for ; Mon, 05 Dec 2016 12:37:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=BQvwytOAkdmbo3p6NBzhIvZYCJqOynndYwPi9iJBVys=; b=xaYsANfroGXLySWpmvNkX5SberfT4gx39Puksyg//93iKoWMTNU802T/s7w/VTEAtz lVW5WpU1cNGWIGVS72wT/MX71EaHB29tzylP4bUBHwU06CVUBMFp4jKc9rvuT8aT3jAM utCJzOmYWG+KqYfpEcGYvd+louyG7S9MdRLIxvCrJAMrug0pQKj9Mu5ENpircgF25a+/ 8SJXkg42k9OpqUNpB6m38eCZmegzoOdtwR+seyZ+lpINlH8dQMO/FA5Y7++PFCzvdREU fPRjYL627ANpFFdG6KvnYmQGD3jYytH89wgf1mS3X3PtO5Y1TCqk1nCt30dSIufmj4kd EhcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=BQvwytOAkdmbo3p6NBzhIvZYCJqOynndYwPi9iJBVys=; b=GKNm7biJe8jmzJVJEj+BL1n6/4FJqO4rn1y5/q3iMICa0DmXxawwRdLrsiQaonDmMQ BlKlviRc3cUooZiXTJSKGZbFEi5pnl/E874FKzCegcJ40RoKtTaLfmxnDFUuivKY7+Ip T72DqQvTqyu8lKUf1cQHd8Pp3UUHbIdBWLBTg1paI+mnDGJ8TQW3kRc1cJ3ZdXKz+en3 ysfBVNrLqEd91YNUVT4DygYXPbcVO2oJ233MgycYCHaPwWzu9f1aAq5iw+JgE0w0aV3I vOwgZ+wBQfEmV6owVvFX15wlqttN2AbCCmcDCxys6WntsKtZ7AReb+QggwiADkL3/zT3 G7dA== X-Gm-Message-State: AKaTC00oeIP/CAGNUn5eBu1pZECmDZkZFJ6wcqLfex3UiFbvFuaNLQ1NQ6JMsFg9/0TgrQ== X-Received: by 10.237.60.11 with SMTP id t11mr53308255qte.227.1480970255062; Mon, 05 Dec 2016 12:37:35 -0800 (PST) Received: from localhost.localdomain.com ([187.112.187.85]) by smtp.gmail.com with ESMTPSA id 31sm10278765qty.30.2016.12.05.12.37.30 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Dec 2016 12:37:33 -0800 (PST) From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: Jon Maxwell , Alex Sidorenko , Alexey Kuznetsov , James Morris , Hideaki YOSHIFUJI , Patrick McHardy , tlfalcon@linux.vnet.ibm.com, Brian King , Eric Dumazet , davem@davemloft.net, marcelo.leitner@gmail.com Subject: [PATCH net v4] tcp: warn on bogus MSS and try to amend it Date: Mon, 5 Dec 2016 18:37:13 -0200 Message-Id: <2056cf96b896aa473ff017b9f223904a14bfed86.1480969929.git.marcelo.leitner@gmail.com> X-Mailer: git-send-email 2.9.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org There have been some reports lately about TCP connection stalls caused by NIC drivers that aren't setting gso_size on aggregated packets on rx path. This causes TCP to assume that the MSS is actually the size of the aggregated packet, which is invalid. Although the proper fix is to be done at each driver, it's often hard and cumbersome for one to debug, come to such root cause and report/fix it. This patch amends this situation in two ways. First, it adds a warning on when this situation occurs, so it gives a hint to those trying to debug this. It also limit the maximum probed MSS to the adverised MSS, as it should never be any higher than that. The result is that the connection may not have the best performance ever but it shouldn't stall, and the admin will have a hint on what to look for. Tested with virtio by forcing gso_size to 0. v2: updated msg per David's suggestion v3: use skb_iif to find the interface and also log its name, per Eric Dumazet's suggestion. As the skb may be backlogged and the interface gone by then, we need to check if the number still has a meaning. v4: use helper tcp_gro_dev_warn() and avoid pr_warn_once inside __once, per David's suggestion Cc: Jonathan Maxwell Signed-off-by: Marcelo Ricardo Leitner --- Thanks net/ipv4/tcp_input.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index a27b9c0e27c08b4e4aeaff3d0bfdf3ae561ba4d8..c71d49ce0c9379cd68317bcc135b7a2761110887 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -128,6 +128,23 @@ int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2; #define REXMIT_LOST 1 /* retransmit packets marked lost */ #define REXMIT_NEW 2 /* FRTO-style transmit of unsent/new packets */ +static void tcp_gro_dev_warn(struct sock *sk, const struct sk_buff *skb) +{ + static bool __once __read_mostly; + + if (!__once) { + struct net_device *dev; + + __once = true; + + rcu_read_lock(); + dev = dev_get_by_index_rcu(sock_net(sk), skb->skb_iif); + pr_warn("%s: Driver has suspect GRO implementation, TCP performance may be compromised.\n", + dev ? dev->name : "Unknown driver"); + rcu_read_unlock(); + } +} + /* Adapt the MSS value used to make delayed ack decision to the * real world. */ @@ -144,7 +161,10 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb) */ len = skb_shinfo(skb)->gso_size ? : skb->len; if (len >= icsk->icsk_ack.rcv_mss) { - icsk->icsk_ack.rcv_mss = len; + icsk->icsk_ack.rcv_mss = min_t(unsigned int, len, + tcp_sk(sk)->advmss); + if (unlikely(icsk->icsk_ack.rcv_mss != len)) + tcp_gro_dev_warn(sk, skb); } else { /* Otherwise, we make more careful check taking into account, * that SACKs block is variable.