Re: [PATCH net v4] selftests/net/openvswitch: guard command substitutions against empty output

[Aaron Conole <aconole@redhat.com>]

Minxi Hou <houminxi@gmail.com> writes:

> When ip-link output is unavailable, when the upcall daemon log has not
> been written yet, or when pahole does not know the OVS drop subsystem
> ID, the affected command substitutions silently produce empty strings.
> The caller then passes empty sha= or pid= arguments to ovs_add_flow,
> or matches against wrong drop reason codes, all without a diagnostic.
>
> Add [ -z ] guards immediately after each assignment. For test_arp_ping,
> also align the MAC extraction to use awk '/link\/ether/' as in
> test_pop_vlan. The drop_reason guard returns ksft_skip because an
> absent subsystem ID is an environment issue, not a test failure.
>
> Signed-off-by: Minxi Hou <houminxi@gmail.com>
> ---

Thanks Minxi - these look good although this is probably better to go to
net-next since it's more like enhancement rather than really a 'fix'.

Reviewed-by: Aaron Conole <aconole@redhat.com>

When you post next version make sure to target it as such.

> Changes since v3:
>   - Corrected subject prefix to [PATCH net] (hardening fix, not new feature).
>
> Changes since v2:
>   - Fixed missing 'net/' in subject path (selftests/net/openvswitch).
>   - Removed --base flag (prerequisite-patch-id flood in v2).
>
> Changes since v1:
>   - Rebased onto current net-next/main. No code changes.
>
>  .../selftests/net/openvswitch/openvswitch.sh     | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index a415e9dec8cd..82f066a0ceed 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -360,6 +360,8 @@ test_psample() {
>  	# sFlow / IPFIX.
>  	nlpid=$(grep -E "listening on upcall packet handler" \
>              $ovs_dir/s0.out | cut -d ":" -f 2 | tr -d ' ')
> +	[ -z "$nlpid" ] && \
> +		{ info "failed to get upcall PID"; return 1; }
>  
>  	ovs_add_flow "test_psample" psample \
>              "in_port(2),eth(),eth_type(0x0800),ipv4()" \
> @@ -393,6 +395,10 @@ test_drop_reason() {
>  	ovs_drop_subsys=$(pahole -C skb_drop_reason_subsys |
>  			      awk '/OPENVSWITCH/ { print $3; }' |
>  			      tr -d ,)
> +	if [ -z "$ovs_drop_subsys" ]; then
> +		info "failed to get OVS drop subsys ID"
> +		return $ksft_skip
> +	fi
>  
>  	sbx_add "test_drop_reason" || return $?
>  
> @@ -491,13 +497,19 @@ test_arp_ping () {
>  	# Setup client namespace
>  	ip netns exec client ip addr add 172.31.110.10/24 dev c1
>  	ip netns exec client ip link set c1 up
> -	HW_CLIENT=`ip netns exec client ip link show dev c1 | grep -E 'link/ether [0-9a-f:]+' | awk '{print $2;}'`
> +	HW_CLIENT=$(ip netns exec client ip link show dev c1 \
> +		| awk '/link\/ether/ {print $2}')
> +	[ -z "$HW_CLIENT" ] && \
> +		{ info "failed to get client hwaddr"; return 1; }
>  	info "Client hwaddr: $HW_CLIENT"
>  
>  	# Setup server namespace
>  	ip netns exec server ip addr add 172.31.110.20/24 dev s1
>  	ip netns exec server ip link set s1 up
> -	HW_SERVER=`ip netns exec server ip link show dev s1 | grep -E 'link/ether [0-9a-f:]+' | awk '{print $2;}'`
> +	HW_SERVER=$(ip netns exec server ip link show dev s1 \
> +		| awk '/link\/ether/ {print $2}')
> +	[ -z "$HW_SERVER" ] && \
> +		{ info "failed to get server hwaddr"; return 1; }
>  	info "Server hwaddr: $HW_SERVER"
>  
>  	ovs_add_flow "test_arp_ping" arpping \