Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0
- [Medium] In tools/testing/selftests/net/rpl_srh_loop.sh the two `ip -6
address add` commands (lines 16-17) that install the local addresses…
--
selftests: net: add RPL SRH loop test
This adds a new shell test that installs two local addresses in a network
namespace and then adds RPL encap routes to check that adjacent local
segments are accepted while a non-local segment between two local ones is
rejected. The script is registered in TEST_PROGS in the net selftests
Makefile.
> diff --git a/tools/testing/selftests/net/rpl_srh_loop.sh
> b/tools/testing/selftests/net/rpl_srh_loop.sh
> new file mode 100755
> index 0000000000000..a2e9991d789d6
> --- /dev/null
> +++ b/tools/testing/selftests/net/rpl_srh_loop.sh
> @@ -0,0 +1,38 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +source lib.sh
> +
> +cleanup()
> +{
> + cleanup_ns "$NS"
> +}
> +
> +trap cleanup EXIT
> +
> +require_command ip
> +setup_ns NS || exit $?
> +
> +ip -n "$NS" -6 address add 2001:db8:1::1/128 dev lo nodad
> +ip -n "$NS" -6 address add 2001:db8:3::1/128 dev lo nodad
[Severity: Medium]
Should these two address adds be checked? The script has no set -e, no
|| exit and no check_err on them, and everything the test asserts depends
on both addresses being present.
If either add fails, does the rest of the script still notice? The
support probe below uses a single segment:
ip -n "$NS" -6 route add 2001:db8:10::/64 \
encap rpl segs 2001:db8:4::1 dev lo 2>/dev/null
and in ipv6_chk_rpl_srh_loop() (net/ipv6/addrconf.c) a single iteration
can never reach the failure branch, because found starts at 0:
if (hash_found) {
if (found && separated) {
ret = 1;
break;
}
separated = false;
found++;
} else {
separated = true;
}
So the probe returns 0 whether or not any local address was installed.
> +
> +if ! ip -n "$NS" -6 route add 2001:db8:10::/64 \
> + encap rpl segs 2001:db8:4::1 dev lo 2>/dev/null; then
> + echo "SKIP: RPL lightweight tunnel support not available"
> + exit $ksft_skip
> +fi
> +
> +RET=0
> +ip -n "$NS" -6 route add 2001:db8:11::/64 \
> + encap rpl segs 2001:db8:1::1,2001:db8:3::1 dev lo
> +check_err $? "Adjacent local addresses were rejected"
> +log_test "RPL accepts adjacent local addresses"
[Severity: Medium]
Can this subtest pass without exercising the path it describes? If
2001:db8:1::1 and 2001:db8:3::1 were never installed, both iterations of
ipv6_chk_rpl_srh_loop() take the else branch (separated = true), ret stays
0, the route add succeeds and check_err 0 reports OK.
The same OK result is produced in both the "addresses present" and
"addresses absent" cases, so is there a way for this subtest to
distinguish them?
> +
> +RET=0
> +ip -n "$NS" -6 route add 2001:db8:12::/64 \
> + encap rpl segs 2001:db8:1::1,2001:db8:2::1,2001:db8:3::1 dev lo \
> + 2>/dev/null
> +check_fail $? "Separated local addresses were accepted"
> +log_test "RPL rejects separated local addresses"
[Severity: Medium]
Does this turn a setup problem into a reported kernel failure? With no
local addresses installed, none of the three segments hash-match, ret
stays 0 in ipv6_chk_rpl_srh_loop(), the route add succeeds, and then:
check_fail 0
-> check_err $((!0))
-> ret_set_ksft_status $ksft_fail "Separated local addresses were
accepted"
log_test folds that into EXIT_STATUS and the script exits 1, so a
namespace where the address adds did not take effect is reported as a
failing kernel test rather than a skip or a setup error.
Since stderr of this route add is discarded with 2>/dev/null and the
address adds are unchecked, is there anything printed that would point at
the real cause? A partial failure, where only one of the two addresses is
installed, gives the same inverted result.
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260911-sympwn-rpl-send-v1-0-1753993bb584%40gmail.com
