From patchwork Fri Jan 5 13:17:17 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Ricardo Leitner X-Patchwork-Id: 856036 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="fmQl4so8"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zClZz4KR8z9ryr for ; Sat, 6 Jan 2018 00:17:47 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752139AbeAENRq (ORCPT ); Fri, 5 Jan 2018 08:17:46 -0500 Received: from mail-qt0-f193.google.com ([209.85.216.193]:33302 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751689AbeAENRc (ORCPT ); Fri, 5 Jan 2018 08:17:32 -0500 Received: by mail-qt0-f193.google.com with SMTP id e2so5644945qti.0; Fri, 05 Jan 2018 05:17:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=HczqmKEXbiDqAiKpM1DQX3amjriJlp9yggpz8Y8hTL4=; b=fmQl4so8pwtwMKsYDiPOly0O5luR8gr03oBKl/UEsRHf0sRcj3RRA1yq27eONonCyS YIgvov66y4qC8O7Ttnwx1ROweP67/xEpAEszy/EB74BtM1PWbKuEOUU1ulejbI9phSHr JTJ3HglDdc/2mAAyQyYRYUoAXNTt3OvXUfGDLJdWm1rKc/sxvX++Q0wobQF7tQddXlno sfZe+YNL79tSNDrNX4MwKrayyWG1rKm4IL/PjJB0MGDSAG/agVyx0IBOwhQr9NFkgiC0 Z3olUXcaEuuvD238WOT0WUkgZNZzyNF2r6HfCPh/p8Uibm4OPt0VuL43EL/kwq5v6CMJ LGQQ== 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:in-reply-to :references; bh=HczqmKEXbiDqAiKpM1DQX3amjriJlp9yggpz8Y8hTL4=; b=R3qx+M/Jpa+C8yCp2qoaOpveTn/VvzgYyu1CIOJBwO1sSAezyl55sRIFIUKBfTPEHo 7JA9ipeyR2xGEV4qKNVsz5/Jeehd1ivI5qvnv5gr/rEIrcHgN7/yiEgJMJMwbAtr4ZUV kwzD1vgktKB7YKeOVLZydsJkoeDvIv+GeClD3F3ZtiB2CS3F5ZZmHYhbK+Q8Y5Xh1y8d fA9Mu6CaGhjeFaZgx+0bfbQ8BMOGmk9Zmjq/ueXJpJnGJK8C0yGiwn0AwhNs9IHGPzVM GVp9L9+VFiIiMr3cae53s8U7CSX+0uBmvfjWyKHNfuEa8LAf87yYgg7kiDXeKTRcIMyZ 4Odg== X-Gm-Message-State: AKwxyteEYVxXh+DMhBS7StpNaO8URxwp1CfPX6qJMKzhgpwpeBsYn//h Wvx5Esj9N6ay841Ftsp3EIs1fg== X-Google-Smtp-Source: ACJfBouuETm6ZIbqtbuq0gPVNM7tahP6Rge7SRQt/dcDkSqfLJ1gZefYmwKpAhlCPnT9/EIvXLrifQ== X-Received: by 10.200.45.148 with SMTP id p20mr4003124qta.249.1515158251971; Fri, 05 Jan 2018 05:17:31 -0800 (PST) Received: from localhost.localdomain.com ([2001:1284:f013:23a9:9544:d086:4c8e:2344]) by smtp.gmail.com with ESMTPSA id u36sm3545562qtc.58.2018.01.05.05.17.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 05 Jan 2018 05:17:31 -0800 (PST) From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: linux-sctp@vger.kernel.org, Xin Long , Vlad Yasevich , Neil Horman , marcelo.leitner@gmail.com Subject: [PATCH net v3 1/2] sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled Date: Fri, 5 Jan 2018 11:17:17 -0200 Message-Id: X-Mailer: git-send-email 2.14.3 In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Currently, if PMTU discovery is disabled on a given transport, but the configured value is higher than the actual PMTU, it is likely that we will get some icmp Frag Needed. The issue is, if PMTU discovery is disabled, we won't update the information and will issue a retransmission immediately, which may very well trigger another ICMP, and another retransmission, leading to a loop. The fix is to simply not trigger immediate retransmissions if PMTU discovery is disabled on the given transport. Changes from v2: - updated stale comment, noticed by Xin Long Signed-off-by: Marcelo Ricardo Leitner --- net/sctp/input.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/net/sctp/input.c b/net/sctp/input.c index 621b5ca3fd1c17c3d7ef7bb1c7677ab98cebbe77..9320661cc41da0b280f69f379128ab7d062e5528 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -399,20 +399,20 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc, return; } - if (t->param_flags & SPP_PMTUD_ENABLE) { - /* Update transports view of the MTU */ - sctp_transport_update_pmtu(t, pmtu); + if (!(t->param_flags & SPP_PMTUD_ENABLE)) + /* We can't allow retransmitting in such case, as the + * retransmission would be sized just as before, and thus we + * would get another icmp, and retransmit again. + */ + return; - /* Update association pmtu. */ - sctp_assoc_sync_pmtu(asoc); - } + /* Update transports view of the MTU */ + sctp_transport_update_pmtu(t, pmtu); - /* Retransmit with the new pmtu setting. - * Normally, if PMTU discovery is disabled, an ICMP Fragmentation - * Needed will never be sent, but if a message was sent before - * PMTU discovery was disabled that was larger than the PMTU, it - * would not be fragmented, so it must be re-transmitted fragmented. - */ + /* Update association pmtu. */ + sctp_assoc_sync_pmtu(asoc); + + /* Retransmit with the new pmtu setting. */ sctp_retransmit(&asoc->outqueue, t, SCTP_RTXR_PMTUD); } From patchwork Fri Jan 5 13:17:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marcelo Ricardo Leitner X-Patchwork-Id: 856035 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="j9nVfQkK"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3zClZs11P3z9s7c for ; Sat, 6 Jan 2018 00:17:41 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752012AbeAENRi (ORCPT ); Fri, 5 Jan 2018 08:17:38 -0500 Received: from mail-qt0-f194.google.com ([209.85.216.194]:42750 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751422AbeAENRf (ORCPT ); Fri, 5 Jan 2018 08:17:35 -0500 Received: by mail-qt0-f194.google.com with SMTP id g9so5632885qth.9; Fri, 05 Jan 2018 05:17:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=B8SRrgu0ni6dLJhKh1AzMGnEJzuG2u0UnBOpW0HSj5g=; b=j9nVfQkKIqJzPpE8+AxoQFrmnjjuCmspLQ9P0kTWdp2f3fnfutzO7q4Lc+MgIwNVT1 UgxxHkZLCKQRnLIugxrfZcMCX84HT9fYbYYwem/AnlrI2w/A05+CfxZuPWgqbQ0b5JCo 6hRHKOCkAqMUFKa1YilDcskdqFwW232ma/q9xAYiQJj4eIGJW1yS08HUi3uzi3SCWHqH 9ysB0VxEcULST1QJBf5U6mvicQgOfYbeWeDJ+SDHGBN6piqlH2BYEzxXJWpvAQamWqjk DlFZGXhzwTI0CD8u688F7ijB8aSn3ooT4BYajyFzGgDbcOArYEcE+fE2R2ZkiBzHrzAI rwmg== 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:in-reply-to :references; bh=B8SRrgu0ni6dLJhKh1AzMGnEJzuG2u0UnBOpW0HSj5g=; b=ITilkZg/0e/b0uaWArHjnzN2PXNH69LCOdkFXZf+GEDRKIkh6NI4kD9tnxVmPVFbox AW8/5vjIEGqOb0p6UI9mhWV/YLtIYy5Vs0xbxo7evOL0ojZYG8cZKOU/NOehR06343BR AI917WwPXDWMwwPlnE0YVQ/kiQsU0DuGwQdltrbeT/qvSwXvp03KtA/1Xl/sSyQXMbuZ TJKnAWJ0dmhf05HV+eSNAfp25W3INGeaLNgnRAZK2TckLRMJf0IdV9xB5Sq4qBqcGE9w xfQajwHcAzA0JUGkFBdcamjCwJ+eaBkA/dRVr4L3pFttfch0J0+9/fmS+dmJ4KpAFPqW F2tg== X-Gm-Message-State: AKwxyte3ulicc+0qOCOVH40DEBzSF3HjWj+zmtze+ao6yMbaEA77NblL zl/0PjCy+2Ss+9BIK1nIROn4YQ== X-Google-Smtp-Source: ACJfBouoXHSZtAXwMz71AulWRpsLwYKbBcse/pz8EryyC7Mh8/XpZnNrVaxRheqJMRjdoE4cay2b0A== X-Received: by 10.237.40.6 with SMTP id r6mr4137118qtd.40.1515158254464; Fri, 05 Jan 2018 05:17:34 -0800 (PST) Received: from localhost.localdomain.com ([2001:1284:f013:23a9:9544:d086:4c8e:2344]) by smtp.gmail.com with ESMTPSA id u36sm3545562qtc.58.2018.01.05.05.17.32 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 05 Jan 2018 05:17:33 -0800 (PST) From: Marcelo Ricardo Leitner To: netdev@vger.kernel.org Cc: linux-sctp@vger.kernel.org, Xin Long , Vlad Yasevich , Neil Horman , marcelo.leitner@gmail.com Subject: [PATCH net v3 2/2] sctp: fix the handling of ICMP Frag Needed for too small MTUs Date: Fri, 5 Jan 2018 11:17:18 -0200 Message-Id: <3927dca7e199eec2e12f4b99f851bb2891c2b9b5.1515152627.git.marcelo.leitner@gmail.com> X-Mailer: git-send-email 2.14.3 In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org syzbot reported a hang involving SCTP, on which it kept flooding dmesg with the message: [ 246.742374] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too low, using default minimum of 512 That happened because whenever SCTP hits an ICMP Frag Needed, it tries to adjust to the new MTU and triggers an immediate retransmission. But it didn't consider the fact that MTUs smaller than the SCTP minimum MTU allowed (512) would not cause the PMTU to change, and issued the retransmission anyway (thus leading to another ICMP Frag Needed, and so on). As IPv4 (ip_rt_min_pmtu=556) and IPv6 (IPV6_MIN_MTU=1280) minimum MTU are higher than that, sctp_transport_update_pmtu() is changed to re-fetch the PMTU that got set after our request, and with that, detect if there was an actual change or not. The fix, thus, skips the immediate retransmission if the received ICMP resulted in no change, in the hope that SCTP will select another path. Note: The value being used for the minimum MTU (512, SCTP_DEFAULT_MINSEGMENT) is not right and instead it should be (576, SCTP_MIN_PMTU), but such change belongs to another patch. Changes from v1: - do not disable PMTU discovery, in the light of commit 06ad391919b2 ("[SCTP] Don't disable PMTU discovery when mtu is small") and as suggested by Xin Long. - changed the way to break the rtx loop by detecting if the icmp resulted in a change or not Changes from v2: none See-also: https://lkml.org/lkml/2017/12/22/811 Reported-by: syzbot Signed-off-by: Marcelo Ricardo Leitner --- include/net/sctp/structs.h | 2 +- net/sctp/input.c | 8 ++++++-- net/sctp/transport.c | 29 +++++++++++++++++++---------- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index 2f8f93da5dc2660f4db37c04f8a434809b3120a1..9a5ccf03a59b1e0a4820e2a978c66f1df8a9a82f 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -966,7 +966,7 @@ void sctp_transport_burst_limited(struct sctp_transport *); void sctp_transport_burst_reset(struct sctp_transport *); unsigned long sctp_transport_timeout(struct sctp_transport *); void sctp_transport_reset(struct sctp_transport *t); -void sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu); +bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu); void sctp_transport_immediate_rtx(struct sctp_transport *); void sctp_transport_dst_release(struct sctp_transport *t); void sctp_transport_dst_confirm(struct sctp_transport *t); diff --git a/net/sctp/input.c b/net/sctp/input.c index 9320661cc41da0b280f69f379128ab7d062e5528..141c9c466ec172ff67930cf38eb02a49e01a12ab 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -406,8 +406,12 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc, */ return; - /* Update transports view of the MTU */ - sctp_transport_update_pmtu(t, pmtu); + /* Update transports view of the MTU. Return if no update was needed. + * If an update wasn't needed/possible, it also doesn't make sense to + * try to retransmit now. + */ + if (!sctp_transport_update_pmtu(t, pmtu)) + return; /* Update association pmtu. */ sctp_assoc_sync_pmtu(asoc); diff --git a/net/sctp/transport.c b/net/sctp/transport.c index 1e5a22430cf56e40a6f323081beb97836b506384..47f82bd794d915188bad037463c2aa14175a55ef 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -248,28 +248,37 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk) transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT; } -void sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu) +bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu) { struct dst_entry *dst = sctp_transport_dst_check(t); + bool change = true; if (unlikely(pmtu < SCTP_DEFAULT_MINSEGMENT)) { - pr_warn("%s: Reported pmtu %d too low, using default minimum of %d\n", - __func__, pmtu, SCTP_DEFAULT_MINSEGMENT); - /* Use default minimum segment size and disable - * pmtu discovery on this transport. - */ - t->pathmtu = SCTP_DEFAULT_MINSEGMENT; - } else { - t->pathmtu = pmtu; + pr_warn_ratelimited("%s: Reported pmtu %d too low, using default minimum of %d\n", + __func__, pmtu, SCTP_DEFAULT_MINSEGMENT); + /* Use default minimum segment instead */ + pmtu = SCTP_DEFAULT_MINSEGMENT; } + pmtu = SCTP_TRUNC4(pmtu); if (dst) { dst->ops->update_pmtu(dst, t->asoc->base.sk, NULL, pmtu); dst = sctp_transport_dst_check(t); } - if (!dst) + if (!dst) { t->af_specific->get_dst(t, &t->saddr, &t->fl, t->asoc->base.sk); + dst = t->dst; + } + + if (dst) { + /* Re-fetch, as under layers may have a higher minimum size */ + pmtu = SCTP_TRUNC4(dst_mtu(dst)); + change = t->pathmtu != pmtu; + } + t->pathmtu = pmtu; + + return change; } /* Caches the dst entry and source address for a transport's destination