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]
Date:   Fri, 3 Mar 2017 12:49:48 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Xin Long' <lucien.xin@...il.com>,
        network dev <netdev@...r.kernel.org>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        "Vlad Yasevich" <vyasevich@...il.com>
Subject: RE: [PATCH net] sctp: change to save MSG_MORE flag into assoc

From: Xin Long
> Sent: 03 March 2017 06:24
> David Laight noticed the support for MSG_MORE with datamsg->force_day
> didn't really work as we expected, as the first msg with MSG_MORE set
> would always block the following chunks' dequeuing.
> 
> This Patch is to rewrite it by saving the MSG_MORE flag into assoc as
> Divid Laight suggested.
   ^ typo

> asoc->force_delay is used to save MSG_MORE flag before a msg is sent.
> Once this msg is queued, asoc->force_delay is set back to 0, so that
> it will not affect other places flushing out queue.

That doesn't seem right nor make sense.

> asoc->force_delay works as a 'local param' here as the msg sending is
> under protection of sock lock.  It would make sctp's MSG_MORE work as
> tcp's.

It is much more important to get MSG_MORE working 'properly' for SCTP
than for TCP. For TCP an application can always use a long send.

...
> @@ -1982,6 +1982,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
>  	 * breaks.
>  	 */
>  	err = sctp_primitive_SEND(net, asoc, datamsg);
> +	asoc->force_delay = 0;
>  	/* Did the lower layer accept the chunk? */
>  	if (err) {
>  		sctp_datamsg_free(datamsg);

I don't think this is right - or needed.
You only get to the above if some test has decided to send data chunks.
So it just means that the NEXT time someone tries to send data all the
queued data gets sent.
I'm guessing that the whole thing gets called in a loop (definitely needed
for very long data chunks, or after the window is opened).
Now if an application sends a lot of (say) 100 byte chunks with MSG_MORE
set it would expect to see a lot of full ethernet frames be sent.
With the above a frame will be sent (containing all but 1 chunk) when the
amount of queued data becomes too large for an ethernet frame, and immediately
followed by a second ethernet frame with 1 chunk in it.

Now it might be that the flag needs clearing when retransmissions are queued.
OTOH they might get sent for other reasons.

	David


Powered by blists - more mailing lists