Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 6 Nov 2020 10:48:24 +0100
From:   Petr Malat <oss@malat.biz>
To:     Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc:     linux-sctp@vger.kernel.org, Vlad Yasevich <vyasevich@gmail.com>,
        Neil Horman <nhorman@tuxdriver.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sctp: Fix sending when PMTU is less than
 SCTP_DEFAULT_MINSEGMENT
Message-ID: <20201106094824.GA7570@bordel.klfree.net>
References: <20201105103946.18771-1-oss@malat.biz>
 <20201106084634.GA3556@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201106084634.GA3556@localhost.localdomain>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On Fri, Nov 06, 2020 at 05:46:34AM -0300, Marcelo Ricardo Leitner wrote:
> On Thu, Nov 05, 2020 at 11:39:47AM +0100, Petr Malat wrote:
> > 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):
>           psize = packet->size;
>           if (packet->transport->asoc)
>                   pmtu = packet->transport->asoc->pathmtu;
>           else
>                   pmtu = packet->transport->pathmtu;
here the returned pmtu will be 512

> 
>           /* 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

>            }
> 
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.

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.

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