Re: [virtio-dev] [PATCH v7] virtio-net: add Max MTU configuration field

["Michael S. Tsirkin" <mst@redhat.com>]

OK I think we are almost there. A minor comment below.

On Fri, Aug 26, 2016 at 01:57:53PM -0400, Aaron Conole wrote:
> Sometimes it is essential for a host to be able to configure MTU
> on guest's NICs to a value other than the assumed 1500 byte value.

essential isn't the way I'd put it.
guests can always configure mtu themselves.
It might be helpful, yes.

One example includes using jumbo frames on the host.

Another is to simplify RX buffer sizing for guests.

Final one is about non-ethernet or other non-standard backends.



> The change adds a new field to configuration area of network
> devices. It will be used to pass a maximum MTU from the device to
> the driver. This will be used by the driver as a maximum value for
> packet sizes during transmission, without transmit segment offloading.
> 
> In addition, in order to support backward and forward compatibility,
> we introduce a new feature bit called VIRTIO_NET_F_MTU.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> ---
> Previous thread:
> https://lists.oasis-open.org/archives/virtio-dev/201603/msg00001.html

changelog?


> Note: should this proposal be accepted and approved, one or more
>       claims disclosed to the TC admin and listed on the Virtio TC
>       IPR page https://www.oasis-open.org/committees/virtio/ipr.php
>       might become Essential Claims.
> 
>  content.tex | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 4b45678..4dc9beb 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3049,6 +3049,14 @@ features.
>  \item[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS (2)] Control channel offloads
>          reconfiguration support.
>  
> +\item[VIRTIO_NET_F_MTU(3)] Maximum negotiated MTU is supported. If
> +    offered by the device, device advises driver about the value of
> +    MTU to be used. If negotiated, the driver uses \field{mtu} as
> +    the maximum MTU value supplied to the operating system.
> +
> +    Note: many operating systems override the MTU value provided by the
> +    driver.
> +
>  \item[VIRTIO_NET_F_MAC (5)] Device has given MAC address.
>  
>  \item[VIRTIO_NET_F_GUEST_TSO4 (7)] Driver can receive TSOv4.
> @@ -3140,11 +3148,16 @@ of each of transmit and receive virtqueues (receiveq1\ldots receiveqN
>  and transmitq1\ldots transmitqN respectively) that can be configured once VIRTIO_NET_F_MQ
>  is negotiated.
>  
> +The following driver-read-only field, \field{mtu} only exists if
> +VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> +use.
> +
>  \begin{lstlisting}
>  struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 mtu;
>  };
>  \end{lstlisting}
>  
> @@ -3153,6 +3166,19 @@ struct virtio_net_config {
>  The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000 inclusive,
>  if it offers VIRTIO_NET_F_MQ.
>  
> +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
> +
> +The device MUST set \field{mtu} to between 68 and 65535 inclusive,
> +if it offers VIRTIO_NET_F_MTU.
> +
> +The device MUST NOT modify \field{mtu} once it has been set.
> +
> +The device MUST NOT pass received packets that exceed \field{mtu} size
> +with gso type NONE or ECN.
> +
> +The device MUST forward transmitted packets of up to MTU size with
> +gso type NONE or ECN, without fragmentation.
> +
>  \drivernormative{\subsubsection}{Device configuration layout}{Device Types / Network Device / Device configuration layout}
>  
>  A driver SHOULD negotiate VIRTIO_NET_F_MAC if the device offers it.
> @@ -3165,6 +3191,16 @@ If the driver does not negotiate the VIRTIO_NET_F_STATUS feature, it SHOULD
>  assume the link is active, otherwise it SHOULD read the link status from
>  the bottom bit of \field{status}.
>  
> +A driver SHOULD supply enough receive buffers to be able to receive at least
> +one receive packet with gso type NONE or ECN.

I would put MUST here. If driver doesn't then networking does not work.
Alternatively, add text explaining what are the consequences.
And/or maybe add device clause saying device can discard packets
if not enough receive buffers.

> +A driver SHOULD negotiate VIRTIO_NET_F_MTU if the device offers it.
> +
> +A driver MUST honor \field{mtu} if it negotiates VIRTIO_NET_F_MTU.

How about we drop this? Generally if you negotiate a feature
we honor it ..


> +A driver MUST NOT transmit packets of size exceeding the value of \field{mtu}
> +with gso type NONE or ECN, if it negotiates VIRTIO_NET_F_MTU.
> +
>  \subsubsection{Legacy Interface: Device configuration layout}\label{sec:Device Types / Network Device / Device configuration layout / Legacy Interface: Device configuration layout}
>  \label{sec:Device Types / Block Device / Feature bits / Device configuration layout / Legacy Interface: Device configuration layout}
>  When using the legacy interface, transitional devices and drivers
> -- 
> 2.5.5
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org