Linux Virtualization - Aug 2015 - [PATCH v2 1/2] virtio-net: rephrase devconf fields description

This set of two patches adds a new field called default_mtu to
the configuration area of network devices. The motivation is to
allow libvirt to set initial MTU different from 1500 on guests
virtual NICs.  We also propose to use this new field to report
MTU changes by the guest OS to the device to facilitate
debugging and mtu tunning.

The first patch just clarify the definition of existing fields in
network device configuration are. The second one adds default_mtu
and its description.

v2 changes:
  * address comments by Michael S. Tsirkin:
    - mac is driver-read-only.
    - two status bits depend on different feature flags.
    - use should/must only in conformance statements.
    - rephrase "is set" -> "is offered" for features.
    - other cosmetic changes.
    - added conformance statements for a device and for a driver
      regarding new 'default_mtu" field.

Victor Kaplansky (2):
  virtio-net: rephrase devconf fields description
  virtio-net: add default_mtu configuration field

 content.tex | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 11 deletions(-)

-- 
--Victor

Clarify general description of the mac, status and
max_virtqueue_pairs fields. Specifically, the old description is
vague about configuration layout and fields offsets when some of
the fields are non valid.

Also clarify that validity of two status bits depends on two
different feature flags.

Signed-off-by: Victor Kaplansky <victork at redhat.com>
---
 content.tex | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/content.tex b/content.tex
index d989d98..342183b 100644
--- a/content.tex
+++ b/content.tex
@@ -3115,23 +3115,14 @@ were needed.
 \subsection{Device configuration layout}\label{sec:Device Types / Network
Device / Device configuration layout}
 \label{sec:Device Types / Block Device / Feature bits / Device configuration
layout}
 
-Three driver-read-only configuration fields are currently defined. The
\field{mac} address field
-always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
-\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
-read-only bits (for the driver) are currently defined for the status field:
-VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
+
+The following configuration fields are currently defined:
 
 \begin{lstlisting}
 #define VIRTIO_NET_S_LINK_UP     1
 #define VIRTIO_NET_S_ANNOUNCE    2
 \end{lstlisting}
 
-The following driver-read-only field, \field{max_virtqueue_pairs} only exists
if
-VIRTIO_NET_F_MQ is set. This field specifies the maximum number
-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.
-
 \begin{lstlisting}
 struct virtio_net_config {
         u8 mac[6];
@@ -3140,6 +3131,35 @@ struct virtio_net_config {
 };
 \end{lstlisting}
 
+\field{mac}, \field{status}, \field{max_virtqueue_pairs} are
+driver-read-only. \field{default_mtu} can also be written
+by the driver after negotiation.
+
+\begin{description}
+\item [\field{mac}] is MAC address assigned by the device and is valid
+    only if VIRTIO_NET_F_MAC is offered.
+
+\item [\field{status}] consists of two driver-read-only status bits:
+
+\begin{description}
+
+\item VIRTIO_NET_S_LINK_UP is valid if VIRTIO_NET_F_STATUS is
+    offered.
+
+\item VIRTIO_NET_S_ANNOUNCE is valid if
+    VIRTIO_NET_F_GUEST_ANNOUNCE is offered.
+
+\end{description}
+
+\item [\field{max_virtqueue_pairs}] tells the driver the maximum
+    number of each of virtqueues (receiveq1\ldots receiveqN and
+    transmitq1\ldots transmitqN respectively) that can be configured
+    on the device once VIRTIO_NET_F_MQ is negotiated.
+    \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is
+    set and can be read by the driver.
+
+\end{description}
+
 \devicenormative{\subsubsection}{Device configuration layout}{Device Types /
Network Device / Device configuration layout}
 
 The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000
inclusive,
-- 
--Victor

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.

Added conformance statements for a device and a driver.

Signed-off-by: Victor Kaplansky <victork at redhat.com>

    Signed-off-by: Victor Kaplansky <victork at redhat.com>
---
 content.tex | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/content.tex b/content.tex
index 342183b..439d005 100644
--- a/content.tex
+++ b/content.tex
@@ -3078,6 +3078,12 @@ features.
 
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
+
+\item[VIRTIO_NET_F_DEFAULT_MTU(24)] Default MTU is supported.  If
+    offered by the device, device advises driver about initial MTU to
+    be used. If negotiated, the driver uses \field{default_mtu} as
+    an initial value and reports MTU changes to the device.
+
 \end{description}
 
 \subsubsection{Feature bit requirements}\label{sec:Device Types / Network
Device / Feature bits / Feature bit requirements}
@@ -3128,6 +3134,7 @@ struct virtio_net_config {
         u8 mac[6];
         le16 status;
         le16 max_virtqueue_pairs;
+        le16 default_mtu;
 };
 \end{lstlisting}
 
@@ -3158,6 +3165,15 @@ by the driver after negotiation.
     \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is
     set and can be read by the driver.
 
+\item [\field{default_mtu}] is a hint to the driver set by the
+    device. It is valid during feature negotiation only if
+    VIRTIO_NET_F_DEFAULT_MTU is offered and holds the initial value
+    of MTU to be used by the driver. If VIRTIO_NET_F_DEFAULT_MTU is
+    negotiated, the driver uses the \field{default_mtu} as an initial
+    value, and also reports MTU changes to the device by writes to
+    \field{default_mtu}.  Such reporting can be used for debugging,
+    or it can be used for tunning MTU along the network.
+
 \end{description}
 
 \devicenormative{\subsubsection}{Device configuration layout}{Device Types /
Network Device / Device configuration layout}
@@ -3165,6 +3181,9 @@ by the driver after negotiation.
 The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000
inclusive,
 if it offers VIRTIO_NET_F_MQ.
 
+The device MUST set \field{default_mtu} to between 68 and 65535
+inclusive, if it offers VIRTIO_NET_F_DEFAULT_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.
@@ -3177,6 +3196,12 @@ 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 negotiate VIRTIO_NET_F_DEFAULT_MTU if the device
+offers it.  If the driver negotiates the VIRTIO_NET_F_DEFAULT_MTU
+feature, the driver MUST use \field{default_mtu} as an initial value
+for MTU and the driver MUST report the value of MTU to
+\field{default_mtu} when MTU is modified by the guest.
+
 \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
-- 
--Victor

On 08/16/2015 09:42 PM, Victor Kaplansky wrote:
> Clarify general description of the mac, status and
> max_virtqueue_pairs fields. Specifically, the old description is
> vague about configuration layout and fields offsets when some of
> the fields are non valid.
>
> Also clarify that validity of two status bits depends on two
> different feature flags.
>
> Signed-off-by: Victor Kaplansky <victork at redhat.com>
> ---
>  content.tex | 42 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index d989d98..342183b 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3115,23 +3115,14 @@ were needed.
>  \subsection{Device configuration layout}\label{sec:Device Types / Network
Device / Device configuration layout}
>  \label{sec:Device Types / Block Device / Feature bits / Device
configuration layout}
>  
> -Three driver-read-only configuration fields are currently defined. The
\field{mac} address field
> -always exists (though is only valid if VIRTIO_NET_F_MAC is set), and
> -\field{status} only exists if VIRTIO_NET_F_STATUS is set. Two
> -read-only bits (for the driver) are currently defined for the status
field:
> -VIRTIO_NET_S_LINK_UP and VIRTIO_NET_S_ANNOUNCE.
> +
> +The following configuration fields are currently defined:
>  
>  \begin{lstlisting}
>  #define VIRTIO_NET_S_LINK_UP     1
>  #define VIRTIO_NET_S_ANNOUNCE    2
>  \end{lstlisting}
>  
> -The following driver-read-only field, \field{max_virtqueue_pairs} only
exists if
> -VIRTIO_NET_F_MQ is set. This field specifies the maximum number
> -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.
> -
>  \begin{lstlisting}
>  struct virtio_net_config {
>          u8 mac[6];
> @@ -3140,6 +3131,35 @@ struct virtio_net_config {
>  };
>  \end{lstlisting}
>  
> +\field{mac}, \field{status}, \field{max_virtqueue_pairs} are
> +driver-read-only. \field{default_mtu} can also be written
> +by the driver after negotiation.
> +
> +\begin{description}
> +\item [\field{mac}] is MAC address assigned by the device and is valid
> +    only if VIRTIO_NET_F_MAC is offered.
> +
> +\item [\field{status}] consists of two driver-read-only status bits:
> +
> +\begin{description}
> +
> +\item VIRTIO_NET_S_LINK_UP is valid if VIRTIO_NET_F_STATUS is
> +    offered.
> +
> +\item VIRTIO_NET_S_ANNOUNCE is valid if
> +    VIRTIO_NET_F_GUEST_ANNOUNCE is offered.
> +
> +\end{description}
> +
> +\item [\field{max_virtqueue_pairs}] tells the driver the maximum
> +    number of each of virtqueues (receiveq1\ldots receiveqN and
> +    transmitq1\ldots transmitqN respectively) that can be configured
> +    on the device once VIRTIO_NET_F_MQ is negotiated.
> +    \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is
> +    set and can be read by the driver.
> +
Hi:

I don't get the point that adding "can be read by the driver".
Looks
like it's hard for hypervisor to detect this?

Thanks

On 08/16/2015 09:42 PM, Victor Kaplansky 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.
>
> Added conformance statements for a device and a driver.
>
> Signed-off-by: Victor Kaplansky <victork at redhat.com>
>
>     Signed-off-by: Victor Kaplansky <victork at redhat.com>
> ---
>  content.tex | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index 342183b..439d005 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3078,6 +3078,12 @@ features.
>  
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_DEFAULT_MTU(24)] Default MTU is supported.  If
> +    offered by the device, device advises driver about initial MTU to
> +    be used. If negotiated, the driver uses \field{default_mtu} as
> +    an initial value and reports MTU changes to the device.
> +
>  \end{description}
>  
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network
Device / Feature bits / Feature bit requirements}
> @@ -3128,6 +3134,7 @@ struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 default_mtu;
Looks like "mtu" is ok, consider we use "mac" instead of
"default_mac".
>  };
>  \end{lstlisting}
>  
> @@ -3158,6 +3165,15 @@ by the driver after negotiation.
>      \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is
>      set and can be read by the driver.
>  
> +\item [\field{default_mtu}] is a hint to the driver set by the
> +    device. It is valid during feature negotiation only if
> +    VIRTIO_NET_F_DEFAULT_MTU is offered and holds the initial value
> +    of MTU to be used by the driver. If VIRTIO_NET_F_DEFAULT_MTU is
> +    negotiated, the driver uses the \field{default_mtu} as an initial
> +    value, and also reports MTU changes to the device by writes to
> +    \field{default_mtu}.  Such reporting can be used for debugging,
> +    or it can be used for tunning MTU along the network.
> +
I vaguely remember that config is read only in some arch or transport
and that's why we introduce another vq cmd to confirm the announcement.
Probably we should do same for this?

On Sun, Aug 16, 2015 at 04:42:25PM +0300, Victor Kaplansky
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.
> 
> Added conformance statements for a device and a driver.
> 
> Signed-off-by: Victor Kaplansky <victork at redhat.com>
> 
>     Signed-off-by: Victor Kaplansky <victork at redhat.com>
> ---
>  content.tex | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 342183b..439d005 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3078,6 +3078,12 @@ features.
>  
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_DEFAULT_MTU(24)] Default MTU is supported.  If
> +    offered by the device, device advises driver about initial MTU to
> +    be used. If negotiated, the driver uses \field{default_mtu} as
> +    an initial value and reports MTU changes to the device.
> +
>  \end{description}
>  
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network
Device / Feature bits / Feature bit requirements}
> @@ -3128,6 +3134,7 @@ struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 default_mtu;
>  };
>  \end{lstlisting}
>  
> @@ -3158,6 +3165,15 @@ by the driver after negotiation.
>      \field{max_virtqueue_pairs} is valid only if VIRTIO_NET_F_MQ is
>      set and can be read by the driver.
>  
> +\item [\field{default_mtu}] is a hint to the driver set by the
> +    device. It is valid during feature negotiation only if
> +    VIRTIO_NET_F_DEFAULT_MTU is offered and holds the initial value
> +    of MTU to be used by the driver. If VIRTIO_NET_F_DEFAULT_MTU is
> +    negotiated, the driver uses the \field{default_mtu} as an initial
> +    value, and also reports MTU changes to the device by writes to
> +    \field{default_mtu}.  Such reporting can be used for debugging,
As already said, it's better to change to 'mtu' since changes can
be reported back by writing to the field.

fbl
> +    or it can be used for tunning MTU along the network.
> +
>  \end{description}
>  
>  \devicenormative{\subsubsection}{Device configuration layout}{Device Types
/ Network Device / Device configuration layout}
> @@ -3165,6 +3181,9 @@ by the driver after negotiation.
>  The device MUST set \field{max_virtqueue_pairs} to between 1 and 0x8000
inclusive,
>  if it offers VIRTIO_NET_F_MQ.
>  
> +The device MUST set \field{default_mtu} to between 68 and 65535
> +inclusive, if it offers VIRTIO_NET_F_DEFAULT_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.
> @@ -3177,6 +3196,12 @@ 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 negotiate VIRTIO_NET_F_DEFAULT_MTU if the device
> +offers it.  If the driver negotiates the VIRTIO_NET_F_DEFAULT_MTU
> +feature, the driver MUST use \field{default_mtu} as an initial value
> +for MTU and the driver MUST report the value of MTU to
> +\field{default_mtu} when MTU is modified by the guest.
> +
>  \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
> -- 
> --Victor
> --
> To unsubscribe from this list: send the line "unsubscribe netdev"
in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>