On Fri, 6 Nov 2020 07:21:06 -0300 Marcelo Ricardo Leitner wrote:

On Fri, Nov 06, 2020 at 10:48:24AM +0100, Petr Malat wrote:

quoted

On Fri, Nov 06, 2020 at 05:46:34AM -0300, Marcelo Ricardo Leitner wrote:  

quoted

On Thu, Nov 05, 2020 at 11:39:47AM +0100, Petr Malat wrote:  

quoted

Function sctp_dst_mtu() never returns lower MTU than
SCTP_TRUNC4(SCTP_DEFAULT_MINSEGMENT) even when the actual MTU is less,
in which case we rely on the IP fragmentation and must enable it.  

This should be being handled at sctp_packet_will_fit():  

sctp_packet_will_fit() does something a little bit different, it
allows fragmentation, if the packet must be longer than the pathmtu
set in SCTP structures, which is never less than 512 (see
sctp_dst_mtu()) even when the actual mtu is less than 512.

One can test it by setting mtu of an interface to e.g. 300,
and sending a longer packet (e.g. 400B):  

quoted

          psize = packet->size;
          if (packet->transport->asoc)
                  pmtu = packet->transport->asoc->pathmtu;
          else
                  pmtu = packet->transport->pathmtu;  

here the returned pmtu will be 512  

Thing is, your patch is using the same vars to check for it:
+       pmtu = tp->asoc ? tp->asoc->pathmtu : tp->pathmtu;

quoted

quoted

          /* Decide if we need to fragment or resubmit later. */
          if (psize + chunk_len > pmtu) {  

This branch will not be taken as the packet length is less then 512  

Right, ok. While then your patch will catch it because pmtu will be
SCTP_DEFAULT_MINSEGMENT, as it is checking with '<='.

quoted

quoted

           }
  

And the whole function will return SCTP_XMIT_OK without setting
ipfragok.

I think the idea of never going bellow 512 in sctp_dst_mtu() is to
reduce overhead of SCTP headers, which is fine, but when we do that,
we must be sure to allow the IP fragmentation, which is currently
missing.  

Hmm. ip frag is probably just worse than higher header/payload
overhead.

quoted

The other option would be to keep track of the real MTU in pathmtu
and perform max(512, pathmtu) in sctp_packet_will_fit() function.  

I need to check where this 512 came from. I don't recall it from top
of my head and it's from before git history. Maybe we should just drop
this limit, if it's artificial. IPV4_MIN_MTU is 68.

quoted

Not sure when exactly this got broken, but using MTU less than 512
used to work in 4.9.  

Uhh, that's a bit old already. If you could narrow it down, that would
be nice.

I'm dropping this from patchwork, if you conclude that the patch is
good as is please repost, thanks!