From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-18 17:55:18
This patch is to add support for MSG_MORE on sctp. Patch 1/2 is an
improvement ahead of patch 2/2 to solve the close block problem
mentioned in https://patchwork.ozlabs.org/patch/372404/.
Xin Long (2):
sctp: flush out queue once assoc state falls into SHUTDOWN_PENDING
sctp: add support for MSG_MORE
include/net/sctp/structs.h | 1 +
net/sctp/output.c | 9 +++------
net/sctp/sm_sideeffect.c | 4 ++++
net/sctp/socket.c | 1 +
4 files changed, 9 insertions(+), 6 deletions(-)
--
2.1.0
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-18 17:55:18
This patch is to flush out queue when assoc state falls into
SHUTDOWN_PENDING if there are still chunks in it, so that the
data can be sent out as soon as possible before sending SHUTDOWN
chunk.
When sctp supports MSG_MORE flag in next patch, this improvement
can also solve the problem that the chunks with MSG_MORE flag
may be stuck in queue when closing an assoc.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/sm_sideeffect.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -872,6 +872,10 @@ static void sctp_cmd_new_state(sctp_cmd_seq_t *cmds,if(!sctp_style(sk,UDP))sk->sk_state_change(sk);}++if(sctp_state(asoc,SHUTDOWN_PENDING)&&+!sctp_outq_is_empty(&asoc->outqueue))+sctp_outq_uncork(&asoc->outqueue,GFP_ATOMIC);}/* Helper function to delete an association. */
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-18 17:59:19
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
Since last patch, sctp flush out queue once assoc state falls into
SHUTDOWN_PENDING, the close block problem mentioned in [1] has been
solved as well.
[1] https://patchwork.ozlabs.org/patch/372404/
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 1 +
net/sctp/output.c | 9 +++------
net/sctp/socket.c | 1 +
3 files changed, 5 insertions(+), 6 deletions(-)
@@ -497,6 +497,7 @@ struct sctp_datamsg {/* Did the messenge fail to send? */intsend_error;u8send_failed:1,+force_delay:1,can_delay;/* should this message be Nagle delayed */};
From: David Miller <davem@davemloft.net> Date: 2017-02-20 15:26:30
From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 19 Feb 2017 01:52:44 +0800
This patch is to add support for MSG_MORE on sctp. Patch 1/2 is an
improvement ahead of patch 2/2 to solve the close block problem
mentioned in https://patchwork.ozlabs.org/patch/372404/.
From: David Laight <hidden> Date: 2017-02-21 14:27:35
From: Xin Long
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
I wouldn't worry about having messages queued that have MSG_MORE clean
when the final message has it set.
While it might be 'nice' to send the data (would have to be tx credit)
waiting for the next data chunk shouldn't be a problem.
I'm not sure I even want to test the current patch!
David
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-23 03:45:57
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I wouldn't worry about having messages queued that have MSG_MORE clean
when the final message has it set.
Yeah, It's an old optimization for bundling. MSG_MORE should NOT
break that.
While it might be 'nice' to send the data (would have to be tx credit)
waiting for the next data chunk shouldn't be a problem.
sorry, you mean it shouldn't send the data if it's waiting for the
next data whenever ?
I'm not sure I even want to test the current patch!
David
From: David Laight <hidden> Date: 2017-02-23 16:04:22
From: Xin Long
Sent: 23 February 2017 03:46
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
One of us isn't understanding the other :-)
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.
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I'm not too worried about that.
Sending the first message was a cheap way to ensure something got
sent if the application lied and didn't send a subsequent message.
The change has hit Linus's tree, I'll should be able to test that
and confirm what I think is going on.
David
On Thu, Feb 23, 2017 at 04:04:10PM +0000, David Laight wrote:
From: Xin Long
quoted
Sent: 23 February 2017 03:46
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
quoted
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
One of us isn't understanding the other :-)
IIRC sctp_packet_can_append_data() is called for the first queued
data chunk in order to decide whether to generate a message that
Perhaps here lies the source of the confusion?
sctp_packet_can_append_data() is called for all queued data chunks, and
not just the first one.
sctp_outq_flush
(retransmissions here, omitted for simplicity)
/* Finally, transmit new packets. */
while ((chunk = sctp_outq_dequeue_data(q)) != NULL) {
sctp_packet_transmit_chunk
sctp_packet_append_chunk
sctp_packet_can_append_data
__sctp_packet_append_chunk
So chunks are checked one by one.
consists only of data chunks.
That's not really its purpose. It's to check if it can append a data
chunk to the packet being prepared, while respecting asoc state, cwnd,
etc.
HTH!
Marcelo
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.
quoted
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I'm not too worried about that.
Sending the first message was a cheap way to ensure something got
sent if the application lied and didn't send a subsequent message.
The change has hit Linus's tree, I'll should be able to test that
and confirm what I think is going on.
David
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-23 18:16:26
On Fri, Feb 24, 2017 at 1:40 AM, Marcelo Ricardo Leitner
[off-list ref] wrote:
On Thu, Feb 23, 2017 at 04:04:10PM +0000, David Laight wrote:
quoted
From: Xin Long
quoted
Sent: 23 February 2017 03:46
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
quoted
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
One of us isn't understanding the other :-)
IIRC sctp_packet_can_append_data() is called for the first queued
data chunk in order to decide whether to generate a message that
Perhaps here lies the source of the confusion?
sctp_packet_can_append_data() is called for all queued data chunks, and
not just the first one.
sctp_outq_flush
(retransmissions here, omitted for simplicity)
/* Finally, transmit new packets. */
while ((chunk = sctp_outq_dequeue_data(q)) != NULL) {
sctp_packet_transmit_chunk
sctp_packet_append_chunk
sctp_packet_can_append_data
__sctp_packet_append_chunk
So chunks are checked one by one.
I think I got David's point.
like, the queue is:
chunk3[null]-->chunk2 [msg_more]-->chunk1 [msg_more]
it dequeue from chunk1, once it returns SCTP_XMIT_DELAY
chunk2, chunk3 will has no chance to dequeue, as it will
goto: sctpflush_out in sctp_outq_flush(), But we are expecting
to send all chunks.
quoted
consists only of data chunks.
That's not really its purpose. It's to check if it can append a data
chunk to the packet being prepared, while respecting asoc state, cwnd,
etc.
HTH!
Marcelo
quoted
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.
quoted
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I'm not too worried about that.
Sending the first message was a cheap way to ensure something got
sent if the application lied and didn't send a subsequent message.
The change has hit Linus's tree, I'll should be able to test that
and confirm what I think is going on.
David
On Fri, Feb 24, 2017 at 02:16:02AM +0800, Xin Long wrote:
On Fri, Feb 24, 2017 at 1:40 AM, Marcelo Ricardo Leitner
[off-list ref] wrote:
quoted
On Thu, Feb 23, 2017 at 04:04:10PM +0000, David Laight wrote:
quoted
From: Xin Long
quoted
Sent: 23 February 2017 03:46
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
quoted
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
One of us isn't understanding the other :-)
IIRC sctp_packet_can_append_data() is called for the first queued
data chunk in order to decide whether to generate a message that
Perhaps here lies the source of the confusion?
sctp_packet_can_append_data() is called for all queued data chunks, and
not just the first one.
sctp_outq_flush
(retransmissions here, omitted for simplicity)
/* Finally, transmit new packets. */
while ((chunk = sctp_outq_dequeue_data(q)) != NULL) {
sctp_packet_transmit_chunk
sctp_packet_append_chunk
sctp_packet_can_append_data
__sctp_packet_append_chunk
So chunks are checked one by one.
I think I got David's point.
like, the queue is:
chunk3[null]-->chunk2 [msg_more]-->chunk1 [msg_more]
it dequeue from chunk1, once it returns SCTP_XMIT_DELAY
chunk2, chunk3 will has no chance to dequeue, as it will
goto: sctpflush_out in sctp_outq_flush(), But we are expecting
to send all chunks.
Ahh yes, exactly.
quoted
quoted
consists only of data chunks.
That's not really its purpose. It's to check if it can append a data
chunk to the packet being prepared, while respecting asoc state, cwnd,
etc.
HTH!
Marcelo
quoted
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.
quoted
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I'm not too worried about that.
Sending the first message was a cheap way to ensure something got
sent if the application lied and didn't send a subsequent message.
The change has hit Linus's tree, I'll should be able to test that
and confirm what I think is going on.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-24 07:16:01
On Fri, Feb 24, 2017 at 12:04 AM, David Laight [off-list ref] wrote:
From: Xin Long
quoted
Sent: 23 February 2017 03:46
On Tue, Feb 21, 2017 at 10:27 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 18 February 2017 17:53
This patch is to add support for MSG_MORE on sctp.
It adds force_delay in sctp_datamsg to save MSG_MORE, and sets it after
creating datamsg according to the send flag. sctp_packet_can_append_data
then uses it to decide if the chunks of this msg will be sent at once or
delay it.
Note that unlike [1], this patch saves MSG_MORE in datamsg, instead of
in assoc. As sctp enqueues the chunks first, then dequeue them one by
one. If it's saved in assoc,the current msg's send flag (MSG_MORE) may
affect other chunks' bundling.
I thought about that and decided that the MSG_MORE flag on the last data
chunk was the only one that mattered.
Indeed looking at any others is broken.
Consider what happens if you have two small chunks queued, the first
with MSG_MORE set, the second with it clear.
I think that sctp_outq_flush() will look at the first chunk and decide it
doesn't need to do anything because sctp_packet_transmit_chunk()
returns SCTP_XMIT_DELAY.
The data chunk with MSG_MORE clear won't even be looked at.
So the data will never be sent.
quoted
It's not that bad as you thought, in sctp_packet_can_append_data():
when inflight == 0 || sctp_sk(asoc->base.sk)->nodelay, the chunks
would be still sent out.
One of us isn't understanding the other :-)
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]
2. send 1 more chunk with MSG_MORE clear, the queue is:
chk4[clear] -> chk3 [clear] -> chk2 [clear] -> chk1 [clear]
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.
is it ok to you to work like that ?
if yes, I propose the fix for this issue is:
What MSG_MORE flag actually does is ignore inflight == 0 and
sctp_sk(asoc->base.sk)->nodelay to delay the chunks, but still
it has to respect the original logic (like !chunk->msg->can_delay
|| !sctp_packet_empty(packet) || ...)
To delay the chunks with MSG_MORE set even when inflight is 0
it especially important here for users.
I'm not too worried about that.
Sending the first message was a cheap way to ensure something got
sent if the application lied and didn't send a subsequent message.
The change has hit Linus's tree, I'll should be able to test that
and confirm what I think is going on.
David
From: David Laight <hidden> Date: 2017-02-24 10:14:25
From: Xin Long
Sent: 24 February 2017 06:44
...
quoted
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
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-25 08:50:19
On Fri, Feb 24, 2017 at 6:14 PM, David Laight [off-list ref] wrote:
From: Xin Long
quoted
Sent: 24 February 2017 06:44
...
quoted
quoted
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.
quoted
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.
quoted
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.
From: Xin Long <lucien.xin@gmail.com> Date: 2017-02-27 06:35:59
On Sat, Feb 25, 2017 at 4:41 PM, Xin Long [off-list ref] wrote:
On Fri, Feb 24, 2017 at 6:14 PM, David Laight [off-list ref] wrote:
quoted
From: Xin Long
quoted
Sent: 24 February 2017 06:44
...
quoted
quoted
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.
quoted
quoted
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.
quoted
quoted
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 ?
even also block the retransmit chunks.
quoted
So just save the last MSG_MORE on the association as I did.
From: David Laight <hidden> Date: 2017-02-27 10:49:52
From: Xin Long
Sent: 27 February 2017 04:49
...
quoted
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.
Or just waiting for transmit window space.
quoted
But now even if retransmit queue is null, chk1-chk4 are still blocked.
can you accept that chkA may block the old chunks ?
even also block the retransmit chunks.
I don't see a problem, the application is about to send another chunk.
It is likely that the whole lot will go in one packet.
If the connection is flow controlled off (even if due to packet loss)
the minor delay waiting for the application won't make any real difference.
Indeed reducing the number of ethernet frames may help increase
overall throughput.
Does a request for chunks to be retransmitted force a response packet be built?
If so it will pick up all the queued data chunks regardless of any saved
MSG_MORE bits.
David
Hi,
On Fri, Feb 24, 2017 at 10:14:09AM +0000, David Laight wrote:
From: Xin Long
quoted
Sent: 24 February 2017 06:44
...
quoted
quoted
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 :-)
Strictly speaking, it's actually both together, as it's a double-linked
list. :-)
quoted
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.
It won't be processing the entire queue if not needed, and it will only
process it on the last sendmsg() call. As the list is double-linked, it
can walk backwards as necessary and stop just at the right point. So
this doesn't imply on any quadratic or exponential factor here, but
linear and only if/when finishing the MSG_MORE block.
If the application is not using MSG_MORE, impact is zero.
The SCTP code is horrid enough as it is.
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
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.
Yes. Isn't that the idea?
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
Marcelo
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.
It won't be processing the entire queue if not needed, and it will only
process it on the last sendmsg() call. As the list is double-linked, it
can walk backwards as necessary and stop just at the right point. So
this doesn't imply on any quadratic or exponential factor here, but
linear and only if/when finishing the MSG_MORE block.
If the application is not using MSG_MORE, impact is zero.
quoted
The SCTP code is horrid enough as it is.
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
quoted
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
Not really.
quoted
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
You are mis-understanding what I think MSG_MORE is for.
It isn't the application saying 'don't send this packet', but rather
'there is no point sending ANY data because I've more data to send'.
There is also the inference that the application will immediately
send the next piece of data.
So it isn't a property of the queued chunk, it is an indication that
the application is going to send more data immediately.
quoted
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.
Yes. Isn't that the idea?
quoted
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
I think you are trying to provide control of chunking that is neither
necessary nor desirable and may give a false indication of what it
might be sensible for the application to have control over.
Regardless of the MSG_MORE flags associated with any specific send()
request there will always be protocol effects (like retransmissions
or flow control 'on') that will generate different 'chunking'.
David
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.
It won't be processing the entire queue if not needed, and it will only
process it on the last sendmsg() call. As the list is double-linked, it
can walk backwards as necessary and stop just at the right point. So
this doesn't imply on any quadratic or exponential factor here, but
linear and only if/when finishing the MSG_MORE block.
If the application is not using MSG_MORE, impact is zero.
quoted
The SCTP code is horrid enough as it is.
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
quoted
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
Not really.
quoted
quoted
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
You are mis-understanding what I think MSG_MORE is for.
It isn't the application saying 'don't send this packet', but rather
'there is no point sending ANY data because I've more data to send'.
I see the difference now, thanks.
There is also the inference that the application will immediately
send the next piece of data.
So it isn't a property of the queued chunk, it is an indication that
the application is going to send more data immediately.
It would basically avoid sending the chunk immediately when inflight==0
and the rest could stay as is then.
As the application is supposed to send more data immediately and clear
the flag, we can send packets as they become full, and also let normal
Nagle processing be. TCP also has a ceiling on this waiting, 200ms
according to man 7 tcp/TCP_CORK.
quoted
quoted
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.
Yes. Isn't that the idea?
quoted
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
I think you are trying to provide control of chunking that is neither
necessary nor desirable and may give a false indication of what it
might be sensible for the application to have control over.
Agreed.
Regardless of the MSG_MORE flags associated with any specific send()
request there will always be protocol effects (like retransmissions
or flow control 'on') that will generate different 'chunking'.
Yes, those are the ones that may lead to some confusion on how it
actually works, and mangling them is not really desired for the
sideeffects that it might have.
Sooner or later we could have bug reports like "hey this chunk shouldn't
have been packed with that." if we stick with the initial proposition,
while with David's view, we are only promising to not send packets with
a single chunk and as long as the application send more data fast enough.
David, are we on the same page now? ;-)
Xin, what do you think?
Marcelo
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.
It won't be processing the entire queue if not needed, and it will only
process it on the last sendmsg() call. As the list is double-linked, it
can walk backwards as necessary and stop just at the right point. So
this doesn't imply on any quadratic or exponential factor here, but
linear and only if/when finishing the MSG_MORE block.
If the application is not using MSG_MORE, impact is zero.
quoted
The SCTP code is horrid enough as it is.
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
quoted
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
Not really.
quoted
quoted
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
You are mis-understanding what I think MSG_MORE is for.
It isn't the application saying 'don't send this packet', but rather
'there is no point sending ANY data because I've more data to send'.
I see the difference now, thanks.
quoted
There is also the inference that the application will immediately
send the next piece of data.
So it isn't a property of the queued chunk, it is an indication that
the application is going to send more data immediately.
It would basically avoid sending the chunk immediately when inflight==0
and the rest could stay as is then.
As the application is supposed to send more data immediately and clear
the flag, we can send packets as they become full, and also let normal
Nagle processing be. TCP also has a ceiling on this waiting, 200ms
according to man 7 tcp/TCP_CORK.
quoted
quoted
quoted
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.
Yes. Isn't that the idea?
quoted
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
I think you are trying to provide control of chunking that is neither
necessary nor desirable and may give a false indication of what it
might be sensible for the application to have control over.
Agreed.
quoted
Regardless of the MSG_MORE flags associated with any specific send()
request there will always be protocol effects (like retransmissions
or flow control 'on') that will generate different 'chunking'.
Yes, those are the ones that may lead to some confusion on how it
actually works, and mangling them is not really desired for the
sideeffects that it might have.
Sooner or later we could have bug reports like "hey this chunk shouldn't
have been packed with that." if we stick with the initial proposition,
while with David's view, we are only promising to not send packets with
a single chunk and as long as the application send more data fast enough.
David, are we on the same page now? ;-)
Xin, what do you think?
If we insist that MSG_MORE means not to send ANY data, I compromise.
does ANY include retransmission DATA? should MSG_MORE block
retransmission ?
On Thu, Mar 23, 2017 at 12:35:46PM +0800, Xin Long wrote:
On Thu, Mar 23, 2017 at 1:33 AM, Marcelo Ricardo Leitner
[off-list ref] wrote:
quoted
On Wed, Mar 22, 2017 at 02:07:37PM +0000, David Laight wrote:
quoted
Regardless of the MSG_MORE flags associated with any specific send()
request there will always be protocol effects (like retransmissions
or flow control 'on') that will generate different 'chunking'.
Yes, those are the ones that may lead to some confusion on how it
actually works, and mangling them is not really desired for the
sideeffects that it might have.
Sooner or later we could have bug reports like "hey this chunk shouldn't
have been packed with that." if we stick with the initial proposition,
while with David's view, we are only promising to not send packets with
a single chunk and as long as the application send more data fast enough.
David, are we on the same page now? ;-)
Xin, what do you think?
If we insist that MSG_MORE means not to send ANY data, I compromise.
does ANY include retransmission DATA? should MSG_MORE block
retransmission ?
That's not really what he meant by that, I think. That "ANY" in there is
a way to refer to the entire buf and not that msg sendmsg is sending.
Later I explained what I got from his explanation, which should be more
like:
"If MSG_MORE was used, and there are no packets in flight, do not send a
packet right away because the application is going to send more data."
Would have to handle the (Not-)Nagle situation too:
"If not using Nagle and using MSG_MORE, try to not generate a packet
right away." (because this may send packets with a single chunk even if
in_flight != 0)
In both cases, if the flush is generated by other triggers, it's okay.
Because if there are chunks already queued, they will be sent as soon as
in_flight reaches 0 or some other break is lifted (flow control).
Holding the chunk that was queued with MSG_MORE and sending a partial
packet in this case because of MSG_MORE is not good, it's possibly not
saving anything.
Marcelo
From: Xin Long <lucien.xin@gmail.com> Date: 2017-03-24 16:09:11
On Fri, Mar 24, 2017 at 12:42 AM, Marcelo Ricardo Leitner
[off-list ref] wrote:
On Thu, Mar 23, 2017 at 12:35:46PM +0800, Xin Long wrote:
quoted
On Thu, Mar 23, 2017 at 1:33 AM, Marcelo Ricardo Leitner
[off-list ref] wrote:
quoted
On Wed, Mar 22, 2017 at 02:07:37PM +0000, David Laight wrote:
quoted
Regardless of the MSG_MORE flags associated with any specific send()
request there will always be protocol effects (like retransmissions
or flow control 'on') that will generate different 'chunking'.
Yes, those are the ones that may lead to some confusion on how it
actually works, and mangling them is not really desired for the
sideeffects that it might have.
Sooner or later we could have bug reports like "hey this chunk shouldn't
have been packed with that." if we stick with the initial proposition,
while with David's view, we are only promising to not send packets with
a single chunk and as long as the application send more data fast enough.
David, are we on the same page now? ;-)
Xin, what do you think?
If we insist that MSG_MORE means not to send ANY data, I compromise.
does ANY include retransmission DATA? should MSG_MORE block
retransmission ?
That's not really what he meant by that, I think. That "ANY" in there is
a way to refer to the entire buf and not that msg sendmsg is sending.
Later I explained what I got from his explanation, which should be more
like:
"If MSG_MORE was used, and there are no packets in flight, do not send a
packet right away because the application is going to send more data."
Would have to handle the (Not-)Nagle situation too:
"If not using Nagle and using MSG_MORE, try to not generate a packet
right away." (because this may send packets with a single chunk even if
in_flight != 0)
In both cases, if the flush is generated by other triggers, it's okay.
Because if there are chunks already queued, they will be sent as soon as
in_flight reaches 0 or some other break is lifted (flow control).
Holding the chunk that was queued with MSG_MORE and sending a partial
packet in this case because of MSG_MORE is not good, it's possibly not
saving anything.
Makes sence, thanks for making this clear, will post a new fix.
From: David Laight <hidden> Date: 2017-03-24 17:38:16
From: Marcelo Ricardo Leitner
Sent: 23 March 2017 16:42
...
quoted
quoted
David, are we on the same page now? ;-)
Probably...
quoted
quoted
Xin, what do you think?
If we insist that MSG_MORE means not to send ANY data, I compromise.
does ANY include retransmission DATA? should MSG_MORE block
retransmission ?
That's not really what he meant by that, I think. That "ANY" in there is
a way to refer to the entire buf and not that msg sendmsg is sending.
Later I explained what I got from his explanation, which should be more
like:
"If MSG_MORE was used, and there are no packets in flight, do not send a
packet right away because the application is going to send more data."
Would have to handle the (Not-)Nagle situation too:
"If not using Nagle and using MSG_MORE, try to not generate a packet
right away." (because this may send packets with a single chunk even if
in_flight != 0)
In both cases, if the flush is generated by other triggers, it's okay.
Because if there are chunks already queued, they will be sent as soon as
in_flight reaches 0 or some other break is lifted (flow control).
Holding the chunk that was queued with MSG_MORE and sending a partial
packet in this case because of MSG_MORE is not good, it's possibly not
saving anything.
It is also worth remembering that this is all about whether the first
chunk in an ethernet frame is a data chunk.
If a frame is being sent for some other reason (eg ack or heartbeat)
then it will collect queued data chunks.
I've seen hearbeats collect data chunks, I've not checked that this
doesn't happen for heartbeats that are probing IP addresses.
David
From: David Laight <hidden> Date: 2017-03-28 10:30:23
From: Marcelo Ricardo Leitner
Sent: 21 March 2017 22:04
...
quoted
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
quoted
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
quoted
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
quoted
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.
Yes. Isn't that the idea?
quoted
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
I think I've realised why we were disagreeing....
For TCP MSG_MORE was implemented as an alternative to 'corking' for
allocations that used multiple send() calls to send a single application
level message (eg those that send a length and data separately).
The real solution is to use sendv() (or writev() if sendv() doesn't exist).
SCTP is different, every send() generates a separate data chunk.
The problem in SCTP is that if an application has more than one data
chunk to send it needs to stop the kernel sending anything until
it has sent all the data chunks - otherwise they all go out in
separate ethernet packets.
A TCP application can avoid this by doing a single send() for all the
data. This doesn't work for SCTP because it would generate a single
data chunk.
David
On Tue, Mar 28, 2017 at 10:29:27AM +0000, David Laight wrote:
From: Marcelo Ricardo Leitner
quoted
Sent: 21 March 2017 22:04
...
quoted
quoted
quoted
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.
So...?
I mean, I'm okay with that but that doesn't explain why we can't do as
Xin proposed on previous email here.
quoted
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.
Our expectations are the same and that's what the proposed solution also
achieves, no?
quoted
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.
With the fix proposed by Xin, this would be more like: ... all of the
_non-held_ chunks will be sent.
After all, application asked to hold them, for whatever reason it had.
quoted
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.
Yes. Isn't that the idea?
quoted
So just save the last MSG_MORE on the association as I did.
I don't see the reason to change that. Your reply seem to reject the
idea but I cannot get the reason why. The solution proposed is more
complex, yes, allows more control, yes, but those aren't real issues
here.
I think I've realised why we were disagreeing....
For TCP MSG_MORE was implemented as an alternative to 'corking' for
allocations that used multiple send() calls to send a single application
level message (eg those that send a length and data separately).
The real solution is to use sendv() (or writev() if sendv() doesn't exist).
SCTP is different, every send() generates a separate data chunk.
The problem in SCTP is that if an application has more than one data
chunk to send it needs to stop the kernel sending anything until
it has sent all the data chunks - otherwise they all go out in
separate ethernet packets.
A TCP application can avoid this by doing a single send() for all the
data. This doesn't work for SCTP because it would generate a single
data chunk.
Yep, it was based on TCP one since day 0. There is also the way MSG_MORE
works for UDP, in which it appends the subsequent write to the same
datagram. But then, for SCTP, we should use EOR flag, as Michael Tuexen
had indicated here on the list a few months ago:
"""
What about adding support for the explicit EOR mode as specified in
https://tools.ietf.org/html/rfc6458#section-8.1.26
"""
Thus why it was very based on TCP implementation, so maybe in the future
we could have both options available for SCTP.
Marcelo