netinet & netpfil tests failing

From: Gleb Smirnoff <glebius_at_freebsd.org>
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