On 14/10/2026 22:56, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
>
> Low priority / nit picks for your consideration:
>
> - [Low] The commit message does not mention two new runtime requirements
> added by this patch.
(...)
>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> index e850a87429b61..ef2738720e249 100755
>> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
>
> [ ... ]
>
>> @@ -105,32 +109,31 @@ cleanup()
>>
>> mptcp_lib_check_mptcp
>> mptcp_lib_check_kallsyms
>> -mptcp_lib_check_tools ip "${iptables}" "${ip6tables}"
>> +mptcp_lib_check_tools ip nft jq
>
> [Severity: Low]
> This isn't a bug, but should the commit message list the new runtime
> requirements? The script now needs jq and an nft built with JSON support
> (nft -j list ruleset). The shared mptcp_lib_check_tools() helper in
> mptcp_lib.sh also gets new "nft" and "jq" cases.
>
> The commit message only says:
>
> After conversion, the generated output matches the original
> iptables/ip6tables behavior.
>
> The reason for using JSON and jq appears only in the v2 notes below the
> --- line, and those notes are dropped when the patch is applied.
>
> This check runs at top level, before any subtest. So a system that has
> iptables but no jq, or an nft without JSON support, now skips the whole
> script. That includes do_mptcp_sockopt_tests and do_tcpinq_tests, which
> don't use netfilter.
>
> Could the commit message mention the jq and nft JSON dependencies and the
> mptcp_lib.sh change?
nft is implicit. 'jq' is used in many other selftests. No need to
re-spin (but if we do, yes we can mention it just to please Clashiko).
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
