Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why usrsctp_sendv sometimes returns EWOULDBLOCK after connection has established? #142

Closed
starwarfan opened this issue Apr 13, 2017 · 30 comments

Comments

@starwarfan
Copy link

The non_blocking mode is set to true. The usrsctp_sendv sometimes returns EWOULDBLOCK after the SCTP_COMM_UP event. And this keep appearing during the video data transimission. Though the data transmission is not stop due to this error, I want to know what causes an EWOULDBLOCK on sending. Does this mean the program sends the data too fast?

@tuexen
Copy link
Member

tuexen commented Apr 13, 2017

usrsctp_sendv() returns EWOULDBLOCK if you are trying to send more data than fits in the socket send buffer. So this is an indication that you are sending too fast. You can increase the send buffer size by using the usrsctp_setsockopt with the name SO_SNDBUF.

@starwarfan
Copy link
Author

I enlarge the SO_SNDBUF once an EWOULDBLOCK received (up to 2^28). But the EWOULDBLOCK keeps showing at the almost same frequency (serveral minutes) as before. So maybe it's not due to my send buffer size. Though I do not care about a little data loss, but the error is annoying and the TCP connection is OK.

@lrs0304
Copy link

lrs0304 commented Oct 10, 2017

Subscribe

@lgrahl
Copy link
Contributor

lgrahl commented Oct 10, 2017

In non-blocking mode, you will always have to handle EWOULDBLOCK. Just pause sending for a while. It's not something that's usrsctp-specific but rather all traditional socket APIs work that way in non-blocking mode. However, usrsctp-neat will provide you with write events, so you know when sending is possible. So, maybe you want to switch to that fork (which will eventually be merged back into this repo).
Edit: There's also a send_cb parameter in usrsctp_socket that will be called when the buffer contains less data than a specific threshold. When that function is being called, you can send pending messages.

If you don't want to handle EWOULDBLOCK, then there's no proper way around blocking mode.

@taylor-b
Copy link
Contributor

We've been using "SCTP_SENDER_DRY_EVENT" instead of send_cb. Will this make a significant difference?

@tuexen
Copy link
Member

tuexen commented Oct 30, 2017

The send_cb is fired if there is there is some room in the send buffer (controlled by the threshold provided). If you use the SCTP_SENDER_DRY_EVENT you wait until the send buffer is empty. This gives only limited throughput...

@taylor-b
Copy link
Contributor

taylor-b commented Oct 30, 2017

So it's possible in cases with, say, lots of packet loss, that relying on SCTP_SENDER_DRY_EVENT could harm our throughput immensely? Wow, I can't believe we've been doing it wrong all this time. It's a wonder why the thousands of WebRTC data channel users didn't notice (until now).

@tuexen
Copy link
Member

tuexen commented Oct 30, 2017

In case of packetloss, you wait until everything has been retransmitted successfully, before the SCTP_SENDER_DRY_EVENT fires. Only after that you would start transmitting new data. So, yes, it can hurt...

@lgrahl
Copy link
Contributor

lgrahl commented Oct 30, 2017

@tuexen: Wouldn't it also hurt in case of no packet loss? When a reliable transmission occurs, the data needs to be acknowledged before the send buffer can be considered completely empty, correct? So, throughput is decreased because the implementation waits for an ack. Then, subsequent data is copied into the buffer and only then sending can start again.

@lgrahl
Copy link
Contributor

lgrahl commented Oct 30, 2017

@taylor-b I'm assuming you're talking about the Chrome codebase for WebRTC data channels? Unless something has changed in the code, SCTP_SENDER_DRY_EVENT is the SCTP notification you use for raising the RTCDataChannel.onbufferedamountlow event, right? At least I couldn't find the other end of the signal for SignalReadyToSend and apparently there's no buffer in between JS-land and usrsctp that needs to be flushed, so this is what I assumed.

Again, unless something has changed, the onbufferedamountlow in Chrome needs to be addressed anyway as it currently ignores bufferedAmountLowTreshold. Firefox keeps track of the buffered amount somewhat incorrect because it only looks at its own buffers and not the ones of usrsctp as well. But this is a workaround because there's no way to retrieve the current buffer fill status of a single stream in usrsctp... unless this has changed, @tuexen?

@taylor-b
Copy link
Contributor

@lgrahl Right... So it alternates between completely filling up the buffer and completely draining it. Agree it seems like a problem.

@shacharz
Copy link

@lgrahl I think additionally to what you stated, by only listening to SCTP_SENDER_DRY_EVENT each time the channel will fill the buffer and hit the EWOULDBLOCK it'll be forced down to slow start, and cwng will go down to maximum burst which is like 3-4 packets IIRC - hurting throughput tremendously.

@tuexen
Copy link
Member

tuexen commented Feb 4, 2020

I think the discussion has finished. If not, re-open.

@ibc
Copy link

ibc commented Jul 9, 2020

Too late but what is the default sending buffer size value?

@taylor-b
Copy link
Contributor

taylor-b commented Jul 9, 2020

256K.

@ibc
Copy link

ibc commented Jul 9, 2020

Thanks

@tuexen
Copy link
Member

tuexen commented Jul 9, 2020

Too late but what is the default sending buffer size value?

262144 Bytes. Can be changed like in https://github.com/sctplab/usrsctp/blob/master/programs/ekr_loop.c#L494.

@jmillan
Copy link
Contributor

jmillan commented Jul 30, 2020

Let me ask this question here, since I think it follows the thread.

Does the sb_threshold argument in usrsctp_socket serve so send_cb is fired whenever the send buffer has sb_threshold bytes (or more) free?

If so, this can be used as the bufferedAmountLow W3C event, with the difference that send_cb would be called when the send buffer free memory is greater than sb_threshold rather than the queued data being less than a threshold given by bufferedAmountLowThreshold.

Is that correct?

@taylor-b
Copy link
Contributor

Does the sb_threshold argument in usrsctp_socket serve so send_cb is fired whenever the send buffer has sb_threshold bytes (or more) free?

Yes. And send_cb will eventually trigger bufferedAmountLow, but bufferedAmount doesn't actually represent the full amount of data buffered. At least in Chrome, we don't keep track of the amount of data usrsctp has buffered itself; bufferedAmount is the amount of data we have buffered at the application level, after usrsctp has already started returning EWOULDBLOCK.

However, send_cb is what will trigger Chrome to continue sending these messages that have been buffered, which will eventually result in bufferedAmountLow when its buffer has been drained enough.

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

It makes sense that, in Chrome, an extra buffer layer is used when there is no enough space in usrsctp.

Good to see that send_cb can be fired within a defined buffer threshold to achieve this behaviour.

Thanks

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

And send_cb will eventually trigger bufferedAmountLow

Since you say that it will eventually do it.., Is there any estimated time for this implementation?

On the other hand, I see that send_cb is not fired with a ulp_info void *, as opposed to recv_cb. Which makes it impossible to associate a callback with the corresponding user level data.

I see that adding such argument to the callback would be feasible by just taking the same argument value for both callbacks from usrsctp_socket(). As it's already done for recv_cb.

@tuexen and company, I believe that adding the ulp_info argument to the send_cb would be certainly easy (I may be wrong). Does it make sense such an addition prior to waiting for the bufferedAmountLow implementation, which would on the other side also require this change?

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

BTW: I could do the PR for adding the ulp_info argument to send_cb.

@tuexen
Copy link
Member

tuexen commented Jul 31, 2020

The only problem I have with the change is that it breaks backwards compatibility.... But maybe it is worth breaking it.

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

The only problem I have with the change is that it breaks backwards compatibility.... But maybe it is worth breaking it.

Any suggestion on how to overcome such change in the code? Is there any similar update that is not backwards compatible so I can have a look?

Like defining one or another signature depending on the version, etc.

@tuexen
Copy link
Member

tuexen commented Jul 31, 2020

@taylor-b Didn't you also suggest the adding of the ulp_info to the send_cb?

The good point is that compilers will detect the change...

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

The good point is that compilers will detect the change...

That for sure :-)

@jmillan
Copy link
Contributor

jmillan commented Jul 31, 2020

PR: #511

@taylor-b
Copy link
Contributor

Since you say that it will eventually do it.., Is there any estimated time for this implementation?

It's already implemented; what I meant by "eventually" is that it may take additional time for the WebRTC-level buffer to be drained. Imagine that bufferedAmountLowThreshold is set to 1KB and 200KB is buffered. WebRTC might attempt to send additional messages, only to run into EWOULDBLOCK again before bufferedAmount reaches 1KB, causing us to wait for send_cb again, etc.

On the other hand, I see that send_cb is not fired with a ulp_info void *, as opposed to recv_cb. Which makes it impossible to associate a callback with the corresponding user level data.

It does provide the socket*, so the socket address can be used to associate it with user level data. But I wouldn't be opposed to adding ulp_info, to avoid using the socket address in this way. It would also need to be added to conn_output though.

@jmillan
Copy link
Contributor

jmillan commented Aug 1, 2020

It's already implemented; what I meant by "eventually" is that it may take additional time for the WebRTC-level buffer to be drained. Imagine that bufferedAmountLowThreshold is set to 1KB and 200KB is buffered. WebRTC might attempt to send additional messages, only to run into EWOULDBLOCK again before bufferedAmount reaches 1KB, causing us to wait for send_cb again, etc.

Oh, I misunderstood you, thanks for the clarification.

@tuexen
Copy link
Member

tuexen commented Aug 1, 2020

It does provide the socket*, so the socket address can be used to associate it with user level data. But I wouldn't be opposed to adding ulp_info, to avoid using the socket address in this way. It would also need to be added to conn_output though.

conn_output is a lower layer output, so it doesn't make sense to provide an ulp_info there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants