Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH 14/28] virtio: console: Separate out console-specific data into a separate struct
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-14-git-send-email-amit.shah@redhat.com>

Move out console-specific stuff into a separate struct from 'struct
port' as we need to maintain two lists: one for all the ports (which
includes consoles) and one only for consoles since the hvc callbacks
only give us the vtermno.

This makes console handling cleaner.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   57 ++++++++++++++++++++++++++--------------
 1 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 3111e4c..da8a3ff 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -52,6 +52,24 @@ static struct ports_driver_data pdrvdata;
 
 DEFINE_SPINLOCK(pdrvdata_lock);
 
+/* This struct holds information that's relevant only for console ports */
+struct console {
+	/* We'll place all consoles in a list in the pdrvdata struct */
+	struct list_head list;
+
+	/* The hvc device associated with this console port */
+	struct hvc_struct *hvc;
+
+	/*
+	 * This number identifies the number that we used to register
+	 * with hvc in hvc_instantiate() and hvc_alloc(); this is the
+	 * number passed on by the hvc callbacks to us to
+	 * differentiate between the other console ports handled by
+	 * this driver
+	 */
+	u32 vtermno;
+};
+
 /*
  * This is a per-device struct that stores data common to all the
  * ports for that device (vdev->priv).
@@ -103,15 +121,11 @@ struct port {
 	 */
 	spinlock_t readbuf_list_lock;
 
-	/* For console ports, hvc != NULL and these are valid. */
-	/* The hvc device */
-	struct hvc_struct *hvc;
-
-	/* We'll place all consoles in a list in the pdrvdata struct */
-	struct list_head list;
-
-	/* Our vterm number. */
-	u32 vtermno;
+	/*
+	 * The entries in this struct will be valid if this port is
+	 * hooked up to an hvc console
+	 */
+	struct console cons;
 };
 
 /* This is the very early arch-specified put chars function. */
@@ -120,12 +134,15 @@ static int (*early_put_chars)(u32, const char *, int);
 static struct port *find_port_by_vtermno(u32 vtermno)
 {
 	struct port *port;
+	struct console *cons;
 	unsigned long flags;
 
 	spin_lock_irqsave(&pdrvdata_lock, flags);
-	list_for_each_entry(port, &pdrvdata.consoles, list) {
-		if (port->vtermno == vtermno)
+	list_for_each_entry(cons, &pdrvdata.consoles, list) {
+		if (cons->vtermno == vtermno) {
+			port = container_of(cons, struct port, cons);
 			goto out;
+		}
 	}
 	port = NULL;
 out:
@@ -297,7 +314,7 @@ static void resize_console(struct port *port)
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_console_config, rows),
 				  &ws.ws_row, sizeof(u16));
-		hvc_resize(port->hvc, ws);
+		hvc_resize(port->cons.hvc, ws);
 	}
 }
 
@@ -444,7 +461,7 @@ static void rx_work_handler(struct work_struct *work)
 	struct port *port;
 	struct port_buffer *buf;
 	unsigned int tmplen;
-	bool activity = false;
+	bool console_activity = false;
 
 	portdev = container_of(work, struct ports_device, rx_work);
 
@@ -458,9 +475,9 @@ static void rx_work_handler(struct work_struct *work)
 		list_add_tail(&buf->list, &port->readbuf_head);
 		spin_unlock_irq(&port->readbuf_list_lock);
 
-		activity |= hvc_poll(port->hvc);
+		console_activity |= hvc_poll(port->cons.hvc);
 	}
-	if (activity)
+	if (console_activity)
 		hvc_kick();
 
 	fill_receive_queue(portdev);
@@ -531,10 +548,10 @@ static int __devinit add_port(struct ports_device *portdev)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	port->vtermno = pdrvdata.next_vtermno;
-	port->hvc = hvc_alloc(port->vtermno, 0, &hv_ops, PAGE_SIZE);
-	if (IS_ERR(port->hvc)) {
-		err = PTR_ERR(port->hvc);
+	port->cons.vtermno = pdrvdata.next_vtermno;
+	port->cons.hvc = hvc_alloc(port->cons.vtermno, 0, &hv_ops, PAGE_SIZE);
+	if (IS_ERR(port->cons.hvc)) {
+		err = PTR_ERR(port->cons.hvc);
 		goto free;
 	}
 
@@ -544,7 +561,7 @@ static int __devinit add_port(struct ports_device *portdev)
 	/* Add to vtermno list. */
 	spin_lock_irq(&pdrvdata_lock);
 	pdrvdata.next_vtermno++;
-	list_add(&port->list, &pdrvdata.consoles);
+	list_add(&port->cons.list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 
 	return 0;
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 13/28] virtio: console: Create a buffer pool for sending data to host
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-13-git-send-email-amit.shah@redhat.com>

The old way of sending data to the host was to populate one buffer and
then wait till the host consumed it before sending the next chunk of
data.

Also, there was no support to send large chunks of data.

We now maintain a per-device list of buffers that are ready to be
passed on to the host.

This patch adds support to send big chunks in multiple buffers of
PAGE_SIZE each to the host.

When the host consumes the data and signals to us via an interrupt, we
add the consumed buffer back to our unconsumed list.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |  159 +++++++++++++++++++++++++++++++++-------
 1 files changed, 131 insertions(+), 28 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e8dabae..3111e4c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -67,9 +67,13 @@ struct ports_device {
 	struct work_struct rx_work;
 
 	struct list_head unused_read_head;
+	struct list_head unused_write_head;
 
 	/* To protect the list of unused read buffers and the in_vq */
 	spinlock_t read_list_lock;
+
+	/* To protect the list of unused write buffers and the out_vq */
+	spinlock_t write_list_lock;
 };
 
 /* This struct holds individual buffers received for each port */
@@ -129,42 +133,64 @@ out:
 	return port;
 }
 
-/*
- * The put_chars() callback is pretty straightforward.
- *
- * We turn the characters into a scatter-gather list, add it to the
- * output queue and then kick the Host.  Then we sit here waiting for
- * it to finish: inefficient in theory, but in practice
- * implementations will do it immediately (lguest's Launcher does).
- */
-static int put_chars(u32 vtermno, const char *buf, int count)
+static ssize_t send_buf(struct port *port, const char *in_buf, size_t in_count)
 {
 	struct scatterlist sg[1];
-	struct port *port;
 	struct virtqueue *out_vq;
-	unsigned int len;
+	struct port_buffer *buf;
+	size_t in_offset, copy_size;
+	ssize_t ret;
+	unsigned long irqf;
 
-	port = find_port_by_vtermno(vtermno);
-	if (!port)
+	if (!in_count)
 		return 0;
 
-	if (unlikely(early_put_chars))
-		return early_put_chars(vtermno, buf, count);
-
 	out_vq = port->portdev->out_vq;
-	/* This is a convenient routine to initialize a single-elem sg list */
-	sg_init_one(sg, buf, count);
-
-	/* This shouldn't fail: if it does, we lose chars. */
-	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, port) >= 0) {
-		/* Tell Host to go! */
-		out_vq->vq_ops->kick(out_vq);
-		while (!out_vq->vq_ops->get_buf(out_vq, &len))
-			cpu_relax();
+
+	in_offset = 0; /* offset in the user buffer */
+	spin_lock_irqsave(&port->portdev->write_list_lock, irqf);
+	while (in_count - in_offset) {
+		copy_size = min(in_count - in_offset, PAGE_SIZE);
+
+		if (list_empty(&port->portdev->unused_write_head))
+			break;
+
+		buf = list_first_entry(&port->portdev->unused_write_head,
+				       struct port_buffer, list);
+		list_del(&buf->list);
+		spin_unlock_irqrestore(&port->portdev->write_list_lock, irqf);
+
+		/*
+		 * Since we're not sure when the host will actually
+		 * consume the data and tell us about it, we have
+		 * to copy the data here in case the caller
+		 * frees the in_buf
+		 */
+		memcpy(buf->buf, in_buf + in_offset, copy_size);
+
+		buf->len = copy_size;
+		sg_init_one(sg, buf->buf, buf->len);
+
+		spin_lock_irqsave(&port->portdev->write_list_lock, irqf);
+		ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
+		if (ret < 0) {
+			memset(buf->buf, 0, buf->len);
+			list_add_tail(&buf->list,
+				      &port->portdev->unused_write_head);
+			break;
+		}
+		in_offset += buf->len;
+
+		/* No space left in the vq anyway */
+		if (!ret)
+			break;
 	}
+	/* Tell Host to go! */
+	out_vq->vq_ops->kick(out_vq);
+	spin_unlock_irqrestore(&port->portdev->write_list_lock, irqf);
 
-	/* We're expected to return the amount of data we wrote: all of it. */
-	return count;
+	/* We're expected to return the amount of data we wrote */
+	return in_offset;
 }
 
 /*
@@ -212,6 +238,29 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
 }
 
 /*
+ * The put_chars() callback is pretty straightforward.
+ *
+ * We turn the characters into a scatter-gather list, add it to the output
+ * queue and then kick the Host.
+ *
+ * If the data to be output spans more than a page, it's split into
+ * page-sized buffers and then individual buffers are pushed to the Host.
+ */
+static int put_chars(u32 vtermno, const char *buf, int count)
+{
+	struct port *port;
+
+	port = find_port_by_vtermno(vtermno);
+	if (!port)
+		return 0;
+
+	if (unlikely(early_put_chars))
+		return early_put_chars(vtermno, buf, count);
+
+	return send_buf(port, buf, count);
+}
+
+/*
  * get_chars() is the callback from the hvc_console infrastructure
  * when an interrupt is received.
  *
@@ -318,6 +367,23 @@ out:
 	return buf;
 }
 
+/*
+ * This function is only called from the init routine so the spinlock
+ * for the unused_write_head list isn't taken
+ */
+static void alloc_write_bufs(struct ports_device *portdev)
+{
+	struct port_buffer *buf;
+	int i;
+
+	for (i = 0; i < 30; i++) {
+		buf = get_buf(PAGE_SIZE);
+		if (!buf)
+			break;
+		list_add_tail(&buf->list, &portdev->unused_write_head);
+	}
+}
+
 static void fill_receive_queue(struct ports_device *portdev)
 {
 	struct scatterlist sg[1];
@@ -408,6 +474,40 @@ static void rx_intr(struct virtqueue *vq)
 	schedule_work(&portdev->rx_work);
 }
 
+/*
+ * This is the interrupt handler for buffers that get received on the
+ * output virtqueue, which is an indication that Host consumed the
+ * data we sent it.  Since all our buffers going out are of a fixed
+ * size we can just reuse them instead of freeing them and allocating
+ * new ones.
+ *
+ * Zero out the buffer so that we don't leak any information from
+ * other processes.  There's a small optimisation here as well: the
+ * buffers are PAGE_SIZE-sized; but instead of zeroing the entire
+ * page, we just zero the length that was most recently used and we
+ * can be sure the rest of the page is already set to 0s.
+ *
+ * So once we zero them out we add them back to the unused buffers
+ * list.
+ */
+static void tx_intr(struct virtqueue *vq)
+{
+	struct ports_device *portdev;
+	struct port_buffer *buf;
+	unsigned long flags;
+	unsigned int tmplen;
+
+	portdev = vq->vdev->priv;
+
+	spin_lock_irqsave(&portdev->write_list_lock, flags);
+	while ((buf = vq->vq_ops->get_buf(vq, &tmplen))) {
+		/* 0 the buffer to not leak data from other processes */
+		memset(buf->buf, 0, buf->len);
+		list_add_tail(&buf->list, &portdev->unused_write_head);
+	}
+	spin_unlock_irqrestore(&portdev->write_list_lock, flags);
+}
+
 static int __devinit add_port(struct ports_device *portdev)
 {
 	struct port *port;
@@ -460,7 +560,7 @@ fail:
  */
 static int __devinit virtcons_probe(struct virtio_device *vdev)
 {
-	vq_callback_t *callbacks[] = { rx_intr, NULL};
+	vq_callback_t *callbacks[] = { rx_intr, tx_intr };
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	struct ports_device *portdev;
@@ -484,12 +584,15 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	portdev->out_vq = vqs[1];
 
 	spin_lock_init(&portdev->read_list_lock);
+	spin_lock_init(&portdev->write_list_lock);
 
 	INIT_LIST_HEAD(&portdev->unused_read_head);
+	INIT_LIST_HEAD(&portdev->unused_write_head);
 
 	INIT_WORK(&portdev->rx_work, &rx_work_handler);
 
 	fill_receive_queue(portdev);
+	alloc_write_bufs(portdev);
 
 	/* We only have one port. */
 	err = add_port(portdev);
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 12/28] virtio: console: Buffer data that comes in from the host
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-12-git-send-email-amit.shah@redhat.com>

The console could be flooded with data from the host; handle
this situation by buffering the data.

We move from the per-port inbuf to a per-device queue of buffers,
shared by all the ports for that device, ready to receive host data.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |  219 +++++++++++++++++++++++++++++++----------
 1 files changed, 168 insertions(+), 51 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a80d15c..e8dabae 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -65,6 +65,23 @@ struct ports_device {
 	 * interrupt
 	 */
 	struct work_struct rx_work;
+
+	struct list_head unused_read_head;
+
+	/* To protect the list of unused read buffers and the in_vq */
+	spinlock_t read_list_lock;
+};
+
+/* This struct holds individual buffers received for each port */
+struct port_buffer {
+	struct list_head list;
+
+	char *buf;
+
+	/* length of the buffer */
+	size_t len;
+	/* offset in the buf from which to consume data */
+	size_t offset;
 };
 
 /* This struct holds the per-port data */
@@ -72,9 +89,15 @@ struct port {
 	/* Pointer to the parent virtio_console device */
 	struct ports_device *portdev;
 
-	/* This is our input buffer, and how much data is left in it. */
-	char *inbuf;
-	unsigned int used_len, offset;
+	/* Buffer management */
+	struct list_head readbuf_head;
+
+	/*
+	 * To protect the readbuf_head list.  Has to be a spinlock
+	 * because it can be called from interrupt context
+	 * (get_char())
+	 */
+	spinlock_t readbuf_list_lock;
 
 	/* For console ports, hvc != NULL and these are valid. */
 	/* The hvc device */
@@ -145,67 +168,71 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 }
 
 /*
- * Create a scatter-gather list representing our input buffer and put
- * it in the queue.
+ * Give out the data that's requested from the buffers that we have
+ * queued up.
  */
-static void add_inbuf(struct port *port)
+static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
 {
-	struct virtqueue *in_vq;
-	struct scatterlist sg[1];
-
-	sg_init_one(sg, port->inbuf, PAGE_SIZE);
+	struct port_buffer *buf, *buf2;
+	ssize_t out_offset, ret;
+	unsigned long flags;
 
-	in_vq = port->portdev->in_vq;
-	/* Should always be able to add one buffer to an empty queue. */
-	if (in_vq->vq_ops->add_buf(in_vq, sg, 0, 1, port) < 0)
-		BUG();
-	in_vq->vq_ops->kick(in_vq);
+	out_offset = 0;
+	/*
+	 * Not taking the port->readbuf_list_lock here relying on the
+	 * fact that buffers are taken out from the list only in this
+	 * function so buf2 should be available all the time.
+	 */
+	list_for_each_entry_safe(buf, buf2, &port->readbuf_head, list) {
+		size_t copy_size;
+
+		copy_size = out_count;
+		if (copy_size > buf->len - buf->offset)
+			copy_size = buf->len - buf->offset;
+
+		memcpy(out_buf + out_offset, buf->buf + buf->offset, copy_size);
+
+		/* Return the number of bytes actually copied */
+		ret = copy_size;
+		buf->offset += ret;
+		out_offset += ret;
+		out_count -= ret;
+
+		if (buf->len - buf->offset == 0) {
+			spin_lock_irqsave(&port->readbuf_list_lock, flags);
+			list_del(&buf->list);
+			spin_unlock_irqrestore(&port->readbuf_list_lock, flags);
+			kfree(buf->buf);
+			kfree(buf);
+		}
+		if (!out_count)
+			break;
+	}
+	return out_offset;
 }
 
 /*
  * get_chars() is the callback from the hvc_console infrastructure
  * when an interrupt is received.
  *
- * Most of the code deals with the fact that the hvc_console()
- * infrastructure only asks us for 16 bytes at a time.  We keep
- * in_offset and in_used fields for partially-filled buffers.
+ * We call out to fill_readbuf that gets us the required data from the
+ * buffers that are queued up.
  */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
 	struct port *port;
-	struct virtqueue *in_vq;
 
 	port = find_port_by_vtermno(vtermno);
 	if (!port)
 		return 0;
 
-	in_vq = port->portdev->in_vq;
 	/* If we don't have an input queue yet, we can't get input. */
-	BUG_ON(!in_vq);
-
-	/* No more in buffer?  See if they've (re)used it. */
-	if (port->offset == port->used_len) {
-		unsigned int len;
-
-		if (!in_vq->vq_ops->get_buf(in_vq, &len))
-			return 0;
-		port->used_len = len;
-		port->offset = 0;
-	}
-
-	/* You want more than we have to give?  Well, try wanting less! */
-	if (port->offset + count > port->used_len)
-		count = port->used_len - port->offset;
+	BUG_ON(!port->portdev->in_vq);
 
-	/* Copy across to their buffer and increment offset. */
-	memcpy(buf, port->inbuf + port->offset, count);
-	port->offset += count;
-
-	/* Finished?  Re-register buffer so Host will use it again. */
-	if (port->offset == port->used_len)
-		add_inbuf(port);
+	if (list_empty(&port->readbuf_head))
+		return 0;
 
-	return count;
+	return fill_readbuf(port, buf, count);
 }
 
 static void resize_console(struct port *port)
@@ -274,16 +301,103 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &hv_ops);
 }
 
+static struct port_buffer *get_buf(size_t buf_size)
+{
+	struct port_buffer *buf;
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		goto out;
+	buf->buf = kzalloc(buf_size, GFP_KERNEL);
+	if (!buf->buf) {
+		kfree(buf);
+		goto out;
+	}
+	buf->len = buf_size;
+out:
+	return buf;
+}
+
+static void fill_receive_queue(struct ports_device *portdev)
+{
+	struct scatterlist sg[1];
+	struct port_buffer *buf;
+	struct virtqueue *vq;
+	int buf_size, ret;
+
+	vq = portdev->in_vq;
+	buf_size = PAGE_SIZE;
+	do {
+		buf = get_buf(buf_size);
+		if (!buf)
+			break;
+		sg_init_one(sg, buf->buf, buf_size);
+
+		spin_lock(&portdev->read_list_lock);
+		ret = vq->vq_ops->add_buf(vq, sg, 0, 1, buf);
+		if (ret < 0) {
+			spin_unlock(&portdev->read_list_lock);
+			kfree(buf->buf);
+			kfree(buf);
+			break;
+		}
+		/*
+		 * We have to keep track of the unused buffers so that
+		 * they can be freed when the module is being removed
+		 */
+		list_add_tail(&buf->list, &portdev->unused_read_head);
+		spin_unlock(&portdev->read_list_lock);
+	} while (ret > 0);
+
+	spin_lock(&portdev->read_list_lock);
+	vq->vq_ops->kick(vq);
+	spin_unlock(&portdev->read_list_lock);
+}
+
+static void *get_incoming_buf(struct ports_device *portdev,
+			      unsigned int *len)
+{
+	struct port_buffer *buf;
+	struct virtqueue *vq;
+
+	vq = portdev->in_vq;
+
+	spin_lock(&portdev->read_list_lock);
+	buf = vq->vq_ops->get_buf(vq, len);
+	if (buf) {
+		/* The buffer is no longer unused */
+		list_del(&buf->list);
+	}
+	spin_unlock(&portdev->read_list_lock);
+	return buf;
+}
+
 static void rx_work_handler(struct work_struct *work)
 {
+	struct ports_device *portdev;
 	struct port *port;
+	struct port_buffer *buf;
+	unsigned int tmplen;
 	bool activity = false;
 
-	list_for_each_entry(port, &pdrvdata.consoles, list)
-		activity |= hvc_poll(port->hvc);
+	portdev = container_of(work, struct ports_device, rx_work);
+
+	/* We currently have only one port */
+	port = find_port_by_vtermno(0);
+	while ((buf = get_incoming_buf(portdev, &tmplen))) {
+		buf->len = tmplen;
+		buf->offset = 0;
+
+		spin_lock_irq(&port->readbuf_list_lock);
+		list_add_tail(&buf->list, &port->readbuf_head);
+		spin_unlock_irq(&port->readbuf_list_lock);
 
+		activity |= hvc_poll(port->hvc);
+	}
 	if (activity)
 		hvc_kick();
+
+	fill_receive_queue(portdev);
 }
 
 static void rx_intr(struct virtqueue *vq)
@@ -305,9 +419,6 @@ static int __devinit add_port(struct ports_device *portdev)
 		goto fail;
 
 	port->portdev = portdev;
-	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!port->inbuf)
-		goto free;
 
 	/*
 	 * The first argument of hvc_alloc() is the virtual console
@@ -327,15 +438,15 @@ static int __devinit add_port(struct ports_device *portdev)
 		goto free;
 	}
 
+	spin_lock_init(&port->readbuf_list_lock);
+	INIT_LIST_HEAD(&port->readbuf_head);
+
 	/* Add to vtermno list. */
 	spin_lock_irq(&pdrvdata_lock);
 	pdrvdata.next_vtermno++;
 	list_add(&port->list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 
-	/* Register the input buffer the first time. */
-	add_inbuf(port);
-
 	return 0;
 free:
 	kfree(port);
@@ -372,8 +483,14 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	portdev->in_vq = vqs[0];
 	portdev->out_vq = vqs[1];
 
+	spin_lock_init(&portdev->read_list_lock);
+
+	INIT_LIST_HEAD(&portdev->unused_read_head);
+
 	INIT_WORK(&portdev->rx_work, &rx_work_handler);
 
+	fill_receive_queue(portdev);
+
 	/* We only have one port. */
 	err = add_port(portdev);
 	if (err)
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 11/28] virtio: console: Introduce a workqueue for handling host->guest notifications
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-11-git-send-email-amit.shah@redhat.com>

We currently only maintain one buffer per port for any data the
host sends us. If we're slow in consuming that data, we might lose
old data. To buffer the data that the host sends us, we need to be
able to allocate buffers as and when the host sends us some.

Using a workqueue gives us the freedom to allocate kernel buffers
since the workqueues are executed in process context.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   43 ++++++++++++++++++++++++++++------------
 1 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5a3d5df..a80d15c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -21,6 +21,7 @@
 #include <linux/spinlock.h>
 #include <linux/virtio.h>
 #include <linux/virtio_console.h>
+#include <linux/workqueue.h>
 #include "hvc_console.h"
 
 /*
@@ -58,6 +59,12 @@ DEFINE_SPINLOCK(pdrvdata_lock);
 struct ports_device {
 	struct virtqueue *in_vq, *out_vq;
 	struct virtio_device *vdev;
+
+	/*
+	 * Workqueue handlers where we process deferred work after an
+	 * interrupt
+	 */
+	struct work_struct rx_work;
 };
 
 /* This struct holds the per-port data */
@@ -243,18 +250,6 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 	hp->irq_requested = 0;
 }
 
-static void hvc_handle_input(struct virtqueue *vq)
-{
-	struct port *port;
-	bool activity = false;
-
-	list_for_each_entry(port, &pdrvdata.consoles, list)
-		activity |= hvc_poll(port->hvc);
-
-	if (activity)
-		hvc_kick();
-}
-
 /* The operations for the console. */
 static const struct hv_ops hv_ops = {
 	.get_chars = get_chars,
@@ -279,6 +274,26 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &hv_ops);
 }
 
+static void rx_work_handler(struct work_struct *work)
+{
+	struct port *port;
+	bool activity = false;
+
+	list_for_each_entry(port, &pdrvdata.consoles, list)
+		activity |= hvc_poll(port->hvc);
+
+	if (activity)
+		hvc_kick();
+}
+
+static void rx_intr(struct virtqueue *vq)
+{
+	struct ports_device *portdev;
+
+	portdev = vq->vdev->priv;
+	schedule_work(&portdev->rx_work);
+}
+
 static int __devinit add_port(struct ports_device *portdev)
 {
 	struct port *port;
@@ -334,7 +349,7 @@ fail:
  */
 static int __devinit virtcons_probe(struct virtio_device *vdev)
 {
-	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
+	vq_callback_t *callbacks[] = { rx_intr, NULL};
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
 	struct ports_device *portdev;
@@ -357,6 +372,8 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	portdev->in_vq = vqs[0];
 	portdev->out_vq = vqs[1];
 
+	INIT_WORK(&portdev->rx_work, &rx_work_handler);
+
 	/* We only have one port. */
 	err = add_port(portdev);
 	if (err)
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 10/28] virtio: console: ensure console size is updated on hvc open
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-10-git-send-email-amit.shah@redhat.com>

When multiple console support is added, ensure each port's size gets
updated when a new one is opened via hvc.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   33 +++++++++++++++++----------------
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d4e2327..5a3d5df 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -201,27 +201,28 @@ static int get_chars(u32 vtermno, char *buf, int count)
 	return count;
 }
 
-/*
- * virtio console configuration. This supports:
- * - console resize
- */
-static void virtcons_apply_config(struct virtio_device *dev)
+static void resize_console(struct port *port)
 {
+	struct virtio_device *vdev;
 	struct winsize ws;
 
-	if (virtio_has_feature(dev, VIRTIO_CONSOLE_F_SIZE)) {
-		dev->config->get(dev,
-				 offsetof(struct virtio_console_config, cols),
-				 &ws.ws_col, sizeof(u16));
-		dev->config->get(dev,
-				 offsetof(struct virtio_console_config, rows),
-				 &ws.ws_row, sizeof(u16));
-		/* This is the pre-multiport style: we use control messages
-		 * these days which specify the port.  So this means port 0. */
-		hvc_resize(find_port_by_vtermno(0)->hvc, ws);
+	vdev = port->portdev->vdev;
+	if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_SIZE)) {
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_console_config, cols),
+				  &ws.ws_col, sizeof(u16));
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_console_config, rows),
+				  &ws.ws_row, sizeof(u16));
+		hvc_resize(port->hvc, ws);
 	}
 }
 
+static void virtcons_apply_config(struct virtio_device *vdev)
+{
+	resize_console(find_port_by_vtermno(0));
+}
+
 /* We set the configuration at this point, since we now have a tty */
 static int notifier_add_vio(struct hvc_struct *hp, int data)
 {
@@ -232,7 +233,7 @@ static int notifier_add_vio(struct hvc_struct *hp, int data)
 		return -EINVAL;
 
 	hp->irq_requested = 1;
-	virtcons_apply_config(port->portdev->vdev);
+	resize_console(port);
 
 	return 0;
 }
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 09/28] virtio: console: struct ports for multiple ports per device.
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-9-git-send-email-amit.shah@redhat.com>

Rather than assume a single port, add a 'struct ports_device' which
stores data related to all the ports for that device.

Currently, there's only one port and is hooked up with hvc, but that
will change.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |  154 +++++++++++++++++++++++-----------------
 1 files changed, 88 insertions(+), 66 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 98a5249..d4e2327 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -51,9 +51,20 @@ static struct ports_driver_data pdrvdata;
 
 DEFINE_SPINLOCK(pdrvdata_lock);
 
-struct port {
+/*
+ * This is a per-device struct that stores data common to all the
+ * ports for that device (vdev->priv).
+ */
+struct ports_device {
 	struct virtqueue *in_vq, *out_vq;
 	struct virtio_device *vdev;
+};
+
+/* This struct holds the per-port data */
+struct port {
+	/* Pointer to the parent virtio_console device */
+	struct ports_device *portdev;
+
 	/* This is our input buffer, and how much data is left in it. */
 	char *inbuf;
 	unsigned int used_len, offset;
@@ -100,6 +111,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct scatterlist sg[1];
 	struct port *port;
+	struct virtqueue *out_vq;
 	unsigned int len;
 
 	port = find_port_by_vtermno(vtermno);
@@ -109,14 +121,15 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
 
+	out_vq = port->portdev->out_vq;
 	/* This is a convenient routine to initialize a single-elem sg list */
 	sg_init_one(sg, buf, count);
 
 	/* This shouldn't fail: if it does, we lose chars. */
-	if (port->out_vq->vq_ops->add_buf(port->out_vq, sg, 1, 0, port) >= 0) {
+	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, port) >= 0) {
 		/* Tell Host to go! */
-		port->out_vq->vq_ops->kick(port->out_vq);
-		while (!port->out_vq->vq_ops->get_buf(port->out_vq, &len))
+		out_vq->vq_ops->kick(out_vq);
+		while (!out_vq->vq_ops->get_buf(out_vq, &len))
 			cpu_relax();
 	}
 
@@ -130,13 +143,16 @@ static int put_chars(u32 vtermno, const char *buf, int count)
  */
 static void add_inbuf(struct port *port)
 {
+	struct virtqueue *in_vq;
 	struct scatterlist sg[1];
+
 	sg_init_one(sg, port->inbuf, PAGE_SIZE);
 
+	in_vq = port->portdev->in_vq;
 	/* Should always be able to add one buffer to an empty queue. */
-	if (port->in_vq->vq_ops->add_buf(port->in_vq, sg, 0, 1, port) < 0)
+	if (in_vq->vq_ops->add_buf(in_vq, sg, 0, 1, port) < 0)
 		BUG();
-	port->in_vq->vq_ops->kick(port->in_vq);
+	in_vq->vq_ops->kick(in_vq);
 }
 
 /*
@@ -150,18 +166,23 @@ static void add_inbuf(struct port *port)
 static int get_chars(u32 vtermno, char *buf, int count)
 {
 	struct port *port;
+	struct virtqueue *in_vq;
 
 	port = find_port_by_vtermno(vtermno);
 	if (!port)
 		return 0;
 
+	in_vq = port->portdev->in_vq;
 	/* If we don't have an input queue yet, we can't get input. */
-	BUG_ON(!port->in_vq);
+	BUG_ON(!in_vq);
 
 	/* No more in buffer?  See if they've (re)used it. */
 	if (port->offset == port->used_len) {
-		if (!port->in_vq->vq_ops->get_buf(port->in_vq, &port->used_len))
+		unsigned int len;
+
+		if (!in_vq->vq_ops->get_buf(in_vq, &len))
 			return 0;
+		port->used_len = len;
 		port->offset = 0;
 	}
 
@@ -186,7 +207,6 @@ static int get_chars(u32 vtermno, char *buf, int count)
  */
 static void virtcons_apply_config(struct virtio_device *dev)
 {
-	struct port *port = dev->priv;
 	struct winsize ws;
 
 	if (virtio_has_feature(dev, VIRTIO_CONSOLE_F_SIZE)) {
@@ -196,7 +216,9 @@ static void virtcons_apply_config(struct virtio_device *dev)
 		dev->config->get(dev,
 				 offsetof(struct virtio_console_config, rows),
 				 &ws.ws_row, sizeof(u16));
-		hvc_resize(port->hvc, ws);
+		/* This is the pre-multiport style: we use control messages
+		 * these days which specify the port.  So this means port 0. */
+		hvc_resize(find_port_by_vtermno(0)->hvc, ws);
 	}
 }
 
@@ -210,7 +232,7 @@ static int notifier_add_vio(struct hvc_struct *hp, int data)
 		return -EINVAL;
 
 	hp->irq_requested = 1;
-	virtcons_apply_config(port->vdev);
+	virtcons_apply_config(port->portdev->vdev);
 
 	return 0;
 }
@@ -222,9 +244,13 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 
 static void hvc_handle_input(struct virtqueue *vq)
 {
-	struct port *port = vq->vdev->priv;
+	struct port *port;
+	bool activity = false;
 
-	if (hvc_poll(port->hvc))
+	list_for_each_entry(port, &pdrvdata.consoles, list)
+		activity |= hvc_poll(port->hvc);
+
+	if (activity)
 		hvc_kick();
 }
 
@@ -252,67 +278,21 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &hv_ops);
 }
 
-static struct port *__devinit add_port(u32 vtermno)
+static int __devinit add_port(struct ports_device *portdev)
 {
 	struct port *port;
-
-	port = kzalloc(sizeof *port, GFP_KERNEL);
-	if (!port)
-		return NULL;
-
-	port->used_len = port->offset = 0;
-	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!port->inbuf) {
-		kfree(port);
-		return NULL;
-	}
-	port->hvc = NULL;
-	port->vtermno = vtermno;
-	return port;
-}
-
-static void free_port(struct port *port)
-{
-	kfree(port->inbuf);
-	kfree(port);
-}
-
-/*
- * Once we're further in boot, we get probed like any other virtio
- * device.  At this stage we set up the output virtqueue.
- *
- * To set up and manage our virtual console, we call hvc_alloc().
- * Since we never remove the console device we never need this pointer
- * again.
- *
- * Finally we put our input buffer in the input queue, ready to
- * receive.
- */
-static int __devinit virtcons_probe(struct virtio_device *vdev)
-{
-	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
-	const char *names[] = { "input", "output" };
-	struct virtqueue *vqs[2];
-	struct port *port;
 	int err;
 
 	err = -ENOMEM;
-	port = add_port(pdrvdata.next_vtermno);
+	port = kzalloc(sizeof *port, GFP_KERNEL);
 	if (!port)
 		goto fail;
 
-	/* Attach this port to this virtio_device, and vice-versa. */
-	port->vdev = vdev;
-	vdev->priv = port;
-
-	/* Find the queues. */
-	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
-	if (err)
+	port->portdev = portdev;
+	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!port->inbuf)
 		goto free;
 
-	port->in_vq = vqs[0];
-	port->out_vq = vqs[1];
-
 	/*
 	 * The first argument of hvc_alloc() is the virtual console
 	 * number.  The second argument is the parameter for the
@@ -324,10 +304,11 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
+	port->vtermno = pdrvdata.next_vtermno;
 	port->hvc = hvc_alloc(port->vtermno, 0, &hv_ops, PAGE_SIZE);
 	if (IS_ERR(port->hvc)) {
 		err = PTR_ERR(port->hvc);
-		goto free_vqs;
+		goto free;
 	}
 
 	/* Add to vtermno list. */
@@ -339,6 +320,47 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	/* Register the input buffer the first time. */
 	add_inbuf(port);
 
+	return 0;
+free:
+	kfree(port);
+fail:
+	return err;
+}
+
+/*
+ * Once we're further in boot, we get probed like any other virtio
+ * device.
+ */
+static int __devinit virtcons_probe(struct virtio_device *vdev)
+{
+	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
+	const char *names[] = { "input", "output" };
+	struct virtqueue *vqs[2];
+	struct ports_device *portdev;
+	int err;
+
+	err = -ENOMEM;
+	portdev = kzalloc(sizeof *portdev, GFP_KERNEL);
+	if (!portdev)
+		goto fail;
+
+	/* Attach this portdev to this virtio_device, and vice-versa. */
+	portdev->vdev = vdev;
+	vdev->priv = portdev;
+
+	/* Find the queues. */
+	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
+	if (err)
+		goto free;
+
+	portdev->in_vq = vqs[0];
+	portdev->out_vq = vqs[1];
+
+	/* We only have one port. */
+	err = add_port(portdev);
+	if (err)
+		goto free_vqs;
+
 	/* Start using the new console output. */
 	early_put_chars = NULL;
 	return 0;
@@ -346,7 +368,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 free_vqs:
 	vdev->config->del_vqs(vdev);
 free:
-	free_port(port);
+	kfree(portdev);
 fail:
 	return err;
 }
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 08/28] virtio: console: remove global var
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-8-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

Now we can use an allocation function to remove our global console variable.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   70 +++++++++++++++++++++++++++-------------
 1 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c133bf6..98a5249 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -32,6 +32,18 @@
  * across multiple devices and multiple ports per device.
  */
 struct ports_driver_data {
+	/*
+	 * This is used to keep track of the number of hvc consoles
+	 * spawned by this driver.  This number is given as the first
+	 * argument to hvc_alloc().  To correctly map an initial
+	 * console spawned via hvc_instantiate to the console being
+	 * hooked up via hvc_alloc, we need to pass the same vtermno.
+	 *
+	 * We also just assume the first console being initialised was
+	 * the first one that got used as the initial console.
+	 */
+	unsigned int next_vtermno;
+
 	/* All the console devices handled by this driver */
 	struct list_head consoles;
 };
@@ -57,9 +69,6 @@ struct port {
 	u32 vtermno;
 };
 
-/* We have one port ready to go immediately, for a console. */
-static struct port console;
-
 /* This is the very early arch-specified put chars function. */
 static int (*early_put_chars)(u32, const char *, int);
 
@@ -243,6 +252,31 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 	return hvc_instantiate(0, 0, &hv_ops);
 }
 
+static struct port *__devinit add_port(u32 vtermno)
+{
+	struct port *port;
+
+	port = kzalloc(sizeof *port, GFP_KERNEL);
+	if (!port)
+		return NULL;
+
+	port->used_len = port->offset = 0;
+	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!port->inbuf) {
+		kfree(port);
+		return NULL;
+	}
+	port->hvc = NULL;
+	port->vtermno = vtermno;
+	return port;
+}
+
+static void free_port(struct port *port)
+{
+	kfree(port->inbuf);
+	kfree(port);
+}
+
 /*
  * Once we're further in boot, we get probed like any other virtio
  * device.  At this stage we set up the output virtqueue.
@@ -262,25 +296,15 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 	struct port *port;
 	int err;
 
-	port = &console;
-	if (port->vdev) {
-		dev_warn(&port->vdev->dev,
-			 "Multiple virtio-console devices not supported yet\n");
-		return -EEXIST;
-	}
+	err = -ENOMEM;
+	port = add_port(pdrvdata.next_vtermno);
+	if (!port)
+		goto fail;
 
 	/* Attach this port to this virtio_device, and vice-versa. */
 	port->vdev = vdev;
 	vdev->priv = port;
 
-	/* This is the scratch page we use to receive console input */
-	port->used_len = 0;
-	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!port->inbuf) {
-		err = -ENOMEM;
-		goto fail;
-	}
-
 	/* Find the queues. */
 	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
 	if (err)
@@ -291,17 +315,16 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 
 	/*
 	 * The first argument of hvc_alloc() is the virtual console
-	 * number, so we use zero.  The second argument is the
-	 * parameter for the notification mechanism (like irq
-	 * number). We currently leave this as zero, virtqueues have
-	 * implicit notifications.
+	 * number.  The second argument is the parameter for the
+	 * notification mechanism (like irq number).  We currently
+	 * leave this as zero, virtqueues have implicit notifications.
 	 *
 	 * The third argument is a "struct hv_ops" containing the
 	 * put_chars(), get_chars(), notifier_add() and notifier_del()
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	port->hvc = hvc_alloc(0, 0, &hv_ops, PAGE_SIZE);
+	port->hvc = hvc_alloc(port->vtermno, 0, &hv_ops, PAGE_SIZE);
 	if (IS_ERR(port->hvc)) {
 		err = PTR_ERR(port->hvc);
 		goto free_vqs;
@@ -309,6 +332,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 
 	/* Add to vtermno list. */
 	spin_lock_irq(&pdrvdata_lock);
+	pdrvdata.next_vtermno++;
 	list_add(&port->list, &pdrvdata.consoles);
 	spin_unlock_irq(&pdrvdata_lock);
 
@@ -322,7 +346,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 free_vqs:
 	vdev->config->del_vqs(vdev);
 free:
-	kfree(port->inbuf);
+	free_port(port);
 fail:
 	return err;
 }
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 07/28] virtio: console: don't assume a single console port.
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-7-git-send-email-amit.shah@redhat.com>

Keep a list of all ports being used as a console, and provide a lock
and a lookup function.  The hvc callbacks only give us a vterm number,
so we need to map this.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   72 +++++++++++++++++++++++++++++++++++++----
 1 files changed, 65 insertions(+), 7 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 2bbdbf5..c133bf6 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -17,10 +17,28 @@
  */
 #include <linux/err.h>
 #include <linux/init.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
 #include <linux/virtio.h>
 #include <linux/virtio_console.h>
 #include "hvc_console.h"
 
+/*
+ * This is a global struct for storing common data for all the devices
+ * this driver handles.
+ *
+ * Mainly, it has a linked list for all the consoles in one place so
+ * that callbacks from hvc for get_chars(), put_chars() work properly
+ * across multiple devices and multiple ports per device.
+ */
+struct ports_driver_data {
+	/* All the console devices handled by this driver */
+	struct list_head consoles;
+};
+static struct ports_driver_data pdrvdata;
+
+DEFINE_SPINLOCK(pdrvdata_lock);
+
 struct port {
 	struct virtqueue *in_vq, *out_vq;
 	struct virtio_device *vdev;
@@ -28,8 +46,15 @@ struct port {
 	char *inbuf;
 	unsigned int used_len, offset;
 
+	/* For console ports, hvc != NULL and these are valid. */
 	/* The hvc device */
 	struct hvc_struct *hvc;
+
+	/* We'll place all consoles in a list in the pdrvdata struct */
+	struct list_head list;
+
+	/* Our vterm number. */
+	u32 vtermno;
 };
 
 /* We have one port ready to go immediately, for a console. */
@@ -38,6 +63,22 @@ static struct port console;
 /* This is the very early arch-specified put chars function. */
 static int (*early_put_chars)(u32, const char *, int);
 
+static struct port *find_port_by_vtermno(u32 vtermno)
+{
+	struct port *port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&pdrvdata_lock, flags);
+	list_for_each_entry(port, &pdrvdata.consoles, list) {
+		if (port->vtermno == vtermno)
+			goto out;
+	}
+	port = NULL;
+out:
+	spin_unlock_irqrestore(&pdrvdata_lock, flags);
+	return port;
+}
+
 /*
  * The put_chars() callback is pretty straightforward.
  *
@@ -49,8 +90,12 @@ static int (*early_put_chars)(u32, const char *, int);
 static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct scatterlist sg[1];
+	struct port *port;
 	unsigned int len;
-	struct port *port = &console;
+
+	port = find_port_by_vtermno(vtermno);
+	if (!port)
+		return 0;
 
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
@@ -95,7 +140,11 @@ static void add_inbuf(struct port *port)
  */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
-	struct port *port = &console;
+	struct port *port;
+
+	port = find_port_by_vtermno(vtermno);
+	if (!port)
+		return 0;
 
 	/* If we don't have an input queue yet, we can't get input. */
 	BUG_ON(!port->in_vq);
@@ -142,14 +191,17 @@ static void virtcons_apply_config(struct virtio_device *dev)
 	}
 }
 
-/*
- * we support only one console, the hvc struct is a global var We set
- * the configuration at this point, since we now have a tty
- */
+/* We set the configuration at this point, since we now have a tty */
 static int notifier_add_vio(struct hvc_struct *hp, int data)
 {
+	struct port *port;
+
+	port = find_port_by_vtermno(hp->vtermno);
+	if (!port)
+		return -EINVAL;
+
 	hp->irq_requested = 1;
-	virtcons_apply_config(console.vdev);
+	virtcons_apply_config(port->vdev);
 
 	return 0;
 }
@@ -255,6 +307,11 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 		goto free_vqs;
 	}
 
+	/* Add to vtermno list. */
+	spin_lock_irq(&pdrvdata_lock);
+	list_add(&port->list, &pdrvdata.consoles);
+	spin_unlock_irq(&pdrvdata_lock);
+
 	/* Register the input buffer the first time. */
 	add_inbuf(port);
 
@@ -291,6 +348,7 @@ static struct virtio_driver virtio_console = {
 
 static int __init init(void)
 {
+	INIT_LIST_HEAD(&pdrvdata.consoles);
 	return register_virtio_driver(&virtio_console);
 }
 module_init(init);
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 06/28] virtio: console: use vdev->priv to avoid accessing global var.
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-6-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

Part of removing our "one console" assumptions, use vdev->priv to point
to the port (currently == the global console).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 99d9927..2bbdbf5 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -128,6 +128,7 @@ static int get_chars(u32 vtermno, char *buf, int count)
  */
 static void virtcons_apply_config(struct virtio_device *dev)
 {
+	struct port *port = dev->priv;
 	struct winsize ws;
 
 	if (virtio_has_feature(dev, VIRTIO_CONSOLE_F_SIZE)) {
@@ -137,7 +138,7 @@ static void virtcons_apply_config(struct virtio_device *dev)
 		dev->config->get(dev,
 				 offsetof(struct virtio_console_config, rows),
 				 &ws.ws_row, sizeof(u16));
-		hvc_resize(console.hvc, ws);
+		hvc_resize(port->hvc, ws);
 	}
 }
 
@@ -160,7 +161,9 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 
 static void hvc_handle_input(struct virtqueue *vq)
 {
-	if (hvc_poll(console.hvc))
+	struct port *port = vq->vdev->priv;
+
+	if (hvc_poll(port->hvc))
 		hvc_kick();
 }
 
@@ -213,7 +216,10 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
 			 "Multiple virtio-console devices not supported yet\n");
 		return -EEXIST;
 	}
+
+	/* Attach this port to this virtio_device, and vice-versa. */
 	port->vdev = vdev;
+	vdev->priv = port;
 
 	/* This is the scratch page we use to receive console input */
 	port->used_len = 0;
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 05/28] virtio: console: port encapsulation
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-5-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

We are heading towards a multiple-"port" system, so as part of weaning off
globals we encapsulate the information into 'struct port'.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |  103 +++++++++++++++++++++-------------------
 1 files changed, 54 insertions(+), 49 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index bfc0abf..99d9927 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -21,15 +21,19 @@
 #include <linux/virtio_console.h>
 #include "hvc_console.h"
 
-static struct virtqueue *in_vq, *out_vq;
-static struct virtio_device *vdev;
-
-/* This is our input buffer, and how much data is left in it. */
-static unsigned int in_len;
-static char *in, *inbuf;
+struct port {
+	struct virtqueue *in_vq, *out_vq;
+	struct virtio_device *vdev;
+	/* This is our input buffer, and how much data is left in it. */
+	char *inbuf;
+	unsigned int used_len, offset;
+
+	/* The hvc device */
+	struct hvc_struct *hvc;
+};
 
-/* The hvc device */
-static struct hvc_struct *hvc;
+/* We have one port ready to go immediately, for a console. */
+static struct port console;
 
 /* This is the very early arch-specified put chars function. */
 static int (*early_put_chars)(u32, const char *, int);
@@ -46,6 +50,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct scatterlist sg[1];
 	unsigned int len;
+	struct port *port = &console;
 
 	if (unlikely(early_put_chars))
 		return early_put_chars(vtermno, buf, count);
@@ -53,15 +58,11 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	/* This is a convenient routine to initialize a single-elem sg list */
 	sg_init_one(sg, buf, count);
 
-	/*
-	 * add_buf wants a token to identify this buffer: we hand it
-	 * any non-NULL pointer, since there's only ever one buffer.
-	 */
-	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, (void *)1) >= 0) {
+	/* This shouldn't fail: if it does, we lose chars. */
+	if (port->out_vq->vq_ops->add_buf(port->out_vq, sg, 1, 0, port) >= 0) {
 		/* Tell Host to go! */
-		out_vq->vq_ops->kick(out_vq);
-		/* Chill out until it's done with the buffer. */
-		while (!out_vq->vq_ops->get_buf(out_vq, &len))
+		port->out_vq->vq_ops->kick(port->out_vq);
+		while (!port->out_vq->vq_ops->get_buf(port->out_vq, &len))
 			cpu_relax();
 	}
 
@@ -73,15 +74,15 @@ static int put_chars(u32 vtermno, const char *buf, int count)
  * Create a scatter-gather list representing our input buffer and put
  * it in the queue.
  */
-static void add_inbuf(void)
+static void add_inbuf(struct port *port)
 {
 	struct scatterlist sg[1];
-	sg_init_one(sg, inbuf, PAGE_SIZE);
+	sg_init_one(sg, port->inbuf, PAGE_SIZE);
 
-	/* We should always be able to add one buffer to an empty queue. */
-	if (in_vq->vq_ops->add_buf(in_vq, sg, 0, 1, inbuf) < 0)
+	/* Should always be able to add one buffer to an empty queue. */
+	if (port->in_vq->vq_ops->add_buf(port->in_vq, sg, 0, 1, port) < 0)
 		BUG();
-	in_vq->vq_ops->kick(in_vq);
+	port->in_vq->vq_ops->kick(port->in_vq);
 }
 
 /*
@@ -94,28 +95,29 @@ static void add_inbuf(void)
  */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
+	struct port *port = &console;
+
 	/* If we don't have an input queue yet, we can't get input. */
-	BUG_ON(!in_vq);
+	BUG_ON(!port->in_vq);
 
-	/* No buffer?  Try to get one. */
-	if (!in_len) {
-		in = in_vq->vq_ops->get_buf(in_vq, &in_len);
-		if (!in)
+	/* No more in buffer?  See if they've (re)used it. */
+	if (port->offset == port->used_len) {
+		if (!port->in_vq->vq_ops->get_buf(port->in_vq, &port->used_len))
 			return 0;
+		port->offset = 0;
 	}
 
 	/* You want more than we have to give?  Well, try wanting less! */
-	if (in_len < count)
-		count = in_len;
+	if (port->offset + count > port->used_len)
+		count = port->used_len - port->offset;
 
 	/* Copy across to their buffer and increment offset. */
-	memcpy(buf, in, count);
-	in += count;
-	in_len -= count;
+	memcpy(buf, port->inbuf + port->offset, count);
+	port->offset += count;
 
 	/* Finished?  Re-register buffer so Host will use it again. */
-	if (in_len == 0)
-		add_inbuf();
+	if (port->offset == port->used_len)
+		add_inbuf(port);
 
 	return count;
 }
@@ -135,7 +137,7 @@ static void virtcons_apply_config(struct virtio_device *dev)
 		dev->config->get(dev,
 				 offsetof(struct virtio_console_config, rows),
 				 &ws.ws_row, sizeof(u16));
-		hvc_resize(hvc, ws);
+		hvc_resize(console.hvc, ws);
 	}
 }
 
@@ -146,7 +148,7 @@ static void virtcons_apply_config(struct virtio_device *dev)
 static int notifier_add_vio(struct hvc_struct *hp, int data)
 {
 	hp->irq_requested = 1;
-	virtcons_apply_config(vdev);
+	virtcons_apply_config(console.vdev);
 
 	return 0;
 }
@@ -158,7 +160,7 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
 
 static void hvc_handle_input(struct virtqueue *vq)
 {
-	if (hvc_poll(hvc))
+	if (hvc_poll(console.hvc))
 		hvc_kick();
 }
 
@@ -197,23 +199,26 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
  * Finally we put our input buffer in the input queue, ready to
  * receive.
  */
-static int __devinit virtcons_probe(struct virtio_device *dev)
+static int __devinit virtcons_probe(struct virtio_device *vdev)
 {
 	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
 	const char *names[] = { "input", "output" };
 	struct virtqueue *vqs[2];
+	struct port *port;
 	int err;
 
-	if (vdev) {
-		dev_warn(&vdev->dev,
+	port = &console;
+	if (port->vdev) {
+		dev_warn(&port->vdev->dev,
 			 "Multiple virtio-console devices not supported yet\n");
 		return -EEXIST;
 	}
-	vdev = dev;
+	port->vdev = vdev;
 
 	/* This is the scratch page we use to receive console input */
-	inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!inbuf) {
+	port->used_len = 0;
+	port->inbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!port->inbuf) {
 		err = -ENOMEM;
 		goto fail;
 	}
@@ -223,8 +228,8 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	if (err)
 		goto free;
 
-	in_vq = vqs[0];
-	out_vq = vqs[1];
+	port->in_vq = vqs[0];
+	port->out_vq = vqs[1];
 
 	/*
 	 * The first argument of hvc_alloc() is the virtual console
@@ -238,14 +243,14 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	hvc = hvc_alloc(0, 0, &hv_ops, PAGE_SIZE);
-	if (IS_ERR(hvc)) {
-		err = PTR_ERR(hvc);
+	port->hvc = hvc_alloc(0, 0, &hv_ops, PAGE_SIZE);
+	if (IS_ERR(port->hvc)) {
+		err = PTR_ERR(port->hvc);
 		goto free_vqs;
 	}
 
 	/* Register the input buffer the first time. */
-	add_inbuf();
+	add_inbuf(port);
 
 	/* Start using the new console output. */
 	early_put_chars = NULL;
@@ -254,7 +259,7 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 free_vqs:
 	vdev->config->del_vqs(vdev);
 free:
-	kfree(inbuf);
+	kfree(port->inbuf);
 fail:
 	return err;
 }
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 04/28] virtio: console: We support only one device at a time
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-4-git-send-email-amit.shah@redhat.com>

We support only one virtio_console device at a time. If multiple are
found, error out if one is already initialized.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 791be4e..bfc0abf 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -204,6 +204,11 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	struct virtqueue *vqs[2];
 	int err;
 
+	if (vdev) {
+		dev_warn(&vdev->dev,
+			 "Multiple virtio-console devices not supported yet\n");
+		return -EEXIST;
+	}
 	vdev = dev;
 
 	/* This is the scratch page we use to receive console input */
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 03/28] hvc_console: make the ops pointer const.
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, linuxppc-dev, virtualization
In-Reply-To: <1259391051-7752-3-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

This is nicer for modern R/O protection.  And noone needs it non-const, so
constify the callers as well.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: linuxppc-dev@ozlabs.org
---
 drivers/char/hvc_beat.c       |    2 +-
 drivers/char/hvc_console.c    |    7 ++++---
 drivers/char/hvc_console.h    |    7 ++++---
 drivers/char/hvc_iseries.c    |    2 +-
 drivers/char/hvc_iucv.c       |    2 +-
 drivers/char/hvc_rtas.c       |    2 +-
 drivers/char/hvc_udbg.c       |    2 +-
 drivers/char/hvc_vio.c        |    2 +-
 drivers/char/hvc_xen.c        |    2 +-
 drivers/char/virtio_console.c |    2 +-
 10 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/char/hvc_beat.c b/drivers/char/hvc_beat.c
index 0afc8b8..6913fc3 100644
--- a/drivers/char/hvc_beat.c
+++ b/drivers/char/hvc_beat.c
@@ -84,7 +84,7 @@ static int hvc_beat_put_chars(uint32_t vtermno, const char *buf, int cnt)
 	return cnt;
 }
 
-static struct hv_ops hvc_beat_get_put_ops = {
+static const struct hv_ops hvc_beat_get_put_ops = {
 	.get_chars = hvc_beat_get_chars,
 	.put_chars = hvc_beat_put_chars,
 };
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index a632f25..299772f 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -125,7 +125,7 @@ static struct hvc_struct *hvc_get_by_index(int index)
  * console interfaces but can still be used as a tty device.  This has to be
  * static because kmalloc will not work during early console init.
  */
-static struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
+static const struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
 static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
 	{[0 ... MAX_NR_HVC_CONSOLES - 1] = -1};
 
@@ -247,7 +247,7 @@ static void destroy_hvc_struct(struct kref *kref)
  * vty adapters do NOT get an hvc_instantiate() callback since they
  * appear after early console init.
  */
-int hvc_instantiate(uint32_t vtermno, int index, struct hv_ops *ops)
+int hvc_instantiate(uint32_t vtermno, int index, const struct hv_ops *ops)
 {
 	struct hvc_struct *hp;
 
@@ -749,7 +749,8 @@ static const struct tty_operations hvc_ops = {
 };
 
 struct hvc_struct __devinit *hvc_alloc(uint32_t vtermno, int data,
-					struct hv_ops *ops, int outbuf_size)
+				       const struct hv_ops *ops,
+				       int outbuf_size)
 {
 	struct hvc_struct *hp;
 	int i;
diff --git a/drivers/char/hvc_console.h b/drivers/char/hvc_console.h
index 10950ca..52ddf4d 100644
--- a/drivers/char/hvc_console.h
+++ b/drivers/char/hvc_console.h
@@ -55,7 +55,7 @@ struct hvc_struct {
 	int outbuf_size;
 	int n_outbuf;
 	uint32_t vtermno;
-	struct hv_ops *ops;
+	const struct hv_ops *ops;
 	int irq_requested;
 	int data;
 	struct winsize ws;
@@ -76,11 +76,12 @@ struct hv_ops {
 };
 
 /* Register a vterm and a slot index for use as a console (console_init) */
-extern int hvc_instantiate(uint32_t vtermno, int index, struct hv_ops *ops);
+extern int hvc_instantiate(uint32_t vtermno, int index,
+			   const struct hv_ops *ops);
 
 /* register a vterm for hvc tty operation (module_init or hotplug add) */
 extern struct hvc_struct * __devinit hvc_alloc(uint32_t vtermno, int data,
-				struct hv_ops *ops, int outbuf_size);
+				const struct hv_ops *ops, int outbuf_size);
 /* remove a vterm from hvc tty operation (module_exit or hotplug remove) */
 extern int hvc_remove(struct hvc_struct *hp);
 
diff --git a/drivers/char/hvc_iseries.c b/drivers/char/hvc_iseries.c
index 936d05b..fd02426 100644
--- a/drivers/char/hvc_iseries.c
+++ b/drivers/char/hvc_iseries.c
@@ -197,7 +197,7 @@ done:
 	return sent;
 }
 
-static struct hv_ops hvc_get_put_ops = {
+static const struct hv_ops hvc_get_put_ops = {
 	.get_chars = get_chars,
 	.put_chars = put_chars,
 	.notifier_add = notifier_add_irq,
diff --git a/drivers/char/hvc_iucv.c b/drivers/char/hvc_iucv.c
index b8a5d65..619f7d1 100644
--- a/drivers/char/hvc_iucv.c
+++ b/drivers/char/hvc_iucv.c
@@ -922,7 +922,7 @@ static int hvc_iucv_pm_restore_thaw(struct device *dev)
 
 
 /* HVC operations */
-static struct hv_ops hvc_iucv_ops = {
+static const struct hv_ops hvc_iucv_ops = {
 	.get_chars = hvc_iucv_get_chars,
 	.put_chars = hvc_iucv_put_chars,
 	.notifier_add = hvc_iucv_notifier_add,
diff --git a/drivers/char/hvc_rtas.c b/drivers/char/hvc_rtas.c
index 88590d0..61c4a61 100644
--- a/drivers/char/hvc_rtas.c
+++ b/drivers/char/hvc_rtas.c
@@ -71,7 +71,7 @@ static int hvc_rtas_read_console(uint32_t vtermno, char *buf, int count)
 	return i;
 }
 
-static struct hv_ops hvc_rtas_get_put_ops = {
+static const struct hv_ops hvc_rtas_get_put_ops = {
 	.get_chars = hvc_rtas_read_console,
 	.put_chars = hvc_rtas_write_console,
 };
diff --git a/drivers/char/hvc_udbg.c b/drivers/char/hvc_udbg.c
index bd63ba8..b0957e6 100644
--- a/drivers/char/hvc_udbg.c
+++ b/drivers/char/hvc_udbg.c
@@ -58,7 +58,7 @@ static int hvc_udbg_get(uint32_t vtermno, char *buf, int count)
 	return i;
 }
 
-static struct hv_ops hvc_udbg_ops = {
+static const struct hv_ops hvc_udbg_ops = {
 	.get_chars = hvc_udbg_get,
 	.put_chars = hvc_udbg_put,
 };
diff --git a/drivers/char/hvc_vio.c b/drivers/char/hvc_vio.c
index 10be343..27370e9 100644
--- a/drivers/char/hvc_vio.c
+++ b/drivers/char/hvc_vio.c
@@ -77,7 +77,7 @@ static int filtered_get_chars(uint32_t vtermno, char *buf, int count)
 	return got;
 }
 
-static struct hv_ops hvc_get_put_ops = {
+static const struct hv_ops hvc_get_put_ops = {
 	.get_chars = filtered_get_chars,
 	.put_chars = hvc_put_chars,
 	.notifier_add = notifier_add_irq,
diff --git a/drivers/char/hvc_xen.c b/drivers/char/hvc_xen.c
index a6ee32b..94f8c26 100644
--- a/drivers/char/hvc_xen.c
+++ b/drivers/char/hvc_xen.c
@@ -120,7 +120,7 @@ static int read_console(uint32_t vtermno, char *buf, int len)
 	return recv;
 }
 
-static struct hv_ops hvc_ops = {
+static const struct hv_ops hvc_ops = {
 	.get_chars = read_console,
 	.put_chars = write_console,
 	.notifier_add = notifier_add_irq,
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 1d844a4..791be4e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -163,7 +163,7 @@ static void hvc_handle_input(struct virtqueue *vq)
 }
 
 /* The operations for the console. */
-static struct hv_ops hv_ops = {
+static const struct hv_ops hv_ops = {
 	.get_chars = get_chars,
 	.put_chars = put_chars,
 	.notifier_add = notifier_add_vio,
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 02/28] virtio: console: statically initialize virtio_cons
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-2-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

That way, we can make it const as is good kernel style.  We use a separate
indirection for the early console, rather than mugging ops.put_chars.

We rename it hv_ops, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c |   60 +++++++++++++++++++++++-----------------
 1 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 26e238c..1d844a4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -28,12 +28,12 @@ static struct virtio_device *vdev;
 static unsigned int in_len;
 static char *in, *inbuf;
 
-/* The operations for our console. */
-static struct hv_ops virtio_cons;
-
 /* The hvc device */
 static struct hvc_struct *hvc;
 
+/* This is the very early arch-specified put chars function. */
+static int (*early_put_chars)(u32, const char *, int);
+
 /*
  * The put_chars() callback is pretty straightforward.
  *
@@ -47,6 +47,9 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	struct scatterlist sg[1];
 	unsigned int len;
 
+	if (unlikely(early_put_chars))
+		return early_put_chars(vtermno, buf, count);
+
 	/* This is a convenient routine to initialize a single-elem sg list */
 	sg_init_one(sg, buf, count);
 
@@ -118,21 +121,6 @@ static int get_chars(u32 vtermno, char *buf, int count)
 }
 
 /*
- * Console drivers are initialized very early so boot messages can go
- * out, so we do things slightly differently from the generic virtio
- * initialization of the net and block drivers.
- *
- * At this stage, the console is output-only.  It's too early to set
- * up a virtqueue, so we let the drivers do some boutique early-output
- * thing.
- */
-int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
-{
-	virtio_cons.put_chars = put_chars;
-	return hvc_instantiate(0, 0, &virtio_cons);
-}
-
-/*
  * virtio console configuration. This supports:
  * - console resize
  */
@@ -174,6 +162,30 @@ static void hvc_handle_input(struct virtqueue *vq)
 		hvc_kick();
 }
 
+/* The operations for the console. */
+static struct hv_ops hv_ops = {
+	.get_chars = get_chars,
+	.put_chars = put_chars,
+	.notifier_add = notifier_add_vio,
+	.notifier_del = notifier_del_vio,
+	.notifier_hangup = notifier_del_vio,
+};
+
+/*
+ * Console drivers are initialized very early so boot messages can go
+ * out, so we do things slightly differently from the generic virtio
+ * initialization of the net and block drivers.
+ *
+ * At this stage, the console is output-only.  It's too early to set
+ * up a virtqueue, so we let the drivers do some boutique early-output
+ * thing.
+ */
+int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
+{
+	early_put_chars = put_chars;
+	return hvc_instantiate(0, 0, &hv_ops);
+}
+
 /*
  * Once we're further in boot, we get probed like any other virtio
  * device.  At this stage we set up the output virtqueue.
@@ -209,13 +221,6 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	in_vq = vqs[0];
 	out_vq = vqs[1];
 
-	/* Start using the new console output. */
-	virtio_cons.get_chars = get_chars;
-	virtio_cons.put_chars = put_chars;
-	virtio_cons.notifier_add = notifier_add_vio;
-	virtio_cons.notifier_del = notifier_del_vio;
-	virtio_cons.notifier_hangup = notifier_del_vio;
-
 	/*
 	 * The first argument of hvc_alloc() is the virtual console
 	 * number, so we use zero.  The second argument is the
@@ -228,7 +233,7 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	 * pointers.  The final argument is the output buffer size: we
 	 * can do any size, so we put PAGE_SIZE here.
 	 */
-	hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
+	hvc = hvc_alloc(0, 0, &hv_ops, PAGE_SIZE);
 	if (IS_ERR(hvc)) {
 		err = PTR_ERR(hvc);
 		goto free_vqs;
@@ -236,6 +241,9 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 
 	/* Register the input buffer the first time. */
 	add_inbuf();
+
+	/* Start using the new console output. */
+	early_put_chars = NULL;
 	return 0;
 
 free_vqs:
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 01/28] virtio: console: comment cleanup
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1259391051-7752-1-git-send-email-amit.shah@redhat.com>

From: Rusty Russell <rusty@rustcorp.com.au>

Remove old lguest-style comments.

[Amit: - wingify comments acc. to kernel style
       - indent comments ]

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 drivers/char/virtio_console.c  |  108 ++++++++++++++++++++--------------------
 include/linux/virtio_console.h |    6 ++-
 2 files changed, 58 insertions(+), 56 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index a035ae3..26e238c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1,18 +1,5 @@
-/*D:300
- * The Guest console driver
- *
- * Writing console drivers is one of the few remaining Dark Arts in Linux.
- * Fortunately for us, the path of virtual consoles has been well-trodden by
- * the PowerPC folks, who wrote "hvc_console.c" to generically support any
- * virtual console.  We use that infrastructure which only requires us to write
- * the basic put_chars and get_chars functions and call the right register
- * functions.
- :*/
-
-/*M:002 The console can be flooded: while the Guest is processing input the
- * Host can send more.  Buffering in the Host could alleviate this, but it is a
- * difficult problem in general. :*/
-/* Copyright (C) 2006, 2007 Rusty Russell, IBM Corporation
+/*
+ * Copyright (C) 2006, 2007, 2009 Rusty Russell, IBM Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -34,8 +21,6 @@
 #include <linux/virtio_console.h>
 #include "hvc_console.h"
 
-/*D:340 These represent our input and output console queues, and the virtio
- * operations for them. */
 static struct virtqueue *in_vq, *out_vq;
 static struct virtio_device *vdev;
 
@@ -49,12 +34,14 @@ static struct hv_ops virtio_cons;
 /* The hvc device */
 static struct hvc_struct *hvc;
 
-/*D:310 The put_chars() callback is pretty straightforward.
+/*
+ * The put_chars() callback is pretty straightforward.
  *
- * We turn the characters into a scatter-gather list, add it to the output
- * queue and then kick the Host.  Then we sit here waiting for it to finish:
- * inefficient in theory, but in practice implementations will do it
- * immediately (lguest's Launcher does). */
+ * We turn the characters into a scatter-gather list, add it to the
+ * output queue and then kick the Host.  Then we sit here waiting for
+ * it to finish: inefficient in theory, but in practice
+ * implementations will do it immediately (lguest's Launcher does).
+ */
 static int put_chars(u32 vtermno, const char *buf, int count)
 {
 	struct scatterlist sg[1];
@@ -63,8 +50,10 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	/* This is a convenient routine to initialize a single-elem sg list */
 	sg_init_one(sg, buf, count);
 
-	/* add_buf wants a token to identify this buffer: we hand it any
-	 * non-NULL pointer, since there's only ever one buffer. */
+	/*
+	 * add_buf wants a token to identify this buffer: we hand it
+	 * any non-NULL pointer, since there's only ever one buffer.
+	 */
 	if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, (void *)1) >= 0) {
 		/* Tell Host to go! */
 		out_vq->vq_ops->kick(out_vq);
@@ -77,8 +66,10 @@ static int put_chars(u32 vtermno, const char *buf, int count)
 	return count;
 }
 
-/* Create a scatter-gather list representing our input buffer and put it in the
- * queue. */
+/*
+ * Create a scatter-gather list representing our input buffer and put
+ * it in the queue.
+ */
 static void add_inbuf(void)
 {
 	struct scatterlist sg[1];
@@ -90,12 +81,14 @@ static void add_inbuf(void)
 	in_vq->vq_ops->kick(in_vq);
 }
 
-/*D:350 get_chars() is the callback from the hvc_console infrastructure when
- * an interrupt is received.
+/*
+ * get_chars() is the callback from the hvc_console infrastructure
+ * when an interrupt is received.
  *
- * Most of the code deals with the fact that the hvc_console() infrastructure
- * only asks us for 16 bytes at a time.  We keep in_offset and in_used fields
- * for partially-filled buffers. */
+ * Most of the code deals with the fact that the hvc_console()
+ * infrastructure only asks us for 16 bytes at a time.  We keep
+ * in_offset and in_used fields for partially-filled buffers.
+ */
 static int get_chars(u32 vtermno, char *buf, int count)
 {
 	/* If we don't have an input queue yet, we can't get input. */
@@ -123,14 +116,16 @@ static int get_chars(u32 vtermno, char *buf, int count)
 
 	return count;
 }
-/*:*/
 
-/*D:320 Console drivers are initialized very early so boot messages can go out,
- * so we do things slightly differently from the generic virtio initialization
- * of the net and block drivers.
+/*
+ * Console drivers are initialized very early so boot messages can go
+ * out, so we do things slightly differently from the generic virtio
+ * initialization of the net and block drivers.
  *
- * At this stage, the console is output-only.  It's too early to set up a
- * virtqueue, so we let the drivers do some boutique early-output thing. */
+ * At this stage, the console is output-only.  It's too early to set
+ * up a virtqueue, so we let the drivers do some boutique early-output
+ * thing.
+ */
 int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
 {
 	virtio_cons.put_chars = put_chars;
@@ -157,8 +152,8 @@ static void virtcons_apply_config(struct virtio_device *dev)
 }
 
 /*
- * we support only one console, the hvc struct is a global var
- * We set the configuration at this point, since we now have a tty
+ * we support only one console, the hvc struct is a global var We set
+ * the configuration at this point, since we now have a tty
  */
 static int notifier_add_vio(struct hvc_struct *hp, int data)
 {
@@ -179,13 +174,17 @@ static void hvc_handle_input(struct virtqueue *vq)
 		hvc_kick();
 }
 
-/*D:370 Once we're further in boot, we get probed like any other virtio device.
- * At this stage we set up the output virtqueue.
+/*
+ * Once we're further in boot, we get probed like any other virtio
+ * device.  At this stage we set up the output virtqueue.
  *
- * To set up and manage our virtual console, we call hvc_alloc().  Since we
- * never remove the console device we never need this pointer again.
+ * To set up and manage our virtual console, we call hvc_alloc().
+ * Since we never remove the console device we never need this pointer
+ * again.
  *
- * Finally we put our input buffer in the input queue, ready to receive. */
+ * Finally we put our input buffer in the input queue, ready to
+ * receive.
+ */
 static int __devinit virtcons_probe(struct virtio_device *dev)
 {
 	vq_callback_t *callbacks[] = { hvc_handle_input, NULL};
@@ -203,8 +202,6 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	}
 
 	/* Find the queues. */
-	/* FIXME: This is why we want to wean off hvc: we do nothing
-	 * when input comes in. */
 	err = vdev->config->find_vqs(vdev, 2, vqs, callbacks, names);
 	if (err)
 		goto free;
@@ -219,15 +216,18 @@ static int __devinit virtcons_probe(struct virtio_device *dev)
 	virtio_cons.notifier_del = notifier_del_vio;
 	virtio_cons.notifier_hangup = notifier_del_vio;
 
-	/* The first argument of hvc_alloc() is the virtual console number, so
-	 * we use zero.  The second argument is the parameter for the
-	 * notification mechanism (like irq number). We currently leave this
-	 * as zero, virtqueues have implicit notifications.
+	/*
+	 * The first argument of hvc_alloc() is the virtual console
+	 * number, so we use zero.  The second argument is the
+	 * parameter for the notification mechanism (like irq
+	 * number). We currently leave this as zero, virtqueues have
+	 * implicit notifications.
 	 *
-	 * The third argument is a "struct hv_ops" containing the put_chars()
-	 * get_chars(), notifier_add() and notifier_del() pointers.
-	 * The final argument is the output buffer size: we can do any size,
-	 * so we put PAGE_SIZE here. */
+	 * The third argument is a "struct hv_ops" containing the
+	 * put_chars(), get_chars(), notifier_add() and notifier_del()
+	 * pointers.  The final argument is the output buffer size: we
+	 * can do any size, so we put PAGE_SIZE here.
+	 */
 	hvc = hvc_alloc(0, 0, &virtio_cons, PAGE_SIZE);
 	if (IS_ERR(hvc)) {
 		err = PTR_ERR(hvc);
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index fe88517..9e0da40 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -3,8 +3,10 @@
 #include <linux/types.h>
 #include <linux/virtio_ids.h>
 #include <linux/virtio_config.h>
-/* This header, excluding the #ifdef __KERNEL__ part, is BSD licensed so
- * anyone can use the definitions to implement compatible drivers/servers. */
+/*
+ * This header, excluding the #ifdef __KERNEL__ part, is BSD licensed so
+ * anyone can use the definitions to implement compatible drivers/servers.
+ */
 
 /* Feature bits */
 #define VIRTIO_CONSOLE_F_SIZE	0	/* Does host provide console size? */
-- 
1.6.2.5

^ permalink raw reply related

* [PATCH 00/28] virtio: console: Fixes, support for generic ports
From: Amit Shah @ 2009-11-28  6:50 UTC (permalink / raw)
  To: rusty; +Cc: Amit Shah, virtualization

Hey Rusty,

This is a respin of my patches on top of the series you posted.

I've taken the liberty to modify your patches for style consistency
and comments where needed, keeping the authorship info intact.

I've had to update a few of your patches for functional changes, I've
changed the authorship info on those patches. The (only) major
functional change is to have a list for ports instead of an array
created at device probe time since we will have hotplug support where
we'll need to add new ports anyway.

I've incorporated some of your comments in this series. My
justifications to a few things that differ (some of this may be a
repeat from my previous mails):

* Only one write buffer: to write entire chunks of a write request, we
  have to have some buffer pool. Entire chunks are needed to be
  written in case of vnc copy/paste buffers as vnc clients only use
  the data in the first buffer otherwise. Better than spinning. Also,
  we can't sleep on the output path since we can be called from irq
  context (from hvc).
  
* One vq per port: We have to pre-declare the number of vqs needed at
  probe-time as a result of the MSI support; port hot-plug becomes
  difficult then.

I've passed each patch through an automated test suite that tests for
all the functionality here (caching, throttling, hotplug, hot-unplug,
console IO) twice: once with a qemu that supports this functionality
and once without. It all works fine.

I've also tested with -smp 2 (though looks like qemu's chardevs
rearrange some buffers in case of fast transfers -- there's a race
somewhere there; I've confirmed the kernel sends out all the data in
proper sequence and that the host virtio device queues it up for the
chardevs in proper sequence as well).

The testsuite tests for:
* the 'name' attribute and the symlinks that get created via udev
  scripts in /dev/virtio-ports
* caching
* throttling
* console - with running 'find /'
* open/close of chardevs
* read/write/poll
* sending a large file (> 100 MB) in either direction and matching
  sha1sums.

Since you pointed out that we need not add new feature bits for each
new functionality here, splitting became easier and I've now come up
with even more (but smaller) patches :-)

Please review.

Thanks,
		Amit

Amit Shah (22):
  virtio: console: We support only one device at a time
  virtio: console: don't assume a single console port.
  virtio: console: struct ports for multiple ports per device.
  virtio: console: ensure console size is updated on hvc open
  virtio: console: Introduce a workqueue for handling host->guest
    notifications
  virtio: console: Buffer data that comes in from the host
  virtio: console: Create a buffer pool for sending data to host
  virtio: console: Separate out console-specific data into a separate
    struct
  virtio: console: Separate out console init into a new function
  virtio: console: Introduce a 'header' for each buffer towards
    supporting multiport
  virtio: console: Add a new MULTIPORT feature, support for generic
    ports
  virtio: console: Associate each port with a char device
  virtio: console: Prepare for writing to / reading from userspace
    buffers
  virtio: console: Add file operations to ports for
    open/read/write/poll
  virtio: console: Ensure only one process can have a port open at a
    time
  virtio: console: Register with sysfs and create a 'name' attribute
    for ports
  virtio: console: Add throttling support to prevent flooding ports
  virtio: console: Add option to remove cached buffers on port close
  virtio: console: Handle port hot-plug
  hvc_console: Export (GPL'ed) hvc_remove
  virtio: console: Add ability to hot-unplug ports
  virtio: console: Add debugfs files for each port to expose debug info

Rusty Russell (6):
  virtio: console: comment cleanup
  virtio: console: statically initialize virtio_cons
  hvc_console: make the ops pointer const.
  virtio: console: port encapsulation
  virtio: console: use vdev->priv to avoid accessing global var.
  virtio: console: remove global var

 drivers/char/Kconfig           |    8 +
 drivers/char/hvc_beat.c        |    2 +-
 drivers/char/hvc_console.c     |    8 +-
 drivers/char/hvc_console.h     |    7 +-
 drivers/char/hvc_iseries.c     |    2 +-
 drivers/char/hvc_iucv.c        |    2 +-
 drivers/char/hvc_rtas.c        |    2 +-
 drivers/char/hvc_udbg.c        |    2 +-
 drivers/char/hvc_vio.c         |    2 +-
 drivers/char/hvc_xen.c         |    2 +-
 drivers/char/virtio_console.c  | 1500 ++++++++++++++++++++++++++++++++++++----
 include/linux/virtio_console.h |   46 ++-
 12 files changed, 1415 insertions(+), 168 deletions(-)

^ permalink raw reply

* Re: [PATCHv3 0/4] macvlan: add vepa and bridge mode
From: David Miller @ 2009-11-26 23:53 UTC (permalink / raw)
  To: kaber
  Cc: herbert, eric.dumazet, anna.fischer, netdev, bridge, linux-kernel,
	virtualization, lk-netdev, gerhard.stenzel, arnd, ebiederm, jens,
	pmullaney, shemminger
In-Reply-To: <4B0EAC29.60601@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Thu, 26 Nov 2009 17:26:17 +0100

> Arnd Bergmann wrote:
>> Version 2 description:
>> The patch to iproute2 has not changed, so I'm not including
>> it this time. Patch 4/4 (the netlink interface) is basically
>> unchanged as well but included for completeness.
>> 
>> The other changes have moved forward a bit, to the point where
>> I find them a lot cleaner and am more confident in the code
>> being ready for inclusion. The implementation hardly resembles
>> Erics original patch now, so I've dropped his signed-off-by.
>> 
>> Please take a look and ack if you are happy so we can get it
>> into 2.6.33.
> 
> Looks good to me, nice work.
> 
> Acked-by: Patrick McHardy <kaber@trash.net>
> 
> for the entire series.

All applied to net-next-2.6, thanks everyone!

^ permalink raw reply

* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Patrick McHardy @ 2009-11-26 21:14 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, netdev, bridge,
	linux-kernel, virtualization, Mark Smith, Gerhard Stenzel,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <200911261844.59912.arnd@arndb.de>

Arnd Bergmann wrote:
> On Thursday 26 November 2009, Patrick McHardy wrote:
>> In addition to those already handled, I'd say
>>
>> - priority: affects qdisc classification, may refer to classes of the
>>   old namespace
>> - ipvs_property: might cause packets to incorrectly skip netfilter hooks
>> - nf_trace: might trigger packet tracing
>> - nf_bridge: contains references to network devices in the old NS,
>>   also indicates packet was bridged
>> - iif: index is only valid in the originating namespace
>> - probably secmark.
> 
> ok
> 
>> - tc_index: classification result, should only be set in the namespace
>>   of the classifier
>> - tc_verd: RTTL etc. should begin at zero again
> 
> Wouldn't that defeat the purpose of RTTL? If you create a loop
> across two devices in different namespaces, it may no longer get
> detected. Or is that a different problem again?

Mhh good point, that would indeed be possible. OTOH using ingress
filtering in one namespace currently might cause the packet to get
dropped in a different namespace because the ttl runs out. For now
I'd suggest to go the safe route and keep the TTL intact until we
can come up with something better.

> +void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
> +{
> +	if (skb->dev && !net_eq(dev_net(skb->dev), dev_net(dev))) {
> +		secpath_reset(skb);
> +		skb_dst_drop(skb);
> +		nf_reset(skb);
> +		skb_init_secmark(skb);
> +		skb->mark = 0;
> +		skb->priority = 0;
> +		skb->nf_trace = 0;
> +		skb->ipvs_property = 0;
> +#ifdef CONFIG_NET_SCHED
> +		skb->tc_index = 0;
> +#ifdef CONFIG_NET_CLS_ACT
> +		skb->tc_verd = SET_TC_VERD(skb->tc_verd, 0);
> +		skb->tc_verd = SET_TC_RTTL(skb->tc_verd, 0);
> +#endif
> +#endif

This makes we wonder which ones we actually should keep. Most of the
others get reinitialized anyways, so maybe its better to simply clear
the entire area up until ->tail like f.i. skb_recycle_check().

> +	}
> +	skb->dev = dev;
> +	skb->skb_iif = skb->dev->ifindex;

This doesn't seem necessary, if the packet goes through
netif_receive_skb, it will be set anyways.

> +}
> +EXPORT_SYMBOL(skb_set_dev);

^ permalink raw reply

* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-26 17:44 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, netdev, bridge,
	linux-kernel, virtualization, Mark Smith, Gerhard Stenzel,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <4B0E9FD0.4040107@trash.net>

On Thursday 26 November 2009, Patrick McHardy wrote:
> In addition to those already handled, I'd say
> 
> - priority: affects qdisc classification, may refer to classes of the
>   old namespace
> - ipvs_property: might cause packets to incorrectly skip netfilter hooks
> - nf_trace: might trigger packet tracing
> - nf_bridge: contains references to network devices in the old NS,
>   also indicates packet was bridged
> - iif: index is only valid in the originating namespace
> - probably secmark.

ok

> - tc_index: classification result, should only be set in the namespace
>   of the classifier
> - tc_verd: RTTL etc. should begin at zero again

Wouldn't that defeat the purpose of RTTL? If you create a loop
across two devices in different namespaces, it may no longer get
detected. Or is that a different problem again?

	Arnd <><

---

net: maintain namespace isolation between vlan and real device

In the vlan and macvlan drivers, the start_xmit function forwards
data to the dev_queue_xmit function for another device, which may
potentially belong to a different namespace.

To make sure that classification stays within a single namespace,
this resets the potentially critical fields.

Still needs testing, don't apply

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/macvlan.c     |    2 +-
 include/linux/netdevice.h |    9 +++++++++
 net/8021q/vlan_dev.c      |    2 +-
 net/core/dev.c            |   37 +++++++++++++++++++++++++++++++++----
 4 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 322112c..edcebf1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -269,7 +269,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 xmit_world:
-	skb->dev = vlan->lowerdev;
+	skb_set_dev(skb, vlan->lowerdev);
 	return dev_queue_xmit(skb);
 }
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9428793..fdf4a1a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1009,6 +1009,15 @@ static inline bool netdev_uses_dsa_tags(struct net_device *dev)
 	return 0;
 }
 
+#ifdef CONFIG_NET_NS
+static inline void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+	skb->dev = dev;
+}
+#else /* CONFIG_NET_NS */
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev);
+#endif
+
 static inline bool netdev_uses_trailer_tags(struct net_device *dev)
 {
 #ifdef CONFIG_NET_DSA_TAG_TRAILER
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index de0dc6b..51fcfff 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -323,7 +323,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
 	}
 
 
-	skb->dev = vlan_dev_info(dev)->real_dev;
+	skb_set_dev(skb, vlan_dev_info(dev)->real_dev);
 	len = skb->len;
 	ret = dev_queue_xmit(skb);
 
diff --git a/net/core/dev.c b/net/core/dev.c
index f8baa15..220d4e4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1448,13 +1448,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
 	if (skb->len > (dev->mtu + dev->hard_header_len))
 		return NET_RX_DROP;
 
-	skb_dst_drop(skb);
+	skb_set_dev(skb, dev);
 	skb->tstamp.tv64 = 0;
 	skb->pkt_type = PACKET_HOST;
 	skb->protocol = eth_type_trans(skb, dev);
-	skb->mark = 0;
-	secpath_reset(skb);
-	nf_reset(skb);
 	return netif_rx(skb);
 }
 EXPORT_SYMBOL_GPL(dev_forward_skb);
@@ -1614,6 +1611,39 @@ static bool dev_can_checksum(struct net_device *dev, struct sk_buff *skb)
 	return false;
 }
 
+/**
+ * skb_dev_set -- assign a buffer to a new device
+ * @skb: buffer for the new device
+ * @dev: network device
+ *
+ * If an skb is owned by a device already, we have to reset
+ * all data private to the namespace a device belongs to
+ * before assigning it a new device.
+ */
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+	if (skb->dev && !net_eq(dev_net(skb->dev), dev_net(dev))) {
+		secpath_reset(skb);
+		skb_dst_drop(skb);
+		nf_reset(skb);
+		skb_init_secmark(skb);
+		skb->mark = 0;
+		skb->priority = 0;
+		skb->nf_trace = 0;
+		skb->ipvs_property = 0;
+#ifdef CONFIG_NET_SCHED
+		skb->tc_index = 0;
+#ifdef CONFIG_NET_CLS_ACT
+		skb->tc_verd = SET_TC_VERD(skb->tc_verd, 0);
+		skb->tc_verd = SET_TC_RTTL(skb->tc_verd, 0);
+#endif
+#endif
+	}
+	skb->dev = dev;
+	skb->skb_iif = skb->dev->ifindex;
+}
+EXPORT_SYMBOL(skb_set_dev);
+
 /*
  * Invalidate hardware checksum when packet is to be mangled, and
  * complete checksum manually on outgoing path.

^ permalink raw reply related

* Re: [PATCH 1/4] veth: move loopback logic to common location
From: Eric W. Biederman @ 2009-11-26 16:38 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, netdev, bridge,
	linux-kernel, virtualization, Mark Smith, Gerhard Stenzel,
	Arnd Bergmann, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <4B0E9FD0.4040107@trash.net>

Patrick McHardy <kaber@trash.net> writes:

> Arnd Bergmann wrote:
>> On Tuesday 24 November 2009, Patrick McHardy wrote:
>>> Eric W. Biederman wrote:
>>>> I don't quite follow what you intend with dev_queue_xmit when the macvlan
>>>> is in one namespace and the real physical device is in another.  Are
>>>> you mentioning that the packet classifier runs in the namespace where
>>>> the primary device lives with packets from a different namespace?
>>> Exactly. And I think we should make sure that the namespace of
>>> the macvlan device can't (deliberately or accidentally) cause
>>> misclassification.
>> 
>> This is independent of my series and a preexisting problem, right?
>
> Correct.
>
>> Which fields do you think need to be reset to maintain namespace
>> isolation for the outbound path in macvlan?
>
> In addition to those already handled, I'd say
>
> - priority: affects qdisc classification, may refer to classes of the
>   old namespace
> - ipvs_property: might cause packets to incorrectly skip netfilter hooks
> - nf_trace: might trigger packet tracing
> - nf_bridge: contains references to network devices in the old NS,
>   also indicates packet was bridged
> - iif: index is only valid in the originating namespace
> - tc_index: classification result, should only be set in the namespace
>   of the classifier
> - tc_verd: RTTL etc. should begin at zero again
> - probably secmark.

Wow.  I thought we were trying to reduce skbuff, where did all of those
fields come from?  Regarless that sounds like a good list to get stomped.

Eric

^ permalink raw reply

* Re: [PATCHv3 0/4] macvlan: add vepa and bridge mode
From: Patrick McHardy @ 2009-11-26 16:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, netdev, bridge,
	linux-kernel, virtualization, Mark Smith, Gerhard Stenzel,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <1259251631-11617-1-git-send-email-arnd@arndb.de>

Arnd Bergmann wrote:
> Version 2 description:
> The patch to iproute2 has not changed, so I'm not including
> it this time. Patch 4/4 (the netlink interface) is basically
> unchanged as well but included for completeness.
> 
> The other changes have moved forward a bit, to the point where
> I find them a lot cleaner and am more confident in the code
> being ready for inclusion. The implementation hardly resembles
> Erics original patch now, so I've dropped his signed-off-by.
> 
> Please take a look and ack if you are happy so we can get it
> into 2.6.33.

Looks good to me, nice work.

Acked-by: Patrick McHardy <kaber@trash.net>

for the entire series.

^ permalink raw reply

* [PATCHv3 4/4] macvlan: export macvlan mode through netlink
From: Arnd Bergmann @ 2009-11-26 16:07 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, bridge, linux-kernel,
	virtualization, Mark Smith, Gerhard Stenzel, Arnd Bergmann,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <1259251631-11617-1-git-send-email-arnd@arndb.de>

In order to support all three modes of macvlan at
runtime, extend the existing netlink protocol
to allow choosing the mode per macvlan slave
interface.

This depends on a matching patch to iproute2
in order to become accessible in user land.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/macvlan.c   |   56 +++++++++++++++++++++++++++++++++++++++++-----
 include/linux/if_link.h |   15 ++++++++++++
 2 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index d6bd843..322112c 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -33,12 +33,6 @@
 
 #define MACVLAN_HASH_SIZE	(1 << BITS_PER_BYTE)
 
-enum macvlan_mode {
-	MACVLAN_MODE_PRIVATE	= 1,
-	MACVLAN_MODE_VEPA	= 2,
-	MACVLAN_MODE_BRIDGE	= 4,
-};
-
 struct macvlan_port {
 	struct net_device	*dev;
 	struct hlist_head	vlan_hash[MACVLAN_HASH_SIZE];
@@ -614,6 +608,17 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
 		if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS])))
 			return -EADDRNOTAVAIL;
 	}
+
+	if (data && data[IFLA_MACVLAN_MODE]) {
+		switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
+		case MACVLAN_MODE_PRIVATE:
+		case MACVLAN_MODE_VEPA:
+		case MACVLAN_MODE_BRIDGE:
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
 	return 0;
 }
 
@@ -678,6 +683,10 @@ static int macvlan_newlink(struct net *src_net, struct net_device *dev,
 	vlan->dev      = dev;
 	vlan->port     = port;
 
+	vlan->mode     = MACVLAN_MODE_VEPA;
+	if (data && data[IFLA_MACVLAN_MODE])
+		vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+
 	err = register_netdevice(dev);
 	if (err < 0)
 		return err;
@@ -699,6 +708,36 @@ static void macvlan_dellink(struct net_device *dev, struct list_head *head)
 		macvlan_port_destroy(port->dev);
 }
 
+static int macvlan_changelink(struct net_device *dev,
+		struct nlattr *tb[], struct nlattr *data[])
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+	if (data && data[IFLA_MACVLAN_MODE])
+		vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+	return 0;
+}
+
+static size_t macvlan_get_size(const struct net_device *dev)
+{
+	return nla_total_size(4);
+}
+
+static int macvlan_fill_info(struct sk_buff *skb,
+				const struct net_device *dev)
+{
+	struct macvlan_dev *vlan = netdev_priv(dev);
+
+	NLA_PUT_U32(skb, IFLA_MACVLAN_MODE, vlan->mode);
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
+static const struct nla_policy macvlan_policy[IFLA_MACVLAN_MAX + 1] = {
+	[IFLA_MACVLAN_MODE] = { .type = NLA_U32 },
+};
+
 static struct rtnl_link_ops macvlan_link_ops __read_mostly = {
 	.kind		= "macvlan",
 	.priv_size	= sizeof(struct macvlan_dev),
@@ -707,6 +746,11 @@ static struct rtnl_link_ops macvlan_link_ops __read_mostly = {
 	.validate	= macvlan_validate,
 	.newlink	= macvlan_newlink,
 	.dellink	= macvlan_dellink,
+	.maxtype	= IFLA_MACVLAN_MAX,
+	.policy		= macvlan_policy,
+	.changelink	= macvlan_changelink,
+	.get_size	= macvlan_get_size,
+	.fill_info	= macvlan_fill_info,
 };
 
 static int macvlan_device_event(struct notifier_block *unused,
diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 1d3b242..6674791 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -181,4 +181,19 @@ struct ifla_vlan_qos_mapping {
 	__u32 to;
 };
 
+/* MACVLAN section */
+enum {
+	IFLA_MACVLAN_UNSPEC,
+	IFLA_MACVLAN_MODE,
+	__IFLA_MACVLAN_MAX,
+};
+
+#define IFLA_MACVLAN_MAX (__IFLA_MACVLAN_MAX - 1)
+
+enum macvlan_mode {
+	MACVLAN_MODE_PRIVATE = 1, /* don't talk to other macvlans */
+	MACVLAN_MODE_VEPA    = 2, /* talk to other ports through ext bridge */
+	MACVLAN_MODE_BRIDGE  = 4, /* talk to bridge ports directly */
+};
+
 #endif /* _LINUX_IF_LINK_H */
-- 
1.6.3.3

^ permalink raw reply related

* [PATCHv3 3/4] macvlan: implement bridge, VEPA and private mode
From: Arnd Bergmann @ 2009-11-26 16:07 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, bridge, linux-kernel,
	virtualization, Mark Smith, Gerhard Stenzel, Arnd Bergmann,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <1259251631-11617-1-git-send-email-arnd@arndb.de>

This allows each macvlan slave device to be in one
of three modes, depending on the use case:

MACVLAN_PRIVATE:
  The device never communicates with any other device
  on the same upper_dev. This even includes frames
  coming back from a reflective relay, where supported
  by the adjacent bridge.

MACVLAN_VEPA:
  The new Virtual Ethernet Port Aggregator (VEPA) mode,
  we assume that the adjacent bridge returns all frames
  where both source and destination are local to the
  macvlan port, i.e. the bridge is set up as a reflective
  relay.
  Broadcast frames coming in from the upper_dev get
  flooded to all macvlan interfaces in VEPA mode.
  We never deliver any frames locally.

MACVLAN_BRIDGE:
  We provide the behavior of a simple bridge between
  different macvlan interfaces on the same port. Frames
  from one interface to another one get delivered directly
  and are not sent out externally. Broadcast frames get
  flooded to all other bridge ports and to the external
  interface, but when they come back from a reflective
  relay, we don't deliver them again.
  Since we know all the MAC addresses, the macvlan bridge
  mode does not require learning or STP like the bridge
  module does.

Based on an earlier patch "macvlan: Reflect macvlan packets
meant for other macvlan devices" by Eric Biederman.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Eric Biederman <ebiederm@xmission.com>
---
 drivers/net/macvlan.c |   80 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 72 insertions(+), 8 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 1e7faf9..d6bd843 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -29,9 +29,16 @@
 #include <linux/if_link.h>
 #include <linux/if_macvlan.h>
 #include <net/rtnetlink.h>
+#include <net/xfrm.h>
 
 #define MACVLAN_HASH_SIZE	(1 << BITS_PER_BYTE)
 
+enum macvlan_mode {
+	MACVLAN_MODE_PRIVATE	= 1,
+	MACVLAN_MODE_VEPA	= 2,
+	MACVLAN_MODE_BRIDGE	= 4,
+};
+
 struct macvlan_port {
 	struct net_device	*dev;
 	struct hlist_head	vlan_hash[MACVLAN_HASH_SIZE];
@@ -59,6 +66,7 @@ struct macvlan_dev {
 	struct macvlan_port	*port;
 	struct net_device	*lowerdev;
 	struct macvlan_rx_stats *rx_stats;
+	enum macvlan_mode	mode;
 };
 
 
@@ -134,11 +142,14 @@ static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
 }
 
 static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
-				 const struct ethhdr *eth)
+				 const struct ethhdr *eth, bool local)
 {
 	if (!skb)
 		return NET_RX_DROP;
 
+	if (local)
+		return dev_forward_skb(dev, skb);
+
 	skb->dev = dev;
 	if (!compare_ether_addr_64bits(eth->h_dest,
 				       dev->broadcast))
@@ -150,7 +161,9 @@ static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
 }
 
 static void macvlan_broadcast(struct sk_buff *skb,
-			      const struct macvlan_port *port)
+			      const struct macvlan_port *port,
+			      struct net_device *src,
+			      enum macvlan_mode mode)
 {
 	const struct ethhdr *eth = eth_hdr(skb);
 	const struct macvlan_dev *vlan;
@@ -164,8 +177,12 @@ static void macvlan_broadcast(struct sk_buff *skb,
 
 	for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
 		hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
+			if (vlan->dev == src || !(vlan->mode & mode))
+				continue;
+
 			nskb = skb_clone(skb, GFP_ATOMIC);
-			err = macvlan_broadcast_one(nskb, vlan->dev, eth);
+			err = macvlan_broadcast_one(nskb, vlan->dev, eth,
+					 mode == MACVLAN_MODE_BRIDGE);
 			macvlan_count_rx(vlan, skb->len + ETH_HLEN,
 					 err == NET_RX_SUCCESS, 1);
 		}
@@ -178,6 +195,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 	const struct ethhdr *eth = eth_hdr(skb);
 	const struct macvlan_port *port;
 	const struct macvlan_dev *vlan;
+	const struct macvlan_dev *src;
 	struct net_device *dev;
 	unsigned int len;
 
@@ -186,7 +204,25 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 		return skb;
 
 	if (is_multicast_ether_addr(eth->h_dest)) {
-		macvlan_broadcast(skb, port);
+		src = macvlan_hash_lookup(port, eth->h_source);
+		if (!src)
+			/* frame comes from an external address */
+			macvlan_broadcast(skb, port, NULL,
+					  MACVLAN_MODE_PRIVATE |
+					  MACVLAN_MODE_VEPA    |
+					  MACVLAN_MODE_BRIDGE);
+		else if (src->mode == MACVLAN_MODE_VEPA)
+			/* flood to everyone except source */
+			macvlan_broadcast(skb, port, src->dev,
+					  MACVLAN_MODE_VEPA |
+					  MACVLAN_MODE_BRIDGE);
+		else if (src->mode == MACVLAN_MODE_BRIDGE)
+			/*
+			 * flood only to VEPA ports, bridge ports
+			 * already saw the frame on the way out.
+			 */
+			macvlan_broadcast(skb, port, src->dev,
+					  MACVLAN_MODE_VEPA);
 		return skb;
 	}
 
@@ -212,18 +248,46 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 	return NULL;
 }
 
+static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	const struct macvlan_dev *vlan = netdev_priv(dev);
+	const struct macvlan_port *port = vlan->port;
+	const struct macvlan_dev *dest;
+
+	if (vlan->mode == MACVLAN_MODE_BRIDGE) {
+		const struct ethhdr *eth = (void *)skb->data;
+
+		/* send to other bridge ports directly */
+		if (is_multicast_ether_addr(eth->h_dest)) {
+			macvlan_broadcast(skb, port, dev, MACVLAN_MODE_BRIDGE);
+			goto xmit_world;
+		}
+
+		dest = macvlan_hash_lookup(port, eth->h_dest);
+		if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
+			unsigned int length = skb->len + ETH_HLEN;
+			int ret = dev_forward_skb(dest->dev, skb);
+			macvlan_count_rx(dest, length,
+					 ret == NET_RX_SUCCESS, 0);
+
+			return NET_XMIT_SUCCESS;
+		}
+	}
+
+xmit_world:
+	skb->dev = vlan->lowerdev;
+	return dev_queue_xmit(skb);
+}
+
 static netdev_tx_t macvlan_start_xmit(struct sk_buff *skb,
 				      struct net_device *dev)
 {
 	int i = skb_get_queue_mapping(skb);
 	struct netdev_queue *txq = netdev_get_tx_queue(dev, i);
-	const struct macvlan_dev *vlan = netdev_priv(dev);
 	unsigned int len = skb->len;
 	int ret;
 
-	skb->dev = vlan->lowerdev;
-	ret = dev_queue_xmit(skb);
-
+	ret = macvlan_queue_xmit(skb, dev);
 	if (likely(ret == NET_XMIT_SUCCESS)) {
 		txq->tx_packets++;
 		txq->tx_bytes += len;
-- 
1.6.3.3

^ permalink raw reply related

* [PATCHv3 2/4] macvlan: cleanup rx statistics
From: Arnd Bergmann @ 2009-11-26 16:07 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, bridge, linux-kernel,
	virtualization, Mark Smith, Gerhard Stenzel, Arnd Bergmann,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <1259251631-11617-1-git-send-email-arnd@arndb.de>

We have very similar code for rx statistics in
two places in the macvlan driver, with a third
one being added in the next patch.

Consolidate them into one function to improve
overall readability of the driver.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/macvlan.c |   70 ++++++++++++++++++++++++++++--------------------
 1 files changed, 41 insertions(+), 29 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index ae2b5c7..1e7faf9 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -116,42 +116,58 @@ static int macvlan_addr_busy(const struct macvlan_port *port,
 	return 0;
 }
 
+static inline void macvlan_count_rx(const struct macvlan_dev *vlan,
+				    unsigned int len, bool success,
+				    bool multicast)
+{
+	struct macvlan_rx_stats *rx_stats;
+
+	rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
+	if (likely(success)) {
+		rx_stats->rx_packets++;;
+		rx_stats->rx_bytes += len;
+		if (multicast)
+			rx_stats->multicast++;
+	} else {
+		rx_stats->rx_errors++;
+	}
+}
+
+static int macvlan_broadcast_one(struct sk_buff *skb, struct net_device *dev,
+				 const struct ethhdr *eth)
+{
+	if (!skb)
+		return NET_RX_DROP;
+
+	skb->dev = dev;
+	if (!compare_ether_addr_64bits(eth->h_dest,
+				       dev->broadcast))
+		skb->pkt_type = PACKET_BROADCAST;
+	else
+		skb->pkt_type = PACKET_MULTICAST;
+
+	return netif_rx(skb);
+}
+
 static void macvlan_broadcast(struct sk_buff *skb,
 			      const struct macvlan_port *port)
 {
 	const struct ethhdr *eth = eth_hdr(skb);
 	const struct macvlan_dev *vlan;
 	struct hlist_node *n;
-	struct net_device *dev;
 	struct sk_buff *nskb;
 	unsigned int i;
-	struct macvlan_rx_stats *rx_stats;
+	int err;
 
 	if (skb->protocol == htons(ETH_P_PAUSE))
 		return;
 
 	for (i = 0; i < MACVLAN_HASH_SIZE; i++) {
 		hlist_for_each_entry_rcu(vlan, n, &port->vlan_hash[i], hlist) {
-			dev = vlan->dev;
-			rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
-
 			nskb = skb_clone(skb, GFP_ATOMIC);
-			if (nskb == NULL) {
-				rx_stats->rx_errors++;
-				continue;
-			}
-
-			rx_stats->rx_bytes += skb->len + ETH_HLEN;
-			rx_stats->rx_packets++;
-			rx_stats->multicast++;
-
-			nskb->dev = dev;
-			if (!compare_ether_addr_64bits(eth->h_dest, dev->broadcast))
-				nskb->pkt_type = PACKET_BROADCAST;
-			else
-				nskb->pkt_type = PACKET_MULTICAST;
-
-			netif_rx(nskb);
+			err = macvlan_broadcast_one(nskb, vlan->dev, eth);
+			macvlan_count_rx(vlan, skb->len + ETH_HLEN,
+					 err == NET_RX_SUCCESS, 1);
 		}
 	}
 }
@@ -163,7 +179,7 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 	const struct macvlan_port *port;
 	const struct macvlan_dev *vlan;
 	struct net_device *dev;
-	struct macvlan_rx_stats *rx_stats;
+	unsigned int len;
 
 	port = rcu_dereference(skb->dev->macvlan_port);
 	if (port == NULL)
@@ -183,15 +199,11 @@ static struct sk_buff *macvlan_handle_frame(struct sk_buff *skb)
 		kfree_skb(skb);
 		return NULL;
 	}
-	rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id());
+	len = skb->len + ETH_HLEN;
 	skb = skb_share_check(skb, GFP_ATOMIC);
-	if (skb == NULL) {
-		rx_stats->rx_errors++;
+	macvlan_count_rx(vlan, len, skb != NULL, 0);
+	if (!skb)
 		return NULL;
-	}
-
-	rx_stats->rx_bytes += skb->len + ETH_HLEN;
-	rx_stats->rx_packets++;
 
 	skb->dev = dev;
 	skb->pkt_type = PACKET_HOST;
-- 
1.6.3.3

^ permalink raw reply related

* [PATCHv3 1/4] veth: move loopback logic to common location
From: Arnd Bergmann @ 2009-11-26 16:07 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, bridge, linux-kernel,
	virtualization, Mark Smith, Gerhard Stenzel, Arnd Bergmann,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller
In-Reply-To: <1259251631-11617-1-git-send-email-arnd@arndb.de>

The veth driver contains code to forward an skb
from the start_xmit function of one network
device into the receive path of another device.

Moving that code into a common location lets us
reuse the code for direct forwarding of data
between macvlan ports, and possibly in other
drivers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/veth.c        |   17 +++--------------
 include/linux/netdevice.h |    2 ++
 net/core/dev.c            |   40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 2d657f2..6c4b5a2 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -155,8 +155,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct veth_net_stats *stats, *rcv_stats;
 	int length, cpu;
 
-	skb_orphan(skb);
-
 	priv = netdev_priv(dev);
 	rcv = priv->peer;
 	rcv_priv = netdev_priv(rcv);
@@ -168,20 +166,12 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (!(rcv->flags & IFF_UP))
 		goto tx_drop;
 
-	if (skb->len > (rcv->mtu + MTU_PAD))
-		goto rx_drop;
-
-        skb->tstamp.tv64 = 0;
-	skb->pkt_type = PACKET_HOST;
-	skb->protocol = eth_type_trans(skb, rcv);
 	if (dev->features & NETIF_F_NO_CSUM)
 		skb->ip_summed = rcv_priv->ip_summed;
 
-	skb->mark = 0;
-	secpath_reset(skb);
-	nf_reset(skb);
-
-	length = skb->len;
+	length = skb->len + ETH_HLEN;
+	if (dev_forward_skb(rcv, skb) != NET_RX_SUCCESS)
+		goto rx_drop;
 
 	stats->tx_bytes += length;
 	stats->tx_packets++;
@@ -189,7 +179,6 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 	rcv_stats->rx_bytes += length;
 	rcv_stats->rx_packets++;
 
-	netif_rx(skb);
 	return NETDEV_TX_OK;
 
 tx_drop:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 97873e3..9428793 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1562,6 +1562,8 @@ extern int		dev_set_mac_address(struct net_device *,
 extern int		dev_hard_start_xmit(struct sk_buff *skb,
 					    struct net_device *dev,
 					    struct netdev_queue *txq);
+extern int		dev_forward_skb(struct net_device *dev,
+					struct sk_buff *skb);
 
 extern int		netdev_budget;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index ccefa24..f8baa15 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -105,6 +105,7 @@
 #include <net/dst.h>
 #include <net/pkt_sched.h>
 #include <net/checksum.h>
+#include <net/xfrm.h>
 #include <linux/highmem.h>
 #include <linux/init.h>
 #include <linux/kmod.h>
@@ -1419,6 +1420,45 @@ static inline void net_timestamp(struct sk_buff *skb)
 		skb->tstamp.tv64 = 0;
 }
 
+/**
+ * dev_forward_skb - loopback an skb to another netif
+ *
+ * @dev: destination network device
+ * @skb: buffer to forward
+ *
+ * return values:
+ *	NET_RX_SUCCESS	(no congestion)
+ *	NET_RX_DROP     (packet was dropped)
+ *
+ * dev_forward_skb can be used for injecting an skb from the
+ * start_xmit function of one device into the receive queue
+ * of another device.
+ *
+ * The receiving device may be in another namespace, so
+ * we have to clear all information in the skb that could
+ * impact namespace isolation.
+ */
+int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
+{
+	skb_orphan(skb);
+
+	if (!(dev->flags & IFF_UP))
+		return NET_RX_DROP;
+
+	if (skb->len > (dev->mtu + dev->hard_header_len))
+		return NET_RX_DROP;
+
+	skb_dst_drop(skb);
+	skb->tstamp.tv64 = 0;
+	skb->pkt_type = PACKET_HOST;
+	skb->protocol = eth_type_trans(skb, dev);
+	skb->mark = 0;
+	secpath_reset(skb);
+	nf_reset(skb);
+	return netif_rx(skb);
+}
+EXPORT_SYMBOL_GPL(dev_forward_skb);
+
 /*
  *	Support routine. Sends outgoing frames to any network
  *	taps currently in use.
-- 
1.6.3.3

^ permalink raw reply related

* [PATCHv3 0/4] macvlan: add vepa and bridge mode
From: Arnd Bergmann @ 2009-11-26 16:07 UTC (permalink / raw)
  To: netdev
  Cc: Herbert Xu, Eric Dumazet, Anna Fischer, bridge, linux-kernel,
	virtualization, Mark Smith, Gerhard Stenzel, Arnd Bergmann,
	Eric W. Biederman, Jens Osterkamp, Patrick Mullaney,
	Stephen Hemminger, David Miller

Not many changes this time, just integrated a bug fix
and all the coding style feedback from Eric Dumazet
and Patrick McHardy.

I'll keep the patch for network namespaces on the tx
path out of this series for now, because the discussion
is still ongoing and it addresses an unrelated issue.

---

Version 2 description:
The patch to iproute2 has not changed, so I'm not including
it this time. Patch 4/4 (the netlink interface) is basically
unchanged as well but included for completeness.

The other changes have moved forward a bit, to the point where
I find them a lot cleaner and am more confident in the code
being ready for inclusion. The implementation hardly resembles
Erics original patch now, so I've dropped his signed-off-by.

Please take a look and ack if you are happy so we can get it
into 2.6.33.

---

Version 1 description:
This is based on an earlier patch from Eric Biederman adding
forwarding between macvlans. I extended his approach to
allow the administrator to choose the mode for each macvlan,
and to implement a functional VEPA between macvlan.

Still missing from this is support for communication between
the lower device that the macvlans are based on. This would
be extremely useful but as others have found out before me
requires significant changes not only to macvlan but also
to the common transmit path.

I've tested VEPA operation with the hairpin support
added to the bridge driver by Anna Fischer.

---
Arnd Bergmann (4):
  veth: move loopback logic to common location
  macvlan: cleanup rx statistics
  macvlan: implement bridge, VEPA and private mode
  macvlan: export macvlan mode through netlink

 drivers/net/macvlan.c     |  188 +++++++++++++++++++++++++++++++++++++--------
 drivers/net/veth.c        |   17 +----
 include/linux/if_link.h   |   15 ++++
 include/linux/netdevice.h |    2 +
 net/core/dev.c            |   40 ++++++++++
 5 files changed, 214 insertions(+), 48 deletions(-)

^ permalink raw reply


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