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

Initial implementation #1

Merged
merged 3 commits into from
May 24, 2019
Merged

Initial implementation #1

merged 3 commits into from
May 24, 2019

Conversation

oxtoacart
Copy link
Contributor

No description provided.

@oxtoacart oxtoacart force-pushed the init branch 2 times, most recently from 2890eb7 to 242304f Compare May 21, 2019 13:53
@oxtoacart oxtoacart changed the title [WIP] Initial implementation Initial implementation May 22, 2019
@oxtoacart
Copy link
Contributor Author

I've tried running the unit test on Travis and Wercker, and neither support creating a TUN device. The problem is that they (and Circle CI) run unprivileged docker containers for security reasons, and such containers don't have access to /dev/net/tun.

@oxtoacart oxtoacart force-pushed the init branch 3 times, most recently from 086d0fc to 701b6c1 Compare May 23, 2019 18:34
.drone.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
conntrack_linux.go Outdated Show resolved Hide resolved
gonat.go Outdated Show resolved Hide resolved
gonat.go Show resolved Hide resolved
gonat_linux.go Show resolved Hide resolved
gonat_linux.go Show resolved Hide resolved
gonat_linux.go Outdated Show resolved Hide resolved
packet.go Show resolved Hide resolved
gonat.go Show resolved Hide resolved
@oxtoacart
Copy link
Contributor Author

@hwh33 I've pushed updates that address your insightful code review comments.

gonat_linux.go Show resolved Hide resolved
packet.go Show resolved Hide resolved
packet.go Show resolved Hide resolved
gonat_linux.go Outdated Show resolved Hide resolved
@hwh33
Copy link
Contributor

hwh33 commented May 24, 2019

Would you mind explaining why we use ops.Go to spawn goroutines? I looked at the package a bit and it looks like it's adding context to the goroutines. That context looks useful, but how is it actually used? Is it logged somewhere? Or accessible to and used by the goroutines somehow?

Also thanks for the patience with all the questions on this PR haha

@oxtoacart
Copy link
Contributor Author

Would you mind explaining why we use ops.Go to spawn goroutines?

golog reports the context associated with the current goroutine when it logs. The same context is also included when we report to borda.

For example, here's a log seen while running Lantern:

May 24 13:09:32.754 - 0m3s DEBUG balancer: balancer.go:355 Successfully dialed via fp-donyc3u16-20190523-138 (142.93.178.69:443) to connect://borda.lantern.io:443 on pass 0 (52.953949ms) [beam=37 local_proxy_type=http op=balancer_dial_details origin=borda.lantern.io:443 origin_host=borda.lantern.io origin_port=443 remotely_proxied=true root_op=proxy user_agent=Go-http-client/1.1]

All those key=value pairs in square brackets are the context.

@oxtoacart
Copy link
Contributor Author

Also thanks for the patience with all the questions on this PR haha

Thanks for the thorough review!

@hwh33
Copy link
Contributor

hwh33 commented May 24, 2019

LGTM!

Thank you for the explanation of ops.Go and the use of the context. That's helpful, especially with the example.

@oxtoacart oxtoacart merged commit 4cddefb into master May 24, 2019
@oxtoacart
Copy link
Contributor Author

Thanks for the thorough review!

@oxtoacart oxtoacart deleted the init branch May 24, 2019 15:27
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

Successfully merging this pull request may close these issues.

2 participants