[I was going once more over your comments before sending the new patch, realised I didn't answer...] On Tue, 26 May 2020 15:52:49 +0200 Phil Sutter <phil@xxxxxx> wrote: > Hi, > > On Tue, May 26, 2020 at 01:12:36AM +0200, Stefano Brivio wrote: > > On Mon, 25 May 2020 17:59:52 +0200 > > Phil Sutter <phil@xxxxxx> wrote: > > > On Sun, May 24, 2020 at 02:59:57PM +0200, Stefano Brivio wrote: > > > > It might be convenient to run tests from a development branch that > > > > resides on another host, and if we break connectivity on the test > > > > host as tests are executed, we con't run them this way. > > > > > > > > To preserve connectivity, for shell tests, we can simply use the > > > > 'forward' hook instead of 'input' in chains/0036_policy_variable_0 > > > > and transactions/0011_chain_0, without affecting test coverage. > > > > > > > > For py tests, this is more complicated as some test cases install > > > > chains for all the available hooks, and we would probably need a > > > > more refined approach to avoid dropping relevant traffic, so I'm > > > > not covering that right now. > > > > > > This is a recurring issue, iptables testsuites suffer from this problem > > > as well. There it was solved by running everything in a dedicated netns: > > > > > > iptables/tests/shell: Call testscripts via 'unshare -n <file>'. > > > iptables-test.py: If called with --netns, 'ip netns exec <foo>' is > > > added as prefix to any of the iptables commands. > > > > Funny, I thought about doing that in the past and stopped before I could > > even type 'unshare'. Silly, everything will break, I thought. > > > > Nope, not at all, now that you say that... both 'unshare -n > > ./nft-test.py' and 'unshare -n ./run-tests.sh' worked flawlessly. > > > > A minor concern I have is that if CONFIG_NETNS is not set we can't do > > that. But we could add a check and run them in the init namespace if > > namespaces are not available, that looks reasonable enough. > > Sounds like over-engineering, although your point is valid, of course. > In iptables shell testsuite was changed to always call unshare back in > 2018, I don't think anyone has complained yet. So either everyone has > netns support already (likely, given the container inflation) or nobody > apart from hardliners actually run those tests (even more likely). :) On embedded systems, in my experience, even in the post-modern era, it's quite common to run kernels without support for networking namespaces, or a busybox build without the unshare(1) applet (which was implemented not so long ago, in 2016). And one might want to run tests there because of some specific peculiarities (say, endianness, word size, or just debugging). Perhaps more commonly, Python bindings for unshare() might not be installed. The check is quite straigthforward, so I would rather add it. > > > I considered doing the same in nftables testsuites several times but > > > never managed to keep me motivated enough. Maybe you want to give it a > > > try? > > > > I would do that from the main script itself (and figure out how it > > should be done in Python, too), just once, it looks cleaner and we > > don't change how test scripts are invoked. Something like this: > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/testing/selftests/netfilter/nft_concat_range.sh#n1463 > > Maybe support a hidden parameter and do a self-call wrapped by 'unshare' > unless that parameter was passed? Yes, it's what the example I quoted does. Well, I hope you meant the same thing. :) -- Stefano
