Re: [PATCH net-next v5 3/3] selftests: net: add a test for the gtp netlink family

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The test does not skip when the kernel lacks GTP support,
  although the commit message says it will.

--

selftests: net: add a test for the gtp netlink family

This adds a Python selftest that uses the gtp ynl spec to exercise the gtp
generic netlink family. It creates a gtp device, then adds, dumps and
deletes PDP contexts by TEI and by MS address. It also adds a GtpFamily
ynl wrapper and registers the test.

> diff --git a/tools/testing/selftests/net/gtp.py b/tools/testing/selftests/net/gtp.py
> new file mode 100755
> index 0000000000000..199bc44091909
> --- /dev/null
> +++ b/tools/testing/selftests/net/gtp.py

[ ... ]

> +_NO_GTP_ERRORS = ("Operation not supported", "Garbage instead of arguments")
> +
> +
> +def _setup(ns):

[ ... ]

> +    try:
> +        ip("link add gtp0 type gtp role ggsn", ns=str(ns))
> +    except CmdExitFailure as e:
> +        # gtp.ko not available, or iproute2 without gtp support: any other
> +        # failure to create the device is a real error.
> +        if not any(msg in e.cmd.stderr for msg in _NO_GTP_ERRORS):
> +            raise
> +        raise KsftSkipEx("no gtp support (CONFIG_GTP, iproute2)") from e

[Severity: Medium]
Will this skip when the kernel has no GTP support? The commit message says
"The test is skipped when the kernel or iproute2 lack GTP support".

With CONFIG_GTP=n, or with gtp.ko not installed, request_module() for
rtnl-link-gtp finds nothing and ops stays NULL. The request then reaches
this branch in __rtnl_newlink() in net/core/rtnetlink.c:

	if (!ops) {
		NL_SET_ERR_MSG(extack, "Unknown device type");
		return -EOPNOTSUPP;
	}

iproute2 requests extended ACKs. When an extack message is present it
prints only "Error: Unknown device type." and returns early, so it never
prints the "RTNETLINK answers: Operation not supported" line.

Neither string in _NO_GTP_ERRORS matches that output. _setup() then
re-raises CmdExitFailure, and add_del_pdp, add_two_pdp and del_unknown_pdp
each report a failure instead of a skip.

The iproute2-without-gtp case ("Garbage instead of arguments") looks like
it is handled correctly. The "Operation not supported" string seems to
match only iproute2 builds without extack support.

Could "Unknown device type" be added to _NO_GTP_ERRORS? Another option is
to check whether the gtp link kind is available before creating the
device.

> +    defer(ip, "link del gtp0", ns=str(ns))
> +    ip("link set gtp0 up", ns=str(ns))

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012024800.400616-1-anilkaushikwireless%40gmail.com