In D20616#445619, @kristof wrote:

Only if the firewall needs to read/write actual packet data. Protocol headers (TCP, IP, etc.) are always stored in a normal mbuf at the start of a packet's mbuf chain. Unmapped mbufs only hold payload data that is stored in a socket buffer, so most of the filters I can think of off the top of my head as well as things like NAT should only operate on the normal mbuf holding the headers.

Okay, thanks. That should indeed just work. The 'pf_check_proto_cksum()' flow, assuming there's no hardware assist, might break. I suspect that hardware which uses unmapped mbufs is always going to have checksum offload, so that's probably not an issue either.

I think it would be good to have this committed into 11.3 release. So you will be able to see how many users will complain that they need this support, if any.

LGTM.

Can you also update ixv driver? We discovered problems with ixv+VLANs on some KVM hosts with the stock driver, but the driver 1.5.15 from Intel's site works well.

• s/bpf_epoch_buffer/bpf_program_buffer/g
• update some comments
• add copyright line
• add refcount to bpf_d and use it in bpfwrite

move bpf_updated() into bpf_setf() to reduce BPF_LOCK() flipping

There is at least one problem that with this patch becomes easy reproducible. With default optimize_writers=0 bpt_mtap() can catch several packets in the time between bpf_setif() and bpf_setf(), because empty filter means "accept all" by bpf_filter(). Maybe it is time to remove optimize_writers variable and use this behavior by default? I.e. by default link new bpf_if into writers only list, and re-link it into readers list when application sets filter? Or change its value to be 1 by default?

In D19886#431575, @hselasky wrote:

FYI: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=237329

I have no objection. This subsystem is currently broken, but nobody wants to fix it. So, if you tested this patch and it helps to solve your problem, I'm ok, since description looks reasonable.

In D20163#434567, @jhb wrote:

FWIW, my limited testing of IPsec doesn't use if_ipsec, but instead I used setkey. I think having the rc.d scripts for 'ipsec_enable' autoloading ipsec.ko is reasonable.

I think there are too few users of if_ipsec, to make assumption that all users who use IPsec will use ifconfig(8). AFAIR, it is not the problem, you can just add symlink if_ipsec.ko -> ipsec.ko. But you also need some tweaks that will load ipsec,ko when ipsec_enable is "YES".

I'm sorry, I completely missed this change in the past. But it looks like it can break ipfw firewall rules, since rcvif is now union with snd_tag. And this means, rcvif can be initialized for packets that were not actually received on specified interface. ipfw uses rcvif in rules to check that a packet was received on specified interface, and this check was correct even for outgoing packets. Now it looks like such checks can be incorrect.

The epoch_call_drain() function is indeed needed (at least to fix such panic https://reviews.freebsd.org/F4491011).
But your example shows that epoch based reclamation is just wrongly used. The right solution should be keeping ifnet detached until all possible consumers stop reference it, and only then it will be safe to free ifnet pointer.

In D20070#432259, @markj wrote:

I'm not quite familiar with this code. Is it safe enough to make INP_WUNLOCK(); /* some code */ INP_WLOCK(); without holding extra reference to PCB? Is is it impossible, that another thread can destroy PCB when we release lock?

Sorry, I don't see where the question is coming from. In general, no, you have to acquire a reference before dropping the lock. That's what the old version of the diff did, in order to acquire the sleepable IN_MULTI lock. But dropping the PCB lock introduces races. I changed the code to acquire the IN_MULTI lock first, so we don't have to drop the PCB lock anymore.

I'm not quite familiar with this code. Is it safe enough to make INP_WUNLOCK(); /* some code */ INP_WLOCK(); without holding extra reference to PCB? Is is it impossible, that another thread can destroy PCB when we release lock?

LGTM.

In D20006#430085, @kib wrote:

Well, I can take only 8bytes. But what I see on Linux (not me, this is copy/paste from somebody else actions):

# ifconfig ib5
Ifconfig uses the ioctl access method to get the full address information, which limits hardware addresses to 8 bytes.
Because Infiniband address has 20 bytes, only the first 8 bytes are displayed correctly.
Ifconfig is obsolete! For replacement check ip.
ib5       Link encap:InfiniBand  HWaddr 80:00:08:87:FE:80:00:00:00:00:00:00:00:00:00:00:00:00:00:00
          inet addr:16.7.5.140  Bcast:16.7.255.255  Mask:255.255.0.0
          inet6 addr: fe80::ee0d:9a03:43:f7bd/64 Scope:Link

In D20006#430051, @kib wrote:

Hm, from my reading of RFC, there is a very explicit requirement to form 20-byte link-local address as [0:qpn:GID], and the 16-byte l/l should be formed by truncating. I also verified that after the patch, l/l address on the ibX is same as configured by Linux.
Are you worrying about the embedded scope id ? If scope is implanted before the call, I can preserve it ?

I'm not sure that is correct. in6_get_hw_ifid() is used by in6_ifattach_linklocal() via get_ifid() to generate IPv6 link local address. IPv6 link local address has already filled first 8 bytes. And in this change you override them. Note in the line 238 says that first 8 bytes should be preserved.

It looks like INFINIBAND_ALEN defines link address length for infiniband. It is 20 bytes. It seems the check if (addrlen != 16) will be always successful.

• remove RSS-related chunks.
• allow use any port number within [V_ipport_hifirstauto, V_ipport_hilastauto] range.

We have single machine, that after automatic firmware upgrade fails to attache the driver:

It seems this is not enough to prevent panics. Simple kernel module to reproduce the panic.

Document GRE-in-UDP in gre(4).