netinet & netpfil tests failing
Date: Tue, 18 Jan 2022 02:07:43 UTC
Hi, just to remind that I'm responsible for multiple tests failing and to refresh the context, kind of explaining why the hell they aren't fixed yet?! The long old discussion can be found in this thread in December, link to last message: https://lists.freebsd.org/archives/dev-commits-src-main/2021-December/002581.html Summarized refreshed context follows. The reason for failing tests is complex. There a constellation of factors [bugs] that attribute to it: * Jails are reference counted and jail destroy may be delayed. Test suite usually didn't trigger delayed jail destroy and expectation of many tests is that immediately after 'jail -r' all resources are released, especially network interfaces in a jail are if_vmove()'d to vnet0. * My original change to inpcb database protection ignored the fact that inp->inp_cred->cr_prison is dereferenced and read during a fast pcb lookup. The prison doesn't have neither network epoch nor SMR protection. That was a bug and to fix it me & Mark decided that an elegant idea would be to delay crfree() when a pcb is destroyed from immediate call to SMR-delayed destructor. This fixed the race, but created another bug. Since every vnet had its own pcb zone, a dying jail won't ever free its resources, it will stay forever. This was mitigated by making the pcb zone global. Now pcbs are correctly recycled, but there is no guarantee that upon return from 'jail -r' the jail is already fully cleared. * Back to tests. As tests expect 'jail -r' to immediately free resources. Right after 'jail -r' tests do 'ifconfig ${ifname} destroy', where ifname is the interface that was just popped up back to vnet0 from the destroyed jail. Now this 'ifconfig destroy' fails, but test suite ignores this error. A test succeeds. However, some time later, usually after other tests, the jail is indeed destroyed and surprise interfaces out of nowhere pop up at vnet0. Of course this is definite memory&resource leak, but not the reason why tests are failing. * Another factor - scapy. The python scapy library would emit warning to stderr if it sees interface without any IP address. This happens right at 'import scapy'. The test suite considers a test failed if it has something on stderr, even if it returned success. So, result is that some test (absolutely unrelated to pcbs) leaves a jail with interfaces, then jail is released, interfaced pop up at vnet0, and then some other test (absolutely unrelated to pcbs) using scapy writes a warning to stderr and triggers failure. My & Mark are now seeing three approaches to the problem: * Reclaim the memory from pcb zone(s), when jail is destroyed, returning back the old behaviour that with test suites 'jail -r' is always synchronous. Some prerequisites for this approach are here: https://reviews.freebsd.org/D33868 * Protect jails with epoch, bypass the cred pointer in inpcb and in the lookup check inp->inp_prison->pr_foo. After that the crfree() can be moved back to the immediate inpcb free procedure. Mark has a quick & dirty proof of concept for this approach. * In the test suite destroy the interface from the jail: 'jexec jname ifconfig ${ifname} destroy'. I'd like to add a few words on the last option. To me it seems most elegant as we are improving the test suite instead of changing kernel to meet demands of the suite. However, it doesn't work :( Why? Why does 'jexec jname ifconfig epair0b destroy' or 'jexec jname ifconfig lo1 destroy' returns ENXIO? Because the interface was created within vnet0 and is linked on vnet0 cloner's list. To repeat: epair0b ifnet is linked to the jail's list of network interfaces, but it linked on vnet0 list of epair(4) ifcloner. Likewise, some lo4 interface would also be in the jail list of interfaces, but on vnet0 if_loop cloner. This makes it impossible to destroy such interface from inside the jail. Neither it is possible to destroy it from the outside, for obvious reasons. There are more side effects about this. For example the only reason why we can't create an interface with the same name inside a jail using its cloner list is call to ifunit() in the beginning of if_clone_createif(). This definitely is a part of design, since if_clone_create()/if_clone_destroy() would lookup vnet0 cloner list in case if interface is not found on the current vnet list. To put it short, it is yet another problem created by if_vmove :( Not an easy one to fix and makes the third approach to the problem complicated. To sum up: I'm sorry for tests broken, I'm working on it, it isn't easy problem. Suggestions and help are welcome. -- Gleb Smirnoff