Netdev List
 help / color / mirror / Atom feed
* [PATCH] inetpeer: reduce stack usage
From: Eric Dumazet @ 2011-04-12  8:39 UTC (permalink / raw)
  To: Scot Doyle, David Miller; +Cc: Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302593469.3603.44.camel@edumazet-laptop>

On 64bit arches, we use 752 bytes of stack when cleanup_once() is called
from inet_getpeer().

Lets share the avl stack to save ~376 bytes.

Before patch :

# objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl

0x000006c3 unlink_from_pool [inetpeer.o]:		376
0x00000721 unlink_from_pool [inetpeer.o]:		376
0x00000cb1 inet_getpeer [inetpeer.o]:			376
0x00000e6d inet_getpeer [inetpeer.o]:			376
0x0004 inet_initpeers [inetpeer.o]:			112
# size net/ipv4/inetpeer.o
   text	   data	    bss	    dec	    hex	filename
   5320	    432	     21	   5773	   168d	net/ipv4/inetpeer.o

After patch :

objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl 
0x00000c11 inet_getpeer [inetpeer.o]:			376
0x00000dcd inet_getpeer [inetpeer.o]:			376
0x00000ab9 peer_check_expire [inetpeer.o]:		328
0x00000b7f peer_check_expire [inetpeer.o]:		328
0x0004 inet_initpeers [inetpeer.o]:			112
# size net/ipv4/inetpeer.o
   text	   data	    bss	    dec	    hex	filename
   5163	    432	     21	   5616	   15f0	net/ipv4/inetpeer.o

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Scot Doyle <lkml@scotdoyle.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Cc: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
---
 net/ipv4/inetpeer.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index dd1b20e..9df4e63 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -354,7 +354,8 @@ static void inetpeer_free_rcu(struct rcu_head *head)
 }
 
 /* May be called with local BH enabled. */
-static void unlink_from_pool(struct inet_peer *p, struct inet_peer_base *base)
+static void unlink_from_pool(struct inet_peer *p, struct inet_peer_base *base,
+			     struct inet_peer __rcu **stack[PEER_MAXDEPTH])
 {
 	int do_free;
 
@@ -368,7 +369,6 @@ static void unlink_from_pool(struct inet_peer *p, struct inet_peer_base *base)
 	 * We use refcnt=-1 to alert lockless readers this entry is deleted.
 	 */
 	if (atomic_cmpxchg(&p->refcnt, 1, -1) == 1) {
-		struct inet_peer __rcu **stack[PEER_MAXDEPTH];
 		struct inet_peer __rcu ***stackptr, ***delp;
 		if (lookup(&p->daddr, stack, base) != p)
 			BUG();
@@ -422,7 +422,7 @@ static struct inet_peer_base *peer_to_base(struct inet_peer *p)
 }
 
 /* May be called with local BH enabled. */
-static int cleanup_once(unsigned long ttl)
+static int cleanup_once(unsigned long ttl, struct inet_peer __rcu **stack[PEER_MAXDEPTH])
 {
 	struct inet_peer *p = NULL;
 
@@ -454,7 +454,7 @@ static int cleanup_once(unsigned long ttl)
 		 * happen because of entry limits in route cache. */
 		return -1;
 
-	unlink_from_pool(p, peer_to_base(p));
+	unlink_from_pool(p, peer_to_base(p), stack);
 	return 0;
 }
 
@@ -524,7 +524,7 @@ struct inet_peer *inet_getpeer(struct inetpeer_addr *daddr, int create)
 
 	if (base->total >= inet_peer_threshold)
 		/* Remove one less-recently-used entry. */
-		cleanup_once(0);
+		cleanup_once(0, stack);
 
 	return p;
 }
@@ -540,6 +540,7 @@ static void peer_check_expire(unsigned long dummy)
 {
 	unsigned long now = jiffies;
 	int ttl, total;
+	struct inet_peer __rcu **stack[PEER_MAXDEPTH];
 
 	total = compute_total();
 	if (total >= inet_peer_threshold)
@@ -548,7 +549,7 @@ static void peer_check_expire(unsigned long dummy)
 		ttl = inet_peer_maxttl
 				- (inet_peer_maxttl - inet_peer_minttl) / HZ *
 					total / inet_peer_threshold * HZ;
-	while (!cleanup_once(ttl)) {
+	while (!cleanup_once(ttl, stack)) {
 		if (jiffies != now)
 			break;
 	}



^ permalink raw reply related

* Re: Loopback and Nagle's algorithm
From: Alejandro Riveira Fernández @ 2011-04-12  9:42 UTC (permalink / raw)
  To: Adam McLaurin; +Cc: linux-kernel, netdev
In-Reply-To: <1302575869.13492.1440076201@webmail.messagingengine.com>

El Mon, 11 Apr 2011 22:37:49 -0400
"Adam McLaurin" <lkml@irotas.net> escribió:

 Just CCing netdev

> I understand that disabling Nagle's algorithm via TCP_NODELAY will
> generally degrade throughput. However, in my scenario (150 byte
> messages, sending as fast as possible), the actual throughput penalty
> over the network is marginal (maybe 10% at most).
> 
> However, when I disable Nagle's algorithm when connecting over loopback,
> the performance hit is *huge* - 10x reduction in throughput.
> 
> The question is, why is disabling Nagle's algorithm on loopback so much
> worse w.r.t. throughput? Is there anything I can do to reduce the
> incurred throughput penalty?
> 
> Thanks,
> Adam
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* [PATCH] net: Do not wrap sysctl igmp_max_memberships in IP_MULTICAST
From: Joakim Tjernlund @ 2011-04-12  9:49 UTC (permalink / raw)
  To: netdev; +Cc: Joakim Tjernlund

controlling igmp_max_membership is useful even when IP_MULTICAST
is off.
Quagga(an OSPF deamon) uses multicast addresses for all interfaces
using a single socket and hits igmp_max_membership limit when
there are 20 interfaces or more.
Always export sysctl igmp_max_memberships in proc, just like
igmp_max_msf

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 net/ipv4/sysctl_net_ipv4.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index d96c1da..9cc2824 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -306,7 +306,6 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_do_large_bitmap,
 	},
-#ifdef CONFIG_IP_MULTICAST
 	{
 		.procname	= "igmp_max_memberships",
 		.data		= &sysctl_igmp_max_memberships,
@@ -314,8 +313,6 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
-
-#endif
 	{
 		.procname	= "igmp_max_msf",
 		.data		= &sysctl_igmp_max_msf,
-- 
1.7.3.4


^ permalink raw reply related

* Re: Kernel panic when using bridge
From: Eric Dumazet @ 2011-04-12 11:49 UTC (permalink / raw)
  To: Scot Doyle; +Cc: Stephen Hemminger, Hiroaki SHIMODA, netdev, Jan Luebbe
In-Reply-To: <4DA3F909.5020609@scotdoyle.com>

Le mardi 12 avril 2011 à 02:02 -0500, Scot Doyle a écrit :
> On 04/12/2011 12:51 AM, Eric Dumazet wrote:
> >
> > Oh well, sorry (not enough time these days to even test patches)
> >
> > 	if (!skb_dst(skb)) {
> 
> --- br_netfilter.c.a    2011-04-01 02:37:53.000000000 -0500
> +++ br_netfilter.c.b    2011-04-12 00:29:00.000000000 -0500
> @@ -221,6 +221,7 @@ static int br_parse_ip_options(struct sk
>       struct ip_options *opt;
>       struct iphdr *iph;
>       struct net_device *dev = skb->dev;
> +    struct rtable *rt;
>       u32 len;
> 
>       iph = ip_hdr(skb);
> @@ -255,6 +256,16 @@ static int br_parse_ip_options(struct sk
>           return 0;
>       }
> 
> +    /* Associate bogus bridge route table */
> +    if (!skb_dst(skb)) {
> +        rt = bridge_parent_rtable(dev);
> +        if (!rt) {
> +            kfree_skb(skb);
> +            return 0;
> +        }
> +        skb_dst_set_noref(skb,&rt->dst);
> +    }
> +
>       opt->optlen = iph->ihl*4 - sizeof(struct iphdr);
>       if (ip_options_compile(dev_net(dev), opt, skb))
>           goto inhdr_error;
> 
> 
> Now we are making progress! With the patch above from Stephen and Eric, 
> I cannot make the kernel panic when sending packets to the IP address of 
> the bridge.
> 
> However, if a guest virtual machine is sharing the bridge with the host 
> via a tap device, I can cause a host panic by targeting the IP address 
> of the guest. Is this an unrelated problem?
> 
> Here are two kernel panics. The guest virtual machine was pingable 
> before being attacked with IP Stack Checker's tcpsic command. Spanning 
> Tree Protocol was off during the first panic and on during the second.
> 
> ------------
> 
> [  606.921739] br0: port 2(tap0) entering forwarding state
> [  636.058941] Kernel panic - not syncing: stack-protector: Kernel stack 
> is corrupted in: ffffffff812c2781
> [  636.058942]
> [  636.069789] Pid: 2261, comm: kvm Tainted: G        W   2.6.39-rc2+ #11
> [  636.076292] Call Trace:
> [  636.078725] <IRQ>  [<ffffffff8132ad78>] ? panic+0x92/0x1a1
> [  636.084287]  [<ffffffff8104abe8>] ? _local_bh_enable_ip.clone.8+0x20/0x8c
> [  636.091044]  [<ffffffff812c2781>] ? icmp_send+0x337/0x349
> [  636.096418]  [<ffffffff810454e5>] ? __stack_chk_fail+0x17/0x17
> [  636.102221]  [<ffffffff812c2781>] ? icmp_send+0x337/0x349
> [  636.107595]  [<ffffffff81298527>] ? nf_iterate+0x41/0x7e
> [  636.112883]  [<ffffffff81298527>] ? nf_iterate+0x41/0x7e
> [  636.118172]  [<ffffffffa017b0d4>] ? br_flood+0xc8/0xc8 [bridge]
> [  636.124065]  [<ffffffffa017b250>] ? __br_deliver+0xb0/0xb0 [bridge]
> [  636.130302]  [<ffffffff812985d7>] ? nf_hook_slow+0x73/0x114
> [  636.135850]  [<ffffffffa017b250>] ? __br_deliver+0xb0/0xb0 [bridge]
> [  636.142089]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.148586]  [<ffffffffa017b250>] ? __br_deliver+0xb0/0xb0 [bridge]
> [  636.154826]  [<ffffffffa017b186>] ? NF_HOOK.clone.5+0x3c/0x56 [bridge]
> [  636.161323]  [<ffffffffa017bfe1>] ? 
> br_handle_frame_finish+0x158/0x1c7 [bridge]
> [  636.168601]  [<ffffffffa0180689>] ? 
> br_nf_pre_routing_finish+0x1d4/0x1e1 [bridge]
> [  636.176052]  [<ffffffffa017fc76>] ? NF_HOOK_THRESH+0x3b/0x55 [bridge]
> [  636.182463]  [<ffffffffa0180c84>] ? br_nf_pre_routing+0x3be/0x3cb 
> [bridge]
> [  636.189307]  [<ffffffff812985d7>] ? nf_hook_slow+0x73/0x114
> [  636.194852]  [<ffffffff81298527>] ? nf_iterate+0x41/0x7e
> [  636.200139]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.206637]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.213133]  [<ffffffff812985d7>] ? nf_hook_slow+0x73/0x114
> [  636.218679]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.225177]  [<ffffffffa017bfe1>] ? 
> br_handle_frame_finish+0x158/0x1c7 [bridge]
> [  636.232455]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.238954]  [<ffffffffa017be6f>] ? NF_HOOK.clone.4+0x3c/0x56 [bridge]
> [  636.245452]  [<ffffffff812a7d8e>] ? tcp_gro_receive+0xa1/0x204
> [  636.251258]  [<ffffffffa017c1e5>] ? br_handle_frame+0x195/0x1ac [bridge]
> [  636.257928]  [<ffffffffa017c050>] ? 
> br_handle_frame_finish+0x1c7/0x1c7 [bridge]
> [  636.265204]  [<ffffffff812764ef>] ? __netif_receive_skb+0x2a7/0x450
> [  636.271443]  [<ffffffff81276928>] ? netif_receive_skb+0x52/0x58
> [  636.277335]  [<ffffffff81276e2a>] ? napi_gro_receive+0x1f/0x2f
> [  636.283139]  [<ffffffff812769ff>] ? napi_skb_finish+0x1c/0x31
> [  636.288865]  [<ffffffffa0241fcd>] ? igb_poll+0x6d9/0x9ee [igb]
> [  636.294673]  [<ffffffffa003bde2>] ? scsi_run_queue+0x2ce/0x30a [scsi_mod]
> [  636.301431]  [<ffffffffa017be89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  636.307930]  [<ffffffff812764ef>] ? __netif_receive_skb+0x2a7/0x450
> [  636.314168]  [<ffffffff81276f55>] ? net_rx_action+0xa4/0x1b1
> [  636.319800]  [<ffffffff8104ad26>] ? __do_softirq+0xb8/0x176
> [  636.325346]  [<ffffffff81333c5c>] ? call_softirq+0x1c/0x30
> [  636.330807]  [<ffffffff8100aa57>] ? do_softirq+0x3f/0x84
> [  636.336092]  [<ffffffff8104af91>] ? irq_exit+0x3f/0x8f
> [  636.341204]  [<ffffffff8100a793>] ? do_IRQ+0x85/0x9e
> [  636.346146]  [<ffffffff8132cbd3>] ? common_interrupt+0x13/0x13
> [  636.351949] <EOI>  [<ffffffff81271f58>] ? arch_local_irq_save+0x12/0x1b
> [  636.358629]  [<ffffffff8100a9f2>] ? arch_local_irq_restore+0x2/0x8
> [  636.364781]  [<ffffffff8127680d>] ? netif_rx_ni+0x1e/0x27
> [  636.370154]  [<ffffffffa01557d2>] ? tun_get_user+0x3a3/0x3cb [tun]
> [  636.376305]  [<ffffffffa0155bd8>] ? tun_get_socket+0x3b/0x3b [tun]
> [  636.382457]  [<ffffffffa0155c36>] ? tun_chr_aio_write+0x5e/0x79 [tun]
> [  636.388869]  [<ffffffff810f6b07>] ? do_sync_readv_writev+0x9a/0xd5
> [  636.395021]  [<ffffffff810371f3>] ? need_resched+0x1a/0x23
> [  636.400481]  [<ffffffff8132b725>] ? _cond_resched+0x9/0x20
> [  636.405941]  [<ffffffff810f5f77>] ? copy_from_user+0x18/0x30
> [  636.411573]  [<ffffffff8115fbf6>] ? security_file_permission+0x18/0x33
> [  636.418068]  [<ffffffff810f6d55>] ? do_readv_writev+0xa4/0x11a
> [  636.423873]  [<ffffffff810f7913>] ? fput+0x1a/0x1a2
> [  636.428726]  [<ffffffff810f6f39>] ? sys_writev+0x45/0x90
> [  636.434012]  [<ffffffff81332a52>] ? system_call_fastpath+0x16/0x1b
> 
> ------------
> 
> [  110.442839] br0: port 2(tap0) entering forwarding state
> [  136.948700] Kernel panic - not syncing: stack-protector: Kernel stack 
> is corrupted in: ffffffff812c2781
> [  136.948702]
> [  136.959561] Pid: 1093, comm: md123_resync Not tainted 2.6.39-rc2+ #11
> [  136.965977] Call Trace:
> [  136.968408] <IRQ>  [<ffffffff8132ad78>] ? panic+0x92/0x1a1
> [  136.973970]  [<ffffffff8104abe8>] ? _local_bh_enable_ip.clone.8+0x20/0x8c
> [  136.980727]  [<ffffffff812c2781>] ? icmp_send+0x337/0x349
> [  136.986102]  [<ffffffff810454e5>] ? __stack_chk_fail+0x17/0x17
> [  136.991906]  [<ffffffff812c2781>] ? icmp_send+0x337/0x349
> [  136.997281]  [<ffffffff81298527>] ? nf_iterate+0x41/0x7e
> [  137.002570]  [<ffffffffa0198fe1>] ? 
> br_handle_frame_finish+0x158/0x1c7 [bridge]
> [  137.009847]  [<ffffffffa019d689>] ? 
> br_nf_pre_routing_finish+0x1d4/0x1e1 [bridge]
> [  137.017297]  [<ffffffffa019cc76>] ? NF_HOOK_THRESH+0x3b/0x55 [bridge]
> [  137.023707]  [<ffffffffa019dc84>] ? br_nf_pre_routing+0x3be/0x3cb 
> [bridge]
> [  137.030551]  [<ffffffff81298527>] ? nf_iterate+0x41/0x7e
> [  137.035837]  [<ffffffff8103704d>] ? test_tsk_need_resched+0xe/0x17
> [  137.041991]  [<ffffffffa0198e89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  137.048488]  [<ffffffffa0198e89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  137.054984]  [<ffffffff812985d7>] ? nf_hook_slow+0x73/0x114
> [  137.060531]  [<ffffffffa0198e89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  137.067028]  [<ffffffffa0198e89>] ? NF_HOOK.clone.4+0x56/0x56 [bridge]
> [  137.073526]  [<ffffffffa0198e6f>] ? NF_HOOK.clone.4+0x3c/0x56 [bridge]
> [  137.080023]  [<ffffffff812a7d8e>] ? tcp_gro_receive+0xa1/0x204
> [  137.085830]  [<ffffffffa01991e5>] ? br_handle_frame+0x195/0x1ac [bridge]
> [  137.092500]  [<ffffffffa0199050>] ? 
> br_handle_frame_finish+0x1c7/0x1c7 [bridge]
> [  137.099776]  [<ffffffff812764ef>] ? __netif_receive_skb+0x2a7/0x450
> [  137.106013]  [<ffffffff81276928>] ? netif_receive_skb+0x52/0x58
> [  137.111906]  [<ffffffff81276e2a>] ? napi_gro_receive+0x1f/0x2f
> [  137.117713]  [<ffffffff812769ff>] ? napi_skb_finish+0x1c/0x31
> [  137.123438]  [<ffffffffa0226fcd>] ? igb_poll+0x6d9/0x9ee [igb]
> [  137.129243]  [<ffffffff8109034f>] ? handle_irq_event+0x40/0x55
> [  137.135049]  [<ffffffff8132cbd3>] ? common_interrupt+0x13/0x13
> [  137.140854]  [<ffffffff81276f55>] ? net_rx_action+0xa4/0x1b1
> [  137.146487]  [<ffffffff8104ad26>] ? __do_softirq+0xb8/0x176
> [  137.152034]  [<ffffffff81333c5c>] ? call_softirq+0x1c/0x30
> [  137.157494]  [<ffffffff8100aa57>] ? do_softirq+0x3f/0x84
> [  137.162779]  [<ffffffff8104af91>] ? irq_exit+0x3f/0x8f
> [  137.167893]  [<ffffffff8100a793>] ? do_IRQ+0x85/0x9e
> [  137.172833]  [<ffffffff8132cbd3>] ? common_interrupt+0x13/0x13
> [  137.178636] <EOI>  [<ffffffff8106fc1a>] ? arch_local_irq_restore+0x2/0x8
> [  137.185408]  [<ffffffffa0050fca>] ? _scsih_qcmd+0x54f/0x561 [mpt2sas]
> [  137.191823]  [<ffffffffa01e452f>] ? scsi_dispatch_cmd+0x180/0x219 
> [scsi_mod]
> [  137.198841]  [<ffffffffa01ea385>] ? scsi_request_fn+0x3e6/0x413 
> [scsi_mod]
> [  137.205683]  [<ffffffff81187470>] ? elv_rqhash_add.clone.15+0x26/0x4c
> [  137.212095]  [<ffffffff8118bde2>] ? __blk_run_queue+0x5e/0x84
> [  137.217814]  [<ffffffff8118d63c>] ? __make_request+0x273/0x28f
> [  137.223619]  [<ffffffff8118b569>] ? generic_make_request+0x267/0x2e1
> [  137.229943]  [<ffffffff8105eb49>] ? remove_wait_queue+0x11/0x4d
> [  137.235837]  [<ffffffffa0002417>] ? raise_barrier+0x162/0x16f [raid1]
> [  137.242246]  [<ffffffff8103eba4>] ? try_to_wake_up+0x17c/0x17c
> [  137.248052]  [<ffffffffa0002f2f>] ? sync_request+0x567/0x583 [raid1]
> [  137.254379]  [<ffffffffa00bd834>] ? md_do_sync+0x776/0xb8e [md_mod]
> [  137.260617]  [<ffffffff8100e537>] ? sched_clock+0x5/0x8
> [  137.265819]  [<ffffffffa00bde83>] ? md_thread+0xfa/0x118 [md_mod]
> [  137.271886]  [<ffffffffa00bdd89>] ? md_rdev_init+0x8f/0x8f [md_mod]
> [  137.278124]  [<ffffffffa00bdd89>] ? md_rdev_init+0x8f/0x8f [md_mod]
> [  137.284362]  [<ffffffff8105e497>] ? kthread+0x7a/0x82
> [  137.289390]  [<ffffffff81333b64>] ? kernel_thread_helper+0x4/0x10
> [  137.295454]  [<ffffffff8105e41d>] ? kthread_worker_fn+0x149/0x149
> [  137.301519]  [<ffffffff81333b60>] ? gs_change+0x13/0x13
> 

Considering recent changes in ip_options_echo() I would suggest to add
following patch and/or revert commit 8628bd8af7c4c14f40
(ipv4: Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
ip_options_echo())

Thanks

diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index 28a736f..35f2bf9 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -200,6 +200,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
 		*dptr++ = IPOPT_END;
 		dopt->optlen++;
 	}
+	if (unlikely(dopt->optlen > 40)) {
+		pr_err("ip_options_echo() fatal error optlen=%u > 40\n", dopt->optlen);
+		print_hex_dump(KERN_ERR, "ip options: ", DUMP_PREFIX_OFFSET,
+			16, 1, dopt->__data, dopt->optlen, false);
+	}
 	return 0;
 }
 



^ permalink raw reply related

* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Vladislav Zolotarov @ 2011-04-12 12:10 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <20110411202630.C079D13909@rere.qmqm.pl>

On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> Since ndo_fix_features callback is postponing features change when
> bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> has to be called again when this condition changes. Previously,
> ethtool_ops->set_flags callback returned -EBUSY in that case
> (it's not possible in the new model).

ACK (with reservations). ;)

Could u, pls., just add this comment I've asked for in the previous
e-mail? 

The things I first thought to comment on are:
	- Removing TPA_ENABLED_FLAG the similar way u've removed the
bp->rx_csum.
	- Merging the code handling 'features' in bnx2x_init_bp() with the
similar code in bnx2x_init_dev().

However I think it would be right if we clear our mess by ourselves and
that u have already done much enough... ;) 

I've run our standard test suite (which in particular heavily tests the
RX_CSUM and LRO flags toggling) on this patch and it passed it.

Thanks a lot, Michal.
vlad

> 
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> v4: - complete bp->rx_csum -> NETIF_F_RXCSUM conversion
>     - add check for failed ndo_set_features in ndo_open callback
> v3: - include NETIF_F_LRO in hw_features
>     - don't call netdev_update_features() if bnx2x_nic_load() failed
> v2: - comment in ndo_fix_features callback
> ---
>  drivers/net/bnx2x/bnx2x.h         |    1 -
>  drivers/net/bnx2x/bnx2x_cmn.c     |   54 +++++++++++++++++++--
>  drivers/net/bnx2x/bnx2x_cmn.h     |    3 +
>  drivers/net/bnx2x/bnx2x_ethtool.c |   95 -------------------------------------
>  drivers/net/bnx2x/bnx2x_main.c    |   41 +++++++++-------
>  5 files changed, 75 insertions(+), 119 deletions(-)
> 
> diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
> index b7ff87b..fefd1d5 100644
> --- a/drivers/net/bnx2x/bnx2x.h
> +++ b/drivers/net/bnx2x/bnx2x.h
> @@ -918,7 +918,6 @@ struct bnx2x {
>  
>  	int			tx_ring_size;
>  
> -	u32			rx_csum;
>  /* L2 header size + 2*VLANs (8 bytes) + LLC SNAP (8 bytes) */
>  #define ETH_OVREHEAD		(ETH_HLEN + 8 + 8)
>  #define ETH_MIN_PACKET_SIZE		60
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
> index e83ac6d..7f49cf4 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/bnx2x/bnx2x_cmn.c
> @@ -640,7 +640,7 @@ reuse_rx:
>  
>  			skb_checksum_none_assert(skb);
>  
> -			if (bp->rx_csum) {
> +			if (bp->dev->features & NETIF_F_RXCSUM) {
>  				if (likely(BNX2X_RX_CSUM_OK(cqe)))
>  					skb->ip_summed = CHECKSUM_UNNECESSARY;
>  				else
> @@ -2443,11 +2443,21 @@ alloc_err:
>  
>  }
>  
> +static int bnx2x_reload_if_running(struct net_device *dev)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +
> +	if (unlikely(!netif_running(dev)))
> +		return 0;
> +
> +	bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> +	return bnx2x_nic_load(bp, LOAD_NORMAL);
> +}
> +
>  /* called with rtnl_lock */
>  int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
> -	int rc = 0;
>  
>  	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
>  		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> @@ -2464,12 +2474,44 @@ int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
>  	 */
>  	dev->mtu = new_mtu;
>  
> -	if (netif_running(dev)) {
> -		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> -		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> +	return bnx2x_reload_if_running(dev);
> +}
> +
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +
> +	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> +		netdev_err(dev, "Handling parity error recovery. Try again later\n");
> +
> +		/* Don't allow bnx2x_set_features() to be called now. */
> +		return dev->features;
> +	}
> +
> +	/* TPA requires Rx CSUM offloading */
> +	if (!(features & NETIF_F_RXCSUM) || bp->disable_tpa)
> +		features &= ~NETIF_F_LRO;
> +
> +	return features;
> +}
> +
> +int bnx2x_set_features(struct net_device *dev, u32 features)
> +{
> +	struct bnx2x *bp = netdev_priv(dev);
> +	u32 flags = bp->flags;
> +
> +	if (features & NETIF_F_LRO)
> +		flags |= TPA_ENABLE_FLAG;
> +	else
> +		flags &= ~TPA_ENABLE_FLAG;
> +
> +	if (flags ^ bp->flags) {
> +		bp->flags = flags;
> +
> +		return bnx2x_reload_if_running(dev);
>  	}
>  
> -	return rc;
> +	return 0;
>  }
>  
>  void bnx2x_tx_timeout(struct net_device *dev)
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
> index 775fef0..1cdab69 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.h
> +++ b/drivers/net/bnx2x/bnx2x_cmn.h
> @@ -431,6 +431,9 @@ void bnx2x_free_mem_bp(struct bnx2x *bp);
>   */
>  int bnx2x_change_mtu(struct net_device *dev, int new_mtu);
>  
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features);
> +int bnx2x_set_features(struct net_device *dev, u32 features);
> +
>  /**
>   * tx timeout netdev callback
>   *
> diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
> index 1479994..ad7d91e 100644
> --- a/drivers/net/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/bnx2x/bnx2x_ethtool.c
> @@ -1299,91 +1299,6 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
>  	return 0;
>  }
>  
> -static int bnx2x_set_flags(struct net_device *dev, u32 data)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -	int changed = 0;
> -	int rc = 0;
> -
> -	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> -		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> -		return -EAGAIN;
> -	}
> -
> -	if (!(data & ETH_FLAG_RXVLAN))
> -		return -EINVAL;
> -
> -	if ((data & ETH_FLAG_LRO) && bp->rx_csum && bp->disable_tpa)
> -		return -EINVAL;
> -
> -	rc = ethtool_op_set_flags(dev, data, ETH_FLAG_LRO | ETH_FLAG_RXVLAN |
> -					ETH_FLAG_TXVLAN | ETH_FLAG_RXHASH);
> -	if (rc)
> -		return rc;
> -
> -	/* TPA requires Rx CSUM offloading */
> -	if ((data & ETH_FLAG_LRO) && bp->rx_csum) {
> -		if (!(bp->flags & TPA_ENABLE_FLAG)) {
> -			bp->flags |= TPA_ENABLE_FLAG;
> -			changed = 1;
> -		}
> -	} else if (bp->flags & TPA_ENABLE_FLAG) {
> -		dev->features &= ~NETIF_F_LRO;
> -		bp->flags &= ~TPA_ENABLE_FLAG;
> -		changed = 1;
> -	}
> -
> -	if (changed && netif_running(dev)) {
> -		bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> -		rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> -	}
> -
> -	return rc;
> -}
> -
> -static u32 bnx2x_get_rx_csum(struct net_device *dev)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -
> -	return bp->rx_csum;
> -}
> -
> -static int bnx2x_set_rx_csum(struct net_device *dev, u32 data)
> -{
> -	struct bnx2x *bp = netdev_priv(dev);
> -	int rc = 0;
> -
> -	if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> -		printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> -		return -EAGAIN;
> -	}
> -
> -	bp->rx_csum = data;
> -
> -	/* Disable TPA, when Rx CSUM is disabled. Otherwise all
> -	   TPA'ed packets will be discarded due to wrong TCP CSUM */
> -	if (!data) {
> -		u32 flags = ethtool_op_get_flags(dev);
> -
> -		rc = bnx2x_set_flags(dev, (flags & ~ETH_FLAG_LRO));
> -	}
> -
> -	return rc;
> -}
> -
> -static int bnx2x_set_tso(struct net_device *dev, u32 data)
> -{
> -	if (data) {
> -		dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -		dev->features |= NETIF_F_TSO6;
> -	} else {
> -		dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
> -		dev->features &= ~NETIF_F_TSO6;
> -	}
> -
> -	return 0;
> -}
> -
>  static const struct {
>  	char string[ETH_GSTRING_LEN];
>  } bnx2x_tests_str_arr[BNX2X_NUM_TESTS] = {
> @@ -2207,16 +2122,6 @@ static const struct ethtool_ops bnx2x_ethtool_ops = {
>  	.set_ringparam		= bnx2x_set_ringparam,
>  	.get_pauseparam		= bnx2x_get_pauseparam,
>  	.set_pauseparam		= bnx2x_set_pauseparam,
> -	.get_rx_csum		= bnx2x_get_rx_csum,
> -	.set_rx_csum		= bnx2x_set_rx_csum,
> -	.get_tx_csum		= ethtool_op_get_tx_csum,
> -	.set_tx_csum		= ethtool_op_set_tx_hw_csum,
> -	.set_flags		= bnx2x_set_flags,
> -	.get_flags		= ethtool_op_get_flags,
> -	.get_sg			= ethtool_op_get_sg,
> -	.set_sg			= ethtool_op_set_sg,
> -	.get_tso		= ethtool_op_get_tso,
> -	.set_tso		= bnx2x_set_tso,
>  	.self_test		= bnx2x_self_test,
>  	.get_sset_count		= bnx2x_get_sset_count,
>  	.get_strings		= bnx2x_get_strings,
> diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
> index f3cf889..5fd7cbb 100644
> --- a/drivers/net/bnx2x/bnx2x_main.c
> +++ b/drivers/net/bnx2x/bnx2x_main.c
> @@ -7728,6 +7728,7 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
>  						return;
>  					}
>  
> +					netdev_update_features(bp->dev);
>  					return;
>  				}
>  			} else { /* non-leader */
> @@ -7755,10 +7756,12 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
>  					  * the "process kill". It's an exit
>  					  * point for a non-leader.
>  					  */
> -					bnx2x_nic_load(bp, LOAD_NORMAL);
> +					int rc = bnx2x_nic_load(bp, LOAD_NORMAL);
>  					bp->recovery_state =
>  						BNX2X_RECOVERY_DONE;
>  					smp_wmb();
> +					if (!rc)
> +						netdev_update_features(bp->dev);
>  					return;
>  				}
>  			}
> @@ -8904,8 +8907,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>  	bp->multi_mode = multi_mode;
>  	bp->int_mode = int_mode;
>  
> -	bp->dev->features |= NETIF_F_GRO;
> -
>  	/* Set TPA flags */
>  	if (disable_tpa) {
>  		bp->flags &= ~TPA_ENABLE_FLAG;
> @@ -8925,8 +8926,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>  
>  	bp->tx_ring_size = MAX_TX_AVAIL;
>  
> -	bp->rx_csum = 1;
> -
>  	/* make sure that the numbers are in the right granularity */
>  	bp->tx_ticks = (50 / BNX2X_BTR) * BNX2X_BTR;
>  	bp->rx_ticks = (25 / BNX2X_BTR) * BNX2X_BTR;
> @@ -8954,6 +8953,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>  static int bnx2x_open(struct net_device *dev)
>  {
>  	struct bnx2x *bp = netdev_priv(dev);
> +	int rc;
>  
>  	netif_carrier_off(dev);
>  
> @@ -8993,7 +8993,14 @@ static int bnx2x_open(struct net_device *dev)
>  
>  	bp->recovery_state = BNX2X_RECOVERY_DONE;
>  
> -	return bnx2x_nic_load(bp, LOAD_OPEN);
> +	rc = bnx2x_nic_load(bp, LOAD_OPEN);
> +	if (!rc) {
> +		netdev_update_features(bp->dev);
> +		if (bp->state != BNX2X_STATE_OPEN)
> +			return -EBUSY;
> +	}
> +
> +	return rc;
>  }
>  
>  /* called with rtnl_lock */
> @@ -9304,6 +9311,8 @@ static const struct net_device_ops bnx2x_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  	.ndo_do_ioctl		= bnx2x_ioctl,
>  	.ndo_change_mtu		= bnx2x_change_mtu,
> +	.ndo_fix_features	= bnx2x_fix_features,
> +	.ndo_set_features	= bnx2x_set_features,
>  	.ndo_tx_timeout		= bnx2x_tx_timeout,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller	= poll_bnx2x,
> @@ -9430,20 +9439,18 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
>  
>  	dev->netdev_ops = &bnx2x_netdev_ops;
>  	bnx2x_set_ethtool_ops(dev);
> -	dev->features |= NETIF_F_SG;
> -	dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> +
>  	if (bp->flags & USING_DAC_FLAG)
>  		dev->features |= NETIF_F_HIGHDMA;
> -	dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -	dev->features |= NETIF_F_TSO6;
> -	dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
>  
> -	dev->vlan_features |= NETIF_F_SG;
> -	dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> -	if (bp->flags & USING_DAC_FLAG)
> -		dev->vlan_features |= NETIF_F_HIGHDMA;
> -	dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> -	dev->vlan_features |= NETIF_F_TSO6;
> +	dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
> +		NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_HW_VLAN_TX;
> +
> +	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_RX;
> +
> +	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;
>  
>  #ifdef BCM_DCBNL
>  	dev->dcbnl_ops = &bnx2x_dcbnl_ops;




^ permalink raw reply

* Re: 2.6.39-rc2 boot crash
From: Patrick McHardy @ 2011-04-12 12:49 UTC (permalink / raw)
  To: Evgeniy Polyakov
  Cc: Eric B Munson, David Miller, dave, linux-kernel, gregkh,
	ksrinivasan, NetDev
In-Reply-To: <20110411220657.GB5783@ioremap.net>

On 12.04.2011 00:06, Evgeniy Polyakov wrote:
> Hi.
> 
> On Mon, Apr 11, 2011 at 05:07:47PM -0400, Eric B Munson (emunson@mgebm.net) wrote:
>>> I can't figure this out, the only thing that should have changed is the
>>> time the initial PROC_CN_MCAST_LISTEN message is received. Apparently
>>> at that point connector is not fully initialized yet. Please post your
>>> config and the full boot log. Thanks.
>>>
>>
>> I am still seeing this on Linus' tree, is there anything more I can do to help
>> track the problem?

Sorry, I had a hardware failure, I'm back working on this now.

> Patrick, do you need my assist on this bug?

Thanks, but I can meanwhile reproduce the problem, so I think I
should have a fix soon.

^ permalink raw reply

* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 13:02 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302608951.3233.33.camel@edumazet-laptop>

Hi! 

On Tue, 2011-04-12 at 13:49 +0200, Eric Dumazet wrote:
> Considering recent changes in ip_options_echo() I would suggest to add
> following patch and/or revert commit 8628bd8af7c4c14f40
> (ipv4: Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
> ip_options_echo())

I've read this thread, but I'm not sure why my patch is related to these
kernel panics. The behavior is only changed for packets with the
timestamp option in prespecified addresses mode. Even then, it shouldn't
cause ip_options_build to write to unallocated memory later.

> diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
> index 28a736f..35f2bf9 100644
> --- a/net/ipv4/ip_options.c
> +++ b/net/ipv4/ip_options.c
> @@ -200,6 +200,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
>                 *dptr++ = IPOPT_END;
>                 dopt->optlen++;
>         }
> +       if (unlikely(dopt->optlen > 40)) {
> +               pr_err("ip_options_echo() fatal error optlen=%u > 40\n", dopt->optlen);
> +               print_hex_dump(KERN_ERR, "ip options: ", DUMP_PREFIX_OFFSET,
> +                       16, 1, dopt->__data, dopt->optlen, false);
> +       }
>         return 0;
>  }

Here you check dopt->optlen, which certainly should be 40 at most. The
calculation of dopt->optlen wasn't changed by my patch, though.

Regards,
Jan


^ permalink raw reply

* Re: Loopback and Nagle's algorithm
From: Eric Dumazet @ 2011-04-12 13:08 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Adam McLaurin, Will Newton, linux-kernel, netdev
In-Reply-To: <alpine.LRH.2.00.1104121354010.6953@twin.jikos.cz>

Le mardi 12 avril 2011 à 13:54 +0200, Jiri Kosina a écrit :
> On Tue, 12 Apr 2011, Adam McLaurin wrote:
> 
> > > It may be caused by an increase in context switch rate, as both sender
> > > and receiver are on the same machine.
> > 
> > I'm not sure that's what's happening, since the box where I'm running
> > this test has 8 physical CPU's and 32 cores in total.
> 
> Have you tried firing up the testcase under perf, to see what it reveals 
> as the bottleneck?
> 
CC netdev

This rings a bell here.

I suspect we hit mod_timer() / lock_timer_base()  because of delack
timer constantly changing.

I remember raising this point last year :

http://kerneltrap.org/mailarchive/linux-netdev/2010/5/20/6277741

David answer : 
http://kerneltrap.org/mailarchive/linux-netdev/2010/6/2/6278430

I am afraid no change was done...

^ permalink raw reply

* Re: Kernel panic when using bridge
From: Eric Dumazet @ 2011-04-12 13:15 UTC (permalink / raw)
  To: Jan Lübbe; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302613353.30934.22.camel@polaris.local>

Le mardi 12 avril 2011 à 15:02 +0200, Jan Lübbe a écrit :
> Hi! 
> 
> On Tue, 2011-04-12 at 13:49 +0200, Eric Dumazet wrote:
> > Considering recent changes in ip_options_echo() I would suggest to add
> > following patch and/or revert commit 8628bd8af7c4c14f40
> > (ipv4: Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
> > ip_options_echo())
> 
> I've read this thread, but I'm not sure why my patch is related to these
> kernel panics. The behavior is only changed for packets with the
> timestamp option in prespecified addresses mode. Even then, it shouldn't
> cause ip_options_build to write to unallocated memory later.
> 
> > diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
> > index 28a736f..35f2bf9 100644
> > --- a/net/ipv4/ip_options.c
> > +++ b/net/ipv4/ip_options.c
> > @@ -200,6 +200,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
> >                 *dptr++ = IPOPT_END;
> >                 dopt->optlen++;
> >         }
> > +       if (unlikely(dopt->optlen > 40)) {
> > +               pr_err("ip_options_echo() fatal error optlen=%u > 40\n", dopt->optlen);
> > +               print_hex_dump(KERN_ERR, "ip options: ", DUMP_PREFIX_OFFSET,
> > +                       16, 1, dopt->__data, dopt->optlen, false);
> > +       }
> >         return 0;
> >  }
> 
> Here you check dopt->optlen, which certainly should be 40 at most. The
> calculation of dopt->optlen wasn't changed by my patch, though.

Check again the thread Jan.

Scot is using a tool (IP Stack Checker's tcpsic) to forge random tcp
packets.

Maybe your patch is fine but requires a change in a previous function,
to make sure we deny some crazy packet before generating an ip_options
with more than 40 bytes, in an icmp_send() reply.

I took a look at this ip_options stuff and must say its really hard to
even _read_ the code. Understanding it might need several days or a new
brain ?

I cannot Ack or Nack your patch, I must admit it. Isnt it frightening ?




^ permalink raw reply

* [PATCH] r8169: Be verbose when unable to load fw patch
From: Borislav Petkov @ 2011-04-12 13:32 UTC (permalink / raw)
  To: Francois Romieu; +Cc: linux-kernel, Borislav Petkov, netdev

From: Borislav Petkov <borislav.petkov@amd.com>

When the driver fails loading the firmware, it doesn't say which patch
exactly it is missing. We have three different firmware images depending
on the hw revision, so mention the exact patch name it is unable to load
in the warning message.

Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
 drivers/net/r8169.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 493b0de..b068115 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2248,7 +2248,7 @@ static void rtl8168d_1_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x05, 0x001b);
 	if ((rtl_readphy(tp, 0x06) != 0xbf00) ||
 	    (rtl_apply_firmware(tp, FIRMWARE_8168D_1) < 0)) {
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8168D_1);
 	}
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -2353,7 +2353,7 @@ static void rtl8168d_2_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x05, 0x001b);
 	if ((rtl_readphy(tp, 0x06) != 0xb300) ||
 	    (rtl_apply_firmware(tp, FIRMWARE_8168D_2) < 0)) {
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8168D_2);
 	}
 
 	rtl_writephy(tp, 0x1f, 0x0000);
@@ -2475,7 +2475,7 @@ static void rtl8105e_hw_phy_config(struct rtl8169_private *tp)
 	msleep(100);
 
 	if (rtl_apply_firmware(tp, FIRMWARE_8105E_1) < 0)
-		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+		netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8105E_1);
 
 	rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
 }
-- 
1.7.4

^ permalink raw reply related

* Bonding/LACP on RTL8169sc/8110sc (R1869)
From: Jonathan Thibault @ 2011-04-12 13:35 UTC (permalink / raw)
  To: netdev; +Cc: Francois Romieu

Greetings all,

I have a a pair of Jetway motherboards with an add-on 3Gbit LAN modules
and have been doing some testing for a linux router project.  I found
that I while I can get LACP working properly using eth0 and eth1, using
the exact same setup with any of the other three lan fails.  Is this a
known issue?

This is on Linux 2.6.32.10.  Here is a blurb of dmesg showing the NICs
detected.

eth0: RTL8168b/8111b at 0xf8adc000, 00:30:18:ac:a6:80, XID 0c200000 IRQ 24
eth1: RTL8168b/8111b at 0xf8ae0000, 00:30:18:ac:a6:81, XID 0c200000 IRQ 25
eth2: RTL8169sc/8110sc at 0xf8ae4c00, 00:30:18:ae:34:3a, XID 18000000 IRQ 18
eth3: RTL8169sc/8110sc at 0xf8ae8800, 00:30:18:ae:34:3b, XID 18000000 IRQ 19
eth4: RTL8169sc/8110sc at 0xf8aec400, 00:30:18:ae:34:3c, XID 18000000 IRQ 16

I can probably manage this project using only eth0 and eth1 in LACP
configuration but I figured I'd give a heads up.

The interfaces seem able to detect when the link is up or down, bonding
removes and adds them to the LACP trunk but I can't get traffic through
them.

Kind regards,

Jonathan

^ permalink raw reply

* profile my proto_hook.function
From: zhou rui @ 2011-04-12 14:00 UTC (permalink / raw)
  To: netdev

hi
I used a prot_hook(added via dev_add_pack,protocol=ETH_P_ALL) to
process packets in kernel part.but the performance was bad

ksoftirqd used almost 100% cpu on my 1G nic(intel e1000,w/o RSS) XEON
5400 hypertown machine.

looks my  packet processing is too slow. but how to profile this?
i.e. how many CPU resources used in softirq and normal kernel protocol
stack(tcp/ip,etc)
and how many used in my packet processing function?

and any idea to improve it?how about moving the processing function to driver?

thanks
rui

^ permalink raw reply

* Re: profile my proto_hook.function
From: Eric Dumazet @ 2011-04-12 14:03 UTC (permalink / raw)
  To: zhou rui; +Cc: netdev
In-Reply-To: <BANLkTi==HczThr66Yr0-Rm-RkEUw44sqrg@mail.gmail.com>

Le mardi 12 avril 2011 à 22:00 +0800, zhou rui a écrit :
> hi
> I used a prot_hook(added via dev_add_pack,protocol=ETH_P_ALL) to
> process packets in kernel part.but the performance was bad
> 
> ksoftirqd used almost 100% cpu on my 1G nic(intel e1000,w/o RSS) XEON
> 5400 hypertown machine.
> 
> looks my  packet processing is too slow. but how to profile this?
> i.e. how many CPU resources used in softirq and normal kernel protocol
> stack(tcp/ip,etc)
> and how many used in my packet processing function?
> 
> and any idea to improve it?how about moving the processing function to driver?

You coud use perf tool, included in kernel sources

# cd ~your_kernel_sources
# cd tools/perf
# make

<run your workload, and record activity :>
# ./perf record -a -g sleep 10

Then later, you can take a look at results

./perf report




^ permalink raw reply

* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Michał Mirosław @ 2011-04-12 14:07 UTC (permalink / raw)
  To: Vladislav Zolotarov; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <1302610228.32697.298.camel@lb-tlvb-vladz>

On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > Since ndo_fix_features callback is postponing features change when
> > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > has to be called again when this condition changes. Previously,
> > ethtool_ops->set_flags callback returned -EBUSY in that case
> > (it's not possible in the new model).
> ACK (with reservations). ;)
> Could u, pls., just add this comment I've asked for in the previous
> e-mail? 

The one about vlan_features? I'll just fix the comment in netdevice.h
instead, since it might be not clear enough.

> The things I first thought to comment on are:
> 	- Removing TPA_ENABLED_FLAG the similar way u've removed the
> bp->rx_csum.
> 	- Merging the code handling 'features' in bnx2x_init_bp() with the
> similar code in bnx2x_init_dev().
> 
> However I think it would be right if we clear our mess by ourselves and
> that u have already done much enough... ;) 
> 
> I've run our standard test suite (which in particular heavily tests the
> RX_CSUM and LRO flags toggling) on this patch and it passed it.

It might be possible to get rid of ndo_set_features, since it looks like
the reset/recovery handler is doing unload/load itself. This needs more
digging into the driver than this simple conversion.

Best Regards,
Michał Mirosław

^ permalink raw reply

* [PATCH] net: vlan_features comment clarification
From: Michał Mirosław @ 2011-04-12 14:07 UTC (permalink / raw)
  To: netdev

Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
 include/linux/netdevice.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 09d2624..cb8178a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1035,7 +1035,7 @@ struct net_device {
 	u32			hw_features;
 	/* user-requested features */
 	u32			wanted_features;
-	/* VLAN feature mask */
+	/* mask of features inheritable by VLAN devices */
 	u32			vlan_features;
 
 	/* Net device feature bits; if you change something,
-- 
1.7.2.5


^ permalink raw reply related

* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 14:19 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302614145.3233.47.camel@edumazet-laptop>

On Tue, 2011-04-12 at 15:15 +0200, Eric Dumazet wrote: 
> Le mardi 12 avril 2011 à 15:02 +0200, Jan Lübbe a écrit :
> > Here you check dopt->optlen, which certainly should be 40 at most. The
> > calculation of dopt->optlen wasn't changed by my patch, though.
> 
> Check again the thread Jan.
> 
> Scot is using a tool (IP Stack Checker's tcpsic) to forge random tcp
> packets.

> Maybe your patch is fine but requires a change in a previous function,
> to make sure we deny some crazy packet before generating an ip_options
> with more than 40 bytes, in an icmp_send() reply.

One thing which could expose a problem is that it now will timestamp the
packet in the last 'slot', too. (which it didn't before)

In general, there is not a lot of error-checking in the options stuff.

> I took a look at this ip_options stuff and must say its really hard to
> even _read_ the code. Understanding it might need several days or a new
> brain ?

It took me some days do even figure out how it is supposed to fit
together...

> I cannot Ack or Nack your patch, I must admit it. Isnt it frightening?

David Miller already declared this code as 'officially terrible'...

Your patch should catch those forged packets before more harmful things
can go wrong, but even before my patch, i think forged packets could
cause trouble...

Regards,
Jan


^ permalink raw reply

* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Vladislav Zolotarov @ 2011-04-12 14:36 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <20110412140708.GA21835@rere.qmqm.pl>

On Tue, 2011-04-12 at 07:07 -0700, Michał Mirosław wrote:
> On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> > On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > > Since ndo_fix_features callback is postponing features change when
> > > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > > has to be called again when this condition changes. Previously,
> > > ethtool_ops->set_flags callback returned -EBUSY in that case
> > > (it's not possible in the new model).
> > ACK (with reservations). ;)
> > Could u, pls., just add this comment I've asked for in the previous
> > e-mail? 
> 
> The one about vlan_features? 

Yeah, this one.

> I'll just fix the comment in netdevice.h
> instead, since it might be not clear enough.

Could u still add this comment to bnx2x in your patch as well. It'll
just make these code section more clear regardless the netdevice.h
contents... ;)



> 
> > The things I first thought to comment on are:
> > 	- Removing TPA_ENABLED_FLAG the similar way u've removed the
> > bp->rx_csum.
> > 	- Merging the code handling 'features' in bnx2x_init_bp() with the
> > similar code in bnx2x_init_dev().
> > 
> > However I think it would be right if we clear our mess by ourselves and
> > that u have already done much enough... ;) 
> > 
> > I've run our standard test suite (which in particular heavily tests the
> > RX_CSUM and LRO flags toggling) on this patch and it passed it.
> 
> It might be possible to get rid of ndo_set_features, since it looks like
> the reset/recovery handler is doing unload/load itself. This needs more
> digging into the driver than this simple conversion.

Hmm... I don't get u here. Although the recovery handler does
unload/load itself if there has been an attempt to change features
during the recovery it won't be able to get applied until the whole
recovery process ends. So, this patch added the extra call for
netdev_update_features() right after we know that the recovery process
has successfully ended. So, could u, pls., explain exactly what do u
mean here by saying "It might be possible to get rid of
ndo_set_features"?

thanks,
vlad

> 
> Best Regards,
> Michał Mirosław
> 




^ permalink raw reply

* Re: Kernel panic when using bridge
From: Eric Dumazet @ 2011-04-12 14:49 UTC (permalink / raw)
  To: Jan Lübbe; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302617968.30934.34.camel@polaris.local>

Le mardi 12 avril 2011 à 16:19 +0200, Jan Lübbe a écrit :

> Your patch should catch those forged packets before more harmful things
> can go wrong, but even before my patch, i think forged packets could
> cause trouble...

Well, this is a debugging aid and wont avoid a crash later (since we
already made an out of bounds write, generally on a stack slot)

Of course, this might be a complete shot in the dark, but a
stackprotector fault in icmp_send() really sounds like a problem in
ip_options_echo() [ or bad input data given to this function ]


Other related changes (but as old as v2.6.22) :

commit 11a03f78fbf15a866ba
([NetLabel]: core network changes)




^ permalink raw reply

* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Michał Mirosław @ 2011-04-12 14:49 UTC (permalink / raw)
  To: Vladislav Zolotarov; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <1302619012.6750.8.camel@lb-tlvb-vladz>

On Tue, Apr 12, 2011 at 05:36:52PM +0300, Vladislav Zolotarov wrote:
> On Tue, 2011-04-12 at 07:07 -0700, Michał Mirosław wrote:
> > On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> > > On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > > > Since ndo_fix_features callback is postponing features change when
> > > > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > > > has to be called again when this condition changes. Previously,
> > > > ethtool_ops->set_flags callback returned -EBUSY in that case
> > > > (it's not possible in the new model).
> > > ACK (with reservations). ;)
> > > Could u, pls., just add this comment I've asked for in the previous
> > > e-mail? 
> > The one about vlan_features? 
> Yeah, this one.
> > I'll just fix the comment in netdevice.h
> > instead, since it might be not clear enough.
> Could u still add this comment to bnx2x in your patch as well. It'll
> just make these code section more clear regardless the netdevice.h
> contents... ;)

This would be duplicating information easily found elsewhere.
I don't like it. :)

> > > The things I first thought to comment on are:
> > > 	- Removing TPA_ENABLED_FLAG the similar way u've removed the
> > > bp->rx_csum.
> > > 	- Merging the code handling 'features' in bnx2x_init_bp() with the
> > > similar code in bnx2x_init_dev().
> > > 
> > > However I think it would be right if we clear our mess by ourselves and
> > > that u have already done much enough... ;) 
> > > 
> > > I've run our standard test suite (which in particular heavily tests the
> > > RX_CSUM and LRO flags toggling) on this patch and it passed it.
> > It might be possible to get rid of ndo_set_features, since it looks like
> > the reset/recovery handler is doing unload/load itself. This needs more
> > digging into the driver than this simple conversion.
> Hmm... I don't get u here. Although the recovery handler does
> unload/load itself if there has been an attempt to change features
> during the recovery it won't be able to get applied until the whole
> recovery process ends. So, this patch added the extra call for
> netdev_update_features() right after we know that the recovery process
> has successfully ended. So, could u, pls., explain exactly what do u
> mean here by saying "It might be possible to get rid of
> ndo_set_features"?

Hmm. I thought one, and wrote another.

Since bnx2x_parity_recover() runs with rtnl_lock(), as should
netdev_update_features(), then in case the recovery is in progress
it should be enough to not call bnx2x_reload_if_running() then
and just change the flags --- changes will be picked up on bnx2x_nic_load()
after recovery is complete. This removes the need for netdev_update_features()
calls.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] inetpeer: reduce stack usage
From: Hiroaki SHIMODA @ 2011-04-12 14:51 UTC (permalink / raw)
  To: Eric Dumazet, David Miller; +Cc: Scot Doyle, Stephen Hemminger, netdev
In-Reply-To: <1302597580.3233.14.camel@edumazet-laptop>

On Tue, 12 Apr 2011 10:39:40 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On 64bit arches, we use 752 bytes of stack when cleanup_once() is called
> from inet_getpeer().
> 
> Lets share the avl stack to save ~376 bytes.
> 
> Before patch :
> 
> # objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl
> 
> 0x000006c3 unlink_from_pool [inetpeer.o]:		376
> 0x00000721 unlink_from_pool [inetpeer.o]:		376
> 0x00000cb1 inet_getpeer [inetpeer.o]:			376
> 0x00000e6d inet_getpeer [inetpeer.o]:			376
> 0x0004 inet_initpeers [inetpeer.o]:			112
> # size net/ipv4/inetpeer.o
>    text	   data	    bss	    dec	    hex	filename
>    5320	    432	     21	   5773	   168d	net/ipv4/inetpeer.o
> 
> After patch :
> 
> objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl 
> 0x00000c11 inet_getpeer [inetpeer.o]:			376
> 0x00000dcd inet_getpeer [inetpeer.o]:			376
> 0x00000ab9 peer_check_expire [inetpeer.o]:		328
> 0x00000b7f peer_check_expire [inetpeer.o]:		328
> 0x0004 inet_initpeers [inetpeer.o]:			112
> # size net/ipv4/inetpeer.o
>    text	   data	    bss	    dec	    hex	filename
>    5163	    432	     21	   5616	   15f0	net/ipv4/inetpeer.o
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Scot Doyle <lkml@scotdoyle.com>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>

I couldn't understand that actually cleanup_once() was called
from inet_getpeer() and then the stack overflow was hit,
but this patch surely reduces stack usage.

Reviewed-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>

Thanks.

^ permalink raw reply

* Re: [PATCH] inetpeer: reduce stack usage
From: Eric Dumazet @ 2011-04-12 14:55 UTC (permalink / raw)
  To: Hiroaki SHIMODA; +Cc: David Miller, Scot Doyle, Stephen Hemminger, netdev
In-Reply-To: <20110412235126.63841dfe.shimoda.hiroaki@gmail.com>

Le mardi 12 avril 2011 à 23:51 +0900, Hiroaki SHIMODA a écrit :

> I couldn't understand that actually cleanup_once() was called
> from inet_getpeer() and then the stack overflow was hit,
> but this patch surely reduces stack usage.
> 
> Reviewed-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
> 

Well, I dont believe we actually hit a stack overflow in Scot Doyle
reported crashes, but it certainly is better to use a bit less stack
anyway ;)

Thanks for reviewing !



^ permalink raw reply

* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 15:13 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302619749.3233.56.camel@edumazet-laptop>

On Tue, 2011-04-12 at 16:49 +0200, Eric Dumazet wrote: 
> Of course, this might be a complete shot in the dark, but a
> stackprotector fault in icmp_send() really sounds like a problem in
> ip_options_echo() [ or bad input data given to this function ]

It was my understanding that all IP options given to ip_options_echo are
either from local sources or have gone through ip_options_compile, which
seems to verify that the sum of the individual option lengths do not
exceed the ip header. So there wouldn't need to be additional checks in
ip_options_echo.

If this is not the case, we need size checks in ip_options_echo before
copying over each option.

> Other related changes (but as old as v2.6.22) :
> 
> commit 11a03f78fbf15a866ba
> ([NetLabel]: core network changes)

When investigating the problem I had with timestamps, i found that most
of the lines in ip_options_echo and _compile have not been changed since
before 2.2 (some even before 2.0). The newer changes have all been
updates for changed API elsewhere in the stack.

Regards,
Jan


^ permalink raw reply

* Re: [RFC] iproute2: Fix meta match u32 with 0xffffffff
From: Stephen Hemminger @ 2011-04-12 15:19 UTC (permalink / raw)
  To: tgraf; +Cc: netdev
In-Reply-To: <1302595009.3664.8.camel@lsx>

On Tue, 12 Apr 2011 09:56:49 +0200
Thomas Graf <tgraf@redhat.com> wrote:

> On Mon, 2011-04-11 at 11:52 -0700, Stephen Hemminger wrote: 
> > The value 0xffffffff is a valid mask and bstrtoul() would return
> > ULONG_MAX which was the error value. Resolve the problem by separating
> > return value and error indication.
> >  
> > -unsigned long bstrtoul(const struct bstr *b)
> > +int bstrtoul(const struct bstr *b, unsigned long *lp)
> >  {
> >  	char *inv = NULL;
> > -	unsigned long l;
> >  	char buf[b->len+1];
> >  
> > +	if (b->len == 0)
> > +		return -EINVAL;
> > +
> >  	memcpy(buf, b->data, b->len);
> >  	buf[b->len] = '\0';
> >  
> > -	l = strtoul(buf, &inv, 0);
> > -	if (l == ULONG_MAX || inv == buf)
> > -		return ULONG_MAX;
> > +	*lp = strtoul(buf, &inv, 0);
> > +	if (inv == buf)
> > +		return -EINVAL;
> > +
> > +	if (*lp == ULONG_MAX || errno == ERANGE)
> > +		return -ERANGE;
> >  
> > -	return l;
> > +	return 0;
> >  }
> 
> This is definitely much better but we still can't parse ULONG_MAX
> as string representative. Checking glibc docs, the only way to do it is
> to ignore the return value for error checking and look errno.
> 

I think the error case is ret == ULONG_MAX && errno == ERANGE
If there is no error, then strtoul doesn't set errno.



-- 

^ permalink raw reply

* [PATCH 1/2] net/sis900: store MAC into perm_addr for SiS 900, 630E, 635 and 96x variants
From: Otavio Salvador @ 2011-04-12 15:30 UTC (permalink / raw)
  To: netdev; +Cc: Otavio Salvador

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 drivers/net/sis900.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/net/sis900.c b/drivers/net/sis900.c
index cb317cd..484f795 100644
--- a/drivers/net/sis900.c
+++ b/drivers/net/sis900.c
@@ -240,7 +240,8 @@ static const struct ethtool_ops sis900_ethtool_ops;
  *	@net_dev: the net device to get address for
  *
  *	Older SiS900 and friends, use EEPROM to store MAC address.
- *	MAC address is read from read_eeprom() into @net_dev->dev_addr.
+ *	MAC address is read from read_eeprom() into @net_dev->dev_addr and
+ *	@net_dev->perm_addr.
  */
 
 static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_device *net_dev)
@@ -261,6 +262,9 @@ static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_de
 	for (i = 0; i < 3; i++)
 	        ((u16 *)(net_dev->dev_addr))[i] = read_eeprom(ioaddr, i+EEPROMMACAddr);
 
+	/* Store MAC Address in perm_addr */
+	memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
 	return 1;
 }
 
@@ -271,7 +275,8 @@ static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_de
  *
  *	SiS630E model, use APC CMOS RAM to store MAC address.
  *	APC CMOS RAM is accessed through ISA bridge.
- *	MAC address is read into @net_dev->dev_addr.
+ *	MAC address is read into @net_dev->dev_addr and
+ *	@net_dev->perm_addr.
  */
 
 static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
@@ -296,6 +301,10 @@ static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
 		outb(0x09 + i, 0x70);
 		((u8 *)(net_dev->dev_addr))[i] = inb(0x71);
 	}
+
+	/* Store MAC Address in perm_addr */
+	memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
 	pci_write_config_byte(isa_bridge, 0x48, reg & ~0x40);
 	pci_dev_put(isa_bridge);
 
@@ -310,7 +319,7 @@ static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
  *
  *	SiS635 model, set MAC Reload Bit to load Mac address from APC
  *	to rfdr. rfdr is accessed through rfcr. MAC address is read into
- *	@net_dev->dev_addr.
+ *	@net_dev->dev_addr and @net_dev->perm_addr.
  */
 
 static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
@@ -334,6 +343,9 @@ static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
 		*( ((u16 *)net_dev->dev_addr) + i) = inw(ioaddr + rfdr);
 	}
 
+	/* Store MAC Address in perm_addr */
+	memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
 	/* enable packet filtering */
 	outl(rfcrSave | RFEN, rfcr + ioaddr);
 
@@ -353,7 +365,7 @@ static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
  *	EEDONE signal to refuse EEPROM access by LAN.
  *	The EEPROM map of SiS962 or SiS963 is different to SiS900.
  *	The signature field in SiS962 or SiS963 spec is meaningless.
- *	MAC address is read into @net_dev->dev_addr.
+ *	MAC address is read into @net_dev->dev_addr and @net_dev->perm_addr.
  */
 
 static int __devinit sis96x_get_mac_addr(struct pci_dev * pci_dev,
@@ -372,6 +384,9 @@ static int __devinit sis96x_get_mac_addr(struct pci_dev * pci_dev,
 			for (i = 0; i < 3; i++)
 			        ((u16 *)(net_dev->dev_addr))[i] = read_eeprom(ioaddr, i+EEPROMMACAddr);
 
+			/* Store MAC Address in perm_addr */
+			memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
 			outl(EEDONE, ee_addr);
 			return 1;
 		} else {
-- 
1.7.4.1


^ permalink raw reply related

* [PATCH 2/2] net/natsami: store MAC into perm_addr
From: Otavio Salvador @ 2011-04-12 15:30 UTC (permalink / raw)
  To: netdev; +Cc: Otavio Salvador
In-Reply-To: <1302622241-11871-1-git-send-email-otavio@ossystems.com.br>

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 drivers/net/natsemi.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index aa2813e..1074231 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -860,6 +860,9 @@ static int __devinit natsemi_probe1 (struct pci_dev *pdev,
 		prev_eedata = eedata;
 	}
 
+	/* Store MAC Address in perm_addr */
+	memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
+
 	dev->base_addr = (unsigned long __force) ioaddr;
 	dev->irq = irq;
 
-- 
1.7.4.1


^ permalink raw reply related


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