Hangbin Liu liuhangbin@gmail.com writes:
+cleanup_ns() +{
- local ns=""
- local errexit=0
- # disable errexit temporary
- if [[ $- =~ "e" ]]; then
errexit=1set +e- fi
- for ns in "$@"; do
ip netns delete "${ns}" &> /dev/nullbusywait 2 "ip netns list | grep -vq $1" &> /dev/null
The grep would get confused by substrings of other names. This should be grep -vq "^$ns$".
if ip netns list | grep -q $1; then
Busywait returns != 0 when the wait condition is not reached within a given time. So it should be possible to roll the duplicated if-grep into the busywait line like so:
if ! busywait 2 "ip netns etc."; then
echo "Failed to remove namespace $1"return $ksft_skip
This does not restore the errexit.
I think it might be clearest to have this function as a helper, say __cleanup_ns, and then have a wrapper that does the errexit management:
cleanup_ns() { local errexit local rc
# disable errexit temporarily if [[ $- =~ "e" ]]; then errexit=1 set +e fi
__cleanup_ns "$@" rc=$?
[ $errexit -eq 1 ] && set -e return $rc }
If this comes up more often, we can have a helper like with_disabled_errexit or whatever, that does this management and dispatches to "$@", so cleanup_ns() would become:
cleanup_ns() { with_disabled_errexit __cleanup_ns "$@" }
fi- done
- [ $errexit -eq 1 ] && set -e
- return 0
+}
+# By default, remove all netns before EXIT. +cleanup_all_ns() +{
- cleanup_ns $NS_LIST
+} +trap cleanup_all_ns EXIT
Hmm, OK, this is a showstopper for inclusion from forwarding/lib.sh, because basically all users of forwarding/lib.sh use the EXIT trap.
I wonder if we need something like these push_cleanup / on_exit helpers:
https://github.com/pmachata/stuff/blob/master/ptp-test/lib.sh#L15
But I don't want to force this on your already large patchset :) So just ignore the bit about including from forwarding/lib.sh.
+# setup netns with given names as prefix. e.g +# setup_ns local remote +setup_ns() +{
- local ns=""
- # the ns list we created in this call
- local ns_list=""
- while [ -n "$1" ]; do
I would find it more readable if this used the same iteration approach as the 'for ns in "$@"' above. The $1/shift approach used here is somewhat confusing.
# Some test may setup/remove same netns multi timesif unset $1 2> /dev/null; thenns="${1,,}-$(mktemp -u XXXXXX)"eval readonly $1=$nselseeval ns='$'$1cleanup_ns $nsfiip netns add $nsif ! ip netns list | grep -q $ns; then
As above, the grep could get confused. But in fact wouldn't just checking the exit code of ip netns add be enough?
echo "Failed to create namespace $1"cleanup_ns $ns_listreturn $ksft_skipfiip -n $ns link set lo upns_list="$ns_list $ns"shift- done
- NS_LIST="$NS_LIST $ns_list"
+}