Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
> I found some bugs, see below.
> Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
>
> On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
> > This addes multiqueue support to virtio_net driver. In multiple queue
> > modes, the driver expects the number of queue paris is equal to the
> > number of vcpus. To eliminate the contention bettwen vcpus and
> > virtqueues, per-cpu virtqueue pairs
> > were implemented through:
> Lots of typos above - try running ispell on it :)
>
Sure.
> > - select the txq based on the smp processor id.
> > - smp affinity hint were set to the vcpu that owns the queue pairs.
> >
> > Signed-off-by: Krishna Kumar
> > Signed-off-by: Jason Wang
> > ---
> >
> > drivers/net/virtio_net.c| 472
> > ++- include/uapi/linux/virtio_net.h
> > | 16 ++
> > 2 files changed, 385 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 266f712..912f5b2 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -81,16 +81,25 @@ struct virtnet_info {
> >
> > struct virtio_device *vdev;
> > struct virtqueue *cvq;
> > struct net_device *dev;
> >
> > - struct send_queue sq;
> > - struct receive_queue rq;
> > + struct send_queue *sq;
> > + struct receive_queue *rq;
> >
> > unsigned int status;
> >
> > + /* Max # of queue pairs supported by the device */
> > + u16 max_queue_pairs;
> > +
> > + /* # of queue pairs currently used by the driver */
> > + u16 curr_queue_pairs;
> > +
> >
> > /* I like... big packets and I cannot lie! */
> > bool big_packets;
> >
> > /* Host will merge rx buffers for big packets (shake it! shake it!) */
> > bool mergeable_rx_bufs;
> >
> > + /* Has control virtqueue */
> > + bool has_cvq;
> > +
> >
> > /* enable config space updates */
> > bool config_enable;
> >
> > @@ -125,6 +134,32 @@ struct padded_vnet_hdr {
> >
> > char padding[6];
> >
> > };
> >
> > +static const struct ethtool_ops virtnet_ethtool_ops;
> > +
> > +
> > +/* Converting between virtqueue no. and kernel tx/rx queue no.
> > + * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> > + */
> > +static int vq2txq(struct virtqueue *vq)
> > +{
> > + return (virtqueue_get_queue_index(vq) - 1) / 2;
> > +}
> > +
> > +static int txq2vq(int txq)
> > +{
> > + return txq * 2 + 1;
> > +}
> > +
> > +static int vq2rxq(struct virtqueue *vq)
> > +{
> > + return virtqueue_get_queue_index(vq) / 2;
> > +}
> > +
> > +static int rxq2vq(int rxq)
> > +{
> > + return rxq * 2;
> > +}
> > +
> >
> > static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
> > {
> >
> > return (struct skb_vnet_hdr *)skb->cb;
> >
> > @@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
> >
> > virtqueue_disable_cb(vq);
> >
> > /* We were probably waiting for more output buffers. */
> >
> > - netif_wake_queue(vi->dev);
> > + netif_wake_subqueue(vi->dev, vq2txq(vq));
> >
> > }
> >
> > static void set_skb_frag(struct sk_buff *skb, struct page *page,
> >
> > @@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
> > gfp_t gfp)>
> > static void skb_recv_done(struct virtqueue *rvq)
> > {
> >
> > struct virtnet_info *vi = rvq->vdev->priv;
> >
> > - struct receive_queue *rq = >rq;
> > + struct receive_queue *rq = >rq[vq2rxq(rvq)];
> >
> > /* Schedule NAPI, Suppress further interrupts if successful. */
> > if (napi_schedule_prep(>napi)) {
> >
> > @@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
> >
> > struct virtnet_info *vi =
> >
> > container_of(work, struct virtnet_info, refill.work);
> >
> > bool still_empty;
> >
> > + int i;
> > +
> > + for (i = 0; i < vi->max_queue_pairs; i++) {
> > + struct receive_queue *rq = >rq[i];
> >
> > - napi_disable(>rq.napi);
> > - still_empty = !try_fill_recv(>rq, GFP_KERNEL);
> > - virtnet_napi_enable(>rq);
> > + napi_disable(>napi);
> > + still_empty = !try_fill_recv(rq, GFP_KERNEL);
> > + virtnet_napi_enable(rq);
> >
> > - /* In theory, this can happen: if we don't get any buffers in
> > -* we will *never* try to fill again. */
> > - if (still_empty)
> > - schedule_delayed_work(>refill, HZ/2);
> > + /* In theory, this can happen: if we don't get any buffers in
> > +* we will *never* try to fill again.
> > +*/
> > + if (still_empty)
> > + schedule_delayed_work(>refill, HZ/2);
> > + }
> >
> > }
> >
> > static int virtnet_poll(struct napi_struct *napi, int budget)
> >
> > @@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb)>
> > static netdev_tx_t start_xmit(struct
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
This addes multiqueue support to virtio_net driver. In multiple queue
modes, the driver expects the number of queue paris is equal to the
number of vcpus. To eliminate the contention bettwen vcpus and
virtqueues, per-cpu virtqueue pairs
were implemented through:
Lots of typos above - try running ispell on it :)
Sure.
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar krkum...@in.ibm.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 472
++- include/uapi/linux/virtio_net.h
| 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb-cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi-dev);
+ netif_wake_subqueue(vi-dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
gfp_t gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq-vdev-priv;
- struct receive_queue *rq = vi-rq;
+ struct receive_queue *rq = vi-rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(rq-napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct receive_queue *rq = vi-rq[i];
- napi_disable(vi-rq.napi);
- still_empty = !try_fill_recv(vi-rq, GFP_KERNEL);
- virtnet_napi_enable(vi-rq);
+ napi_disable(rq-napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
-* we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(vi-refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+* we will *never* try to fill again.
+*/
+ if (still_empty)
+ schedule_delayed_work(vi-refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct
sk_buff *skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device
*dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = vi-sq;
+ int qnum = skb_get_queue_mapping(skb);
+ struct send_queue *sq = vi-sq[qnum];
int capacity;
/*
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On 12/04/2012 11:11 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 04, 2012 at 10:45:33PM +0800, Jason Wang wrote:
>> On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
>>> I found some bugs, see below.
>>> Also some style nitpicking, this is not mandatory to address.
>> Thanks for the reviewing.
>>> On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
>>>
[...]
+ set = false;
>>> This will overwrite affinity if it was set by userspace.
>>> Just
>>> if (set)
>>> return;
>>> will not have this problem.
>> But we need handle the situtaiton when switch back to sq from mq mode.
>> Otherwise we may still get the affinity hint used in mq.
>> This kind of overwrite
>> is unavoidable or is there some method to detect whether userspac write
>> something new?
> If we didn't set the affinity originally we should not overwrite it.
> I think this means we need a flag that tells us that
> virtio set the affinity.
Ok.
[...]
+
+ /* Parameters for control virtqueue, if any */
+ if (vi->has_cvq) {
+ callbacks[total_vqs - 1] = NULL;
+ names[total_vqs - 1] = kasprintf(GFP_KERNEL, "control");
+ }
+ /* Allocate/initialize parameters for send/receive virtqueues */
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ callbacks[rxq2vq(i)] = skb_recv_done;
+ callbacks[txq2vq(i)] = skb_xmit_done;
+ names[rxq2vq(i)] = kasprintf(GFP_KERNEL, "input.%d", i);
+ names[txq2vq(i)] = kasprintf(GFP_KERNEL, "output.%d", i);
+ }
>>> We would need to check kasprintf return value.
>> Looks like a better method is to make the name as a memeber of receive_queue
>> and send_queue, and use sprintf here.
>>> Also if you allocate names from slab we'll need to free them
>>> later.
>> Then it could be freed when the send_queue and receive_queue is freed.
>>> It's probably easier to just use fixed names for now -
>>> it's not like the index is really useful.
>> Looks useful for debugging e.g. check whether the irq distribution is as
>> expected.
> Well it doesn't really matter which one goes where, right?
> As long as interrupts are distributed well.
Yes, anyway, we decide to store the name in the send/receive queue, so I
will keep the index.
>
+
+ ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
+ (const char **)names);
>>> Please avoid casts, use a proper type for names.
>> I'm consider we need a minor change in this api, we need allocate the names
>> dynamically which could not be a const char **.
> I don't see why. Any use that allocates on the fly as
> you did would leak memory. Any use like you suggest
> e.g. allocating as part of send/receive structure
> would be fine.
True
+ if (ret)
+ goto err_names;
+
+ if (vi->has_cvq) {
+ vi->cvq = vqs[total_vqs - 1];
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VLAN))
vi->dev->features |= NETIF_F_HW_VLAN_FILTER;
}
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ vi->rq[i].vq = vqs[rxq2vq(i)];
+ vi->sq[i].vq = vqs[txq2vq(i)];
+ }
+
+ kfree(callbacks);
+ kfree(vqs);
>>> Who frees names if there's no error?
>>>
>> The virtio core does not copy the name, so it need this and only used for
>> debugging if I'm reading the code correctly.
> No, virtio core does not free either individual vq name or the names
> array passed in. So this leaks memory.
Yes, so when we use the names in receive/send queue, it can be freed
during queue destroying.
[...]
> @@ -1276,24 +1531,29 @@ static int virtnet_freeze(struct virtio_device
> *vdev)>
> static int virtnet_restore(struct virtio_device *vdev)
> {
>
> struct virtnet_info *vi = vdev->priv;
>
> - int err;
> + int err, i;
>
> err = init_vqs(vi);
> if (err)
>
> return err;
>
> if (netif_running(vi->dev))
>
> - virtnet_napi_enable(>rq);
> + for (i = 0; i < vi->max_queue_pairs; i++)
> + virtnet_napi_enable(>rq[i]);
>
> netif_device_attach(vi->dev);
>
> - if (!try_fill_recv(>rq, GFP_KERNEL))
> - schedule_delayed_work(>refill, 0);
> + for (i = 0; i < vi->max_queue_pairs; i++)
> + if (!try_fill_recv(>rq[i], GFP_KERNEL))
> + schedule_delayed_work(>refill, 0);
>
> mutex_lock(>config_lock);
> vi->config_enable = true;
> mutex_unlock(>config_lock);
>
> + if (vi->has_cvq && virtio_has_feature(vi->vdev, VIRTIO_NET_F_RFS))
> + virtnet_set_queues(vi);
> +
>>> I think it's easier to test
>>> if (curr_queue_pairs == max_queue_pairs)
>>> within virtnet_set_queues and make it
>>> a NOP if so.
>> Still need to send the command during restore since we
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tue, Dec 04, 2012 at 10:45:33PM +0800, Jason Wang wrote:
> On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
> > I found some bugs, see below.
> > Also some style nitpicking, this is not mandatory to address.
>
> Thanks for the reviewing.
> >
> > On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
> > > This addes multiqueue support to virtio_net driver. In multiple queue
> > > modes, the driver expects the number of queue paris is equal to the
> > > number of vcpus. To eliminate the contention bettwen vcpus and
> > > virtqueues, per-cpu virtqueue pairs
> > > were implemented through:
> > Lots of typos above - try running ispell on it :)
> >
>
> Sure.
> > > - select the txq based on the smp processor id.
> > > - smp affinity hint were set to the vcpu that owns the queue pairs.
> > >
> > > Signed-off-by: Krishna Kumar
> > > Signed-off-by: Jason Wang
> > > ---
> > >
> > > drivers/net/virtio_net.c| 472
> > > ++- include/uapi/linux/virtio_net.h
> > > | 16 ++
> > > 2 files changed, 385 insertions(+), 103 deletions(-)
> > >
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 266f712..912f5b2 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -81,16 +81,25 @@ struct virtnet_info {
> > >
> > > struct virtio_device *vdev;
> > > struct virtqueue *cvq;
> > > struct net_device *dev;
> > >
> > > - struct send_queue sq;
> > > - struct receive_queue rq;
> > > + struct send_queue *sq;
> > > + struct receive_queue *rq;
> > >
> > > unsigned int status;
> > >
> > > + /* Max # of queue pairs supported by the device */
> > > + u16 max_queue_pairs;
> > > +
> > > + /* # of queue pairs currently used by the driver */
> > > + u16 curr_queue_pairs;
> > > +
> > >
> > > /* I like... big packets and I cannot lie! */
> > > bool big_packets;
> > >
> > > /* Host will merge rx buffers for big packets (shake it! shake it!) */
> > > bool mergeable_rx_bufs;
> > >
> > > + /* Has control virtqueue */
> > > + bool has_cvq;
> > > +
> > >
> > > /* enable config space updates */
> > > bool config_enable;
> > >
> > > @@ -125,6 +134,32 @@ struct padded_vnet_hdr {
> > >
> > > char padding[6];
> > >
> > > };
> > >
> > > +static const struct ethtool_ops virtnet_ethtool_ops;
> > > +
> > > +
> > > +/* Converting between virtqueue no. and kernel tx/rx queue no.
> > > + * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> > > + */
> > > +static int vq2txq(struct virtqueue *vq)
> > > +{
> > > + return (virtqueue_get_queue_index(vq) - 1) / 2;
> > > +}
> > > +
> > > +static int txq2vq(int txq)
> > > +{
> > > + return txq * 2 + 1;
> > > +}
> > > +
> > > +static int vq2rxq(struct virtqueue *vq)
> > > +{
> > > + return virtqueue_get_queue_index(vq) / 2;
> > > +}
> > > +
> > > +static int rxq2vq(int rxq)
> > > +{
> > > + return rxq * 2;
> > > +}
> > > +
> > >
> > > static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
> > > {
> > >
> > > return (struct skb_vnet_hdr *)skb->cb;
> > >
> > > @@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
> > >
> > > virtqueue_disable_cb(vq);
> > >
> > > /* We were probably waiting for more output buffers. */
> > >
> > > - netif_wake_queue(vi->dev);
> > > + netif_wake_subqueue(vi->dev, vq2txq(vq));
> > >
> > > }
> > >
> > > static void set_skb_frag(struct sk_buff *skb, struct page *page,
> > >
> > > @@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
> > > gfp_t gfp)>
> > > static void skb_recv_done(struct virtqueue *rvq)
> > > {
> > >
> > > struct virtnet_info *vi = rvq->vdev->priv;
> > >
> > > - struct receive_queue *rq = >rq;
> > > + struct receive_queue *rq = >rq[vq2rxq(rvq)];
> > >
> > > /* Schedule NAPI, Suppress further interrupts if successful. */
> > > if (napi_schedule_prep(>napi)) {
> > >
> > > @@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
> > >
> > > struct virtnet_info *vi =
> > >
> > > container_of(work, struct virtnet_info, refill.work);
> > >
> > > bool still_empty;
> > >
> > > + int i;
> > > +
> > > + for (i = 0; i < vi->max_queue_pairs; i++) {
> > > + struct receive_queue *rq = >rq[i];
> > >
> > > - napi_disable(>rq.napi);
> > > - still_empty = !try_fill_recv(>rq, GFP_KERNEL);
> > > - virtnet_napi_enable(>rq);
> > > + napi_disable(>napi);
> > > + still_empty = !try_fill_recv(rq, GFP_KERNEL);
> > > + virtnet_napi_enable(rq);
> > >
> > > - /* In theory, this can happen: if we don't get any buffers in
> > > - * we will *never* try to fill again. */
> > > - if (still_empty)
> > > - schedule_delayed_work(>refill, HZ/2);
> > > + /* In theory, this can happen: if we don't get any buffers in
> > > + * we will *never* try to fill again.
> > > + */
> > > + if (still_empty)
> > > +
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
> I found some bugs, see below.
> Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
>
> On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
> > This addes multiqueue support to virtio_net driver. In multiple queue
> > modes, the driver expects the number of queue paris is equal to the
> > number of vcpus. To eliminate the contention bettwen vcpus and
> > virtqueues, per-cpu virtqueue pairs
> > were implemented through:
> Lots of typos above - try running ispell on it :)
>
Sure.
> > - select the txq based on the smp processor id.
> > - smp affinity hint were set to the vcpu that owns the queue pairs.
> >
> > Signed-off-by: Krishna Kumar
> > Signed-off-by: Jason Wang
> > ---
> >
> > drivers/net/virtio_net.c| 472
> > ++- include/uapi/linux/virtio_net.h
> > | 16 ++
> > 2 files changed, 385 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 266f712..912f5b2 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -81,16 +81,25 @@ struct virtnet_info {
> >
> > struct virtio_device *vdev;
> > struct virtqueue *cvq;
> > struct net_device *dev;
> >
> > - struct send_queue sq;
> > - struct receive_queue rq;
> > + struct send_queue *sq;
> > + struct receive_queue *rq;
> >
> > unsigned int status;
> >
> > + /* Max # of queue pairs supported by the device */
> > + u16 max_queue_pairs;
> > +
> > + /* # of queue pairs currently used by the driver */
> > + u16 curr_queue_pairs;
> > +
> >
> > /* I like... big packets and I cannot lie! */
> > bool big_packets;
> >
> > /* Host will merge rx buffers for big packets (shake it! shake it!) */
> > bool mergeable_rx_bufs;
> >
> > + /* Has control virtqueue */
> > + bool has_cvq;
> > +
> >
> > /* enable config space updates */
> > bool config_enable;
> >
> > @@ -125,6 +134,32 @@ struct padded_vnet_hdr {
> >
> > char padding[6];
> >
> > };
> >
> > +static const struct ethtool_ops virtnet_ethtool_ops;
> > +
> > +
> > +/* Converting between virtqueue no. and kernel tx/rx queue no.
> > + * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> > + */
> > +static int vq2txq(struct virtqueue *vq)
> > +{
> > + return (virtqueue_get_queue_index(vq) - 1) / 2;
> > +}
> > +
> > +static int txq2vq(int txq)
> > +{
> > + return txq * 2 + 1;
> > +}
> > +
> > +static int vq2rxq(struct virtqueue *vq)
> > +{
> > + return virtqueue_get_queue_index(vq) / 2;
> > +}
> > +
> > +static int rxq2vq(int rxq)
> > +{
> > + return rxq * 2;
> > +}
> > +
> >
> > static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
> > {
> >
> > return (struct skb_vnet_hdr *)skb->cb;
> >
> > @@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
> >
> > virtqueue_disable_cb(vq);
> >
> > /* We were probably waiting for more output buffers. */
> >
> > - netif_wake_queue(vi->dev);
> > + netif_wake_subqueue(vi->dev, vq2txq(vq));
> >
> > }
> >
> > static void set_skb_frag(struct sk_buff *skb, struct page *page,
> >
> > @@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
> > gfp_t gfp)>
> > static void skb_recv_done(struct virtqueue *rvq)
> > {
> >
> > struct virtnet_info *vi = rvq->vdev->priv;
> >
> > - struct receive_queue *rq = >rq;
> > + struct receive_queue *rq = >rq[vq2rxq(rvq)];
> >
> > /* Schedule NAPI, Suppress further interrupts if successful. */
> > if (napi_schedule_prep(>napi)) {
> >
> > @@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
> >
> > struct virtnet_info *vi =
> >
> > container_of(work, struct virtnet_info, refill.work);
> >
> > bool still_empty;
> >
> > + int i;
> > +
> > + for (i = 0; i < vi->max_queue_pairs; i++) {
> > + struct receive_queue *rq = >rq[i];
> >
> > - napi_disable(>rq.napi);
> > - still_empty = !try_fill_recv(>rq, GFP_KERNEL);
> > - virtnet_napi_enable(>rq);
> > + napi_disable(>napi);
> > + still_empty = !try_fill_recv(rq, GFP_KERNEL);
> > + virtnet_napi_enable(rq);
> >
> > - /* In theory, this can happen: if we don't get any buffers in
> > -* we will *never* try to fill again. */
> > - if (still_empty)
> > - schedule_delayed_work(>refill, HZ/2);
> > + /* In theory, this can happen: if we don't get any buffers in
> > +* we will *never* try to fill again.
> > +*/
> > + if (still_empty)
> > + schedule_delayed_work(>refill, HZ/2);
> > + }
> >
> > }
> >
> > static int virtnet_poll(struct napi_struct *napi, int budget)
> >
> > @@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct
> > sk_buff *skb)>
> > static netdev_tx_t start_xmit(struct
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
> This addes multiqueue support to virtio_net driver. In multiple queue modes,
> the
> driver expects the number of queue paris is equal to the number of vcpus. To
> eliminate the contention bettwen vcpus and virtqueues, per-cpu virtqueue pairs
> were implemented through:
Lots of typos above - try running ispell on it :)
>
> - select the txq based on the smp processor id.
> - smp affinity hint were set to the vcpu that owns the queue pairs.
>
> Signed-off-by: Krishna Kumar
> Signed-off-by: Jason Wang
> ---
> drivers/net/virtio_net.c| 472
> ++-
> include/uapi/linux/virtio_net.h | 16 ++
> 2 files changed, 385 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 266f712..912f5b2 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -81,16 +81,25 @@ struct virtnet_info {
> struct virtio_device *vdev;
> struct virtqueue *cvq;
> struct net_device *dev;
> - struct send_queue sq;
> - struct receive_queue rq;
> + struct send_queue *sq;
> + struct receive_queue *rq;
> unsigned int status;
>
> + /* Max # of queue pairs supported by the device */
> + u16 max_queue_pairs;
> +
> + /* # of queue pairs currently used by the driver */
> + u16 curr_queue_pairs;
> +
> /* I like... big packets and I cannot lie! */
> bool big_packets;
>
> /* Host will merge rx buffers for big packets (shake it! shake it!) */
> bool mergeable_rx_bufs;
>
> + /* Has control virtqueue */
> + bool has_cvq;
> +
> /* enable config space updates */
> bool config_enable;
>
> @@ -125,6 +134,32 @@ struct padded_vnet_hdr {
> char padding[6];
> };
>
> +static const struct ethtool_ops virtnet_ethtool_ops;
> +
> +
> +/* Converting between virtqueue no. and kernel tx/rx queue no.
> + * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
> + */
> +static int vq2txq(struct virtqueue *vq)
> +{
> + return (virtqueue_get_queue_index(vq) - 1) / 2;
> +}
> +
> +static int txq2vq(int txq)
> +{
> + return txq * 2 + 1;
> +}
> +
> +static int vq2rxq(struct virtqueue *vq)
> +{
> + return virtqueue_get_queue_index(vq) / 2;
> +}
> +
> +static int rxq2vq(int rxq)
> +{
> + return rxq * 2;
> +}
> +
> static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
> {
> return (struct skb_vnet_hdr *)skb->cb;
> @@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
> virtqueue_disable_cb(vq);
>
> /* We were probably waiting for more output buffers. */
> - netif_wake_queue(vi->dev);
> + netif_wake_subqueue(vi->dev, vq2txq(vq));
> }
>
> static void set_skb_frag(struct sk_buff *skb, struct page *page,
> @@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t
> gfp)
> static void skb_recv_done(struct virtqueue *rvq)
> {
> struct virtnet_info *vi = rvq->vdev->priv;
> - struct receive_queue *rq = >rq;
> + struct receive_queue *rq = >rq[vq2rxq(rvq)];
>
> /* Schedule NAPI, Suppress further interrupts if successful. */
> if (napi_schedule_prep(>napi)) {
> @@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
> struct virtnet_info *vi =
> container_of(work, struct virtnet_info, refill.work);
> bool still_empty;
> + int i;
> +
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + struct receive_queue *rq = >rq[i];
>
> - napi_disable(>rq.napi);
> - still_empty = !try_fill_recv(>rq, GFP_KERNEL);
> - virtnet_napi_enable(>rq);
> + napi_disable(>napi);
> + still_empty = !try_fill_recv(rq, GFP_KERNEL);
> + virtnet_napi_enable(rq);
>
> - /* In theory, this can happen: if we don't get any buffers in
> - * we will *never* try to fill again. */
> - if (still_empty)
> - schedule_delayed_work(>refill, HZ/2);
> + /* In theory, this can happen: if we don't get any buffers in
> + * we will *never* try to fill again.
> + */
> + if (still_empty)
> + schedule_delayed_work(>refill, HZ/2);
> + }
> }
>
> static int virtnet_poll(struct napi_struct *napi, int budget)
> @@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
> *skb)
> static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> - struct send_queue *sq = >sq;
> + int qnum = skb_get_queue_mapping(skb);
> + struct send_queue *sq = >sq[qnum];
> int capacity;
>
> /* Free up any pending old buffers before queueing new ones. */
> @@ -664,13 +706,14 @@ static netdev_tx_t start_xmit(struct sk_buff
[PATCH net-next 2/3] virtio_net: multiqueue support
This addes multiqueue support to virtio_net driver. In multiple queue modes, the
driver expects the number of queue paris is equal to the number of vcpus. To
eliminate the contention bettwen vcpus and virtqueues, per-cpu virtqueue pairs
were implemented through:
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar
Signed-off-by: Jason Wang
---
drivers/net/virtio_net.c| 472 ++-
include/uapi/linux/virtio_net.h | 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb->cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi->dev);
+ netif_wake_subqueue(vi->dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t
gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq->vdev->priv;
- struct receive_queue *rq = >rq;
+ struct receive_queue *rq = >rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(>napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i < vi->max_queue_pairs; i++) {
+ struct receive_queue *rq = >rq[i];
- napi_disable(>rq.napi);
- still_empty = !try_fill_recv(>rq, GFP_KERNEL);
- virtnet_napi_enable(>rq);
+ napi_disable(>napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
-* we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(>refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+* we will *never* try to fill again.
+*/
+ if (still_empty)
+ schedule_delayed_work(>refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = >sq;
+ int qnum = skb_get_queue_mapping(skb);
+ struct send_queue *sq = >sq[qnum];
int capacity;
/* Free up any pending old buffers before queueing new ones. */
@@ -664,13 +706,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
if (likely(capacity == -ENOMEM)) {
if (net_ratelimit())
dev_warn(>dev,
-"TX queue failure: out of memory\n");
+"TXQ (%d) failure: out of memory\n",
+
[PATCH net-next 2/3] virtio_net: multiqueue support
This addes multiqueue support to virtio_net driver. In multiple queue modes, the
driver expects the number of queue paris is equal to the number of vcpus. To
eliminate the contention bettwen vcpus and virtqueues, per-cpu virtqueue pairs
were implemented through:
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar krkum...@in.ibm.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 472 ++-
include/uapi/linux/virtio_net.h | 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb-cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi-dev);
+ netif_wake_subqueue(vi-dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t
gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq-vdev-priv;
- struct receive_queue *rq = vi-rq;
+ struct receive_queue *rq = vi-rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(rq-napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct receive_queue *rq = vi-rq[i];
- napi_disable(vi-rq.napi);
- still_empty = !try_fill_recv(vi-rq, GFP_KERNEL);
- virtnet_napi_enable(vi-rq);
+ napi_disable(rq-napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
-* we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(vi-refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+* we will *never* try to fill again.
+*/
+ if (still_empty)
+ schedule_delayed_work(vi-refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = vi-sq;
+ int qnum = skb_get_queue_mapping(skb);
+ struct send_queue *sq = vi-sq[qnum];
int capacity;
/* Free up any pending old buffers before queueing new ones. */
@@ -664,13 +706,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct
net_device *dev)
if (likely(capacity == -ENOMEM)) {
if (net_ratelimit())
dev_warn(dev-dev,
-TX queue failure: out of memory\n);
+TXQ (%d) failure:
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
This addes multiqueue support to virtio_net driver. In multiple queue modes,
the
driver expects the number of queue paris is equal to the number of vcpus. To
eliminate the contention bettwen vcpus and virtqueues, per-cpu virtqueue pairs
were implemented through:
Lots of typos above - try running ispell on it :)
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar krkum...@in.ibm.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 472
++-
include/uapi/linux/virtio_net.h | 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb-cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi-dev);
+ netif_wake_subqueue(vi-dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t
gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq-vdev-priv;
- struct receive_queue *rq = vi-rq;
+ struct receive_queue *rq = vi-rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(rq-napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct receive_queue *rq = vi-rq[i];
- napi_disable(vi-rq.napi);
- still_empty = !try_fill_recv(vi-rq, GFP_KERNEL);
- virtnet_napi_enable(vi-rq);
+ napi_disable(rq-napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
- * we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(vi-refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+ * we will *never* try to fill again.
+ */
+ if (still_empty)
+ schedule_delayed_work(vi-refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff
*skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = vi-sq;
+ int qnum = skb_get_queue_mapping(skb);
+ struct send_queue *sq = vi-sq[qnum];
int capacity;
/* Free up any pending old buffers before queueing new ones. */
@@ -664,13 +706,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb,
struct net_device *dev)
if (likely(capacity == -ENOMEM)) {
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
This addes multiqueue support to virtio_net driver. In multiple queue
modes, the driver expects the number of queue paris is equal to the
number of vcpus. To eliminate the contention bettwen vcpus and
virtqueues, per-cpu virtqueue pairs
were implemented through:
Lots of typos above - try running ispell on it :)
Sure.
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar krkum...@in.ibm.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 472
++- include/uapi/linux/virtio_net.h
| 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb-cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi-dev);
+ netif_wake_subqueue(vi-dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
gfp_t gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq-vdev-priv;
- struct receive_queue *rq = vi-rq;
+ struct receive_queue *rq = vi-rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(rq-napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct receive_queue *rq = vi-rq[i];
- napi_disable(vi-rq.napi);
- still_empty = !try_fill_recv(vi-rq, GFP_KERNEL);
- virtnet_napi_enable(vi-rq);
+ napi_disable(rq-napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
-* we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(vi-refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+* we will *never* try to fill again.
+*/
+ if (still_empty)
+ schedule_delayed_work(vi-refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct
sk_buff *skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device
*dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = vi-sq;
+ int qnum = skb_get_queue_mapping(skb);
+ struct send_queue *sq = vi-sq[qnum];
int capacity;
/*
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On Tue, Dec 04, 2012 at 10:45:33PM +0800, Jason Wang wrote:
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
This addes multiqueue support to virtio_net driver. In multiple queue
modes, the driver expects the number of queue paris is equal to the
number of vcpus. To eliminate the contention bettwen vcpus and
virtqueues, per-cpu virtqueue pairs
were implemented through:
Lots of typos above - try running ispell on it :)
Sure.
- select the txq based on the smp processor id.
- smp affinity hint were set to the vcpu that owns the queue pairs.
Signed-off-by: Krishna Kumar krkum...@in.ibm.com
Signed-off-by: Jason Wang jasow...@redhat.com
---
drivers/net/virtio_net.c| 472
++- include/uapi/linux/virtio_net.h
| 16 ++
2 files changed, 385 insertions(+), 103 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 266f712..912f5b2 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -81,16 +81,25 @@ struct virtnet_info {
struct virtio_device *vdev;
struct virtqueue *cvq;
struct net_device *dev;
- struct send_queue sq;
- struct receive_queue rq;
+ struct send_queue *sq;
+ struct receive_queue *rq;
unsigned int status;
+ /* Max # of queue pairs supported by the device */
+ u16 max_queue_pairs;
+
+ /* # of queue pairs currently used by the driver */
+ u16 curr_queue_pairs;
+
/* I like... big packets and I cannot lie! */
bool big_packets;
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
+ /* Has control virtqueue */
+ bool has_cvq;
+
/* enable config space updates */
bool config_enable;
@@ -125,6 +134,32 @@ struct padded_vnet_hdr {
char padding[6];
};
+static const struct ethtool_ops virtnet_ethtool_ops;
+
+
+/* Converting between virtqueue no. and kernel tx/rx queue no.
+ * 0:rx0 1:tx0 2:rx1 3:tx1 ... 2N:rxN 2N+1:txN 2N+2:cvq
+ */
+static int vq2txq(struct virtqueue *vq)
+{
+ return (virtqueue_get_queue_index(vq) - 1) / 2;
+}
+
+static int txq2vq(int txq)
+{
+ return txq * 2 + 1;
+}
+
+static int vq2rxq(struct virtqueue *vq)
+{
+ return virtqueue_get_queue_index(vq) / 2;
+}
+
+static int rxq2vq(int rxq)
+{
+ return rxq * 2;
+}
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb-cb;
@@ -165,7 +200,7 @@ static void skb_xmit_done(struct virtqueue *vq)
virtqueue_disable_cb(vq);
/* We were probably waiting for more output buffers. */
- netif_wake_queue(vi-dev);
+ netif_wake_subqueue(vi-dev, vq2txq(vq));
}
static void set_skb_frag(struct sk_buff *skb, struct page *page,
@@ -502,7 +537,7 @@ static bool try_fill_recv(struct receive_queue *rq,
gfp_t gfp)
static void skb_recv_done(struct virtqueue *rvq)
{
struct virtnet_info *vi = rvq-vdev-priv;
- struct receive_queue *rq = vi-rq;
+ struct receive_queue *rq = vi-rq[vq2rxq(rvq)];
/* Schedule NAPI, Suppress further interrupts if successful. */
if (napi_schedule_prep(rq-napi)) {
@@ -532,15 +567,21 @@ static void refill_work(struct work_struct *work)
struct virtnet_info *vi =
container_of(work, struct virtnet_info, refill.work);
bool still_empty;
+ int i;
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ struct receive_queue *rq = vi-rq[i];
- napi_disable(vi-rq.napi);
- still_empty = !try_fill_recv(vi-rq, GFP_KERNEL);
- virtnet_napi_enable(vi-rq);
+ napi_disable(rq-napi);
+ still_empty = !try_fill_recv(rq, GFP_KERNEL);
+ virtnet_napi_enable(rq);
- /* In theory, this can happen: if we don't get any buffers in
- * we will *never* try to fill again. */
- if (still_empty)
- schedule_delayed_work(vi-refill, HZ/2);
+ /* In theory, this can happen: if we don't get any buffers in
+ * we will *never* try to fill again.
+ */
+ if (still_empty)
+ schedule_delayed_work(vi-refill, HZ/2);
+ }
}
static int virtnet_poll(struct napi_struct *napi, int budget)
@@ -650,7 +691,8 @@ static int xmit_skb(struct send_queue *sq, struct
sk_buff *skb)
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device
*dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- struct send_queue *sq = vi-sq;
+ int qnum =
Re: [PATCH net-next 2/3] virtio_net: multiqueue support
On 12/04/2012 11:11 PM, Michael S. Tsirkin wrote:
On Tue, Dec 04, 2012 at 10:45:33PM +0800, Jason Wang wrote:
On Tuesday, December 04, 2012 03:24:22 PM Michael S. Tsirkin wrote:
I found some bugs, see below.
Also some style nitpicking, this is not mandatory to address.
Thanks for the reviewing.
On Tue, Dec 04, 2012 at 07:07:57PM +0800, Jason Wang wrote:
[...]
+ set = false;
This will overwrite affinity if it was set by userspace.
Just
if (set)
return;
will not have this problem.
But we need handle the situtaiton when switch back to sq from mq mode.
Otherwise we may still get the affinity hint used in mq.
This kind of overwrite
is unavoidable or is there some method to detect whether userspac write
something new?
If we didn't set the affinity originally we should not overwrite it.
I think this means we need a flag that tells us that
virtio set the affinity.
Ok.
[...]
+
+ /* Parameters for control virtqueue, if any */
+ if (vi-has_cvq) {
+ callbacks[total_vqs - 1] = NULL;
+ names[total_vqs - 1] = kasprintf(GFP_KERNEL, control);
+ }
+ /* Allocate/initialize parameters for send/receive virtqueues */
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ callbacks[rxq2vq(i)] = skb_recv_done;
+ callbacks[txq2vq(i)] = skb_xmit_done;
+ names[rxq2vq(i)] = kasprintf(GFP_KERNEL, input.%d, i);
+ names[txq2vq(i)] = kasprintf(GFP_KERNEL, output.%d, i);
+ }
We would need to check kasprintf return value.
Looks like a better method is to make the name as a memeber of receive_queue
and send_queue, and use sprintf here.
Also if you allocate names from slab we'll need to free them
later.
Then it could be freed when the send_queue and receive_queue is freed.
It's probably easier to just use fixed names for now -
it's not like the index is really useful.
Looks useful for debugging e.g. check whether the irq distribution is as
expected.
Well it doesn't really matter which one goes where, right?
As long as interrupts are distributed well.
Yes, anyway, we decide to store the name in the send/receive queue, so I
will keep the index.
+
+ ret = vi-vdev-config-find_vqs(vi-vdev, total_vqs, vqs, callbacks,
+ (const char **)names);
Please avoid casts, use a proper type for names.
I'm consider we need a minor change in this api, we need allocate the names
dynamically which could not be a const char **.
I don't see why. Any use that allocates on the fly as
you did would leak memory. Any use like you suggest
e.g. allocating as part of send/receive structure
would be fine.
True
+ if (ret)
+ goto err_names;
+
+ if (vi-has_cvq) {
+ vi-cvq = vqs[total_vqs - 1];
if (virtio_has_feature(vi-vdev, VIRTIO_NET_F_CTRL_VLAN))
vi-dev-features |= NETIF_F_HW_VLAN_FILTER;
}
+
+ for (i = 0; i vi-max_queue_pairs; i++) {
+ vi-rq[i].vq = vqs[rxq2vq(i)];
+ vi-sq[i].vq = vqs[txq2vq(i)];
+ }
+
+ kfree(callbacks);
+ kfree(vqs);
Who frees names if there's no error?
The virtio core does not copy the name, so it need this and only used for
debugging if I'm reading the code correctly.
No, virtio core does not free either individual vq name or the names
array passed in. So this leaks memory.
Yes, so when we use the names in receive/send queue, it can be freed
during queue destroying.
[...]
@@ -1276,24 +1531,29 @@ static int virtnet_freeze(struct virtio_device
*vdev)
static int virtnet_restore(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev-priv;
- int err;
+ int err, i;
err = init_vqs(vi);
if (err)
return err;
if (netif_running(vi-dev))
- virtnet_napi_enable(vi-rq);
+ for (i = 0; i vi-max_queue_pairs; i++)
+ virtnet_napi_enable(vi-rq[i]);
netif_device_attach(vi-dev);
- if (!try_fill_recv(vi-rq, GFP_KERNEL))
- schedule_delayed_work(vi-refill, 0);
+ for (i = 0; i vi-max_queue_pairs; i++)
+ if (!try_fill_recv(vi-rq[i], GFP_KERNEL))
+ schedule_delayed_work(vi-refill, 0);
mutex_lock(vi-config_lock);
vi-config_enable = true;
mutex_unlock(vi-config_lock);
+ if (vi-has_cvq virtio_has_feature(vi-vdev, VIRTIO_NET_F_RFS))
+ virtnet_set_queues(vi);
+
I think it's easier to test
if (curr_queue_pairs == max_queue_pairs)
within virtnet_set_queues and make it
a NOP if so.
Still need to send the command during restore since we reset the device
during
freezing.
Then maybe check vi-has_cvq virtio_has_feature(vi-vdev,
VIRTIO_NET_F_RFS) in there?
Right.
[...]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
