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 patch removes `select MCTP_FLOWS` from `config MCTP_TEST`
in net/mctp/Kconfig.
--
net: mctp: Stop selecting flow support for KUnit tests
This commit removes "select MCTP_FLOWS" from MCTP_TEST in net/mctp/Kconfig.
The selection also enabled SKB_EXTENSIONS, and KUnit style guidance says
tests should not pull in dependencies this way. The two flow-specific tests
already skip when MCTP_FLOWS is disabled.
> diff --git a/net/mctp/Kconfig b/net/mctp/Kconfig
> index d8d3413a37f7e..3a5c0e70da776 100644
> --- a/net/mctp/Kconfig
> +++ b/net/mctp/Kconfig
> @@ -14,7 +14,6 @@ menuconfig MCTP
>
> config MCTP_TEST
> bool "MCTP core tests" if !KUNIT_ALL_TESTS
> - select MCTP_FLOWS
[Severity: Medium]
Will this break kunit.py run --alltests?
tools/testing/kunit/configs/all_tests.config
still asks for flow support:
tools/testing/kunit/configs/all_tests.config:
CONFIG_NET=y
CONFIG_MCTP=y
CONFIG_MCTP_FLOWS=y
Commit 109a5331143d, the one named in the Fixes: tag, added this line
together with the select.
MCTP_FLOWS is a bool with no prompt and no default:
net/mctp/Kconfig:
config MCTP_FLOWS
bool
depends on MCTP
select SKB_EXTENSIONS
That means a config fragment cannot turn it on. Only a select can.
After this patch the only selector left is MCTP_TRANSPORT_I2C in
drivers/net/mctp/Kconfig. That symbol depends on I2C_SLAVE, and neither
one is on by default or set in all_tests.config. So after olddefconfig,
MCTP_FLOWS stays unset in the generated .config.
validate_config() in tools/testing/kunit/kunit_kernel.py then calls
Kconfig.is_subset_of() in kunit_config.py:
b = other._entries.get(name)
if b is None:
if value == 'n':
continue
return False
This returns False for CONFIG_MCTP_FLOWS=y. The run then stops at config
time with "Not all Kconfig options selected in kunitconfig were in the
generated .config", before any KUnit suite is built. That affects every
suite in the all-tests run, not only the MCTP ones.
Should all_tests.config be updated in this patch as well? One option is to
drop CONFIG_MCTP_FLOWS=y. In that case mctp_test_packet_flow() and
mctp_test_fragment_flow() in net/mctp/test/route-test.c would always skip
in the all-tests configuration.
Another option is to replace the line with CONFIG_I2C_SLAVE=y and
CONFIG_MCTP_TRANSPORT_I2C=y. The flow tests would then still run. That
matches the validation notes, which say the enabled configuration used the
I2C transport to select MCTP_FLOWS.
Since the patch carries a Fixes: tag, a stable backport would bring this
mismatch along with it.
> depends on MCTP=y && KUNIT=y
> default KUNIT_ALL_TESTS
>
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925192510.93840-1-kmehltretter%40gmail.com
