Re: [PATCH net-next 02/15] selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

[netdev-bot+sashiko@kernel.org]

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 fixes a reproducible flake in the bridge NFQUEUE
  selftest (about one failure a day in NIPA), but its message has no…

--

selftests: netfilter: nft_queue.sh: only queue icmp echo request/reply

The patch changes the ipv4 nfq rule in load_ruleset() so it only queues
icmp echo request/reply, which the icmpv6 rule already does. ICMP time
exceeded messages left over after the stress test no longer inflate the
bridge queue packet counts.

> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> Reviewed-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

[Severity: Low]
This isn't a bug, but should this have a Fixes: tag?

The flake looks like it started with:

commit 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

That commit added test_queue_bridge() and called it at the end of the
script, after test_queue_stress():

    test_queue_stress
    ...
    # turns router into a bridge
    test_queue_bridge

It also loaded the existing "ip protocol icmp queue bypass" rule for the
bridge family, and then checked exact counts:

    load_ruleset "bridge" "filter" 10
    test_queue 10 "bridge"

Before that commit, this rule was only loaded for the inet tests. Those
run before the stress test, so the delayed reassembly timeout ICMP errors
were never counted.

8a2cfe7951f6 says it was added ahead of a kernel fix that checks bridge
ports are still valid on reinject. That fix could be picked up for
stable. If the test goes to stable trees without this follow-up, the
bridge test would flake there as well.

Would something like this be worth adding?

Fixes: 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261018005638.1776148-2-pablo%40netfilter.org