[virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Sridhar Samudrala]

VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
driver to act as a standby for another device with the same MAC address.

Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
---
 content.tex | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/content.tex b/content.tex
index be18234..010b6ee 100644
--- a/content.tex
+++ b/content.tex
@@ -2525,6 +2525,9 @@ features.
 
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
+
+\item[VIRTIO_NET_F_STANDBY(62)] Driver acts as standby for another
+    device with the same MAC
 \end{description}
 
 \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
@@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
 size exceeding the value of \field{mtu} (plus low level ethernet header length)
 with \field{gso_type} NONE or ECN.
 
+A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.
+
+If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby for
+another device with the same MAC address when available. The hypervisor can
+hot-plug a primary device with same MAC address if the feature is successfully
+negotiated with the driver.
+
 \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.14.4


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Cornelia Huck]

On Thu, 19 Jul 2018 14:29:40 -0700
Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:

> VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> driver to act as a standby for another device with the same MAC address.
> 
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
> ---
>  content.tex | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/content.tex b/content.tex
> index be18234..010b6ee 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2525,6 +2525,9 @@ features.
>  
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_STANDBY(62)] Driver acts as standby for another
> +    device with the same MAC

Isn't it the _device_ that acts as the standby?

>  \end{description}
>  
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> @@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
>  size exceeding the value of \field{mtu} (plus low level ethernet header length)
>  with \field{gso_type} NONE or ECN.
>  
> +A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.

I'm not sure that this is the right section of the spec. Maybe we need
a new normative driver section for "cross-device features" (better
names wanted :), and the same for devices?

> +
> +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby for
> +another device with the same MAC address when available. The hypervisor can
> +hot-plug a primary device with same MAC address if the feature is successfully
> +negotiated with the driver.

I don't think you should add implementation details like hotplugging
into the spec.

What about:

"If the driver negotiates VIRTIO_NET_F_STANDBY, the device MAY act as a
standby device for another device with the same MAC address, the
'failover device'. The 'failover device' MUST NOT [or maybe SHOULD
NOT?] be accessible if VIRTIO_NET_F_STANDBY has not been negotiated by
the driver."

One thing that makes defining the behaviour of this feature bit a bit
difficult is that you need to describe both what the virtio device and
driver are supposed to be doing (which is what this spec is all about)
and also the behaviour of the hypervisor. Maybe it would be good to keep
the normative statements, and add an explanatory section describing in
more detail what the hypervisor is to do and what the guest can expect?

> +
>  \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


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Michael S. Tsirkin]

On Fri, Jul 20, 2018 at 09:44:50AM +0200, Cornelia Huck wrote:
> On Thu, 19 Jul 2018 14:29:40 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> 
> > VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> > driver to act as a standby for another device with the same MAC address.
> > 
> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
> > ---
> >  content.tex | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/content.tex b/content.tex
> > index be18234..010b6ee 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -2525,6 +2525,9 @@ features.
> >  
> >  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
> >      channel.
> > +
> > +\item[VIRTIO_NET_F_STANDBY(62)] Driver acts as standby for another
> > +    device with the same MAC
> 
> Isn't it the _device_ that acts as the standby?

I think so, yes.


> >  \end{description}
> >  
> >  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> > @@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
> >  size exceeding the value of \field{mtu} (plus low level ethernet header length)
> >  with \field{gso_type} NONE or ECN.
> >  
> > +A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.
> 
> I'm not sure that this is the right section of the spec. Maybe we need
> a new normative driver section for "cross-device features" (better
> names wanted :), and the same for devices?

You mean if we ever extend this to non-network devices?
This is a network specific feature bit so what makes
it a cross-device feature?

> > +
> > +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby
> for
> > +another device with the same MAC address when available. The hypervisor can
> > +hot-plug a primary device with same MAC address if the feature is successfully
> > +negotiated with the driver.
> 
> I don't think you should add implementation details like hotplugging
> into the spec.
> 
> What about:
> 
> "If the driver negotiates VIRTIO_NET_F_STANDBY, the device MAY act as a
> standby device for another device with the same MAC address, the
> 'failover device'.

I find the name "failover device" confusing. Linux came up with
names primary and standby.

> The 'failover device' MUST NOT [or maybe SHOULD
> NOT?

Maybe should not, since virtio can be removed, there's a window
where device is still accessible.

>] be accessible if VIRTIO_NET_F_STANDBY has not been negotiated by
> the driver."
> 
> One thing that makes defining the behaviour of this feature bit a bit
> difficult is that you need to describe both what the virtio device and
> driver are supposed to be doing (which is what this spec is all about)
> and also the behaviour of the hypervisor. Maybe it would be good to keep
> the normative statements, and add an explanatory section describing in
> more detail what the hypervisor is to do and what the guest can expect?

Yes I think we need a separate section describing the failover
operation. All that can be described there.

> 
> > +
> >  \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

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Cornelia Huck]

On Sun, 22 Jul 2018 18:37:42 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Jul 20, 2018 at 09:44:50AM +0200, Cornelia Huck wrote:
> > On Thu, 19 Jul 2018 14:29:40 -0700
> > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> >   
> > > VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> > > driver to act as a standby for another device with the same MAC address.
> > > 
> > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
> > > ---
> > >  content.tex | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)

> > > @@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
> > >  size exceeding the value of \field{mtu} (plus low level ethernet header length)
> > >  with \field{gso_type} NONE or ECN.
> > >  
> > > +A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.  
> > 
> > I'm not sure that this is the right section of the spec. Maybe we need
> > a new normative driver section for "cross-device features" (better
> > names wanted :), and the same for devices?  
> 
> You mean if we ever extend this to non-network devices?
> This is a network specific feature bit so what makes
> it a cross-device feature?

No, the network device location is alright. But it is in a section that
deals with how the config space etc. is handled, and I'm not sure
whether it fits well with the other statements in there. Not a major
gripe, though.

> 
> > > +
> > > +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby  
> > for  
> > > +another device with the same MAC address when available. The hypervisor can
> > > +hot-plug a primary device with same MAC address if the feature is successfully
> > > +negotiated with the driver.  
> > 
> > I don't think you should add implementation details like hotplugging
> > into the spec.
> > 
> > What about:
> > 
> > "If the driver negotiates VIRTIO_NET_F_STANDBY, the device MAY act as a
> > standby device for another device with the same MAC address, the
> > 'failover device'.  
> 
> I find the name "failover device" confusing. Linux came up with
> names primary and standby.

"...the device MAY act as a standby device for a primary device with
the same MAC address."

?

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Michael S. Tsirkin]

On Mon, Jul 23, 2018 at 11:03:11AM +0200, Cornelia Huck wrote:
> On Sun, 22 Jul 2018 18:37:42 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Fri, Jul 20, 2018 at 09:44:50AM +0200, Cornelia Huck wrote:
> > > On Thu, 19 Jul 2018 14:29:40 -0700
> > > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
> > >   
> > > > VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> > > > driver to act as a standby for another device with the same MAC address.
> > > > 
> > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
> > > > ---
> > > >  content.tex | 10 ++++++++++
> > > >  1 file changed, 10 insertions(+)
> 
> > > > @@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
> > > >  size exceeding the value of \field{mtu} (plus low level ethernet header length)
> > > >  with \field{gso_type} NONE or ECN.
> > > >  
> > > > +A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.  
> > > 
> > > I'm not sure that this is the right section of the spec. Maybe we need
> > > a new normative driver section for "cross-device features" (better
> > > names wanted :), and the same for devices?  
> > 
> > You mean if we ever extend this to non-network devices?
> > This is a network specific feature bit so what makes
> > it a cross-device feature?
> 
> No, the network device location is alright. But it is in a section that
> deals with how the config space etc. is handled, and I'm not sure
> whether it fits well with the other statements in there. Not a major
> gripe, though.
> 
> > 
> > > > +
> > > > +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby  
> > > for  
> > > > +another device with the same MAC address when available. The hypervisor can
> > > > +hot-plug a primary device with same MAC address if the feature is successfully
> > > > +negotiated with the driver.  
> > > 
> > > I don't think you should add implementation details like hotplugging
> > > into the spec.
> > > 
> > > What about:
> > > 
> > > "If the driver negotiates VIRTIO_NET_F_STANDBY, the device MAY act as a
> > > standby device for another device with the same MAC address, the
> > > 'failover device'.  
> > 
> > I find the name "failover device" confusing. Linux came up with
> > names primary and standby.
> 
> "...the device MAY act as a standby device for a primary device with
> the same MAC address."
> 
> ?

Right. But I agree we need a section that defines failover operation.

-- 
MST

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Cornelia Huck]

On Mon, 23 Jul 2018 12:22:38 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jul 23, 2018 at 11:03:11AM +0200, Cornelia Huck wrote:
> > On Sun, 22 Jul 2018 18:37:42 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >   
> > > On Fri, Jul 20, 2018 at 09:44:50AM +0200, Cornelia Huck wrote:  
> > > > On Thu, 19 Jul 2018 14:29:40 -0700
> > > > Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:

> > > > > +
> > > > > +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby    
> > > > for    
> > > > > +another device with the same MAC address when available. The hypervisor can
> > > > > +hot-plug a primary device with same MAC address if the feature is successfully
> > > > > +negotiated with the driver.    
> > > > 
> > > > I don't think you should add implementation details like hotplugging
> > > > into the spec.
> > > > 
> > > > What about:
> > > > 
> > > > "If the driver negotiates VIRTIO_NET_F_STANDBY, the device MAY act as a
> > > > standby device for another device with the same MAC address, the
> > > > 'failover device'.    
> > > 
> > > I find the name "failover device" confusing. Linux came up with
> > > names primary and standby.  
> > 
> > "...the device MAY act as a standby device for a primary device with
> > the same MAC address."
> > 
> > ?  
> 
> Right. But I agree we need a section that defines failover operation.

The basics for VIRTIO_NET_F_STANDBY are probably the most pressing
right now, we can still fill in the detailed description when we have
figured out whether the suggested approach actually works (it seems
there haven't been any QEMU patches implementing the visibility
handling yet?)

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Siwei Liu]

On Thu, Jul 19, 2018 at 2:29 PM, Sridhar Samudrala
<sridhar.samudrala@intel.com> wrote:
> VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> driver to act as a standby for another device with the same MAC address.
You should use another feature bit to represent the match-by-MAC
grouping. I think VIRTIO_NET_F_STANDBY is more of a failover concept
and shouldn't marry to any grouping mechanism. What you propose simply
kills off the possibility of introducing other grouping mechanisms
which don't rely on MAC address at all.

-Siwei

>
> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com
> ---
>  content.tex | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/content.tex b/content.tex
> index be18234..010b6ee 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -2525,6 +2525,9 @@ features.
>
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
> +
> +\item[VIRTIO_NET_F_STANDBY(62)] Driver acts as standby for another
> +    device with the same MAC
>  \end{description}
>
>  \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device / Feature bits / Feature bit requirements}
> @@ -2636,6 +2639,13 @@ If the driver negotiates VIRTIO_NET_F_MTU, it MUST NOT transmit packets of
>  size exceeding the value of \field{mtu} (plus low level ethernet header length)
>  with \field{gso_type} NONE or ECN.
>
> +A driver SHOULD negotiate VIRTIO_NET_F_STANDBY feature if the device offers it.
> +
> +If the driver negotiates VIRTIO_NET_F_STANDBY, it should act as a standby for
> +another device with the same MAC address when available. The hypervisor can
> +hot-plug a primary device with same MAC address if the feature is successfully
> +negotiated with the driver.
> +
>  \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.14.4
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature

[Cornelia Huck]

On Fri, 20 Jul 2018 01:41:00 -0700
Siwei Liu <loseweigh@gmail.com> wrote:

> On Thu, Jul 19, 2018 at 2:29 PM, Sridhar Samudrala
> <sridhar.samudrala@intel.com> wrote:
> > VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> > driver to act as a standby for another device with the same MAC address.  
> You should use another feature bit to represent the match-by-MAC
> grouping. I think VIRTIO_NET_F_STANDBY is more of a failover concept
> and shouldn't marry to any grouping mechanism. What you propose simply
> kills off the possibility of introducing other grouping mechanisms
> which don't rely on MAC address at all.

I disagree. VIRTIO_NET_F_STANDBY as used by the Linux driver today does
indicate matching by MAC, so it makes sense for the spec to define it
that way.

We should use new feature bits for any alternative matching mechanisms.
We probably can either
- introduce new features (VIRTIO_NET_F_STANDBY_UUID or so) for any
  distinct matching mechanism, or
- introduce VIRTIO_NET_F_STANDBY2 (for the concept) and different
  features for different matching mechanisms (including by MAC) which
  depend on it.

The problem is that we already have code around that relies on the
semantic of "VIRTIO_NET_F_STANDBY gets you a standby device with the
same MAC", so we can't just redefine it in the spec. An unfortunate
effect of merging something that does not have a proper spec and not
even an agreed semantic :/

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org