From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Vorel Subject: Re: [PATCH iproute2 4/4] testsuite: remove gre kmods if the test loads them Date: Sat, 15 Dec 2018 18:37:07 +0100 Message-ID: <20181215173707.GB29950@x230> References: <20181215153051.13166-1-bluca@debian.org> <20181215153320.14113-1-bluca@debian.org> Reply-To: Petr Vorel Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, stephen@networkplumber.org, Petr Vorel To: Luca Boccassi Return-path: Received: from mx2.suse.de ([195.135.220.15]:56106 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726494AbeLORhM (ORCPT ); Sat, 15 Dec 2018 12:37:12 -0500 Content-Disposition: inline In-Reply-To: <20181215153320.14113-1-bluca@debian.org> Sender: netdev-owner@vger.kernel.org List-ID: Hi Luca, > The tunnel test leaves behind link devices created by the GRE kernel > modules: > $ ip -br link > ... > gre0@NONE DOWN 0.0.0.0 > gretap0@NONE DOWN 00:00:00:00:00:00 > erspan0@NONE DOWN 00:00:00:00:00:00 > ip6tnl0@NONE DOWN :: > ip6gre0@NONE DOWN 00:00:00:00: > $ lsmod | grep gre > ip6_gre 40960 0 > ip6_tunnel 40960 1 ip6_gre > ip_gre 32768 0 > ip_tunnel 24576 1 ip_gre > gre 16384 2 ip6_gre,ip_gre > Check beforehand if the gre kernel module is loaded, and if not unload > them all at the end of the test. This should avoid causing problems if > a user is already using GRE for other purposes. > Signed-off-by: Luca Boccassi Reviewed-by: Petr Vorel > --- > testsuite/tests/ip/tunnel/add_tunnel.t | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > diff --git a/testsuite/tests/ip/tunnel/add_tunnel.t b/testsuite/tests/ip/tunnel/add_tunnel.t > index 3f5a9d3c..76f8b011 100755 > --- a/testsuite/tests/ip/tunnel/add_tunnel.t > +++ b/testsuite/tests/ip/tunnel/add_tunnel.t > @@ -4,6 +4,15 @@ > TUNNEL_NAME="tunnel_test_ip" > +# note that checkbashism reports command -v, but dash supports it and it's posix compliant NOTE: also 'busybox sh' and mksh (used also in older Android) support it. BTW: yes, it's not optional in POSIX 2008/2013, it's optional in 2004 [1]. But I'd put this info only into commit message, most people probably does not care. > +if command -v lsmod >/dev/null 2>&1 && command -v rmmod >/dev/null 2>&1 > +then > + KMODS="ip6_gre ip6_tunnel ip_gre ip_tunnel gre" > + COUNT_KMODS_BEFORE=$(lsmod | grep -c -e "^ip6_gre" -e "^ip6_tunnel" -e "^ip_gre" -e "^ip_tunnel" -e "^gre") > +else > + KMODS="" > +fi ... > +if [ -n "$KMODS" ] > +then > + # unload kernel modules to remove dummy interfaces only if they were not in use beforehand > + COUNT_KMODS_AFTER=$(lsmod | grep -c -e "^ip6_gre" -e "^ip6_tunnel" -e "^ip_gre" -e "^ip_tunnel" -e "^gre") > + if [ "$COUNT_KMODS_BEFORE" -eq 0 ] && [ "$COUNT_KMODS_AFTER" -gt 0 ] > + then > + for mod in $KMODS > + do > + sudo rmmod "$mod" > + done > + else > + ts_log "[gre kernel module was loaded before test, not removing]" > + fi > +fi You repeating yourself with module names. How about something like this: KMODS="ip6_gre ip6_tunnel ip_gre ip_tunnel gre" # unload kernel modules to remove dummy interfaces only if they were not in use beforehand KMODS_REMOVE= if command -v lsmod >/dev/null 2>&1 && command -v rmmod >/dev/null 2>&1; then for i in $KMODS; do lsmod |grep -q "^$i " || KMODS_REMOVE="$KMODS_REMOVE $i"; done fi ... for mod in $KMODS_REMOVE; do sudo rmmod "$mod" done I.e. keeping modules to remove (you can also loaded modules, if you want to warn about it, but I'd ignore it). BTW I'd use 'then' and 'do' on the same line (i.e.: if [ ... ]; then) + define variable before usage. [1] https://stackoverflow.com/questions/34572700/is-command-v-option-required-in-a-posix-shell-is-posh-compliant-with-posix Kind regards, Petr