* [PATCHv2 RFC 1/4] virtio_ring: add capacity check API
From: Michael S. Tsirkin @ 2011-06-02 15:42 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <cover.1307029008.git.mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Add API to check ring capacity. Because of the option
to use indirect buffers, this returns the worst
case, not the normal case capacity.
Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
drivers/virtio/virtio_ring.c | 8 ++++++++
include/linux/virtio.h | 5 +++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 68b9136..23422f1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -344,6 +344,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
}
EXPORT_SYMBOL_GPL(virtqueue_get_buf);
+int virtqueue_min_capacity(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_min_capacity);
+
void virtqueue_disable_cb(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7108857..209220d 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
* vq: the struct virtqueue we're talking about.
* len: the length written into the buffer
* Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_min_capacity: get the current capacity of the queue
+ * vq: the struct virtqueue we're talking about.
+ * Returns the current worst case capacity of the queue.
* virtqueue_disable_cb: disable callbacks
* vq: the struct virtqueue we're talking about.
* Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
+int virtqueue_min_capacity(struct virtqueue *vq);
+
void virtqueue_disable_cb(struct virtqueue *vq);
bool virtqueue_enable_cb(struct virtqueue *vq);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 RFC 0/4] virtio and vhost-net capacity handling
From: Michael S. Tsirkin @ 2011-06-02 15:42 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
OK, here's a new attempt to use the new capacity api. I also added more
comments to clarify the logic. Hope this is more readable. Let me know
pls.
This is on top of the patches applied by Rusty.
Warning: untested. Posting now to give people chance to
comment on the API.
Changes from v1:
- fix comment in patch 2 to correct confusion noted by Rusty
- rewrite patch 3 along the lines suggested by Rusty
note: it's not exactly the same but I hope it's close
enough, the main difference is that mine does limited
polling even in the unlikely xmit failure case.
- added a patch to not return capacity from add_buf
it always looked like a weird hack
Michael S. Tsirkin (4):
virtio_ring: add capacity check API
virtio_net: fix tx capacity checks using new API
virtio_net: limit xmit polling
Revert "virtio: make add_buf return capacity remaining:
drivers/net/virtio_net.c | 111 ++++++++++++++++++++++++++----------------
drivers/virtio/virtio_ring.c | 10 +++-
include/linux/virtio.h | 7 ++-
3 files changed, 84 insertions(+), 44 deletions(-)
--
1.7.5.53.gc233e
^ permalink raw reply
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-02 15:34 UTC (permalink / raw)
To: Krishna Kumar2
Cc: habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
lguest-uLR06cmDAlY/bJ5BZ2RsiQ, Shirley Ma,
kvm-u79uwXL29TY76Z2rM5mHXA, Carsten Otte,
linux-s390-u79uwXL29TY76Z2rM5mHXA, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <OFFF109303.D838CAE2-ON652578A3.0053357D-652578A3.00549366-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org>
On Thu, Jun 02, 2011 at 08:56:42PM +0530, Krishna Kumar2 wrote:
> "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote on 06/02/2011 08:13:46 PM:
>
> > > Please review this patch to see if it looks reasonable:
> >
> > Hmm, since you decided to work on top of my patch,
> > I'd appreciate split-up fixes.
>
> OK (that also explains your next comment).
>
> > > 1. Picked comments/code from MST's code and Rusty's review.
> > > 2. virtqueue_min_capacity() needs to be called only if it returned
> > > empty the last time it was called.
> > > 3. Fix return value bug in free_old_xmit_skbs (hangs guest).
> > > 4. Stop queue only if capacity is not enough for next xmit.
> >
> > That's what we always did ...
>
> I had made the patch against your patch, hence this change (sorry for
> the confusion!).
>
> > > 5. Fix/clean some likely/unlikely checks (hopefully).
> > >
> > > I have done some minimal netperf tests with this.
> > >
> > > With this patch, add_buf returning capacity seems to be useful - it
> > > allows less virtio API calls.
> >
> > Why bother? It's cheap ...
>
> If add_buf retains it's functionality to return the capacity (it
> is going to need a change to return 0 otherwise anyway), is it
> useful to call another function at each xmit?
>
> > > +static bool free_old_xmit_skbs(struct virtnet_info *vi, int to_free)
> > > +{
> > > + bool empty = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2;
> > > +
> > > + do {
> > > + if (!free_one_old_xmit_skb(vi)) {
> > > + /* No more skbs to free up */
> > > break;
> > > - pr_debug("Sent skb %p\n", skb);
> > > - vi->dev->stats.tx_bytes += skb->len;
> > > - vi->dev->stats.tx_packets++;
> > > - dev_kfree_skb_any(skb);
> > > - }
> > > - return r;
> > > + }
> > > +
> > > + if (empty) {
> > > + /* Check again if there is enough space */
> > > + empty = virtqueue_min_capacity(vi->svq) <
> > > + MAX_SKB_FRAGS + 2;
> > > + } else {
> > > + --to_free;
> > > + }
> > > + } while (to_free > 0);
> > > +
> > > + return !empty;
> > > }
> >
> > Why bother doing the capacity check in this function?
>
> To return whether we have enough space for next xmit. It should call
> it only once unless space is running out. Does it sound OK?
>
> > > - if (unlikely(ret < 0)) {
> > > + if (unlikely(capacity < 0)) {
> > > + /*
> > > + * Failure to queue should be impossible. The only way to
> > > + * reach here is if we got a cb before 3/4th of space was
> > > + * available. We could stop the queue and re-enable
> > > + * callbacks (and possibly return TX_BUSY), but we don't
> > > + * bother since this is impossible.
> >
> > It's far from impossible. The 3/4 thing is only a hint, and old devices
> > don't support it anyway.
>
> OK, I will re-put back your comment.
>
> > > - if (!likely(free_old_xmit_skbs(vi, 2))) {
> > > - netif_stop_queue(dev);
> > > - if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> > > - /* More just got used, free them and recheck. */
> > > - if (!likely(free_old_xmit_skbs(vi, 0))) {
> > > - netif_start_queue(dev);
> > > - virtqueue_disable_cb(vi->svq);
> > > + /*
> > > + * Apparently nice girls don't return TX_BUSY; check capacity and
> > > + * stop the queue before it gets out of hand. Naturally, this
> wastes
> > > + * entries.
> > > + */
> > > + if (capacity < 2+MAX_SKB_FRAGS) {
> > > + /*
> > > + * We don't have enough space for the next packet. Try
> > > + * freeing more.
> > > + */
> > > + if (likely(!free_old_xmit_skbs(vi, UINT_MAX))) {
> > > + netif_stop_queue(dev);
> > > + if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> > > + /* More just got used, free them and recheck. */
> > > + if (likely(free_old_xmit_skbs(vi, UINT_MAX))) {
> >
> > Is this where the bug was?
>
> Return value in free_old_xmit() was wrong. I will re-do against the
> mainline kernel.
>
> Thanks,
>
> - KK
Just noting that I'm working on that patch as well, it might
be more efficient if we don't both of us do this in parallel :)
--
MST
^ permalink raw reply
* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Eric W. Biederman @ 2011-06-02 15:26 UTC (permalink / raw)
To: Changli Gao
Cc: David Miller, shemminger, greearb, nicolas.2p.debian, jpirko,
netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <BANLkTimjfHKh3dpPEzznKiK=Md4v0iTD9A@mail.gmail.com>
Changli Gao <xiaosuo@gmail.com> writes:
> On Thu, Jun 2, 2011 at 9:03 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> -static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
>> +static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
>> {
>> - if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
>> - if (skb_cow(skb, skb_headroom(skb)) < 0)
>> - skb = NULL;
>> - if (skb) {
>> - /* Lifted from Gleb's VLAN code... */
>> - memmove(skb->data - ETH_HLEN,
>> - skb->data - VLAN_ETH_HLEN, 12);
>> - skb->mac_header += VLAN_HLEN;
>> - }
>> + if (skb_cow(skb, skb_headroom(skb)) < 0)
>> + skb = NULL;
>> + if (skb) {
>
> I think an else branch maybe more readable here.
Probably most readable would be simply returning NULL immediately on
error. In this patch I just removed the if statement, and I would like
to avoid mixing different bug fixes in the same patch if possible.
>> + /* Lifted from Gleb's VLAN code... */
>> + memmove(skb->data - ETH_HLEN,
>> + skb->data - VLAN_ETH_HLEN, 12);
>> + skb->mac_header += VLAN_HLEN;
>
> skb->mac_len should be adjusted too.
Given how vlan_untag is called at the moment it does appear
that the skb->mac_len = skb->network_header - skb->mac_header
in __netif_receive_skb that used to handle this for is no longer
doing this for us.
My feel is that either we need to do all of the header resets and the
vlan_untagging together. So we either need this all together before or
after the another_round label:
So the proper fix is probably something like this.
diff --git a/net/core/dev.c b/net/core/dev.c
index bcb05cb..8fe50d4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3102,9 +3102,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
skb->skb_iif = skb->dev->ifindex;
orig_dev = skb->dev;
- skb_reset_network_header(skb);
- skb_reset_transport_header(skb);
- skb->mac_len = skb->network_header - skb->mac_header;
pt_prev = NULL;
@@ -3119,6 +3116,9 @@ another_round:
if (unlikely(!skb))
goto out;
}
+ skb_reset_network_header(skb);
+ skb_reset_transport_header(skb);
+ skb->mac_len = skb->network_header - skb->mac_header;
#ifdef CONFIG_NET_CLS_ACT
if (skb->tc_verd & TC_NCLS) {
^ permalink raw reply related
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Krishna Kumar2 @ 2011-06-02 15:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
lguest-uLR06cmDAlY/bJ5BZ2RsiQ, Shirley Ma,
kvm-u79uwXL29TY76Z2rM5mHXA, Carsten Otte,
linux-s390-u79uwXL29TY76Z2rM5mHXA, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <20110602144346.GA7995-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
"Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote on 06/02/2011 08:13:46 PM:
> > Please review this patch to see if it looks reasonable:
>
> Hmm, since you decided to work on top of my patch,
> I'd appreciate split-up fixes.
OK (that also explains your next comment).
> > 1. Picked comments/code from MST's code and Rusty's review.
> > 2. virtqueue_min_capacity() needs to be called only if it returned
> > empty the last time it was called.
> > 3. Fix return value bug in free_old_xmit_skbs (hangs guest).
> > 4. Stop queue only if capacity is not enough for next xmit.
>
> That's what we always did ...
I had made the patch against your patch, hence this change (sorry for
the confusion!).
> > 5. Fix/clean some likely/unlikely checks (hopefully).
> >
> > I have done some minimal netperf tests with this.
> >
> > With this patch, add_buf returning capacity seems to be useful - it
> > allows less virtio API calls.
>
> Why bother? It's cheap ...
If add_buf retains it's functionality to return the capacity (it
is going to need a change to return 0 otherwise anyway), is it
useful to call another function at each xmit?
> > +static bool free_old_xmit_skbs(struct virtnet_info *vi, int to_free)
> > +{
> > + bool empty = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2;
> > +
> > + do {
> > + if (!free_one_old_xmit_skb(vi)) {
> > + /* No more skbs to free up */
> > break;
> > - pr_debug("Sent skb %p\n", skb);
> > - vi->dev->stats.tx_bytes += skb->len;
> > - vi->dev->stats.tx_packets++;
> > - dev_kfree_skb_any(skb);
> > - }
> > - return r;
> > + }
> > +
> > + if (empty) {
> > + /* Check again if there is enough space */
> > + empty = virtqueue_min_capacity(vi->svq) <
> > + MAX_SKB_FRAGS + 2;
> > + } else {
> > + --to_free;
> > + }
> > + } while (to_free > 0);
> > +
> > + return !empty;
> > }
>
> Why bother doing the capacity check in this function?
To return whether we have enough space for next xmit. It should call
it only once unless space is running out. Does it sound OK?
> > - if (unlikely(ret < 0)) {
> > + if (unlikely(capacity < 0)) {
> > + /*
> > + * Failure to queue should be impossible. The only way to
> > + * reach here is if we got a cb before 3/4th of space was
> > + * available. We could stop the queue and re-enable
> > + * callbacks (and possibly return TX_BUSY), but we don't
> > + * bother since this is impossible.
>
> It's far from impossible. The 3/4 thing is only a hint, and old devices
> don't support it anyway.
OK, I will re-put back your comment.
> > - if (!likely(free_old_xmit_skbs(vi, 2))) {
> > - netif_stop_queue(dev);
> > - if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> > - /* More just got used, free them and recheck. */
> > - if (!likely(free_old_xmit_skbs(vi, 0))) {
> > - netif_start_queue(dev);
> > - virtqueue_disable_cb(vi->svq);
> > + /*
> > + * Apparently nice girls don't return TX_BUSY; check capacity and
> > + * stop the queue before it gets out of hand. Naturally, this
wastes
> > + * entries.
> > + */
> > + if (capacity < 2+MAX_SKB_FRAGS) {
> > + /*
> > + * We don't have enough space for the next packet. Try
> > + * freeing more.
> > + */
> > + if (likely(!free_old_xmit_skbs(vi, UINT_MAX))) {
> > + netif_stop_queue(dev);
> > + if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> > + /* More just got used, free them and recheck. */
> > + if (likely(free_old_xmit_skbs(vi, UINT_MAX))) {
>
> Is this where the bug was?
Return value in free_old_xmit() was wrong. I will re-do against the
mainline kernel.
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Changli Gao @ 2011-06-02 14:54 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, shemminger, greearb, nicolas.2p.debian, jpirko,
netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m11uzcidvq.fsf_-_@fess.ebiederm.org>
On Thu, Jun 2, 2011 at 9:03 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> -static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
> +static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
> {
> - if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
> - if (skb_cow(skb, skb_headroom(skb)) < 0)
> - skb = NULL;
> - if (skb) {
> - /* Lifted from Gleb's VLAN code... */
> - memmove(skb->data - ETH_HLEN,
> - skb->data - VLAN_ETH_HLEN, 12);
> - skb->mac_header += VLAN_HLEN;
> - }
> + if (skb_cow(skb, skb_headroom(skb)) < 0)
> + skb = NULL;
> + if (skb) {
I think an else branch maybe more readable here.
> + /* Lifted from Gleb's VLAN code... */
> + memmove(skb->data - ETH_HLEN,
> + skb->data - VLAN_ETH_HLEN, 12);
> + skb->mac_header += VLAN_HLEN;
skb->mac_len should be adjusted too.
> }
> return skb;
> }
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [TRIVIAL PATCH next 15/15] net: Convert vmalloc/memset to vzalloc
From: Pablo Neira Ayuso @ 2011-06-02 14:49 UTC (permalink / raw)
To: Joe Perches
Cc: Patrick McHardy, Andy Grover, Jiri Kosina, David S. Miller,
netfilter-devel, netfilter, coreteam, netdev, linux-kernel,
rds-devel
In-Reply-To: <045996b9688bfaa2efbd8999405a67e23cd0a075.1306603968.git.joe@perches.com>
David,
Are you going to take this patch? it includes one chunck which is out of
netfilter scope.
Thanks.
On 28/05/11 19:36, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> net/netfilter/x_tables.c | 5 ++---
> net/rds/ib_cm.c | 6 ++----
> 2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
> index b0869fe..71441b9 100644
> --- a/net/netfilter/x_tables.c
> +++ b/net/netfilter/x_tables.c
> @@ -776,12 +776,11 @@ static int xt_jumpstack_alloc(struct xt_table_info *i)
>
> size = sizeof(void **) * nr_cpu_ids;
> if (size > PAGE_SIZE)
> - i->jumpstack = vmalloc(size);
> + i->jumpstack = vzalloc(size);
> else
> - i->jumpstack = kmalloc(size, GFP_KERNEL);
> + i->jumpstack = kzalloc(size, GFP_KERNEL);
> if (i->jumpstack == NULL)
> return -ENOMEM;
> - memset(i->jumpstack, 0, size);
>
> i->stacksize *= xt_jumpstack_multiplier;
> size = sizeof(void *) * i->stacksize;
> diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
> index fd453dd..6ecaf78 100644
> --- a/net/rds/ib_cm.c
> +++ b/net/rds/ib_cm.c
> @@ -374,23 +374,21 @@ static int rds_ib_setup_qp(struct rds_connection *conn)
> goto out;
> }
>
> - ic->i_sends = vmalloc_node(ic->i_send_ring.w_nr * sizeof(struct rds_ib_send_work),
> + ic->i_sends = vzalloc_node(ic->i_send_ring.w_nr * sizeof(struct rds_ib_send_work),
> ibdev_to_node(dev));
> if (!ic->i_sends) {
> ret = -ENOMEM;
> rdsdebug("send allocation failed\n");
> goto out;
> }
> - memset(ic->i_sends, 0, ic->i_send_ring.w_nr * sizeof(struct rds_ib_send_work));
>
> - ic->i_recvs = vmalloc_node(ic->i_recv_ring.w_nr * sizeof(struct rds_ib_recv_work),
> + ic->i_recvs = vzalloc_node(ic->i_recv_ring.w_nr * sizeof(struct rds_ib_recv_work),
> ibdev_to_node(dev));
> if (!ic->i_recvs) {
> ret = -ENOMEM;
> rdsdebug("recv allocation failed\n");
> goto out;
> }
> - memset(ic->i_recvs, 0, ic->i_recv_ring.w_nr * sizeof(struct rds_ib_recv_work));
>
> rds_ib_recv_init_ack(ic);
>
^ permalink raw reply
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-02 14:43 UTC (permalink / raw)
To: Krishna Kumar2
Cc: habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
lguest-uLR06cmDAlY/bJ5BZ2RsiQ, Shirley Ma,
kvm-u79uwXL29TY76Z2rM5mHXA, Carsten Otte,
linux-s390-u79uwXL29TY76Z2rM5mHXA, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <OF990F08C5.2ECE35B1-ON652578A3.004DC0FA-652578A3.004E3FE4-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org>
On Thu, Jun 02, 2011 at 07:47:48PM +0530, Krishna Kumar2 wrote:
> > OK, I have something very similar, but I still dislike the screw the
> > latency part: this path is exactly what the IBM guys seem to hit. So I
> > created two functions: one tries to free a constant number and another
> > one up to capacity. I'll post that now.
>
> Please review this patch to see if it looks reasonable:
Hmm, since you decided to work on top of my patch,
I'd appreciate split-up fixes.
> 1. Picked comments/code from MST's code and Rusty's review.
> 2. virtqueue_min_capacity() needs to be called only if it returned
> empty the last time it was called.
> 3. Fix return value bug in free_old_xmit_skbs (hangs guest).
> 4. Stop queue only if capacity is not enough for next xmit.
That's what we always did ...
> 5. Fix/clean some likely/unlikely checks (hopefully).
>
> I have done some minimal netperf tests with this.
>
> With this patch, add_buf returning capacity seems to be useful - it
> allows less virtio API calls.
Why bother? It's cheap ...
>
> Signed-off-by: Krishna Kumar <krkumar2-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org>
> ---
> drivers/net/virtio_net.c | 105 ++++++++++++++++++++++---------------
> 1 file changed, 64 insertions(+), 41 deletions(-)
>
> diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
> --- org/drivers/net/virtio_net.c 2011-06-02 15:49:25.000000000 +0530
> +++ new/drivers/net/virtio_net.c 2011-06-02 19:13:02.000000000 +0530
> @@ -509,27 +509,43 @@ again:
> return received;
> }
>
> -/* Check capacity and try to free enough pending old buffers to enable queueing
> - * new ones. If min_skbs > 0, try to free at least the specified number of skbs
> - * even if the ring already has sufficient capacity. Return true if we can
> - * guarantee that a following virtqueue_add_buf will succeed. */
> -static bool free_old_xmit_skbs(struct virtnet_info *vi, int min_skbs)
> +/* Return true if freed a skb, else false */
> +static inline bool free_one_old_xmit_skb(struct virtnet_info *vi)
> {
> struct sk_buff *skb;
> unsigned int len;
> - bool r;
>
> - while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
> - min_skbs-- > 0) {
> - skb = virtqueue_get_buf(vi->svq, &len);
> - if (unlikely(!skb))
> + skb = virtqueue_get_buf(vi->svq, &len);
> + if (unlikely(!skb))
> + return 0;
> +
> + pr_debug("Sent skb %p\n", skb);
> + vi->dev->stats.tx_bytes += skb->len;
> + vi->dev->stats.tx_packets++;
> + dev_kfree_skb_any(skb);
> + return 1;
> +}
> +
> +static bool free_old_xmit_skbs(struct virtnet_info *vi, int to_free)
> +{
> + bool empty = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2;
> +
> + do {
> + if (!free_one_old_xmit_skb(vi)) {
> + /* No more skbs to free up */
> break;
> - pr_debug("Sent skb %p\n", skb);
> - vi->dev->stats.tx_bytes += skb->len;
> - vi->dev->stats.tx_packets++;
> - dev_kfree_skb_any(skb);
> - }
> - return r;
> + }
> +
> + if (empty) {
> + /* Check again if there is enough space */
> + empty = virtqueue_min_capacity(vi->svq) <
> + MAX_SKB_FRAGS + 2;
> + } else {
> + --to_free;
> + }
> + } while (to_free > 0);
> +
> + return !empty;
> }
Why bother doing the capacity check in this function?
> static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
> @@ -582,46 +598,53 @@ static int xmit_skb(struct virtnet_info
> static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> - int ret, n;
> + int capacity;
>
> - /* Free up space in the ring in case this is the first time we get
> - * woken up after ring full condition. Note: this might try to free
> - * more than strictly necessary if the skb has a small
> - * number of fragments, but keep it simple. */
> - free_old_xmit_skbs(vi, 0);
> + /* Try to free 2 buffers for every 1 xmit, to stay ahead. */
> + free_old_xmit_skbs(vi, 2);
>
> /* Try to transmit */
> - ret = xmit_skb(vi, skb);
> + capacity = xmit_skb(vi, skb);
>
> - /* Failure to queue is unlikely. It's not a bug though: it might happen
> - * if we get an interrupt while the queue is still mostly full.
> - * We could stop the queue and re-enable callbacks (and possibly return
> - * TX_BUSY), but as this should be rare, we don't bother. */
> - if (unlikely(ret < 0)) {
> + if (unlikely(capacity < 0)) {
> + /*
> + * Failure to queue should be impossible. The only way to
> + * reach here is if we got a cb before 3/4th of space was
> + * available. We could stop the queue and re-enable
> + * callbacks (and possibly return TX_BUSY), but we don't
> + * bother since this is impossible.
It's far from impossible. The 3/4 thing is only a hint, and old devices
don't support it anyway.
> + */
> if (net_ratelimit())
> - dev_info(&dev->dev, "TX queue failure: %d\n", ret);
> + dev_info(&dev->dev, "TX queue failure: %d\n", capacity);
> dev->stats.tx_dropped++;
> kfree_skb(skb);
> return NETDEV_TX_OK;
> }
> +
> virtqueue_kick(vi->svq);
>
> /* Don't wait up for transmitted skbs to be freed. */
> skb_orphan(skb);
> nf_reset(skb);
>
> - /* Apparently nice girls don't return TX_BUSY; check capacity and stop
> - * the queue before it gets out of hand.
> - * Naturally, this wastes entries. */
> - /* We transmit one skb, so try to free at least two pending skbs.
> - * This is so that we don't hog the skb memory unnecessarily. */
> - if (!likely(free_old_xmit_skbs(vi, 2))) {
> - netif_stop_queue(dev);
> - if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> - /* More just got used, free them and recheck. */
> - if (!likely(free_old_xmit_skbs(vi, 0))) {
> - netif_start_queue(dev);
> - virtqueue_disable_cb(vi->svq);
> + /*
> + * Apparently nice girls don't return TX_BUSY; check capacity and
> + * stop the queue before it gets out of hand. Naturally, this wastes
> + * entries.
> + */
> + if (capacity < 2+MAX_SKB_FRAGS) {
> + /*
> + * We don't have enough space for the next packet. Try
> + * freeing more.
> + */
> + if (likely(!free_old_xmit_skbs(vi, UINT_MAX))) {
> + netif_stop_queue(dev);
> + if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
> + /* More just got used, free them and recheck. */
> + if (likely(free_old_xmit_skbs(vi, UINT_MAX))) {
Is this where the bug was?
> + netif_start_queue(dev);
> + virtqueue_disable_cb(vi->svq);
> + }
> }
> }
> }
^ permalink raw reply
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Krishna Kumar2 @ 2011-06-02 14:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
lguest-uLR06cmDAlY/bJ5BZ2RsiQ, Shirley Ma,
kvm-u79uwXL29TY76Z2rM5mHXA, Carsten Otte,
linux-s390-u79uwXL29TY76Z2rM5mHXA, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <20110602133425.GJ7141-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6024 bytes --]
> OK, I have something very similar, but I still dislike the screw the
> latency part: this path is exactly what the IBM guys seem to hit. So I
> created two functions: one tries to free a constant number and another
> one up to capacity. I'll post that now.
Please review this patch to see if it looks reasonable (inline and
attachment):
1. Picked comments/code from Michael's code and Rusty's review.
2. virtqueue_min_capacity() needs to be called only if it returned
empty the last time it was called.
3. Fix return value bug in free_old_xmit_skbs (hangs guest).
4. Stop queue only if capacity is not enough for next xmit.
5. Fix/clean some likely/unlikely checks (hopefully).
6. I think xmit_skb cannot return error since
virtqueue_enable_cb_delayed() can return false only if
3/4th space became available, which is what we check.
6. The comments for free_old_xmit_skbs needs to be more
clear (not done).
I have done some minimal netperf tests with this.
With this patch, add_buf returning capacity seems to be useful - it
allows using fewer virtio API calls.
(See attached file: patch)
Signed-off-by: Krishna Kumar <krkumar2-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org>
---
drivers/net/virtio_net.c | 105 ++++++++++++++++++++++---------------
1 file changed, 64 insertions(+), 41 deletions(-)
diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
--- org/drivers/net/virtio_net.c 2011-06-02 15:49:25.000000000 +0530
+++ new/drivers/net/virtio_net.c 2011-06-02 19:13:02.000000000 +0530
@@ -509,27 +509,43 @@ again:
return received;
}
-/* Check capacity and try to free enough pending old buffers to enable
queueing
- * new ones. If min_skbs > 0, try to free at least the specified number
of skbs
- * even if the ring already has sufficient capacity. Return true if we
can
- * guarantee that a following virtqueue_add_buf will succeed. */
-static bool free_old_xmit_skbs(struct virtnet_info *vi, int min_skbs)
+/* Return true if freed a skb, else false */
+static inline bool free_one_old_xmit_skb(struct virtnet_info *vi)
{
struct sk_buff *skb;
unsigned int len;
- bool r;
- while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
- min_skbs-- > 0) {
- skb = virtqueue_get_buf(vi->svq, &len);
- if (unlikely(!skb))
+ skb = virtqueue_get_buf(vi->svq, &len);
+ if (unlikely(!skb))
+ return 0;
+
+ pr_debug("Sent skb %p\n", skb);
+ vi->dev->stats.tx_bytes += skb->len;
+ vi->dev->stats.tx_packets++;
+ dev_kfree_skb_any(skb);
+ return 1;
+}
+
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int to_free)
+{
+ bool empty = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2;
+
+ do {
+ if (!free_one_old_xmit_skb(vi)) {
+ /* No more skbs to free up */
break;
- pr_debug("Sent skb %p\n", skb);
- vi->dev->stats.tx_bytes += skb->len;
- vi->dev->stats.tx_packets++;
- dev_kfree_skb_any(skb);
- }
- return r;
+ }
+
+ if (empty) {
+ /* Check again if there is enough space */
+ empty = virtqueue_min_capacity(vi->svq) <
+ MAX_SKB_FRAGS + 2;
+ } else {
+ --to_free;
+ }
+ } while (to_free > 0);
+
+ return !empty;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -582,46 +598,53 @@ static int xmit_skb(struct virtnet_info
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- int ret, n;
+ int capacity;
- /* Free up space in the ring in case this is the first time we get
- * woken up after ring full condition. Note: this might try to free
- * more than strictly necessary if the skb has a small
- * number of fragments, but keep it simple. */
- free_old_xmit_skbs(vi, 0);
+ /* Try to free 2 buffers for every 1 xmit, to stay ahead. */
+ free_old_xmit_skbs(vi, 2);
/* Try to transmit */
- ret = xmit_skb(vi, skb);
+ capacity = xmit_skb(vi, skb);
- /* Failure to queue is unlikely. It's not a bug though: it might happen
- * if we get an interrupt while the queue is still mostly full.
- * We could stop the queue and re-enable callbacks (and possibly
return
- * TX_BUSY), but as this should be rare, we don't bother. */
- if (unlikely(ret < 0)) {
+ if (unlikely(capacity < 0)) {
+ /*
+ * Failure to queue should be impossible. The only way to
+ * reach here is if we got a cb before 3/4th of space was
+ * available. We could stop the queue and re-enable
+ * callbacks (and possibly return TX_BUSY), but we don't
+ * bother since this is impossible.
+ */
if (net_ratelimit())
- dev_info(&dev->dev, "TX queue failure: %d\n", ret);
+ dev_info(&dev->dev, "TX queue failure: %d\n", capacity);
dev->stats.tx_dropped++;
kfree_skb(skb);
return NETDEV_TX_OK;
}
+
virtqueue_kick(vi->svq);
/* Don't wait up for transmitted skbs to be freed. */
skb_orphan(skb);
nf_reset(skb);
- /* Apparently nice girls don't return TX_BUSY; check capacity and stop
- * the queue before it gets out of hand.
- * Naturally, this wastes entries. */
- /* We transmit one skb, so try to free at least two pending skbs.
- * This is so that we don't hog the skb memory unnecessarily. */
- if (!likely(free_old_xmit_skbs(vi, 2))) {
- netif_stop_queue(dev);
- if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
- /* More just got used, free them and recheck. */
- if (!likely(free_old_xmit_skbs(vi, 0))) {
- netif_start_queue(dev);
- virtqueue_disable_cb(vi->svq);
+ /*
+ * Apparently nice girls don't return TX_BUSY; check capacity and
+ * stop the queue before it gets out of hand. Naturally, this wastes
+ * entries.
+ */
+ if (capacity < 2+MAX_SKB_FRAGS) {
+ /*
+ * We don't have enough space for the next packet. Try
+ * freeing more.
+ */
+ if (likely(!free_old_xmit_skbs(vi, UINT_MAX))) {
+ netif_stop_queue(dev);
+ if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
+ /* More just got used, free them and recheck. */
+ if (likely(free_old_xmit_skbs(vi, UINT_MAX))) {
+ netif_start_queue(dev);
+ virtqueue_disable_cb(vi->svq);
+ }
}
}
}
[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 5775 bytes --]
"Michael S. Tsirkin" <mst@redhat.com> wrote on 06/02/2011 07:04:25 PM:
> OK, I have something very similar, but I still dislike the screw the
> latency part: this path is exactly what the IBM guys seem to hit. So I
> created two functions: one tries to free a constant number and another
> one up to capacity. I'll post that now.
Please review this patch to see if it looks reasonable:
1. Picked comments/code from MST's code and Rusty's review.
2. virtqueue_min_capacity() needs to be called only if it returned
empty the last time it was called.
3. Fix return value bug in free_old_xmit_skbs (hangs guest).
4. Stop queue only if capacity is not enough for next xmit.
5. Fix/clean some likely/unlikely checks (hopefully).
I have done some minimal netperf tests with this.
With this patch, add_buf returning capacity seems to be useful - it
allows less virtio API calls.
Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
drivers/net/virtio_net.c | 105 ++++++++++++++++++++++---------------
1 file changed, 64 insertions(+), 41 deletions(-)
diff -ruNp org/drivers/net/virtio_net.c new/drivers/net/virtio_net.c
--- org/drivers/net/virtio_net.c 2011-06-02 15:49:25.000000000 +0530
+++ new/drivers/net/virtio_net.c 2011-06-02 19:13:02.000000000 +0530
@@ -509,27 +509,43 @@ again:
return received;
}
-/* Check capacity and try to free enough pending old buffers to enable queueing
- * new ones. If min_skbs > 0, try to free at least the specified number of skbs
- * even if the ring already has sufficient capacity. Return true if we can
- * guarantee that a following virtqueue_add_buf will succeed. */
-static bool free_old_xmit_skbs(struct virtnet_info *vi, int min_skbs)
+/* Return true if freed a skb, else false */
+static inline bool free_one_old_xmit_skb(struct virtnet_info *vi)
{
struct sk_buff *skb;
unsigned int len;
- bool r;
- while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
- min_skbs-- > 0) {
- skb = virtqueue_get_buf(vi->svq, &len);
- if (unlikely(!skb))
+ skb = virtqueue_get_buf(vi->svq, &len);
+ if (unlikely(!skb))
+ return 0;
+
+ pr_debug("Sent skb %p\n", skb);
+ vi->dev->stats.tx_bytes += skb->len;
+ vi->dev->stats.tx_packets++;
+ dev_kfree_skb_any(skb);
+ return 1;
+}
+
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int to_free)
+{
+ bool empty = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2;
+
+ do {
+ if (!free_one_old_xmit_skb(vi)) {
+ /* No more skbs to free up */
break;
- pr_debug("Sent skb %p\n", skb);
- vi->dev->stats.tx_bytes += skb->len;
- vi->dev->stats.tx_packets++;
- dev_kfree_skb_any(skb);
- }
- return r;
+ }
+
+ if (empty) {
+ /* Check again if there is enough space */
+ empty = virtqueue_min_capacity(vi->svq) <
+ MAX_SKB_FRAGS + 2;
+ } else {
+ --to_free;
+ }
+ } while (to_free > 0);
+
+ return !empty;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -582,46 +598,53 @@ static int xmit_skb(struct virtnet_info
static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
- int ret, n;
+ int capacity;
- /* Free up space in the ring in case this is the first time we get
- * woken up after ring full condition. Note: this might try to free
- * more than strictly necessary if the skb has a small
- * number of fragments, but keep it simple. */
- free_old_xmit_skbs(vi, 0);
+ /* Try to free 2 buffers for every 1 xmit, to stay ahead. */
+ free_old_xmit_skbs(vi, 2);
/* Try to transmit */
- ret = xmit_skb(vi, skb);
+ capacity = xmit_skb(vi, skb);
- /* Failure to queue is unlikely. It's not a bug though: it might happen
- * if we get an interrupt while the queue is still mostly full.
- * We could stop the queue and re-enable callbacks (and possibly return
- * TX_BUSY), but as this should be rare, we don't bother. */
- if (unlikely(ret < 0)) {
+ if (unlikely(capacity < 0)) {
+ /*
+ * Failure to queue should be impossible. The only way to
+ * reach here is if we got a cb before 3/4th of space was
+ * available. We could stop the queue and re-enable
+ * callbacks (and possibly return TX_BUSY), but we don't
+ * bother since this is impossible.
+ */
if (net_ratelimit())
- dev_info(&dev->dev, "TX queue failure: %d\n", ret);
+ dev_info(&dev->dev, "TX queue failure: %d\n", capacity);
dev->stats.tx_dropped++;
kfree_skb(skb);
return NETDEV_TX_OK;
}
+
virtqueue_kick(vi->svq);
/* Don't wait up for transmitted skbs to be freed. */
skb_orphan(skb);
nf_reset(skb);
- /* Apparently nice girls don't return TX_BUSY; check capacity and stop
- * the queue before it gets out of hand.
- * Naturally, this wastes entries. */
- /* We transmit one skb, so try to free at least two pending skbs.
- * This is so that we don't hog the skb memory unnecessarily. */
- if (!likely(free_old_xmit_skbs(vi, 2))) {
- netif_stop_queue(dev);
- if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
- /* More just got used, free them and recheck. */
- if (!likely(free_old_xmit_skbs(vi, 0))) {
- netif_start_queue(dev);
- virtqueue_disable_cb(vi->svq);
+ /*
+ * Apparently nice girls don't return TX_BUSY; check capacity and
+ * stop the queue before it gets out of hand. Naturally, this wastes
+ * entries.
+ */
+ if (capacity < 2+MAX_SKB_FRAGS) {
+ /*
+ * We don't have enough space for the next packet. Try
+ * freeing more.
+ */
+ if (likely(!free_old_xmit_skbs(vi, UINT_MAX))) {
+ netif_stop_queue(dev);
+ if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
+ /* More just got used, free them and recheck. */
+ if (likely(free_old_xmit_skbs(vi, UINT_MAX))) {
+ netif_start_queue(dev);
+ virtqueue_disable_cb(vi->svq);
+ }
}
}
}
[-- Attachment #3: Type: text/plain, Size: 156 bytes --]
_______________________________________________
Lguest mailing list
Lguest-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/lguest
^ permalink raw reply
* Hi from Karl.
From: Karl Kampor @ 2011-06-02 13:38 UTC (permalink / raw)
Dear Friend,
This message might meet you in utmost surprise, however, it's just my urgent need for a foreign partner that made me to contact you for this transaction. I am Mr Karl Kampor a banker by profession and currently holding the post a manager in the auditing and accounting section of the bank. I have the opportunity of transferring a left over fund by one of my bank late customer were i work in the tune of ($10.8 million). This fund have been laying dormant for the past five or six years now. My proposal is to package and present you as next of kin to this inheritance and want you to know that i discover this fund during our last years audit and i kept this a secret to my self alone since there haven't been any claim for this fund.
But i have some questions for you:- (1) Can you handle this project? (2) Can i give you this trust? (3) Can you provide an account? were this money will be transfer which is very important in this transaction and after the successful transfer we shall share in ratio of 40% for you and 60% for me.
I expect your urgent response should you be interested in this project and treat this business with utmost confidentiality and send me the following:
(1) Full names:.......
(2) Private phone number:.......
(3) Current residential address:........
(4) Occupation:.......
(5) Age and Sex:......
Thanks for your co-operations
Mr Karl Kampor
^ permalink raw reply
* Re: [PATCH RFC 3/3] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-02 13:34 UTC (permalink / raw)
To: Rusty Russell
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87pqmwj3am.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
On Thu, Jun 02, 2011 at 01:24:57PM +0930, Rusty Russell wrote:
> On Wed, 1 Jun 2011 12:50:03 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Current code might introduce a lot of latency variation
> > if there are many pending bufs at the time we
> > attempt to transmit a new one. This is bad for
> > real-time applications and can't be good for TCP either.
> >
> > Free up just enough to both clean up all buffers
> > eventually and to be able to xmit the next packet.
>
> OK, I found this quite confusing to read.
>
> > - while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
> > + while ((r = virtqueue_min_capacity(vi->svq) < MAX_SKB_FRAGS + 2) ||
> > + min_skbs-- > 0) {
> > + skb = virtqueue_get_buf(vi->svq, &len);
> > + if (unlikely(!skb))
> > + break;
> > pr_debug("Sent skb %p\n", skb);
> > vi->dev->stats.tx_bytes += skb->len;
> > vi->dev->stats.tx_packets++;
> > dev_kfree_skb_any(skb);
> > }
> > + return r;
> > }
>
> Gah... what a horrible loop.
>
> Basically, this patch makes hard-to-read code worse, and we should try
> to make it better.
>
> Currently, xmit *can* fail when an xmit interrupt wakes the queue, but
> the packet(s) xmitted didn't free up enough space for the new packet.
> With indirect buffers this only happens if we hit OOM (and thus go to
> direct buffers).
>
> We could solve this by only waking the queue in skb_xmit_done if the
> capacity is >= 2 + MAX_SKB_FRAGS. But can we do it without a race?
I don't think so.
> If not, then I'd really prefer to see this, because I think it's clearer:
>
> // Try to free 2 buffers for every 1 xmit, to stay ahead.
> free_old_buffers(2)
>
> if (!add_buf()) {
> // Screw latency, free them all.
> free_old_buffers(UINT_MAX)
> // OK, this can happen if we are using direct buffers,
> // and the xmit interrupt woke us but the packets
> // xmitted were smaller than this one. Rare though.
> if (!add_buf())
> Whinge and stop queue, maybe loop.
> }
>
> if (capacity < 2 + MAX_SKB_FRAGS) {
> // We don't have enough for the next packet? Try
> // freeing more.
> free_old_buffers(UINT_MAX);
> if (capacity < 2 + MAX_SKB_FRAGS) {
> Stop queue, maybe loop.
> }
>
> The current code makes my head hurt :(
>
> Thoughts?
> Rusty.
OK, I have something very similar, but I still dislike the screw the
latency part: this path is exactly what the IBM guys seem to hit. So I
created two functions: one tries to free a constant number and another
one up to capacity. I'll post that now.
--
MST
^ permalink raw reply
* Re: [PATCH RFC 1/3] virtio_ring: add capacity check API
From: Michael S. Tsirkin @ 2011-06-02 13:30 UTC (permalink / raw)
To: Rusty Russell
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87sjrthti1.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
On Thu, Jun 02, 2011 at 11:41:50AM +0930, Rusty Russell wrote:
> On Wed, 1 Jun 2011 12:49:46 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Add API to check ring capacity. Because of the option
> > to use indirect buffers, this returns the worst
> > case, not the normal case capacity.
>
> Can we drop the silly "add_buf() returns capacity" hack then?
>
> Thanks,
> Rusty.
Sure.
^ permalink raw reply
* Re: [PATCH RFC 2/3] virtio_net: fix tx capacity checks using new API
From: Michael S. Tsirkin @ 2011-06-02 13:28 UTC (permalink / raw)
To: Rusty Russell
Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87vcwphtkd.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>
On Thu, Jun 02, 2011 at 11:40:26AM +0930, Rusty Russell wrote:
> On Wed, 1 Jun 2011 12:49:54 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > In the (rare) case where new descriptors are used
> > while virtio_net enables vq callback for the TX vq,
> > virtio_net uses the number of sg entries in the skb it frees to
> > calculate how many descriptors in the ring have just been made
> > available. But this value is an overestimate: with indirect buffers
> > each skb only uses one descriptor entry, meaning we may wake the queue
> > only to find we still can't transmit anything.
>
> This is a bit misleading.
>
> The value is an overestimate, but so is the requirement for
> 2+MAX_SKB_FRAGS, *unless* we suddenly drop into direct mode due to OOM.
>
> Thanks,
> Rusty.
I agree, it's unlikely.
s/still can't transmit anything/are still out of space and need to stop
the ring almost at once/
Better?
^ permalink raw reply
* winner
From: Microsoft @ 2011-06-02 19:18 UTC (permalink / raw)
You have won 500.000 GBP
send your phone number
and address
^ permalink raw reply
* Re: [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Jiri Pirko @ 2011-06-02 13:15 UTC (permalink / raw)
To: Eric W. Biederman
Cc: David Miller, shemminger, greearb, nicolas.2p.debian, xiaosuo,
netdev, kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <m11uzcidvq.fsf_-_@fess.ebiederm.org>
Thu, Jun 02, 2011 at 03:03:53PM CEST, ebiederm@xmission.com wrote:
>
>Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag
>but rather in vlan_do_receive. Otherwise the vlan header
>will not be properly put on the packet in the case of
>vlan header accelleration.
>
>As we remove the check from vlan_check_reorder_header
>rename it vlan_reorder_header to keep the naming clean.
>
>Fix up the skb->pkt_type early so we don't look at the packet
>after adding the vlan tag, which guarantees we don't goof
>and look at the wrong field.
>
>Use a simple if statement instead of a complicated switch
>statement to decided that we need to increment rx_stats
>for a multicast packet.
>
>Hopefully at somepoint we will just declare the case where
>VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove
>the code. Until then this keeps it working correctly.
>
>Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Looking good to me. Thanks Eric.
Reviewed-by: Jiri Pirko <jpirko@redhat.com>
^ permalink raw reply
* [PATCH] vlan: Fix the ingress VLAN_FLAG_REORDER_HDR check v2
From: Eric W. Biederman @ 2011-06-02 13:03 UTC (permalink / raw)
To: David Miller
Cc: shemminger, greearb, nicolas.2p.debian, jpirko, xiaosuo, netdev,
kaber, fubar, eric.dumazet, andy, jesse
In-Reply-To: <20110601.205940.1179055860757569997.davem@davemloft.net>
Testing of VLAN_FLAG_REORDER_HDR does not belong in vlan_untag
but rather in vlan_do_receive. Otherwise the vlan header
will not be properly put on the packet in the case of
vlan header accelleration.
As we remove the check from vlan_check_reorder_header
rename it vlan_reorder_header to keep the naming clean.
Fix up the skb->pkt_type early so we don't look at the packet
after adding the vlan tag, which guarantees we don't goof
and look at the wrong field.
Use a simple if statement instead of a complicated switch
statement to decided that we need to increment rx_stats
for a multicast packet.
Hopefully at somepoint we will just declare the case where
VLAN_FLAG_REORDER_HDR is cleared as unsupported and remove
the code. Until then this keeps it working correctly.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
include/linux/if_vlan.h | 25 ++++++++++++++++++++--
net/8021q/vlan_core.c | 50 ++++++++++++++++++++++------------------------
2 files changed, 46 insertions(+), 29 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 290bd8a..821f0e3 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -220,7 +220,7 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
}
/**
- * __vlan_put_tag - regular VLAN tag inserting
+ * vlan_insert_tag - regular VLAN tag inserting
* @skb: skbuff to tag
* @vlan_tci: VLAN TCI to insert
*
@@ -229,8 +229,10 @@ static inline int vlan_hwaccel_receive_skb(struct sk_buff *skb,
*
* Following the skb_unshare() example, in case of error, the calling function
* doesn't have to worry about freeing the original skb.
+ *
+ * Does not change skb->protocol so this function can be used during receive.
*/
-static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, u16 vlan_tci)
{
struct vlan_ethhdr *veth;
@@ -250,8 +252,25 @@ static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
/* now, the TCI */
veth->h_vlan_TCI = htons(vlan_tci);
- skb->protocol = htons(ETH_P_8021Q);
+ return skb;
+}
+/**
+ * __vlan_put_tag - regular VLAN tag inserting
+ * @skb: skbuff to tag
+ * @vlan_tci: VLAN TCI to insert
+ *
+ * Inserts the VLAN tag into @skb as part of the payload
+ * Returns a VLAN tagged skb. If a new skb is created, @skb is freed.
+ *
+ * Following the skb_unshare() example, in case of error, the calling function
+ * doesn't have to worry about freeing the original skb.
+ */
+static inline struct sk_buff *__vlan_put_tag(struct sk_buff *skb, u16 vlan_tci)
+{
+ skb = vlan_insert_tag(skb, vlan_tci);
+ if (skb)
+ skb->protocol = htons(ETH_P_8021Q);
return skb;
}
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 41495dc..735c818 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -23,6 +23,20 @@ bool vlan_do_receive(struct sk_buff **skbp)
return false;
skb->dev = vlan_dev;
+ if (skb->pkt_type == PACKET_OTHERHOST) {
+ /* Our lower layer thinks this is not local, let's make sure.
+ * This allows the VLAN to have a different MAC than the
+ * underlying device, and still route correctly. */
+ if (!compare_ether_addr(eth_hdr(skb)->h_dest,
+ vlan_dev->dev_addr))
+ skb->pkt_type = PACKET_HOST;
+ }
+
+ if (unlikely(!(vlan_dev_info(vlan_dev)->flags & VLAN_FLAG_REORDER_HDR))) {
+ skb = *skbp = vlan_insert_tag(skb, skb->vlan_tci);
+ if (!skb)
+ return false;
+ }
skb->priority = vlan_get_ingress_priority(vlan_dev, skb->vlan_tci);
skb->vlan_tci = 0;
@@ -31,22 +45,8 @@ bool vlan_do_receive(struct sk_buff **skbp)
u64_stats_update_begin(&rx_stats->syncp);
rx_stats->rx_packets++;
rx_stats->rx_bytes += skb->len;
-
- switch (skb->pkt_type) {
- case PACKET_BROADCAST:
- break;
- case PACKET_MULTICAST:
+ if (skb->pkt_type == PACKET_MULTICAST)
rx_stats->rx_multicast++;
- break;
- case PACKET_OTHERHOST:
- /* Our lower layer thinks this is not local, let's make sure.
- * This allows the VLAN to have a different MAC than the
- * underlying device, and still route correctly. */
- if (!compare_ether_addr(eth_hdr(skb)->h_dest,
- vlan_dev->dev_addr))
- skb->pkt_type = PACKET_HOST;
- break;
- }
u64_stats_update_end(&rx_stats->syncp);
return true;
@@ -89,17 +89,15 @@ gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
}
EXPORT_SYMBOL(vlan_gro_frags);
-static struct sk_buff *vlan_check_reorder_header(struct sk_buff *skb)
+static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
{
- if (vlan_dev_info(skb->dev)->flags & VLAN_FLAG_REORDER_HDR) {
- if (skb_cow(skb, skb_headroom(skb)) < 0)
- skb = NULL;
- if (skb) {
- /* Lifted from Gleb's VLAN code... */
- memmove(skb->data - ETH_HLEN,
- skb->data - VLAN_ETH_HLEN, 12);
- skb->mac_header += VLAN_HLEN;
- }
+ if (skb_cow(skb, skb_headroom(skb)) < 0)
+ skb = NULL;
+ if (skb) {
+ /* Lifted from Gleb's VLAN code... */
+ memmove(skb->data - ETH_HLEN,
+ skb->data - VLAN_ETH_HLEN, 12);
+ skb->mac_header += VLAN_HLEN;
}
return skb;
}
@@ -161,7 +159,7 @@ struct sk_buff *vlan_untag(struct sk_buff *skb)
skb_pull_rcsum(skb, VLAN_HLEN);
vlan_set_encap_proto(skb, vhdr);
- skb = vlan_check_reorder_header(skb);
+ skb = vlan_reorder_header(skb);
if (unlikely(!skb))
goto err_free;
--
1.7.5.1.217.g4e3aa
^ permalink raw reply related
* Re: [PATCH] ipvs: restore support for iptables SNAT
From: Simon Horman @ 2011-06-02 13:01 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <4DE7793F.1060108@netfilter.org>
On Thu, Jun 02, 2011 at 01:51:27PM +0200, Pablo Neira Ayuso wrote:
> On 02/06/11 02:09, Simon Horman wrote:
> > From: Julian Anastasov <ja@ssi.bg>
> >
> > Fix the IPVS priority in LOCAL_IN hook,
> > so that SNAT target in POSTROUTING is supported for IPVS
> > traffic as in 2.6.36 where it worked depending on
> > module load order.
> >
> > Before 2.6.37 we used priority 100 in LOCAL_IN to
> > process remote requests. We used the same priority as
> > iptables SNAT and if IPVS handlers are installed before
> > SNAT handlers we supported SNAT in POSTROUTING for the IPVS
> > traffic. If SNAT is installed before IPVS, the netfilter
> > handlers are before IPVS and netfilter checks the NAT
> > table twice for the IPVS requests: once in LOCAL_IN where
> > IPS_SRC_NAT_DONE is set and second time in POSTROUTING
> > where the SNAT rules are ignored because IPS_SRC_NAT_DONE
> > was already set in LOCAL_IN.
> >
> > But in 2.6.37 we changed the IPVS priority for
> > LOCAL_IN with the goal to be unique (101) forgetting the
> > fact that for IPVS traffic we should not walk both
> > LOCAL_IN and POSTROUTING nat tables.
> >
> > So, change the priority for processing remote
> > IPVS requests from 101 to 99, i.e. before NAT_SRC (100)
> > because we prefer to support SNAT in POSTROUTING
> > instead of LOCAL_IN. It also moves the priority for
> > IPVS replies from 99 to 98. Use constants instead of
> > magic numbers at these places.
>
> I have applied this to my net-next-2.6 tree. Once it hits linus tree,
> I'll pass it to -stable.
>
> http://1984.lsi.us.es/git/?p=net-next-2.6/.git;a=shortlog;h=refs/heads/pablo/nf-next-2.6-updates
Thanks Pablo.
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: remove one synchronize_net()
From: Pablo Neira Ayuso @ 2011-06-02 13:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Patrick McHardy, Netfilter Development Mailinglist, netdev
In-Reply-To: <1306509313.3445.5.camel@edumazet-laptop>
On 27/05/11 17:15, Eric Dumazet wrote:
> No point to wait a rcu grace period before the unregisters.
applied, thanks
^ permalink raw reply
* Re: [PATCH] Add Fujitsu 1000base-SX PCI ID to tg3
From: Meelis Roos @ 2011-06-02 12:35 UTC (permalink / raw)
To: Matt Carlson; +Cc: Michael Chan, netdev@vger.kernel.org
In-Reply-To: <20110601211401.GB1482@mcarlson.broadcom.com>
> > This patch adds the PCI ID of Fujitsu 1000base-SX NIC to tg3 driver.
> > Tested to detect the card, MAC and serdes, not tested with link at the
> > moment since I have no fiber switch here. I did not add new constants to
> > the pci_ids.h header file since these constants are used only here.
> >
> > Signed-off-by: Meelis Roos <mroos@linux.ee>
> >
> > diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> > index 7a5daef..7b1901e 100644
> > --- a/drivers/net/tg3.c
> > +++ b/drivers/net/tg3.c
> > @@ -273,6 +273,7 @@ static DEFINE_PCI_DEVICE_TABLE(tg3_pci_tbl) = {
> > {PCI_DEVICE(PCI_VENDOR_ID_ALTIMA, PCI_DEVICE_ID_ALTIMA_AC1003)},
> > {PCI_DEVICE(PCI_VENDOR_ID_ALTIMA, PCI_DEVICE_ID_ALTIMA_AC9100)},
> > {PCI_DEVICE(PCI_VENDOR_ID_APPLE, PCI_DEVICE_ID_APPLE_TIGON3)},
> > + {PCI_DEVICE(0x10cf, 0x11a2)}, /* Fujitsu 1000base-SX with BCM5703SKHB */
> > {}
> > };
>
> I don't have any problems with the patch, but before we integrate it,
> can we make sure it passes traffic? I'd hate to add support for a
> device and later find out it doesn't work.
Checked with direct link to another NIC, link came up.
[ 99.067432] tg3 0000:01:0b.0: eth1: Link is up at 1000 Mbps, full duplex
[ 99.067662] tg3 0000:01:0b.0: eth1: Flow control is on for TX and on for RX
I can also see DHCP requests from one interface on the other with
tcpdump, so data gets also through. Seems to work.
--
Meelis Roos (mroos@linux.ee)
^ permalink raw reply
* Re: [PATCH] netfilter, ipvs: Avoid undefined order of evaluation in assignments to struct nf_conn *
From: Pablo Neira Ayuso @ 2011-06-02 12:59 UTC (permalink / raw)
To: Simon Horman
Cc: Jesper Juhl, linux-kernel, netfilter, coreteam, netfilter-devel,
lvs-devel, netdev, David S. Miller, Patrick McHardy,
Julian Anastasov, Wensong Zhang
In-Reply-To: <20110529232337.GD5856@verge.net.au>
On 30/05/11 01:23, Simon Horman wrote:
> Acked-by: Simon Horman <horms@verge.net.au>
applied, thanks
^ permalink raw reply
* [PATCHv2 NEXT 1/1] netxen: suppress false lro warning messages
From: amit.salecha @ 2011-06-02 12:24 UTC (permalink / raw)
To: davem
Cc: netdev, ameen.rahman, anirban.chakraborty, Sucheta Chakraborty,
Amit Kumar Salecha
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
When interface is down, driver prints false warning messages
during lro configuration through ethtool.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/netxen/netxen_nic_hw.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 5cef718..3f89e57 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -809,6 +809,9 @@ int netxen_config_hw_lro(struct netxen_adapter *adapter, int enable)
u64 word;
int rv = 0;
+ if (!test_bit(__NX_FW_ATTACHED, &adapter->state))
+ return 0;
+
memset(&req, 0, sizeof(nx_nic_req_t));
req.qhdr = cpu_to_le64(NX_HOST_REQUEST << 23);
@@ -959,6 +962,9 @@ int netxen_send_lro_cleanup(struct netxen_adapter *adapter)
u64 word;
int rv;
+ if (!test_bit(__NX_FW_ATTACHED, &adapter->state))
+ return 0;
+
memset(&req, 0, sizeof(nx_nic_req_t));
req.qhdr = cpu_to_le64(NX_HOST_REQUEST << 23);
--
1.7.3.3
^ permalink raw reply related
* [PATCH NEXT 1/1] netxen: suppress false lro warning messages
From: amit.salecha @ 2011-06-02 11:50 UTC (permalink / raw)
To: davem
Cc: netdev, ameen.rahman, anirban.chakraborty, Sucheta Chakraborty,
Amit Kumar Salecha
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Driver prints false warning messages during lro configuration
through ethtool, when interface is down.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/netxen/netxen_nic_hw.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index 5cef718..0bd6615 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -809,6 +809,9 @@ int netxen_config_hw_lro(struct netxen_adapter *adapter, int enable)
u64 word;
int rv = 0;
+ if (test_bit(__NX_FW_ATTACHED, &adapter->state))
+ return 0;
+
memset(&req, 0, sizeof(nx_nic_req_t));
req.qhdr = cpu_to_le64(NX_HOST_REQUEST << 23);
@@ -959,6 +962,9 @@ int netxen_send_lro_cleanup(struct netxen_adapter *adapter)
u64 word;
int rv;
+ if (test_bit(__NX_FW_ATTACHED, &adapter->state))
+ return 0;
+
memset(&req, 0, sizeof(nx_nic_req_t));
req.qhdr = cpu_to_le64(NX_HOST_REQUEST << 23);
--
1.7.3.3
^ permalink raw reply related
* Re: [PATCH] ipvs: restore support for iptables SNAT
From: Pablo Neira Ayuso @ 2011-06-02 11:51 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, netfilter, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <1306973394-29504-2-git-send-email-horms@verge.net.au>
On 02/06/11 02:09, Simon Horman wrote:
> From: Julian Anastasov <ja@ssi.bg>
>
> Fix the IPVS priority in LOCAL_IN hook,
> so that SNAT target in POSTROUTING is supported for IPVS
> traffic as in 2.6.36 where it worked depending on
> module load order.
>
> Before 2.6.37 we used priority 100 in LOCAL_IN to
> process remote requests. We used the same priority as
> iptables SNAT and if IPVS handlers are installed before
> SNAT handlers we supported SNAT in POSTROUTING for the IPVS
> traffic. If SNAT is installed before IPVS, the netfilter
> handlers are before IPVS and netfilter checks the NAT
> table twice for the IPVS requests: once in LOCAL_IN where
> IPS_SRC_NAT_DONE is set and second time in POSTROUTING
> where the SNAT rules are ignored because IPS_SRC_NAT_DONE
> was already set in LOCAL_IN.
>
> But in 2.6.37 we changed the IPVS priority for
> LOCAL_IN with the goal to be unique (101) forgetting the
> fact that for IPVS traffic we should not walk both
> LOCAL_IN and POSTROUTING nat tables.
>
> So, change the priority for processing remote
> IPVS requests from 101 to 99, i.e. before NAT_SRC (100)
> because we prefer to support SNAT in POSTROUTING
> instead of LOCAL_IN. It also moves the priority for
> IPVS replies from 99 to 98. Use constants instead of
> magic numbers at these places.
I have applied this to my net-next-2.6 tree. Once it hits linus tree,
I'll pass it to -stable.
http://1984.lsi.us.es/git/?p=net-next-2.6/.git;a=shortlog;h=refs/heads/pablo/nf-next-2.6-updates
^ permalink raw reply
* Re: [PATCH net-next-2.6] netfilter: add more values to enum ip_conntrack_info
From: Pablo Neira Ayuso @ 2011-06-02 11:44 UTC (permalink / raw)
To: Eric Dumazet
Cc: Patrick McHardy, Netfilter Development Mailinglist, netdev,
David Miller
In-Reply-To: <1305812667.3028.66.camel@edumazet-laptop>
On 19/05/11 15:44, Eric Dumazet wrote:
> Following error is raised (and other similar ones) :
>
> net/ipv4/netfilter/nf_nat_standalone.c: In function ‘nf_nat_fn’:
> net/ipv4/netfilter/nf_nat_standalone.c:119:2: warning: case value ‘4’
> not in enumerated type ‘enum ip_conntrack_info’
>
> gcc barfs on adding two enum values and getting a not enumerated
> result :
>
> case IP_CT_RELATED+IP_CT_IS_REPLY:
>
> Add missing enum values
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: David Miller <davem@davemloft.net>
I have applied this to my net-next-2.6 tree.
http://1984.lsi.us.es/git/?p=net-next-2.6/.git;a=shortlog;h=refs/heads/pablo/nf-next-2.6-updates
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/5] net: ep93xx_eth: pass struct device to DMA API functions
From: Russell King - ARM Linux @ 2011-06-02 9:52 UTC (permalink / raw)
To: Mika Westerberg; +Cc: netdev, hsweeten, ryan, kernel, linux-arm-kernel
In-Reply-To: <89df467f9811b4f869513de3ada90ce7de74c6d1.1307006502.git.mika.westerberg@iki.fi>
On Thu, Jun 02, 2011 at 12:38:13PM +0300, Mika Westerberg wrote:
> We shouldn't use NULL for any DMA API functions, unless we are dealing with
> ISA or EISA device. So pass correct struct dev pointer to these functions.
Thanks for doing this - it certainly makes things more correct, even
if the original problem has been resolved by the troublesome GFP_DMA
commit having been reverted. As such I think this patch series should
still be applied.
I think the series should be re-ordered so that patch 5 is first however,
as you can do that with or without the remainder of the patches.
For the entire series:
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox