| Message ID | 1358317007-29972-1-git-send-email-akong@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, January 16, 2013 02:16:47 PM akong@redhat.com wrote: > From: Amos Kong <akong@redhat.com> > > In virtio-net guest driver, currently we write MAC address to > pci config space 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. > > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > V2: check guest's iov_len before memcpy > --- > hw/virtio-net.c | 10 ++++++++++ > hw/virtio-net.h | 9 ++++++++- > 2 files changed, 18 insertions(+), 1 deletion(-) > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index dc7c6d6..d05f98f 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice > *vdev, uint32_t features) VirtIONet *n = to_virtio_net(vdev); > > features |= (1 << VIRTIO_NET_F_MAC); > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > if (!peer_has_vnet_hdr(n)) { > features &= ~(0x1 << VIRTIO_NET_F_CSUM); > @@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice > *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but > also these: */ > features |= (1 << VIRTIO_NET_F_MAC); > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > features |= (1 << VIRTIO_NET_F_CSUM); > features |= (1 << VIRTIO_NET_F_HOST_TSO4); > features |= (1 << VIRTIO_NET_F_HOST_TSO6); > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t > cmd, { > struct virtio_net_ctrl_mac mac_data; > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > + elem->out_sg[1].iov_len == ETH_ALEN) { > + /* Set MAC address */ > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > + return VIRTIO_NET_OK; > + } > + > if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || > elem->out_sg[1].iov_len < sizeof(mac_data) || > elem->out_sg[2].iov_len < sizeof(mac_data)) > diff --git a/hw/virtio-net.h b/hw/virtio-net.h > index d46fb98..9394cc0 100644 > --- a/hw/virtio-net.h > +++ b/hw/virtio-net.h > @@ -44,6 +44,8 @@ > #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering > */ #define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support > */ > > +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > + I wonder whether we need a DEFINE_PROP_BIT to disable and compat this feature. Consider we may migrate from a new version to an old version. > #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ > > #define TX_TIMER_INTERVAL 150000 /* 150 us */ > @@ -106,7 +108,7 @@ typedef uint8_t virtio_net_ctrl_ack; > #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5 > > /* > - * Control the MAC filter table. > + * Control the MAC > * > * The MAC filter table is managed by the hypervisor, the guest should > * assume the size is infinite. Filtering should be considered > @@ -119,6 +121,10 @@ typedef uint8_t virtio_net_ctrl_ack; > * first sg list contains unicast addresses, the second is for multicast. > * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature > * is available. > + * > + * The ADDR_SET command requests one out scatterlist, it contains a > + * 6 bytes MAC address. This functionality is present if the > + * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available. > */ > struct virtio_net_ctrl_mac { > uint32_t entries; > @@ -126,6 +132,7 @@ struct virtio_net_ctrl_mac { > }; > #define VIRTIO_NET_CTRL_MAC 1 > #define VIRTIO_NET_CTRL_MAC_TABLE_SET 0 > + #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1 > > /* > * Control VLAN filtering -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 02:37:34PM +0800, Jason Wang wrote: > On Wednesday, January 16, 2013 02:16:47 PM akong@redhat.com wrote: > > From: Amos Kong <akong@redhat.com> > > > > In virtio-net guest driver, currently we write MAC address to > > pci config space 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. > > > > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > V2: check guest's iov_len before memcpy > > --- > > hw/virtio-net.c | 10 ++++++++++ > > hw/virtio-net.h | 9 ++++++++- > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > index dc7c6d6..d05f98f 100644 > > --- a/hw/virtio-net.c > > +++ b/hw/virtio-net.c > > @@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice > > *vdev, uint32_t features) VirtIONet *n = to_virtio_net(vdev); > > > > features |= (1 << VIRTIO_NET_F_MAC); > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > > > if (!peer_has_vnet_hdr(n)) { > > features &= ~(0x1 << VIRTIO_NET_F_CSUM); > > @@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice > > *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but > > also these: */ > > features |= (1 << VIRTIO_NET_F_MAC); > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > features |= (1 << VIRTIO_NET_F_CSUM); > > features |= (1 << VIRTIO_NET_F_HOST_TSO4); > > features |= (1 << VIRTIO_NET_F_HOST_TSO6); > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t > > cmd, { > > struct virtio_net_ctrl_mac mac_data; > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > + /* Set MAC address */ > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > + return VIRTIO_NET_OK; > > + } > > + > > if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || > > elem->out_sg[1].iov_len < sizeof(mac_data) || > > elem->out_sg[2].iov_len < sizeof(mac_data)) > > diff --git a/hw/virtio-net.h b/hw/virtio-net.h > > index d46fb98..9394cc0 100644 > > --- a/hw/virtio-net.h > > +++ b/hw/virtio-net.h > > @@ -44,6 +44,8 @@ > > #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering > > */ #define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support > > */ > > > > +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > + > > I wonder whether we need a DEFINE_PROP_BIT to disable and compat this feature. > Consider we may migrate from a new version to an old version. Yes I think we do. > > #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ > > > > #define TX_TIMER_INTERVAL 150000 /* 150 us */ > > @@ -106,7 +108,7 @@ typedef uint8_t virtio_net_ctrl_ack; > > #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5 > > > > /* > > - * Control the MAC filter table. > > + * Control the MAC > > * > > * The MAC filter table is managed by the hypervisor, the guest should > > * assume the size is infinite. Filtering should be considered > > @@ -119,6 +121,10 @@ typedef uint8_t virtio_net_ctrl_ack; > > * first sg list contains unicast addresses, the second is for multicast. > > * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature > > * is available. > > + * > > + * The ADDR_SET command requests one out scatterlist, it contains a > > + * 6 bytes MAC address. This functionality is present if the > > + * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available. > > */ > > struct virtio_net_ctrl_mac { > > uint32_t entries; > > @@ -126,6 +132,7 @@ struct virtio_net_ctrl_mac { > > }; > > #define VIRTIO_NET_CTRL_MAC 1 > > #define VIRTIO_NET_CTRL_MAC_TABLE_SET 0 > > + #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1 > > > > /* > > * Control VLAN filtering -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 02:37:34PM +0800, Jason Wang wrote: > On Wednesday, January 16, 2013 02:16:47 PM akong@redhat.com wrote: > > From: Amos Kong <akong@redhat.com> > > > > In virtio-net guest driver, currently we write MAC address to > > pci config space 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. > > > > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > V2: check guest's iov_len before memcpy > > --- > > hw/virtio-net.c | 10 ++++++++++ > > hw/virtio-net.h | 9 ++++++++- > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > index dc7c6d6..d05f98f 100644 > > --- a/hw/virtio-net.c > > +++ b/hw/virtio-net.c > > @@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice > > *vdev, uint32_t features) VirtIONet *n = to_virtio_net(vdev); > > > > features |= (1 << VIRTIO_NET_F_MAC); > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > > > if (!peer_has_vnet_hdr(n)) { > > features &= ~(0x1 << VIRTIO_NET_F_CSUM); > > @@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice > > *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but > > also these: */ > > features |= (1 << VIRTIO_NET_F_MAC); > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > features |= (1 << VIRTIO_NET_F_CSUM); > > features |= (1 << VIRTIO_NET_F_HOST_TSO4); > > features |= (1 << VIRTIO_NET_F_HOST_TSO6); > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t > > cmd, { > > struct virtio_net_ctrl_mac mac_data; > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > + /* Set MAC address */ > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > + return VIRTIO_NET_OK; > > + } > > + > > if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || > > elem->out_sg[1].iov_len < sizeof(mac_data) || > > elem->out_sg[2].iov_len < sizeof(mac_data)) > > diff --git a/hw/virtio-net.h b/hw/virtio-net.h > > index d46fb98..9394cc0 100644 > > --- a/hw/virtio-net.h > > +++ b/hw/virtio-net.h > > @@ -44,6 +44,8 @@ > > #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering > > */ #define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support > > */ > > > > +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > + > > I wonder whether we need a DEFINE_PROP_BIT to disable and compat this feature. > Consider we may migrate from a new version to an old version. I agree, migration needs to be handled. The bit should never change while the device is initialized and running. We should also never start rejecting or ignoring the command if it was available before. Stefan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 09:59:14AM +0100, Stefan Hajnoczi wrote: > On Wed, Jan 16, 2013 at 02:37:34PM +0800, Jason Wang wrote: > > On Wednesday, January 16, 2013 02:16:47 PM akong@redhat.com wrote: > > > From: Amos Kong <akong@redhat.com> > > > > > > In virtio-net guest driver, currently we write MAC address to > > > pci config space 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. > > > > > > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility. > > > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > > --- > > > V2: check guest's iov_len before memcpy > > > --- > > > hw/virtio-net.c | 10 ++++++++++ > > > hw/virtio-net.h | 9 ++++++++- > > > 2 files changed, 18 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > > index dc7c6d6..d05f98f 100644 > > > --- a/hw/virtio-net.c > > > +++ b/hw/virtio-net.c > > > @@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice > > > *vdev, uint32_t features) VirtIONet *n = to_virtio_net(vdev); > > > > > > features |= (1 << VIRTIO_NET_F_MAC); > > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > > > > > if (!peer_has_vnet_hdr(n)) { > > > features &= ~(0x1 << VIRTIO_NET_F_CSUM); > > > @@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice > > > *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but > > > also these: */ > > > features |= (1 << VIRTIO_NET_F_MAC); > > > + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); > > > features |= (1 << VIRTIO_NET_F_CSUM); > > > features |= (1 << VIRTIO_NET_F_HOST_TSO4); > > > features |= (1 << VIRTIO_NET_F_HOST_TSO6); > > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t > > > cmd, { > > > struct virtio_net_ctrl_mac mac_data; > > > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > > + /* Set MAC address */ > > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > > + return VIRTIO_NET_OK; > > > + } > > > + > > > if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || > > > elem->out_sg[1].iov_len < sizeof(mac_data) || > > > elem->out_sg[2].iov_len < sizeof(mac_data)) > > > diff --git a/hw/virtio-net.h b/hw/virtio-net.h > > > index d46fb98..9394cc0 100644 > > > --- a/hw/virtio-net.h > > > +++ b/hw/virtio-net.h > > > @@ -44,6 +44,8 @@ > > > #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering > > > */ #define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support > > > */ > > > > > > +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ > > > + > > > > I wonder whether we need a DEFINE_PROP_BIT to disable and compat this feature. > > Consider we may migrate from a new version to an old version. > > I agree, migration needs to be handled. The bit should never change > while the device is initialized and running. We should also never start > rejecting or ignoring the command if it was available before. > > Stefan It's the same for all feature bits. Soo all we need to do is clear them when running with a compat machine type. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
akong@redhat.com writes: > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > { > struct virtio_net_ctrl_mac mac_data; > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > + elem->out_sg[1].iov_len == ETH_ALEN) { > + /* Set MAC address */ > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > + return VIRTIO_NET_OK; > + } Does the rest of the net device still rely on the layout of descriptors? If so, OK, we'll fix them all together. If not, this introduces a new one. Cheers, Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote: > akong@redhat.com writes: > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > { > > struct virtio_net_ctrl_mac mac_data; > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > + /* Set MAC address */ > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > + return VIRTIO_NET_OK; > > + } > > Does the rest of the net device still rely on the layout of descriptors? No, only info string of net client relies on n->mac > If so, OK, we'll fix them all together. If not, this introduces a new > one. > > Cheers, > Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 17, 2013 at 01:45:11PM +0800, Amos Kong wrote: > On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote: > > akong@redhat.com writes: > > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > > { > > > struct virtio_net_ctrl_mac mac_data; > > > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > > + /* Set MAC address */ > > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > > + return VIRTIO_NET_OK; > > > + } > > > > Does the rest of the net device still rely on the layout of descriptors? > > No, only info string of net client relies on n->mac I misunderstood. There is no clear limitation of how much descriptor are used for each vq command, but many commands rely on the layout of descriptiors. eg: virtio-net: VIRTIO_NET_CTRL_RX_PROMISC VIRTIO_NET_CTRL_RX_ALLMULTI VIRTIO_NET_CTRL_MAC_TABLE_SET etc > > If so, OK, we'll fix them all together. If not, this introduces a new > > one. > > > > Cheers, > > Rusty. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 17, 2013 at 01:45:11PM +0800, Amos Kong wrote: > On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote: > > akong@redhat.com writes: > > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > > { > > > struct virtio_net_ctrl_mac mac_data; > > > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > > + /* Set MAC address */ > > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > > + return VIRTIO_NET_OK; > > > + } > > > > Does the rest of the net device still rely on the layout of descriptors? > > No, only info string of net client relies on n->mac I think the question is whether the hw/virtio-net.c code makes assumptions about virtqueue descriptor layout (e.g. sg[0] is the header, sg[1] is the data buffer). The answer is yes, the control virtqueue function directly accesses iov[n]. Additional patches would be required to convert the existing hw/virtio-net.c code to make no assumptions about virtqueue descriptor layout. It's outside the scope of this series. Stefan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jan 17, 2013 at 09:39:54AM +0100, Stefan Hajnoczi wrote: > On Thu, Jan 17, 2013 at 01:45:11PM +0800, Amos Kong wrote: > > On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote: > > > akong@redhat.com writes: > > > > @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > > > { > > > > struct virtio_net_ctrl_mac mac_data; > > > > > > > > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && > > > > + elem->out_sg[1].iov_len == ETH_ALEN) { > > > > + /* Set MAC address */ > > > > + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); > > > > + qemu_format_nic_info_str(&n->nic->nc, n->mac); > > > > + return VIRTIO_NET_OK; > > > > + } > > > > > > Does the rest of the net device still rely on the layout of descriptors? > > > > No, only info string of net client relies on n->mac > > I think the question is whether the hw/virtio-net.c code makes > assumptions about virtqueue descriptor layout (e.g. sg[0] is the header, > sg[1] is the data buffer). > > The answer is yes, the control virtqueue function directly accesses > iov[n]. > > Additional patches would be required to convert the existing > hw/virtio-net.c code to make no assumptions about virtqueue descriptor > layout. It's outside the scope of this series. > > Stefan It's not hard at all though - the harder part is data path processing, this has been done already. Will send a patch shortly. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/hw/virtio-net.c b/hw/virtio-net.c index dc7c6d6..d05f98f 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -247,6 +247,7 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) VirtIONet *n = to_virtio_net(vdev); features |= (1 << VIRTIO_NET_F_MAC); + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); if (!peer_has_vnet_hdr(n)) { features &= ~(0x1 << VIRTIO_NET_F_CSUM); @@ -282,6 +283,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but also these: */ features |= (1 << VIRTIO_NET_F_MAC); + features |= (1 << VIRTIO_NET_F_CTRL_MAC_ADDR); features |= (1 << VIRTIO_NET_F_CSUM); features |= (1 << VIRTIO_NET_F_HOST_TSO4); features |= (1 << VIRTIO_NET_F_HOST_TSO6); @@ -349,6 +351,14 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, { struct virtio_net_ctrl_mac mac_data; + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET && elem->out_num == 2 && + elem->out_sg[1].iov_len == ETH_ALEN) { + /* Set MAC address */ + memcpy(n->mac, elem->out_sg[1].iov_base, elem->out_sg[1].iov_len); + qemu_format_nic_info_str(&n->nic->nc, n->mac); + return VIRTIO_NET_OK; + } + if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || elem->out_sg[1].iov_len < sizeof(mac_data) || elem->out_sg[2].iov_len < sizeof(mac_data)) diff --git a/hw/virtio-net.h b/hw/virtio-net.h index d46fb98..9394cc0 100644 --- a/hw/virtio-net.h +++ b/hw/virtio-net.h @@ -44,6 +44,8 @@ #define VIRTIO_NET_F_CTRL_VLAN 19 /* Control channel VLAN filtering */ #define VIRTIO_NET_F_CTRL_RX_EXTRA 20 /* Extra RX mode control support */ +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */ + #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */ #define TX_TIMER_INTERVAL 150000 /* 150 us */ @@ -106,7 +108,7 @@ typedef uint8_t virtio_net_ctrl_ack; #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5 /* - * Control the MAC filter table. + * Control the MAC * * The MAC filter table is managed by the hypervisor, the guest should * assume the size is infinite. Filtering should be considered @@ -119,6 +121,10 @@ typedef uint8_t virtio_net_ctrl_ack; * first sg list contains unicast addresses, the second is for multicast. * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature * is available. + * + * The ADDR_SET command requests one out scatterlist, it contains a + * 6 bytes MAC address. This functionality is present if the + * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available. */ struct virtio_net_ctrl_mac { uint32_t entries; @@ -126,6 +132,7 @@ struct virtio_net_ctrl_mac { }; #define VIRTIO_NET_CTRL_MAC 1 #define VIRTIO_NET_CTRL_MAC_TABLE_SET 0 + #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1 /* * Control VLAN filtering