Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The tests hardcode relative paths to in-tree pyynl scripts and YAML specs, which breaks when tests are run from an installed directory. - [Medium] The map lookup script uses the GNU awk extension strtonum(), causing the test to unconditionally fail on systems using non-GNU awk (like mawk). --
--- Patch [12]: [RFC PATCH net-next 12/13] selftests: drivers/net: add knod tests --- commit 8c1df38ed71ba173e2fb3bc36bc8224d06cad0df Author: Taehee Yoo <[email protected]> selftests: drivers/net: add knod tests Add attach/detach and XDP offload selftests for knod, exercising the genetlink control plane and the BPF data path over a knod-capable NIC. > diff --git a/tools/testing/selftests/drivers/net/knod/lib.sh > b/tools/testing/selftests/drivers/net/knod/lib.sh > new file mode 100755 > index 0000000000000..d0d9b03f109dc > --- /dev/null > +++ b/tools/testing/selftests/drivers/net/knod/lib.sh > @@ -0,0 +1,181 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# > +# lib.sh - KNOD XDP offload test utilities > +# > +# The KNOD control plane is the "knod" generic-netlink family; it is driven > +# here through the in-tree ynl CLI (tools/net/ynl/pyynl/cli.py) so the tests > +# need no dedicated user-space tool. > + > +KSRC=$(cd "$(dirname "${BASH_SOURCE[0]}")/../../../../../.." && pwd) > +readonly KNOD_YNL="$KSRC/tools/net/ynl/pyynl/cli.py" > +readonly KNOD_SPEC="$KSRC/Documentation/netlink/specs/knod.yaml" [Severity: Medium] Will this hardcoded relative path cause the tests to fail when run from an installed directory? When tests are installed (e.g., via make install) and run out-of-tree, these paths to cli.py and the YAML specs will likely be invalid since they are not copied to the install directory by default. [ ... ] > +knod_map_lookup_u64() { > + local map_id=$1 > + local key=$2 > + local hex > + > + hex=$(bpftool map lookup id "$map_id" \ > + key $key 0 0 0 2>/dev/null | \ > + grep -o 'value:.*' | sed 's/value: //') > + if [ -z "$hex" ]; then > + echo 0 > + return > + fi > + > + printf '%d' "$(echo "$hex" | awk '{ > + v = 0; > + for (i = 8; i >= 1; i--) > + v = v * 256 + strtonum("0x" $i); > + printf "0x%x", v; > + }')" [Severity: Medium] Does the use of the strtonum() function break this script on systems using non-GNU awk? The strtonum() function is a gawk-specific extension. On systems where the default awk is mawk or BusyBox awk, this may fail with an undefined function error, causing the extracted map value to evaluate to 0 and the test to falsely fail. > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
