public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] virtio-spec: set mac address by a new vq command
@ 2013-01-16  7:33 akong
  2013-01-16  9:13 ` Stefan Hajnoczi
  0 siblings, 1 reply; 4+ messages in thread
From: akong @ 2013-01-16  7:33 UTC (permalink / raw)
  To: virtualization, rusty; +Cc: kvm, mst

From: Amos Kong <akong@redhat.com>

Virtio-net driver currently programs MAC address byte by byte,
this means that we have an intermediate step where mac is wrong.
This patch introduced a new control command to set MAC address
in one time, and added a new feature flag VIRTIO_NET_F_MAC_ADDR
for this feature.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 virtio-spec.lyx | 45 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/virtio-spec.lyx b/virtio-spec.lyx
index 1ba9992..03f5a49 100644
--- a/virtio-spec.lyx
+++ b/virtio-spec.lyx
@@ -56,6 +56,7 @@
 \html_math_output 0
 \html_css_as_file 0
 \html_be_strict false
+\author -1930653948 "Amos Kong" akong@redhat.com
 \author -608949062 "Rusty Russell,,," 
 \author -385801441 "Cornelia Huck" cornelia.huck@de.ibm.com
 \author 1112500848 "Rusty Russell" rusty@rustcorp.com.au
@@ -4391,6 +4392,14 @@ VIRTIO_NET_F_GUEST_ANNOUNCE(21) Guest can send gratuitous packets.
 
 \change_inserted 1986246365 1352742808
 VIRTIO_NET_F_MQ(22) Device supports multiqueue with automatic receive steering.
+\change_inserted -1930653948 1358319033
+
+\end_layout
+
+\begin_layout Description
+
+\change_inserted -1930653948 1358319080
+VIRTIO_NET_F_CTRL_MAC_ADDR(23) Set MAC address.
 \change_unchanged
 
 \end_layout
@@ -5284,7 +5293,11 @@ The class VIRTIO_NET_CTRL_RX has two commands: VIRTIO_NET_CTRL_RX_PROMISC
 \end_layout
 
 \begin_layout Subsubsection*
-Setting MAC Address Filtering
+Setting MAC Address
+\change_deleted -1930653948 1358318470
+ Filtering
+\change_unchanged
+
 \end_layout
 
 \begin_layout Standard
@@ -5324,6 +5337,17 @@ struct virtio_net_ctrl_mac {
 \begin_layout Plain Layout
 
  #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0 
+\change_inserted -1930653948 1358318313
+
+\end_layout
+
+\begin_layout Plain Layout
+
+\change_inserted -1930653948 1358318331
+
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
+\change_unchanged
+
 \end_layout
 
 \end_inset
@@ -5349,6 +5373,25 @@ T_CTRL_MAC_TABLE_SET.
  The command-specific-data is two variable length tables of 6-byte MAC addresses.
  The first table contains unicast addresses, and the second contains multicast
  addresses.
+\change_inserted -1930653948 1358318545
+
+\end_layout
+
+\begin_layout Standard
+
+\change_inserted -1930653948 1358320004
+The command VIRTIO_NET_CTRL_MAC_ADDR_SET is used to set 
+\begin_inset Quotes eld
+\end_inset
+
+physical
+\begin_inset Quotes erd
+\end_inset
+
+ address of the network card.
+ The command-specific-data is a 6-byte MAC address.
+\change_unchanged
+
 \end_layout
 
 \begin_layout Subsection*
-- 
1.7.11.7

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

* Re: [PATCH] virtio-spec: set mac address by a new vq command
  2013-01-16  7:33 [PATCH] virtio-spec: set mac address by a new vq command akong
@ 2013-01-16  9:13 ` Stefan Hajnoczi
  2013-01-16  9:22   ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Stefan Hajnoczi @ 2013-01-16  9:13 UTC (permalink / raw)
  To: akong; +Cc: mst, kvm, virtualization

On Wed, Jan 16, 2013 at 03:33:24PM +0800, akong@redhat.com wrote:
> +\change_inserted -1930653948 1358320004
> +The command VIRTIO_NET_CTRL_MAC_ADDR_SET is used to set 
> +\begin_inset Quotes eld
> +\end_inset
> +
> +physical
> +\begin_inset Quotes erd
> +\end_inset
> +
> + address of the network card.

The "physical" address of the network card?  That term is not defined
anywhere in the specification.

Perhaps it's best to explain that the config space "mac" field and
VIRTIO_NET_CTRL_MAC_ADDR_SET both set the default MAC address which rx
filtering accepts.  (The MAC table is an additional set of MAC addresses
which rx filtering accepts.)

It would also be worth explaining that VIRTIO_NET_CTRL_MAC_ADDR_SET is
atomic whereas the config space "mac" field is not.  Therefore,
VIRTIO_NET_CTRL_MAC_ADDR_SET is preferred, especially while the NIC is
up.

Stefan

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

* Re: [PATCH] virtio-spec: set mac address by a new vq command
  2013-01-16  9:13 ` Stefan Hajnoczi
@ 2013-01-16  9:22   ` Michael S. Tsirkin
  2013-01-17  6:34     ` Amos Kong
  0 siblings, 1 reply; 4+ messages in thread
From: Michael S. Tsirkin @ 2013-01-16  9:22 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kvm, virtualization

On Wed, Jan 16, 2013 at 10:13:28AM +0100, Stefan Hajnoczi wrote:
> On Wed, Jan 16, 2013 at 03:33:24PM +0800, akong@redhat.com wrote:
> > +\change_inserted -1930653948 1358320004
> > +The command VIRTIO_NET_CTRL_MAC_ADDR_SET is used to set 
> > +\begin_inset Quotes eld
> > +\end_inset
> > +
> > +physical
> > +\begin_inset Quotes erd
> > +\end_inset
> > +
> > + address of the network card.
> 
> The "physical" address of the network card?  That term is not defined
> anywhere in the specification.
> 
> Perhaps it's best to explain that the config space "mac" field and
> VIRTIO_NET_CTRL_MAC_ADDR_SET both set the default MAC address which rx
> filtering accepts.  (The MAC table is an additional set of MAC addresses
> which rx filtering accepts.)
> 
> It would also be worth explaining that VIRTIO_NET_CTRL_MAC_ADDR_SET is
> atomic whereas the config space "mac" field is not.  Therefore,
> VIRTIO_NET_CTRL_MAC_ADDR_SET is preferred, especially while the NIC is
> up.
> 
> Stefan

It's probably best to simply make the config space field
read-only if the feature bit is acked.

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

* Re: [PATCH] virtio-spec: set mac address by a new vq command
  2013-01-16  9:22   ` Michael S. Tsirkin
@ 2013-01-17  6:34     ` Amos Kong
  0 siblings, 0 replies; 4+ messages in thread
From: Amos Kong @ 2013-01-17  6:34 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: kvm, virtualization

On Wed, Jan 16, 2013 at 11:22:23AM +0200, Michael S. Tsirkin wrote:
> On Wed, Jan 16, 2013 at 10:13:28AM +0100, Stefan Hajnoczi wrote:
> > On Wed, Jan 16, 2013 at 03:33:24PM +0800, akong@redhat.com wrote:
> > > +\change_inserted -1930653948 1358320004
> > > +The command VIRTIO_NET_CTRL_MAC_ADDR_SET is used to set 
> > > +\begin_inset Quotes eld
> > > +\end_inset
> > > +
> > > +physical
> > > +\begin_inset Quotes erd
> > > +\end_inset
> > > +
> > > + address of the network card.
> > 
> > The "physical" address of the network card?  That term is not defined
> > anywhere in the specification.

I will replace it with 'the default MAC address'

> > Perhaps it's best to explain that the config space "mac" field and
> > VIRTIO_NET_CTRL_MAC_ADDR_SET both set the default MAC address which rx
> > filtering accepts.  (The MAC table is an additional set of MAC addresses
> > which rx filtering accepts.)
> > 
> > It would also be worth explaining that VIRTIO_NET_CTRL_MAC_ADDR_SET is
> > atomic whereas the config space "mac" field is not.  Therefore,
> > VIRTIO_NET_CTRL_MAC_ADDR_SET is preferred, especially while the NIC is
> > up.

Ok, will send a v2, thanks.

> > Stefan
 
> It's probably best to simply make the config space field
> read-only if the feature bit is acked.

It's reasonable.

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index c18e276..54c5eae 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -93,7 +93,8 @@ static void virtio_net_set_config(VirtIODevice
*vdev, const uint8_t *config)
 
     memcpy(&netcfg, config, sizeof(netcfg));
 
-    if (memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
+    if (!(n->vdev.guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) &&
+        memcmp(netcfg.mac, n->mac, ETH_ALEN)) {
         memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(&n->nic->nc, n->mac);
     }

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

end of thread, other threads:[~2013-01-17  6:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-16  7:33 [PATCH] virtio-spec: set mac address by a new vq command akong
2013-01-16  9:13 ` Stefan Hajnoczi
2013-01-16  9:22   ` Michael S. Tsirkin
2013-01-17  6:34     ` Amos Kong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox