I looked at other nodes that implement the ng_newhook_t method. They already use M_NOWAIT, so I think this patch is good.

The load simply allows this bug to manifest itself. Under load, the node's queue already contains the item's, so if a message item is sent at that moment (ngtstl shutdown), it will also go to the queue and will be processed by ngthread(). Without load, the queue is mostly empty, the message item will be delivered/processed by the thread of the calling process directly: ngctl -> syscall (sandto(2)) -> ngc_send (ng_socket(4)) -> ng_eiface(4) without entering to the EPOCH section. Therefore, the bug does not manifest itself.

JFYI, the same situation. If you call "ngctl shutdown ngeth0:" under load:

I think the problem is somewhat broader. Any message item can be added to the node's queue under load or if the queue is blocked. This message item will be processed by ngthread() in the EPOCH section. There are many places in the code of the nodes that perform actions prohibited in the EPOCH section.

Overall, this patch looks good to me.

In D30638#688320, @np wrote:

I know it's not directly related but the 'if' here is not needed. If you're tightening the len checks in the function then maybe clean this too?

out:

if (m != NULL)
        m_freem(m);

• Move ethernet packet length check to vxlan_input().

I prefer having sound loaded as a module in fact, but until we have module autoloading by PCI ID this should be available in GENERIC.

The capsicum support should look something like this:

Have you tested this code with CAPSICUM enabled?

As I mentioned previously, the other thing to consider when using kvmclock in the current patch is the need for a system call for clock_gettime(), gettimeofday(), etc.

@me_freebsd_mathieu.digital, what a CPU model was used for testing?

I have no objections.

I'm not an English expert, so you may be right.
Technically, I have no questions, I think this patch can be committed.

Did you mean D28559?

I have no competence in this area. Maybe you wanted to add @fsu to the reviewers? He was working on improving ext2/3/4 support.

Lutz, do you have any plans for the upcoming changes?
I also thought about getting rid of ng_bridge from NG_NODE_FORCE_WRITER. Since rcv_data is always called in the NET_EPOCH context, I think we can do it like @kp did it for if_bridge(4) (see D24250).

I don't really like how ng_etner(4) is implemented as a whole. But I think this patch is correct and should be committed.

@grehan do you have any objections?

Eliminate issues identified by reviewers.

@vmaffione, what do you think about this patch?

Fix indentation.

Add a comment about locking.

I see various errors in the man page:

Do you have any performance measurements?
Is it have advantages over injecting packets through ng_socket(4) or ng_device(4)?

Is this PR: 241133 ?

After looking at your abandoned review (D26420) where you add parsing of IPv6 addresses. I wondered if we could use the generic inet_aton(3) functions available from kernel space?

Sorry to start a discussion here, but we have a similar problem with bhyve. When it is necessary to deliver packets from VM with partial checksum and TSO to the host stack (inbound path).
For example, we need to solve the next path:
VM (virtio-net, TSO, partial checksum) -> if_bridge/ng_bridge -> if_tuntap/ng_eiface -> host stack.

I really like the changes related to option handling.

@vmaffione , do you have any objections?

• Rebase
• Use ',' as options separator.
• Move '#include <sys/sysctl.h>' to NETGRAPH section.

• Free optscopy early.
• While I'm here, fix a memory leak in e1000 frontend.

• Fix the indicated issues.

It looks good to me. Thank you.

@vmaffione , I left '/' as a separator, because ';' used by the shell (tcsh, sh), so there is a need to enclosure the option string.
I think the best solution is still to pass the full line of options to the backend. Then we can use ',' as the delimiter.
I have not completely understood how backward compatibility can be broken? May you clarify?

• Reuse tap backend functions.

@vmaffione do you have any objections?

• Revert: 'relpath' -> 'path' option.
• Correctly calculate the maximum available socket buffer size, as is done in the kernel.

Please keep it "path".
"relpath" is useless in this situation, because your node is not yet connected and can't use a relative path at all.