OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


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


On 01/08/2016 03:03 PM, Aaron Conole wrote:
> Vlad Yasevich <vyasevic@redhat.com> writes:
> 
>> 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.
> 
> Does this really need a new status bit? There is a configuration change
> notification that happens from host->guest. Should the host->guest
> change be incorporated with that? If so, then for the host->guest case,
> we don't need a separate bit. IE: host updates mtu and asserts config change.
> 
> From guest->host, we may want to use a different field; like mtu_guest
> with associated status bit. If guest wants to change the MTU, it fills
> mtu_guest, sets mtu status bit and waits for device to copy the MTU change,
> from 'mtu_guest' to 'mtu' and clear the mtu_guest field + bit, and
> assert the configuration change notification. That way, the device could
> reject some MTU changes.
> 
> What do you think? Makes sense or is it too complicated?

Makes sense..   I was thinking of using the status bit for explicit support, but
I suppose it's not really needed.  I guess a bit would just have a more explicit
signal from host to guest the mtu value has changed.

The guest->host side is the one that I have the concern.  There have to be some
safeguards so that we don't enter the ping-pong scenario where host and guest keep
insisting on different values.  My current thinking is that guest value can't exceed
the value set by the host, but need to think a bit more.  Not sure if this stuff
belongs in the spec either.

Thanks
-vlad

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



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]