From: "Michael S. Tsirkin" <mst@redhat.com>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: kvm@vger.kernel.org, akong@redhat.com, qemu-devel@nongnu.org,
stefanha@redhat.com, virtualization@lists.linux-foundation.org
Subject: Re: [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control
Date: Thu, 17 Jan 2013 14:13:08 +0200 [thread overview]
Message-ID: <20130117121308.GA16345@redhat.com> (raw)
In-Reply-To: <87mww8y5bb.fsf@rustcorp.com.au>
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?
> 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 <mst@redhat.com>
---
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) > elem->out_sg[2].iov_len)
+ if (mac_data.entries * ETH_ALEN > iov_size(iov, iov_cnt)) {
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;
- }
+ 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);
+ 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;
@@ -428,30 +438,32 @@ 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) < 1) {
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);
next prev parent reply other threads:[~2013-01-17 12:09 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-16 5:56 [Qemu-devel] [PATCH v2 0/2] make mac programming for virtio net more robust akong
2013-01-16 5:57 ` [Qemu-devel] [PATCH v2 1/2] move virtnet_send_command() above virtnet_set_mac_address() akong
2013-01-16 5:57 ` [Qemu-devel] [PATCH v2 2/2] virtio-net: introduce a new control to set macaddr akong
2013-01-16 6:20 ` Jason Wang
2013-01-16 8:24 ` Amos Kong
2013-01-16 8:36 ` Michael S. Tsirkin
2013-01-16 6:16 ` [Qemu-devel] [QEMU PATCH v2] virtio-net: introduce a new macaddr control akong
2013-01-16 6:37 ` Jason Wang
2013-01-16 8:19 ` Michael S. Tsirkin
2013-01-16 8:59 ` Stefan Hajnoczi
2013-01-16 9:07 ` Michael S. Tsirkin
2013-01-17 1:19 ` Rusty Russell
2013-01-17 5:45 ` Amos Kong
2013-01-17 8:37 ` Amos Kong
2013-01-17 8:39 ` Stefan Hajnoczi
2013-01-17 9:34 ` Michael S. Tsirkin
2013-01-17 12:13 ` Michael S. Tsirkin [this message]
2013-01-18 2:43 ` Amos Kong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130117121308.GA16345@redhat.com \
--to=mst@redhat.com \
--cc=akong@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=qemu-devel@nongnu.org \
--cc=rusty@rustcorp.com.au \
--cc=stefanha@redhat.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).