Dns over quic support: bis #62
Conversation
Connection & streams errors are now handled as prescribed by the rfc. Comment some implementation choices. C-style prints switched to C++ streams.
Also correct a mistake in getting the target address: next_target() was called at each udp send.
The double free was caused by a misuse of an unique_ptr which freed a buffer owned by quicly. Replacing it by a vector make it work nicely.
uvw::UDPHandle::send takes ownership of the data only when it is passed using an unique_ptr.
The connection is also properly freed, wich plugs a memleak.
This allows to send the connection close message as quickly as possible, and potentially save some packet round trips. E.g., on one particular test instance, a 1-query burst goes from 23 udp packets exchanged to only 12.
A segfault occured at quic connection close, when _quic_session was reset, but packets where still comming. An assert fail occured when receive_data was called on a close()'d quic session. Some extraneous prints removed.
VanStratum
force-pushed
the
dns-over-quic
branch
from
b190bba
to
1f114ea
Compare
|
Hi @VanStratum can you give us a status update here - are you still working on this and hoping to land it? |
|
Yes, at this point there is only minor improvement to be made for it to
be ready, mainly code style, migrating to the last version of quicly,
some doc, and merging.
|
|
Ok awesome, @saradickinson is interested in this work and helping land it. It would be great if we could get that done by the end of september. I can help review. |
|
Okay, I'm focused on finishing my master thesis at the moment, but I will have more time in about two weeks. |
With an adequately defined call to bind(), the handle knows if it must send over IPv4 or IPv6.
The session ending of DoQ and of the tcp-based mode is slightly different, putting them in the same function resulted in obscure code.
Main change is the use of length field, in the same way as DoT or DNS over TCP.
|
It think the code is ready for review. It builds fine with the last version of quicly, and I just migrated it to the latest draft. |
|
@VanStratum shouldn't this PR be against |
|
Yes, I must have made an inattention mistake six mounth ago.
|
|
@VanStratum do you think this work is salvageable - can it be reopened against master branch? |
|
I just moved it to point to master. |
|
Nice! I'll start a review. |
Follow-up on #11.
Currently WIP