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 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
