I'd like to enable it when it is certainly safe to.

Should the V_ip6_mrouting_enabled be turned off immediately ? So the fast path see it as soon as possible ?

I'm struggling with the net epoch... I'm not certain whether a non-atomic read access in the fast path ( probably also want atomic-write in this place ) is sufficient or not.

IMHO, @zlei comment on ip6 also applies here.

I'd like to enable it when it is certainly safe to.

+1

Shouldn't be (EBUSY)?
IMHO, admin request to unload is not invalid.

What is the meaning of version btw?
AFAIK, this line here is redundant. however, we may choose to kept it for backward compatibility.

in multicast(4):

After the multicast routing socket is open, it can be used to enable or
disable multicast forwarding in the kernel:

/* IPv4 */
int v = 1; /* 1 to enable, or 0 to disable */
setsockopt(mrouter_s4, IPPROTO_IP, MRT_INIT, (void *)&v, sizeof(v));

This is wrong obviously. we can't disable multicast forwarding by setting v to 0.
We should use MRT_DONE here.
Do you want to fix the manual or you want to change this logic? (I can help with manual if you want)

Same as above:

multicast(4):

/* IPv6 */
int v = 1; /* 1 to enable, or 0 to disable */
setsockopt(mrouter_s6, IPPROTO_IPV6, MRT6_INIT, (void *)&v, sizeof(v));

we should use MRT6_DONE instead of passing v = 0 to socket.

We should fix the manual. I was going to write an update to multicast.4 to describe the FIB-related changes (not included in the patch stack yet), but if you want to go ahead and fix the issue you found, that would be great.

I think this code is ok in fact: X_ip_mforward() is locked by the mrouter_lock(), so the ordering within this locked section does not matter. The v6 code is broken though.

That's not quite sufficient: the bug here is that we do not hold the mfc6 lock here, so there is no synchronization with the forwarding path.

Note that the multicast forwarding code is synchronized by a mutex, not by net_epoch. So it's not very important. But the locking here is insufficient, I'll update the patch shortly.

Sure.

We should fix the manual. I was going to write an update to multicast.4 to describe the FIB-related changes (not included in the patch stack yet), but if you want to go ahead and fix the issue you found, that would be great.

I greatly appreciate the work you are doing to make multicast forwarding VRF capable in FreeBSD.

However, I really, really wish my use of the term FIB had not been misunderstood to refer to individual instances of routing tries all those years ago. I shouldn't really feel responsible, but I was a participant in the original thread.

This is a clash of terminology. What the rest of the world understands as IP VRF is a superset of what is called "FIB" in FreeBSD, where a "FIB index" is roughly analogous to a VRF instance. It is particularly problematic when discussing FreeBSD "FIB" with industry professionals who are unfamiliar with contemporary FreeBSD, or students.

Unfortunately the "FIB" terminology is quite pervasive in the kernel implementation and tooling (setfib(1), route(8)'s -fib, etc.). On the other hand, I believe we currently don't have any central place which documents what a FIB index actually is. There are some networking man pages like netintro(4), route(4), which don't mention FIBs at all. We should update one of these, probably netintro(4), to 1) document the concept and 2) explain the terminology. It's unlikely I will get to this in the near future.