All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] virtio-net: default_mtu - new conf. field
@ 2015-08-16 13:42 Victor Kaplansky
  2015-08-16 13:42 ` [PATCH v2 1/2] virtio-net: rephrase devconf fields description Victor Kaplansky
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-16 13:42 UTC (permalink / raw)
  To: virtio-dev; +Cc: mst, jasowang, netdev, virtualization

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-16 13:42 [PATCH v2 0/2] virtio-net: default_mtu - new conf. field Victor Kaplansky
  2015-08-16 13:42 ` [PATCH v2 1/2] virtio-net: rephrase devconf fields description Victor Kaplansky
@ 2015-08-16 13:42 ` Victor Kaplansky
  2015-08-17  2:43   ` Jason Wang
  2015-08-17  2:43   ` Jason Wang
  2015-08-16 13:42 ` [PATCH v2 2/2] virtio-net: add default_mtu configuration field Victor Kaplansky
  2015-08-16 13:42 ` Victor Kaplansky
  3 siblings, 2 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-16 13:42 UTC (permalink / raw)
  To: virtio-dev; +Cc: mst, jasowang, netdev, virtualization

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@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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-16 13:42 [PATCH v2 0/2] virtio-net: default_mtu - new conf. field Victor Kaplansky
@ 2015-08-16 13:42 ` Victor Kaplansky
  2015-08-16 13:42 ` Victor Kaplansky
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-16 13:42 UTC (permalink / raw)
  To: virtio-dev; +Cc: netdev, virtualization, mst

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@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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-16 13:42 [PATCH v2 0/2] virtio-net: default_mtu - new conf. field Victor Kaplansky
                   ` (2 preceding siblings ...)
  2015-08-16 13:42 ` [PATCH v2 2/2] virtio-net: add default_mtu configuration field Victor Kaplansky
@ 2015-08-16 13:42 ` Victor Kaplansky
  2015-08-17  3:07   ` Jason Wang
                     ` (2 more replies)
  3 siblings, 3 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-16 13:42 UTC (permalink / raw)
  To: virtio-dev; +Cc: mst, jasowang, netdev, virtualization

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@redhat.com>

    Signed-off-by: Victor Kaplansky <victork@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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-16 13:42 [PATCH v2 0/2] virtio-net: default_mtu - new conf. field Victor Kaplansky
  2015-08-16 13:42 ` [PATCH v2 1/2] virtio-net: rephrase devconf fields description Victor Kaplansky
  2015-08-16 13:42 ` Victor Kaplansky
@ 2015-08-16 13:42 ` Victor Kaplansky
  2015-08-16 13:42 ` Victor Kaplansky
  3 siblings, 0 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-16 13:42 UTC (permalink / raw)
  To: virtio-dev; +Cc: netdev, virtualization, mst

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@redhat.com>

    Signed-off-by: Victor Kaplansky <victork@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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-16 13:42 ` Victor Kaplansky
  2015-08-17  2:43   ` Jason Wang
@ 2015-08-17  2:43   ` Jason Wang
  2015-08-19 11:54     ` Victor Kaplansky
  2015-08-19 11:54     ` Victor Kaplansky
  1 sibling, 2 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-17  2:43 UTC (permalink / raw)
  To: Victor Kaplansky, virtio-dev; +Cc: mst, netdev, virtualization



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@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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-16 13:42 ` Victor Kaplansky
@ 2015-08-17  2:43   ` Jason Wang
  2015-08-17  2:43   ` Jason Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-17  2:43 UTC (permalink / raw)
  To: Victor Kaplansky, virtio-dev; +Cc: netdev, virtualization, mst



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@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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-16 13:42 ` Victor Kaplansky
@ 2015-08-17  3:07   ` Jason Wang
  2015-08-19 11:31     ` Victor Kaplansky
  2015-08-19 11:31     ` Victor Kaplansky
  2015-08-20 19:31   ` Flavio Leitner
  2015-08-20 19:31   ` Flavio Leitner
  2 siblings, 2 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-17  3:07 UTC (permalink / raw)
  To: Victor Kaplansky, virtio-dev; +Cc: netdev, virtualization, mst



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@redhat.com>
>
>     Signed-off-by: Victor Kaplansky <victork@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?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-17  3:07   ` Jason Wang
@ 2015-08-19 11:31     ` Victor Kaplansky
  2015-08-20  2:48       ` Jason Wang
  2015-08-20  2:48       ` Jason Wang
  2015-08-19 11:31     ` Victor Kaplansky
  1 sibling, 2 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-19 11:31 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtio-dev, mst, netdev, virtualization

On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote:
> 
> 
> On 08/16/2015 09:42 PM, Victor Kaplansky wrote:
> > @@ -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".

Good point. I'll change the name in the next version of the patch.

> 
> >  };
> >  \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?

If so, we need to add one more feature bit to confirm the ability
of the driver to report MTU, or we can weaken the requirement in
conformance statement and write "the driver may report the MTU".
What do you say?

-- Victor

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-17  3:07   ` Jason Wang
  2015-08-19 11:31     ` Victor Kaplansky
@ 2015-08-19 11:31     ` Victor Kaplansky
  1 sibling, 0 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-19 11:31 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtio-dev, netdev, virtualization, mst

On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote:
> 
> 
> On 08/16/2015 09:42 PM, Victor Kaplansky wrote:
> > @@ -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".

Good point. I'll change the name in the next version of the patch.

> 
> >  };
> >  \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?

If so, we need to add one more feature bit to confirm the ability
of the driver to report MTU, or we can weaken the requirement in
conformance statement and write "the driver may report the MTU".
What do you say?

-- Victor

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-17  2:43   ` Jason Wang
  2015-08-19 11:54     ` Victor Kaplansky
@ 2015-08-19 11:54     ` Victor Kaplansky
  2015-08-20  2:46       ` Jason Wang
  2015-08-20  2:46       ` Jason Wang
  1 sibling, 2 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-19 11:54 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtio-dev, mst, netdev, virtualization

On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote:
> 
> 
> 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@redhat.com>
> > ---
> > +
> > +\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.
> > +
> 
> 
> I don't get the point that adding "can be read by the driver". Looks
> like it's hard for hypervisor to detect this?

AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets
the value of 'max_virtqueue_pairs' even before driver negotiated
VIRTIO_NET_F_MQ. If so, the driver can read the value of
'max_virtqueue_pairs' during negotiation and potentially this
value can even affect negotiation decision of the driver.  

If above is correct, I'll change the description to make this
point more clear.

Thanks,
-- Victor

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-17  2:43   ` Jason Wang
@ 2015-08-19 11:54     ` Victor Kaplansky
  2015-08-19 11:54     ` Victor Kaplansky
  1 sibling, 0 replies; 18+ messages in thread
From: Victor Kaplansky @ 2015-08-19 11:54 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtio-dev, netdev, virtualization, mst

On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote:
> 
> 
> 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@redhat.com>
> > ---
> > +
> > +\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.
> > +
> 
> 
> I don't get the point that adding "can be read by the driver". Looks
> like it's hard for hypervisor to detect this?

AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets
the value of 'max_virtqueue_pairs' even before driver negotiated
VIRTIO_NET_F_MQ. If so, the driver can read the value of
'max_virtqueue_pairs' during negotiation and potentially this
value can even affect negotiation decision of the driver.  

If above is correct, I'll change the description to make this
point more clear.

Thanks,
-- Victor

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-19 11:54     ` Victor Kaplansky
@ 2015-08-20  2:46       ` Jason Wang
  2015-08-20  2:46       ` Jason Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-20  2:46 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, mst, netdev, virtualization



On 08/19/2015 07:54 PM, Victor Kaplansky wrote:
> On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote:
>>
>> 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@redhat.com>
>>> ---
>>> +
>>> +\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.
>>> +
>>
>> I don't get the point that adding "can be read by the driver". Looks
>> like it's hard for hypervisor to detect this?
> AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets
> the value of 'max_virtqueue_pairs' even before driver negotiated
> VIRTIO_NET_F_MQ. If so, the driver can read the value of
> 'max_virtqueue_pairs' during negotiation and potentially this
> value can even affect negotiation decision of the driver.  

Looks not, it's legal to negotiate VIRTIO_NET_F_MQ with
max_virtqueue_pairs = 1.

>
> If above is correct, I'll change the description to make this
> point more clear.
>
> Thanks,
> -- Victor

Driver also sets 'mac' before driver negotiated VIRTIO_NET_F_MAC, so I'm
not sure this is really needed.

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 1/2] virtio-net: rephrase devconf fields description
  2015-08-19 11:54     ` Victor Kaplansky
  2015-08-20  2:46       ` Jason Wang
@ 2015-08-20  2:46       ` Jason Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-20  2:46 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, netdev, virtualization, mst



On 08/19/2015 07:54 PM, Victor Kaplansky wrote:
> On Mon, Aug 17, 2015 at 10:43:46AM +0800, Jason Wang wrote:
>>
>> 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@redhat.com>
>>> ---
>>> +
>>> +\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.
>>> +
>>
>> I don't get the point that adding "can be read by the driver". Looks
>> like it's hard for hypervisor to detect this?
> AFAIU, if the device sets VIRTIO_NET_F_MQ, the device also sets
> the value of 'max_virtqueue_pairs' even before driver negotiated
> VIRTIO_NET_F_MQ. If so, the driver can read the value of
> 'max_virtqueue_pairs' during negotiation and potentially this
> value can even affect negotiation decision of the driver.  

Looks not, it's legal to negotiate VIRTIO_NET_F_MQ with
max_virtqueue_pairs = 1.

>
> If above is correct, I'll change the description to make this
> point more clear.
>
> Thanks,
> -- Victor

Driver also sets 'mac' before driver negotiated VIRTIO_NET_F_MAC, so I'm
not sure this is really needed.

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-19 11:31     ` Victor Kaplansky
@ 2015-08-20  2:48       ` Jason Wang
  2015-08-20  2:48       ` Jason Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-20  2:48 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, mst, netdev, virtualization



On 08/19/2015 07:31 PM, Victor Kaplansky wrote:
> On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote:
>>
>> On 08/16/2015 09:42 PM, Victor Kaplansky wrote:
>>> @@ -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".
> Good point. I'll change the name in the next version of the patch.
>
>>>  };
>>>  \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?
> If so, we need to add one more feature bit to confirm the ability
> of the driver to report MTU, or we can weaken the requirement in
> conformance statement and write "the driver may report the MTU".
> What do you say?
>
> -- Victor

Either looks good for me. (Need confirm the read only issue with the
editors of other transport or arch)

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-19 11:31     ` Victor Kaplansky
  2015-08-20  2:48       ` Jason Wang
@ 2015-08-20  2:48       ` Jason Wang
  1 sibling, 0 replies; 18+ messages in thread
From: Jason Wang @ 2015-08-20  2:48 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, netdev, virtualization, mst



On 08/19/2015 07:31 PM, Victor Kaplansky wrote:
> On Mon, Aug 17, 2015 at 11:07:15AM +0800, Jason Wang wrote:
>>
>> On 08/16/2015 09:42 PM, Victor Kaplansky wrote:
>>> @@ -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".
> Good point. I'll change the name in the next version of the patch.
>
>>>  };
>>>  \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?
> If so, we need to add one more feature bit to confirm the ability
> of the driver to report MTU, or we can weaken the requirement in
> conformance statement and write "the driver may report the MTU".
> What do you say?
>
> -- Victor

Either looks good for me. (Need confirm the read only issue with the
editors of other transport or arch)

Thanks

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-16 13:42 ` Victor Kaplansky
  2015-08-17  3:07   ` Jason Wang
  2015-08-20 19:31   ` Flavio Leitner
@ 2015-08-20 19:31   ` Flavio Leitner
  2 siblings, 0 replies; 18+ messages in thread
From: Flavio Leitner @ 2015-08-20 19:31 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, mst, jasowang, netdev, virtualization

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@redhat.com>
> 
>     Signed-off-by: Victor Kaplansky <victork@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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/2] virtio-net: add default_mtu configuration field
  2015-08-16 13:42 ` Victor Kaplansky
  2015-08-17  3:07   ` Jason Wang
@ 2015-08-20 19:31   ` Flavio Leitner
  2015-08-20 19:31   ` Flavio Leitner
  2 siblings, 0 replies; 18+ messages in thread
From: Flavio Leitner @ 2015-08-20 19:31 UTC (permalink / raw)
  To: Victor Kaplansky; +Cc: virtio-dev, netdev, virtualization, mst

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@redhat.com>
> 
>     Signed-off-by: Victor Kaplansky <victork@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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-08-20 19:31 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-16 13:42 [PATCH v2 0/2] virtio-net: default_mtu - new conf. field Victor Kaplansky
2015-08-16 13:42 ` [PATCH v2 1/2] virtio-net: rephrase devconf fields description Victor Kaplansky
2015-08-16 13:42 ` Victor Kaplansky
2015-08-17  2:43   ` Jason Wang
2015-08-17  2:43   ` Jason Wang
2015-08-19 11:54     ` Victor Kaplansky
2015-08-19 11:54     ` Victor Kaplansky
2015-08-20  2:46       ` Jason Wang
2015-08-20  2:46       ` Jason Wang
2015-08-16 13:42 ` [PATCH v2 2/2] virtio-net: add default_mtu configuration field Victor Kaplansky
2015-08-16 13:42 ` Victor Kaplansky
2015-08-17  3:07   ` Jason Wang
2015-08-19 11:31     ` Victor Kaplansky
2015-08-20  2:48       ` Jason Wang
2015-08-20  2:48       ` Jason Wang
2015-08-19 11:31     ` Victor Kaplansky
2015-08-20 19:31   ` Flavio Leitner
2015-08-20 19:31   ` Flavio Leitner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.