Okay, I think I've got this sorted now. The patch now leaves pf.c untouched and instead:

In D56559#1295233, @glebius wrote:

In D56559#1294810, @rcm wrote:

So I think the question is really: are we okay with pf_route
forwarding broadcasts when ip_forward (nominally) doesn't? If yes, then I do agree at least a few lines in the man page be introduced that calls out this difference, and maybe prescribes example rules that can be used to plug any undesirable leaks.

I think we are okay. Packet filters are really designed to make network stacks to malfunction.

A good documentation change would be better than a policy.

In D56559#1295079, @kp wrote:

I do also see test failures in sys/netpfil/pf/pfsync:rt_af, sys/netpfil/pf/src_track:sn_types_compat and sys/netpfil/pf/src_track:sn_types_pass with this patch. I've not debugged these, but this change does appear to be responsible.

In D56559#1294669, @eugen_grosbein.net wrote:

In D56559#1294664, @rcm wrote:

In D56559#1294663, @glebius wrote:

I always assumed "policy routing" by packet filters a tool that allows to shoot into ones leg. I can imagine some weird scenarios where people would use pf to actually inject packets where it won't be routed by the normal stack.

That's fair. Though, currently, an operator who uses pf route-to without realizing it can emit L2 broadcasts across broadcast domains is foot-shooting themselves unknowingly. :)

We do not prevent a superuser from foot-shooting and we do not assume that superuser is stupid. A warning should be enough.

In D56559#1294663, @glebius wrote:

I always assumed "policy routing" by packet filters a tool that allows to shoot into ones leg. I can imagine some weird scenarios where people would use pf to actually inject packets where it won't be routed by the normal stack.

I have submitted a draft PR upstream to port https://reviews.freebsd.org/D31263 over to dhcpcd.

Added back atf_skip for i386/armv7 to jail_cleanup

In D43591#994155, @kevans wrote:

Seems to LGTM. At some point we may want to update lbh->bootonce as new environments are activated/deactivated, but this is sufficient for bectl's needs and we don't really make many guarantees about the caching (or lack thereof) that occurs in the handle today, IIRC.

1. Adds test cases for destroy and rename cases
2. Fix bootonce leak

Handles destroying bootonce case

In D43591#994147, @kevans wrote:

Can you add a test for this in sbin/bectl/tests/bectl_test.sh as well, please? Presumably you could use zfsbootcfg -z rather than checking the bectl list output.

I note that we should also do something on destroy of the bootonce dataset.

Thanks for that. LGTM

This looks good to me.

In D41436#976157, @jlduran_gmail.com wrote:

In D41436#976151, @rcm wrote:

In D41436#976128, @jlduran_gmail.com wrote:

In D41436#976125, @rcm wrote:

Would it be possible to also get a v6 case?

Line 29?

ah yes, I should have scrolled up.

Is that really testing for the same case?

# setup_vnet(pytest:TestRtNlRoute:test_add_route4_ll_gw)
run: '/sbin/sysctl net.inet6.ip6.dad_count=0'
run: '/sbin/ifconfig epair0a up'
run: '/usr/sbin/ndp -i epair0a -disabled'
run: '/sbin/ifconfig epair0a inet6 2001:db8::1/64'
run: '/sbin/ifconfig epair0a inet6'
run: '/sbin/ifconfig tun create'

The whole point of D41330 was to handle the case where the interface doesn't have a v4/v6 address.

I'm working on adding it. Just like the IPv4 one (using tun, without a gateway), I'll name the tests add_route{4,6}_ll_if_gw.

In D41436#976128, @jlduran_gmail.com wrote:

In D41436#976125, @rcm wrote:

Would it be possible to also get a v6 case?

Line 29?

Would it be possible to also get a v6 case?

@kp and I actually discussed this back in June in response to an internal need. In fact, he pointed me towards this exact interface ifconfig_get_orig_name.

Thanks for doing that. LGTM

Added @jrtc27 and @mhorne to chime in concerning @imp's question concerning risc-v (and possibly armv7)

In D42459#969394, @imp wrote:

I generally like this change, one or two quibbles to work out.
I'd also split the lua and other stuff into separate commits (the old lua code will work with the new loader.efi, which I like as well).
But I can do the splitting if that's a hassle. This is otherwise fairly clean so I wouldn't mind a small amount of extra work.

Stripped out lua diff to submit separately

In D42415#969126, @arichardson wrote:

I see you have a reduced test case, would it be possible to turn it into a regression test for in the ld-elf tests?

Address Warner's comment regarding legacy hints and update the man page as suggested by Kyle

remove the entire if statement