In D48163#1098396, @markj wrote:

In D48163#1098248, @glebius wrote:

On second thought this probably needs to be mentioned in Relnotes and not MFC-ed. Cause it might be that somebody was relying on the stack to drop those packets before firewall.

Well, prior to commit 713264f6b8bc5f they weren't, and that commit is not very old. A RELNOTES entry is reasonable.

Note that our stack will drop such packets even if forwarding is enabled. I guess the concern is that a firewall redirect might turn a packet to 0.0.0.0 into a valid packet?

Looks good to me.

Looks good to me.

In D47940#1094028, @kib wrote:

Let me explain my PoV.

There should be no cast in the SYSINIT() macros at all. Consumer must pass void (*)(void *) function pointer and void *-compatible arg to the macro. The initialization function shall do the required casts.

In D47992#1094000, @markj wrote:

In D47992#1093998, @zlei wrote:

Suppose then that the parent jail is destroyed before the "freeprison" list is processed

I think that is not possible. Child jails should be always freed / destroyed prior to parent jail. See the above logic

	TAILQ_INSERT_TAIL(&freeprison, pr, pr_list);
...
	pr = pr->pr_parent;

So the freeprison list will have correct order, that child jails come first then parent last, if parent jail's pr_uref reach 0.

In the race that I hit, freeprison contains only the child jail, freed by a call to jail_remove(2). The parent jail is freed by a different thread.

Suppose then that the parent jail is destroyed before the "freeprison" list is processed

Remove only for the udata right now, to move fast.

In D47940#1093320, @kib wrote:

BTW, why does SYSINIT() enforces the cast? Isn't it more correct to not cast in the macro, and allow a compiler to check that the function passed matches the type?

In D47915#1092672, @imp wrote:

This did go away where indicated, and is no longer used and appears to not be needed (kldload works, eh?)

Move the the logic

-       last = SI_SUB_COPYRIGHT;
+       last = SI_SUB_DUMMY;

to D47907 .

FIRECRACKER has option VERBOSE_SYSINIT but default off. Short circuiting as the global variable verbose_sysinit is almost constant.

Looks good to me.

Long ago, the bio_transient_map was replaced by a vmem arena.

That appears to be FreeBSD 10.x days.

Looks good to me.

The tests looks good. I need some time to read the code carefully ...

Looks good to me.

Looks good to me.

And, with this change, it seems we no longer require nexthop to be loopback interface (V_lo) for blackhole / rejected routes.

Is this a replacement of D46301 ?

Looks good to me.

Looks good to me.