-
Notifications
You must be signed in to change notification settings - Fork 2k
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
dist/tools/ethos: add setup_network.sh script #11816
Conversation
FYI in #11808 I also makes use of this script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on #11808 and #11818, I was able to run make test
using ethos without using sudo. There is quite some code duplication between setup_network.sh
and start_network.sh
. I feel they can probably factorized, something like this:
diff --git a/dist/tools/ethos/setup_network.sh b/dist/tools/ethos/setup_network.sh
index baee92b2d..7fab50a99 100755
--- a/dist/tools/ethos/setup_network.sh
+++ b/dist/tools/ethos/setup_network.sh
@@ -14,8 +14,8 @@ remove_tap() {
ip tuntap del ${TAP} mode tap
}
-cleanup() {
- echo "Cleaning up..."
+cleanup_setup() {
+ echo "Cleaning up setup ..."
remove_tap
ip a d fd00:dead:beef::1/128 dev lo
trap "" INT QUIT TERM EXIT
@@ -43,7 +43,6 @@ if [ -z "$_USER" ]; then
fi
fi
-trap "cleanup" INT QUIT TERM EXIT
-
+trap "cleanup_setup" INT QUIT TERM EXIT
create_tap && start_uhcpd
diff --git a/dist/tools/ethos/start_network.sh b/dist/tools/ethos/start_network.sh
index 928f05453..fc90e6b2e 100755
--- a/dist/tools/ethos/start_network.sh
+++ b/dist/tools/ethos/start_network.sh
@@ -2,31 +2,14 @@
ETHOS_DIR="$(dirname $(readlink -f $0))"
-create_tap() {
- ip tuntap add ${TAP} mode tap user ${USER}
- sysctl -w net.ipv6.conf.${TAP}.forwarding=1
- sysctl -w net.ipv6.conf.${TAP}.accept_ra=0
- ip link set ${TAP} up
- ip a a fe80::1/64 dev ${TAP}
- ip a a fd00:dead:beef::1/128 dev lo
- ip route add ${PREFIX} via fe80::2 dev ${TAP}
-}
-
-remove_tap() {
- ip tuntap del ${TAP} mode tap
-}
-
cleanup() {
echo "Cleaning up..."
- remove_tap
- ip a d fd00:dead:beef::1/128 dev lo
- kill ${UHCPD_PID}
+ kill -TERM $(pgrep -f "uhcpd" | xargs)
trap "" INT QUIT TERM EXIT
}
-start_uhcpd() {
- ${UHCPD} ${TAP} ${PREFIX} > /dev/null &
- UHCPD_PID=$!
+setup_network() {
+ ${ETHOS_DIR}/setup_network.sh $1 $2 &
}
PORT=$1
@@ -46,5 +29,4 @@ UHCPD="$(readlink -f "${ETHOS_DIR}/../uhcpd/bin")/uhcpd"
trap "cleanup" INT QUIT TERM EXIT
-
-create_tap && start_uhcpd && "${ETHOS_DIR}/ethos" ${TAP} ${PORT} ${BAUDRATE}
+setup_network ${TAP} ${PREFIX} && "${ETHOS_DIR}/ethos" ${TAP} ${PORT} ${BAUDRATE}
Travis is complaining about a trailing white space in the README. Can you add a some doc in the README specifying the difference between setup_network.sh
and start_network.sh
(I know it is not really related but its weird having only one of them documented).
@kaspar030 ping |
dff6886
to
6f1147a
Compare
fixed |
I've integrated your proposal, with minor changes. (e.g., start_network.sh is now remembering the PID of the called setup_network, instead of blindly killing all uhcpds on the system). But this needs now to be tested with existing users of start_network.sh... |
How about we split this deduplication into a seperate PR, so we can focus on #11818 first? |
What kind of testing? Does this need to be considered as an API change? The functionality is the same and the way the script is called is the same, this could only cause problems for user that are sourcing the script and using the defined functions right?
I would be Ok with this. |
6f1147a
to
a3ec33d
Compare
yeah, but the implementation is slightly different. So if start_network.sh works as before needs to be tested, e.g., with the border router example.
done, #11840 |
OK! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaspar030 this one still needs squashing! |
a3ec33d
to
f74e5e7
Compare
thx, done! |
GO! |
Thanks! |
Contribution description
This PR provides an alternative to start_network.sh.
start_network.sh is supposed to be use as TERMPROG and needs to be started as root.
It sets up a tap device, starts uhcpd in background, then launches ethos.
This PR's setup_network.sh drops starting ethos and starts uhcpd in foreground.
The use-case is:
sudo setup_network.sh <params>
in one terminal, keep runningmake term/test
in another terminal, re-using the existing networkTesting procedure
This is used in the soon-to-be-openened SUIT PR. I suggest using that PR as testbed.
Issues/PRs references