SCTP PMTU discovery fixes
Subsystems (3)
Marcelo Ricardo Leitner
maintainerv3 cover SCTP PMTU discovery fixes
This patchset fixes 2 issues with PMTU discovery that can lead to flood of retransmissions.
The first patch fixes the issue for when PMTUD is disabled by the application, while the second fixes it for when its enabled.
Please consider these to stable.
Thanks,
Marcelo Ricardo Leitner (2):
sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
sctp: fix the handling of ICMP Frag Needed for too small MTUs
include/net/sctp/structs.h | 2 +- net/sctp/input.c | 28 ++++++++++++++++------------ net/sctp/transport.c | 29 +++++++++++++++++++---------- 3 files changed, 36 insertions(+), 23 deletions(-)
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH v3 1/2 sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
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
--- net/sctp/input.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
net/sctp/input.c +12 -12
@@ -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 ); }
Signature · 1 line
2.14.3
Marcelo Ricardo Leitner
maintainerPATCH v3 2/2 sctp: fix the handling of ICMP Frag Needed for too small MTUs
In reply to Marcelo Ricardo Leitnersyzbot 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_pmtuU6) and IPv6 (IPV6_MIN_MTU80) 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
--- include/net/sctp/structs.h | 2 +- net/sctp/input.c | 8 ++++++-- net/sctp/transport.c | 29 +++++++++++++++++++---------- 3 files changed, 26 insertions(+), 13 deletions(-)
include/net/sctp/structs.h +1 -1
@@ -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);
net/sctp/input.c +6 -2
@@ -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);
net/sctp/transport.c +19 -10
@@ -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
Signature · 1 line
2.14.3
Quoted 9 lines · Marcelo Ricardo Leitner
This patchset fixes 2 issues with PMTU discovery that can lead to flood of retransmissions.
The first patch fixes the issue for when PMTUD is disabled by the application, while the second fixes it for when its enabled.Please consider these to stable.
Thanks,
Quoted 17 lines
Marcelo Ricardo Leitner (2):
sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
sctp: fix the handling of ICMP Frag Needed for too small MTUsinclude/net/sctp/structs.h | 2 +- net/sctp/input.c | 28 ++++++++++++++++------------ net/sctp/transport.c | 29 +++++++++++++++++++---------- 3 files changed, 36 insertions(+), 23 deletions(-)Signature · 6 lines
2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Series
Quoted 13 lines · Marcelo Ricardo Leitner
This patchset fixes 2 issues with PMTU discovery that can lead to flood of retransmissions.
The first patch fixes the issue for when PMTUD is disabled by the application, while the second fixes it for when its enabled.Please consider these to stable.
Thanks,
Marcelo Ricardo Leitner (2):
sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
sctp: fix the handling of ICMP Frag Needed for too small MTUs
Quoted 8 lines
include/net/sctp/structs.h | 2 +- net/sctp/input.c | 28 ++++++++++++++++------------ net/sctp/transport.c | 29 +++++++++++++++++++---------- 3 files changed, 36 insertions(+), 23 deletions(-)--
2.14.3
David Miller
maintainerFrom: Marcelo Ricardo Leitner <[email protected]>
Date: Fri, 5 Jan 2018 11:17:16 -0200
Quoted 6 lines
This patchset fixes 2 issues with PMTU discovery that can lead to flood of retransmissions.
The first patch fixes the issue for when PMTUD is disabled by the application, while the second fixes it for when its enabled.Please consider these to stable.
Applied and queued up for -stable, thank you.