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

cleanup(engine): remove the TProxy abstraction #2224

Closed
bassosimone opened this issue Aug 23, 2022 · 0 comments · Fixed by ooni/probe-cli#874
Closed

cleanup(engine): remove the TProxy abstraction #2224

bassosimone opened this issue Aug 23, 2022 · 0 comments · Fixed by ooni/probe-cli#874
Assignees
Labels
cleanup There's need to cleanup stuff a bit ooni/probe-engine refactoring

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Aug 23, 2022

Around one year ago we experimented using the TProxy abstraction for implementing integration testing. Where this experiment had been partially successful in allowing us to write integration testing, we eventually stopped it midway when we removed the Fall 2021 prototype of the websteps experiment. Since then, this code is basically unused. What's more, it is also becoming partially annoying because it introduces an interface between the high-level part of netxlite and the getaddrinfo engine (which I noticed while working on #1516). So, my argument here is that the code that survived from the removal of websteps Fall 2021 is ~50 LoC of unused abstractions and we probably want to remove this unused abstraction. Shall we need something similar in the future, we could introduce it again. Recently, @fortuna mentioned to me that probably shadowsocks is a good way to have a VPN on a probe (e.g., a mobile probe) that moves the data outside the device into a network in which we implement censorship for testing. If we implement such a plan, probably we need something slightly different from what we used to do with TProxy. Hence, I am going to remove TProxy.

@bassosimone bassosimone added refactoring ooni/probe-engine cleanup There's need to cleanup stuff a bit labels Aug 23, 2022
@bassosimone bassosimone self-assigned this Aug 23, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 23, 2022
While there, replace mixture of mocking and real connections inside
quicping with pure mocking of network connections.

See ooni/probe#2224
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 23, 2022
* cleanup: remove UnderlyingNetworkLibrary and TProxy

While there, replace mixture of mocking and real connections inside
quicping with pure mocking of network connections.

Closes ooni/probe#2224

* cleanup: we don't need a SimpleResolver now

This type was only used by UnderlyingNetworkLibrary and all the
rest of the code uses Resolver. So, let's avoid complexity by zapping
the SimpleResolver type and merging it inside Resolver.
bassosimone added a commit to ooni/probe-cli that referenced this issue Oct 12, 2022
This functionality has slightly changed since when we removed it
in ooni/probe#2224.

Nevertheless, in #969, we
determined that something like the previous TProxy, with small
changes, was required to support ooni/probe#2340.
bassosimone added a commit to ooni/probe-cli that referenced this issue Oct 12, 2022
We originally removed the TProxy in ooni/probe#2224. Nevertheless, in #969, we determined that something like the previous TProxy, with small changes, was required to support ooni/probe#2340. So, this pull request reintroduces a slightly-modified TProxy functionality that better adapts to the `--remote=REMOTE` use case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup There's need to cleanup stuff a bit ooni/probe-engine refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant