mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: netdev-bot+sashiko@kernel.org
To: markdamirkan@gmail.com
Cc: dsahern@kernel.org, idosch@nvidia.com, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, alex.aring@gmail.com, shuah@kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net 2/2] selftests: net: add RPL SRH loop test
Date: Tue, 15 Sep 2026 23:43:15 +0000	[thread overview]
Message-ID: <178951579554.22033.5001333303876038052@kernel.org> (raw)
In-Reply-To: <20260911-sympwn-rpl-send-v1-2-1753993bb584@gmail.com>

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

      parent reply	other threads:[~2026-09-15 23:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 23:11 [PATCH net 0/2] ipv6: rpl: fix loop detection for separated local addresses Mark Amirkan via B4 Relay
2026-09-11 23:11 ` [PATCH net 1/2] " Mark Amirkan via B4 Relay
2026-09-15 15:27   ` Ido Schimmel
2026-09-11 23:11 ` [PATCH net 2/2] selftests: net: add RPL SRH loop test Mark Amirkan via B4 Relay
2026-09-15 15:27   ` Ido Schimmel
2026-09-15 23:43   ` netdev-bot+sashiko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=178951579554.22033.5001333303876038052@kernel.org \
    --to=netdev-bot+sashiko@kernel.org \
    --cc=alex.aring@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=markdamirkan@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®