I'd love to see tests running faster, but I'm not expert to tell if we are going to have any side effects of this change.

IMHO, if everything in the network stack is coded properly, we don't need explicit epoch_drain_callbacks() anywhere and this KPI should disappear.

I actually like "wild". The "unconnected" is already used for inpcbs that are not put into any hash, e.g. right after socket(2).

I screwed up, but not on this hash. Looking at cd5ff4e841fb I see that I lost MTX_DUPOK on UDP endpoint hash.

• Fix the wild_okay case: either side being INADDR_ANY is okay

• Cover the IPv6 counterpart, too, of course.

I'm not blocking your change. Just suggesting.

No objection, but I'm not ND expert. The test coding looks good.

Two options: 1) embed all of this in struct tcp_rack:

#ifdef TCP_REQUEST_TRK
        /* Response tracking addons. */
        uint8_t t_tcpreq_req;   /* Request count */
        uint8_t t_tcpreq_open;  /* Number of open range requests */
        uint8_t t_tcpreq_closed;        /* Number of closed range requests */
        uint32_t tcp_hybrid_start;      /* Num of times we started hybrid pacing */
        uint32_t tcp_hybrid_stop;       /* Num of times we stopped hybrid pacing */
        uint32_t tcp_hybrid_error;      /* Num of times we failed to start hybrid pacing */
        struct tcp_sendfile_track *t_tcpreq_info;
#endif

or 2) create a new structure that holds all of the above, lazily allocate it and have a pointer from struct tcp_rack to it.

Given this is purely a RACK feature, I would suggest to move it from generic struct tcpcb to struct tcp_rack.

Thanks! Maybe provide an XXX comment that we intentionally (but why?) make non-default vnets to always have it set to 1? Make it a documented strangeness.

In D56374#1290430, @zlei wrote:

They're not directly related to if_vmove(), but to the routines and bpf. For example there is still a little window for the data path to see a NULL if->if_bpf.

In D56473#1292731, @markj wrote:

IMHO, every use of IS_DEFAULT_VNET() is a mark of incorrect code :) Ideally the function shall be split into two: one for SYSINIT() and other for VNET_SYSINIT().

Why, what's the advantage?

To be more verbose, in this particular case, an equivalent but better change would be:

// at line 86
VNET_DEFINE(uint32_t, _rt_numfibs) = 1;
...
static void
vnet_rtnumfibs_init(const void *unused __unused)
{
        int num_rtables_base;

IMHO, every use of IS_DEFAULT_VNET() is a mark of incorrect code :) Ideally the function shall be split into two: one for SYSINIT() and other for VNET_SYSINIT().

I don't think it is possible to reproduce that. The program always passes correct optval to a correct socket. The flag enabling setsockopt() shall never fail in the kernel. Treat that more as a stylistic change. It might be useful for FreeBSD-derived projects, e.g. if Mac OS X refreshes their ping(1) with our ping, as they already did in the past, and their kernel may fail at these setsockopts.

Sorry, the unconditional check was a wrong suggestion. The correct one is:

if (cmd->cmd_securelevel > 0 && securelevel_gt(nlp_get_cred(nlp), cmd->cmd_securelevel)) {

No, this is not the right way to use enums.

The previous version (modulo the mistake) looked better. What's the point in the additional bool? All existing declarations rely on sparse initialization, so would have .cmd_securelevel = 0 always. If you add cmd_securelevel_set, it would be .cmd_securelevel_set = false. Thus, checking .cmd_securelevel_set for being true has no difference to checking .cmd_securelevel to be positive. I'd suggest to just do the securelevel_gt() check unconditionally.

But my test machine panics now right after booting when running ntp. Need to work around that first.

In D56374#1290307, @zlei wrote:

I have WIP to overhaul the if_attach() and if_detach(). But that is a longer task and I think a workaround shall fit right now.

IMHO, the description is good enough and clear. Later, native-speakers-doc-committers can improve.

Not an expert. Looks good code wise.

Not an expert in the area.

@bms any objections on this change?

Whoa. This also needs to be merged to all stable/X.

We can fix nestat(1) to ignore ENOMEM on sysctl(3):

I have no objection to this change. And thanks for splitting the unrelated part into a separate change.

Thanks!

Thanks for the change!

Thanks a lot for noticing the problem. I didn't test, but it looks to me that the fix will regress the very first line of the output. It is now printed immediately and will consist of zeroes or very small values.

Guido, thanks a lot for working on that!

I already went through this in other test cases. A sleep for a second makes test slower to execute but still doesn't give a 100% protection against flakyness. When tests are executed in parallel on a slow machine you may still have a failure sometimes. Here is the solution I came to so far, poll until tcpdump(1)s are all blocked on "bpf" wait channel. See 38edf96b1787ce3d8c00e4466348dab891c7a9ea.

This is kTLS that violates the correct order. This is a known problem (at least to some of us).

In D56134#1286016, @zlei wrote:

I think the word ndp is a bit misleading. I read it this is a change for the userspace one usr.sbin/ndp. I'd prefer nd6_rtr: xxx or nd6: xxx, to indicate the change is for the kernel part.