Minxi Hou <[email protected]> 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 <[email protected]>
> ---

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 <[email protected]>

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 \

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to