In D47534#1085033, @jlduran wrote:

Sigh, it looks like this commit broke the following test:

# create two interfaces
if1=$(ifconfig epair create)
if2=$(ifconfig epair create)
# assign IP addresses in the same subnet
ifconfig $if1 inet 192.0.2.1/24
ifconfig $if2 inet 192.0.2.2/24
# Verify that the route points to the first interface (fails, as $if2 was added last, it points to $if2)
netstat -r4n | grep 192.0.2.0/24

IMHO, we need to fix this behaviour. 
First PINNED route should have priority and second attempt to add the same route on $if2 should fail with EEXIST.
But then the test will fail, because after address deletion from $if1 there will not be any PINNED routes.

In D48163#1098658, @markj wrote:

I don't quite follow - why is the source address necessarily unspecified? The modified check is looking only at the destination address. An unspecified source address can arise in practice, e.g., DHCP clients will generate such packets.

In D48163#1098396, @markj wrote:

I guess the concern is that a firewall redirect might turn a packet to 0.0.0.0 into a valid packet?

I didn't test the patch, so if it is works for you I have no objection. :-)

I think this patch should do what you need.

IMHO when we already have an interface route, we should fail on trying to add the same route on different interface. And I think this should fail even before adding route, when an IP address will be configured.
If you want to test the case that was fixed in D47534 you need to add some static route, then configure interface route that will replace this static route, because interface route has higher priority.

I just tested these commands from PR:

ifconfig tun0 create
ifconfig tun0 10.10.10.10 20.20.20.20
route -n delete -host 20.20.20.20 -interface tun0

with this patch:

--- a/sys/netlink/route/rt.c
+++ b/sys/netlink/route/rt.c
@@ -1010,8 +1010,9 @@ rtnl_handle_delroute(struct nlmsghdr *hdr, struct nlpcb *nlp,
                return (EINVAL);
        }

In D46301#1091706, @franco_opnsense.org wrote:

(attrs.rta_rtflags & RTF_PINNED) ? RTM_F_FORCE : 0

I don't like the idea that you can easily remove PINNED route, but it seems it always worked before.
However as I see, route(8) should pass RTF_PINNED flag to netlink via attrs.rta_rtflags. At least we should reduce use of RTM_F_FORCE only for case when RTF_PINNED was sent from userland.

You probably can directly call similar to bpf_ifdetach() function from if_vmove(). It is called from ioctl context, so you can make detaching synchronously.

Probably for network related code ENOBUFS is better than ENOMEM.

I think you need to modify IFF_CANTCHANGE in sys/net/if.h

• Document some features, also reduce the diff.
• Fix bug in mac:radix table: lookup addr doesn't work due to wrong args order in memcpy

• Document some features, also reduce the diff.

Maybe it would be better implement such feature via named flag, like ignore_source is implemented? Also if_gre(4) has the same problem.

In D45762#1045093, @glebius wrote:

I think you should had not abandon this revision. enc(4) creates suboptimal packets, and this should be improved. Hence I suggested to use __predict_false() in the pf(4) review.

I think it is firewall problem when it can not handle some unexpected data. Pfil hook expects that mbus has M_PKTHDR and m->m_pkthdr.len in this case should not be 0, even when m_len is 0. Thus, I think if doesn't work properly, it should be fixed in firewall.

Probably you can simplify some similar checks in in6_src.c too, e.g. IP6PO_VALID_PKTINFO and IP6PO_VALID_NHINFO. Not sure how it impacts your cache misses measurements.

Probably we should increase esps_notdb or esps_invalid counter here.

I think https://reviews.freebsd.org/D32811, https://reviews.freebsd.org/D33064 also are related.

In D42988#981761, @glebius wrote:

Do you plan to rework access to if_afdata? There are still several panics related to access to already freed if_afdata[AF_INET6].

Can you please assign those PRs to me? Or send links to information if there is no PR.

Isn't IPv6 part affected too?

In D42988#980253, @glebius wrote:

The if_afdata[] array comes from the old BSD times when it was expected that there would be support for many many address families (e.g. IPX, AppleTalk, etc). Right now it has only two entries AF_INET and AF_INET6. It is very very very unlikely it will ever get a third one. It is much more likely that the array will go away and we will have just ifp->if_inet and ifp->if_inet6. Or maybe something more complicated. Anyway, the access to this data is going to change anyway, so there is no point in overdesigning it right now. Any solution for the sake of IfAPI cleanness is acceptable.

Looks similar to what linux does.

I don't like adding extra printfs on fast path processing. This can easily make your system unresponsive.

I think previous logic was correct and derived from code before IPsec overhaul.

In D40421#920356, @imp wrote:

This looks generally good.
I think I have an arm server with bt mode. Any chance it will work?

LGTM. Do you plan to implement NAT_T_FRAG in kernel somehow?

LGTM.

we can't recover why would we ever have identical 4-tuples in the hash