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.
--
selftests: mptcp: convert iptables to nftables for mptcp_sockopt.sh
This patch moves the mark rules in mptcp_sockopt.sh from iptables/ip6tables
to nftables and keeps the same table and chain names. check_mark() now reads
the drop counter from nft JSON output and parses it with jq.
mptcp_lib_check_tools() gets new checks for nft and jq.
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index b9d14647f4019..20853d0198ffc 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -528,7 +528,14 @@ mptcp_lib_check_tools() {
> exit ${KSFT_SKIP}
> fi
> ;;
> - "iptables"* | "ip6tables"*)
> + "nft")
> + if ! nft --version &> /dev/null || \
> + ! nft -j list ruleset &> /dev/null; then
> + mptcp_lib_pr_skip "Could not run all tests
> without nft -j"
> + exit ${KSFT_SKIP}
> + fi
> + ;;
> + "iptables"* | "ip6tables"* | "jq")
> if ! "${tool}" -V &> /dev/null; then
> mptcp_lib_pr_skip "Could not run all tests
> without ${tool}"
> exit ${KSFT_SKIP}
> 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?
[ ... ]
--
Sashiko AI review ยท
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013-net-next-mptcp-misc-feat-7-4-part2-v1-0-1181e5d2fda4%40kernel.org
