[Differential] D20276: [bhyve][virtio-net] Allow guest VM's to set JUMBO MTU in case of using the VALE switch.

[v.maffione_gmail.com (Vincenzo Maffione)]

v.maffione_gmail.com added a comment.


  Yes, but I think the story is more complex than that. Guests can publish single-descriptor chains or multi-descriptor chains, depending on feature negotiation and driver implementation. With mergeable buffers enabled, or TSO features disabled, guests will normally submit single-descriptor chains (because it makes sense), but this is not mandatory. With TSO enabled and mergeable buffers disabled, guests will normally pass in multi-descriptor chains, each one describing 64K or 32K buffers.
  It makes sense to add more vq methods to handle the mergeable rx bufs case, but  think they should be moved to usr.sbin/bhyve/virtio.c, so that they can be reused.

INLINE COMMENTS

> pci_virtio_net.c:525
> +
> +	if (ndesc < minavail)
> +		return (0);

why this check? if there are not enough descriptors to cover 'len' bytes the function will return 0 anyway.

> pci_virtio_net.c:526
> +	if (ndesc < minavail)
> +		return (0);
> +

You are mixing declarations and code here. Is it allowed?

> pci_virtio_net.c:670
>  
> -	/* Interrupt if needed, including for NOTIFY_ON_EMPTY. */
> +	vq_inc_used_idx_and_last_avail(vq, used);
>  	vq_endchains(vq, 1);

I think you should call vq_inc_used_idx_and_last_avail() after each packet, otherwise you are introducing artificial latency. As a side effect, this allows you to remove the "start" argument ("used" local variable) and the used local variable.

> pci_virtio_net.c:673
> +
> +	return;
>  }

why this return?

CHANGES SINCE LAST ACTION
  https://reviews.freebsd.org/D20276/new/

REVISION DETAIL
  https://reviews.freebsd.org/D20276

EMAIL PREFERENCES
  https://reviews.freebsd.org/settings/panel/emailpreferences/

To: aleksandr.fedorov_itglobal.com, #bhyve, jhb, rgrimes, krion, v.maffione_gmail.com
Cc: mizhka_gmail.com, novel, olevole_olevole.ru, freebsd-virtualization-list, evgueni.gavrilov_itglobal.com, bcran