Netdev List
 help / color / mirror / Atom feed
* [3/3] [NEIGH] Fix timer leak in neigh_changeaddr
From: Herbert Xu @ 2005-10-23  7:33 UTC (permalink / raw)
  To: Reuben Farrelly; +Cc: akpm, linux-kernel, netdev, acme, davem, greearb
In-Reply-To: <E1ETaJB-0004a0-00@gondolin.me.apana.org.au>

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

[NEIGH] Fix timer leak in neigh_changeaddr

neigh_changeaddr attempts to delete neighbour timers without setting
nud_state.  This doesn't work because the timer may have already fired
when we acquire the write lock in neigh_changeaddr.  The result is that
the timer may keep firing for quite a while until the entry reaches
NEIGH_FAILED.

It should be setting the nud_state straight away so that if the timer
has already fired it can simply exit once we relinquish the lock.

In fact, this whole function is simply duplicating the logic in
neigh_ifdown which in turn is already doing the right thing when
it comes to deleting timers and setting nud_state.

So all we have to do is take that code out and put it into a common
function and make both neigh_changeaddr and neigh_ifdown call it.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p3.patch --]
[-- Type: text/plain, Size: 1446 bytes --]

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -175,39 +175,10 @@ static void pneigh_queue_purge(struct sk
 	}
 }
 
-void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev)
 {
 	int i;
 
-	write_lock_bh(&tbl->lock);
-
-	for (i=0; i <= tbl->hash_mask; i++) {
-		struct neighbour *n, **np;
-
-		np = &tbl->hash_buckets[i];
-		while ((n = *np) != NULL) {
-			if (dev && n->dev != dev) {
-				np = &n->next;
-				continue;
-			}
-			*np = n->next;
-			write_lock_bh(&n->lock);
-			n->dead = 1;
-			neigh_del_timer(n);
-			write_unlock_bh(&n->lock);
-			neigh_release(n);
-		}
-	}
-
-        write_unlock_bh(&tbl->lock);
-}
-
-int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
-{
-	int i;
-
-	write_lock_bh(&tbl->lock);
-
 	for (i = 0; i <= tbl->hash_mask; i++) {
 		struct neighbour *n, **np = &tbl->hash_buckets[i];
 
@@ -243,7 +214,19 @@ int neigh_ifdown(struct neigh_table *tbl
 			neigh_release(n);
 		}
 	}
+}
 
+void neigh_changeaddr(struct neigh_table *tbl, struct net_device *dev)
+{
+	write_lock_bh(&tbl->lock);
+	neigh_flush_dev(tbl, dev);
+	write_unlock_bh(&tbl->lock);
+}
+
+int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev)
+{
+	write_lock_bh(&tbl->lock);
+	neigh_flush_dev(tbl, dev);
 	pneigh_ifdown(tbl, dev);
 	write_unlock_bh(&tbl->lock);
 

^ permalink raw reply

* [2/3] [NEIGH] Fix add_timer race in neigh_add_timer
From: Herbert Xu @ 2005-10-23  7:32 UTC (permalink / raw)
  To: Reuben Farrelly; +Cc: akpm, linux-kernel, netdev, acme, davem, greearb
In-Reply-To: <E1ETaJB-0004a0-00@gondolin.me.apana.org.au>

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

[NEIGH] Fix add_timer race in neigh_add_timer

neigh_add_timer cannot use add_timer unconditionally.  The reason is that
by the time it has obtained the write lock someone else (e.g., neigh_update)
could have already added a new timer.

So it should only use mod_timer and deal with its return value accordingly.

This bug would have led to rare neighbour cache entry leaks.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p2.patch --]
[-- Type: text/plain, Size: 523 bytes --]

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -816,10 +816,10 @@ static void neigh_timer_handler(unsigned
 	}
 
 	if (neigh->nud_state & NUD_IN_TIMER) {
-		neigh_hold(neigh);
 		if (time_before(next, jiffies + HZ/2))
 			next = jiffies + HZ/2;
-		neigh_add_timer(neigh, next);
+		if (!mod_timer(&neigh->timer, next))
+			neigh_hold(neigh);
 	}
 	if (neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) {
 		struct sk_buff *skb = skb_peek(&neigh->arp_queue);

^ permalink raw reply

* [1/3] [NEIGH] Print stack trace in neigh_add_timer
From: Herbert Xu @ 2005-10-23  7:31 UTC (permalink / raw)
  To: Reuben Farrelly; +Cc: akpm, linux-kernel, netdev, acme, davem, greearb
In-Reply-To: <E1ETaJB-0004a0-00@gondolin.me.apana.org.au>

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

[NEIGH] Print stack trace in neigh_add_timer

Stack traces are very helpful in determining the exact nature of a bug.
So let's print a stack trace when the timer is added twice.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p1.patch --]
[-- Type: text/plain, Size: 326 bytes --]

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -732,6 +732,7 @@ static inline void neigh_add_timer(struc
 	if (unlikely(mod_timer(&n->timer, when))) {
 		printk("NEIGH: BUG, double timer add, state is %x\n",
 		       n->nud_state);
+		dump_stack();
 	}
 }
 

^ permalink raw reply

* [0/3] Fix timer bugs in neighbour cache
From: Herbert Xu @ 2005-10-23  7:30 UTC (permalink / raw)
  To: Reuben Farrelly; +Cc: akpm, linux-kernel, netdev, acme, davem, greearb
In-Reply-To: <43534273.2050106@reub.net>

Reuben Farrelly <reuben-lkml@reub.net> wrote:
> 
> Oct 17 18:49:40 tornado kernel: NEIGH: BUG, double timer add, state is 1
> Oct 17 18:51:04 tornado last message repeated 3 times
> Oct 17 18:52:05 tornado last message repeated 5 times
> Oct 17 18:52:11 tornado last message repeated 2 times

Excellent.  Looks like we actually caught something.  Pity we don't have
a stack trace which means that there might be more bugs.

Anyway, here are three patches which should fix this.  This should go
into 2.6.14.

Arnaldo, you can pull them from

master.kernel.org:/pub/scm/linux/kernel/git/herbert/net-2.6.git

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Abolish everything you owe without paying another dime
From: krystal martin @ 2005-10-22 16:19 UTC (permalink / raw)
  To: Tawana Perez


Do away with all you are indebted for with out paying another cent.  

End the calls. Bring to an end to the payments! As it turns out most
lending establishments are doing something illegal. Amazing but correct!
Join us for thorough fine points concerning our system at 0.00 payment or
obligation. You have not anything to lose and plenty to gain.

http://es.geocities.com/kaelea_glover/?s=EA.Get rid of all that you owe
without sending an other dime
Conprehensive information or to bring to a close receiving or to see postal

Boost sales and exposure overnight thru professional and massive electronic
mail marketing
Deal with the biggest and the best marketmajors@turbonett.com

An electric current will instantly be directed upon your foe, rendering him
wholly unconscious for the period of one hour. During that time you will
have opportunity to escape
As for your enemy, after regaining consciousness he will suffer no
inconvenience from the encounter beyond a slight headache

^ permalink raw reply

* Re: [NF+IPsec 1/6]: Remove okfn usage in ip_vs_core.c
From: Julian Anastasov @ 2005-10-22 13:45 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Kernel Netdev Mailing List, Netfilter Development Mailinglist,
	Herbert Xu
In-Reply-To: <4352EEAB.8030708@trash.net>


	Hello,

On Mon, 17 Oct 2005, Patrick McHardy wrote:

> This is my current set of netfilter+IPsec patches with Herbert's
> suggestions incorporated. Changes since the last posted patches:
>
> - remove okfn use in ipvs and ip_conntrack to avoid deep
>    callchains with IPsec

	Such NF_STOP usage in IPVS looks ok

Acked-by: Julian Anastasov <ja@ssi.bg>

> - only pass packets to netfilter after tunnel mode transforms,
>    except for once in plain before encapsulation or after
>    decapsulation.
> - NAT support

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* ちょっとエッチな女性の為の出会いサイト
From: 安心−出会える系 @ 2005-10-22  2:11 UTC (permalink / raw)
  To: netdev


女性の為の安心出会いサイト
大人の出会い、メル友探し、まじめな恋人探しなど
全てのジャンルを扱うNO.1出会いサイト。
http://futarinojikan.net/24/

完全無料で入会後も様々な特典があり。
純愛系から人妻で秘密の関係まで
自分のスタイルに合わせて利用できる、
安心出会いサイトです。
http://futarinojikan.net/24/

^ permalink raw reply

* Re: [patch] forcedeth: add support for interrupt mitigation
From: Jeff Garzik @ 2005-10-21 21:33 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Netdev, Ayaz Abdulla
In-Reply-To: <43592FE5.20106@colorfullife.com>

Manfred Spraul wrote:
> Hi,
> 
> The current forcedeth driver doesn't support interrupt mitigation, this 
> can result in an incredible number of interrupts/sec for gigabit links. 
> The attached patch adds a throughput mode that enables an interrupt 
> mitigation scheme.
> 
> 
> -- 
>    Manfred
> 
> 
> ------------------------------------------------------------------------
> 
> --- 2.6/drivers/net/forcedeth.c	2005-10-20 23:17:24.000000000 +0200
> +++ build-2.6/drivers/net/forcedeth.c	2005-10-19 21:01:55.000000000 +0200
> @@ -98,6 +98,7 @@
>   *	0.43: 10 Aug 2005: Add support for tx checksum.
>   *	0.44: 20 Aug 2005: Add support for scatter gather and segmentation.
>   *	0.45: 18 Sep 2005: Remove nv_stop/start_rx from every link check
> + *	0.46: 20 Aug 2005: Add irq optimization modes.
>   *
>   * Known bugs:
>   * We suspect that on some hardware no TX done interrupts are generated.
> @@ -109,7 +110,7 @@
>   * DEV_NEED_TIMERIRQ will not harm you on sane hardware, only generating a few
>   * superfluous timer interrupts from the nic.
>   */
> -#define FORCEDETH_VERSION		"0.45"
> +#define FORCEDETH_VERSION		"0.46"
>  #define DRV_NAME			"forcedeth"
>  
>  #include <linux/module.h>
> @@ -164,7 +165,8 @@
>  #define NVREG_IRQ_LINK			0x0040
>  #define NVREG_IRQ_TX_ERROR		0x0080
>  #define NVREG_IRQ_TX1			0x0100
> -#define NVREG_IRQMASK_WANTED		0x00df
> +#define NVREG_IRQMASK_THROUGHPUT	0x00df
> +#define NVREG_IRQMASK_CPU		0x0040
>  
>  #define NVREG_IRQ_UNKNOWN	(~(NVREG_IRQ_RX_ERROR|NVREG_IRQ_RX|NVREG_IRQ_RX_NOBUF|NVREG_IRQ_TX_ERR| \
>  					NVREG_IRQ_TX_OK|NVREG_IRQ_TIMER|NVREG_IRQ_LINK|NVREG_IRQ_TX_ERROR| \
> @@ -178,7 +180,8 @@
>   * NVREG_POLL_DEFAULT=97 would result in an interval length of 1 ms
>   */
>  	NvRegPollingInterval = 0x00c,
> -#define NVREG_POLL_DEFAULT	970
> +#define NVREG_POLL_DEFAULT_THROUGHPUT	970
> +#define NVREG_POLL_DEFAULT_CPU	13
>  	NvRegMisc1 = 0x080,
>  #define NVREG_MISC1_HD		0x02
>  #define NVREG_MISC1_FORCE	0x3b0f3c
> @@ -539,6 +542,18 @@
>   */
>  static int max_interrupt_work = 5;
>  
> +/*
> + * Optimization can be either throuput mode or cpu mode
> + */
> +#define NV_OPTIMIZATION_MODE_THROUGHPUT 0
> +#define NV_OPTIMIZATION_MODE_CPU        1
> +static int optimization_mode = NV_OPTIMIZATION_MODE_THROUGHPUT;
> +
> +/*
> + * Poll interval for timer irq
> + */
> +static int poll_interval = -1;

The code changes themselves seem correct, but this patch suffers from a 
few overall problems.

* "throughput or cpu" doesn't tell the user very much -- or me, for that 
matter.  Does "cpu mode" mean low latency, high interrupt count?  If so, 
just allow the user to choose between throughput and latency.

* making this a static decision at module load time is sub-optimal.  99% 
of users will simply use the default.  the option should be 
per-interface, ideally.  controlled by ethtool?

* there is zero information on how to use poll interval.  again, 99% of 
users will simply use the default.  since the poll interval is written 
directly to a hardware register, you should give the user some idea of 
the unit of measure (ms? ticks? bus cycles? ns?), and some idea of min/max.

^ permalink raw reply

* Re: [PATCH,CFT] forcedeth: Remove superflous rx engine stop/start cycles.
From: Jeff Garzik @ 2005-10-21 21:24 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Netdev, Ayaz Abdulla
In-Reply-To: <432D7B98.8050307@colorfullife.com>

Manfred Spraul wrote:
> Hi all,
> 
> Ayaz noticed that forcedeth stops and restarts the rx engine every 3 
> seconds (link timeout). The attached patch fixes that.
> It also contains a larger whitespace cleanup: I've replaced a few spaces 
> in the comments with tabs.
> 
> Please test it.

Patch OK, but dropped since version 0.44 was dropped.

	Jeff

^ permalink raw reply

* Re: [PATCH 3/2] forcedeth: Compile fix forcedeth 0.44
From: Jeff Garzik @ 2005-10-21 21:23 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Ayaz Abdulla, Netdev
In-Reply-To: <432D771D.7050107@colorfullife.com>

Manfred Spraul wrote:
> Hi,
> 
> forcedeth-0.44 contains a spurious ; in nv_release_txskb. gcc-4 compiles 
> it with a warning, older compilers might reject it.
> The attached onliner fixes that, sorry.
> 
> Signed-off-By: Manfred Spraul <manfred@colorfullife.com>
> 
> 
> ------------------------------------------------------------------------
> 
> --- 2.6/drivers/net/forcedeth.c	2005-09-18 16:12:10.000000000 +0200
> +++ build-2.6/drivers/net/forcedeth.c	2005-09-18 16:14:19.000000000 +0200
> @@ -923,7 +923,7 @@ static int nv_init_ring(struct net_devic
>  static void nv_release_txskb(struct net_device *dev, unsigned int skbnr)
>  {
>  	struct fe_priv *np = get_nvpriv(dev);
> -	struct sk_buff *skb = np->tx_skbuff[skbnr];;
> +	struct sk_buff *skb = np->tx_skbuff[skbnr];
>  	unsigned int j, entry, fragments;

obviously OK, but dropped since the previous patch was dropped

^ permalink raw reply

* Re: [PATCH 2/2] forcedeth: scatter gather and segmentation offload support
From: Jeff Garzik @ 2005-10-21 21:23 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Netdev, Ayaz Abdulla
In-Reply-To: <432D7354.8000503@colorfullife.com>

Manfred Spraul wrote:
> The attached patch adds scatter gather and segmentation offload support
> into forcedeth driver.
> 
> This patch has been tested by NVIDIA and reviewed by Manfred.
> 
> Notes:
> - Manfred mentioned that mapping of pages could take time and should not
> be under spinlock for performance reasons
> - During testing with netperf, I have noticed a connection running
> segmentation offload gets "unoffloaded" by the kernel due to possible
> retransmissions.
> 
> Thanks,
> Ayaz
> 
> Signed-off-by: Ayaz Abdulla <aabdulla@nvidia.com>
> Signed-off-By: Manfred Spraul <manfred@colorfullife.com>

Patch needs to be updated per [minor] comments below, and resent.


> ------------------------------------------------------------------------
> 
> --- orig-2.6/drivers/net/forcedeth.c	2005-09-06 11:54:41.000000000 -0700
> +++ 2.6/drivers/net/forcedeth.c	2005-09-06 13:52:50.000000000 -0700
> @@ -915,21 +920,44 @@
>  	return nv_alloc_rx(dev);
>  }
>  
> +static void nv_release_txskb(struct net_device *dev, unsigned int skbnr)
> +{
> +	struct fe_priv *np = get_nvpriv(dev);

Remove get_nvpriv() and call netdev_priv() directly.


> +	struct sk_buff *skb = np->tx_skbuff[skbnr];;
> +	unsigned int j, entry, fragments;
> +			
> +	dprintk(KERN_INFO "%s: nv_release_txskb for skbnr %d, skb %p\n",
> +		dev->name, skbnr, np->tx_skbuff[skbnr]);
> +	
> +	entry = skbnr;
> +	if ((fragments = skb_shinfo(skb)->nr_frags) != 0) {
> +		for (j = fragments; j >= 1; j--) {
> +			skb_frag_t *frag = &skb_shinfo(skb)->frags[j-1];
> +			pci_unmap_page(np->pci_dev, np->tx_dma[entry],
> +				       frag->size,
> +				       PCI_DMA_TODEVICE);
> +			entry = (entry - 1) % TX_RING;
> +		}
> +	}
> +	pci_unmap_single(np->pci_dev, np->tx_dma[entry],
> +			 skb->len - skb->data_len,
> +			 PCI_DMA_TODEVICE);
> +	dev_kfree_skb_irq(skb);
> +	np->tx_skbuff[skbnr] = NULL;
> +}
> +
>  static void nv_drain_tx(struct net_device *dev)
>  {
>  	struct fe_priv *np = get_nvpriv(dev);

ditto, etc.


> -	int i;
> +	unsigned int i;
> +	
>  	for (i = 0; i < TX_RING; i++) {
>  		if (np->desc_ver == DESC_VER_1 || np->desc_ver == DESC_VER_2)
>  			np->tx_ring.orig[i].FlagLen = 0;
>  		else
>  			np->tx_ring.ex[i].FlagLen = 0;
>  		if (np->tx_skbuff[i]) {
> -			pci_unmap_single(np->pci_dev, np->tx_dma[i],
> -						np->tx_skbuff[i]->len,
> -						PCI_DMA_TODEVICE);
> -			dev_kfree_skb(np->tx_skbuff[i]);
> -			np->tx_skbuff[i] = NULL;
> +			nv_release_txskb(dev, i);
>  			np->stats.tx_dropped++;
>  		}
>  	}
> @@ -968,28 +996,69 @@
>  static int nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
>  	struct fe_priv *np = get_nvpriv(dev);
> -	int nr = np->next_tx % TX_RING;
> -	u32 tx_checksum = (skb->ip_summed == CHECKSUM_HW ? (NV_TX2_CHECKSUM_L3|NV_TX2_CHECKSUM_L4) : 0);
> +	u32 tx_flags_extra = (np->desc_ver == DESC_VER_1 ? NV_TX_LASTPACKET : NV_TX2_LASTPACKET);
> +	unsigned int fragments = skb_shinfo(skb)->nr_frags;
> +	unsigned int nr = (np->next_tx + fragments) % TX_RING;
> +	unsigned int i;
> +
> +	spin_lock_irq(&np->lock);
> +	wmb();

wmb() appears spurious AFAICS, with spinlock

> +	if ((np->next_tx - np->nic_tx + fragments) > TX_LIMIT_STOP) {

Why not references MAX_SKB_FRAGS like everyone else?


> +		spin_unlock_irq(&np->lock);
> +		netif_stop_queue(dev);
> +		return 1;

new code should properly use NETDEV_TX_xxx return code


> @@ -1020,7 +1087,8 @@
>  {
>  	struct fe_priv *np = get_nvpriv(dev);

use netdev_priv() directly

The rest looks OK.

	Jeff

^ permalink raw reply

* RE: [patch] forcedeth: add support for interrupt mitigation
From: Ayaz Abdulla @ 2005-10-21 21:10 UTC (permalink / raw)
  To: Jeff Garzik, Francois Romieu; +Cc: Manfred Spraul, Netdev, John W. Linville

Yes, thats right. I just gave Manfred the hardware interrupt mitigation
patch (v46). Next, I will implement NAPI.

-----Original Message-----
From: Jeff Garzik [mailto:jgarzik@pobox.com] 
Sent: Friday, October 21, 2005 2:07 PM
To: Francois Romieu
Cc: Manfred Spraul; Netdev; Ayaz Abdulla; John W. Linville
Subject: Re: [patch] forcedeth: add support for interrupt mitigation


FWIW the ideal is to implement both NAPI -and- hardware interrupt 
mitigation.

	Jeff

^ permalink raw reply

* Re: [patch] forcedeth: add support for interrupt mitigation
From: Jeff Garzik @ 2005-10-21 21:07 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Manfred Spraul, Netdev, Ayaz Abdulla, John W. Linville
In-Reply-To: <20051021202908.GA18966@electric-eye.fr.zoreil.com>

FWIW the ideal is to implement both NAPI -and- hardware interrupt 
mitigation.

	Jeff

^ permalink raw reply

* Re: [patch] forcedeth: add support for interrupt mitigation
From: John W. Linville @ 2005-10-21 20:42 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Manfred Spraul, Netdev, Ayaz Abdulla
In-Reply-To: <20051021202908.GA18966@electric-eye.fr.zoreil.com>

On Fri, Oct 21, 2005 at 10:29:08PM +0200, Francois Romieu wrote:
> Manfred Spraul <manfred@colorfullife.com> :
> > The current forcedeth driver doesn't support interrupt mitigation, this 
> > can result in an incredible number of interrupts/sec for gigabit links. 
> > The attached patch adds a throughput mode that enables an interrupt 
> > mitigation scheme.
> 
> Naïve question: what about the NAPI way ?

I was thinking the same thing...although it appears forcedeth already
supports NAPI...

What does this offer over the NAPI support?

John
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply

* Re: [patch] forcedeth: add support for interrupt mitigation
From: Francois Romieu @ 2005-10-21 20:29 UTC (permalink / raw)
  To: Manfred Spraul; +Cc: Netdev, Ayaz Abdulla
In-Reply-To: <43592FE5.20106@colorfullife.com>

Manfred Spraul <manfred@colorfullife.com> :
> The current forcedeth driver doesn't support interrupt mitigation, this 
> can result in an incredible number of interrupts/sec for gigabit links. 
> The attached patch adds a throughput mode that enables an interrupt 
> mitigation scheme.

Naïve question: what about the NAPI way ?

--
Ueimor

^ permalink raw reply

* [patch] forcedeth: add support for interrupt mitigation
From: Manfred Spraul @ 2005-10-21 18:13 UTC (permalink / raw)
  To: Netdev; +Cc: Ayaz Abdulla

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Hi,

The current forcedeth driver doesn't support interrupt mitigation, this 
can result in an incredible number of interrupts/sec for gigabit links. 
The attached patch adds a throughput mode that enables an interrupt 
mitigation scheme.


--
    Manfred

[-- Attachment #2: patch-forcedeth-046-optimization-mode --]
[-- Type: text/plain, Size: 7593 bytes --]

--- 2.6/drivers/net/forcedeth.c	2005-10-20 23:17:24.000000000 +0200
+++ build-2.6/drivers/net/forcedeth.c	2005-10-19 21:01:55.000000000 +0200
@@ -98,6 +98,7 @@
  *	0.43: 10 Aug 2005: Add support for tx checksum.
  *	0.44: 20 Aug 2005: Add support for scatter gather and segmentation.
  *	0.45: 18 Sep 2005: Remove nv_stop/start_rx from every link check
+ *	0.46: 20 Aug 2005: Add irq optimization modes.
  *
  * Known bugs:
  * We suspect that on some hardware no TX done interrupts are generated.
@@ -109,7 +110,7 @@
  * DEV_NEED_TIMERIRQ will not harm you on sane hardware, only generating a few
  * superfluous timer interrupts from the nic.
  */
-#define FORCEDETH_VERSION		"0.45"
+#define FORCEDETH_VERSION		"0.46"
 #define DRV_NAME			"forcedeth"
 
 #include <linux/module.h>
@@ -164,7 +165,8 @@
 #define NVREG_IRQ_LINK			0x0040
 #define NVREG_IRQ_TX_ERROR		0x0080
 #define NVREG_IRQ_TX1			0x0100
-#define NVREG_IRQMASK_WANTED		0x00df
+#define NVREG_IRQMASK_THROUGHPUT	0x00df
+#define NVREG_IRQMASK_CPU		0x0040
 
 #define NVREG_IRQ_UNKNOWN	(~(NVREG_IRQ_RX_ERROR|NVREG_IRQ_RX|NVREG_IRQ_RX_NOBUF|NVREG_IRQ_TX_ERR| \
 					NVREG_IRQ_TX_OK|NVREG_IRQ_TIMER|NVREG_IRQ_LINK|NVREG_IRQ_TX_ERROR| \
@@ -178,7 +180,8 @@
  * NVREG_POLL_DEFAULT=97 would result in an interval length of 1 ms
  */
 	NvRegPollingInterval = 0x00c,
-#define NVREG_POLL_DEFAULT	970
+#define NVREG_POLL_DEFAULT_THROUGHPUT	970
+#define NVREG_POLL_DEFAULT_CPU	13
 	NvRegMisc1 = 0x080,
 #define NVREG_MISC1_HD		0x02
 #define NVREG_MISC1_FORCE	0x3b0f3c
@@ -539,6 +542,18 @@
  */
 static int max_interrupt_work = 5;
 
+/*
+ * Optimization can be either throuput mode or cpu mode
+ */
+#define NV_OPTIMIZATION_MODE_THROUGHPUT 0
+#define NV_OPTIMIZATION_MODE_CPU        1
+static int optimization_mode = NV_OPTIMIZATION_MODE_THROUGHPUT;
+
+/*
+ * Poll interval for timer irq
+ */
+static int poll_interval = -1;
+
 static inline struct fe_priv *get_nvpriv(struct net_device *dev)
 {
 	return netdev_priv(dev);
@@ -1330,67 +1345,71 @@
 			if (!(Flags & NV_RX_DESCRIPTORVALID))
 				goto next_pkt;
 
-			if (Flags & NV_RX_MISSEDFRAME) {
-				np->stats.rx_missed_errors++;
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & (NV_RX_ERROR1|NV_RX_ERROR2|NV_RX_ERROR3)) {
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX_CRCERR) {
-				np->stats.rx_crc_errors++;
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX_OVERFLOW) {
-				np->stats.rx_over_errors++;
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX_ERROR4) {
-				len = nv_getlen(dev, np->rx_skbuff[i]->data, len);
-				if (len < 0) {
+			if (Flags & NV_RX_ERROR) {
+				if (Flags & NV_RX_MISSEDFRAME) {
+					np->stats.rx_missed_errors++;
 					np->stats.rx_errors++;
 					goto next_pkt;
 				}
-			}
-			/* framing errors are soft errors. */
-			if (Flags & NV_RX_FRAMINGERR) {
-				if (Flags & NV_RX_SUBSTRACT1) {
-					len--;
+				if (Flags & (NV_RX_ERROR1|NV_RX_ERROR2|NV_RX_ERROR3)) {
+					np->stats.rx_errors++;
+					goto next_pkt;
+				}
+				if (Flags & NV_RX_CRCERR) {
+					np->stats.rx_crc_errors++;
+					np->stats.rx_errors++;
+					goto next_pkt;
+				}
+				if (Flags & NV_RX_OVERFLOW) {
+					np->stats.rx_over_errors++;
+					np->stats.rx_errors++;
+					goto next_pkt;
+				}
+				if (Flags & NV_RX_ERROR4) {
+					len = nv_getlen(dev, np->rx_skbuff[i]->data, len);
+					if (len < 0) {
+						np->stats.rx_errors++;
+						goto next_pkt;
+					}
+				}
+				/* framing errors are soft errors. */
+				if (Flags & NV_RX_FRAMINGERR) {
+					if (Flags & NV_RX_SUBSTRACT1) {
+						len--;
+					}
 				}
 			}
 		} else {
 			if (!(Flags & NV_RX2_DESCRIPTORVALID))
 				goto next_pkt;
 
-			if (Flags & (NV_RX2_ERROR1|NV_RX2_ERROR2|NV_RX2_ERROR3)) {
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX2_CRCERR) {
-				np->stats.rx_crc_errors++;
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX2_OVERFLOW) {
-				np->stats.rx_over_errors++;
-				np->stats.rx_errors++;
-				goto next_pkt;
-			}
-			if (Flags & NV_RX2_ERROR4) {
-				len = nv_getlen(dev, np->rx_skbuff[i]->data, len);
-				if (len < 0) {
+			if (Flags & NV_RX2_ERROR) {
+				if (Flags & (NV_RX2_ERROR1|NV_RX2_ERROR2|NV_RX2_ERROR3)) {
 					np->stats.rx_errors++;
 					goto next_pkt;
 				}
-			}
-			/* framing errors are soft errors */
-			if (Flags & NV_RX2_FRAMINGERR) {
-				if (Flags & NV_RX2_SUBSTRACT1) {
-					len--;
+				if (Flags & NV_RX2_CRCERR) {
+					np->stats.rx_crc_errors++;
+					np->stats.rx_errors++;
+					goto next_pkt;
+				}
+				if (Flags & NV_RX2_OVERFLOW) {
+					np->stats.rx_over_errors++;
+					np->stats.rx_errors++;
+					goto next_pkt;
+				}
+				if (Flags & NV_RX2_ERROR4) {
+					len = nv_getlen(dev, np->rx_skbuff[i]->data, len);
+					if (len < 0) {
+						np->stats.rx_errors++;
+						goto next_pkt;
+					}
+				}
+				/* framing errors are soft errors */
+				if (Flags & NV_RX2_FRAMINGERR) {
+					if (Flags & NV_RX2_SUBSTRACT1) {
+						len--;
+					}
 				}
 			}
 			Flags &= NV_RX2_CHECKSUMMASK;
@@ -1810,22 +1829,18 @@
 		if (!(events & np->irqmask))
 			break;
 
-		if (events & (NVREG_IRQ_TX1|NVREG_IRQ_TX_OK|NVREG_IRQ_TX_ERROR|NVREG_IRQ_TX_ERR)) {
+		spin_lock(&np->lock);
+		nv_tx_done(dev);
+		spin_unlock(&np->lock);
+		
+		nv_rx_process(dev);
+		if (nv_alloc_rx(dev)) {
 			spin_lock(&np->lock);
-			nv_tx_done(dev);
+			if (!np->in_shutdown)
+				mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
 			spin_unlock(&np->lock);
 		}
-
-		if (events & (NVREG_IRQ_RX_ERROR|NVREG_IRQ_RX|NVREG_IRQ_RX_NOBUF)) {
-			nv_rx_process(dev);
-			if (nv_alloc_rx(dev)) {
-				spin_lock(&np->lock);
-				if (!np->in_shutdown)
-					mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
-				spin_unlock(&np->lock);
-			}
-		}
-
+		
 		if (events & NVREG_IRQ_LINK) {
 			spin_lock(&np->lock);
 			nv_link_irq(dev);
@@ -2226,7 +2241,14 @@
 	writel(NVREG_RNDSEED_FORCE | (i&NVREG_RNDSEED_MASK), base + NvRegRandomSeed);
 	writel(NVREG_UNKSETUP1_VAL, base + NvRegUnknownSetupReg1);
 	writel(NVREG_UNKSETUP2_VAL, base + NvRegUnknownSetupReg2);
-	writel(NVREG_POLL_DEFAULT, base + NvRegPollingInterval);
+	if (poll_interval == -1) {
+		if (optimization_mode == NV_OPTIMIZATION_MODE_THROUGHPUT)
+			writel(NVREG_POLL_DEFAULT_THROUGHPUT, base + NvRegPollingInterval);
+		else
+			writel(NVREG_POLL_DEFAULT_CPU, base + NvRegPollingInterval);
+	}
+	else
+		writel(poll_interval, base + NvRegPollingInterval);
 	writel(NVREG_UNKSETUP6_VAL, base + NvRegUnknownSetupReg6);
 	writel((np->phyaddr << NVREG_ADAPTCTL_PHYSHIFT)|NVREG_ADAPTCTL_PHYVALID|NVREG_ADAPTCTL_RUNNING,
 			base + NvRegAdapterControl);
@@ -2510,7 +2532,11 @@
 	} else {
 		np->tx_flags = NV_TX2_VALID;
 	}
-	np->irqmask = NVREG_IRQMASK_WANTED;
+	if (optimization_mode == NV_OPTIMIZATION_MODE_THROUGHPUT)
+		np->irqmask = NVREG_IRQMASK_THROUGHPUT;
+	else
+		np->irqmask = NVREG_IRQMASK_CPU;
+
 	if (id->driver_data & DEV_NEED_TIMERIRQ)
 		np->irqmask |= NVREG_IRQ_TIMER;
 	if (id->driver_data & DEV_NEED_LINKTIMER) {
@@ -2698,6 +2724,10 @@
 
 module_param(max_interrupt_work, int, 0);
 MODULE_PARM_DESC(max_interrupt_work, "forcedeth maximum events handled per interrupt");
+module_param(optimization_mode, int, 0);
+MODULE_PARM_DESC(optimization_mode, "forcedeth optimization for througput (0) or cpu (1) mode");
+module_param(poll_interval, int, 0);
+MODULE_PARM_DESC(poll_interval, "forcedeth polling interval for timer irq");
 
 MODULE_AUTHOR("Manfred Spraul <manfred@colorfullife.com>");
 MODULE_DESCRIPTION("Reverse Engineered nForce ethernet driver");

^ permalink raw reply

* Re: [PATCH]behavior of ip6_route_input() for link local address.
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2005-10-21  8:27 UTC (permalink / raw)
  To: yanzheng, davem; +Cc: netdev, linux-kernel
In-Reply-To: <43572256.40101@21cn.com>

Hello.

In article <43572256.40101@21cn.com> (at Thu, 20 Oct 2005 12:51:34 +0800), Yan Zheng <yanzheng@21cn.com> says:

> I find that linux will reply echo request destined to an address which belongs to an interface other than the one from which the request received.
> This behavior doesn't make sense for link local address.

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

Please note that sender does need to setup neighbor
entry by hand to reproduce this bug.
(Link-local address on eth1 is not visible on eth0,
from the point of view of neighbor discovery in IPv6.)

 +--------+               +--------+
 | sender |               | router |
 +---+----+               +-+----+-+
     |eth0              eth0|    |eth1
-----+----------------------+-  -+--------------

-- 
YOSHIFUJI Hideaki @ USAGI Project  <yoshfuji@linux-ipv6.org>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

^ permalink raw reply

* af_rose.c
From: Bernard Pidoux @ 2005-10-20 22:12 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: David S. Miller, Arnaldo Carvalho de Melo, dann frazier, chrisw,
	netdev, linux-hams, Jean-Paul ROUBELAT
In-Reply-To: <20051018203932.GA11053@linux-mips.org>

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

Hi,

Concerning rose module, here is a patch proposed by Jean-Paul F6FBB for 
af_rose.c

It is against linux-2.6.14-rc5

This patch only adds a loop near the end of rose_connect() in order to 
make it try to connect rose next_neighbour node in case of connect 
failure, until we reach the true end of the neighbour list.

With the guys of FADCA and other french hams using ROSE/FPAC we found 
that ROSE did not scan the alternate routes even when there was one 
available.

This could be an important point in case of catastrophic events with 
telecommunication failure. The possibility to handle emergency traffic 
via alternate routes could be a vital necessity.

The principle of this patch has been tested with kernel 2.2 and 2.6 and 
it works well using F6FBB ROSE/FPAC packet switch application.

I suggest that it should be included into the next kernel rose module.

73 de Bernard, f6bvp


[-- Attachment #2: af_rose.c.2.6.14-rc5.diff --]
[-- Type: text/x-patch, Size: 701 bytes --]

--- net/rose/af_rose.c	2005-10-20 23:27:15.000000000 +0200
+++ net/rose/af_rose.c	2005-10-20 23:38:20.000000000 +0200
@@ -751,7 +751,7 @@
 
 		rose_insert_socket(sk);		/* Finish the bind */
 	}
-
+rose_try_next_neigh:
 	rose->dest_addr   = addr->srose_addr;
 	rose->dest_call   = addr->srose_call;
 	rose->rand        = ((long)rose & 0xFFFF) + rose->lci;
@@ -809,6 +809,11 @@
 	}
 
 	if (sk->sk_state != TCP_ESTABLISHED) {
+	/* Try next neighbour */
+		rose->neighbour = rose_get_neigh(&addr->srose_addr, &cause, &diagnostic);
+		if (rose->neighbour)
+			goto rose_try_next_neigh;
+	/* No more neighbour */
 		sock->state = SS_UNCONNECTED;
 		return sock_error(sk);	/* Always set at this point */
 	}

^ permalink raw reply

* Re: [PATCH] skge support for Marvell chips in Toshiba laptops
From: Daniel Drake @ 2005-10-20 15:38 UTC (permalink / raw)
  To: Krzysztof Oledzki; +Cc: Jesse Barnes, shemminger, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.62.0510201645340.20245@bizon.gios.gov.pl>

Krzysztof Oledzki wrote:
> What is the name of sky2 driver? I can't to find it in:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.14-rc4/2.6.14-rc4-mm1/broken-out/ 
> 
> 
> Wrong place?

Its pulled in from the netdev tree.

Daniel

^ permalink raw reply

* Re: [PATCH] skge support for Marvell chips in Toshiba laptops
From: Krzysztof Oledzki @ 2005-10-20 14:50 UTC (permalink / raw)
  To: Daniel Drake; +Cc: Jesse Barnes, shemminger, netdev, linux-kernel
In-Reply-To: <4356A1F5.5010200@gentoo.org>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1151 bytes --]



On Wed, 19 Oct 2005, Daniel Drake wrote:

> Hi Jesse,
>
> Jesse Barnes wrote:
>> Here's a small patch to add the PCI ID and chip type of the chip in my 
>> Toshiba laptop to the skge driver.  I haven't tested it much (just insmoded 
>> it and run ethtool against the corresponding eth1 device), but it doesn't 
>> crash my system, so unless this configuration has already been tested and 
>> is known to have problems, it might be good to add this patch.
>> 
>> I'll test some more with a real network when I get home.
>
> The device ID you added (0x4351) is already claimed by the new sky2 driver.
>
> Unless theres a mistake in sky2's device table, your laptop contains a 
> Yukon-II adapter which is incompatible with the original Yukon chips (skge = 
> Yukon, sky2 = Yukon-II).
>
> On the other hand, I believe Stephen could do with some extra sky2 testing :)
> You can find it in the latest -mm releases.

What is the name of sky2 driver? I can't to find it in:
ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.14-rc4/2.6.14-rc4-mm1/broken-out/

Wrong place?

Best regards,

 			Krzysztof Olędzki

^ permalink raw reply

* Re: [patch 2.6.14-rc4] b44: alternate allocation option for DMA descriptors
From: John W. Linville @ 2005-10-20 14:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-kernel, netdev, jgarzik, pp
In-Reply-To: <1129792626.7620.248.camel@gaston>

On Thu, Oct 20, 2005 at 05:17:05PM +1000, Benjamin Herrenschmidt wrote:

> So basically, what you are doing is: if allocation fails, you try to get
> memory using GFP_KERNEL. If it happens to be in the low 2Gb of memory,
> use it, if not, drop it.
> 
> Did I get that right ?

Yes, that is basically correct.  I wish I had something more clever
than that...suggestions welcome...

John
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply

* Re: [patch 2.6.14-rc3] sundance: include MII address 0 in PHY probe
From: Jeff Garzik @ 2005-10-20 14:07 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-kernel, netdev
In-Reply-To: <10192005080734.15936@bilbo.tuxdriver.com>

John W. Linville wrote:
> Include MII address 0 at the end of the PHY scan.  This covers the
> entire range of possible MII addresses.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

applied

^ permalink raw reply

* Re: [patch netdev-2.6] e1000: Driver version, white space, comments, device id & other
From: Jeff Garzik @ 2005-10-20 14:07 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-kernel, netdev, john.ronciak, ganesh.venkatesan,
	mallikarjuna.chilakala
In-Reply-To: <10192005104008.17766@bilbo.tuxdriver.com>

applied

^ permalink raw reply

* Re: [PATCH] X25: Add ITU-T facilites
From: Arnd Bergmann @ 2005-10-20 12:30 UTC (permalink / raw)
  To: Andrew Hendry
  Cc: linux-os (Dick Johnson), Arnaldo Carvalho de Melo,
	YOSHIFUJI Hideaki / ?$B5HF#1QL@, eis, linux-x25, linux-kernel,
	netdev
In-Reply-To: <1129770654.3574.1154.camel@localhost.localdomain>

On Dunnersdag 20 Oktober 2005 03:10, Andrew Hendry wrote:
> __u32 or unsigned int look to be the norm for other similar headers,
> whats the recommended type of types to be used?
> 
Use __{u,s}{32,16,8} for interfaces and {u,s}{64,32,16,8} internally,
if you care about the exact size, otherwise use unsigned int.
See also http://lwn.net/Articles/113367/

	Arnd <><

^ permalink raw reply

* Re: [PATCH] X25: Add ITU-T facilites
From: Bernd Jendrissek @ 2005-10-20  7:41 UTC (permalink / raw)
  To: Andrew Hendry
  Cc: linux-os (Dick Johnson), Arnaldo Carvalho de Melo,
	YOSHIFUJI Hideaki / ?$B5HF#1QL@, eis, linux-x25, linux-kernel,
	netdev
In-Reply-To: <1129770654.3574.1154.camel@localhost.localdomain>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Thu, Oct 20, 2005 at 11:10:54AM +1000, Andrew Hendry wrote:
> __u32 or unsigned int look to be the norm for other similar headers,
> whats the recommended type of types to be used?

Dunno if this helps, but AFAIK the C language (as of 1999 or is it 2000)
blesses uint32_t and friends.

Indeed, <linux/types.h> (in the kernel source tree) defines these
*standard* sized types.

HTH

- -- 
A PC without Windows is like ice cream without ketchup.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Please fetch my new key 804177F8 from hkp://wwwkeys.eu.pgp.net/

iD8DBQFDV0ohwyMv24BBd/gRAnOAAKCpTJLsIIQCGW4X/KMyoIMwZ0TNewCff2vE
IO7gmOpr1zuFa515lmIPWno=
=uytG
-----END PGP SIGNATURE-----

^ permalink raw reply


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