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

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

[Minxi Hou]

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>
---

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 \
-- 
2.54.0

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

[Paolo Abeni]

On 6/4/26 6:30 PM, Minxi Hou wrote:
> 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>

Whoops, I unintentionally replied to an old revision; same point, I
think this is net-next material.

@ovs crew: WDYT?

/P

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

[Eelco Chaudron]

On 9 Jun 2026, at 12:58, Paolo Abeni wrote:

> On 6/4/26 6:30 PM, Minxi Hou wrote:
>> 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>
>
> Whoops, I unintentionally replied to an old revision; same point, I
> think this is net-next material.
>
> @ovs crew: WDYT?

+1 on net-next.

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

[Aaron Conole]

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 \

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Fri,  5 Jun 2026 00:30:16 +0800 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [net,v4] selftests/net/openvswitch: guard command substitutions against empty output
    https://git.kernel.org/netdev/net-next/c/9415471e01c1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html