* [Qemu-devel] [QEMU PATCH v4 0/3] virtio-net: fix of ctrl commands
@ 2013-01-19 1:54 ` akong
0 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, rusty, qemu-devel, stefanha, virtualization
From: Amos Kong <akong@redhat.com>
Currently virtio-net code relys on the layout of descriptor,
this patchset removed the assumptions and introduced a control
command to set mac address. Last patch is a trivial renaming.
V2: check guest's iov_len
V3: fix of migration compatibility
make mac field in config space read-only when new feature is acked
V4: add fix of descriptor layout assumptions, trivial rename
Amos Kong (2):
virtio-net: introduce a new macaddr control
virtio-net: rename ctrl rx commands
Michael S. Tsirkin (1):
virtio-net: remove layout assumptions for ctrl vq
hw/pc_piix.c | 4 ++
hw/virtio-net.c | 143 ++++++++++++++++++++++++++++++++++----------------------
hw/virtio-net.h | 26 +++++++----
3 files changed, 109 insertions(+), 64 deletions(-)
--
1.7.11.7
^ permalink raw reply [flat|nested] 18+ messages in thread* [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-19 1:54 ` [Qemu-devel] " akong
@ 2013-01-19 1:54 ` akong
-1 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, qemu-devel, stefanha, virtualization
From: "Michael S. Tsirkin" <mst@redhat.com>
Virtio-net code makes assumption about virtqueue descriptor layout
(e.g. sg[0] is the header, sg[1] is the data buffer).
This patch makes code not rely on the layout of descriptors.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 74 insertions(+), 54 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 3bb01b1..113e194 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -315,44 +315,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;
@@ -360,54 +360,71 @@ 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 (sizeof(mac_data.entries) +
- (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+ if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
+ }
+ iov_discard_front(&iov, &iov_cnt, s);
+
+ 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);
+ if (s != mac_data.entries * ETH_ALEN) {
+ return VIRTIO_NET_OK;
+ }
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 (sizeof(mac_data.entries) +
- (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
+ if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
+ }
- if (mac_data.entries) {
- if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
- memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
- elem->out_sg[2].iov_base + sizeof(mac_data),
- mac_data.entries * ETH_ALEN);
- n->mac_table.in_use += mac_data.entries;
- } else {
- n->mac_table.multi_overflow = 1;
+ iov_discard_front(&iov, &iov_cnt, s);
+
+ if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
+ return VIRTIO_NET_ERR;
+ }
+
+ if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
+ s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+ mac_data.entries * ETH_ALEN);
+ if (s != mac_data.entries * ETH_ALEN) {
+ return VIRTIO_NET_OK;
}
+ n->mac_table.in_use += mac_data.entries;
+ } else {
+ n->mac_table.multi_overflow = 1;
}
return VIRTIO_NET_OK;
}
static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
{
uint16_t vid;
+ size_t s;
- if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(vid)) {
- error_report("virtio-net ctrl invalid vlan command");
+ s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
+ if (s != sizeof(vid)) {
return VIRTIO_NET_ERR;
}
- vid = lduw_p(elem->out_sg[1].iov_base);
-
if (vid >= MAX_VLAN)
return VIRTIO_NET_ERR;
@@ -427,30 +444,33 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
struct virtio_net_ctrl_hdr ctrl;
virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
VirtQueueElement elem;
+ size_t s;
+ struct iovec *iov;
+ unsigned int iov_cnt;
while (virtqueue_pop(vq, &elem)) {
- if ((elem.in_num < 1) || (elem.out_num < 1)) {
+ if (iov_size(elem.in_sg, elem.in_num) < sizeof(status) ||
+ iov_size(elem.out_sg, elem.out_num) < sizeof(ctrl)) {
error_report("virtio-net ctrl missing headers");
exit(1);
}
- if (elem.out_sg[0].iov_len < sizeof(ctrl) ||
- elem.in_sg[elem.in_num - 1].iov_len < sizeof(status)) {
- error_report("virtio-net ctrl header not in correct element");
- exit(1);
+ iov = elem.out_sg;
+ iov_cnt = elem.out_num;
+ s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+ iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
+ if (s != sizeof(ctrl)) {
+ status = VIRTIO_NET_ERR;
+ } else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+ status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
+ } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
+ status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
+ } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
+ status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
}
- ctrl.class = ldub_p(elem.out_sg[0].iov_base);
- ctrl.cmd = ldub_p(elem.out_sg[0].iov_base + sizeof(ctrl.class));
-
- if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE)
- status = virtio_net_handle_rx_mode(n, ctrl.cmd, &elem);
- else if (ctrl.class == VIRTIO_NET_CTRL_MAC)
- status = virtio_net_handle_mac(n, ctrl.cmd, &elem);
- else if (ctrl.class == VIRTIO_NET_CTRL_VLAN)
- status = virtio_net_handle_vlan_table(n, ctrl.cmd, &elem);
-
- stb_p(elem.in_sg[elem.in_num - 1].iov_base, status);
+ s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
+ assert(s == sizeof(status));
virtqueue_push(vq, &elem, sizeof(status));
virtio_notify(vdev, vq);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
@ 2013-01-19 1:54 ` akong
0 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, rusty, qemu-devel, stefanha, virtualization
From: "Michael S. Tsirkin" <mst@redhat.com>
Virtio-net code makes assumption about virtqueue descriptor layout
(e.g. sg[0] is the header, sg[1] is the data buffer).
This patch makes code not rely on the layout of descriptors.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
1 file changed, 74 insertions(+), 54 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 3bb01b1..113e194 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -315,44 +315,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;
@@ -360,54 +360,71 @@ 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 (sizeof(mac_data.entries) +
- (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
+ if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
+ }
+ iov_discard_front(&iov, &iov_cnt, s);
+
+ 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);
+ if (s != mac_data.entries * ETH_ALEN) {
+ return VIRTIO_NET_OK;
+ }
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 (sizeof(mac_data.entries) +
- (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
+ if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
+ }
- if (mac_data.entries) {
- if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
- memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
- elem->out_sg[2].iov_base + sizeof(mac_data),
- mac_data.entries * ETH_ALEN);
- n->mac_table.in_use += mac_data.entries;
- } else {
- n->mac_table.multi_overflow = 1;
+ iov_discard_front(&iov, &iov_cnt, s);
+
+ if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
+ return VIRTIO_NET_ERR;
+ }
+
+ if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
+ s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
+ mac_data.entries * ETH_ALEN);
+ if (s != mac_data.entries * ETH_ALEN) {
+ return VIRTIO_NET_OK;
}
+ n->mac_table.in_use += mac_data.entries;
+ } else {
+ n->mac_table.multi_overflow = 1;
}
return VIRTIO_NET_OK;
}
static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
- VirtQueueElement *elem)
+ struct iovec *iov, unsigned int iov_cnt)
{
uint16_t vid;
+ size_t s;
- if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(vid)) {
- error_report("virtio-net ctrl invalid vlan command");
+ s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
+ if (s != sizeof(vid)) {
return VIRTIO_NET_ERR;
}
- vid = lduw_p(elem->out_sg[1].iov_base);
-
if (vid >= MAX_VLAN)
return VIRTIO_NET_ERR;
@@ -427,30 +444,33 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
struct virtio_net_ctrl_hdr ctrl;
virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
VirtQueueElement elem;
+ size_t s;
+ struct iovec *iov;
+ unsigned int iov_cnt;
while (virtqueue_pop(vq, &elem)) {
- if ((elem.in_num < 1) || (elem.out_num < 1)) {
+ if (iov_size(elem.in_sg, elem.in_num) < sizeof(status) ||
+ iov_size(elem.out_sg, elem.out_num) < sizeof(ctrl)) {
error_report("virtio-net ctrl missing headers");
exit(1);
}
- if (elem.out_sg[0].iov_len < sizeof(ctrl) ||
- elem.in_sg[elem.in_num - 1].iov_len < sizeof(status)) {
- error_report("virtio-net ctrl header not in correct element");
- exit(1);
+ iov = elem.out_sg;
+ iov_cnt = elem.out_num;
+ s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+ iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
+ if (s != sizeof(ctrl)) {
+ status = VIRTIO_NET_ERR;
+ } else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+ status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
+ } else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
+ status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
+ } else if (ctrl.class == VIRTIO_NET_CTRL_VLAN) {
+ status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt);
}
- ctrl.class = ldub_p(elem.out_sg[0].iov_base);
- ctrl.cmd = ldub_p(elem.out_sg[0].iov_base + sizeof(ctrl.class));
-
- if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE)
- status = virtio_net_handle_rx_mode(n, ctrl.cmd, &elem);
- else if (ctrl.class == VIRTIO_NET_CTRL_MAC)
- status = virtio_net_handle_mac(n, ctrl.cmd, &elem);
- else if (ctrl.class == VIRTIO_NET_CTRL_VLAN)
- status = virtio_net_handle_vlan_table(n, ctrl.cmd, &elem);
-
- stb_p(elem.in_sg[elem.in_num - 1].iov_base, status);
+ s = iov_from_buf(elem.in_sg, elem.in_num, 0, &status, sizeof(status));
+ assert(s == sizeof(status));
virtqueue_push(vq, &elem, sizeof(status));
virtio_notify(vdev, vq);
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-19 1:54 ` [Qemu-devel] " akong
@ 2013-01-19 2:08 ` Amos Kong
-1 siblings, 0 replies; 18+ messages in thread
From: Amos Kong @ 2013-01-19 2:08 UTC (permalink / raw)
To: mst; +Cc: stefanha, qemu-devel, kvm, virtualization
On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
>
> Virtio-net code makes assumption about virtqueue descriptor layout
> (e.g. sg[0] is the header, sg[1] is the data buffer).
>
> This patch makes code not rely on the layout of descriptors.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 74 insertions(+), 54 deletions(-)
Had tested this patch with following scenarios:
VIRTIO_NET_CTRL_RX_MODE
1) ip link eth0 promisc on/off
2) ip link set eth0 allmulticast on/off
3) ip link set eth0 multicast on/off
VIRTIO_NET_CTRL_MAC
4) ifconfig eth0 hw ether 52:54:00:12:34:57
5) ping guest after joined guest into multicast
group (225.0.0.1 ~ 225.0.0.10)
VIRTIO_NET_CTRL_VLAN
6) vconfig add eth0 2; vconfig rem eth0.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
@ 2013-01-19 2:08 ` Amos Kong
0 siblings, 0 replies; 18+ messages in thread
From: Amos Kong @ 2013-01-19 2:08 UTC (permalink / raw)
To: mst; +Cc: rusty, stefanha, qemu-devel, kvm, virtualization
On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
>
> Virtio-net code makes assumption about virtqueue descriptor layout
> (e.g. sg[0] is the header, sg[1] is the data buffer).
>
> This patch makes code not rely on the layout of descriptors.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 74 insertions(+), 54 deletions(-)
Had tested this patch with following scenarios:
VIRTIO_NET_CTRL_RX_MODE
1) ip link eth0 promisc on/off
2) ip link set eth0 allmulticast on/off
3) ip link set eth0 multicast on/off
VIRTIO_NET_CTRL_MAC
4) ifconfig eth0 hw ether 52:54:00:12:34:57
5) ping guest after joined guest into multicast
group (225.0.0.1 ~ 225.0.0.10)
VIRTIO_NET_CTRL_VLAN
6) vconfig add eth0 2; vconfig rem eth0.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-19 1:54 ` [Qemu-devel] " akong
(?)
(?)
@ 2013-01-21 16:03 ` Stefan Hajnoczi
2013-01-22 14:38 ` Amos Kong
2013-01-22 14:38 ` Amos Kong
-1 siblings, 2 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2013-01-21 16:03 UTC (permalink / raw)
To: akong; +Cc: virtualization, stefanha, qemu-devel, kvm, mst
On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> From: "Michael S. Tsirkin" <mst@redhat.com>
>
> Virtio-net code makes assumption about virtqueue descriptor layout
> (e.g. sg[0] is the header, sg[1] is the data buffer).
>
> This patch makes code not rely on the layout of descriptors.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
> hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 74 insertions(+), 54 deletions(-)
>
> diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> index 3bb01b1..113e194 100644
> --- a/hw/virtio-net.c
> +++ b/hw/virtio-net.c
> @@ -315,44 +315,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;
> @@ -360,54 +360,71 @@ 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 (sizeof(mac_data.entries) +
> - (mac_data.entries * ETH_ALEN) > elem->out_sg[1].iov_len)
> + if (s != sizeof(mac_data.entries)) {
> return VIRTIO_NET_ERR;
> + }
> + iov_discard_front(&iov, &iov_cnt, s);
> +
> + if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
The (possible) byteswap was lost. ldl_p() copies from target endianness
to host endianness.
> + 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);
> + if (s != mac_data.entries * ETH_ALEN) {
> + return VIRTIO_NET_OK;
s/VIRTIO_NET_OK/VIRTIO_NET_ERR/
> + }
> 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));
Same deal with mac_data.entries byteswap.
>
> - if (sizeof(mac_data.entries) +
> - (mac_data.entries * ETH_ALEN) > elem->out_sg[2].iov_len)
> + if (s != sizeof(mac_data.entries)) {
> return VIRTIO_NET_ERR;
> + }
>
> - if (mac_data.entries) {
> - if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
> - memcpy(n->mac_table.macs + (n->mac_table.in_use * ETH_ALEN),
> - elem->out_sg[2].iov_base + sizeof(mac_data),
> - mac_data.entries * ETH_ALEN);
> - n->mac_table.in_use += mac_data.entries;
> - } else {
> - n->mac_table.multi_overflow = 1;
> + iov_discard_front(&iov, &iov_cnt, s);
> +
> + if (mac_data.entries * ETH_ALEN != iov_size(iov, iov_cnt)) {
> + return VIRTIO_NET_ERR;
> + }
> +
> + if (n->mac_table.in_use + mac_data.entries <= MAC_TABLE_ENTRIES) {
> + s = iov_to_buf(iov, iov_cnt, 0, n->mac_table.macs,
> + mac_data.entries * ETH_ALEN);
> + if (s != mac_data.entries * ETH_ALEN) {
> + return VIRTIO_NET_OK;
VIRTIO_NET_ERR
> }
> + n->mac_table.in_use += mac_data.entries;
> + } else {
> + n->mac_table.multi_overflow = 1;
> }
>
> return VIRTIO_NET_OK;
> }
>
> static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
> - VirtQueueElement *elem)
> + struct iovec *iov, unsigned int iov_cnt)
> {
> uint16_t vid;
> + size_t s;
>
> - if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(vid)) {
> - error_report("virtio-net ctrl invalid vlan command");
> + s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
> + if (s != sizeof(vid)) {
> return VIRTIO_NET_ERR;
> }
>
> - vid = lduw_p(elem->out_sg[1].iov_base);
> -
Byteswap missing.
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-21 16:03 ` Stefan Hajnoczi
@ 2013-01-22 14:38 ` Amos Kong
2013-01-22 14:49 ` Stefan Hajnoczi
2013-01-22 14:38 ` Amos Kong
1 sibling, 1 reply; 18+ messages in thread
From: Amos Kong @ 2013-01-22 14:38 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: mst, kvm, qemu-devel, stefanha, virtualization
On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> > From: "Michael S. Tsirkin" <mst@redhat.com>
> >
> > Virtio-net code makes assumption about virtqueue descriptor layout
> > (e.g. sg[0] is the header, sg[1] is the data buffer).
> >
> > This patch makes code not rely on the layout of descriptors.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> > hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> > 1 file changed, 74 insertions(+), 54 deletions(-)
> >
> > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > index 3bb01b1..113e194 100644
> > --- a/hw/virtio-net.c
> > +++ b/hw/virtio-net.c
> > @@ -315,44 +315,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;
> > @@ -360,54 +360,71 @@ 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));
Hi Stefan, can we adjust the endianness after each iov_to_buf() copy?
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 72d7857..0088d6c 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -321,6 +321,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
size_t s;
s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
+ on = ldub_p(&on);
if (s != sizeof(on)) {
return VIRTIO_NET_ERR;
}
@@ -362,7 +363,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+ mac_data.entries = ldl_p(&mac_data.entries);
if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
}
@@ -389,7 +390,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+ mac_data.entries = ldl_p(&mac_data.entries);
if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
}
@@ -421,6 +422,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
size_t s;
s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
+ vid = lduw_p(&vid);
if (s != sizeof(vid)) {
return VIRTIO_NET_ERR;
}
@@ -458,6 +460,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
iov = elem.out_sg;
iov_cnt = elem.out_num;
s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+ ctrl.class = ldub_p(&ctrl.class);
+ ctrl.cmd = ldub_p(&ctrl.class + sizeof(ctrl.class));
iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
if (s != sizeof(ctrl)) {
status = VIRTIO_NET_ERR;
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-22 14:38 ` Amos Kong
@ 2013-01-22 14:49 ` Stefan Hajnoczi
0 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2013-01-22 14:49 UTC (permalink / raw)
To: Amos Kong; +Cc: virtualization, qemu-devel, kvm, mst
On Tue, Jan 22, 2013 at 10:38:14PM +0800, Amos Kong wrote:
> On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote:
> > On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> > > From: "Michael S. Tsirkin" <mst@redhat.com>
> > >
> > > Virtio-net code makes assumption about virtqueue descriptor layout
> > > (e.g. sg[0] is the header, sg[1] is the data buffer).
> > >
> > > This patch makes code not rely on the layout of descriptors.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > ---
> > > hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> > > 1 file changed, 74 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > > index 3bb01b1..113e194 100644
> > > --- a/hw/virtio-net.c
> > > +++ b/hw/virtio-net.c
> > > @@ -315,44 +315,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;
> > > @@ -360,54 +360,71 @@ 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));
>
> Hi Stefan, can we adjust the endianness after each iov_to_buf() copy?
Yes.
It's only necessary for uint16_t and larger types since a single byte
cannot be swapped (so ldub_p() is not needed).
Stefan
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq
2013-01-21 16:03 ` Stefan Hajnoczi
2013-01-22 14:38 ` Amos Kong
@ 2013-01-22 14:38 ` Amos Kong
1 sibling, 0 replies; 18+ messages in thread
From: Amos Kong @ 2013-01-22 14:38 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: virtualization, stefanha, qemu-devel, kvm, mst
On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote:
> > From: "Michael S. Tsirkin" <mst@redhat.com>
> >
> > Virtio-net code makes assumption about virtqueue descriptor layout
> > (e.g. sg[0] is the header, sg[1] is the data buffer).
> >
> > This patch makes code not rely on the layout of descriptors.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> > hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------
> > 1 file changed, 74 insertions(+), 54 deletions(-)
> >
> > diff --git a/hw/virtio-net.c b/hw/virtio-net.c
> > index 3bb01b1..113e194 100644
> > --- a/hw/virtio-net.c
> > +++ b/hw/virtio-net.c
> > @@ -315,44 +315,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;
> > @@ -360,54 +360,71 @@ 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));
Hi Stefan, can we adjust the endianness after each iov_to_buf() copy?
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 72d7857..0088d6c 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -321,6 +321,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
size_t s;
s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on));
+ on = ldub_p(&on);
if (s != sizeof(on)) {
return VIRTIO_NET_ERR;
}
@@ -362,7 +363,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+ mac_data.entries = ldl_p(&mac_data.entries);
if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
}
@@ -389,7 +390,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries,
sizeof(mac_data.entries));
-
+ mac_data.entries = ldl_p(&mac_data.entries);
if (s != sizeof(mac_data.entries)) {
return VIRTIO_NET_ERR;
}
@@ -421,6 +422,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
size_t s;
s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid));
+ vid = lduw_p(&vid);
if (s != sizeof(vid)) {
return VIRTIO_NET_ERR;
}
@@ -458,6 +460,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
iov = elem.out_sg;
iov_cnt = elem.out_num;
s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl));
+ ctrl.class = ldub_p(&ctrl.class);
+ ctrl.cmd = ldub_p(&ctrl.class + sizeof(ctrl.class));
iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
if (s != sizeof(ctrl)) {
status = VIRTIO_NET_ERR;
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control
2013-01-19 1:54 ` [Qemu-devel] " akong
@ 2013-01-19 1:54 ` akong
-1 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: qemu-devel, stefanha, virtualization, rusty, kvm
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, it's atomic.
VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
"mac" field will be set to read-only when VIRTIO_NET_F_CTRL_MAC_ADDR
is acked.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/pc_piix.c | 4 ++++
hw/virtio-net.c | 15 ++++++++++++++-
hw/virtio-net.h | 12 ++++++++++--
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0a6923d..6218350 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -297,6 +297,10 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
.driver = "usb-tablet",\
.property = "usb_version",\
.value = stringify(1),\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "ctrl_mac_addr",\
+ .value = "off", \
}
static QEMUMachine pc_machine_v1_3 = {
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 113e194..cf40ff2 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);
}
@@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
struct virtio_net_ctrl_mac mac_data;
size_t s;
+ if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
+ if (iov_size(iov, iov_cnt) != ETH_ALEN) {
+ return VIRTIO_NET_ERR;
+ }
+ s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
+ if (s != sizeof(n->mac)) {
+ return VIRTIO_NET_ERR;
+ }
+ qemu_format_nic_info_str(&n->nic->nc, n->mac);
+ return VIRTIO_NET_OK;
+ }
+
if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
return VIRTIO_NET_ERR;
}
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..1ec632f 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
@@ -158,5 +165,6 @@ struct virtio_net_ctrl_mac {
DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
- DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)
+ DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
+ DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true)
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control
@ 2013-01-19 1:54 ` akong
0 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, rusty, qemu-devel, stefanha, virtualization
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, it's atomic.
VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
"mac" field will be set to read-only when VIRTIO_NET_F_CTRL_MAC_ADDR
is acked.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/pc_piix.c | 4 ++++
hw/virtio-net.c | 15 ++++++++++++++-
hw/virtio-net.h | 12 ++++++++++--
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0a6923d..6218350 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -297,6 +297,10 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
.driver = "usb-tablet",\
.property = "usb_version",\
.value = stringify(1),\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "ctrl_mac_addr",\
+ .value = "off", \
}
static QEMUMachine pc_machine_v1_3 = {
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 113e194..cf40ff2 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);
}
@@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
struct virtio_net_ctrl_mac mac_data;
size_t s;
+ if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
+ if (iov_size(iov, iov_cnt) != ETH_ALEN) {
+ return VIRTIO_NET_ERR;
+ }
+ s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
+ if (s != sizeof(n->mac)) {
+ return VIRTIO_NET_ERR;
+ }
+ qemu_format_nic_info_str(&n->nic->nc, n->mac);
+ return VIRTIO_NET_OK;
+ }
+
if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
return VIRTIO_NET_ERR;
}
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..1ec632f 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
@@ -158,5 +165,6 @@ struct virtio_net_ctrl_mac {
DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
- DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)
+ DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
+ DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true)
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control
2013-01-19 1:54 ` [Qemu-devel] " akong
(?)
@ 2013-01-21 16:08 ` Stefan Hajnoczi
2013-01-22 11:37 ` Amos Kong
-1 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2013-01-21 16:08 UTC (permalink / raw)
To: akong; +Cc: kvm, mst, qemu-devel, virtualization, stefanha
On Sat, Jan 19, 2013 at 09:54:27AM +0800, akong@redhat.com wrote:
> @@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> struct virtio_net_ctrl_mac mac_data;
> size_t s;
>
> + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
> + if (iov_size(iov, iov_cnt) != ETH_ALEN) {
> + return VIRTIO_NET_ERR;
> + }
> + s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
> + if (s != sizeof(n->mac)) {
> + return VIRTIO_NET_ERR;
> + }
Since iov_size() was checked before iov_to_buf(), we never hit this
error. And if we did n->mac would be trashed (i.e. error handling is
not complete).
I think assert(s == sizeof(n->mac)) is more appropriate appropriate.
Also, please change ETH_ALEN to sizeof(n->mac) to make the relationship
between the check and the copy clear.
Stefan
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control
2013-01-21 16:08 ` Stefan Hajnoczi
@ 2013-01-22 11:37 ` Amos Kong
0 siblings, 0 replies; 18+ messages in thread
From: Amos Kong @ 2013-01-22 11:37 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kvm, mst, qemu-devel, virtualization, stefanha
On Mon, Jan 21, 2013 at 05:08:26PM +0100, Stefan Hajnoczi wrote:
> On Sat, Jan 19, 2013 at 09:54:27AM +0800, akong@redhat.com wrote:
> > @@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
> > struct virtio_net_ctrl_mac mac_data;
> > size_t s;
> >
> > + if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
> > + if (iov_size(iov, iov_cnt) != ETH_ALEN) {
> > + return VIRTIO_NET_ERR;
> > + }
> > + s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
> > + if (s != sizeof(n->mac)) {
> > + return VIRTIO_NET_ERR;
> > + }
> Since iov_size() was checked before iov_to_buf(), we never hit this
> error. And if we did n->mac would be trashed (i.e. error handling is
> not complete).
You are right.
iov_size() computes the size by accounting iov[].iov_lens, the first
check is enough.
> I think assert(s == sizeof(n->mac)) is more appropriate appropriate.
> Also, please change ETH_ALEN to sizeof(n->mac) to make the relationship
> between the check and the copy clear.
>
Will update this patch.
if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
if (iov_size(iov, iov_cnt) != sizeof(n->mac)) {
return VIRTIO_NET_ERR;
}
s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
assert(s == sizeof(n->mac));
qemu_format_nic_info_str(&n->nic->nc, n->mac);
return VIRTIO_NET_OK;
}
> Stefan
^ permalink raw reply [flat|nested] 18+ messages in thread
* [QEMU PATCH v4 2/3] virtio-net: introduce a new macaddr control
2013-01-19 1:54 ` [Qemu-devel] " akong
` (2 preceding siblings ...)
(?)
@ 2013-01-19 1:54 ` akong
-1 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, qemu-devel, stefanha, virtualization
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, it's atomic.
VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
"mac" field will be set to read-only when VIRTIO_NET_F_CTRL_MAC_ADDR
is acked.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/pc_piix.c | 4 ++++
hw/virtio-net.c | 15 ++++++++++++++-
hw/virtio-net.h | 12 ++++++++++--
3 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 0a6923d..6218350 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -297,6 +297,10 @@ static QEMUMachine pc_i440fx_machine_v1_4 = {
.driver = "usb-tablet",\
.property = "usb_version",\
.value = stringify(1),\
+ },{\
+ .driver = "virtio-net-pci",\
+ .property = "ctrl_mac_addr",\
+ .value = "off", \
}
static QEMUMachine pc_machine_v1_3 = {
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 113e194..cf40ff2 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);
}
@@ -350,6 +351,18 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd,
struct virtio_net_ctrl_mac mac_data;
size_t s;
+ if (cmd == VIRTIO_NET_CTRL_MAC_ADDR_SET) {
+ if (iov_size(iov, iov_cnt) != ETH_ALEN) {
+ return VIRTIO_NET_ERR;
+ }
+ s = iov_to_buf(iov, iov_cnt, 0, &n->mac, sizeof(n->mac));
+ if (s != sizeof(n->mac)) {
+ return VIRTIO_NET_ERR;
+ }
+ qemu_format_nic_info_str(&n->nic->nc, n->mac);
+ return VIRTIO_NET_OK;
+ }
+
if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) {
return VIRTIO_NET_ERR;
}
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index d46fb98..1ec632f 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
@@ -158,5 +165,6 @@ struct virtio_net_ctrl_mac {
DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \
DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \
DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \
- DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true)
+ DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \
+ DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true)
#endif
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [QEMU PATCH v4 3/3] virtio-net: rename ctrl rx commands
2013-01-19 1:54 ` [Qemu-devel] " akong
@ 2013-01-19 1:54 ` akong
-1 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: qemu-devel, stefanha, virtualization, rusty, kvm
From: Amos Kong <akong@redhat.com>
This patch makes rx commands consistent with specification.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/virtio-net.c | 14 +++++++-------
hw/virtio-net.h | 14 +++++++-------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index cf40ff2..5700e22 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -326,17 +326,17 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
return VIRTIO_NET_ERR;
}
- if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
+ if (cmd == VIRTIO_NET_CTRL_RX_PROMISC) {
n->promisc = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLMULTI) {
n->allmulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLUNI) {
n->alluni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOMULTI) {
n->nomulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOUNI) {
n->nouni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOBCAST) {
n->nobcast = on;
} else {
return VIRTIO_NET_ERR;
@@ -474,7 +474,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
if (s != sizeof(ctrl)) {
status = VIRTIO_NET_ERR;
- } else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+ } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
} else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1ec632f..c0bb284 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -99,13 +99,13 @@ typedef uint8_t virtio_net_ctrl_ack;
* 0 and 1 are supported with the VIRTIO_NET_F_CTRL_RX feature.
* Commands 2-5 are added with VIRTIO_NET_F_CTRL_RX_EXTRA.
*/
-#define VIRTIO_NET_CTRL_RX_MODE 0
- #define VIRTIO_NET_CTRL_RX_MODE_PROMISC 0
- #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI 1
- #define VIRTIO_NET_CTRL_RX_MODE_ALLUNI 2
- #define VIRTIO_NET_CTRL_RX_MODE_NOMULTI 3
- #define VIRTIO_NET_CTRL_RX_MODE_NOUNI 4
- #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5
+#define VIRTIO_NET_CTRL_RX 0
+ #define VIRTIO_NET_CTRL_RX_PROMISC 0
+ #define VIRTIO_NET_CTRL_RX_ALLMULTI 1
+ #define VIRTIO_NET_CTRL_RX_ALLUNI 2
+ #define VIRTIO_NET_CTRL_RX_NOMULTI 3
+ #define VIRTIO_NET_CTRL_RX_NOUNI 4
+ #define VIRTIO_NET_CTRL_RX_NOBCAST 5
/*
* Control the MAC
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [QEMU PATCH v4 3/3] virtio-net: rename ctrl rx commands
@ 2013-01-19 1:54 ` akong
0 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, rusty, qemu-devel, stefanha, virtualization
From: Amos Kong <akong@redhat.com>
This patch makes rx commands consistent with specification.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/virtio-net.c | 14 +++++++-------
hw/virtio-net.h | 14 +++++++-------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index cf40ff2..5700e22 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -326,17 +326,17 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
return VIRTIO_NET_ERR;
}
- if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
+ if (cmd == VIRTIO_NET_CTRL_RX_PROMISC) {
n->promisc = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLMULTI) {
n->allmulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLUNI) {
n->alluni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOMULTI) {
n->nomulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOUNI) {
n->nouni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOBCAST) {
n->nobcast = on;
} else {
return VIRTIO_NET_ERR;
@@ -474,7 +474,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
if (s != sizeof(ctrl)) {
status = VIRTIO_NET_ERR;
- } else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+ } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
} else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1ec632f..c0bb284 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -99,13 +99,13 @@ typedef uint8_t virtio_net_ctrl_ack;
* 0 and 1 are supported with the VIRTIO_NET_F_CTRL_RX feature.
* Commands 2-5 are added with VIRTIO_NET_F_CTRL_RX_EXTRA.
*/
-#define VIRTIO_NET_CTRL_RX_MODE 0
- #define VIRTIO_NET_CTRL_RX_MODE_PROMISC 0
- #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI 1
- #define VIRTIO_NET_CTRL_RX_MODE_ALLUNI 2
- #define VIRTIO_NET_CTRL_RX_MODE_NOMULTI 3
- #define VIRTIO_NET_CTRL_RX_MODE_NOUNI 4
- #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5
+#define VIRTIO_NET_CTRL_RX 0
+ #define VIRTIO_NET_CTRL_RX_PROMISC 0
+ #define VIRTIO_NET_CTRL_RX_ALLMULTI 1
+ #define VIRTIO_NET_CTRL_RX_ALLUNI 2
+ #define VIRTIO_NET_CTRL_RX_NOMULTI 3
+ #define VIRTIO_NET_CTRL_RX_NOUNI 4
+ #define VIRTIO_NET_CTRL_RX_NOBCAST 5
/*
* Control the MAC
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [QEMU PATCH v4 3/3] virtio-net: rename ctrl rx commands
2013-01-19 1:54 ` [Qemu-devel] " akong
` (4 preceding siblings ...)
(?)
@ 2013-01-19 1:54 ` akong
-1 siblings, 0 replies; 18+ messages in thread
From: akong @ 2013-01-19 1:54 UTC (permalink / raw)
To: mst; +Cc: kvm, qemu-devel, stefanha, virtualization
From: Amos Kong <akong@redhat.com>
This patch makes rx commands consistent with specification.
Signed-off-by: Amos Kong <akong@redhat.com>
---
hw/virtio-net.c | 14 +++++++-------
hw/virtio-net.h | 14 +++++++-------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index cf40ff2..5700e22 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -326,17 +326,17 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd,
return VIRTIO_NET_ERR;
}
- if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) {
+ if (cmd == VIRTIO_NET_CTRL_RX_PROMISC) {
n->promisc = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLMULTI) {
n->allmulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_ALLUNI) {
n->alluni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOMULTI) {
n->nomulti = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOUNI) {
n->nouni = on;
- } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) {
+ } else if (cmd == VIRTIO_NET_CTRL_RX_NOBCAST) {
n->nobcast = on;
} else {
return VIRTIO_NET_ERR;
@@ -474,7 +474,7 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
iov_discard_front(&iov, &iov_cnt, sizeof(ctrl));
if (s != sizeof(ctrl)) {
status = VIRTIO_NET_ERR;
- } else if (ctrl.class == VIRTIO_NET_CTRL_RX_MODE) {
+ } else if (ctrl.class == VIRTIO_NET_CTRL_RX) {
status = virtio_net_handle_rx_mode(n, ctrl.cmd, iov, iov_cnt);
} else if (ctrl.class == VIRTIO_NET_CTRL_MAC) {
status = virtio_net_handle_mac(n, ctrl.cmd, iov, iov_cnt);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 1ec632f..c0bb284 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -99,13 +99,13 @@ typedef uint8_t virtio_net_ctrl_ack;
* 0 and 1 are supported with the VIRTIO_NET_F_CTRL_RX feature.
* Commands 2-5 are added with VIRTIO_NET_F_CTRL_RX_EXTRA.
*/
-#define VIRTIO_NET_CTRL_RX_MODE 0
- #define VIRTIO_NET_CTRL_RX_MODE_PROMISC 0
- #define VIRTIO_NET_CTRL_RX_MODE_ALLMULTI 1
- #define VIRTIO_NET_CTRL_RX_MODE_ALLUNI 2
- #define VIRTIO_NET_CTRL_RX_MODE_NOMULTI 3
- #define VIRTIO_NET_CTRL_RX_MODE_NOUNI 4
- #define VIRTIO_NET_CTRL_RX_MODE_NOBCAST 5
+#define VIRTIO_NET_CTRL_RX 0
+ #define VIRTIO_NET_CTRL_RX_PROMISC 0
+ #define VIRTIO_NET_CTRL_RX_ALLMULTI 1
+ #define VIRTIO_NET_CTRL_RX_ALLUNI 2
+ #define VIRTIO_NET_CTRL_RX_NOMULTI 3
+ #define VIRTIO_NET_CTRL_RX_NOUNI 4
+ #define VIRTIO_NET_CTRL_RX_NOBCAST 5
/*
* Control the MAC
--
1.7.11.7
^ permalink raw reply related [flat|nested] 18+ messages in thread