From patchwork Sat Apr 1 14:00:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Ricardo Leitner X-Patchwork-Id: 745996 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 3vwKlL4jyfz9ryv for ; Sun, 2 Apr 2017 01:00:46 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="a0r7GlpY"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751474AbdDAOAk (ORCPT ); Sat, 1 Apr 2017 10:00:40 -0400 Received: from mail-qk0-f195.google.com ([209.85.220.195]:33429 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751437AbdDAOAj (ORCPT ); Sat, 1 Apr 2017 10:00:39 -0400 Received: by mail-qk0-f195.google.com with SMTP id p22so13875821qka.0 for ; Sat, 01 Apr 2017 07:00:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=8BjkBf9Zk7jsD0FRrFjQ3fp80l0fXx2I0vaNGhsxBLM=; b=a0r7GlpYWFlrWGI6DZwu70Cky0UK3nkSehcOaAQQk2lxK8xsi0gYdpwp9u6X5/grzv RGLvh984S4I2j8qJFp67QsZVgHfL+Ew+bO7Wdvclrxj+Cy7EkaciPXAoZVF2IjHFK2kb LrYgSjpt1o9CtwGc6dsCik9bwr4tp/87u06ACIC47uZP0SQGinCY04IIyz240XhGv9gg URuz44WL7vVTqXo6B24aS7KfcZ9bOSpvbO3qtOLT5+aoSLo5IDEyr0Q1iJR02dZzXrEd yrZv1mosBWETlvvek0KJExzS+rx0hOgAKNnwVoW04Wevp0RaigpBCCa0xXZDtb1jDQBk u9Ow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=8BjkBf9Zk7jsD0FRrFjQ3fp80l0fXx2I0vaNGhsxBLM=; b=sdcbWrHIeans4d44y9LsKb9qCLBml47VbwhUgl0Elj7AcT5p37rTqjZf9pkcrHRu3l l2qcdORNQqWeBWBj/q3nP6QIOdKLchoPYVTF6PzpH3i4gBYn9yPmXjyr0gipq/o/kMcK q5JAtdZvbx7KSvWtLyb9lbtCspvJrVe0mMyM9tufU1x8vc6799DrOtq87/BfemSHPnGv 9uLtVLNM2eupivDCnU/4wx/+Xs0qOALkuPX9jkIFOiiEak5TgEb+ZKw2PJKhydiDe8CD zcxqy6bkVcr3x84c1zYGh/lYRzz8bHeBSFZSy5JYJxv7JbyJVl4dBk2lOPrXwTMqbpJZ o26w== X-Gm-Message-State: AFeK/H1lqYYIiY7fJ363E0osJ/kkZe+asVEZoDupoF9+qVSKPvfbdelb/vb1s3OTtqy6Zg== X-Received: by 10.55.5.210 with SMTP id 201mr7556085qkf.249.1491055238480; Sat, 01 Apr 2017 07:00:38 -0700 (PDT) Received: from localhost.localdomain.com ([168.194.163.141]) by smtp.gmail.com with ESMTPSA id n77sm3291848qkn.6.2017.04.01.07.00.34 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 01 Apr 2017 07:00:36 -0700 (PDT) From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: Eric Dumazet , Jon Maxwell , Markus Trippelsdorf Subject: [PATCH net] tcp: minimize false-positives on TCP/GRO check Date: Sat, 1 Apr 2017 11:00:21 -0300 Message-Id: <47c94077ffa8988118a19514389b42fccdc2f6ef.1491054871.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 Markus Trippelsdorf reported that after commit dcb17d22e1c2 ("tcp: warn on bogus MSS and try to amend it") the kernel started logging the warning for a NIC driver that doesn't even support GRO. It was diagnosed that it was possibly caused on connections that were using TCP Timestamps but some packets lacked the Timestamps option. As we reduce rcv_mss when timestamps are used, the lack of them would cause the packets to be bigger than expected, although this is a valid case. As this warning is more as a hint, getting a clean-cut on the threshold is probably not worth the execution time spent on it. This patch thus alleviates the false-positives with 2 quick checks: by accounting for the entire TCP option space and also checking against the interface MTU if it's available. These changes, specially the MTU one, might mask some real positives, though if they are really happening, it's possible that sooner or later it will be triggered anyway. Reported-by: Markus Trippelsdorf Cc: Eric Dumazet Signed-off-by: Marcelo Ricardo Leitner --- net/ipv4/tcp_input.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index c43119726a62e494063fd940001b483215d0fe26..97ac6776e47d36ea5c59051714a0f77e3eeb46fc 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -126,7 +126,8 @@ 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 void tcp_gro_dev_warn(struct sock *sk, const struct sk_buff *skb, + unsigned int len) { static bool __once __read_mostly; @@ -137,8 +138,9 @@ static void tcp_gro_dev_warn(struct sock *sk, const struct sk_buff *skb) 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"); + if (!dev || len >= dev->mtu) + pr_warn("%s: Driver has suspect GRO implementation, TCP performance may be compromised.\n", + dev ? dev->name : "Unknown driver"); rcu_read_unlock(); } } @@ -161,8 +163,10 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb) if (len >= icsk->icsk_ack.rcv_mss) { 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); + /* Account for possibly-removed options */ + if (unlikely(len > icsk->icsk_ack.rcv_mss + + MAX_TCP_OPTION_SPACE)) + tcp_gro_dev_warn(sk, skb, len); } else { /* Otherwise, we make more careful check taking into account, * that SACKs block is variable.