* [PATCH net 1/4] sctp: allow IP fragmentation when PLPMTUD enters Error state
2021-10-28 9:36 [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD Xin Long
@ 2021-10-28 9:36 ` Xin Long
2021-10-28 9:36 ` [PATCH net 2/4] sctp: reset probe_timer in sctp_transport_pl_update Xin Long
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2021-10-28 9:36 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, kuba, Marcelo Ricardo Leitner
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 <yinxu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
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 4dfb5ea82b05..cdfdbd353c67 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 a3d3ca6dd63d..1f2dfad768d5 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) {
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/4] sctp: reset probe_timer in sctp_transport_pl_update
2021-10-28 9:36 [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD Xin Long
2021-10-28 9:36 ` [PATCH net 1/4] sctp: allow IP fragmentation when PLPMTUD enters Error state Xin Long
@ 2021-10-28 9:36 ` Xin Long
2021-10-28 9:36 ` [PATCH net 3/4] sctp: subtract sctphdr len in sctp_transport_pl_hlen Xin Long
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2021-10-28 9:36 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, kuba, Marcelo Ricardo Leitner
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 <lucien.xin@gmail.com>
---
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 69bab88ad66b..bc00410223b0 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)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 3/4] sctp: subtract sctphdr len in sctp_transport_pl_hlen
2021-10-28 9:36 [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD Xin Long
2021-10-28 9:36 ` [PATCH net 1/4] sctp: allow IP fragmentation when PLPMTUD enters Error state Xin Long
2021-10-28 9:36 ` [PATCH net 2/4] sctp: reset probe_timer in sctp_transport_pl_update Xin Long
@ 2021-10-28 9:36 ` Xin Long
2021-10-28 9:36 ` [PATCH net 4/4] sctp: return true only for pathmtu update in sctp_transport_pl_toobig Xin Long
2021-10-29 11:40 ` [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2021-10-28 9:36 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, kuba, Marcelo Ricardo Leitner
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 <lucien.xin@gmail.com>
---
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 bc00410223b0..189fdb9db162 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)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 4/4] sctp: return true only for pathmtu update in sctp_transport_pl_toobig
2021-10-28 9:36 [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD Xin Long
` (2 preceding siblings ...)
2021-10-28 9:36 ` [PATCH net 3/4] sctp: subtract sctphdr len in sctp_transport_pl_hlen Xin Long
@ 2021-10-28 9:36 ` Xin Long
2021-10-29 11:40 ` [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: Xin Long @ 2021-10-28 9:36 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: davem, kuba, Marcelo Ricardo Leitner
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 <lucien.xin@gmail.com>
---
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 1f2dfad768d5..133f1719bf1b 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)
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD
2021-10-28 9:36 [PATCH net 0/4] sctp: a couple of fixes for PLPMTUD Xin Long
` (3 preceding siblings ...)
2021-10-28 9:36 ` [PATCH net 4/4] sctp: return true only for pathmtu update in sctp_transport_pl_toobig Xin Long
@ 2021-10-29 11:40 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-10-29 11:40 UTC (permalink / raw)
To: Xin Long; +Cc: netdev, linux-sctp, davem, kuba, marcelo.leitner
Hello:
This series was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Thu, 28 Oct 2021 05:36:00 -0400 you wrote:
> Four fixes included in this patchset:
>
> - fix the packet sending in Error state.
> - fix the timer stop when transport update dst.
> - fix the outer header len calculation.
> - fix the return value for toobig processing.
>
> [...]
Here is the summary with links:
- [net,1/4] sctp: allow IP fragmentation when PLPMTUD enters Error state
https://git.kernel.org/netdev/net/c/40171248bb89
- [net,2/4] sctp: reset probe_timer in sctp_transport_pl_update
https://git.kernel.org/netdev/net/c/c6ea04ea692f
- [net,3/4] sctp: subtract sctphdr len in sctp_transport_pl_hlen
https://git.kernel.org/netdev/net/c/cc4665ca646c
- [net,4/4] sctp: return true only for pathmtu update in sctp_transport_pl_toobig
https://git.kernel.org/netdev/net/c/75cf662c64dd
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread