Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Thu, Jan 17, 2013 at 02:13:08PM +0200, Michael S. Tsirkin wrote:
> On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote:
> > ak...@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.
>
> The following fixes all existing users.
> Got to deal with some urgent stuff so did not test yet -
> Amos, would you like to include this in your patchset
> and build on it, test it all together?
No problem.
> If not I'll get to it next week.
>
> --->
>
> virtio-net: remove layout assumptions for ctrl vq
>
> Signed-off-by: Michael S. Tsirkin
...
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote:
> ak...@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.
The following fixes all existing users.
Got to deal with some urgent stuff so did not test yet -
Amos, would you like to include this in your patchset
and build on it, test it all together?
If not I'll get to it next week.
--->
virtio-net: remove layout assumptions for ctrl vq
Signed-off-by: Michael S. Tsirkin
---
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 4d80a25..5d1e084 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -316,44 +316,44 @@ static void virtio_net_set_features(VirtIODevice *vdev,
uint32_t features)
}
static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
{
uint8_t on;
+size_t s;
-if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) {
-error_report("virtio-net ctrl invalid rx mode command");
-exit(1);
+s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof on);
+if (s != sizeof on) {
+return VIRTIO_NET_ERR;
}
-on = ldub_p(elem->out_sg[1].iov_base);
-
-if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC)
+if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
n->promisc = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
n->allmulti = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
n->alluni = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
n->nomulti = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
n->nouni = on;
-else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST)
+} else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
n->nobcast = on;
-else
+} else {
return VIRTIO_NET_ERR;
+}
return VIRTIO_NET_OK;
}
static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
{
struct virtio_net_ctrl_mac mac_data;
+size_t s;
-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))
+if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
return VIRTIO_NET_ERR;
+}
n->mac_table.in_use = 0;
n->mac_table.first_multi = 0;
@@ -361,54 +361,64 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t
cmd,
n->mac_table.multi_overflow = 0;
memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN);
-mac_data.entries = ldl_p(elem->out_sg[1].iov_base);
+s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries, sizeof
mac_data.entries);
+if (s != sizeof mac_data.entries) {
+return VIRTIO_NET_ERR;
+}
+
+iov_discard_front(&iov, &iov_cnt, s);
+assert(s == sizeof mac_data.entries);
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
return VIRTIO_NET_ERR;
+}
if (mac_data.entries <= MAC_TABLE_ENTRIES) {
-memcpy(n->mac_table.macs, elem->out_sg[1].iov_base + sizeof(mac_data),
- mac_data.entries * ETH_ALEN);
+s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+ mac_data.entries * ETH_ALEN);
n->mac_table.in_use += mac_data.entries;
} else {
n->mac_table.uni_overflow = 1;
}
+iov_discard_front(&iov, &iov_cnt, mac_data.entries * ETH_ALEN);
+
n->mac_table.first_multi = n->mac_table.in_use;
-mac_data.entries = ldl_p(elem->out_sg[2].iov_base);
+s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries, sizeof
mac_data.entries);
+if (s != sizeof mac_data.entries) {
+return VIRTIO_NET_ERR;
+}
+
+iov_discard_front(&iov, &iov_cnt, s);
+assert(s == sizeof mac_data.entries);
-if (sizeof(mac_data.entries) +
-(mac_data.entries * ETH_ALEN) >
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
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:
> > > ak...@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.
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
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:
> > ak...@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
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
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:
> > ak...@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.
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Thu, Jan 17, 2013 at 11:49:20AM +1030, Rusty Russell wrote:
> ak...@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.
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
ak...@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.
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Wed, Jan 16, 2013 at 02:37:34PM +0800, Jason Wang wrote:
> On Wednesday, January 16, 2013 02:16:47 PM ak...@redhat.com wrote:
> > From: Amos Kong
> >
> > 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
> > ---
> > 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
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
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 ak...@redhat.com wrote:
> > > From: Amos Kong
> > >
> > > 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
> > > ---
> > > 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.
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Wed, Jan 16, 2013 at 02:37:34PM +0800, Jason Wang wrote:
> On Wednesday, January 16, 2013 02:16:47 PM ak...@redhat.com wrote:
> > From: Amos Kong
> >
> > 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
> > ---
> > 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_UP1 /* Link is up */
> >
> > #define TX_TIMER_INTERVAL 15 /* 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_MAC1
> > #define VIRTIO_NET_CTRL_MAC_TABLE_SET0
> > + #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
> >
> > /*
> > * Control VLAN filtering
Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
On Wednesday, January 16, 2013 02:16:47 PM ak...@redhat.com wrote:
> From: Amos Kong
>
> 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
> ---
> 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_UP1 /* Link is up */
>
> #define TX_TIMER_INTERVAL 15 /* 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_MAC1
> #define VIRTIO_NET_CTRL_MAC_TABLE_SET0
> + #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
>
> /*
> * Control VLAN filtering
[Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
From: Amos Kong
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
---
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 */
+
#define VIRTIO_NET_S_LINK_UP1 /* Link is up */
#define TX_TIMER_INTERVAL 15 /* 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_MAC1
#define VIRTIO_NET_CTRL_MAC_TABLE_SET0
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
/*
* Control VLAN filtering
--
1.7.11.7
