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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 25 Feb 2017 16:41:34 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     David Laight <David.Laight@...lab.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

On Fri, Feb 24, 2017 at 6:14 PM, David Laight <David.Laight@...lab.com> wrote:
>
> 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 :-)
haha, just  a model.

>
> > 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.
you check the codes in last email, it's not processing the entire queue.

1). only when queue has delay chunk inside by checking queue->has_delay
    and current chunk has msg_more flag.

2). will break on the first chunk with clear in the queue.

but yes, in 2), extra work has to be done than before, but not much.

>
> > 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.
understand this.

what I'm worried about is if the msg_more is saved in assoc:
     chk4[clear] -> chk3 [clear] -> chk2 [clear] -> chk1 [clear]
then when you send a small chkA with MSG_MORE,
the queue will be like:
     chkA [set] -> chk4[set] -> chk3 [set] -> chk2 [set] -> chk1 [set]
because msg_more is saved in assoc, every chunk can look at it.
chk1 - chk4 are big enough to be packed into a packet, they were
not sent last time because a lot of chunks are in the retransmit
queue.

But now even if retransmit queue is null, chk1-chk4 are still blocked.

can you accept that chkA may block the old chunks ?

>
> So just save the last MSG_MORE on the association as I did.
I will think about it, thanks.

>
>         David