Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Mon, 23 Jul 2018 12:22:38 +0300 "Michael S. Tsirkin" 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" 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 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
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" 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 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 > > > --- > > > > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Sun, 22 Jul 2018 18:37:42 +0300 "Michael S. Tsirkin" 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 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 > > --- > > > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Fri, Jul 20, 2018 at 09:44:50AM +0200, Cornelia Huck wrote: > On Thu, 19 Jul 2018 14:29:40 -0700 > Sridhar Samudrala 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 > --- > > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Fri, 20 Jul 2018 01:41:00 -0700 Siwei Liu wrote: > On Thu, Jul 19, 2018 at 2:29 PM, Sridhar Samudrala > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Thu, Jul 19, 2018 at 2:29 PM, Sridhar Samudrala 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 --- > 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-unsubscr...@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org > - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] [PATCH] content: Introduce VIRTIO_NET_F_STANDBY feature
On Thu, 19 Jul 2018 14:29:40 -0700 Sridhar Samudrala 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 --- > 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-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org