Re: git: b60600ceeb68 - main - pf tests: Serialize

From: Mark Johnston <markj_at_freebsd.org>
Date: Fri, 31 Mar 2023 00:26:30 UTC
On Fri, Mar 31, 2023 at 09:15:27AM +0900, Kristof Provost wrote:
> On 31 Mar 2023, at 9:05, Mark Johnston wrote:
> > On Fri, Mar 31, 2023 at 08:56:56AM +0900, Kristof Provost wrote:
> >> On 31 Mar 2023, at 8:36, Mark Johnston wrote:
> >>> The branch main has been updated by markj:
> >>>
> >>> URL: https://cgit.FreeBSD.org/src/commit/?id=b60600ceeb68d1001d61222830e0be3441ef0885
> >>>
> >>> commit b60600ceeb68d1001d61222830e0be3441ef0885
> >>> Author:     Mark Johnston <markj@FreeBSD.org>
> >>> AuthorDate: 2023-03-25 12:55:41 +0000
> >>> Commit:     Mark Johnston <markj@FreeBSD.org>
> >>> CommitDate: 2023-03-30 23:35:59 +0000
> >>>
> >>>     pf tests: Serialize
> >>>
> >>>     These tests reuse jail names and cannot run in parallel.  Until this is
> >>>     fixed - which is desirable since these takes take a while to run - tell
> >>>     kyua to serialize them.
> >>>
> >>
> >> The tests also recycle IP ranges, so merely changing the jail names is insufficient.
> >
> > Could you please give an example?  I looked at some of the tests but can
> > only see cases where addresses are assigned within the vnet jail(s)
> > created by the tests, in which case I'd expect no problems.
> >
> Altq:hfsq assigns 192.0.2.1/24 on an epair on the host, so does altq:match, altq:cbq_vlan, altq:codel_bridge, dup:dup_to, ether:mac, ether:proto and .. then I got bored looking.
> 
> It’s not just the pf tests either. For example the netinet/forward:fwd_ip_icmp_iface_fast_success test does that too (well, 192.0.2.1/29, but anyway). There are going to be more, this is just from a very quick look.

I see now, thanks.

> >> Realistically the easiest way to get these to run in parallel would be to run each test in its own vnet so both overlapping IP ranges and name conflicts don’t matter.
> >
> > Yeah, I was wondering whether it'd be possible to have kyua handle the
> > creation and teardown of a per-test jail, if only to avoid having to go
> > through all of the tests which use static jail names.
> 
> That’s what I was thinking as well, yes.
> 
> We might be able to get to a point where all the tests have unique jail names, but we’re never going to manage to de-conflict all of the IP assignments. Even if we could, it’d make writing tests harder and that’s counterproductive.

As it happens I'm looking at netinet6/forward6 tests, which have exactly
the same problem.  Each of those tests creates an epair, puts one end in
a vnet jail, and configures the other on the host.  It was pretty easy
to simply create a second jail for the other end of the epair, and to my
eye doesn't make the test much more complicated.  Some more commands
have to be prefixed with jexec is all.

I'm sure that converting the pf tests would be much more tedious, if not
more complicated.  There is some downside to having kyua manage even
more context, since that could make tests harder to debug, and kyua
already makes that somewhat unpleasant.

> The only issue I can think of with having the tests run in their own vnet is that there may be some tests that play with non-vneted sysctls, so we will have to make that vnet feature optional.

Yeah, I think this would have to be opt-in.