As the author of the original patch about increasing MTU, I consider this change to be correct.

My main objection is that backward compatibility is broken. We can no longer have uplink[N] and link[N] at the same time. This may break existing software.

Abandon due to D39510

I don't like this approach. We should stop using IF_QUEUE altogether.

@elenamihailescu22_gmail.com , I really respect your work and what you have done.

@corvink , I think this commit is too early. Because we haven't finished supporting snapshots, but this commit already fixes the user interface for live migration, which is not finished at all.

I think we should bring support for snapshots first. And enable BHYVE_SNAPSHOT by default.

In D39805#906206, @markj wrote:

Would anyone be willing to test this on an AMD system?

up

In D39369#896103, @zlei wrote:

Generally looks good to me.

The patch slightly increases RX PPS: ~ 100 Kpps.

From to ?

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

@vmaffione ?

Thanks. I will submit this patch in a separate review where we can discuss the details.

My main objection is that it is bad to extend 'struct protosw' for only one consumer ( ng_kocket(4) ).

In D36451#828133, @mjg wrote:

can i get some feedback here? the netgraph panic is partially blocking other work

@rew, do you have any plans to continue this work? If not, I'd like to take over.

Looks good to me.

I'm concerned about two things:

I have this problem in STABLE-13 with disk image located on NFS share.

@vmaffione , do you have any objections?

In D34260#775362, @markj wrote:

BTW, mdstart_vnode() does not set bio_resid after handling a BIO_FLUSH. I suspect it does not matter in practice, since by convention BIO_FLUSH commands have bio_length == 0.

In D34185#773670, @vmaffione wrote:

Question: there are three more locations in that source file where NG_SEND_DATA_ONLY() is called and it is not apparent that we are inside NET_EPOCH.
Should we enter NET_EPOCH also there, or we are already in?

In D26268#756453, @markj wrote:

Should this be committed?

I don't understand what the alignment does in this case.
Does it fix a bug or is it an optimization?
What part of the code requires this alignment, is it the TCP/IP stack?
Because not only ng_eiface(4) can inject Ethernet packets into the network stack.

The polynomial 0x8005 is CRC-16-IBM or CRC-16-ANSI.

This is done in FreeBSD 13.

I am also against the use of generic messages.

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.