From 40171248bb8934537fec8fbaf718e57c8add187c Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:01 -0400 Subject: [PATCH 1/4] sctp: allow IP fragmentation when PLPMTUD enters Error state Currently when PLPMTUD enters Error state, transport pathmtu will be set to MIN_PLPMTU(512) while probe is continuing with BASE_PLPMTU(1200). It will cause pathmtu to stay in a very small value, even if the real pmtu is some value like 1000. RFC8899 doesn't clearly say how to set the value in Error state. But one possibility could be keep using BASE_PLPMTU for the real pmtu, but allow to do IP fragmentation when it's in Error state. As it says in rfc8899#section-5.4: Some paths could be unable to sustain packets of the BASE_PLPMTU size. The Error State could be implemented to provide robustness to such paths. This allows fallback to a smaller than desired PLPMTU rather than suffer connectivity failure. This could utilize methods such as endpoint IP fragmentation to enable the PL sender to communicate using packets smaller than the BASE_PLPMTU. This patch is to set pmtu to BASE_PLPMTU instead of MIN_PLPMTU for Error state in sctp_transport_pl_send/toobig(), and set packet ipfragok for non-probe packets when it's in Error state. Fixes: 1dc68c194571 ("sctp: do state transition when PROBE_COUNT == MAX_PROBES on HB send path") Reported-by: Ying Xu Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/output.c | 13 ++++++++----- net/sctp/transport.c | 4 ++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/net/sctp/output.c b/net/sctp/output.c index 4dfb5ea82b05b..cdfdbd353c678 100644 --- a/net/sctp/output.c +++ b/net/sctp/output.c @@ -581,13 +581,16 @@ int sctp_packet_transmit(struct sctp_packet *packet, gfp_t gfp) chunk = list_entry(packet->chunk_list.next, struct sctp_chunk, list); sk = chunk->skb->sk; - /* check gso */ if (packet->size > tp->pathmtu && !packet->ipfragok && !chunk->pmtu_probe) { - if (!sk_can_gso(sk)) { - pr_err_once("Trying to GSO but underlying device doesn't support it."); - goto out; + if (tp->pl.state == SCTP_PL_ERROR) { /* do IP fragmentation if in Error state */ + packet->ipfragok = 1; + } else { + if (!sk_can_gso(sk)) { /* check gso */ + pr_err_once("Trying to GSO but underlying device doesn't support it."); + goto out; + } + gso = 1; } - gso = 1; } /* alloc head skb */ diff --git a/net/sctp/transport.c b/net/sctp/transport.c index a3d3ca6dd63dd..1f2dfad768d52 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -269,7 +269,7 @@ bool sctp_transport_pl_send(struct sctp_transport *t) if (t->pl.probe_size == SCTP_BASE_PLPMTU) { /* BASE_PLPMTU Confirmation Failed */ t->pl.state = SCTP_PL_ERROR; /* Base -> Error */ - t->pl.pmtu = SCTP_MIN_PLPMTU; + t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); sctp_assoc_sync_pmtu(t->asoc); } @@ -366,7 +366,7 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) if (pmtu >= SCTP_MIN_PLPMTU && pmtu < SCTP_BASE_PLPMTU) { t->pl.state = SCTP_PL_ERROR; /* Base -> Error */ - t->pl.pmtu = SCTP_MIN_PLPMTU; + t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); } } else if (t->pl.state == SCTP_PL_SEARCH) { From c6ea04ea692fa0d8e7faeb133fcd28e3acf470a0 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:02 -0400 Subject: [PATCH 2/4] sctp: reset probe_timer in sctp_transport_pl_update sctp_transport_pl_update() is called when transport update its dst and pathmtu, instead of stopping the PLPMTUD probe timer, PLPMTUD should start over and reset the probe timer. Otherwise, the PLPMTUD service would stop. Fixes: 92548ec2f1f9 ("sctp: add the probe timer in transport for PLPMTUD") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- include/net/sctp/sctp.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index 69bab88ad66b1..bc00410223b03 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -653,12 +653,10 @@ static inline void sctp_transport_pl_update(struct sctp_transport *t) if (t->pl.state == SCTP_PL_DISABLED) return; - if (del_timer(&t->probe_timer)) - sctp_transport_put(t); - t->pl.state = SCTP_PL_BASE; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pl.probe_size = SCTP_BASE_PLPMTU; + sctp_transport_reset_probe_timer(t); } static inline bool sctp_transport_pl_enabled(struct sctp_transport *t) From cc4665ca646c96181a7c00198aa72c59e0c576e8 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:03 -0400 Subject: [PATCH 3/4] sctp: subtract sctphdr len in sctp_transport_pl_hlen sctp_transport_pl_hlen() is called to calculate the outer header length for PL. However, as the Figure in rfc8899#section-4.4: Any additional headers .--- MPS -----. | | | v v v +------------------------------+ | IP | ** | PL | protocol data | +------------------------------+ <----- PLPMTU -----> <---------- PMTU --------------> Outer header are IP + Any additional headers, which doesn't include Packetization Layer itself header, namely sctphdr, whereas sctphdr is counted by __sctp_mtu_payload(). The incorrect calculation caused the link pathmtu to be set larger than expected by t->pl.pmtu + sctp_transport_pl_hlen(). This patch is to fix it by subtracting sctphdr len in sctp_transport_pl_hlen(). Fixes: d9e2e410ae30 ("sctp: add the constants/variables and states and some APIs for transport") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- include/net/sctp/sctp.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h index bc00410223b03..189fdb9db1622 100644 --- a/include/net/sctp/sctp.h +++ b/include/net/sctp/sctp.h @@ -626,7 +626,8 @@ static inline __u32 sctp_min_frag_point(struct sctp_sock *sp, __u16 datasize) static inline int sctp_transport_pl_hlen(struct sctp_transport *t) { - return __sctp_mtu_payload(sctp_sk(t->asoc->base.sk), t, 0, 0); + return __sctp_mtu_payload(sctp_sk(t->asoc->base.sk), t, 0, 0) - + sizeof(struct sctphdr); } static inline void sctp_transport_pl_reset(struct sctp_transport *t) From 75cf662c64dd8543f56c329c69eba18141c8fd9f Mon Sep 17 00:00:00 2001 From: Xin Long Date: Thu, 28 Oct 2021 05:36:04 -0400 Subject: [PATCH 4/4] sctp: return true only for pathmtu update in sctp_transport_pl_toobig sctp_transport_pl_toobig() supposes to return true only if there's pathmtu update, so that in sctp_icmp_frag_needed() it would call sctp_assoc_sync_pmtu() and sctp_retransmit(). This patch is to fix these return places in sctp_transport_pl_toobig(). Fixes: 836964083177 ("sctp: do state transition when receiving an icmp TOOBIG packet") Signed-off-by: Xin Long Signed-off-by: David S. Miller --- net/sctp/transport.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/sctp/transport.c b/net/sctp/transport.c index 1f2dfad768d52..133f1719bf1b7 100644 --- a/net/sctp/transport.c +++ b/net/sctp/transport.c @@ -368,6 +368,7 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } } else if (t->pl.state == SCTP_PL_SEARCH) { if (pmtu >= SCTP_BASE_PLPMTU && pmtu < t->pl.pmtu) { @@ -378,11 +379,10 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.probe_high = 0; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } else if (pmtu > t->pl.pmtu && pmtu < t->pl.probe_size) { t->pl.probe_size = pmtu; t->pl.probe_count = 0; - - return false; } } else if (t->pl.state == SCTP_PL_COMPLETE) { if (pmtu >= SCTP_BASE_PLPMTU && pmtu < t->pl.pmtu) { @@ -393,10 +393,11 @@ static bool sctp_transport_pl_toobig(struct sctp_transport *t, u32 pmtu) t->pl.probe_high = 0; t->pl.pmtu = SCTP_BASE_PLPMTU; t->pathmtu = t->pl.pmtu + sctp_transport_pl_hlen(t); + return true; } } - return true; + return false; } bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)