This is supposed to fix this INVARIANTS only panic:

VNASSERT failed: (flags & tounset) == tounset not true at /usr/src/ocafirmware/FreeBSD/sys/kern/vfs_subr.c:7446 (vn_irflag_unset_locked)
0xfffff8047bdb0c08: type VREG state VSTATE_CONSTRUCTED op 0xffffffff812a4508
    usecount 2, writecount 1, refcount 40 seqc users 0
    hold count flags ()
    flags (VIRF_PGREAD|VMP_LAZYLIST) VI_LOCKed
    v_object 0xfffff81a41ace2b0 ref 0 pages 148 cleanbuf 12 dirtybuf 25
    lock type ufs: EXCL by thread 0xfffff806d43d8000 (pid 14038, nginx, tid 104106)
        nlink=1, effnlink=1, size=2350813, extsize=0
        generation=2d3a9092, uid=80, gid=0, flags=0x0
        ino 9003324, on dev nda3p8
panic: vn_irflag_unset_locked: some of the passed flags not set (have 2, passed 512)

Makes sense to me. Thanks for good explanation in the commit message.

P.S. Oleksandr, if you got review from any other domain expert than Warner, consider this approved by mentor (me).

Not a domain expert, so leaving this 100% to Warner. Code & style wise everything LGTM.

I'd suggest to split the default increase and the added new option into two separate commits.

To me this looks like a more generic problem rather than just the watchdogd's problem. Should every daemon have this feature? Or should we say that /var/run shall be mdmfs(8) on hosts with read-only root?

In D57154#1309864, @markj wrote:

In D57154#1309827, @glebius wrote:

The ioctl path shall not use the epoch.

Why? What about the sysctl path?

In D57155#1309850, @bz wrote:

Unrelated but FYI I recently started to extend show ifnet again which helped me to fix two bugs so far; just in case that's on anyone's list as well.

• Update the ddb(4) handler. The porthash is now empty.

The ioctl path shall not use the epoch. The list is protected by IFNET_WLOCK(). Ideally the IFNET_WLOCK should become a sleepable lock. A temporary solution would be to keep IFNET_WLOCK() as is and allocate a temporary buffer to fill, then unlock and do copyout from this buffer. To make buffer size guessing easier we can track number of members in the V_ifg_head and try to allocate this size.

For this particular patch, I'd suggest the following.

Sorry for widening the discussion, but I think it is better to discuss this more rather then end up with a quick fix. At Netflix Drew is working on a module that is very different from NFS, but is also a fat source of traffic. We have a very similar function in there.

Of course this gets things a bit more sophisticated. I don't like that syncache_socket() will again pull the ipopts from the mbuf as the syncache_add() already just did. Maybe after the fix try to make this less sophisticated?

I like Patrick's change more, but maybe with more verbose comment.

Is this a quick fix mentioned in D56942 that later is to be refactored to delayed free in lagg protocol detach?

I don't think this is a right fix. A route lookup now doesn't guarantee the same interface will be used in the future. There is dynamic routing, weighted routing, policy routing, etc etc etc.

In D56942#1305647, @markj wrote:

I do not see how protocol switching can be fixed without updating protocol pr_attach and pr_detach implementations to be epoch-aware. This means, at minimum:

1. sc_proto must be loaded/updated using acquire/release semantics.
2. pr_detach implementations must use deferred free(), i.e., NET_EPOCH_CALL().

In the commit text, in the phrase it'll drop packets from the queue but fail to remove them, I'd suggest to change drop to free or m_freem.

In D56726#1302322, @pouria wrote:

Do you plan to document this new behavior in bind(2)?

In D56778#1300993, @markj wrote:

But the integration of net_epoch and preexisting locks is still messy and unclear, and in order to fix that we must first simplify existing locking as much as possible, IMHO.

As Zhenlei said, we don't need much parallelism in the configuration path.

Mark, thanks a lot for fixing that. I just started to investigate yesterday, but you were faster. Thanks!

Thanks for working on this, Zhenlei!

Dave, I'm very sorry for not looking at your earlier submissions. Trust me, this is not a reluctance to add them to the src, but just a lack of time. Would you like me to review your nodes on github or would you prefer to refresh the phabricator revisions where you suggested the nodes to add to src? I will try to find some time.

Please take into account Mark's suggestion.

Sorry for slightly switching context. This actually links to the recent "regression" that I planted. With a new API to get exact list of possible bpf taps, instead of guessing via getifaddrs(3) + list USB buses, the building of the list requires privileges. Previously you could run tcpdump -D without privileges on FreeBSD, now it can't. The wireshark/tcpdump guys think that it is a regression and should be fixed.

@markj I adjusted naming per your suggestion. Can you please approve?

• Use in_pcblookup_with_lock() instead of in_pcblookup_locked().

This one to be abandoned as is. An alternative better be a new revision.

If you find a committer (maybe @ae ?), there is no objection from my side, as the added cruft is isolated to stable/14 and won't travel with us into the future.

I wonder if anybody have actually tested that old mode on this relatively new driver. I wonder if anybody in the world runs with the device polling today at all.

Maybe we need kern.trueosreldate added?

Appears I was wrong. We have an instrument for jails to fake the kernel version, added in https://reviews.freebsd.org/D1948. With jails configured this way we can't use kern.osreldate to tell ipfw version.

Excellent. Once we have TCP own socket buffer, it could be part of soisdisconnected() method of TCP. Now it just sits next to this call.

You don't need a special sysctl for that. You can use kern.osreldate, which is generic sysctl designed exactly for such kind of problems.

IMHO, Vova's plan sounds better than a sysctl.

No objections! Good plan for the stable branches. Thanks!

Thanks!

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.

In D56562#1294701, @rmacklem wrote:

For the old way done in FreeBSD-14, the kgss_gssd_handle
would be set NULL when the gssd daemon terminated, at least
when it happened gracefully.

I assume that, for netlink, this never happens?
(ie. Once clnt_nl_create() creates it, it lives forever.)
--> Which means checking for it being non-NULL is a waste

of time?

Bjoern, can you please confirm you are fine with the change.

P.S. Touch to raw_ip.c reminds me of my long desire to move those pointers to some more appropriate place at least.

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.