add support for tun/tap devices #14
Conversation
|
Hi Luis! Thank you very much for your contribution! Overall, it looks good to me and I'll be happy to merge this PR. Would you mind making some minor adjustments before I merge it? I left a few comments there. |
Makefile
Outdated
|
|
||
| net: | ||
| tunctl -t tap0 -u `whoami` | ||
| brctl addif docker0 tap0 |
There was a problem hiding this comment.
Probably docker0 could be replaced with a dynamic lookup of the bridge interface name docker network inspect -f {{.Options}} bridge. Although it still wouldn't be a fully universal solution, it'd make this target more universal.
Additionally, the bridge's interface name could become a variable with the default name derived from Docker's default bridge interface. In such case, a user would be able to override it via an environment variable.
Makefile
Outdated
| fi | ||
|
|
||
| net: | ||
| tunctl -t tap0 -u `whoami` |
There was a problem hiding this comment.
Both tunctl and brctl are deprecated at this time. I'd ask you to use ip instead. It'd also decrease the needed dependencies for this project.
There was a problem hiding this comment.
didnt know they were going to deprecate it :(
Makefile
Outdated
| brctl addif docker0 tap0 | ||
| ip link set tap0 up | ||
|
|
||
| run-debian: |
There was a problem hiding this comment.
This target doesn't really mention debian anywhere in the command. Probably, run-qemu-with-net or something like that would be a more suitable name. Also, it de facto depends on the linux.img and net targets. I think you could just reflect this fact in the target line.
2 participants
Hello there,
Thanks for making this project opensource. Its really useful for me already!
Idea of this PR is to add network support. Not sure if we want all of them (nat/bridged/etc). For now already works if you have a 'docker0' interface which is pretty common I'd say.
Any ideas/feedback are appreciated :)