I agree that this default is a long due to be changed. Needs to be mentioned in Release Notes, though.

In D45039#1027550, @kp wrote:

In D45039#1027522, @glebius wrote:

Is this whole stack of reviews a temporary reviews to get easier review for pfsense and network? I think better just squash all my submissions on top of your work and push that to FreeBSD/main as one change.

My plan was to push the separate commits, because they detail the individual elements of the improvements and their justifications.

Is this whole stack of reviews a temporary reviews to get easier review for pfsense and network? I think better just squash all my submissions on top of your work and push that to FreeBSD/main as one change.

In D45053#1027303, @kib wrote:

u_long

Good catch, thanks! Must also be merged to stable/14.

Thanks! Please write a longer commit message, that would mention preparatory and cleanup stuff that we did and aren't part of VRRP addition:

I see. There is quite a lot of CARP specific code to be executed when we are VRRP mode and little bit code vice versa. I'm working pull requestes on github for you.

Move comment back.

In D44774#1024640, @kp wrote:

In D44774#1024470, @glebius wrote:

The switching code should just use a local variable to convert between prio and adnskew.

We can't convert between them. They're not different representations of the same concept, they just represent different things.

If we dereference CARP specific stuff in VRRP code or vice versa - that's a bug that needs be fixed rather than masked by non-unionizing data.

But that's what putting them in a union does. It, for lack of a better word, infects carp with vrrp settings and vice versa.

SOCKET_HHOOK instead of TCP_HHOOK

In D44774#1024457, @kp wrote:

Putting those variables in a union means that setting sc_vrrp_prio will clobber sc_advskew (and vice versa). These have different constraints and different meanings. It just makes things harder than they need to be when switching between carp and vrrp.

Or, this can go under SOCKET_HHOOK new option. Cause these two hhooks aren't really related. What do you think?

In D44774#1023611, @kp wrote:

Put carp/vrrp3 specific variables in their own structs.

Looks like driver does "hardware" accounting and legacy accounting at the same time. That's why we see correct counts without your patches. If vtnet_accum_stats() provides us with all what we need, then the driver should declare IFCAP_HWSTATS in its capabilities. May I ask you to add this and also combine D44816 and D44817 into a single review?

IMHO, such trivial changes do not require posting a review.

I'm fine with this change, but I really prefer net-snmp not to know anything about struct tcpcb. I'll take a look at it later.

There is a guard at line 88:

#if defined(_KERNEL) || defined(_WANT_TCPCB)

Maybe move the includes there, just next to netinet/cc/cc.h?
What's the final goal of the change?

In D44581#1017675, @kib wrote:

I do not like it. Errno has the defined scope of providing the communication between kernel and userspace. Some of the 'out of band' values like ERESTART and EJUSTRETURN and valid extensions because they modify the kernel->user edge behavior, all other in the list are hacks.
More, the hack you propose is for very special and local situation, where error can be propagated by other means, if ever needed.

In D44581#1017639, @olce wrote:

So, whom should we ask? imp, jhb, kib, markj, some other people? Or are you confident yourself that we can proceed?

Is there any good reasons to keep in kernel only rather than making it a BSD extension, placing it a few lines above in the errno.h right under #ifndef _POSIX_SOURCE?It is a good error for kldload(1) to return to a user, as long as strerror() supports it.

Reducing multiple error codes to one is losing information and it is confusing regardless of how experienced a user is. Even in case of the least experienced user, they would file a problem report or ask on mailing lists, forums, or whatever, they will come asking for help with reduced information. First thing to help such a user would be to write a dtrace script or ask him to patch kernel with printfs to get the actual error.

Why do we really want to lose a meaningful error code and reduce all errors into one value? IMHO, it is counterproductive. And if we got a new error code type, why can't we propagate it to userland, too?

I was about to ask "where is #ifdef TCP_HHOOKS?" But then I realized that socket_hhh isn't hidden under any ifdef.

In D44615#1017245, @dave_freedave.net wrote:

Actually before I go move things to newhook can I ask do we have consensus that I should just have a separate unr(9) for uplink?