Protect tailq operation with the bucket lock.

Remove unrelated changes.

Review remarks

In D39048#891283, @melifaro wrote:

Nit: it'll break the build w/o NETLINK. I'll come up with a diff similar to D39148 in a day or two.

Review remarks

Review remarks

In D39048#891099, @melifaro wrote:

Q: any concerns over not adding find-by-name handler? It seems cheap codeline-wise on the kernel side and will simplify client side (avoid if_getunit call).

Rebase on top of netlink-ified carp

Review remarks

Remove sleep

In D39113#890320, @zlei wrote:

BTW, without INVARIANTS, I have not encounter any problem even run the test script (as in TEST PLAN) multiple times. Maybe I have good luck, or there're something hide behind net epoch .

carp_master_down_locked() will call ip_output() , arprequest() and nd6_na_output() those all require net epoch. Although the ifp holds a reference but I think without net epoch the system will behave badly, possibly access freed memory.

The epoch system serves to make sure that resources we rely on (such as the ifp, or route or neighbour objects) don't go away while we're using them. It's pretty unlikely for them to actually go away in the tiny slice of time where it'd matter, so it's completely normal that you didn't see any issues even with the missing epoch calls.

In D39113#890307, @zlei wrote:

In D39113#890304, @kp wrote:

It'd be good to add this case to the carp tests. Can you? If not, I'll take a look. I'm doing some carp work as well anyway.

I'm not familiar with the atf test framework, and I have no bandwidth. I'm still struggling with route cache framework yet :(

It'd be good to add this case to the carp tests. Can you? If not, I'll take a look. I'm doing some carp work as well anyway.

Use attributes (i.e. TLV) for everything.

Review remarks

ping6 -> ping -6

Full patch this time.

In D38940#886682, @melifaro wrote:

I'm happy to work with you (or implement a variation of this patch myself) that uses netlink as the control mechanism.

Two non-blocking remarks.

In D38940#887365, @zlei wrote:

In D38940#887195, @kp wrote:

In D38940#887150, @zlei wrote:

This allows carp to be used in unicast mode, which is useful in certain virtual configurations (e.g. AWS, VMWare ESXi, ...)

I use carp in my production environment (ESXi), and I have to enable promisc mode for virtual ports so that multicast frames can forward between VMs.

I do not familiar with AWS, so suppose AWS does not provide mechanism like ESXi. If AWS is willing to provide ( configuration of promisc mode ) then this feature is not that useful.

It doesn't. Yesterday I discovered that the current version of the patch doesn't work in AWS, because it changes the source MAC address to a multicast address. I've got a fix for that (i.e. don't change the MAC address in unicast mode) that allows it to work in AWS.

I think AWS and ESXi by default block those forge packets with CARP(VRRP) multicast source MAC address.

I wouldn't describe the traffic as forged. (And I was wrong, it's not a multicast MAC address, but a "virtual router MAC address"). The VRRP standard at least explicitly requires that address to be used. CARP isn't formally standardised (at least that I could find), but it behaves very like VRRP.
In any event, we need to do things slightly differently to make them work in cloud setups.

If I'm reading this right there's a userspace visible change in definitions (e.g. struct route is no longer visible without _KERNEL). If so, should we do an exp-run over the ports to see what the fallout is going to be like?

In D38940#887150, @zlei wrote:

This allows carp to be used in unicast mode, which is useful in certain virtual configurations (e.g. AWS, VMWare ESXi, ...)

I use carp in my production environment (ESXi), and I have to enable promisc mode for virtual ports so that multicast frames can forward between VMs.

I do not familiar with AWS, so suppose AWS does not provide mechanism like ESXi. If AWS is willing to provide ( configuration of promisc mode ) then this feature is not that useful.

It doesn't. Yesterday I discovered that the current version of the patch doesn't work in AWS, because it changes the source MAC address to a multicast address. I've got a fix for that (i.e. don't change the MAC address in unicast mode) that allows it to work in AWS. Even in promiscuous mode the multicast traffic never arrives, so we do need this at least for AWS, and it would be useful for ESXi to avoid needing to run interfaces in promiscuous mode.

In D38940#886690, @kp wrote:

I'm going to use this as an opportunity to get to know netlink a bit more, so I'll take a stab at a first version myself, and then we'll go from there.

In D38940#886682, @melifaro wrote:

I'm happy to work with you (or implement a variation of this patch myself) that uses netlink as the control mechanism.

I'm a little torn on how to handle the extension in the interface to userspace. I've added a new ioctl for it, but we could also extend the struct (and then teach the existing ioctl to cope with two sizes of structure), or we could convert the whole thing to using netlink, to make future extensions easier.

We're failing a couple of test cases in /usr/tests/sbin/pfctl with this patch. At least one of the failures is due to the duplicate allow-opts, so an easy fix.

With the patch that seems to fail to build on armv6 (and I suspect on i386 as well):

Also, somewhat random question: are we keeping the programming API identical (or close to) Linux?

That's not bad at all. It seems to perform about as well as the current copy-structs-around ioctl() interface.

In D38843#885892, @afedorov wrote:

@kp , just out of curiosity. How many packets per second did you get in your test after this patch?

Yeah, my test box has vm.ndomains: 2.
This version is much, much faster than the pervious, and it does make the code simpler to reason about (than what we had before) as well.

In D38843#884509, @markj wrote:

Thanks! I'll try setting this up. Was it a plain GENERIC-NODEBUG config, or were you testing with options RSS enabled?

In D38843#884498, @markj wrote:

In D38843#884393, @kp wrote:

The intent was also to avoid locking between the sender and receiver side.
Re-running my test setup shows 889.672 Kpps without this change, and with it I see 42.635 Kpps. That's kind of a steep hit.

Wow, indeed. Could you please describe your test setup?

The intent was also to avoid locking between the sender and receiver side.
Re-running my test setup shows 889.672 Kpps without this change, and with it I see 42.635 Kpps. That's kind of a steep hit.