Why not use atomic(9)? I strongly prefer not to have two sets of intrinsics for the same purpose.

More fundamentally, using ck_pr/equivalent atomic(9) here does not accomplish anything other than preventing compiler reordering of the stores. At least a barrier is needed to prevent other CPUs from observing stores out of order.

Can we not encapsulate all three of the array, array length, and epoch context in a structure (one for each AF), and allocate and free them all together? That seems simpler and won't require this tricky synchronization. That is, have

struct prison_ip4_list {
    epoch_context_t ctx;
    int len;
    struct in_addr a[];
};

and have each prison hold a pointer to the current list.

Extra newline.

prison_qcmp_v4 is in in_jail.c, "optional inet". And likewise prison_qcmp_v6 is "optional inet6." Most of the code in this function is generic, but those will need some #ifdef work, or possibly being passed as arguments.

Can we not encapsulate all three of the array, array length, and epoch context in a structure (one for each AF), and allocate and free them all together? That seems simpler and won't require this tricky synchronization. That is, have

struct prison_ip4_list {
    epoch_context_t ctx;
    int len;
    struct in_addr a[];
};

and have each prison hold a pointer to the current list.

This is doable and I like it. That would require massive rewrite, since many functions manipulate pr_ip4 and pr_ip6 arrays. I will post an updated patch, but that would take several days.

prison_qcmp_v4 is in in_jail.c, "optional inet". And likewise prison_qcmp_v6 is "optional inet6." Most of the code in this function is generic, but those will need some #ifdef work, or possibly being passed as arguments.

Thanks for noticing. With updated patch (see reply to Mark) I will take care of NOINET and NOINET6 builds.

Am I right in thinking that this would all be confined to kern_jail.c and in_jail.c?

Yes, 90% would happen in kern_jail.c and in_jail.c and in6_jail.c would shrink a lot. And nothing touched outside of these files.

Later we would be able to remove pr_mtx acquisition inside accessor functions, but that would require callers to guarantee network epoch protection. That would require some sweeping changes, if we really want to go for it.

The removed ip_ip4s and pr_ip6s should probably be replaced by some spares to preserve struct offsets. Or maybe a version bump (and then could IMHO remove the other spares).

What ports kernel modules are known to use struct prison and dereference its fields directly, instead of jail.h KPI?

I don't really know that any actually exist; it's more of a habit I've always been in, that I think was in place when I started. But then, I'm not familiar with the ports codebase. I know that within the kernel itself, there's a lot of bare use of pr_allow and pr_hostname, and wouldn't be surprised if that was reflected elsewhere.

I'd rather not add placeholders (I'd even vote for deleting pr_pspare[3], if asked) and discourage non KPI use of the structure. In the last years we are aiming towards this policy (already done to struct socket, struct inpcb, *pcb, struct ifnet). The fact that we have it hidden under defined(_WANT_PRISON) tells me that same policy is also desired for prison. Cc @pjd who added _WANT_PRSION.

I wasn't thinking userland - I'm pretty sure nothing there uses struct prison. I thought this was for kernel modules that refer to struct prison, and making sure they don't link into a kernel with an altered structure.

We guarantee binary compatibility for kernel modules only in the stable branches. In the main branch we change structures all the time. This change is not planned for merging.

ip6 = prison_ip_copyin(PR_INET6, op, M_WAITOK);

This is the culprit. Must be ip6 count, not M_WAITOK. Don't know how did that happen. Wish we had a typedef for malloc flags.