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

UDP transport #639

Closed
totaam opened this issue Aug 19, 2014 · 18 comments
Closed

UDP transport #639

totaam opened this issue Aug 19, 2014 · 18 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Aug 19, 2014

Rather than using mosh #219, we should be able to do our own UDP transport: we know which packets must arrive and may need to be re-sent (window metadata, etc), and which ones we can just skip when they go missing: we just send a newer update instead (window pixels, cursors, etc..).

Then we can also tune the video encoders and insert key frames when needed (and maybe also lower the default gap between key frames when UDP is used), etc..

This may also helps us achieve other things: we could more easily use parallel processes for encoding since each process can then send its data without needing to synchronize or share the socket. Also useful when using hardware encoders which have their own network ports.

We should not need to worry about UDP hole punching for now, if ever.
I don't think we want to be using a library like UDT either: we would lose some flexibility, and the python bindings are old and not very portable...

@totaam
Copy link
Collaborator Author

totaam commented Dec 26, 2014

Some preliminary notes:

  • we can't do encryption to begin with (see protect against mitm attacks #198, Implementation of API for crypto modules #584) because we use AES CBC: In CBC mode, each block of plaintext is XORed with the previous ciphertext block before being encrypted. This way, each ciphertext block depends on all plaintext blocks processed up to that point. (this breaks as soon as we get packets out of order, or if we drop any..)
  • if we don't get an ACK from the client after a delay, we must probe it (we could augment the paint packet with a new capability)
  • pass flag to video encoders so they can enable support from dropped frames?
  • maybe use a new command line argument: --bind= which would bind to both TCP and UDP ports?
  • how do we take into account the maximum UDP packet size (and detect or change it..)
  • the protocol class would need new threads for UDP: read and write (assuming each end does both - not necessarily the case..)
  • client will need to wait for the paint packets to arrive in the right order, and timeout if one goes MIA, deal with dupes - we could also decode them (at least some of them... like non video) whilst waiting
  • client can ask server for a refresh when things go wrong, we already reset the video pipeline when the client reports decoding errors

very distant future: handle UDP broadcasts for multiple clients

@totaam
Copy link
Collaborator Author

totaam commented Apr 14, 2015

Not as useful as first thought, too many implementation issues, other more pressing issues (ie: #835) would clash with this. So re-scheduling as future.

@totaam
Copy link
Collaborator Author

totaam commented Feb 17, 2016

For the encryption part, we can use CBC with a new IV based on the packet sequence number, or a counter mode, or just DTLS.

@totaam
Copy link
Collaborator Author

totaam commented Feb 17, 2016

See also #1124

The packet loss rate may have an effect on our encoding selection: maybe we should use encodings that tolerate packet loss better. ie: not png or jpeg for big areas.
We already keep track of what hasn't been acked yet, and we can re-send if necessary.

@totaam
Copy link
Collaborator Author

totaam commented Jul 31, 2017

@totaam
Copy link
Collaborator Author

totaam commented Aug 17, 2017

In order to do DTLS, we would need to use something like PyDTLS (no API docs, no examples, no commits for 4 months, no python3 support) or use the raw openssl via python-cryptography (support added in issue 3501 - circa 2.0?) - problem is that >=1.9 doesn't build on macos: #1544, and is a much lower level API.

@totaam
Copy link
Collaborator Author

totaam commented Aug 17, 2017

2017-08-17 14:55:27: antoine uploaded file udp.patch (31.1 KiB)

udp and dtls work in progress: client can send hello wrapped in a UDP frame

@totaam
Copy link
Collaborator Author

totaam commented Aug 21, 2017

Here's how the UDP patch above works:

  • we aggregate packet chunks into one buffer (and so we lose zero copy...)
  • we send this buffer to the other end via UDP, after splitting it into chunks that fit into the MTU (which we can get client-side since we call connect() and we send that value to the server via udp-control packets)
  • when receiving, we reconstruct the original buffer - caching chunks until we have all of them
  • if we are missing some packets (missing sequence number) or chunks, we send a resend request
  • some packet types (in particular the UDP control packet) don't need to honour the sequence number
  • other packets are not resent, we just tell the other end to forget them and we send a new one instead (with a new sequence number)

Still TODO:

  • (re)schedule udp-control packets based on observed latency (currently every set at 2 seconds for debugging)
  • uuid and protocols: should the client make an initial request for one? (syncookie like?)
  • drop AES? (it's a pain and untested)
  • drop DTLS? (or fix it - behaves more like a stream?)
  • drop packet accounting hooks (not reliable anyway?)
  • mmap cannot be used (needs reliable delivery to clear after use)
  • refactor the protocol class and think about RFB: rfb server support #1620
  • cythonize the protocol classes?
  • drop protocol format / parse threads? (we can compress the clipboard in the encode thread)
  • threading races with packet structures?
  • handle client roaming: update target address on server (and reset statistics?)
  • test over wifi, DSL, 3G, etc
  • turn off delta (may fail if we missed the previous paint it relies on), expose "asynchronous" capability of protocol?
  • verify which audio codecs deal with missing packets gracefully, disable the others when using UDP
  • more asynchronous packets (from NetworkProtocol): bell, notify close, info-response, close-window, key-action, key-repeat, server-settings, info-request, encoding, etc.
  • add more packet fail callbacks. Resend: cursors, window icons, pointer position, ping. Drop: audio data
  • maybe add logic to choose between sending a new draw packet and sending just the missing chunks (cheaper)?
  • inband udp control via udp header bitmap, so receiver knows a packet was optional / asynchronous without needing / waiting for a "udp-control" packet?
  • paint ack to be merged with udp-control?
  • ping and ping-echo merged too? piggyback?
  • "udp-control" should not need to be a "ui-packet" server-side - could cause race conditions
  • add bitmap of preceding packets so udp receiver knows which packets it doesn't need to wait for (those we can resend anyway)
  • queued priorities? (clipboard packets only need to wait for each other, not paint, etc)
  • video: try decoding partial packets? only if we have the header? wait only a little bit for those?
  • prevent resends from accumulating and causing the "udp-control" packets to grow too big

Good read: Dealing with IPv6 fragmentation in the DNS: UDP is different, and in UDP a functional response to path message size issue inevitably relies on interaction with the upper-level application protocol.

@totaam
Copy link
Collaborator Author

totaam commented Aug 23, 2017

2017-08-23 09:04:17: antoine uploaded file udp-v6.patch (89.4 KiB)

add some rfb refactoring

@totaam
Copy link
Collaborator Author

totaam commented Aug 27, 2017

Merged the rfb parts and some preparatory work in r16710 + r16712. (see #1620#comment:2)

@totaam
Copy link
Collaborator Author

totaam commented Aug 27, 2017

With the patch above, UDP seems to work quite reliably, even with a high packet loss (10%).
Tested with:

xpra start-desktop --start=xterm --bind-udp=0.0.0.0:10000 -d udp
XPRA_UDP_DROP_PCT=10 xpra attach udp:localhost:10000 --speaker=no -d udp --remote-logging=no

(adding XPRA_USE_ALIASES=0 is useful for debugging network traffic with debug logging or with tcpdump)

Things left to fix:

  • connection fails if the packet loss happens early and server side
  • test over wifi, DSL, 3G, etc
  • performance: profile it, cythonize it, etc
  • audio codecs tests
  • add code to try to re-order audio packets (or generalize into a multiqueue?)
  • maybe merge some packet: ping, udp-control, paint acks
  • man page and wiki updates

@totaam
Copy link
Collaborator Author

totaam commented Aug 29, 2017

2017-08-29 11:30:17: antoine uploaded file udp-v12.patch (66.7 KiB)

more aggressive control packet scheduling, calculate which chunks should have arrived, option to test missing the first packet - which requires more tricky udp-control packet scheduling

@totaam
Copy link
Collaborator Author

totaam commented Aug 29, 2017

Code committed in r16734 - see large commit message.

This code also adds a new flag:

XPRA_UDP_DROP_FIRST=1 xpra ...

(set a higher value to drop the first packet more than once)
This was very useful for testing the early asynchronous re-send logic and uncovered a few subtle bugs.
Performance is still pretty decent, even with 10% packet loss at both ends, and without doing any profiling or optimization!
The code is quite tricky to understand, so there are a lot more docstrings than usual in there.
Basic information has also been added to Network.

Still TODO (probably not all for this release):

  • the jitter value is critical, it is currently hard-coded at 20ms - we will need to derive it somehow (hard without knowing which packets are re-sends... we could live patch the header?)
  • audio packets are not re-ordered, but they should be - we will need a small queue, and cap it using the jitter value
  • test over wifi, DSL, 3G, etc
  • performance: profile it, cythonize it, etc
  • audio codecs tests
  • better integration with pixel encoders: switch on error concealment (more key frames), this may require using a container format rather than plain H264... (similar to #1463 - using the ffmpeg encoder)
  • maybe merge some packet: ping, udp-control, paint acks - all a bit redundant!

@totaam
Copy link
Collaborator Author

totaam commented Sep 28, 2017

Minor enhancements in r16969.

For audio, it may be possible to just let gstreamer handle the dropped and out-of-order packets via rtpjitterbuffer: AAC/RTP streaming:

gst-launch-1.0 audiotestsrc ! audioconvert ! avenc_aac ! rtpmp4apay ! udpsink
gst-launch-1.0 udpsrc ! application/x-rtp,clock-rate=44100,config=40002410adca00 ! \
    rtpjitterbuffer ! rtpmp4adepay ! avdec_aac ! audioconvert ! autoaudiosink

Having to specify the caps on the receiver is a pain though.

@totaam
Copy link
Collaborator Author

totaam commented Oct 5, 2017

For DTLS, wolfcrypt-py might be useful.

@totaam
Copy link
Collaborator Author

totaam commented Oct 24, 2017

This will have to do for this release, will follow up in #1669.

@maxmylyn: can you break it?
(bear in mind that this isn't meant to compete with TCP at this point - just to be usable)

@totaam
Copy link
Collaborator Author

totaam commented Oct 27, 2017

2017-10-27 22:57:43: maxmylyn commented


Running a trunk r17263 Fedora 25 server and client:

Unable to break it, and performance feels pretty good, at least almost as good as TCP.

Since there's another ticket for following up, I'm going to go ahead and close this one.

@totaam
Copy link
Collaborator Author

totaam commented May 22, 2021

UDP support has been removed in 2022238.

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

1 participant