libosmo-abis[master]: Remove use of private oRTP function

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Holger Freyther gerrit-no-reply at lists.osmocom.org
Wed Oct 12 19:10:14 UTC 2016


Patch Set 1: Code-Review-2

> There's nothing to be raised with upstream, I think.  You want to
 > remove access to a private API, and I agree we should try that. 
 > However, then remove the select() based RTCP code completely and
 > rely on polling, and verify that it all still works.  If it does,
 > we can marge such a patch [but not the current one].  Only if it
 > doensn't we indeed should talk to upstream.

I was about to write the same. Max, if you want to move things forward do the full work and not just half of it. A patch like this with all good intentions is making it a lot worse than moving things forward.

So what you changed is:

* select the RTCP socket
* If it is readable ignore it
* Hope/Assume that reading from the RTP socket will try to read RTCP too (yes, it seems to do that even in old oRTP versions).

BUT:

Let's assume this case:

* oRTP is send/recv
* RTP is blocked by a firewall/bad masquerading
* RTCP arrives..

Now RTCP will never be read and the select will always trigger.. making any user of this code spend 99% CPU time.. and now you, Harald and me have spent time with it.

Try to understand how the code around is using this method, it is static which makes it a lot easier.

-- 
To view, visit https://gerrit.osmocom.org/908
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff1b15c68efca3e02267e0308142c6a7a0c2a974
Gerrit-PatchSet: 1
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-HasComments: No



More information about the gerrit-log mailing list