From fe5b253a6fe4e8768737e0985fc3b3c4572b41e2 Mon Sep 17 00:00:00 2001 From: Kevin Deems Date: Tue, 5 May 2020 15:25:40 -0400 Subject: [PATCH] [Style] Bash Script Clean-Up This PR improves the style of our bash shell scripts through ShellCheck. Commit log: * Early termination, return issues * Revert CI changes, update ONVM script * Fix docker script --- examples/NFD/go.sh | 2 +- examples/go.sh | 6 ++--- examples/start_nf.sh | 14 +++++++----- onvm/go.sh | 20 ++++++++-------- onvm_web/build_web.sh | 6 ++--- onvm_web/start_web_console.sh | 13 +++++------ scripts/docker.sh | 24 +++++++++++--------- scripts/dpdk_helper_scripts.sh | 7 +++--- scripts/install.sh | 22 ++++++++++-------- scripts/no_hyperthread.sh | 4 ++-- scripts/setup_environment.sh | 16 +++++++------ tools/Pktgen/openNetVM-Scripts/run-pktgen.sh | 8 +++---- 12 files changed, 74 insertions(+), 68 deletions(-) diff --git a/examples/NFD/go.sh b/examples/NFD/go.sh index 5bce8c94e..d2e286550 100755 --- a/examples/NFD/go.sh +++ b/examples/NFD/go.sh @@ -14,4 +14,4 @@ if [ ! -f ../../start_nf.sh ]; then exit 1 fi -../../start_nf.sh $NF_DIR "$@" +../../start_nf.sh "$NF_DIR" "$@" diff --git a/examples/go.sh b/examples/go.sh index 8bcb99221..3c7431f49 100755 --- a/examples/go.sh +++ b/examples/go.sh @@ -11,12 +11,12 @@ if [ ! -f ../start_nf.sh ]; then fi SCRIPT=$(readlink -f "$0") -MANAGER_PATH=$(dirname $(dirname "$SCRIPT")) +MANAGER_PATH=$(dirname "$(dirname "$SCRIPT")") -if [[ -z $(ps ww -u root | grep "$MANAGER_PATH/onvm/onvm_mgr/$RTE_TARGET/onvm_mgr") ]] +if [[ -z $(pgrep -u root -f "$MANAGER_PATH/onvm/onvm_mgr/$RTE_TARGET/onvm_mgr") ]] then echo "NF cannot start without a running manager" exit 1 fi -../start_nf.sh $NF_DIR "$@" +../start_nf.sh "$NF_DIR" "$@" diff --git a/examples/start_nf.sh b/examples/start_nf.sh index 5d8cc7458..85078392a 100755 --- a/examples/start_nf.sh +++ b/examples/start_nf.sh @@ -24,14 +24,14 @@ SCRIPT=$(readlink -f "$0") SCRIPTPATH=$(dirname "$SCRIPT") NF_NAME=$1 NF_PATH=$SCRIPTPATH/$NF_NAME -# For NFD NF +# For NFD NF NF_NAME=${NF_PATH##*/} BINARY=$NF_PATH/build/app/$NF_NAME DPDK_BASE_ARGS="-n 3 --proc-type=secondary" # For simple mode, only used for initial dpdk startup DEFAULT_CORE_ID=0 -if [ ! -f $BINARY ]; then +if [ ! -f "$BINARY" ]; then echo "ERROR: NF executable not found, $BINARY doesn't exist" echo "Please verify NF binary name and run script from the NF folder" exit 1 @@ -44,7 +44,7 @@ if [ "$1" = "-F" ] then config=$2 shift 2 - exec sudo $BINARY -F $config "$@" + exec sudo "$BINARY" -F "$config" "$@" fi # Check if -- is present, if so parse dpdk/onvm specific args @@ -61,8 +61,8 @@ done # Spaces before $@ are required otherwise it swallows the first arg for some reason if [[ $dash_dash_cnt -ge 2 ]]; then - DPDK_ARGS="$DPDK_BASE_ARGS $(echo " ""$@" | awk -F "--" '{print $1;}')" - ONVM_ARGS="$(echo " ""$@" | awk -F "--" '{print $2;}')" + DPDK_ARGS="$DPDK_BASE_ARGS $(echo " ""$*" | awk -F "--" '{print $1;}')" + ONVM_ARGS="$(echo " ""$*" | awk -F "--" '{print $2;}')" # Move to NF arguments shift ${non_nf_arg_cnt} if [[ $DPDK_ARGS =~ "-l" && ! $ONVM_ARGS =~ "-m" ]]; then @@ -82,4 +82,6 @@ elif [[ $dash_dash_cnt -eq 1 ]]; then exit 1 fi -exec sudo $BINARY $DPDK_ARGS -- $ONVM_ARGS -- "$@" +# don't mess with variable expansion +# shellcheck disable=SC2086 +exec sudo "$BINARY" $DPDK_ARGS -- $ONVM_ARGS -- "$@" diff --git a/onvm/go.sh b/onvm/go.sh index 4113d3405..c1cbef7ff 100755 --- a/onvm/go.sh +++ b/onvm/go.sh @@ -39,7 +39,7 @@ verbosity=1 # Initialize base virtual address to empty. virt_addr="" -if [[ ! -z $(ps ww -u root | grep "$SCRIPTPATH/onvm_mgr/$RTE_TARGET/onvm_mgr" | grep -v "grep") ]] +if [[ -n $(pgrep -u root -f "$SCRIPTPATH/onvm_mgr/$RTE_TARGET/onvm_mgr") ]] then echo "Manager cannot be started while another is running" exit 1 @@ -47,7 +47,7 @@ fi shift 3 -if [ -z $nf_cores ] +if [ -z "$nf_cores" ] then usage fi @@ -63,14 +63,14 @@ while getopts "a:r:d:s:t:l:p:z:cv" opt; do case $opt in a) virt_addr="--base-virtaddr=$OPTARG";; r) num_srvc="-r $OPTARG";; - d) def_srvc="-d $optarg";; + d) def_srvc="-d $OPTARG";; s) stats="-s $OPTARG";; t) ttl="-t $OPTARG";; l) packet_limit="-l $OPTARG";; p) web_port="$OPTARG";; z) stats_sleep_time="-z $OPTARG";; c) shared_cpu_flag="-c";; - v) verbosity=$(($verbosity+1));; + v) verbosity=$((verbosity+1));; \?) echo "Unknown option -$OPTARG" && usage ;; esac @@ -87,7 +87,7 @@ fi if [ "${stats}" = "-s web" ] then - cd ../onvm_web/ + cd ../onvm_web/ || usage if [ -n "${web_port}" ] then . start_web_console.sh -p "${web_port}" @@ -95,15 +95,17 @@ then . start_web_console.sh fi - cd $ONVM_HOME/onvm + cd "$ONVM_HOME"/onvm || usage fi sudo rm -rf /mnt/huge/rtemap_* -sudo $SCRIPTPATH/onvm_mgr/$RTE_TARGET/onvm_mgr -l $cpu -n 4 --proc-type=primary ${virt_addr} -- -p ${ports} -n ${nf_cores} ${num_srvc} ${def_srvc} ${stats} ${stats_sleep_time} ${verbosity_level} ${ttl} ${packet_limit} ${shared_cpu_flag} +# watch out for variable expansion +# shellcheck disable=SC2086 +sudo "$SCRIPTPATH"/onvm_mgr/"$RTE_TARGET"/onvm_mgr -l "$cpu" -n 4 --proc-type=primary ${virt_addr} -- -p ${ports} -n ${nf_cores} ${num_srvc} ${def_srvc} ${stats} ${stats_sleep_time} ${verbosity_level} ${ttl} ${packet_limit} ${shared_cpu_flag} if [ "${stats}" = "-s web" ] then echo "Killing web stats running with PIDs: $ONVM_WEB_PID, $ONVM_WEB_PID2" - kill $ONVM_WEB_PID - kill $ONVM_WEB_PID2 + kill "$ONVM_WEB_PID" + kill "$ONVM_WEB_PID2" fi diff --git a/onvm_web/build_web.sh b/onvm_web/build_web.sh index 57a108f8f..45ba8ed8e 100755 --- a/onvm_web/build_web.sh +++ b/onvm_web/build_web.sh @@ -52,9 +52,7 @@ cp -r react-app/build ./web-build echo "Minifying web files" for css in ./web-build/static/css/*.css; do - uglifycss --output $css $css + uglifycss --output "$css" "$css" done -for js in $(find ./web-build -name '*.js'); do - uglifyjs --output $js $js -done \ No newline at end of file +find ./web-build -name "*.js" -exec uglifyjs --output {} {} \; diff --git a/onvm_web/start_web_console.sh b/onvm_web/start_web_console.sh index bc372ba51..0a3973c0c 100755 --- a/onvm_web/start_web_console.sh +++ b/onvm_web/start_web_console.sh @@ -52,16 +52,15 @@ while getopts "p:" opt; do esac done - # Start ONVM web stats console at http://localhost: echo -n "Starting openNetVM Web Stats Console at http://localhost:" -echo $web_port +echo "$web_port" is_web_port_in_use=$(sudo netstat -tulpn | grep LISTEN | grep ":$web_port") if [[ "$is_web_port_in_use" != "" ]] then echo "[ERROR] Web port $web_port is in use" - echo $is_web_port_in_use + echo "$is_web_port_in_use" echo "[ERROR] Web stats failed to start" exit 1 fi @@ -70,15 +69,15 @@ is_data_port_in_use=$(sudo netstat -tulpn | grep LISTEN | grep ":8000") if [[ "$is_data_port_in_use" != "" ]] then echo "[ERROR] Port 8000 is in use" - echo $is_data_port_in_use + echo "$is_data_port_in_use" echo "[ERROR] Web stats failed to start" exit 1 fi -cd $ONVM_HOME/onvm_web +cd "$ONVM_HOME"/onvm_web || usage nohup python cors_server.py 8000 & export ONVM_WEB_PID=$! -cd $ONVM_HOME/onvm_web/web-build -nohup python -m SimpleHTTPServer $web_port & +cd "$ONVM_HOME"/onvm_web/web-build || usage +nohup python -m SimpleHTTPServer "$web_port" & export ONVM_WEB_PID2=$! diff --git a/scripts/docker.sh b/scripts/docker.sh index a2dc20e87..0efc247a0 100755 --- a/scripts/docker.sh +++ b/scripts/docker.sh @@ -44,11 +44,11 @@ while getopts :d:c:D:h:o:n: OPTION ; do h) HUGE=${OPTARG} ;; o) ONVM=${OPTARG} ;; n) NAME=${OPTARG} ;; + \?) echo "Unknown option -$OPTARG" && exit 1 esac done DEVICES=() -MODE="" if [[ "${NAME}" == "" ]] || [[ "${HUGE}" == "" ]] || [[ "${ONVM}" == "" ]] ; then echo -e "sudo ./docker.sh -h HUGEPAGES -o ONVM -n NAME [-D DEVICES] [-d DIRECTORY] [-c COMMAND]\n" @@ -65,35 +65,37 @@ for DEV in ${RAW_DEVICES} ; do done if [[ "${DIR}" != "" ]] ; then - DIR="--volume=${DIR}:/$(basename ${DIR})" + DIR="--volume=${DIR}:/$(basename "${DIR}")" fi +#shellcheck disable=SC2086 if [[ "${CMD}" == "" ]] ; then sudo docker run \ --interactive --tty \ --privileged \ - --name=${NAME} \ + --name="${NAME}" \ + --hostname="${NAME}" \ --network bridge \ --volume=/var/run:/var/run \ - --volume=${HUGE}:${HUGE} \ - --volume=${ONVM}:/openNetVM \ + --volume="${HUGE}":"${HUGE}" \ + --volume="${ONVM}":/openNetVM \ ${DIR} \ - ${DEVICES[@]} \ + "${DEVICES[@]}" \ sdnfv/opennetvm \ /bin/bash else sudo docker run \ --detach=true \ --privileged \ - --name=${NAME} \ + --name="${NAME}" \ + --hostname="${NAME}" \ --network bridge \ --volume=/var/run:/var/run \ - --volume=${HUGE}:${HUGE} \ - --volume=${ONVM}:/openNetVM \ + --volume="${HUGE}":"${HUGE}" \ + --volume="${ONVM}":/openNetVM \ ${DIR} \ - ${DEVICES[@]} \ + "${DEVICES[@]}" \ sdnfv/opennetvm \ /bin/bash -c "${CMD}" fi - diff --git a/scripts/dpdk_helper_scripts.sh b/scripts/dpdk_helper_scripts.sh index f0cc052f8..14123cf43 100755 --- a/scripts/dpdk_helper_scripts.sh +++ b/scripts/dpdk_helper_scripts.sh @@ -1,6 +1,6 @@ #!/bin/bash -HUGEPGSZ=`cat /proc/meminfo | grep Hugepagesize | cut -d : -f 2 | tr -d ' '` +HUGEPGSZ=$(grep Hugepagesize /proc/meminfo | cut -d : -f 2 | tr -d ' ') # # Unloads igb_uio.ko. @@ -75,7 +75,6 @@ set_numa_pages() echo > .echo_tmp for d in /sys/devices/system/node/node? ; do - node=$(basename $d) echo "echo $hp_count > $d/hugepages/hugepages-${HUGEPGSZ}/nr_hugepages" >> .echo_tmp done echo "Reserving hugepages" @@ -84,8 +83,8 @@ set_numa_pages() create_mnt_huge - hp_free=$(cat /proc/meminfo | grep HugePages_Free | awk '{print $2}') - if [ $hp_free == "0" ]; then + hp_free=$(grep HugePages_Free /proc/meminfo | awk '{print $2}') + if [ "$hp_free" == "0" ]; then echo "Coudn't set up huge pages. Did you try turning it off and on again?" exit 1 else diff --git a/scripts/install.sh b/scripts/install.sh index faf27cb8e..701db3960 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -70,7 +70,7 @@ fi sudo -v # Ensure we're working relative to the onvm root directory -if [ $(basename $(pwd)) == "scripts" ]; then +if [ "$(basename "$(pwd)")" == "scripts" ]; then cd .. fi @@ -83,33 +83,35 @@ if [ -z "$ONVM_HOME" ]; then fi # Source DPDK helper functions -. $ONVM_HOME/scripts/dpdk_helper_scripts.sh +. "$ONVM_HOME"/scripts/dpdk_helper_scripts.sh set +e remove_igb_uio_module set -e # Compile dpdk -cd $RTE_SDK +cd "$RTE_SDK" echo "Compiling and installing dpdk in $RTE_SDK" # Adding ldflags.txt output for mTCP compatibility -if grep "ldflags.txt" $RTE_SDK/mk/rte.app.mk > /dev/null +if grep "ldflags.txt" "$RTE_SDK"/mk/rte.app.mk > /dev/null then : else - sed -i -e 's/O_TO_EXE_STR =/\$(shell if [ \! -d \${RTE_SDK}\/\${RTE_TARGET}\/lib ]\; then mkdir -p \${RTE_SDK}\/\${RTE_TARGET}\/lib\; fi)\nLINKER_FLAGS = \$(call linkerprefix,\$(LDLIBS))\n\$(shell echo \${LINKER_FLAGS} \> \${RTE_SDK}\/\${RTE_TARGET}\/lib\/ldflags\.txt)\nO_TO_EXE_STR =/g' $RTE_SDK/mk/rte.app.mk + # want to use single quotes for sed operation + # shellcheck disable=SC2016 + sed -i -e 's/O_TO_EXE_STR =/\$(shell if [ \! -d \${RTE_SDK}\/\${RTE_TARGET}\/lib ]\; then mkdir -p \${RTE_SDK}\/\${RTE_TARGET}\/lib\; fi)\nLINKER_FLAGS = \$(call linkerprefix,\$(LDLIBS))\n\$(shell echo \${LINKER_FLAGS} \> \${RTE_SDK}\/\${RTE_TARGET}\/lib\/ldflags\.txt)\nO_TO_EXE_STR =/g' "$RTE_SDK"/mk/rte.app.mk fi sleep 1 -make config T=$RTE_TARGET -make T=$RTE_TARGET -j 8 -make install T=$RTE_TARGET -j 8 +make config T="$RTE_TARGET" +make T="$RTE_TARGET" -j 8 +make install T="$RTE_TARGET" -j 8 # Refresh sudo sudo -v -cd $start_dir +cd "$start_dir" # Setup/Check for free HugePages if the user wants to if [ -z "$ONVM_SKIP_HUGEPAGES" ]; then @@ -118,7 +120,7 @@ fi grep -m 1 "huge" /etc/fstab | cat # Only add to /etc/fstab if user wants it -if [ ${PIPESTATUS[0]} != 0 ] && [ -z "$ONVM_SKIP_FSTAB" ]; then +if [ "${PIPESTATUS[0]}" != 0 ] && [ -z "$ONVM_SKIP_FSTAB" ]; then echo "Adding huge fs to /etc/fstab" sleep 1 sudo sh -c "echo \"huge /mnt/huge hugetlbfs defaults 0 0\" >> /etc/fstab" diff --git a/scripts/no_hyperthread.sh b/scripts/no_hyperthread.sh index c510ea59d..19b74ad53 100755 --- a/scripts/no_hyperthread.sh +++ b/scripts/no_hyperthread.sh @@ -10,10 +10,10 @@ CPUS_TO_SKIP=" $(cat /sys/devices/system/cpu/cpu*/topology/thread_siblings_list # Disable Hyperthreading for CPU_PATH in /sys/devices/system/cpu/cpu[0-9]*; do - CPU="$(echo $CPU_PATH | tr -cd "0-9")" + CPU="$(echo "$CPU_PATH" | tr -cd "0-9")" echo "$CPUS_TO_SKIP" | grep " $CPU " > /dev/null if [ $? -ne 0 ]; then - echo 0 > $CPU_PATH/online + echo 0 > "$CPU_PATH"/online fi done diff --git a/scripts/setup_environment.sh b/scripts/setup_environment.sh index fadd158b3..784358287 100755 --- a/scripts/setup_environment.sh +++ b/scripts/setup_environment.sh @@ -70,14 +70,16 @@ if [ -z "$ONVM_HOME" ]; then fi # Source DPDK helper functions -. $ONVM_HOME/scripts/dpdk_helper_scripts.sh +. "$ONVM_HOME"/scripts/dpdk_helper_scripts.sh # Disable ASLR sudo sh -c "echo 0 > /proc/sys/kernel/randomize_va_space" # Setup/Check for free HugePages if the user wants to if [ -z "$ONVM_SKIP_HUGEPAGES" ]; then - set_numa_pages $hp_count + # hp_count is assigned in ./dpdk_helper_scripts.sh + # shellcheck disable=SC2154 + set_numa_pages "$hp_count" fi # Verify sudo access @@ -85,10 +87,10 @@ sudo -v # Load uio kernel modules grep -m 1 "igb_uio" /proc/modules | cat -if [ ${PIPESTATUS[0]} != 0 ]; then +if [ "${PIPESTATUS[0]}" != 0 ]; then echo "Loading uio kernel modules" sleep 1 - cd $RTE_SDK/$RTE_TARGET/kmod + cd "$RTE_SDK"/"$RTE_TARGET"/kmod sudo modprobe uio sudo insmod igb_uio.ko else @@ -107,7 +109,7 @@ if [ -z "$ONVM_NIC_PCI" ];then read -r -p "Bind interface $id to DPDK? [y/N] " response if [[ $response =~ ^([yY][eE][sS]|[yY])$ ]];then echo "Binding $id to dpdk" - sudo $DPDK_DEVBIND -b igb_uio $id + sudo "$DPDK_DEVBIND" -b igb_uio "$id" fi done else @@ -115,13 +117,13 @@ else for nic_id in $ONVM_NIC_PCI do echo "Binding $nic_id to DPDK" - sudo $DPDK_DEVBIND -b igb_uio $nic_id + sudo "$DPDK_DEVBIND" -b igb_uio "$nic_id" done fi echo "Finished Binding" $DPDK_DEVBIND --status -sudo bash $ONVM_HOME/scripts/no_hyperthread.sh +sudo bash "$ONVM_HOME"/scripts/no_hyperthread.sh echo "Environment setup complete." diff --git a/tools/Pktgen/openNetVM-Scripts/run-pktgen.sh b/tools/Pktgen/openNetVM-Scripts/run-pktgen.sh index 39e409b88..c4c442d74 100755 --- a/tools/Pktgen/openNetVM-Scripts/run-pktgen.sh +++ b/tools/Pktgen/openNetVM-Scripts/run-pktgen.sh @@ -63,10 +63,10 @@ PORT_NUM=$1 echo "Starting pktgen" # Pktgen has to be started from pktgen-dpdk/ -if [ $PORT_NUM -eq "2" ]; then - (cd $PKTGEN_HOME && sudo $PKTGEN_BUILD -c 0xff -n 3 $BLACK_LIST -- -p 0x3 $PORT_MASK -P -m "[1:2].0, [3:4].1" -f $PKTGEN_CONFIG) -elif [ $PORT_NUM -eq "1" ]; then - (cd $PKTGEN_HOME && sudo $PKTGEN_BUILD -c 0xff -n 3 $BLACK_LIST -- -p 0x1 $PORT_MASK -P -m "[1:2].0" -f $PKTGEN_CONFIG) +if [ "$PORT_NUM" -eq "2" ]; then + (cd "$PKTGEN_HOME" && sudo "$PKTGEN_BUILD" -c 0xff -n 3 "$BLACK_LIST" -- -p 0x3 "$PORT_MASK" -P -m "[1:2].0, [3:4].1" -f "$PKTGEN_CONFIG") +elif [ "$PORT_NUM" -eq "1" ]; then + (cd "$PKTGEN_HOME" && sudo "$PKTGEN_BUILD" -c 0xff -n 3 "$BLACK_LIST" -- -p 0x1 "$PORT_MASK" -P -m "[1:2].0" -f "$PKTGEN_CONFIG") else echo "Helper script only supports 1 or 2 ports" exit 0