[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]