Hi,
On Wed, Dec 01, 2021 at 12:12:00PM +0100, Štěpán Němec wrote:
> Wider testing has shown that 128 kB stack is too low (e.g. for systems
> with 64 kB page size), leading to false failures in some environments.
>
> Based on results from a matrix of RHEL 8 and RHEL 9 systems across
> x86_64, aarch64, ppc64le and s390x architectures as well as some
> anecdotal testing of other Linux distros on x86_64 machines, 400 kB
> seems safe: the normal nft stack (which should stay constant during
> this test) on all tested systems doesn't exceed 200 kB (stays around
> 100 kB on typical systems with 4 kB page size), while always growing
> beyond 500 kB in the failing case (nftables before baecd1cf2685) with
> the increased set size.
>
> Fixes: d8ccad2a2b73 ("tests: cover baecd1cf2685 ("segtree: Fix segfault when restoring a huge interval set")")
> Signed-off-by: Štěpán Němec <snemec@xxxxxxxxxx>
> ---
> I haven't been able to find an answer to the question of how much
> stack size can vary across different systems (particularly those
> nftables is likely to run on), so more testing might be useful,
> especially on systems not listed above.
Given that both the old and your new version of the test case reliably
trigger on x86_64, I guess it's good enough - I suppose almost all
testsuite runs happen on that architecture and if the test detects a
regression there, it's sufficient for upstream.
In theory, downstream is fine with the test producing false-negatives in
all but one (mandatory) tested arch, but if your patch improves that I
don't see any downsides to it.
On Wed, Dec 01, 2021 at 12:26:14PM +0100, Florian Westphal wrote:
> We could try to get rid of large on-stack allocations and always malloc
> instead.
I quickly played around with reduced stack size and listing a large
ruleset. Really just a quick test, but I got a segfault in
parser_bison.c. I bet this will be the limiting factor in all attempts
at reducing stack size, bugs aside. :)
Cheers, Phil
