Re: [virtio-dev] [PATCH] virtio-net: add mtu configuration field

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

On Fri, Dec 11, 2015 at 11:49:47AM -0500, Aaron Conole wrote:
> Sometimes it is essential for libvirt to be able to configure MTU
> on guest's NICs to a value different from 1500.
> 
> The change adds a new field to configuration area of network
> devices. It will be used to pass initial MTU from the device to
> the driver, and to pass modified MTU from driver to the device
> when a new MTU is assigned by the guest OS.
> 
> In addition, in order to support backward and forward
> compatibility, we introduce a new feature bit called
> VIRTIO_NET_F_DEFAULT_MTU.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> 
> ---
> 
> This is an attempt at continuing the work done by Victor Kaplansky on
> mtu negiotiation for virtio-net devices. It attempts to pick up from
> https://lists.oasis-open.org/archives/virtio-dev/201508/msg00007.html
> and is just a minor blurb from the first patch along with the 2nd patch
> from the series, and some of the feedback integrated.
> 
>  trunk/content.tex | 20 +++++++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/trunk/content.tex b/trunk/content.tex
> --- a/trunk/content.tex	(revision 551)
> +++ b/trunk/content.tex	(working copy)
> @@ -3078,6 +3078,11 @@
>  
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_MTU(24)] Negotiated MTU is supported. If
> +    offered by the device, device advises driver about initial
> +    MTU to be used. If negotiated, the driver uses \field{mtu} as
> +    an initial value.
>  \end{description}
>  
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> @@ -3132,11 +3137,15 @@
>  and transmitq1\ldots transmitqN respectively) that can be configured once VIRTIO_NET_F_MQ
>  is negotiated.
>  
> +The following driver-read-only field, \field{mtu} can be written by the driver
> +after negotiation.
> +
>  \begin{lstlisting}
>  struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 mtu;
>  };
>  \end{lstlisting}
>  
> @@ -3145,6 +3154,11 @@
>  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.
> +
>  \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.
> @@ -3157,6 +3171,12 @@
>  assume the link is active, otherwise it SHOULD read the link status from
>  the bottom bit of \field{status}.
>  
> +A driver SHOULD negotiate VIRTIO_NET_F_MTU if the device
> +offers it.  If the driver negotiates the VIRTIO_NET_F_MTU
> +feature, the driver MAY use \field{mtu} as an initial value
> +for MTU and



> + the driver MAY report the value of MTU to
> +\field{mtu} when MTU is modified by the guest.
> +

I think making mtu writeable like this will lead to inability to report
host side mtu changes to guest.
How about removing this part for now?

>  \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.0
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org