Hi,

Yeah, Jakub pointed this out too ... 

It's because the test explicitly tries to figure out the right _number_
that openvswitch will use in the drop reasons - had I not slept on this
infrastructure for two years, the test would probably have been written
a lot simpler to start with ;-)

> > https://netdev-ctrl.bots.linux.dev/logview.html?f=%2Flogs%2Fvmksft%2Fnet%2Fresults%2F833743%2F9-openvswitch-sh%2Fstdout#L168
> > 
> > Maybe the test needs to be adapted to get the same info differently?
> > (and adding CONFIG_TRACEPOINTS to the selftest config file)
> The parsing in the test will definitely need to be updated, i.e.,
> the numbers swapped with the names of the drop reasons.

Indeed. Something like this (untested right now, didn't manage to spin
up a test yet):

diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh 
b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index a31f7fb6882d..9b8edfcd2d1a 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -234,7 +234,7 @@ ovs_drop_reason_count()
        local reason=$1
 
        local perf_output=`perf script -i ${ovs_dir}/perf.data -F 
trace:event,trace`
-       local pattern="skb:kfree_skb:.*reason: $reason"
+       local pattern="skb:kfree_skb:.*reason: $reason$"
 
        return `echo "$perf_output" | grep "$pattern" | wc -l`
 }
@@ -790,15 +790,6 @@ test_psample() {
 # - drop packets and verify the right drop reason is reported
 test_drop_reason() {
        which perf >/dev/null 2>&1 || return $ksft_skip
-       which pahole >/dev/null 2>&1 || return $ksft_skip
-
-       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 $?
 
@@ -842,7 +833,7 @@ test_drop_reason() {
                
"in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,proto=1),icmp()" 
'drop'
 
        ovs_drop_record_and_run "test_drop_reason" ip netns exec client ping -c 
2 172.31.110.20
-       ovs_drop_reason_count 0x${ovs_drop_subsys}0001 # OVS_DROP_FLOW_ACTION
+       ovs_drop_reason_count OVS_DROP_LAST_ACTION
        if [[ "$?" -ne "2" ]]; then
                info "Did not detect expected drops: $?"
                return 1
@@ -859,7 +850,7 @@ test_drop_reason() {
 
        ovs_drop_record_and_run \
             "test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 
6000
-       ovs_drop_reason_count 0x${ovs_drop_subsys}0004 # 
OVS_DROP_EXPLICIT_ACTION_ERROR
+       ovs_drop_reason_count OVS_DROP_EXPLICIT_WITH_ERROR
        if [[ "$?" -ne "1" ]]; then
                info "Did not detect expected explicit error drops: $?"
                return 1
@@ -867,7 +858,7 @@ test_drop_reason() {
 
        ovs_drop_record_and_run \
             "test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 
7000
-       ovs_drop_reason_count 0x${ovs_drop_subsys}0003 # 
OVS_DROP_EXPLICIT_ACTION
+       ovs_drop_reason_count OVS_DROP_EXPLICIT
        if [[ "$?" -ne "1" ]]; then
                info "Did not detect expected explicit drops: $?"
                return 1


> IIUC, this change only affects the printing and doesn't affect debugging
> tools like retis that attempt to surface the drop reasons.  But, maybe
> Adrian and Antoine (CCed) may want to have a glance as well.

Not sure how those work? But unless it's interacting with the text
output of trace-cmd report or perf like here, it probably won't care?

johannes

Reply via email to