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

[Florian Westphal <fw@strlen.de>]

netdev-bot+sashiko@kernel.org <netdev-bot+sashiko@kernel.org> 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 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?

Not really.  Was only ever caught by netdev CI which already
carries this fix locally.

> The flake looks like it started with:
> 
> commit 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

Yes.

> Would something like this be worth adding?
> 
> Fixes: 8a2cfe7951f6 ("selftests: nft_queue.sh: add a bridge queue test")

I'm not sure its worth doing.