Drops packets, after re-invite sdp port changed, only when using "asymmetric+strict-source" flags together #1679
Comments
|
Full rtpengine logs for the issue: |
|
Seems to relate to this discussion with @emvondo on google groups. As @smititelu noted we found a possible fix for this by removing the asymmetric flag, but we wanted to open this just to make sure the code works like intended and does not break anything by coincidence. |
yes i wrote an update for rtperngine here for asymmetric flag with strict-source but endpoint learning does not apply for asymmetric currently. so probably need to add a case to handle it. |
|
Are you saying that the port change from the SDP is not being honoured for source checking purposes? |
|
If this question is addressed to us, I'd say yes, this is what we observe and this is how we interpret the call flow and the logs. |
|
Ok, I can see how 900d2be would break that. It changes the source check to check against the learned endpoint address instead of the destination endpoint address when the stream is asymmetric (why? @emvondo ) and then sets the learned endpoint address to the source address of a received packet when the stream is asymmetric, but only once (if learned address is empty) (why? @emvondo ) |
in asymmetric mode destination and source address are not the same. so its the source address of the packet that we check for the strict-source flag. but because by default endpoint (SDP endpoint) is checked for asymmetric too. that check will always fail and no packets will ever be relayed if source packet ip is different from the one in SDP. Thats why we need to save the first ip of received packet for asymmetric mode in the learned_endpoint to make the check further. Now the point is for asymmetric mode there is no way currently to ask to reset or re-learn the ip saved in learned_endpoint as it is for symmetric mode. learned_endpoint is never set for asymmetric mode so i used it to store the ip of first packet in asymmetric mode and then make the comparison. now thats true may be the comparison done once is not the ideal case. thats also the point i raised in the mailing list to have a way to reset it like with symmetric mode. |
|
If you want to use asymmetric and source checking and endpoint learning all together, then I suppose it would be best to retain the normal endpoint learning mechanism even with asymmetric set (i.e. not immediately set the "confirmed" flag, as it now happens in https://github.com/sipwise/rtpengine/blob/master/daemon/media_socket.c#L2355), and then just not use the learned address as destination address. |
|
I kind of understand @emvondo thoughts and arguments and I don't know how a correct implementation for this should look like. The SDP just carries the address where RTP is expected to be received, not where it will come from. So symmetric just seems to be a subset or special case of asymmetric, where RTP simply does come from the same address it is expected to be received. |
hum OK i see. it could be a start of solution because right now i dont see the right way to implement it because in our call flows we have asymmetric RTP and in the same time we also want in asymmetric mode to reject incoming streams that dont match the first source ip catched. But its a dilemma because even with this asymmetric mode you dont know wether the first packet you received comes from authorized source or not so. i dont think there is an ultime solution for that. when there is no forking its ok. but with forked calls that issue appears. so ->el_flags is reset everytime you call rtpengien_offer') or rtpengine_answer() ? |
|
Just want to ask @rfuchs there is no flag to reset asymmetric and strict source flags through rtpengine_offer() and rtpengine_answer() ? or may be its the same answer you provided me on the mailing list |
Honestly I'm not sure what the expected behaviour ought to be because this combination (asymmetric + strict-source) is a very odd use case.
That's why I think that perhaps retaining the usual endpoint learning mechanism even with asymmetric streams could be the way forward. This mechanism uses a delay of ~2 seconds after a signalling event and then locks in to a source address only after that delay. But of course that would negate @smititelu's use case of wanting to achieve an immediate push of the stream to the kernel module, so that should be done only when source checking is also wanted (or perhaps using an extra flag). (There is possibility to improve the usual endpoint learning mechanism for all regular cases here. The stream could be pushed to the kernel immediately and still use a delay of a few seconds to perform endpoint learning at the same time. But this needs to be implemented and can be tricky to get right.)
Yes it is the same answer. There is no such flag. Endpoint learning is normally always re-enabled after a signalling event (that's what the |
exactly . its rather a question of setting at this point. if you know you need it you activate it knowing rtp source ip will never change afterward
yes im also aligned with it. do you see a benefit or penalty to make that delay time dynamic (with a flag passed through offer & answer commands) instead of having it statically configured like today ?
ok thanks did not know that. i will make somes changes on my local copy to test and push a merge request concerning delay for asymmetric RTP. |
@rfuchs it seems not cleared for ASYMMERIC currently isnt it ? https://github.com/sipwise/rtpengine/blob/master/daemon/media_socket.c#L1883 |
|
@emvondo and @rfuchs , regarding the time delay and the kernelization: In my opinion, with the flags "trust-address" and "symmetric" (which seems to be default so you don't need to explicitly pass them) rtpengine should kernelize the stream immediately as it already has all information required to kernelize the stream. No endpoint learning is required. |
|
Also we experience segmentation faults and core dumps in a line touched by this commit: rtpengine logs:
Back trace:
We do not support ICE in our platform and we don't know why the client is trying ICE as we use ICE=remove in all offer/answer requests to rtpengine. Nonetheless it should not crash. Currently it kind of crashes consistently when that client calls, always (4x since yesterday) with the same back trace. Also I am not sure if the crash is caused by this commit or it would also crash without this commit. Just added it here cause it at least is related to that commit. We are running a release based on commit 177b411. |
You can use
Right, because the combination of asymmetric + endpoint learning was never considered. |
|
Related to the flags and the Kamailio rtpengine module documentation: What do you think about removing the possible flags from that module documentation and instead give a link to corresponding part of rtpengine project documentation? I think this is a good idea as basically rtpengine defines what flags it supports and it is simpler to maintain one documentation. |
Yeah can probably do that. The rtpengine docs are already linked AFAIK, but can probably remove the description of most of the flags and just link to the docs. Some basic usage examples should remain though. (The flags handling in the module need a serious overhaul anyway, but that's a topic for another day) |
Hi, we noticed an issue with RTPEngine dropping RTP packets after re-invite SDP port changed, only when using "asymmetric+strict-source" flags together. Traced back issue starts happening after this commit: 900d2be.
RTPEngine logs:
... and packets come in on the new port but rtpengine do not forward them.
A fix for this issue would be either:
I raised this issue, maybe that new commit has some bugs? Please have a look. Please close this if there is no bug with that commit, and any of the above fixes should be applied.
Thank you,
Stefan
The text was updated successfully, but these errors were encountered: