Commit 3822a5ff authored by Marcelo Ricardo Leitner's avatar Marcelo Ricardo Leitner Committed by David S. Miller
Browse files

sctp: align MTU to a word



SCTP is a protocol that is aligned to a word (4 bytes). Thus using bare
MTU can sometimes return values that are not aligned, like for loopback,
which is 65536 but ipv4_mtu() limits that to 65535. This mis-alignment
will cause the last non-aligned bytes to never be used and can cause
issues with congestion control.

So it's better to just consider a lower MTU and keep congestion control
calcs saner as they are based on PMTU.

Same applies to icmp frag needed messages, which is also fixed by this
patch.

One other effect of this is the inability to send MTU-sized packet
without queueing or fragmentation and without hitting Nagle. As the
check performed at sctp_packet_can_append_data():

if (chunk->skb->len + q->out_qlen >= transport->pathmtu - packet->overhead)
	/* Enough data queued to fill a packet */
	return SCTP_XMIT_OK;

with the above example of MTU, if there are no other messages queued,
one cannot send a packet that just fits one packet (65532 bytes) and
without causing DATA chunk fragmentation or a delay.

v2:
 - Added WORD_TRUNC macro
Signed-off-by: default avatarMarcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 31b055ef
...@@ -82,6 +82,11 @@ ...@@ -82,6 +82,11 @@
#define SCTP_PROTOSW_FLAG INET_PROTOSW_PERMANENT #define SCTP_PROTOSW_FLAG INET_PROTOSW_PERMANENT
#endif #endif
/* Round an int up to the next multiple of 4. */
#define WORD_ROUND(s) (((s)+3)&~3)
/* Truncate to the previous multiple of 4. */
#define WORD_TRUNC(s) ((s)&~3)
/* /*
* Function declarations. * Function declarations.
*/ */
...@@ -475,9 +480,6 @@ for (pos = chunk->subh.fwdtsn_hdr->skip;\ ...@@ -475,9 +480,6 @@ for (pos = chunk->subh.fwdtsn_hdr->skip;\
(void *)pos <= (void *)chunk->subh.fwdtsn_hdr->skip + end - sizeof(struct sctp_fwdtsn_skip);\ (void *)pos <= (void *)chunk->subh.fwdtsn_hdr->skip + end - sizeof(struct sctp_fwdtsn_skip);\
pos++) pos++)
/* Round an int up to the next multiple of 4. */
#define WORD_ROUND(s) (((s)+3)&~3)
/* External references. */ /* External references. */
extern struct proto sctp_prot; extern struct proto sctp_prot;
......
...@@ -1406,7 +1406,8 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc) ...@@ -1406,7 +1406,8 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc)
list_for_each_entry(t, &asoc->peer.transport_addr_list, list_for_each_entry(t, &asoc->peer.transport_addr_list,
transports) { transports) {
if (t->pmtu_pending && t->dst) { if (t->pmtu_pending && t->dst) {
sctp_transport_update_pmtu(sk, t, dst_mtu(t->dst)); sctp_transport_update_pmtu(sk, t,
WORD_TRUNC(dst_mtu(t->dst)));
t->pmtu_pending = 0; t->pmtu_pending = 0;
} }
if (!pmtu || (t->pathmtu < pmtu)) if (!pmtu || (t->pathmtu < pmtu))
......
...@@ -606,7 +606,8 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info) ...@@ -606,7 +606,8 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
/* PMTU discovery (RFC1191) */ /* PMTU discovery (RFC1191) */
if (ICMP_FRAG_NEEDED == code) { if (ICMP_FRAG_NEEDED == code) {
sctp_icmp_frag_needed(sk, asoc, transport, info); sctp_icmp_frag_needed(sk, asoc, transport,
WORD_TRUNC(info));
goto out_unlock; goto out_unlock;
} else { } else {
if (ICMP_PROT_UNREACH == code) { if (ICMP_PROT_UNREACH == code) {
......
...@@ -226,7 +226,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk) ...@@ -226,7 +226,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
} }
if (transport->dst) { if (transport->dst) {
transport->pathmtu = dst_mtu(transport->dst); transport->pathmtu = WORD_TRUNC(dst_mtu(transport->dst));
} else } else
transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT; transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
} }
...@@ -280,7 +280,7 @@ void sctp_transport_route(struct sctp_transport *transport, ...@@ -280,7 +280,7 @@ void sctp_transport_route(struct sctp_transport *transport,
return; return;
} }
if (transport->dst) { if (transport->dst) {
transport->pathmtu = dst_mtu(transport->dst); transport->pathmtu = WORD_TRUNC(dst_mtu(transport->dst));
/* Initialize sk->sk_rcv_saddr, if the transport is the /* Initialize sk->sk_rcv_saddr, if the transport is the
* association's active path for getsockname(). * association's active path for getsockname().
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment