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

[Vlad Yasevich <vyasevic@redhat.com>]

On 12/18/2015 06:06 AM, Yan Vugenfirer wrote:
> 
>> On 15 Dec 2015, at 12:24, Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> 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?
> 
> Actually I am OK with this part. Windows will change driver configuration -> driver will report MTU to the host. 
> We don’t have the way to support host report of MTU change to the guest on Windows.

It could be reported through a new status bit that should probably be added to the doc.
This is actually fairly important.  If the mtu on the host side of the device is changed,
we need to be able to change the mtu on the guest side.  Otherwise, a mismatch will cause
MTU sized packets to be dropped.

-vlad

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