netdev - RE: [PATCH net-next 2/2] sctp: add support for MSG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <063D6719AE5E284EB5DD2968C1650D6DCFE6EDE0@AcuExch.aculab.com>
Date:   Fri, 24 Feb 2017 10:14:09 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Xin Long' <lucien.xin@...il.com>
CC:     network dev <netdev@...r.kernel.org>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        "Vlad Yasevich" <vyasevich@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: RE: [PATCH net-next 2/2] sctp: add support for MSG_MORE

From: Xin Long
> Sent: 24 February 2017 06:44
...
> > IIRC sctp_packet_can_append_data() is called for the first queued
> > data chunk in order to decide whether to generate a message that
> > consists only of data chunks.
> > If it returns SCTP_XMIT_OK then a message is built collecting the
> > rest of the queued data chunks (until the window fills).
> >
> > So if I send a message with MSG_MORE set (on an idle connection)
> > SCTP_XMIT_DELAY is returned and a message isn't sent.
> >
> > I now send a second small message, this time with MSG_MORE clear.
> > The message is queued, then the code looks to see if it can send anything.
> >
> > sctp_packet_can_append_data() is called for the first queued chunk.
> > Since it has force_delay set SCTP_XMIT_DELAY is returned and no
> > message is built.
> > The second message isn't even looked at.
> You're right. I can see the problem now.
> 
> What I expected is it should work like:
> 
> 1, send 3 small chunks with MSG_MORE set, the queue is:
>   chk3 [set] -> chk2 [set] -> chk1 [set]

Strange way to write a queue! chk1 points to chk2 :-)

> 2. send 1 more chunk with MSG_MORE clear, the queue is:
>   chk4[clear] -> chk3 [clear] -> chk2 [clear] -> chk1 [clear]

I don't think processing the entire queue is a good idea.
Both from execution time and the effects on the data cache.
The SCTP code is horrid enough as it is.

> 3. then if user send more small chunks with MSG_MORE set,
> the queue is like:
>   chkB[set] -> chkA[set] -> chk4[clear] -> chk3 [clear] -> chk2 [clear] -> chk1 [clear]
> so that the new small chunks' flag will not affect the other chunks bundling.

That isn't really necessary.
The user can't expect to have absolute control over which chunks get bundled
together.
If the above chunks still aren't big enough to fill a frame the code might
as well wait for the next chunk instead of building a packet that contains
chk1 through to chkB.

Remember you'll only get a queued chunk with MSG_MORE clear if data can't be sent.
As soon as data can be sent, if the first chunk has MSG_MORE clear all of the
queued chunks will be sent.

So immediately after your (3) the application is expected to send a chunk
with MSG_MORE clear - at that point all the queued chunks can be sent in
a single packet.

So just save the last MSG_MORE on the association as I did.

	David