Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Paolo Bonzini]

Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> From: Dmitry Fleytman <dfleytma@redhat.com>
> 
> Virtio-net driver currently negotiates network offloads
> on startup via features mechanism and have no ability to
> disable and re-enable offloads later.
> This patch introduced a new control command that allows
> to configure device network offloads state dynamically.
> The patch also introduces a new feature flag
> VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> 
> Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> ---
>  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 143 insertions(+)
> 
> diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> index 3d2f485..e9dbac4 100644
> --- a/virtio-spec.lyx
> +++ b/virtio-spec.lyx
> @@ -60,6 +60,7 @@
>  \author -1930653948 "Amos Kong" 
>  \author -608949062 "Rusty Russell,,," 
>  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
>  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
>  \author 1531152142 "Paolo Bonzini,,," 
>  \author 1717892615 "Alexey Zaytsev,,," 
> @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
>  \end_inset
>  
>  (1) Guest handles packets with partial checksum
> +\change_inserted 460276516 1363712169
> +
> +\end_layout
> +
> +\begin_layout Description
> +
> +\change_inserted 460276516 1363712334
> +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> +\begin_inset space ~
> +\end_inset
> +
> +(2) Control channel offloads reconfiguration support.
> +\change_unchanged

Rusty,

you might have missed this patch because your address was misspelled.

Is there a story behind skipping virtio-net feature bits 2..4?

Paolo

>  \end_layout
>  
>  \begin_layout Description
> @@ -5675,6 +5690,134 @@ virtqueue_pairs = 1
>   
>  \end_layout
>  
> +\begin_layout Subsection*
> +
> +\change_inserted 460276516 1363765850
> +Offloads State Configuration
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363765861
> +If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can
> + send control commands for dynamic offloads state configuration.
> +\end_layout
> +
> +\begin_layout Subsubsection*
> +
> +\change_inserted 460276516 1363765928
> +Setting offloads state
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363713225
> +\begin_inset listings
> +inline false
> +status open
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765996
> +
> +u64 offloads;
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765997
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766044
> +
> +#define VIRTIO_NET_F_GUEST_CSUM       1
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766051
> +
> +#define VIRTIO_NET_F_GUEST_TSO4       7
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766055
> +
> +#define VIRTIO_NET_F_GUEST_TSO6       8
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766064
> +
> +#define VIRTIO_NET_F_GUEST_ECN        9
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766035
> +
> +#define VIRTIO_NET_F_GUEST_UFO        10
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363766031
> +
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765865
> +
> +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS       5
> +\end_layout
> +
> +\begin_layout Plain Layout
> +
> +\change_inserted 460276516 1363765867
> +
> + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET          0
> +\end_layout
> +
> +\end_inset
> +
> +
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766082
> +The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET
> + applies the new offloads configuration.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766435
> +u64 value passed as command data is a bitmask, bits set define offloads
> + to be enabled, bits cleared - offloads to be disabled.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766757
> +There is a corresponding device feature for each offload.
> + Upon feature negotiation corresponding offload gets enabled to preserve
> + backward compartibility.
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 460276516 1363766720
> +Corresponding feature must be negotiated at startup in order to allow dynamic
> + change of specific offload state.
> +\end_layout
> +
>  \begin_layout Chapter*
>  Appendix D: Block Device
>  \end_layout
> 

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Michael S. Tsirkin]

On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
> Il 04/04/2013 09:55, Dmitry Fleytman ha scritto:
> > From: Dmitry Fleytman <dfleytma@redhat.com>
> > 
> > Virtio-net driver currently negotiates network offloads
> > on startup via features mechanism and have no ability to
> > disable and re-enable offloads later.
> > This patch introduced a new control command that allows
> > to configure device network offloads state dynamically.
> > The patch also introduces a new feature flag
> > VIRTIO_NET_F_CTRL_GUEST_OFFLOADS.
> > 
> > Signed-off-by: Dmitry Fleytman <dfleytma@redhat.com>
> > ---
> >  virtio-spec.lyx | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 143 insertions(+)
> > 
> > diff --git a/virtio-spec.lyx b/virtio-spec.lyx
> > index 3d2f485..e9dbac4 100644
> > --- a/virtio-spec.lyx
> > +++ b/virtio-spec.lyx
> > @@ -60,6 +60,7 @@
> >  \author -1930653948 "Amos Kong" 
> >  \author -608949062 "Rusty Russell,,," 
> >  \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
> > +\author 460276516 "Dmitry Fleytman" dfleytma@redhat.com
> >  \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
> >  \author 1531152142 "Paolo Bonzini,,," 
> >  \author 1717892615 "Alexey Zaytsev,,," 
> > @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM
> >  \end_inset
> >  
> >  (1) Guest handles packets with partial checksum
> > +\change_inserted 460276516 1363712169
> > +
> > +\end_layout
> > +
> > +\begin_layout Description
> > +
> > +\change_inserted 460276516 1363712334
> > +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> > +\begin_inset space ~
> > +\end_inset
> > +
> > +(2) Control channel offloads reconfiguration support.
> > +\change_unchanged
> 
> Rusty,
> 
> you might have missed this patch because your address was misspelled.

It's commit 721861222802d0ebc48564d727a3684f7b21cadd in
https://github.com/rustyrussell/virtio-spec

> Is there a story behind skipping virtio-net feature bits 2..4?
> 
> Paolo

Bits 3-4 now :)
I'm curious too.

-- 
MST

Re: [Qemu-devel] [PATCH 1/2 V4] virtio-spec: dynamic network offloads configuration

[Rusty Russell]

"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Mon, May 20, 2013 at 03:43:51PM +0200, Paolo Bonzini wrote:
>> Is there a story behind skipping virtio-net feature bits 2..4?
>> 
>> Paolo
>
> Bits 3-4 now :)
> I'm curious too.

Not a good one :)

The year is 2007.  virtio_net was the posterchild of free expression and
crazy experimentation.  We were all so very young...

The original host TSO bits in virtio_net were like so:

 #define VIRTIO_NET_F_TSO4      1
 #define VIRTIO_NET_F_UFO       2
 #define VIRTIO_NET_F_TSO4_ECN  3
 #define VIRTIO_NET_F_TSO6      4

But I decided we might as well offer all or nothing, and if you couldn't
handle one, you could just do it in software:

 #define VIRTIO_NET_F_GSO       6

Reading the git commits, it seems Linux didn't do UFO in software (at
the time, at least), so the bits were split out again, with the explicit
_HOST_ in the names:

 #define VIRTIO_NET_F_HOST_TSO4 11      /* Host can handle TSOv4 in. */
 #define VIRTIO_NET_F_HOST_TSO6 12      /* Host can handle TSOv6 in. */
 #define VIRTIO_NET_F_HOST_ECN  13      /* Host can handle TSO[6] w/ ECN in. */
 #define VIRTIO_NET_F_HOST_UFO  14      /* Host can handle UFO in. */

This was all before there was a host implementation, so we were playing.

ie. the bits are perfectly fine for reuse.

Cheers,
Rusty.