Netdev List
 help / color / mirror / Atom feed
* [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: Ravikiran G Thirumalai @ 2005-09-13 16:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, dipankar, bharata, shai, Rusty Russell, netdev,
	davem
In-Reply-To: <20050913155112.GB3570@localhost.localdomain>

Patch to use alloc_percpu for dst_entry.refcount.  This patch reduces the
cacheline bouncing of the atomic_t dst_entry.__refcount.  This Patch gets us
55% better tbench throughput, on a 8way x445 box.

Signed-off by: Pravin B. Shelar <pravins@calsoftinc.com>
Signed-off by: Shobhit Dayal <shobhit@calsoftinc.com>
Signed-off by: Christoph Lameter <christoph@lameter.com>
Signed-off by: Ravikiran Thirumalai <kirant@scalex86.org>

Index: alloc_percpu-2.6.13/include/net/dst.h
===================================================================
--- alloc_percpu-2.6.13.orig/include/net/dst.h	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/include/net/dst.h	2005-09-12 16:44:05.000000000 -0700
@@ -35,11 +35,33 @@
 
 struct sk_buff;
 
+#ifdef CONFIG_NUMA
+
+/*	A per cpu instance of this exist for every dst_entry.
+ *	These are the most written fields of dst_entry.
+ */
+struct per_cpu_cnt
+{
+	int 		refcnt;
+	int 		use;
+	unsigned long	lastuse;
+};
+
+#endif
+
 struct dst_entry
 {
 	struct dst_entry        *next;
+#ifdef CONFIG_NUMA
+	/* first cpu that should be checked for time-out */
+	int 			s_cpu;
+	/* per cpu client references   */
+	struct per_cpu_cnt	*pcc;
+#else
 	atomic_t		__refcnt;	/* client references	*/
 	int			__use;
+	unsigned long		lastuse;
+#endif
 	struct dst_entry	*child;
 	struct net_device       *dev;
 	short			error;
@@ -50,7 +72,6 @@
 #define DST_NOPOLICY		4
 #define DST_NOHASH		8
 #define DST_BALANCED            0x10
-	unsigned long		lastuse;
 	unsigned long		expires;
 
 	unsigned short		header_len;	/* more space at head required */
@@ -103,25 +124,94 @@
 
 #ifdef __KERNEL__
 
+#ifdef CONFIG_NUMA
+
+static inline int dst_use(struct dst_entry *dst)
+{
+	int total = 0, cpu;
+
+	for_each_online_cpu(cpu)
+		total += per_cpu_ptr(dst->pcc, cpu)->use;
+	return total;
+}
+
+#define dst_use_inc(__dst) do {					\
+		per_cpu_ptr((__dst)->pcc, get_cpu())->use++ ;	\
+		put_cpu();					\
+	} while(0);
+
+static inline unsigned long dst_lastuse(struct dst_entry *dst)
+{
+	unsigned long max = 0;
+	int cpu;
+
+	for_each_online_cpu(cpu)
+		if (max < per_cpu_ptr(dst->pcc, cpu)->lastuse)
+			max = per_cpu_ptr(dst->pcc, cpu)->lastuse;
+	return max;
+}
+
+#define dst_lastuse_set(__dst)  do {					  \
+		per_cpu_ptr((__dst)->pcc, get_cpu())->lastuse = jiffies ; \
+		put_cpu();						  \
+	} while(0);
+
+static inline int dst_refcnt(struct dst_entry *dst)
+{
+	int cpu, sum = 0;
+
+	for_each_online_cpu(cpu)
+		sum += per_cpu_ptr(dst->pcc, cpu)->refcnt;
+
+	return sum;
+}
+
+#define dst_refcnt_one(__dst) do { 					  \
+			per_cpu_ptr((__dst)->pcc, get_cpu())->refcnt = 1; \
+			put_cpu();		  			  \
+		} while(0);
+
+#define dst_refcnt_dec(__dst) do { 					\
+			per_cpu_ptr((__dst)->pcc, get_cpu())->refcnt--;	\
+			put_cpu();					\
+		} while(0);
+#define dst_hold(__dst) do { 						 \
+			per_cpu_ptr((__dst)->pcc, get_cpu())->refcnt++ ; \
+			put_cpu();					 \
+		} while(0);
+
+#else
+
 #define dst_use(__dst) (__dst)->__use
 #define dst_use_inc(__dst) (__dst)->__use++
 
 #define dst_lastuse(__dst) (__dst)->lastuse
 #define dst_lastuse_set(__dst) (__dst)->lastuse = jiffies
 
-#define dst_update_tu(__dst) do { dst_lastuse_set(__dst);dst_use_inc(__dst); } while (0)
-#define dst_update_rtu(__dst) do { dst_lastuse_set(__dst);dst_hold(__dst);dst_use_inc(__dst); } while (0)
-
 #define dst_refcnt(__dst) atomic_read(&(__dst)->__refcnt)
 #define dst_refcnt_one(__dst) atomic_set(&(__dst)->__refcnt, 1)
 #define dst_refcnt_dec(__dst) atomic_dec(&(__dst)->__refcnt)
 #define dst_hold(__dst) atomic_inc(&(__dst)->__refcnt)
 
+#endif
+#define dst_update_tu(__dst) do { 		\
+		dst_lastuse_set(__dst);		\
+		dst_use_inc(__dst); 		\
+	} while (0);
+
+#define dst_update_rtu(__dst) do { 		\
+		dst_lastuse_set(__dst);		\
+		dst_hold(__dst);		\
+		dst_use_inc(__dst); 		\
+	} while (0)
+
 static inline
 void dst_release(struct dst_entry * dst)
 {
 	if (dst) {
+#if  (!defined (CONFIG_NUMA) || (RT_CACHE_DEBUG >= 2 ))
 		WARN_ON(dst_refcnt(dst) < 1);
+#endif
 		smp_mb__before_atomic_dec();
 		dst_refcnt_dec(dst);
 	}
@@ -271,6 +361,48 @@
 
 extern void		dst_init(void);
 
+/*	This function allocates and initializes rtu array of given dst-entry.
+ */
+static inline int dst_init_rtu_array(struct dst_entry *dst)
+{
+#ifdef CONFIG_NUMA
+	int cpu;
+	dst->pcc = alloc_percpu(struct per_cpu_cnt, GFP_ATOMIC);
+	if(!dst->pcc)
+		return -ENOMEM;
+
+	for_each_cpu(cpu) {
+		per_cpu_ptr(dst->pcc, cpu)->use = 0;
+		per_cpu_ptr(dst->pcc, cpu)->refcnt = 0;
+		per_cpu_ptr(dst->pcc, cpu)->lastuse = jiffies;
+	}
+	dst->s_cpu = smp_processor_id();
+#else
+	atomic_set(&dst->__refcnt, 0);
+	dst->lastuse = jiffies;
+#endif
+	return 0;
+}
+
+static inline void dst_free_rtu_array(struct dst_entry *dst)
+{
+#ifdef CONFIG_NUMA
+	free_percpu(dst->pcc);
+#endif
+}
+
+#if	defined (CONFIG_HOTPLUG_CPU) && defined (CONFIG_NUMA)
+inline static void dst_ref_xfr_cpu_down(struct dst_entry *__dst, int cpu)
+{
+	int refcnt = per_cpu_ptr((__dst)->pcc, cpu)->refcnt;
+	if (refcnt) {
+		per_cpu_ptr((__dst)->pcc, get_cpu())->refcnt += refcnt;
+		put_cpu();
+		per_cpu_ptr((__dst)->pcc, cpu)->refcnt = 0;
+	}
+}
+#endif
+
 struct flowi;
 #ifndef CONFIG_XFRM
 static inline int xfrm_lookup(struct dst_entry **dst_p, struct flowi *fl,
Index: alloc_percpu-2.6.13/net/bridge/br_netfilter.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/bridge/br_netfilter.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/bridge/br_netfilter.c	2005-09-12 12:24:01.000000000 -0700
@@ -85,7 +85,6 @@
 static struct rtable __fake_rtable = {
 	.u = {
 		.dst = {
-			.__refcnt		= ATOMIC_INIT(1),
 			.dev			= &__fake_net_device,
 			.path			= &__fake_rtable.u.dst,
 			.metrics		= {[RTAX_MTU - 1] = 1500},
@@ -1010,6 +1009,10 @@
 {
 	int i;
 
+	if (dst_init_rtu_array(&__fake_rtable.u.dst) < 0)
+		panic("br_netfilter : cannot allocate memory for dst-entry rtu array");
+	dst_refcnt_one(&__fake_rtable.u.dst);
+
 	for (i = 0; i < ARRAY_SIZE(br_nf_ops); i++) {
 		int ret;
 
@@ -1046,4 +1049,5 @@
 #ifdef CONFIG_SYSCTL
 	unregister_sysctl_table(brnf_sysctl_header);
 #endif
+	dst_free_rtu_array(&__fake_rtable.u.dst);
 }
Index: alloc_percpu-2.6.13/net/core/dst.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/core/dst.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/core/dst.c	2005-09-12 12:24:01.000000000 -0700
@@ -131,9 +131,9 @@
 	if (!dst)
 		return NULL;
 	memset(dst, 0, ops->entry_size);
-	atomic_set(&dst->__refcnt, 0);
+	if (dst_init_rtu_array(dst) < 0)
+		return NULL;
 	dst->ops = ops;
-	dst->lastuse = jiffies;
 	dst->path = dst;
 	dst->input = dst_discard_in;
 	dst->output = dst_discard_out;
@@ -200,6 +200,7 @@
 #if RT_CACHE_DEBUG >= 2 
 	atomic_dec(&dst_total);
 #endif
+	dst_free_rtu_array(dst);
 	kmem_cache_free(dst->ops->kmem_cachep, dst);
 
 	dst = child;
Index: alloc_percpu-2.6.13/net/decnet/dn_route.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/decnet/dn_route.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/decnet/dn_route.c	2005-09-12 12:24:01.000000000 -0700
@@ -77,6 +77,7 @@
 #include <linux/netfilter_decnet.h>
 #include <linux/rcupdate.h>
 #include <linux/times.h>
+#include <linux/cpu.h>
 #include <asm/errno.h>
 #include <net/neighbour.h>
 #include <net/dst.h>
@@ -157,7 +158,29 @@
 
 static inline int dn_dst_useful(struct dn_route *rth, unsigned long now, unsigned long expire)
 {
+#ifdef CONFIG_NUMA
+	{
+		int max, sum = 0, age, cpu;
+		struct dst_entry *dst = &rth->u.dst;
+
+		cpu = dst->s_cpu;
+		max = cpu + NR_CPUS;
+		for(sum = 0; cpu < max; cpu++) {
+			int cpu_ = cpu % NR_CPUS;
+			if (cpu_online(cpu_)) {
+				sum += per_cpu_ptr(dst->pcc, cpu_)->refcnt;
+				age = now - per_cpu_ptr(dst->pcc, cpu_)->lastuse;
+				if (age <= expire) {
+					dst->s_cpu = cpu_ ;
+					return 1;
+				}
+			}
+		}
+		return (sum != 0);
+	}
+#else
 	return  (atomic_read(&rth->u.dst.__refcnt) || (now - rth->u.dst.lastuse) < expire) ;
+#endif
 }
 
 static void dn_dst_check_expire(unsigned long dummy)
@@ -1766,6 +1789,43 @@
 
 #endif /* CONFIG_PROC_FS */
 
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+static int __devinit dn_rtcache_cpu_callback(struct notifier_block *nfb,
+                                   unsigned long action,
+                                   void *hcpu)
+{
+	int cpu = (int) hcpu;
+
+	switch(action) {
+		int i;
+		struct dn_route *rt, *next;
+
+		case CPU_DEAD:
+
+		for(i = 0; i < dn_rt_hash_mask; i++) {
+			spin_lock_bh(&dn_rt_hash_table[i].lock);
+
+			if ((rt = dn_rt_hash_table[i].chain) == NULL)
+				goto nothing_to_do;
+
+			for(; rt; rt=next) {
+				dst_ref_xfr_cpu_down(&rt->u.dst, cpu);
+				next = rt->u.rt_next;
+			}
+nothing_to_do:
+			spin_unlock_bh(&dn_rt_hash_table[i].lock);
+		}
+
+		break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dn_rtcache_cpu_notifier =
+			{ &dn_rtcache_cpu_callback, NULL, 0 };
+
+#endif
+
 void __init dn_route_init(void)
 {
 	int i, goal, order;
@@ -1822,10 +1882,16 @@
         dn_dst_ops.gc_thresh = (dn_rt_hash_mask + 1);
 
 	proc_net_fops_create("decnet_cache", S_IRUGO, &dn_rt_cache_seq_fops);
+#if	defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+	register_cpu_notifier(&dn_rtcache_cpu_notifier);
+#endif
 }
 
 void __exit dn_route_cleanup(void)
 {
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+	unregister_cpu_notifier(&dn_rtcache_cpu_notifier);
+#endif
 	del_timer(&dn_route_timer);
 	dn_run_flush(0);
 
Index: alloc_percpu-2.6.13/net/ipv4/route.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/ipv4/route.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/ipv4/route.c	2005-09-12 12:24:01.000000000 -0700
@@ -92,6 +92,7 @@
 #include <linux/jhash.h>
 #include <linux/rcupdate.h>
 #include <linux/times.h>
+#include <linux/cpu.h>
 #include <net/protocol.h>
 #include <net/ip.h>
 #include <net/route.h>
@@ -507,6 +508,54 @@
 		rth->u.dst.expires;
 }
 
+#ifdef CONFIG_NUMA
+
+/*
+ * For NUMA systems, we do not want to sum up all local cpu refcnts every
+ * time. So we consider lastuse element of the dst_entry and start loop
+ * with the cpu where this entry was allocated. If dst_entry is not timed
+ * out then update s_cpu of this dst_entry so that next time we can start from
+ * that cpu.
+ */
+static inline int rt_check_age(struct rtable *rth,
+			unsigned long tmo1, unsigned long tmo2)
+{
+	int max, sum = 0, age, idx;
+	struct dst_entry *dst = &rth->u.dst;
+	unsigned long now = jiffies;
+
+	idx = dst->s_cpu;
+	max = idx + NR_CPUS;
+	for(sum = 0; idx < max; idx++) {
+		int cpu_ = idx % NR_CPUS;
+		if (cpu_online(cpu_)) {
+			sum += per_cpu_ptr(dst->pcc, cpu_)->refcnt;
+			age = now - per_cpu_ptr(dst->pcc, cpu_)->lastuse;
+			if ((age <= tmo1 && !rt_fast_clean(rth)) ||
+					(age <= tmo2 && rt_valuable(rth))) {
+				dst->s_cpu = cpu_ ;
+				return 0;
+			}
+		}
+	}
+	return (sum == 0);
+}
+
+/*
+ * In this function order of examining three factors (ref_cnt, expires,
+ * lastuse) is changed, considering the cost of analyzing refcnt and lastuse
+ * which are localized for each cpu on NUMA.
+ */
+static int rt_may_expire(struct rtable *rth, unsigned long tmo1, unsigned long tmo2)
+{
+	if (rth->u.dst.expires && time_after_eq(jiffies, rth->u.dst.expires))
+		return (dst_refcnt(&rth->u.dst) == 0) ;
+
+	return rt_check_age(rth, tmo1, tmo2);
+}
+
+#else
+
 static int rt_may_expire(struct rtable *rth, unsigned long tmo1, unsigned long tmo2)
 {
 	unsigned long age;
@@ -529,6 +578,8 @@
 out:	return ret;
 }
 
+#endif
+
 /* Bits of score are:
  * 31: very valuable
  * 30: not quite useless
@@ -1108,8 +1159,19 @@
 
 void ip_rt_copy(struct rtable *to, struct rtable *from)
 {
+#ifdef CONFIG_NUMA
+	struct per_cpu_cnt *tmp_pnc;
+	tmp_pnc = to->u.dst.pcc;
+
+	*to = *from;
+	to->u.dst.pcc = tmp_pnc;
+	per_cpu_ptr(to->u.dst.pcc,get_cpu())->use = 1;
+	to->u.dst.s_cpu = smp_processor_id();
+	put_cpu();
+#else
 	*to = *from;
 	to->u.dst.__use 	= 1;
+#endif
 }
 
 void ip_rt_redirect(u32 old_gw, u32 daddr, u32 new_gw,
@@ -3108,6 +3170,33 @@
 }
 __setup("rhash_entries=", set_rhash_entries);
 
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+static int __devinit rtcache_cpu_callback(struct notifier_block *nfb,
+                                   unsigned long action,
+				   void *hcpu)
+{
+	int cpu = (int) hcpu;
+
+	switch(action) {
+		int i ;
+		struct rtable *rth;
+		case CPU_DEAD:
+			for(i = rt_hash_mask; i >= 0; i--) {
+				spin_lock_irq(rt_hash_lock_addr(i));
+				rth = rt_hash_table[i].chain;
+				while(rth) {
+					dst_ref_xfr_cpu_down(&rth->u.dst, cpu);
+					rth = rth->u.rt_next;
+				}
+				spin_unlock_irq(rt_hash_lock_addr(i));
+			}
+			break;
+	}
+	return NOTIFY_OK;
+}
+static struct notifier_block rtcache_cpu_notifier = { &rtcache_cpu_callback, NULL, 0 };
+#endif
+
 int __init ip_rt_init(void)
 {
 	int rc = 0;
@@ -3197,6 +3286,9 @@
 	xfrm_init();
 	xfrm4_init();
 #endif
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+	register_cpu_notifier(&rtcache_cpu_notifier);
+#endif
 	return rc;
 }
 
Index: alloc_percpu-2.6.13/net/ipv6/ip6_fib.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/ipv6/ip6_fib.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/ipv6/ip6_fib.c	2005-09-12 12:24:01.000000000 -0700
@@ -1209,6 +1209,35 @@
 	spin_unlock_bh(&fib6_gc_lock);
 }
 
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+#include <linux/cpu.h>
+inline static int rt6_ref_xfr_cpu_down(struct rt6_info *rt, void *arg)
+{
+	dst_ref_xfr_cpu_down(&rt->u.dst, (int)arg);
+	return 0;
+}
+
+static int __devinit ipv6_rtcache_cpu_callback(struct notifier_block *nfb,
+                                   unsigned long action,
+                                   void *hcpu)
+{
+	int cpu = (int) hcpu;
+
+	switch(action) {
+		case CPU_DEAD:
+			write_lock_bh(&rt6_lock);
+			fib6_clean_tree(&ip6_routing_table, rt6_ref_xfr_cpu_down,
+					0, (void *)cpu);
+			write_unlock_bh(&rt6_lock);
+			break;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block ipv6_rtcache_cpu_notifier =
+				{ &ipv6_rtcache_cpu_callback, NULL, 0 };
+#endif
+
 void __init fib6_init(void)
 {
 	fib6_node_kmem = kmem_cache_create("fib6_nodes",
@@ -1217,10 +1246,16 @@
 					   NULL, NULL);
 	if (!fib6_node_kmem)
 		panic("cannot create fib6_nodes cache");
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+	register_cpu_notifier(&ipv6_rtcache_cpu_notifier);
+#endif
 }
 
 void fib6_gc_cleanup(void)
 {
+#if defined(CONFIG_NUMA) && defined(CONFIG_HOTPLUG_CPU)
+	unregister_cpu_notifier(&ipv6_rtcache_cpu_notifier);
+#endif
 	del_timer(&ip6_fib_timer);
 	kmem_cache_destroy(fib6_node_kmem);
 }
Index: alloc_percpu-2.6.13/net/ipv6/route.c
===================================================================
--- alloc_percpu-2.6.13.orig/net/ipv6/route.c	2005-09-12 12:23:37.000000000 -0700
+++ alloc_percpu-2.6.13/net/ipv6/route.c	2005-09-12 12:24:01.000000000 -0700
@@ -110,8 +110,6 @@
 struct rt6_info ip6_null_entry = {
 	.u = {
 		.dst = {
-			.__refcnt	= ATOMIC_INIT(1),
-			.__use		= 1,
 			.dev		= &loopback_dev,
 			.obsolete	= -1,
 			.error		= -ENETUNREACH,
@@ -2104,6 +2102,10 @@
 						     NULL, NULL);
 	if (!ip6_dst_ops.kmem_cachep)
 		panic("cannot create ip6_dst_cache");
+	if (dst_init_rtu_array(&ip6_null_entry.u.dst) < 0)
+		panic("ip6_route : can't allocate memory for dst-entry array");
+	dst_use_inc(&ipv6_null_entry.u.dist);
+	dst_refcnt_one(&ip6_null_entry.u.dst);
 
 	fib6_init();
 #ifdef 	CONFIG_PROC_FS
@@ -2130,4 +2132,5 @@
 	rt6_ifdown(NULL);
 	fib6_gc_cleanup();
 	kmem_cache_destroy(ip6_dst_ops.kmem_cachep);
+	dst_free_rtu_array(&ip6_null_entry.u.dst);
 }

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Stephen Hemminger @ 2005-09-13 16:26 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: Andrew Morton, linux-kernel, dipankar, bharata, shai,
	Rusty Russell, netdev, davem
In-Reply-To: <20050913161012.GI3570@localhost.localdomain>

On Tue, 13 Sep 2005 09:10:12 -0700
Ravikiran G Thirumalai <kiran@scalex86.org> wrote:

> The net_device has a refcnt used to keep track of it's uses.
> This is used at the time of unregistering the network device
> (module unloading ..) (see netdev_wait_allrefs) .
> For loopback_dev , this refcnt increment/decrement  is causing
> unnecessary traffic on the interlink for NUMA system
> affecting it's performance.  This patch improves tbench numbers by 6% on a
> 8way x86 Xeon (x445).
> 

Since when is bringing a network device up/down performance critical?

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Ben Greear @ 2005-09-13 16:35 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Ravikiran G Thirumalai, Andrew Morton, linux-kernel, dipankar,
	bharata, shai, Rusty Russell, netdev, davem
In-Reply-To: <20050913092659.791bddec@localhost.localdomain>

Stephen Hemminger wrote:
> On Tue, 13 Sep 2005 09:10:12 -0700
> Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
> 
> 
>>The net_device has a refcnt used to keep track of it's uses.
>>This is used at the time of unregistering the network device
>>(module unloading ..) (see netdev_wait_allrefs) .
>>For loopback_dev , this refcnt increment/decrement  is causing
>>unnecessary traffic on the interlink for NUMA system
>>affecting it's performance.  This patch improves tbench numbers by 6% on a
>>8way x86 Xeon (x445).
>>
> 
> 
> Since when is bringing a network device up/down performance critical?

We grab and drop a reference for each poll of a device, roughly.

See dev_hold in _netif_rx_schedule(struct net_device *dev)
in include/netdevice.h, for instance.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Stephen Hemminger @ 2005-09-13 16:46 UTC (permalink / raw)
  To: Ben Greear
  Cc: Ravikiran G Thirumalai, Andrew Morton, linux-kernel, dipankar,
	bharata, shai, Rusty Russell, netdev, davem
In-Reply-To: <4326FFC2.7030803@candelatech.com>

On Tue, 13 Sep 2005 09:35:14 -0700
Ben Greear <greearb@candelatech.com> wrote:

> Stephen Hemminger wrote:
> > On Tue, 13 Sep 2005 09:10:12 -0700
> > Ravikiran G Thirumalai <kiran@scalex86.org> wrote:
> > 
> > 
> >>The net_device has a refcnt used to keep track of it's uses.
> >>This is used at the time of unregistering the network device
> >>(module unloading ..) (see netdev_wait_allrefs) .
> >>For loopback_dev , this refcnt increment/decrement  is causing
> >>unnecessary traffic on the interlink for NUMA system
> >>affecting it's performance.  This patch improves tbench numbers by 6% on a
> >>8way x86 Xeon (x445).
> >>
> > 
> > 
> > Since when is bringing a network device up/down performance critical?
> 
> We grab and drop a reference for each poll of a device, roughly.
> 
> See dev_hold in _netif_rx_schedule(struct net_device *dev)
> in include/netdevice.h, for instance.

Yeah, that would be an issue, especially since the rest of that
path is nicely per-cpu

^ permalink raw reply

* Re: masquerading failure for at least icmp and tcp+sack on amd64
From: Stephen Hemminger @ 2005-09-13 18:09 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Andrew Morton, netdev, Netfilter Development Mailinglist,
	Marc Lehmann
In-Reply-To: <43243AB9.9000705@trash.net>

On Sun, 11 Sep 2005 16:10:01 +0200
Patrick McHardy <kaber@trash.net> wrote:

> Marc Lehmann wrote:
> > On Fri, Sep 09, 2005 at 01:41:34PM +0200, Patrick McHardy <kaber@trash.net> wrote:
> > 
> >>What network driver are you using?
> > 
> > Happens with both skge and sk98.
> 
> Are you sure the same checksum error happens? sk98_lin never sets
> ip_summed to CHECKSUM_HW, it uses either CHECKSUM_UNNECESSARY for
> HW checksummed packets, in which case the check is skipped in
> ip_conntrack, or it uses CHECKSUM_NONE, in which case the checksum
> in the packet must be invalid if the check fails and the packet
> wouldn't be accepted by the final receipient anyway. skge uses 
> CHECKSUM_HW for every packet, so they have nothing in common wrt.
> HW checksumming. This would normally mean we can rule out HW
> checksumming, but for some reason turning it off on skge seems
> to help in your case. Stephen, you're more familiar with the
> sk* drivers than me, anything I'm missing here?
> 

Some background, the semantic of ip_summed is different on the
output than the input path. On input, it means a checksum is
available in skb->csum; and on output it means the packet is
destined for a device that can do hardware checksumming.

I have gotten reports of receive checksum errors on
some systems, it may be related to certain revisions of
hardware. It would be useful to see the message printed out
by the skge driver that shows chip and revision.

Also, on the input path for TCP and UDP, the code does not
depend on the hardware being correct, and if the checksum
is incorrect, it just prints a warning and does a software
checksum before deciding to drop.
Perhaps netfilter code needs to handle that case?

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Eric Dumazet @ 2005-09-13 18:27 UTC (permalink / raw)
  To: Ravikiran G Thirumalai
  Cc: Andrew Morton, linux-kernel, dipankar, bharata, shai,
	Rusty Russell, netdev, davem
In-Reply-To: <20050913161012.GI3570@localhost.localdomain>

Ravikiran G Thirumalai a écrit :
> The net_device has a refcnt used to keep track of it's uses.
> This is used at the time of unregistering the network device
> (module unloading ..) (see netdev_wait_allrefs) .
> For loopback_dev , this refcnt increment/decrement  is causing
> unnecessary traffic on the interlink for NUMA system
> affecting it's performance.  This patch improves tbench numbers by 6% on a
> 8way x86 Xeon (x445).
  ===================================================================
> --- alloc_percpu-2.6.13.orig/include/linux/netdevice.h	2005-08-28 16:41:01.000000000 -0700
> +++ alloc_percpu-2.6.13/include/linux/netdevice.h	2005-09-12 11:54:21.000000000 -0700
> @@ -37,6 +37,7 @@
>  #include <linux/config.h>
>  #include <linux/device.h>
>  #include <linux/percpu.h>
> +#include <linux/bigref.h>
>  
>  struct divert_blk;
>  struct vlan_group;
> @@ -377,7 +378,7 @@
>  	/* device queue lock */
>  	spinlock_t		queue_lock;
>  	/* Number of references to this device */
> -	atomic_t		refcnt;
> +	struct bigref	        netdev_refcnt;	
>  	/* delayed register/unregister */
>  	struct list_head	todo_list;
>  	/* device name hash chain */
> @@ -677,11 +678,11 @@

Hum...

Did you tried to place refcnt/netdev_refcnt in a separate cache line than 
queue_lock ? I got good results too...

 >  	/* device queue lock */
 >  	spinlock_t		queue_lock;
 >  	/* Number of references to this device */
 > -	atomic_t		refcnt;
 > +	struct bigref	        netdev_refcnt ____cacheline_aligned_in_smp ;	
 >  	/* delayed register/unregister */
 >  	struct list_head	todo_list;
 >  	/* device name hash chain */

Every time a cpu take the queue_lock spinlock, it exclusively gets one cache 
line. If another cpu try to access netdev_refcnt, it has to grab this cache 
line (even if properely per_cpu designed, there is still one shared field). In 
fact the whole struct net_device should be re-ordered for SMP/NUMA performance.

Eric

^ permalink raw reply

* [RFC] hippi: change to not use skb private
From: Stephen Hemminger @ 2005-09-13 18:38 UTC (permalink / raw)
  To: David S. Miller, jes; +Cc: netdev, linux-kernel

It looks like the following would fix hippi to not have to put
fields in sk_buff. The ifield looks appears to to be additional
header information that is being passed in the skb but could just
be put in the header.

Does anyone still have the hardware to test this? Is anyone
even using it on 2.6?

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>


Index: hippi-2.6.13/drivers/net/rrunner.c
===================================================================
--- hippi-2.6.13.orig/drivers/net/rrunner.c
+++ hippi-2.6.13/drivers/net/rrunner.c
@@ -1432,7 +1432,6 @@ static int rr_start_xmit(struct sk_buff 
 	struct ring_ctrl *txctrl;
 	unsigned long flags;
 	u32 index, len = skb->len;
-	u32 *ifield;
 	struct sk_buff *new_skb;
 
 	if (readl(&regs->Mode) & FATAL_ERR)
@@ -1440,29 +1439,6 @@ static int rr_start_xmit(struct sk_buff 
 		       readl(&regs->Fail1), readl(&regs->Fail2));
 
 	/*
-	 * We probably need to deal with tbusy here to prevent overruns.
-	 */
-
-	if (skb_headroom(skb) < 8){
-		printk("incoming skb too small - reallocating\n");
-		if (!(new_skb = dev_alloc_skb(len + 8))) {
-			dev_kfree_skb(skb);
-			netif_wake_queue(dev);
-			return -EBUSY;
-		}
-		skb_reserve(new_skb, 8);
-		skb_put(new_skb, len);
-		memcpy(new_skb->data, skb->data, len);
-		dev_kfree_skb(skb);
-		skb = new_skb;
-	}
-
-	ifield = (u32 *)skb_push(skb, 8);
-
-	ifield[0] = 0;
-	ifield[1] = skb->private.ifield;
-
-	/*
 	 * We don't need the lock before we are actually going to start
 	 * fiddling with the control blocks.
 	 */
@@ -1475,7 +1451,7 @@ static int rr_start_xmit(struct sk_buff 
 	rrpriv->tx_skbuff[index] = skb;
 	set_rraddr(&rrpriv->tx_ring[index].addr, pci_map_single(
 		rrpriv->pci_dev, skb->data, len + 8, PCI_DMA_TODEVICE));
-	rrpriv->tx_ring[index].size = len + 8; /* include IFIELD */
+	rrpriv->tx_ring[index].size = len;
 	rrpriv->tx_ring[index].mode = PACKET_START | PACKET_END;
 	txctrl->pi = (index + 1) % TX_RING_ENTRIES;
 	wmb();
Index: hippi-2.6.13/include/linux/skbuff.h
===================================================================
--- hippi-2.6.13.orig/include/linux/skbuff.h
+++ hippi-2.6.13/include/linux/skbuff.h
@@ -194,7 +194,6 @@ struct skb_shared_info {
  *	@nfct: Associated connection, if any
  *	@nfctinfo: Relationship of this skb to the connection
  *	@nf_bridge: Saved data about a bridged frame - see br_netfilter.c
- *      @private: Data which is private to the HIPPI implementation
  *	@tc_index: Traffic control index
  *	@tc_verd: traffic control verdict
  *	@tc_classid: traffic control classid
@@ -267,11 +266,7 @@ struct sk_buff {
 	struct nf_bridge_info	*nf_bridge;
 #endif
 #endif /* CONFIG_NETFILTER */
-#if defined(CONFIG_HIPPI)
-	union {
-		__u32		ifield;
-	} private;
-#endif
+
 #ifdef CONFIG_NET_SCHED
        __u32			tc_index;        /* traffic control index */
 #ifdef CONFIG_NET_CLS_ACT
Index: hippi-2.6.13/net/802/hippi.c
===================================================================
--- hippi-2.6.13.orig/net/802/hippi.c
+++ hippi-2.6.13/net/802/hippi.c
@@ -50,7 +50,8 @@ static int hippi_header(struct sk_buff *
 			unsigned short type, void *daddr, void *saddr,
 			unsigned len)
 {
-	struct hippi_hdr *hip = (struct hippi_hdr *)skb_push(skb, HIPPI_HLEN);
+	struct hippi_hdr *hip = (struct hippi_hdr *)skb_push(skb, HIPPI_HLEN + 8);
+	u32 *ifield = (u32 *) (hip + 1);
 
 	if (!len){
 		len = skb->len - HIPPI_HLEN;
@@ -81,13 +82,14 @@ static int hippi_header(struct sk_buff *
 	hip->snap.oui[2]	= 0x00;
 	hip->snap.ethertype	= htons(type);
 
-	if (daddr)
-	{
+	memset(ifield, 0, 2*sizeof(u32));
+	if (daddr) {
 		memcpy(hip->le.dest_switch_addr, daddr + 3, 3);
-		memcpy(&skb->private.ifield, daddr + 2, 4);
-		return HIPPI_HLEN;
+		memcpy(ifield+1, daddr + 2, 4);
+		return HIPPI_HLEN + 8;
 	}
-	return -((int)HIPPI_HLEN);
+
+	return -((int)HIPPI_HLEN - 8);
 }
 
 
@@ -200,7 +202,7 @@ static void hippi_setup(struct net_devic
 	 * still need a fake ARPHRD to make ifconfig and friends play ball.
 	 */
 	dev->type		= ARPHRD_HIPPI;
-	dev->hard_header_len 	= HIPPI_HLEN;
+	dev->hard_header_len 	= HIPPI_HLEN + 8;
 	dev->mtu		= 65280;
 	dev->addr_len		= HIPPI_ALEN;
 	dev->tx_queue_len	= 25 /* 5 */;
Index: hippi-2.6.13/net/core/skbuff.c
===================================================================
--- hippi-2.6.13.orig/net/core/skbuff.c
+++ hippi-2.6.13/net/core/skbuff.c
@@ -370,9 +370,6 @@ struct sk_buff *skb_clone(struct sk_buff
 	nf_bridge_get(skb->nf_bridge);
 #endif
 #endif /*CONFIG_NETFILTER*/
-#if defined(CONFIG_HIPPI)
-	C(private);
-#endif
 #ifdef CONFIG_NET_SCHED
 	C(tc_index);
 #ifdef CONFIG_NET_CLS_ACT

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Ravikiran G Thirumalai @ 2005-09-13 18:53 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Andrew Morton, linux-kernel, dipankar, bharata, shai,
	Rusty Russell, netdev, davem
In-Reply-To: <43271A28.9090301@cosmosbay.com>

On Tue, Sep 13, 2005 at 08:27:52PM +0200, Eric Dumazet wrote:
> Ravikiran G Thirumalai a écrit :
> 
> Hum...
> 
> Did you tried to place refcnt/netdev_refcnt in a separate cache line than 
> queue_lock ? I got good results too...
> 
> >  	/* device queue lock */
> >  	spinlock_t		queue_lock;
> >  	/* Number of references to this device */
> > -	atomic_t		refcnt;
> > +	struct bigref	        netdev_refcnt ____cacheline_aligned_in_smp ; 
> >  	/* delayed register/unregister */
> >  	struct list_head	todo_list;
> >  	/* device name hash chain */
> 
> Every time a cpu take the queue_lock spinlock, it exclusively gets one 
> cache line. If another cpu try to access netdev_refcnt, it has to grab this 
> cache line (even if properely per_cpu designed, there is still one shared 
> field). In fact the whole struct net_device should be re-ordered for 
> SMP/NUMA performance.

I agree. Maybe placing the queue_lock in a different cacheline is the 
right approach?

Thanks,
Kiran

^ permalink raw reply

* Re: [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: David S. Miller @ 2005-09-13 20:24 UTC (permalink / raw)
  To: kiran; +Cc: akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913161708.GK3570@localhost.localdomain>


There is no way in the world this enormous amount of NUMA
complexity is being added to the destination cache layer.

Sorry.

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: David S. Miller @ 2005-09-13 20:26 UTC (permalink / raw)
  To: shemminger
  Cc: kiran, akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913092659.791bddec@localhost.localdomain>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Tue, 13 Sep 2005 09:26:59 -0700

> Since when is bringing a network device up/down performance critical?

The issue is the dev_get()'s that occur all over the place
to during packet transmit/receive, that's what they are
trying to address.

I'm still against all of these invasive NUMA changes to the
networking though, they are simply too ugly and special cased
to consider seriously.

^ permalink raw reply

* Re: [RFC] hippi: change to not use skb private
From: David S. Miller @ 2005-09-13 20:54 UTC (permalink / raw)
  To: shemminger; +Cc: jes, netdev, linux-kernel
In-Reply-To: <20050913113858.440d3a0f@localhost.localdomain>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Tue, 13 Sep 2005 11:38:58 -0700

> It looks like the following would fix hippi to not have to put
> fields in sk_buff. The ifield looks appears to to be additional
> header information that is being passed in the skb but could just
> be put in the header.

I can't see any problem with this patch, and it fixes
all the realloc stuff that has to potentially occur
in the transmit path.

^ permalink raw reply

* Re: masquerading failure for at least icmp and tcp+sack on amd64
From: David S. Miller @ 2005-09-13 20:59 UTC (permalink / raw)
  To: shemminger; +Cc: akpm, netdev, netfilter-devel, kaber, schmorp
In-Reply-To: <20050913110902.0ad58b90@localhost.localdomain>

From: Stephen Hemminger <shemminger@osdl.org>
Date: Tue, 13 Sep 2005 11:09:02 -0700

> Also, on the input path for TCP and UDP, the code does not
> depend on the hardware being correct, and if the checksum
> is incorrect, it just prints a warning and does a software
> checksum before deciding to drop.
> Perhaps netfilter code needs to handle that case?

I personally think netfilter should do so.

^ permalink raw reply

* Re: [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: Ravikiran G Thirumalai @ 2005-09-13 22:07 UTC (permalink / raw)
  To: David S. Miller
  Cc: akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913.132442.53540386.davem@davemloft.net>

On Tue, Sep 13, 2005 at 01:24:42PM -0700, David S. Miller wrote:
> 
> There is no way in the world this enormous amount of NUMA
> complexity is being added to the destination cache layer.

Agreed the dst changes are ugly; that can be worked on.  But the 
cacheline bouncing problem on the atomic_t dst_entry refcounter has been 
around for quite a while -- even on SMPs, not just NUMA.  We need a solution 
for that.  I thought you were against the dst_entry bloat caused by the 
previous version of the dst patch.  alloc_percpu takes that away.  You had
concerns about workloads with low route locality. Unfortunately we don't have
access to infrastructure setup for such tests :( 

As for the ugliness, would something on the lines of net_device refcounter 
patch in the series above be acceptable?

Thanks,
Kiran

^ permalink raw reply

* Re: [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: David S. Miller @ 2005-09-13 22:12 UTC (permalink / raw)
  To: kiran; +Cc: akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913220737.GA6249@localhost.localdomain>

From: Ravikiran G Thirumalai <kiran@scalex86.org>
Date: Tue, 13 Sep 2005 15:07:37 -0700

> Agreed the dst changes are ugly; that can be worked on.  But the
> cacheline bouncing problem on the atomic_t dst_entry refcounter has
> been around for quite a while -- even on SMPs, not just NUMA.  We
> need a solution for that.  I thought you were against the dst_entry
> bloat caused by the previous version of the dst patch.  alloc_percpu
> takes that away.  You had concerns about workloads with low route
> locality. Unfortunately we don't have access to infrastructure setup
> for such tests :(

You don't have two computers connected on a network?

All you need is that, load a bunch of routes into one system that
point to an IP address which you just force an ARP entry for (so it
just gets lost in the ether) and then generate a rDOS workload through
it from another machine using pktgen.

I'm fine with funny per-cpu memory allocation strategies, perhaps
(would have to see a patch doing _only_ that to be sure).

But using bigrefs, no way.  We have enough trouble making the data
structures small without adding bloat like that.  A busy server can
have hundreds of thousands of dst cache entries active on it, and they
chew up enough memory as is.

^ permalink raw reply

* Re: Route cache performance
From: Simon Kirby @ 2005-09-13 22:14 UTC (permalink / raw)
  To: Alexey Kuznetsov, Robert Olsson, Eric Dumazet, netdev
In-Reply-To: <20050907195911.GA8382@yakov.inr.ac.ru>

On Wed, Sep 07, 2005 at 11:59:11PM +0400, Alexey Kuznetsov wrote:

> Hello!
> 
> > Yes, setting maxbatch to 10000 also results in working gc,
> 
> Could you try lower values? F.e. I guess 300 or a little more
> (it is netdev_max_backlog) should be enough.

300 seems to be sufficient, but I'm not sure what this depends on (load,
HZ, timing of some sort?).  See below for full tests.

> > for the normal case also hurts the DoS case...and it really hurts when
> > the when the DoS case is the normal case.
> 
> 5.7% is not "really hurts" yet. :-)

I decided to try out FreeBSD in comparison as I've heard people saying
that it handles this case quite well.  The results are interesting.

FreeBSD seems to have a route cache; however, it keys only on
destination.  When a new destination is seen, the route table entry that
matched is "cloned" so that the MTU, etc., is copied, the dst rewritten
to the exact IP (as opposed to a network route), and path MTU discovery
results are maintained in this entry, keyed by destination address only.

I'm not sure if Linux could work in the same way with the source routing
tables enabled, but perhaps it's possible to either disable the source
side of the route cache when policy routing is disabled.  Or perhaps a
route cache hash could be instantiated per route table or something.

Actually, is there ever a valid case where the source needs to be tracked
in the route cache when policy routing is disabled?  A local socket will
track MSS correctly while a forwarded packet will create or use an entry
without touching it, so I don't see why not.

Anyway, spoofed source or not go the same speed through FreeBSD.  Also,
there is a "fastforwarding" sysctl that sends forwarded packets from
the input interrupt/poll without queueing them in a soft interrupt
("NETISR").

Polling mode on FreeBSD isn't as nice as NAPI in that it's fully manual
on or off, and when it's on it triggers entirely from the timer interrupt
unless told to also trigger from the idle loop.  The user/kernel balancing
is also manual but I can't seem to get it to forward as fast as with it
disabled no matter how I adjust it.


TEST RESULTS
------------

All Linux tests with NAPI enabled and the e1000 driver native to that
kernel unless otherwise specified.  maxbatch does not exist in kernels
< 2.6.9, and rhash_size does not exist in 2.4.


Sender: 367 Mbps, 717883 pps valid src/dst, 64 byte (Ethernet) packets

2.4.27-rc1: 297 Mbps forwarded (w/idle time?!)
2.4.31: 296 Mbps forwarded (w/idle time?!)
2.6.13-rc6: 173 Mbps forwarded
FreeBSD 5.4-RELEASE (HZ=1000): 103 Mbps forwarded (dead userland)
`- net.inet.ip.fastforwarding=1: 282 Mbps forwarded (dead userland)
   `- kern.polling.enable=1: 75.3 Mbps forwarded
      `- kern.polling.idle_poll=1: 226 Mbps forwarded


Sender: 348 Mbps, 680416 pps random src, valid dst, 64 bytes

(All FreeBSD tests have identical results.)

2.4.27-rc1: 122 Mbps forwarded
2.4.27-rc1 gc_elasticity=1: 182 Mbps forwarded
2.4.27-rc1+2.4.31_e1000: 117 Mbps forwarded
2.4.27-rc1+2.4.31_e1000 gc_elasticity=1: 170 Mbps forwarded
2.4.31: 95.1 Mbps forwarded
2.4.31 gc_elasticity=1: 122 Mbps forwarded

2.6.13-rc6: <1 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=30: <1 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=60: 1.5 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=100: 2.6 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=150: 3.8 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=200: 6.9 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=250: 15.4 Mbps forwarded (dst overflow)
2.6.13-rc6 maxbatch=300: 58.6 Mbps forwarded (gc balanced)
2.6.13-rc6 maxbatch=350: 60.5 Mbps forwarded
2.6.13-rc6 maxbatch=400: 59.4 Mbps forwarded
2.6.13-rc6 maxbatch=450: 59.1 Mbps forwarded
2.6.13-rc6 maxbatch=500: 62.0 Mbps forwarded
2.6.13-rc6 maxbatch=550: 61.9 Mbps forwarded
2.6.13-rc6 maxbatch=1000: 61.4 Mbps forwarded
2.6.13-rc6 maxbatch=2000: 60.2 Mbps forwarded
2.6.13-rc6 maxbatch=3000: 60.1 Mbps forwarded
2.6.13-rc6 maxbatch=5000: 59.1 Mbps forwarded
2.6.13-rc6 maxbatch=MAXINT: 59.1 Mbps forwarded
2.6.13-rc6 dst_free: 66.0 Mbps forwarded
2.6.13-rc6 dst_free max_size=rhash_size: 79.2 Mbps forwarded

------------

2.6 definitely has better dst cache gc balancing than 2.4.  I can set
the max_size=rhash_size in 2.6.13-rc6 and it will just work, even without
adjusting gc_elasticity or gc_thresh.  In 2.4.27 and 2.4.31, the only
parameter that appears to help is gc_elasticity.  If I just adjust
max_size, it overflows and falls over.

I note that the actual read copy update "maxbatch" limit was added in
2.6.9.  Before then, it seems there was no limit (infinite).  Was it
added for latency reasons?

Time permitting, I'd also like to run some profiles.  It's interesting
to note that 2.6 is slower at forwarding even straight duplicate small
packets.  We should definitely get to the bottom of that.

Simon-

^ permalink raw reply

* Re: [patch 7/11] net: Use bigrefs for net_device.refcount
From: Ravikiran G Thirumalai @ 2005-09-13 22:16 UTC (permalink / raw)
  To: David S. Miller
  Cc: shemminger, akpm, linux-kernel, dipankar, bharata, shai, rusty,
	netdev
In-Reply-To: <20050913.132607.113443001.davem@davemloft.net>

On Tue, Sep 13, 2005 at 01:26:07PM -0700, David S. Miller wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Tue, 13 Sep 2005 09:26:59 -0700
> 
> > Since when is bringing a network device up/down performance critical?
> 
> The issue is the dev_get()'s that occur all over the place
> to during packet transmit/receive, that's what they are
> trying to address.
> 
> I'm still against all of these invasive NUMA changes to the
> networking though, they are simply too ugly and special cased
> to consider seriously.

All of them or the dst ones?  Hopefully the netdevice refcounter patch
is not ugly or complicated as the dst ones? And why are they special cased?
Are networking workloads with high route locality not interesting?

Thanks,
Kiran

^ permalink raw reply

* Re: [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: Ravikiran G Thirumalai @ 2005-09-13 23:17 UTC (permalink / raw)
  To: David S. Miller
  Cc: akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913.151216.48124942.davem@davemloft.net>

On Tue, Sep 13, 2005 at 03:12:16PM -0700, David S. Miller wrote:
> From: Ravikiran G Thirumalai <kiran@scalex86.org>
> Date: Tue, 13 Sep 2005 15:07:37 -0700
> ...
> But using bigrefs, no way.  We have enough trouble making the data
> structures small without adding bloat like that.  A busy server can
> have hundreds of thousands of dst cache entries active on it, and they
> chew up enough memory as is.
> 

But even 1 Million dst cache entries would be 16+4 MB additional for a 4 cpu 
box....is that too much?  The alloc_percpu reimplementation interleaves
objects on cache lines, unlike the existing implementation which pads per-cpu
objects to cache lines...

If you are referring to embedded routing devices,
would they use CONFIG_NUMA or CONFIG_SMP?? (bigrefs nicely fold back to
regular atomic_t s on UPs)

Thanks,
Kiran

^ permalink raw reply

* Re: [patch 9/11] net: dst_entry.refcount, use, lastuse to use alloc_percpu
From: David S. Miller @ 2005-09-13 23:27 UTC (permalink / raw)
  To: kiran; +Cc: akpm, linux-kernel, dipankar, bharata, shai, rusty, netdev
In-Reply-To: <20050913231717.GC6249@localhost.localdomain>

From: Ravikiran G Thirumalai <kiran@scalex86.org>
Date: Tue, 13 Sep 2005 16:17:17 -0700

> But even 1 Million dst cache entries would be 16+4 MB additional for
> a 4 cpu box....is that too much?

Absolutely.

Per-cpu counters are great for things like single instance
statistics et al.  But once you start doing them per-object
that's out of control bloat as far as I'm concerned.

^ permalink raw reply

* Re: [PATCH 15/29] ieee80211 Renamed ieee80211_hdr to ieee80211_hdr_4addr
From: Michael Wu @ 2005-09-13 23:45 UTC (permalink / raw)
  To: netdev; +Cc: ieee80211-devel, James Ketrenos
In-Reply-To: <43275856.7010305@linux.intel.com>

This patch plus patch 5 looks very similar to my management frame patch 
except..

On Tuesday 13 September 2005 18:53, James Ketrenos wrote:
> @@ -552,6 +577,17 @@ struct ieee80211_authentication {
>  	struct ieee80211_info_element info_element[0];
>  } __attribute__ ((packed));
>
> +struct ieee80211_disassoc {
Add alias for deauthentication frame here?

> +	struct ieee80211_hdr_3addr header;
> +	u16 reason_code;
If we do not append _code to "status", why append _code to "reason"?

> +	struct ieee80211_info_element info_element[0];
> +} __attribute__ ((packed));
> +
> +struct ieee80211_probe_request {
> +	struct ieee80211_hdr_3addr header;
> +	struct ieee80211_info_element info_element[0];
> +} __attribute__ ((packed));
> +
>  struct ieee80211_probe_response {
>  	struct ieee80211_hdr_3addr header;
>  	u32 time_stamp[2];
> @@ -560,14 +596,25 @@ struct ieee80211_probe_response {
>  	struct ieee80211_info_element info_element[0];
>  } __attribute__ ((packed));
>
> -struct ieee80211_assoc_request_frame {
> +/* Alias beacon for probe_response */
> +#define ieee80211_beacon ieee80211_probe_response
> +
> +struct ieee80211_assoc_request {
> +	struct ieee80211_hdr_3addr header;
> +	u16 capability;
> +	u16 listen_interval;
> +	struct ieee80211_info_element info_element[0];
> +} __attribute__ ((packed));
> +
> +struct ieee80211_reassoc_request {
> +	struct ieee80211_hdr_3addr header;
>  	__le16 capability;
>  	__le16 listen_interval;
>  	u8 current_ap[ETH_ALEN];
>  	struct ieee80211_info_element info_element[0];
>  } __attribute__ ((packed));
>
> -struct ieee80211_assoc_response_frame {
> +struct ieee80211_assoc_response {
>  	struct ieee80211_hdr_3addr header;
>  	__le16 capability;
>  	__le16 status;

^ permalink raw reply

* Re: [PATCH 25/29] ieee80211 use endian-aware types
From: Michael Wu @ 2005-09-13 23:48 UTC (permalink / raw)
  To: netdev; +Cc: James Ketrenos, ieee80211-devel
In-Reply-To: <43276290.2010809@linux.intel.com>

On Tuesday 13 September 2005 19:36, James Ketrenos wrote:
> ieee80211: use endian-aware types
>
> From: Michael Wu <flamingice@sourmilk.net>
>
> This patch:
> - fixes misc. whitespace/comments
> - replaces u16 with __le16/__be16 where appropriate
>
> Signed-off-by: Michael Wu <flamingice@sourmilk.net>
> Signed-off-by: Jiri Benc <jbenc@suse.cz>
>
> Signed-off-by: James Ketrenos <jketreno@linux.intel.com>
>
The original patch already went in, and this patch looks nothing like the one 
I sent in.

-Michael Wu

^ permalink raw reply

* Re: [PATCH 23/29] ieee80211 Added ieee80211_radiotap.h
From: Michael Wu @ 2005-09-13 23:50 UTC (permalink / raw)
  To: netdev
In-Reply-To: <43276231.1000908@linux.intel.com>

On Tuesday 13 September 2005 19:35, James Ketrenos wrote:
> Added ieee80211_radiotap.h to enhance statistic reporting to user space
> from wireless drivers.
>
> Signed-off-by: James Ketrenos <jketreno@linux.intel.com>
I'm not sure, but didn't Mike Kershaw create this patch? I was expecting a 
signed-off-by line by him.

-Michael Wu

^ permalink raw reply

* Re: [PATCH 2.6.13] IPv4/IPv6: USO Scatter-gather approac
From: Jeff Garzik @ 2005-09-14  0:24 UTC (permalink / raw)
  To: ravinandan.arakali
  Cc: netdev, raghavendra.koushik, leonid.grossman, rapuru.sriram,
	ananda.raju
In-Reply-To: <200509140655.j8E6t1jx003507@localhost.localdomain>

Please resend to netdev@vger.kernel.org, so that I may properly comment.

netdev@oss.sgi.com has been retired.

Thanks,

	Jeff

^ permalink raw reply

* Re: masquerading failure for at least icmp and tcp+sack on amd64
From: Patrick McHardy @ 2005-09-14  1:10 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Andrew Morton, netdev, Netfilter Development Mailinglist,
	Marc Lehmann
In-Reply-To: <20050913110902.0ad58b90@localhost.localdomain>

Stephen Hemminger wrote:
> Some background, the semantic of ip_summed is different on the
> output than the input path. On input, it means a checksum is
> available in skb->csum; and on output it means the packet is
> destined for a device that can do hardware checksumming.

Yep, so far the netfilter code should be correct since my batch
of HW checksum fixes.

> I have gotten reports of receive checksum errors on
> some systems, it may be related to certain revisions of
> hardware. It would be useful to see the message printed out
> by the skge driver that shows chip and revision.

That may be the reason. I've audited most relevant code-paths
for this case and they seem to be mostly OK. There are a couple
of cases in the ppp code I'm not sure about yet, but I couldn't
trigger any errors in my test-setup. Anyway they don't seem to
be related to this problem since it also happens with sk98_lin
which doesn't set CHECKSUM_HW.

> Also, on the input path for TCP and UDP, the code does not
> depend on the hardware being correct, and if the checksum
> is incorrect, it just prints a warning and does a software
> checksum before deciding to drop.
> Perhaps netfilter code needs to handle that case?

Yes, this is not handled so far. But is looks like there is
some other problem because it also happens with sk98_lin.

Thanks for your help Stephen.

^ permalink raw reply

* Re: masquerading failure for at least icmp and tcp+sack on amd64
From: Patrick McHardy @ 2005-09-14  1:13 UTC (permalink / raw)
  To: David S. Miller; +Cc: akpm, netdev, netfilter-devel, schmorp, shemminger
In-Reply-To: <20050913.135954.111731835.davem@davemloft.net>

David S. Miller wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Tue, 13 Sep 2005 11:09:02 -0700
> 
> 
>>Also, on the input path for TCP and UDP, the code does not
>>depend on the hardware being correct, and if the checksum
>>is incorrect, it just prints a warning and does a software
>>checksum before deciding to drop.
>>Perhaps netfilter code needs to handle that case?
> 
> I personally think netfilter should do so.

I agree. One thing I've planned for some silent moment is
to clean up the entire netfilter checksumming code (there's
lots of small duplicated chunks). Probably at least some
of it will also be applicable for the remaining stack.

^ permalink raw reply

* Re: masquerading failure for at least icmp and tcp+sack on amd64
From: David S. Miller @ 2005-09-14  3:41 UTC (permalink / raw)
  To: kaber; +Cc: akpm, netdev, netfilter-devel, schmorp, shemminger
In-Reply-To: <43277943.8050700@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 14 Sep 2005 03:13:39 +0200

> David S. Miller wrote:
> > I personally think netfilter should do so.
> 
> I agree. One thing I've planned for some silent moment is
> to clean up the entire netfilter checksumming code (there's
> lots of small duplicated chunks). Probably at least some
> of it will also be applicable for the remaining stack.

There is another thing I thought about today, and that is
to automatically handle this CHECKSUM_HW stuff when the
skb->data area is COW'd via pskb_expand_head() or similar.

I don't know how well that would work, but if it did then
we could consolidate all of this stuff into one spot which
is always nice.

^ 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