From: Karl Hiramoto <karl@hiramoto•org>
To: linux-atm-general@lists•sourceforge.net
Cc: netdev@vger•kernel.org
Subject: [PATCH] br2684 testing needed for packet loss and performance
Date: Fri, 28 Aug 2009 12:38:33 +0200 [thread overview]
Message-ID: <4A97B3A9.6040103@hiramoto.org> (raw)
[-- Attachment #1: Type: text/plain, Size: 647 bytes --]
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
[-- Attachment #2: linux-2.6.28.br2684.patch --]
[-- Type: text/plain, Size: 2781 bytes --]
Signed-off-by: Karl Hiramoto <karl@hiramoto•org>
--- 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);
[-- Attachment #3: linux-2.6.30.br2684.patch --]
[-- Type: text/plain, Size: 2447 bytes --]
Signed-off-by: Karl Hiramoto <karl@hiramoto•org>
--- /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;
next reply other threads:[~2009-08-28 10:39 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-28 10:38 Karl Hiramoto [this message]
2009-08-28 12:25 ` [Linux-ATM-General] [PATCH] br2684 testing needed for packet loss and performance Chas Williams (CONTRACTOR)
2009-08-29 10:24 ` Karl Hiramoto
2009-08-29 11:24 ` [PATCH] atm/br2684: netif_stop_queue() when atm device busy and netif_wake_queue() when we can send packets again Karl Hiramoto
2009-08-31 14:29 ` Chas Williams (CONTRACTOR)
2009-09-03 6:27 ` David Miller
2009-09-03 13:44 ` Chas Williams (CONTRACTOR)
2009-09-10 19:49 ` [Linux-ATM-General] " Philip A. Prindeville
2009-09-10 21:30 ` Karl Hiramoto
2009-09-11 18:48 ` David Miller
2009-09-15 13:44 ` Karl Hiramoto
2009-09-15 14:57 ` Karl Hiramoto
2009-09-16 18:04 ` Philip A. Prindeville
2009-09-11 19:56 ` Philip A. Prindeville
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A97B3A9.6040103@hiramoto.org \
--to=karl@hiramoto$(echo .)org \
--cc=linux-atm-general@lists$(echo .)sourceforge.net \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox