ooni / probe

OONI Probe network measurement tool for detecting internet censorship
https://ooni.org/install
BSD 3-Clause "New" or "Revised" License
751 stars 142 forks source link

probe-cli: Update to quic-go v0.37 #2492

Closed kelmenhorst closed 10 months ago

kelmenhorst commented 1 year ago

Following the update to quic-go with Go 1.20 support ( #2478), we want to update our QUIC code in netxlite to use the new quic-go API, as introduced in release v0.35.0.

Plan:

  1. Substitute calls to quic.DialEarlyContext with calls to quic.DialEarlynetxlite/quic.go

  2. Replace all instances of Listener with *Listener (same for EarlyListener) → simplequicping_test.go.

  3. Use new quic.Transport in (*quicDialerQUICGo).DialContext. Edit: I believe we should not use the quic.Transport after all, due to the following reasons:

    • We do not have the intended use case of the quic.Transport, i.e. multiplexing multiple QUIC connections on a single net.PacketConn. Instead, we establish a 1:1 relationship between net.PacketConn and QUIC connection and take care of closing every UDP socket after using it. The design of quicDialerQUICGo conveniently fits to the new API as it makes sure that we do not use the same net.PacketConn for multiple subsequent Dials.
    • We use quic.DialEarly which uses a single-use Transport under the hood. There is probably no need to handle this extra complexity ourselves.
    • However, if in the future we decide to use the same UDP socket for multiple (concurrent) QUIC connections, we should use quic.Transport. In this case, we should have a singleton quic.Transport in quicDialerQUICGo which creates a single net.PacketConn used by all Dials. This quic.Transport needs to be closed as soon as we are not using the Dialer anymore.
  4. Possibly identify other cases where we pass the same net.PacketConn to Listen and Dial subsequently. → This is not the case.

Related naming issues:

Update (07.08.2023): We want to update to quic-go 0.37.3 which should solve issues we had with the 0.35 version.

bassosimone commented 10 months ago

We merged #1161, so we can close this issue.