fixing a couple of bugs in sctp.c in burst transfers #2427
Conversation
In janus_sctp_pending_message_create the len attribute was not set properly. In the burst transfers, packets got stuck in pending_messages queue!. To fix this the new function janus_sctp_data_ready is called in case of SCTP_SENDER_DRY_EVENT. I moved the first loop of janus_sctp_send_data function to the new created function to push pending messages to sctp stack. At the end of function I called the janus_dtls_sctp_data_ready function.
|
Thanks for your contribution, @afshin2003! Please make sure you sign our CLA, as it's a required step before we can merge this. |
| m = g_queue_peek_head(sctp->pending_messages); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm not sure removing this is a good idea. The main reason why we tried to send all pending messages before sending the data we were just asked to send was to ensure the proper order of the messages. Since you removed this, and now pending messages are sent by the "dry" event, it means it's going to be very likely that a message sent now with janus_sctp_send_data will be sent before a pending message waiting to be sent.
There was a problem hiding this comment.
No, this can not happen.
janus_sctp_send_data will either enqueue the message (in case we are buffering in pending_message) or send it right away.
Once DRY_EVENT occurs, the pending_message will be traversed in FIFO order.
|
Thanks for the effort @afshin2003, I think this is a good enhancement over the current implementation. What you are proposing is a way to avoid message dropping (aka flow control) in case of bursts and I see how you accomplish this by starting the en-queuing in However I have an observation. |
|
It seems the messages are left in the queue in the case of |
|
@uxmaster you're right, we are just peeking the head with |
|
Apologies for the very late reply: we've been busy with the IETF meeting and I've fallen a bit behind with reviews. I think the doubts I had are cleared, after the discussion I read above, so this is good to go for me. Thanks again, @afshin2003, merging! ✌️ |
|
So sorry guys for very late response :( |
In janus_sctp_pending_message_create the len attribute was not set properly.
In the burst transfers, packets got stuck in pending_messages queue!. To fix this the new function janus_sctp_data_ready is called in case of SCTP_SENDER_DRY_EVENT. I moved the first loop of janus_sctp_send_data function to the new created function to push pending messages to sctp stack. At the end of function I called the janus_dtls_sctp_data_ready function.