Bug 87344

Summary:	Handle EWOULDBLOCK in pseudo-TCP code
Product:	nice	Reporter:	Philip Withnall <bugzilla>
Component:	General	Assignee:	Olivier Crête <olivier.crete>
Status:	RESOLVED FIXED	QA Contact:
Severity:	normal
Priority:	medium
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments (updated)

Description Philip Withnall 2014-12-15 23:45:00 UTC

Created attachment 110884 [details]
agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments

As reported on the mailing list[1], the pseudo-TCP code currently aborts the connection if it receives EWOULDBLOCK when transmitting. This is not great.

Patch attached to fix this. The patch is also not great, but definitely better than letting the issue persist.

Also available in git form[2].

Thanks to Radosław Kołodziejczyk for identifying and debugging this.

[1]: http://lists.freedesktop.org/archives/nice/2014-November/001016.html
[2]: http://cgit.collabora.com/git/user/pwith/libnice.git/log/?h=pseudotcp-ewouldblock

Comment 1 Philip Withnall 2014-12-16 00:02:03 UTC

Discussed this with Olivier on IRC, and we decided that a better approach to handling EWOULDBLOCK here would be to drop the packet, and return WR_SUCCESS anyway. The pseudo-TCP code will then recover (eventually) just as if the packet were dropped on the network.

This has the advantage of implicitly throttling the sender, which is what you want when receiving EWOULDBLOCK, since it’s the sender which is overloaded at that point. My patch had the opposite effect: increasing the sender’s CPU load exactly when they can’t handle it.

I’ll produce an updated patch tomorrow.

Comment 2 Philip Withnall 2014-12-17 08:32:31 UTC

Created attachment 110939 [details] [review]
agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments (updated)

Updated patch. git branch also updated.

Comment 3 Olivier Crête 2014-12-20 00:59:45 UTC

++ on the patch !

Comment 4 Philip Withnall 2014-12-20 15:33:25 UTC

Merged, thanks.

commit 8ecd6b89a815856b52d5931dd3a6d90fa9e85757
Author: Philip Withnall <philip@tecnocode.co.uk>
Date:   Mon Dec 15 23:35:13 2014 +0000

    agent: Handle EWOULDBLOCK when transmitting pseudo-TCP segments
    
    The pseudo-TCP code previously didn’t handle EAGAIN or EWOULDBLOCK
    errors from the low-level NiceSocket code. This caused pseudo-TCP
    connections to be dropped if the transmitting socket ever filled up,
    which could cause problems on high bandwidth connections.
    
    Fix the issue by effectively dropping the packet on EWOULDBLOCK. This
    will eventually get picked up by the pseudo-TCP recovery mechanism,
    retransmitting the packet and throttling the sender. This should
    hopefully reduce the system resource usage which caused EWOULDBLOCK in
    the first place.
    
    Spotted and debugged by Radosław Kołodziejczyk
    <radek.kolodziejczyk@gmail.com>.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=87344

 agent/agent.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.