All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v10] virtio-net: add Max MTU configuration field
       [not found] <1473779892-6234-1-git-send-email-aconole@redhat.com>
@ 2016-09-13 15:58 ` Michael S. Tsirkin
  2016-09-13 21:17 ` Michael S. Tsirkin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2016-09-13 15:58 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Victor Kaplansky, virtio-dev, Maxime Coquelin, virtualization

On Tue, Sep 13, 2016 at 11:18:12AM -0400, Aaron Conole wrote:
> It is helpful for a host to indicate it's MTU to be set on guest NICs
> other than the assumed 1500 byte value.  This helps in situations where
> the host network is using Jumbo Frames, or aiding in PMTU discovery by
> configuring a homogenous network.  It is also helpful for sizing receive
> buffers correctly.
> 
> The change adds a new field to configuration area of network
> devices.  It will be used to pass a maximum MTU from the device to
> the driver.  This will be used by the driver as a maximum value for
> packet sizes during transmission, without segmentation offloading.
> 
> In addition, in order to support backward and forward compatibility,
> we introduce a new feature bit called VIRTIO_NET_F_MTU.
> 
> VIRTIO-152
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Cornelia, could you pls ack before I start voting?

> ---
> v1:
> 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.
> 
> v2:
> Rephrase and provide a mechanism for guest->host and host->guest
> communication through a driver read-only and driver write-only field.
> 
> v3:
> Converted to just support initial MTU. Guest->host and Host->guest MTU
> changes are outside the scope of this change.
> 
> v4:
> Removed references to 'initial', since that condition cannot be tested.
> Simply state that if the driver will use the mtu field, it must
> negotiate the feature bit, and if not, it must not.
> 
> v5:
> After feedback from Michael S. Tsirkin
> 
> v6:
> Bit has to change to bit 25 due to an undocumented bit 24 being taken.
> 
> v7:
> Partial rewrite with feedback from MST.  Additional conformance statements
> added.
> 
> v8:
> Clarified the new conformance statements.
> 
> v9:
> Updated the commit log for correctness.  Added ACKs from
> Michael S. Tsirkin, and Maxime Coquelin.  Included the virtio
> issue id.
> 
> v10:
> Update the conformance statement wordings from previous 'offered' form
> to 'succesfully negotiated' form.
> 
>  content.tex | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 4b45678..42c0568 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3049,6 +3049,11 @@ features.
>  \item[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS (2)] Control channel offloads
>          reconfiguration support.
>  
> +\item[VIRTIO_NET_F_MTU(3)] Maximum negotiated MTU is supported. If
> +    offered by the device, device advises driver about the value of
> +    MTU to be used. If negotiated, the driver uses \field{mtu} as
> +    the maximum MTU value supplied to the driver.
> +
>  \item[VIRTIO_NET_F_MAC (5)] Device has given MAC address.
>  
>  \item[VIRTIO_NET_F_GUEST_TSO4 (7)] Driver can receive TSOv4.
> @@ -3140,11 +3145,16 @@ 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.
>  
> +The following driver-read-only field, \field{mtu} only exists if
> +VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> +use.
> +
>  \begin{lstlisting}
>  struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 mtu;
>  };
>  \end{lstlisting}
>  
> @@ -3153,6 +3163,19 @@ struct virtio_net_config {
>  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{mtu} to between 68 and 65535 inclusive,
> +if it offers VIRTIO_NET_F_MTU.
> +
> +The device MUST NOT modify \field{mtu} once it has been set.
> +
> +The device MUST NOT pass received packets that exceed \field{mtu} size
> +with \field{gso_type} NONE or ECN after VIRTIO_NET_F_MTU has been successfully
> +negotiated.
> +
> +The device MUST forward transmitted packets of up to MTU size with
> +\field{gso_type} NONE or ECN, and do so without fragmentation, after
> +VIRTIO_NET_F_MTU has been successfully negotiated.
> +
>  \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.
> @@ -3165,6 +3188,15 @@ 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_MTU if the device offers it.
> +
> +If the driver negotiates VIRTIO_NET_F_MTU, it MUST supply enough receive
> +buffers of size \field{mtu} to be able to receive at least one receive
> +packet with \field{gso_type} NONE or ECN.
> +
> +If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
> +size exceeding the value of \field{mtu} with \field{gso_type} NONE or ECN
> +
>  \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.7.4

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

* Re: [PATCH v10] virtio-net: add Max MTU configuration field
       [not found] <1473779892-6234-1-git-send-email-aconole@redhat.com>
  2016-09-13 15:58 ` [PATCH v10] virtio-net: add Max MTU configuration field Michael S. Tsirkin
@ 2016-09-13 21:17 ` Michael S. Tsirkin
  2016-09-14  6:21 ` Hannes Reinecke
       [not found] ` <20160914000341-mutt-send-email-mst@kernel.org>
  3 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2016-09-13 21:17 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Victor Kaplansky, virtio-dev, Maxime Coquelin, virtualization

On Tue, Sep 13, 2016 at 11:18:12AM -0400, Aaron Conole wrote:
> It is helpful for a host to indicate it's MTU to be set on guest NICs
> other than the assumed 1500 byte value.  This helps in situations where
> the host network is using Jumbo Frames, or aiding in PMTU discovery by
> configuring a homogenous network.  It is also helpful for sizing receive
> buffers correctly.
> 
> The change adds a new field to configuration area of network
> devices.  It will be used to pass a maximum MTU from the device to
> the driver.  This will be used by the driver as a maximum value for
> packet sizes during transmission, without segmentation offloading.
> 
> In addition, in order to support backward and forward compatibility,
> we introduce a new feature bit called VIRTIO_NET_F_MTU.
> 
> VIRTIO-152
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> v1:
> 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.
> 
> v2:
> Rephrase and provide a mechanism for guest->host and host->guest
> communication through a driver read-only and driver write-only field.
> 
> v3:
> Converted to just support initial MTU. Guest->host and Host->guest MTU
> changes are outside the scope of this change.
> 
> v4:
> Removed references to 'initial', since that condition cannot be tested.
> Simply state that if the driver will use the mtu field, it must
> negotiate the feature bit, and if not, it must not.
> 
> v5:
> After feedback from Michael S. Tsirkin
> 
> v6:
> Bit has to change to bit 25 due to an undocumented bit 24 being taken.
> 
> v7:
> Partial rewrite with feedback from MST.  Additional conformance statements
> added.
> 
> v8:
> Clarified the new conformance statements.
> 
> v9:
> Updated the commit log for correctness.  Added ACKs from
> Michael S. Tsirkin, and Maxime Coquelin.  Included the virtio
> issue id.
> 
> v10:
> Update the conformance statement wordings from previous 'offered' form
> to 'succesfully negotiated' form.
> 
>  content.tex | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index 4b45678..42c0568 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3049,6 +3049,11 @@ features.
>  \item[VIRTIO_NET_F_CTRL_GUEST_OFFLOADS (2)] Control channel offloads
>          reconfiguration support.
>  
> +\item[VIRTIO_NET_F_MTU(3)] Maximum negotiated MTU is supported.

I would say "device MTU reporting" is supported.

> If
> +    offered by the device, device advises driver about the value of
> +    MTU to be used.

about its maximum MTU value.

> If negotiated, the driver uses \field{mtu} as
> +    the maximum MTU value supplied to the driver.

Drop "supplied to the driver" here?

> +
>  \item[VIRTIO_NET_F_MAC (5)] Device has given MAC address.
>  
>  \item[VIRTIO_NET_F_GUEST_TSO4 (7)] Driver can receive TSOv4.
> @@ -3140,11 +3145,16 @@ 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.
>  
> +The following driver-read-only field, \field{mtu} only exists if
> +VIRTIO_NET_F_MTU is set. This field specifies the maximum MTU for the driver to
> +use.
> +
>  \begin{lstlisting}
>  struct virtio_net_config {
>          u8 mac[6];
>          le16 status;
>          le16 max_virtqueue_pairs;
> +        le16 mtu;
>  };
>  \end{lstlisting}
>  
> @@ -3153,6 +3163,19 @@ struct virtio_net_config {
>  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{mtu} to between 68 and 65535 inclusive,
> +if it offers VIRTIO_NET_F_MTU.

Apropos, IPv6 requires that every link in the internet have an MTU of
1280 octets or greater.  There are also requirements to be able to
accept 576 byte packets.  Maybe add a SHOULD?

> +
> +The device MUST NOT modify \field{mtu} once it has been set.
> +
> +The device MUST NOT pass received packets that exceed \field{mtu} size
> +with \field{gso_type} NONE or ECN after VIRTIO_NET_F_MTU has been successfully
> +negotiated.
> +
> +The device MUST forward transmitted packets of up to MTU size with
> +\field{gso_type} NONE or ECN, and do so without fragmentation, after
> +VIRTIO_NET_F_MTU has been successfully negotiated.
> +

Maybe clarify that this is MTU + ethernet headers.
E.g. up to MTU size (plus low level headers).

>  \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.
> @@ -3165,6 +3188,15 @@ 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_MTU if the device offers it.
> +
> +If the driver negotiates VIRTIO_NET_F_MTU, it MUST supply enough receive
> +buffers of size \field{mtu}

hmm in fact buffers can and should be bigger, and need to include ethernet headers.
I would say:

> to be able to receive at least one receive
> +packet with \field{gso_type} NONE or ECN.

... of size \field{mtu} (plus low level headers).

> +
> +If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
> +size exceeding the value of \field{mtu} with \field{gso_type} NONE or ECN
> +
>  \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.7.4

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

* Re: [PATCH v10] virtio-net: add Max MTU configuration field
       [not found] <1473779892-6234-1-git-send-email-aconole@redhat.com>
  2016-09-13 15:58 ` [PATCH v10] virtio-net: add Max MTU configuration field Michael S. Tsirkin
  2016-09-13 21:17 ` Michael S. Tsirkin
@ 2016-09-14  6:21 ` Hannes Reinecke
       [not found] ` <20160914000341-mutt-send-email-mst@kernel.org>
  3 siblings, 0 replies; 4+ messages in thread
From: Hannes Reinecke @ 2016-09-14  6:21 UTC (permalink / raw)
  To: Aaron Conole, virtio-dev, virtualization
  Cc: Victor Kaplansky, Maxime Coquelin, Michael S. Tsirkin

On 09/13/2016 05:18 PM, Aaron Conole wrote:
> It is helpful for a host to indicate it's MTU to be set on guest NICs
> other than the assumed 1500 byte value.  This helps in situations where
> the host network is using Jumbo Frames, or aiding in PMTU discovery by
> configuring a homogenous network.  It is also helpful for sizing receive
> buffers correctly.
> 
> The change adds a new field to configuration area of network
> devices.  It will be used to pass a maximum MTU from the device to
> the driver.  This will be used by the driver as a maximum value for
> packet sizes during transmission, without segmentation offloading.
> 
> In addition, in order to support backward and forward compatibility,
> we introduce a new feature bit called VIRTIO_NET_F_MTU.
> 
> VIRTIO-152
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> Cc: Victor Kaplansky <victork@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
Very good. I'm needing this for my FCoE over virtio work.

Reviewed-by: Hannes Reiencke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

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

* Re: [virtio-dev] Re: [PATCH v10] virtio-net: add Max MTU configuration field
       [not found] ` <20160914000341-mutt-send-email-mst@kernel.org>
@ 2016-09-14  7:18   ` Cornelia Huck
  0 siblings, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2016-09-14  7:18 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Victor Kaplansky, virtio-dev, Maxime Coquelin, virtualization

On Wed, 14 Sep 2016 00:17:28 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Sep 13, 2016 at 11:18:12AM -0400, Aaron Conole wrote:

(...)

> > If negotiated, the driver uses \field{mtu} as
> > +    the maximum MTU value supplied to the driver.
> 
> Drop "supplied to the driver" here?

Reword as "If negotiated, the device uses \field{mtu} to supply the
maximum MTU value supported to the driver." ?

I agree with your other comments. Other than that, the update looks
good.

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

end of thread, other threads:[~2016-09-14  7:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1473779892-6234-1-git-send-email-aconole@redhat.com>
2016-09-13 15:58 ` [PATCH v10] virtio-net: add Max MTU configuration field Michael S. Tsirkin
2016-09-13 21:17 ` Michael S. Tsirkin
2016-09-14  6:21 ` Hannes Reinecke
     [not found] ` <20160914000341-mutt-send-email-mst@kernel.org>
2016-09-14  7:18   ` [virtio-dev] " Cornelia Huck

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.