From patchwork Fri Aug 28 10:38:33 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Karl Hiramoto X-Patchwork-Id: 32354 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 39D45B7BA2 for ; Fri, 28 Aug 2009 20:39:34 +1000 (EST) Received: by ozlabs.org (Postfix) id 29484DDD0C; Fri, 28 Aug 2009 20:39:34 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id 75D2FDDD0B for ; Fri, 28 Aug 2009 20:39:33 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751528AbZH1KjX (ORCPT ); Fri, 28 Aug 2009 06:39:23 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751451AbZH1KjX (ORCPT ); Fri, 28 Aug 2009 06:39:23 -0400 Received: from hapkido.dreamhost.com ([66.33.216.122]:53419 "EHLO hapkido.dreamhost.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751430AbZH1KjW (ORCPT ); Fri, 28 Aug 2009 06:39:22 -0400 Received: from spunkymail-a1.g.dreamhost.com (caiajhbdcbhh.dreamhost.com [208.97.132.177]) by hapkido.dreamhost.com (Postfix) with ESMTP id 267AC17EB83 for ; Fri, 28 Aug 2009 03:39:24 -0700 (PDT) Received: from [192.168.10.51] (12.Red-81-44-17.dynamicIP.rima-tde.net [81.44.17.12]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by spunkymail-a1.g.dreamhost.com (Postfix) with ESMTP id 40E8EFE21F; Fri, 28 Aug 2009 03:38:42 -0700 (PDT) Message-ID: <4A97B3A9.6040103@hiramoto.org> Date: Fri, 28 Aug 2009 12:38:33 +0200 From: Karl Hiramoto User-Agent: Thunderbird 2.0.0.22 (X11/20090701) MIME-Version: 1.0 To: linux-atm-general@lists.sourceforge.net Cc: netdev@vger.kernel.org Subject: [PATCH] br2684 testing needed for packet loss and performance Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, Anyone care to test or comment on these patches? I've attached versions for 2.6.28 and 2.6.30. Note I've done all my tests without any QOS settings. I start my link by something like "br2684ctl -c 0 -e 0 -p 1 -b -a 8.32" Without these patches, if you open one or many file transfers, then also do a ping you will notice heavy packet loss. This packet loss is being caused by the call to dev_kfree_skb(skb); With these patches my max throughput drops 20% to 40% or so on a 24 Mbps ADSL2+ line, but no longer have packet loss. On a 15Mbps (and slower) line, I no longer see packet loss with these patches. Thanks --- Karl Signed-off-by: Karl Hiramoto Signed-off-by: Karl Hiramoto --- /usr/src/linux-2.6.30.5/net/atm/br2684.c 2009-06-10 05:05:27.000000000 +0200 +++ linux-2.6.30.5/net/atm/br2684.c 2009-08-28 12:02:41.000000000 +0200 @@ -69,7 +69,7 @@ struct br2684_vcc { struct net_device *device; /* keep old push, pop functions for chaining */ void (*old_push) (struct atm_vcc * vcc, struct sk_buff * skb); - /* void (*old_pop)(struct atm_vcc *vcc, struct sk_buff *skb); */ + void (*old_pop)(struct atm_vcc *vcc, struct sk_buff *skb); enum br2684_encaps encaps; struct list_head brvccs; #ifdef CONFIG_ATM_BR2684_IPFILTER @@ -142,6 +142,22 @@ static struct net_device *br2684_find_de return NULL; } +/* chained vcc->pop function. Check if we should wake the netif_queue */ +static void br2684_pop(struct atm_vcc *vcc, struct sk_buff *skb) +{ + struct br2684_vcc *brvcc = BR2684_VCC(vcc); + struct net_device *net_dev = skb->dev; + + pr_debug("br2684_pop(vcc %p ; net_dev %p )\n", vcc, net_dev); + brvcc->old_pop(vcc, skb); + + if (!net_dev) + return; + + if (atm_may_send(vcc, 0) { + netif_wake_queue(net_dev); + } +} /* * Send a packet out a particular vcc. Not to useful right now, but paves * the way for multiple vcc's per itf. Returns true if we can send, @@ -200,20 +216,20 @@ static int br2684_xmit_vcc(struct sk_buf ATM_SKB(skb)->vcc = atmvcc = brvcc->atmvcc; pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, atmvcc, atmvcc->dev); - if (!atm_may_send(atmvcc, skb->truesize)) { - /* - * We free this here for now, because we cannot know in a higher - * layer whether the skb pointer it supplied wasn't freed yet. - * Now, it always is. - */ - dev_kfree_skb(skb); - return 0; - } atomic_add(skb->truesize, &sk_atm(atmvcc)->sk_wmem_alloc); ATM_SKB(skb)->atm_options = atmvcc->atm_options; dev->stats.tx_packets++; dev->stats.tx_bytes += skb->len; atmvcc->send(atmvcc, skb); + + if (!atm_may_send(atmvcc, 0)) { + netif_stop_queue(brvcc->device); + barrier(); + /*check for race with br2684_pop*/ + if (atm_may_send(atmvcc, 0)) + netif_start_queue(brvcc->device); + } + return 1; } @@ -502,8 +518,10 @@ static int br2684_regvcc(struct atm_vcc atmvcc->user_back = brvcc; brvcc->encaps = (enum br2684_encaps)be.encaps; brvcc->old_push = atmvcc->push; + brvcc->old_pop = atmvcc->pop; barrier(); atmvcc->push = br2684_push; + atmvcc->pop = br2684_pop; rq = &sk_atm(atmvcc)->sk_receive_queue; --- linux-2.6.28.9.orig/net/atm/br2684.c 2009-03-23 22:55:52.000000000 +0100 +++ linux-2.6.28.9/net/atm/br2684.c 2009-08-28 12:00:43.000000000 +0200 @@ -69,7 +69,7 @@ struct br2684_vcc { struct net_device *device; /* keep old push, pop functions for chaining */ void (*old_push) (struct atm_vcc * vcc, struct sk_buff * skb); - /* void (*old_pop)(struct atm_vcc *vcc, struct sk_buff *skb); */ + void (*old_pop)(struct atm_vcc *vcc, struct sk_buff *skb); enum br2684_encaps encaps; struct list_head brvccs; #ifdef CONFIG_ATM_BR2684_IPFILTER @@ -143,6 +143,23 @@ static struct net_device *br2684_find_de return NULL; } +/* chained vcc->pop function. Check if we should wake the netif_queue */ +static void br2684_pop(struct atm_vcc *vcc, struct sk_buff *skb) +{ + struct br2684_vcc *brvcc = BR2684_VCC(vcc); + struct net_device *net_dev = brvcc->device; + + pr_debug("br2684_pop(vcc %p ; net_dev %p )\n", vcc, net_dev); + brvcc->old_pop(vcc, skb); + + if (!net_dev) + return; + + if (atm_may_send(vcc, 0)) { + netif_wake_queue(net_dev); + } +} + /* * Send a packet out a particular vcc. Not to useful right now, but paves * the way for multiple vcc's per itf. Returns true if we can send, @@ -200,20 +217,22 @@ static int br2684_xmit_vcc(struct sk_buf ATM_SKB(skb)->vcc = atmvcc = brvcc->atmvcc; pr_debug("atm_skb(%p)->vcc(%p)->dev(%p)\n", skb, atmvcc, atmvcc->dev); - if (!atm_may_send(atmvcc, skb->truesize)) { - /* - * We free this here for now, because we cannot know in a higher - * layer whether the skb pointer it supplied wasn't freed yet. - * Now, it always is. - */ - dev_kfree_skb(skb); - return 0; - } + atomic_add(skb->truesize, &sk_atm(atmvcc)->sk_wmem_alloc); ATM_SKB(skb)->atm_options = atmvcc->atm_options; brdev->stats.tx_packets++; brdev->stats.tx_bytes += skb->len; atmvcc->send(atmvcc, skb); + + if (!atm_may_send(atmvcc, 0)) { + netif_stop_queue(brvcc->device); + barrier(); + /* check for race with br26864_pop*/ + if (atm_may_send(atmvcc, 0)) { + netif_start_queue(brvcc->device); + } + } + return 1; } @@ -509,8 +528,10 @@ static int br2684_regvcc(struct atm_vcc atmvcc->user_back = brvcc; brvcc->encaps = (enum br2684_encaps)be.encaps; brvcc->old_push = atmvcc->push; + brvcc->old_pop = atmvcc->pop; barrier(); atmvcc->push = br2684_push; + atmvcc->pop = br2684_pop; rq = &sk_atm(atmvcc)->sk_receive_queue; @@ -621,6 +642,8 @@ static int br2684_create(void __user * a write_lock_irq(&devs_lock); brdev->payload = payload; + netif_start_queue(netdev); + brdev->number = list_empty(&br2684_devs) ? 1 : BRPRIV(list_entry_brdev(br2684_devs.prev))->number + 1; list_add_tail(&brdev->br2684_devs, &br2684_devs);