netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sctp: align MTU to a word
@ 2016-03-18 21:39 Marcelo Ricardo Leitner
  2016-03-18 22:09 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-03-18 21:39 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, Vlad Yasevich, linux-sctp

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.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/associola.c | 2 +-
 net/sctp/input.c     | 2 +-
 net/sctp/transport.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index a19b3e60770382507050a56a172ffc5adb2f497a..f89862194093f160c2801c22050690789d956b44 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1406,7 +1406,7 @@ void sctp_assoc_sync_pmtu(struct sock *sk, struct sctp_association *asoc)
 	list_for_each_entry(t, &asoc->peer.transport_addr_list,
 				transports) {
 		if (t->pmtu_pending && t->dst) {
-			sctp_transport_update_pmtu(sk, t, dst_mtu(t->dst));
+			sctp_transport_update_pmtu(sk, t, dst_mtu(t->dst) & ~3);
 			t->pmtu_pending = 0;
 		}
 		if (!pmtu || (t->pathmtu < pmtu))
diff --git a/net/sctp/input.c b/net/sctp/input.c
index db76f1ab4ac2cb16b4568f0d9fbe4e3b90deaa1c..fda32738986971648ff73b05318b8a306d496446 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -606,7 +606,7 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
 
 		/* PMTU discovery (RFC1191) */
 		if (ICMP_FRAG_NEEDED == code) {
-			sctp_icmp_frag_needed(sk, asoc, transport, info);
+			sctp_icmp_frag_needed(sk, asoc, transport, info & ~3);
 			goto out_unlock;
 		} else {
 			if (ICMP_PROT_UNREACH == code) {
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index d517153891a6efca6242ce1a4b3d86afb9026542..22219e8e6e510469d9d5868245bd7e2a9e2faf78 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -226,7 +226,7 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
 	}
 
 	if (transport->dst) {
-		transport->pathmtu = dst_mtu(transport->dst);
+		transport->pathmtu = dst_mtu(transport->dst) & ~3;
 	} else
 		transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
 }
@@ -280,7 +280,7 @@ void sctp_transport_route(struct sctp_transport *transport,
 		return;
 	}
 	if (transport->dst) {
-		transport->pathmtu = dst_mtu(transport->dst);
+		transport->pathmtu = dst_mtu(transport->dst) & ~3;
 
 		/* Initialize sk->sk_rcv_saddr, if the transport is the
 		 * association's active path for getsockname().
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] sctp: align MTU to a word
  2016-03-18 21:39 [PATCH] sctp: align MTU to a word Marcelo Ricardo Leitner
@ 2016-03-18 22:09 ` Eric Dumazet
  2016-03-18 22:33   ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2016-03-18 22:09 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev, Neil Horman, Vlad Yasevich, linux-sctp

On Fri, 2016-03-18 at 18:39 -0300, Marcelo Ricardo Leitner wrote:

> -		transport->pathmtu = dst_mtu(transport->dst);
> +		transport->pathmtu = dst_mtu(transport->dst) & ~3;


Well, surely a helper doing this would be better than spreading & ~3 all
over the places ;)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] sctp: align MTU to a word
  2016-03-18 22:09 ` Eric Dumazet
@ 2016-03-18 22:33   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 3+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-03-18 22:33 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Neil Horman, Vlad Yasevich, linux-sctp

On Fri, Mar 18, 2016 at 03:09:34PM -0700, Eric Dumazet wrote:
> On Fri, 2016-03-18 at 18:39 -0300, Marcelo Ricardo Leitner wrote:
> 
> > -		transport->pathmtu = dst_mtu(transport->dst);
> > +		transport->pathmtu = dst_mtu(transport->dst) & ~3;
> 
> 
> Well, surely a helper doing this would be better than spreading & ~3 all
> over the places ;)

Heh.. agreed
for this and the other patch too, will post a v2
Thanks

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-03-18 22:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-18 21:39 [PATCH] sctp: align MTU to a word Marcelo Ricardo Leitner
2016-03-18 22:09 ` Eric Dumazet
2016-03-18 22:33   ` Marcelo Ricardo Leitner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).