Netdev List
 help / color / mirror / Atom feed
* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface, PERCPU:  allocation failed
From: Eric Dumazet @ 2011-05-19  6:39 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: netdev
In-Reply-To: <f9797bb034f650a24e927629d1ab77d8@visp.net.lb>

Le jeudi 19 mai 2011 à 09:35 +0300, Denys Fedoryshchenko a écrit :
> Hi, again
> 
>  Just tried to upgrade large NAS from 2.6.38.6 to 2.6.39-rc7-git11, and 
>  at same time enabling ipv6 on it.
>  Got that, after ppp2897 brought up (sure it means there is other 2896 
>  available, and also few ethernet vlans, around 32).
>  I am not sure it is a bug, but it looks i had free memory(the box had 
>  8GB free), and lowmem too, also i will try to enable there 64bit kernel 
>  at evening.
> 
>  May 17 16:00:42 194.146.155.70 kernel: [14925.897799] PERCPU: 
>  allocation failed, size=2048 align=4, failed to allocate new chunk
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898163] Pid: 24207, comm: 
>  pppd Not tainted 2.6.39-rc7-git11-build-0058 #4
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898164] Call Trace:
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898169]  [<c0335548>] ? 
>  printk+0x18/0x20
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898173]  [<c017ecd0>] 
>  pcpu_alloc+0x616/0x67a
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898176]  [<c0194a80>] ? 
>  __kmalloc_track_caller+0x68/0xc0
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898189]  [<f8ae196c>] ? 
>  kzalloc+0xb/0xd [ipv6]
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898193]  [<c01320a5>] ? 
>  _local_bh_enable_ip.clone.6+0x18/0x71
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898195]  [<c017ed3e>] 
>  __alloc_percpu+0xa/0xc
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898198]  [<c030aa7d>] 
>  snmp_mib_init+0x2f/0x51
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898207]  [<f8ae2ad0>] 
>  ipv6_add_dev+0x133/0x2a3 [ipv6]
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898209]  [<c030e12d>] ? 
>  ip_mc_init_dev+0x75/0x86
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898211]  [<c0309321>] ? 
>  devinet_sysctl_register+0x34/0x38
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898221]  [<f8ae5754>] 
>  addrconf_notify+0x50/0x6a5 [ipv6]
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898224]  [<c0218f52>] ? 
>  add_uevent_var+0xa3/0xa3
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898226]  [<c0309901>] ? 
>  inetdev_event+0x55/0x3c0
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898230]  [<c01446f9>] 
>  notifier_call_chain+0x26/0x48
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898232]  [<c01447a7>] 
>  raw_notifier_call_chain+0x1a/0x1c
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898236]  [<c02c8115>] 
>  call_netdevice_notifiers+0x44/0x4b
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898238]  [<c01320a5>] ? 
>  _local_bh_enable_ip.clone.6+0x18/0x71
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898240]  [<c0132106>] ? 
>  local_bh_enable_ip+0x8/0xa
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898242]  [<c02ca19b>] 
>  register_netdevice+0x1fb/0x255
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898244]  [<c02ca227>] 
>  register_netdev+0x32/0x41
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898247]  [<c021d5cf>] ? 
>  sprintf+0x1c/0x1e
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898249]  [<c029647a>] 
>  ppp_ioctl+0x224/0xaea
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898252]  [<c01a35cc>] ? 
>  do_filp_open+0x26/0x67
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898254]  [<c0296256>] ? 
>  ppp_write+0x98/0x98
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898256]  [<c01a53ce>] 
>  do_vfs_ioctl+0x45e/0x498
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898258]  [<c01a118e>] ? 
>  getname_flags+0x1e/0xad
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898260]  [<c019391b>] ? 
>  kmem_cache_free+0x14/0x83
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898262]  [<c01ab5bb>] ? 
>  alloc_fd+0x4e/0xba
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898265]  [<c0199465>] ? 
>  do_sys_open+0xdb/0xe5
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898266]  [<c019ac7b>] ? 
>  fput+0x13/0x155
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898268]  [<c01a4387>] ? 
>  do_fcntl+0x227/0x3aa
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898270]  [<c01a543b>] 
>  sys_ioctl+0x33/0x4c
>  May 17 16:00:42 194.146.155.70 kernel: [14925.898273]  [<c0336edd>] 
>  syscall_call+0x7/0xb
> --

Its a known problem : When ipv6 is enabled, we allocate percpu memory to
hold per device snmp counters.

make sure kernel idea of max possible cpus matches real number of cpus.

And yes, switching to 64bit kernel helps a lot.




^ permalink raw reply

* Re: [PATCH] tcp: Implement a two-level initial RTO as per draft RFC 2988bis-02.
From: tsuna @ 2011-05-19  6:42 UTC (permalink / raw)
  To: Alexander Zimmermann
  Cc: David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, hagen,
	eric.dumazet, netdev, linux-kernel
In-Reply-To: <C70B920C-6176-481B-B6D2-E52769291EBC@comsys.rwth-aachen.de>

On Wed, May 18, 2011 at 11:36 PM, Alexander Zimmermann
<alexander.zimmermann@comsys.rwth-aachen.de> wrote:
> If you set the initRTO=0.1s, it's good for me but bad for the rest of the
> world. That's the difference.
>
> Or do you want to implement a lower barrier of 1sec so that you can ensure
> that nobody set the initRTO lower than 1s?

Oh, I see.  Yes, there is a lower bound (and an upper bound) on what
values the kernel will accept as initRTO.  In the patch "Implement a
two-level initial RTO as per draft RFC 2988bis-02" above, I re-used
TCP_RTO_MIN and TCP_RTO_MAX in net/ipv4/sysctl_net_ipv4.c in order to
prevent users from setting a minRTO that's outside this range.  They
are defined as follows in tcp.h:

#define TCP_RTO_MAX     ((unsigned)(120*HZ))
#define TCP_RTO_MIN     ((unsigned)(HZ/5))

So we're talking about a [200ms ; 120s] range no matter what.

-- 
Benoit "tsuna" Sigoure
Software Engineer @ www.StumbleUpon.com

^ permalink raw reply

* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface,  PERCPU:  allocation failed
From: Denys Fedoryshchenko @ 2011-05-19  6:47 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1305787158.3019.12.camel@edumazet-laptop>

 On Thu, 19 May 2011 08:39:18 +0200, Eric Dumazet wrote:
> Le jeudi 19 mai 2011 à 09:35 +0300, Denys Fedoryshchenko a écrit :
>> Hi, again
>>
>>  Just tried to upgrade large NAS from 2.6.38.6 to 2.6.39-rc7-git11, 
>> and
>>  at same time enabling ipv6 on it.
>>  Got that, after ppp2897 brought up (sure it means there is other 
>> 2896
>>  available, and also few ethernet vlans, around 32).
>>  I am not sure it is a bug, but it looks i had free memory(the box 
>> had
>>  8GB free), and lowmem too, also i will try to enable there 64bit 
>> kernel
>>  at evening.
>>
>>  May 17 16:00:42 194.146.155.70 kernel: [14925.897799] PERCPU:
>>  allocation failed, size=2048 align=4, failed to allocate new chunk
>>  May 17 16:00:42 194.146.155.70 kernel: [14925.898163] Pid: 24207, 
>> comm:
>>  pppd Not tainted 2.6.39-rc7-git11-build-0058 #4
>
> Its a known problem : When ipv6 is enabled, we allocate percpu memory 
> to
> hold per device snmp counters.
>
> make sure kernel idea of max possible cpus matches real number of 
> cpus.
>
> And yes, switching to 64bit kernel helps a lot.
>
 Yes, it matches, i guess.
 CONFIG_NR_CPUS=8

 processor       : 7
 vendor_id       : GenuineIntel
 cpu family      : 6
 model           : 26
 model name      : Intel(R) Core(TM) i7 CPU         950  @ 3.07GHz

 Thanks. Then i will simply switch kernel to 64bit, but for now with 
 32bit userspace, since this semi-embedded system
 mass deployed, and i have to maintain it alone (cannot handle both 
 32/64 bit userspace), and some pc's don't have
 lm flag in cpuinfo :)

 I am hitting a lot lowmem limits lately, but the only application that 
 was not working right 32bit userspace/64bit kernel - ipvsadm.
 Should i report it as a bug (i will check if it is still an issue)?


^ permalink raw reply

* [PATCH] tcp: Lower the initial RTO to 1s as per draft RFC 2988bis-02.
From: Benoit Sigoure @ 2011-05-19  6:47 UTC (permalink / raw)
  To: davem, kuznet, pekkas, jmorris, yoshfuji, kaber, hagen,
	eric.dumazet, alexander.zimmermann
  Cc: netdev, linux-kernel, Benoit Sigoure
In-Reply-To: <20110519.014656.1735519603194773578.davem@davemloft.net>

Draft RFC 2988bis-02 recommends that the initial RTO be lowered
from 3 seconds down to 1 second, and that in case of a timeout
during the TCP 3WHS, the RTO should fallback to 3 seconds when
data transmission begins.

Signed-off-by: Benoit Sigoure <tsunanet@gmail.com>
---

Apologies for the spam, I sent this patch from the wrong address and without
sob'ing it.  I build the Linux kernel in a 15G tmpfs (it's faster this way :D)
and I lost my .git/config after a reboot.

 include/net/tcp.h    |    5 ++++-
 net/ipv4/tcp_input.c |   13 +++++++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index cda30ea..274d761 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -122,7 +122,10 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
 #endif
 #define TCP_RTO_MAX	((unsigned)(120*HZ))
 #define TCP_RTO_MIN	((unsigned)(HZ/5))
-#define TCP_TIMEOUT_INIT ((unsigned)(3*HZ))	/* RFC 1122 initial RTO value	*/
+/* The next 2 values come from Draft RFC 2988bis-02. */
+#define TCP_TIMEOUT_INIT ((unsigned)(1*HZ))		/* initial RTO value	*/
+#define TCP_TIMEOUT_INIT_FALLBACK ((unsigned)(3*HZ))	/* initial RTO to fallback to when
+							 * a timeout happens during the 3WHS.	*/
 
 #define TCP_RESOURCE_PROBE_INTERVAL ((unsigned)(HZ/2U)) /* Maximal interval between probes
 					                 * for local resources.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bef9f04..a36bc35 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -868,6 +868,11 @@ static void tcp_init_metrics(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
 	struct dst_entry *dst = __sk_dst_get(sk);
+	/* If we had to retransmit anything during the 3WHS, use
+	 * the initial fallback RTO as per draft RFC 2988bis-02.
+	 */
+	int init_rto = inet_csk(sk)->icsk_retransmits ?
+		TCP_TIMEOUT_INIT_FALLBACK : TCP_TIMEOUT_INIT;
 
 	if (dst == NULL)
 		goto reset;
@@ -890,7 +895,7 @@ static void tcp_init_metrics(struct sock *sk)
 	if (dst_metric(dst, RTAX_RTT) == 0)
 		goto reset;
 
-	if (!tp->srtt && dst_metric_rtt(dst, RTAX_RTT) < (TCP_TIMEOUT_INIT << 3))
+	if (!tp->srtt && dst_metric_rtt(dst, RTAX_RTT) < (init_rto << 3))
 		goto reset;
 
 	/* Initial rtt is determined from SYN,SYN-ACK.
@@ -916,7 +921,7 @@ static void tcp_init_metrics(struct sock *sk)
 		tp->mdev_max = tp->rttvar = max(tp->mdev, tcp_rto_min(sk));
 	}
 	tcp_set_rto(sk);
-	if (inet_csk(sk)->icsk_rto < TCP_TIMEOUT_INIT && !tp->rx_opt.saw_tstamp) {
+	if (inet_csk(sk)->icsk_rto < init_rto && !tp->rx_opt.saw_tstamp) {
 reset:
 		/* Play conservative. If timestamps are not
 		 * supported, TCP will fail to recalculate correct
@@ -924,8 +929,8 @@ reset:
 		 */
 		if (!tp->rx_opt.saw_tstamp && tp->srtt) {
 			tp->srtt = 0;
-			tp->mdev = tp->mdev_max = tp->rttvar = TCP_TIMEOUT_INIT;
-			inet_csk(sk)->icsk_rto = TCP_TIMEOUT_INIT;
+			tp->mdev = tp->mdev_max = tp->rttvar = init_rto;
+			inet_csk(sk)->icsk_rto = init_rto;
 		}
 	}
 	tp->snd_cwnd = tcp_init_cwnd(tp, dst);
-- 
1.7.0.4

^ permalink raw reply related

* Re: [PATCH] tcp: Implement a two-level initial RTO as per draft RFC 2988bis-02.
From: Alexander Zimmermann @ 2011-05-19  6:52 UTC (permalink / raw)
  To: tsuna
  Cc: David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, hagen,
	eric.dumazet, netdev, linux-kernel
In-Reply-To: <BANLkTiku9KTHCm59cib5KY8mz0ewSLsHFQ@mail.gmail.com>

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


Am 19.05.2011 um 08:42 schrieb tsuna:

> On Wed, May 18, 2011 at 11:36 PM, Alexander Zimmermann
> <alexander.zimmermann@comsys.rwth-aachen.de> wrote:
>> If you set the initRTO=0.1s, it's good for me but bad for the rest of the
>> world. That's the difference.
>> 
>> Or do you want to implement a lower barrier of 1sec so that you can ensure
>> that nobody set the initRTO lower than 1s?
> 
> Oh, I see.  Yes, there is a lower bound (and an upper bound) on what
> values the kernel will accept as initRTO.  In the patch "Implement a
> two-level initial RTO as per draft RFC 2988bis-02" above, I re-used
> TCP_RTO_MIN and TCP_RTO_MAX in net/ipv4/sysctl_net_ipv4.c in order to
> prevent users from setting a minRTO that's outside this range.  They
> are defined as follows in tcp.h:
> 
> #define TCP_RTO_MAX     ((unsigned)(120*HZ))
> #define TCP_RTO_MIN     ((unsigned)(HZ/5))
> 
> So we're talking about a [200ms ; 120s] range no matter what.

Why is 200ms a valid lower bound for initRTO? I'm aware of
measurements that 1s is save for Internet, but I don't know of any
studies that 200ms is save... 

> 
> -- 
> Benoit "tsuna" Sigoure
> Software Engineer @ www.StumbleUpon.com

//
// Dipl.-Inform. Alexander Zimmermann
// Department of Computer Science, Informatik 4
// RWTH Aachen University
// Ahornstr. 55, 52056 Aachen, Germany
// phone: (49-241) 80-21422, fax: (49-241) 80-22222
// email: zimmermann@cs.rwth-aachen.de
// web: http://www.umic-mesh.net
//


[-- Attachment #2: Signierter Teil der Nachricht --]
[-- Type: application/pgp-signature, Size: 243 bytes --]

^ permalink raw reply

* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface, PERCPU:  allocation failed
From: Eric Dumazet @ 2011-05-19  6:55 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: netdev
In-Reply-To: <1305787158.3019.12.camel@edumazet-laptop>

Le jeudi 19 mai 2011 à 08:39 +0200, Eric Dumazet a écrit :

> Its a known problem : When ipv6 is enabled, we allocate percpu memory to
> hold per device snmp counters.
> 
> make sure kernel idea of max possible cpus matches real number of cpus.
> 
> And yes, switching to 64bit kernel helps a lot.
> 
> 

Looking at snmp6_alloc_dev(), we allocate three mib per device :

ipstats_mib  (30 * sizeof(u64) * number_of_possible_cpus)
icmpv6_mib    (4 * sizeof(long) * number_of_possible_cpus)
icmpv6msg_mib  (26 * sizeof(long))

For sure icmp ones dont need percpu counter. Plain atomic_long_t
(shared) would be enough, since ICMP messages are rare enough.




^ permalink raw reply

* Re: [PATCH] tcp: Implement a two-level initial RTO as per draft RFC 2988bis-02.
From: tsuna @ 2011-05-19  7:07 UTC (permalink / raw)
  To: Alexander Zimmermann
  Cc: David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber, hagen,
	eric.dumazet, netdev, linux-kernel
In-Reply-To: <8C5DF277-320D-4DEB-A133-EEC301DE58DC@comsys.rwth-aachen.de>

On Wed, May 18, 2011 at 11:52 PM, Alexander Zimmermann
<alexander.zimmermann@comsys.rwth-aachen.de> wrote:
>> So we're talking about a [200ms ; 120s] range no matter what.
>
> Why is 200ms a valid lower bound for initRTO? I'm aware of
> measurements that 1s is save for Internet, but I don't know of any
> studies that 200ms is save...

The constants that are quoted aren't specific to the initRTO.  They're
used to bound the RTO as it gets adjusted during the TCP session.  See
`tcp_set_rto' in tcp_input.c for reference.

-- 
Benoit "tsuna" Sigoure
Software Engineer @ www.StumbleUpon.com

^ permalink raw reply

* [PATCH 1/2] net: ping: make local functions static
From: Changli Gao @ 2011-05-19  7:16 UTC (permalink / raw)
  To: David S. Miller
  Cc: Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, Changli Gao

As these functions are only used in this file.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/ipv4/ping.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 6a21da9..5f9e2d1 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -449,8 +449,8 @@ static int ping_push_pending_frames(struct sock *sk, struct pingfakehdr *pfh, st
 	return ip_push_pending_frames(sk, fl4);
 }
 
-int ping_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
-		 size_t len)
+static int ping_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
+			size_t len)
 {
 	struct net *net = sock_net(sk);
 	struct flowi4 fl4;
@@ -621,8 +621,8 @@ do_confirm:
 	goto out;
 }
 
-int ping_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
-		 size_t len, int noblock, int flags, int *addr_len)
+static int ping_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
+			size_t len, int noblock, int flags, int *addr_len)
 {
 	struct inet_sock *isk = inet_sk(sk);
 	struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;

^ permalink raw reply related

* [PATCH 2/2] net: ping: fix the coding style
From: Changli Gao @ 2011-05-19  7:16 UTC (permalink / raw)
  To: David S. Miller
  Cc: Alexey Kuznetsov, Pekka Savola (ipv6), James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, Changli Gao
In-Reply-To: <1305789361-5366-1-git-send-email-xiaosuo@gmail.com>

The characters in a line should be no more than 80.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/ipv4/ping.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 5f9e2d1..1f3bb11 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -187,7 +187,8 @@ exit:
 	return sk;
 }
 
-static void inet_get_ping_group_range_net(struct net *net, gid_t *low, gid_t *high)
+static void inet_get_ping_group_range_net(struct net *net, gid_t *low,
+					  gid_t *high)
 {
 	gid_t *data = net->ipv4.sysctl_ping_group_range;
 	unsigned seq;
@@ -437,7 +438,8 @@ static int ping_getfrag(void *from, char * to,
 	return 0;
 }
 
-static int ping_push_pending_frames(struct sock *sk, struct pingfakehdr *pfh, struct flowi4 *fl4)
+static int ping_push_pending_frames(struct sock *sk, struct pingfakehdr *pfh,
+				    struct flowi4 *fl4)
 {
 	struct sk_buff *skb = skb_peek(&sk->sk_write_queue);
 
@@ -754,7 +756,9 @@ static struct sock *ping_get_first(struct seq_file *seq, int start)
 	for (state->bucket = start; state->bucket < PING_HTABLE_SIZE;
 	     ++state->bucket) {
 		struct hlist_nulls_node *node;
-		struct hlist_nulls_head *hslot = &ping_table.hash[state->bucket];
+		struct hlist_nulls_head *hslot;
+
+		hslot = &ping_table.hash[state->bucket];
 
 		if (hlist_nulls_empty(hslot))
 			continue;

^ permalink raw reply related

* Re: [PATCH 14/18] virtio: add api for delayed callbacks
From: Michael S. Tsirkin @ 2011-05-19  7:24 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
	Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
	Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87boz3dsoe.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

On Mon, May 16, 2011 at 04:43:21PM +0930, Rusty Russell wrote:
> On Sun, 15 May 2011 15:48:18 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > On Mon, May 09, 2011 at 03:27:33PM +0930, Rusty Russell wrote:
> > > On Wed, 4 May 2011 23:52:33 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > > > Add an API that tells the other side that callbacks
> > > > should be delayed until a lot of work has been done.
> > > > Implement using the new used_event feature.
> > > 
> > > Since you're going to add a capacity query anyway, why not add the
> > > threshold argument here?
> > 
> > I thought that if we keep the API kind of generic
> > there might be more of a chance that future transports
> > will be able to implement it. For example, with an
> > old host we can't commit to a specific index.
> 
> No, it's always a hint anyway: you can be notified before the threshold
> is reached.  But best make it explicit I think.
> 
> Cheers,
> Rusty.


I tried doing that and remembered the real reason I went for this API:

capacity is limited by descriptor table space, not
used ring space: each entry in the used ring frees up multiple entries
in the descriptor ring. Thus the ring can't provide
callback after capacity is N: capacity is only available
after we get bufs.

We could try and make the API pass in the number of freed bufs, however:
- this is not really what virtio-net cares about (it cares about
  capacity)
- if the driver passes a number > number of outstanding bufs, it will
  never get a callback. So to stay correct the driver will need to
  track number of outstanding requests. The simpler API avoids that. 


APIs are easy to change so I'm guessing it's not a major blocker:
we can change later when e.g. block tries to
pass in some kind of extra hint: we'll be smarter
about how this API can change then.

Right?

-- 
MST

^ permalink raw reply

* Re: [PATCH 09/18] virtio: use avail_event index
From: Michael S. Tsirkin @ 2011-05-19  7:27 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Krishna Kumar, Carsten Otte, lguest-uLR06cmDAlY/bJ5BZ2RsiQ,
	Shirley Ma, kvm-u79uwXL29TY76Z2rM5mHXA,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
	habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Heiko Carstens,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
	Tom Lendacky, Martin Schwidefsky, linux390-tA70FqPdS9bQT0dZR+AlfA
In-Reply-To: <87tycsn9lt.fsf-8n+1lVoiYb80n/F98K4Iww@public.gmane.org>

On Wed, May 18, 2011 at 09:49:42AM +0930, Rusty Russell wrote:
> On Tue, 17 May 2011 09:10:31 +0300, "Michael S. Tsirkin" <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Well one can imagine a driver doing:
> > 
> > 	while (virtqueue_get_buf()) {
> > 		virtqueue_add_buf()
> > 	}
> > 	virtqueue_kick()
> > 
> > which looks sensible (batch kicks) but might
> > process any number of bufs between kicks.
> 
> No, we currently only expose the buffers in the kick, so it can only
> fill the ring doing that.
> 
> We could change that (and maybe that's worth looking at)...

That's actually what one of the early patches in the series did.
I guess I can try and reorder the patches, I do believe
it makes sense to publish immediately as this way
host can work in parallel with the guest.

> > If we look at drivers closely enough, I think none
> > of them do the equivalent of the above, but not 100% sure.
> 
> I'm pretty sure we don't have this kind of 'echo' driver yet.  Drivers
> tend to take OS requests and queue them.  The only one which does
> anything even partially sophisticated is the net driver...
> 
> Thanks,
> Rusty.

I guess I'll just need to do the legwork and check then.

-- 
MST

^ permalink raw reply

* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface,  PERCPU:  allocation failed
From: Denys Fedoryshchenko @ 2011-05-19  7:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1305788113.3019.19.camel@edumazet-laptop>

 On Thu, 19 May 2011 08:55:13 +0200, Eric Dumazet wrote:
> Le jeudi 19 mai 2011 à 08:39 +0200, Eric Dumazet a écrit :
>
>> Its a known problem : When ipv6 is enabled, we allocate percpu 
>> memory to
>> hold per device snmp counters.
>>
>> make sure kernel idea of max possible cpus matches real number of 
>> cpus.
>>
>> And yes, switching to 64bit kernel helps a lot.
>>
>>
>
> Looking at snmp6_alloc_dev(), we allocate three mib per device :
>
> ipstats_mib  (30 * sizeof(u64) * number_of_possible_cpus)
> icmpv6_mib    (4 * sizeof(long) * number_of_possible_cpus)
> icmpv6msg_mib  (26 * sizeof(long))
 1920 +
 256 +
 208 = 2386 * 3000ppp's = 7152000, i think it is not that much at any 
 case, if i am not wrong.

 But at any case i will try 64bit.

>
> For sure icmp ones dont need percpu counter. Plain atomic_long_t
> (shared) would be enough, since ICMP messages are rare enough.



^ permalink raw reply

* Re: [PATCH RFC] virtio_net: fix patch: virtio_net: limit xmit polling
From: Rusty Russell @ 2011-05-19  7:30 UTC (permalink / raw)
  To: Michael S. Tsirkin, habanero, Shirley Ma, Krishna Kumar2, kvm,
	steved, Tom
  Cc: Michael S. Tsirkin, virtualization, netdev, linux-kernel
In-Reply-To: <20110518220125.GA26835@redhat.com>

On Thu, 19 May 2011 01:01:25 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> The patch  virtio_net: limit xmit polling
> got the logic reversed: it polled while we had
> capacity not while ring was empty.
> 
> Fix it up and clean up a bit by using a for loop.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> OK, turns out that patch was borken. Here's
> a fix that survived stress test on my box.
> Pushed on my branch, I'll send a rebased series
> with Rusty's comments addressed ASAP.

Normally you would have missed the merge window by now, but I'd really
like this stuff in, so I'm holding it open for this.  I want these patches
in linux-next for at least a few days before I push them.

If you think we're not close enough, please tell me and I'll push
the rest of the virtio patches to Linus now.  

Thanks,
Rusty.

^ permalink raw reply

* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface, PERCPU:  allocation failed
From: Eric Dumazet @ 2011-05-19  7:44 UTC (permalink / raw)
  To: Denys Fedoryshchenko; +Cc: netdev
In-Reply-To: <10f61af229a48d77d06d154b4647cdde@visp.net.lb>

Le jeudi 19 mai 2011 à 10:28 +0300, Denys Fedoryshchenko a écrit :
> On Thu, 19 May 2011 08:55:13 +0200, Eric Dumazet wrote:
> > Le jeudi 19 mai 2011 à 08:39 +0200, Eric Dumazet a écrit :
> >
> >> Its a known problem : When ipv6 is enabled, we allocate percpu 
> >> memory to
> >> hold per device snmp counters.
> >>
> >> make sure kernel idea of max possible cpus matches real number of 
> >> cpus.
> >>
> >> And yes, switching to 64bit kernel helps a lot.
> >>
> >>
> >
> > Looking at snmp6_alloc_dev(), we allocate three mib per device :
> >
> > ipstats_mib  (30 * sizeof(u64) * number_of_possible_cpus)
> > icmpv6_mib    (4 * sizeof(long) * number_of_possible_cpus)
> > icmpv6msg_mib  (26 * sizeof(long))
>  1920 +
>  256 +
>  208 = 2386 * 3000ppp's = 7152000, i think it is not that much at any 
>  case, if i am not wrong.
> 
>  But at any case i will try 64bit.

If you really want to stay 32bit, you might try to enlarge vmalloc aread
(128 Mbytes default) to get room for pcpu data :

grep pcpu /proc/vmallocinfo 


boot param : vmalloc=256M




^ permalink raw reply

* Re: 2.6.39-rc7-git11, x86/32, failed on ppp2897'th interface, PERCPU: allocation failed
From: David Miller @ 2011-05-19  7:51 UTC (permalink / raw)
  To: denys; +Cc: netdev
In-Reply-To: <f9797bb034f650a24e927629d1ab77d8@visp.net.lb>

From: Denys Fedoryshchenko <denys@visp.net.lb>
Date: Thu, 19 May 2011 09:35:29 +0300

> I am not sure it is a bug, but it looks i had free memory(the box had
> 8GB free), and lowmem too, also i will try to enable there 64bit
> kernel at evening.

It's not free memory, you ran out of per-cpu chunks which are allocated
in fixed virtual region(s).

^ permalink raw reply

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19  7:52 UTC (permalink / raw)
  To: Simon Horman, Dave Jones, Julian Anastasov
  Cc: netdev@vger.kernel.org, Wensong Zhang
In-Reply-To: <20110519032611.GG16688@verge.net.au>

Hello
On Thursday 19 May 2011 05:26:14 Simon Horman wrote:
> On Thu, May 19, 2011 at 10:10:46AM +0900, Simon Horman wrote:
> > On Wed, May 18, 2011 at 04:19:15PM -0400, Dave Jones wrote:
> > > I get this oops from ip_vs_ftp..
> > > 
> > >  general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > >  last sysfs file: /sys/module/nf_nat/refcnt
> > >  CPU 3 
> > >  Modules linked in: ip_vs(+) libcrc32c nf_nat nfsd lockd nfs_acl auth_rpcgss sunrpc cpufreq_ondemand powernow_k8 freq_table mperf ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables snd_hda_codec_realtek ppdev snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm microcode edac_core snd_timer k10temp snd pcspkr usb_debug edac_mce_amd soundcore snd_page_alloc sp5100_tco i2c_piix4 parport_pc parport wmi r8169 mii lm63 ipv6 pata_acpi firewire_ohci ata_generic firewire_core crc_itu_t pata_atiixp floppy radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded: nf_nat]
> > >  
> > >  Pid: 1366, comm: modprobe Not tainted 2.6.39-rc7+ #15 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
> > >  RIP: 0010:[<ffffffff8107bddb>]  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > >  RSP: 0018:ffff880114139e68  EFLAGS: 00010206
> > >  RAX: 2f736e74656e2f74 RBX: ffffffffa04265d0 RCX: 0000000000000003
> > >  RDX: 00000000656e6567 RSI: ffffffffa04265d0 RDI: ffffffffa04235d8
> > >  RBP: ffff880114139e68 R08: ffff880114139df8 R09: 0000000000000001
> > >  R10: 0000000000000001 R11: 00000000000001cc R12: ffffffffa0432106
> > >  R13: 0000000000000000 R14: 0000000000007f0d R15: 0000000000410e40
> > >  FS:  00007f2aaf242720(0000) GS:ffff88012a800000(0000) knlGS:0000000000000000
> > >  CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > >  CR2: 00007f2aaea0100f CR3: 000000011424f000 CR4: 00000000000006e0
> > >  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > >  DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > >  Process modprobe (pid: 1366, threadinfo ffff880114138000, task ffff8801146cc7a0)
> > >  Stack:
> > >   ffff880114139e78 ffffffff8107be36 ffff880114139ec8 ffffffff81403058
> > >   0000000000000000 0000000000000000 ffff880114139ea8 0000000000000000
> > >   ffffffffa0432106 0000000000000000 0000000000007f0d 0000000000410e40
> > >  Call Trace:
> > >   [<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
> > >   [<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
> > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > >   [<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
> > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > >   [<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
> > >   [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
> > >   [<ffffffff81096524>] sys_init_module+0x132/0x281
> > >   [<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
> > >  Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48 
> > >  RIP  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > >   RSP <ffff880114139e68>
> > >  ---[ end trace e90d7053ad1a7a5b ]---
> > > 
> > > 
> > > This script replicates the bug.
> > > (it usually oopses after just a few loops)
> > > 
> > > #!/bin/sh
> > > while [ 1 ];
> > > do
> > > 	modprobe ip_vs_ftp
> > > 	modprobe -r ip_vs_ftp
> > > done
> > > 
> > > Looks like something isn't getting cleaned up on module exit
> > > that we fall over when we encounter it next time it gets loaded ?

It's a bug in ip_vs_ftp related to netns.

> > 
> > Thanks Dave, I will look into this.
> 
> Hi Dave,
> 
> I'm not having much luck reproducing this in KVM.
> I will try this evening on real hardware.
> 
> Just to make sure we are testing the same thing, are you using Linus's tree?
> --
I can reproduce the source of the problem,
use multiple netns and then unload the ftp module...
i.e. same list head used in multiple netns

This brings up a question:
- How should ftp be handled in a netns ? 
You might want to have it in one netns but not in another,
this requires changes to ipvsadm

A way of doing it could be a disable switch like --noftp [port,port]
i.e. do not break old apps.

Any other ideas ?

This patch solves the root problem, I'm not sure if this is the way to go
or if we should split the ip_vs_app struct ?

If it's the way to go I can send it as a proper formated patch ...
(after some testing)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 4fff432..481f856 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -797,7 +797,8 @@ struct netns_ipvs {
 	struct list_head	rs_table[IP_VS_RTAB_SIZE];
 	/* ip_vs_app */
 	struct list_head	app_list;
-
+	/* ip_vs_ftp */
+	struct ip_vs_app	*ftp_app;
 	/* ip_vs_proto */
 	#define IP_VS_PROTO_TAB_SIZE	32	/* must be power of 2 */
 	struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE];
diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
index 6b5dd6d..17afb09 100644
--- a/net/netfilter/ipvs/ip_vs_ftp.c
+++ b/net/netfilter/ipvs/ip_vs_ftp.c
@@ -411,25 +411,36 @@ static struct ip_vs_app ip_vs_ftp = {
 static int __net_init __ip_vs_ftp_init(struct net *net)
 {
 	int i, ret;
-	struct ip_vs_app *app = &ip_vs_ftp;
+	struct ip_vs_app *app;
+	struct netns_ipvs *ipvs = net_ipvs(net);
+
+	app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL);
+	if (!app)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&app->a_list);
+	INIT_LIST_HEAD(&app->incs_list);
+	INIT_LIST_HEAD(&app->p_list);
+	ipvs->ftp_app = app;
 
 	ret = register_ip_vs_app(net, app);
 	if (ret)
-		return ret;
+		goto err_exit;
 
 	for (i=0; i<IP_VS_APP_MAX_PORTS; i++) {
 		if (!ports[i])
 			continue;
 		ret = register_ip_vs_app_inc(net, app, app->protocol, ports[i]);
 		if (ret)
-			break;
+			goto err_unreg;
 		pr_info("%s: loaded support on port[%d] = %d\n",
 			app->name, i, ports[i]);
 	}
+	return 0;
 
-	if (ret)
-		unregister_ip_vs_app(net, app);
-
+err_unreg:
+	unregister_ip_vs_app(net, app);
+err_exit:
+	kfree(ipvs->ftp_app);
 	return ret;
 }
 /*
@@ -437,9 +448,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
  */
 static void __ip_vs_ftp_exit(struct net *net)
 {
-	struct ip_vs_app *app = &ip_vs_ftp;
-
-	unregister_ip_vs_app(net, app);
+	unregister_ip_vs_app(net, net_ipvs(net)->ftp_app);
 }
 
 static struct pernet_operations ip_vs_ftp_ops = {


-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ permalink raw reply related

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Simon Horman @ 2011-05-19  7:55 UTC (permalink / raw)
  To: Julian Anastasov; +Cc: Dave Jones, netdev, Wensong Zhang, Hans Schillstrom
In-Reply-To: <alpine.LFD.2.00.1105190930480.1993@ja.ssi.bg>

On Thu, May 19, 2011 at 09:33:55AM +0300, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 19 May 2011, Simon Horman wrote:
> 
> > > >  Call Trace:
> > > >   [<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
> > > >   [<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
> > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > >   [<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
> > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > >   [<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
> > > >   [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
> > > >   [<ffffffff81096524>] sys_init_module+0x132/0x281
> > > >   [<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
> > > >  Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48 
> > > >  RIP  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > > >   RSP <ffff880114139e68>
> > > >  ---[ end trace e90d7053ad1a7a5b ]---
> > > > 
> > > > 
> > > > This script replicates the bug.
> > > > (it usually oopses after just a few loops)
> > > > 
> > > > #!/bin/sh
> > > > while [ 1 ];
> > > > do
> > > > 	modprobe ip_vs_ftp
> > > > 	modprobe -r ip_vs_ftp
> > > > done
> > > > 
> > > > Looks like something isn't getting cleaned up on module exit
> > > > that we fall over when we encounter it next time it gets loaded ?
> > > 
> > > Thanks Dave, I will look into this.
> > 
> > Hi Dave,
> > 
> > I'm not having much luck reproducing this in KVM.
> > I will try this evening on real hardware.
> > 
> > Just to make sure we are testing the same thing, are you using Linus's tree?
> 
> 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> is missing in ip_vs_control_cleanup for sure.

Like this?

>From 840edfcc48e5b98d928ee9d66def761a808945b3 Mon Sep 17 00:00:00 2001
From: Simon Horman <horms@verge.net.au>
Date: Thu, 19 May 2011 16:54:26 +0900
Subject: [PATCH] IPVS: Free resources on module removal

Cc: Julian Anastasov <ja@ssi.bg>
Reported-by: Dave Jones <davej@redhat.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_ctl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 37890f2..9b9039b 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3774,6 +3774,7 @@ err_sock:
 void ip_vs_control_cleanup(void)
 {
 	EnterFunction(2);
+	unregister_netdevice_notifier(&ip_vs_dst_notifier);
 	ip_vs_genl_unregister();
 	nf_unregister_sockopt(&ip_vs_sockopts);
 	LeaveFunction(2);
-- 
1.7.4.4


^ permalink raw reply related

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19  7:58 UTC (permalink / raw)
  To: Julian Anastasov
  Cc: Simon Horman, Dave Jones, netdev@vger.kernel.org, Wensong Zhang,
	Hans Schillstrom
In-Reply-To: <alpine.LFD.2.00.1105190930480.1993@ja.ssi.bg>

On Thursday 19 May 2011 08:33:55 Julian Anastasov wrote:
> 
> 	Hello,
> 
[snip]
> 
> 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> is missing in ip_vs_control_cleanup for sure.
> 
Oops, 
Should I prepare a patch for that one ?

-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ permalink raw reply

* Re: [PATCH] tcp: Implement a two-level initial RTO as per draft RFC  2988bis-02.
From: Hagen Paul Pfeifer @ 2011-05-19  8:02 UTC (permalink / raw)
  To: Alexander Zimmermann
  Cc: tsuna, David Miller, kuznet, pekkas, jmorris, yoshfuji, kaber,
	eric.dumazet, netdev, linux-kernel
In-Reply-To: <8C5DF277-320D-4DEB-A133-EEC301DE58DC@comsys.rwth-aachen.de>


On Thu, 19 May 2011 08:52:10 +0200, Alexander Zimmermann wrote:



>> #define TCP_RTO_MAX     ((unsigned)(120*HZ))

>> #define TCP_RTO_MIN     ((unsigned)(HZ/5))

>> 

>> So we're talking about a [200ms ; 120s] range no matter what.

> 

> Why is 200ms a valid lower bound for initRTO? I'm aware of

> measurements that 1s is save for Internet, but I don't know of any

> studies that 200ms is save... 



TCP_RTO_MAX and TCP_RTO_MIN is the lower/upper bound for the RTO in

general, not for the initial RTO. RFC 2988 specify a lower bound of 1

second but all operating system choose a lower one because at the time

where RFC 2988 was written the clock granularity was not that accurate. The

minimum RTO for FreeBSD is even 30ms! Furthermore, analysis had

demonstrated that a minimum RTO of 1 second badly breaks throughput in

environments faster then 33kB with minor packet loss rate (e.g. 1%).



So yes, it CAN be wise to choose other lower/upper bounds. But keep in

mind that we should NOT artificial limit ourself. I can image data center

scenarios where a initial RTO of <1 match perfectly.



Hagen

^ permalink raw reply

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Simon Horman @ 2011-05-19  8:07 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: Dave Jones, Julian Anastasov, netdev@vger.kernel.org,
	Wensong Zhang
In-Reply-To: <201105190952.49006.hans.schillstrom@ericsson.com>

On Thu, May 19, 2011 at 09:52:47AM +0200, Hans Schillstrom wrote:
> Hello
> On Thursday 19 May 2011 05:26:14 Simon Horman wrote:
> > On Thu, May 19, 2011 at 10:10:46AM +0900, Simon Horman wrote:
> > > On Wed, May 18, 2011 at 04:19:15PM -0400, Dave Jones wrote:
> > > > I get this oops from ip_vs_ftp..
> > > > 
> > > >  general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
> > > >  last sysfs file: /sys/module/nf_nat/refcnt
> > > >  CPU 3 
> > > >  Modules linked in: ip_vs(+) libcrc32c nf_nat nfsd lockd nfs_acl auth_rpcgss sunrpc cpufreq_ondemand powernow_k8 freq_table mperf ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables snd_hda_codec_realtek ppdev snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm microcode edac_core snd_timer k10temp snd pcspkr usb_debug edac_mce_amd soundcore snd_page_alloc sp5100_tco i2c_piix4 parport_pc parport wmi r8169 mii lm63 ipv6 pata_acpi firewire_ohci ata_generic firewire_core crc_itu_t pata_atiixp floppy radeon ttm drm_kms_helper drm i2c_algo_bit i2c_core [last unloaded: nf_nat]
> > > >  
> > > >  Pid: 1366, comm: modprobe Not tainted 2.6.39-rc7+ #15 Gigabyte Technology Co., Ltd. GA-MA78GM-S2H/GA-MA78GM-S2H
> > > >  RIP: 0010:[<ffffffff8107bddb>]  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > > >  RSP: 0018:ffff880114139e68  EFLAGS: 00010206
> > > >  RAX: 2f736e74656e2f74 RBX: ffffffffa04265d0 RCX: 0000000000000003
> > > >  RDX: 00000000656e6567 RSI: ffffffffa04265d0 RDI: ffffffffa04235d8
> > > >  RBP: ffff880114139e68 R08: ffff880114139df8 R09: 0000000000000001
> > > >  R10: 0000000000000001 R11: 00000000000001cc R12: ffffffffa0432106
> > > >  R13: 0000000000000000 R14: 0000000000007f0d R15: 0000000000410e40
> > > >  FS:  00007f2aaf242720(0000) GS:ffff88012a800000(0000) knlGS:0000000000000000
> > > >  CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> > > >  CR2: 00007f2aaea0100f CR3: 000000011424f000 CR4: 00000000000006e0
> > > >  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > >  DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> > > >  Process modprobe (pid: 1366, threadinfo ffff880114138000, task ffff8801146cc7a0)
> > > >  Stack:
> > > >   ffff880114139e78 ffffffff8107be36 ffff880114139ec8 ffffffff81403058
> > > >   0000000000000000 0000000000000000 ffff880114139ea8 0000000000000000
> > > >   ffffffffa0432106 0000000000000000 0000000000007f0d 0000000000410e40
> > > >  Call Trace:
> > > >   [<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
> > > >   [<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
> > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > >   [<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
> > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > >   [<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
> > > >   [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
> > > >   [<ffffffff81096524>] sys_init_module+0x132/0x281
> > > >   [<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
> > > >  Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48 
> > > >  RIP  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > > >   RSP <ffff880114139e68>
> > > >  ---[ end trace e90d7053ad1a7a5b ]---
> > > > 
> > > > 
> > > > This script replicates the bug.
> > > > (it usually oopses after just a few loops)
> > > > 
> > > > #!/bin/sh
> > > > while [ 1 ];
> > > > do
> > > > 	modprobe ip_vs_ftp
> > > > 	modprobe -r ip_vs_ftp
> > > > done
> > > > 
> > > > Looks like something isn't getting cleaned up on module exit
> > > > that we fall over when we encounter it next time it gets loaded ?
> 
> It's a bug in ip_vs_ftp related to netns.
> 
> > > 
> > > Thanks Dave, I will look into this.
> > 
> > Hi Dave,
> > 
> > I'm not having much luck reproducing this in KVM.
> > I will try this evening on real hardware.
> > 
> > Just to make sure we are testing the same thing, are you using Linus's tree?
> > --
> I can reproduce the source of the problem,
> use multiple netns and then unload the ftp module...
> i.e. same list head used in multiple netns
> 
> This brings up a question:
> - How should ftp be handled in a netns ? 
> You might want to have it in one netns but not in another,
> this requires changes to ipvsadm
> 
> A way of doing it could be a disable switch like --noftp [port,port]
> i.e. do not break old apps.
> 
> Any other ideas ?
> 
> This patch solves the root problem, I'm not sure if this is the way to go
> or if we should split the ip_vs_app struct ?
> 
> If it's the way to go I can send it as a proper formated patch ...
> (after some testing)

I'm also unsure what a good solution in the longer term is.
But in the immediate future I think that the best idea is as
simple a fix as possible that can go 2.6.39 or -stable.

> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 4fff432..481f856 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -797,7 +797,8 @@ struct netns_ipvs {
>  	struct list_head	rs_table[IP_VS_RTAB_SIZE];
>  	/* ip_vs_app */
>  	struct list_head	app_list;
> -
> +	/* ip_vs_ftp */
> +	struct ip_vs_app	*ftp_app;
>  	/* ip_vs_proto */
>  	#define IP_VS_PROTO_TAB_SIZE	32	/* must be power of 2 */
>  	struct ip_vs_proto_data *proto_data_table[IP_VS_PROTO_TAB_SIZE];
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index 6b5dd6d..17afb09 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -411,25 +411,36 @@ static struct ip_vs_app ip_vs_ftp = {
>  static int __net_init __ip_vs_ftp_init(struct net *net)
>  {
>  	int i, ret;
> -	struct ip_vs_app *app = &ip_vs_ftp;
> +	struct ip_vs_app *app;
> +	struct netns_ipvs *ipvs = net_ipvs(net);
> +
> +	app = kmemdup(&ip_vs_ftp, sizeof(struct ip_vs_app), GFP_KERNEL);
> +	if (!app)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(&app->a_list);
> +	INIT_LIST_HEAD(&app->incs_list);
> +	INIT_LIST_HEAD(&app->p_list);
> +	ipvs->ftp_app = app;
>  
>  	ret = register_ip_vs_app(net, app);
>  	if (ret)
> -		return ret;
> +		goto err_exit;
>  
>  	for (i=0; i<IP_VS_APP_MAX_PORTS; i++) {
>  		if (!ports[i])
>  			continue;
>  		ret = register_ip_vs_app_inc(net, app, app->protocol, ports[i]);
>  		if (ret)
> -			break;
> +			goto err_unreg;
>  		pr_info("%s: loaded support on port[%d] = %d\n",
>  			app->name, i, ports[i]);
>  	}
> +	return 0;
>  
> -	if (ret)
> -		unregister_ip_vs_app(net, app);
> -
> +err_unreg:
> +	unregister_ip_vs_app(net, app);
> +err_exit:
> +	kfree(ipvs->ftp_app);
>  	return ret;
>  }
>  /*
> @@ -437,9 +448,7 @@ static int __net_init __ip_vs_ftp_init(struct net *net)
>   */
>  static void __ip_vs_ftp_exit(struct net *net)
>  {
> -	struct ip_vs_app *app = &ip_vs_ftp;
> -
> -	unregister_ip_vs_app(net, app);
> +	unregister_ip_vs_app(net, net_ipvs(net)->ftp_app);
>  }
>  
>  static struct pernet_operations ip_vs_ftp_ops = {
> 
> 
> -- 
> Regards
> Hans Schillstrom <hans.schillstrom@ericsson.com>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Julian Anastasov @ 2011-05-19  8:15 UTC (permalink / raw)
  To: Simon Horman; +Cc: Dave Jones, netdev, Wensong Zhang, Hans Schillstrom
In-Reply-To: <20110519075557.GB3922@verge.net.au>


	Hello,

On Thu, 19 May 2011, Simon Horman wrote:

> On Thu, May 19, 2011 at 09:33:55AM +0300, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Thu, 19 May 2011, Simon Horman wrote:
> > 
> > > > >  Call Trace:
> > > > >   [<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
> > > > >   [<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
> > > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > > >   [<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
> > > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > > >   [<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
> > > > >   [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
> > > > >   [<ffffffff81096524>] sys_init_module+0x132/0x281
> > > > >   [<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
> > > > >  Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48 
> > > > >  RIP  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > > > >   RSP <ffff880114139e68>
> > > > >  ---[ end trace e90d7053ad1a7a5b ]---
> > > > > 
> > > > > 
> > > > > This script replicates the bug.
> > > > > (it usually oopses after just a few loops)
> > > > > 
> > > > > #!/bin/sh
> > > > > while [ 1 ];
> > > > > do
> > > > > 	modprobe ip_vs_ftp
> > > > > 	modprobe -r ip_vs_ftp
> > > > > done
> > > > > 
> > > > > Looks like something isn't getting cleaned up on module exit
> > > > > that we fall over when we encounter it next time it gets loaded ?
> > > > 
> > > > Thanks Dave, I will look into this.
> > > 
> > > Hi Dave,
> > > 
> > > I'm not having much luck reproducing this in KVM.
> > > I will try this evening on real hardware.
> > > 
> > > Just to make sure we are testing the same thing, are you using Linus's tree?
> > 
> > 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > is missing in ip_vs_control_cleanup for sure.
> 
> Like this?

	Yes, I think oops is for 2nd or next module load after
first unload forgets entry in notifier list.

> >From 840edfcc48e5b98d928ee9d66def761a808945b3 Mon Sep 17 00:00:00 2001
> From: Simon Horman <horms@verge.net.au>
> Date: Thu, 19 May 2011 16:54:26 +0900
> Subject: [PATCH] IPVS: Free resources on module removal
> 
> Cc: Julian Anastasov <ja@ssi.bg>
> Reported-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>

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

> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 37890f2..9b9039b 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3774,6 +3774,7 @@ err_sock:
>  void ip_vs_control_cleanup(void)
>  {
>  	EnterFunction(2);
> +	unregister_netdevice_notifier(&ip_vs_dst_notifier);
>  	ip_vs_genl_unregister();
>  	nf_unregister_sockopt(&ip_vs_sockopts);
>  	LeaveFunction(2);
> -- 
> 1.7.4.4

^ permalink raw reply

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19  8:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: Julian Anastasov, Dave Jones, netdev@vger.kernel.org,
	Wensong Zhang, Hans Schillstrom
In-Reply-To: <20110519075557.GB3922@verge.net.au>

Hello, Simon
On Thursday 19 May 2011 09:55:57 Simon Horman wrote:
> On Thu, May 19, 2011 at 09:33:55AM +0300, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Thu, 19 May 2011, Simon Horman wrote:
> > 
> > > > >  Call Trace:
> > > > >   [<ffffffff8107be36>] raw_notifier_chain_register+0xe/0x10
> > > > >   [<ffffffff81403058>] register_netdevice_notifier+0x2d/0x1b6
> > > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > > >   [<ffffffffa04322c7>] ip_vs_control_init+0xa5/0xce [ip_vs]
> > > > >   [<ffffffffa0432106>] ? ip_vs_conn_init+0x106/0x106 [ip_vs]
> > > > >   [<ffffffffa0432116>] ip_vs_init+0x10/0x11c [ip_vs]
> > > > >   [<ffffffff81002099>] do_one_initcall+0x7f/0x13a
> > > > >   [<ffffffff81096524>] sys_init_module+0x132/0x281
> > > > >   [<ffffffff814cc702>] system_call_fastpath+0x16/0x1b
> > > > >  Code: 07 ff c8 89 43 48 eb 08 48 89 df e8 dc 95 44 00 4c 89 e6 48 89 df e8 a7 a5 44 00 5b 41 5c 5d c3 55 48 89 e5 66 66 66 66 90 eb 0c <8b> 50 10 39 56 10 7f 0c 48 8d 78 08 48 8b 07 48 85 c0 75 ec 48 
> > > > >  RIP  [<ffffffff8107bddb>] notifier_chain_register+0xb/0x2a
> > > > >   RSP <ffff880114139e68>
> > > > >  ---[ end trace e90d7053ad1a7a5b ]---
> > > > > 
> > > > > 
> > > > > This script replicates the bug.
> > > > > (it usually oopses after just a few loops)
> > > > > 
> > > > > #!/bin/sh
> > > > > while [ 1 ];
> > > > > do
> > > > > 	modprobe ip_vs_ftp
> > > > > 	modprobe -r ip_vs_ftp
> > > > > done
> > > > > 
> > > > > Looks like something isn't getting cleaned up on module exit
> > > > > that we fall over when we encounter it next time it gets loaded ?
> > > > 
> > > > Thanks Dave, I will look into this.
> > > 
> > > Hi Dave,
> > > 
> > > I'm not having much luck reproducing this in KVM.
> > > I will try this evening on real hardware.
> > > 
> > > Just to make sure we are testing the same thing, are you using Linus's tree?
> > 
> > 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > is missing in ip_vs_control_cleanup for sure.
> 
> Like this?

Yes,
we need this patch and the ip_vs_ftp patch in some format.

> 
> From 840edfcc48e5b98d928ee9d66def761a808945b3 Mon Sep 17 00:00:00 2001
> From: Simon Horman <horms@verge.net.au>
> Date: Thu, 19 May 2011 16:54:26 +0900
> Subject: [PATCH] IPVS: Free resources on module removal
> 
> Cc: Julian Anastasov <ja@ssi.bg>
> Reported-by: Dave Jones <davej@redhat.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 37890f2..9b9039b 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3774,6 +3774,7 @@ err_sock:
>  void ip_vs_control_cleanup(void)
>  {
>  	EnterFunction(2);
> +	unregister_netdevice_notifier(&ip_vs_dst_notifier);
>  	ip_vs_genl_unregister();
>  	nf_unregister_sockopt(&ip_vs_sockopts);
>  	LeaveFunction(2);

-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ permalink raw reply

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Simon Horman @ 2011-05-19  8:17 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: Julian Anastasov, Dave Jones, netdev@vger.kernel.org,
	Wensong Zhang, Hans Schillstrom
In-Reply-To: <201105190958.56185.hans.schillstrom@ericsson.com>

On Thu, May 19, 2011 at 09:58:55AM +0200, Hans Schillstrom wrote:
> On Thursday 19 May 2011 08:33:55 Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> [snip]
> > 
> > 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > is missing in ip_vs_control_cleanup for sure.
> > 
> Oops, 
> Should I prepare a patch for that one ?

Could you test the one I posted?
(Or send another one if I got it wrong? :-)

^ permalink raw reply

* [V2 Patch net-next-2.6] netpoll: disable netpoll when enslave a device
From: Amerigo Wang @ 2011-05-19  8:39 UTC (permalink / raw)
  To: linux-kernel
  Cc: akpm, WANG Cong, Neil Horman, Jay Vosburgh, David S. Miller,
	Ian Campbell, Paul E. McKenney, Josh Triplett, netdev
In-Reply-To: <20110518105558.GA3203@hmsreliant.think-freely.org>

Currently we do nothing when we enslave a net device which is running netconsole.
Neil pointed out that we may get weird results in such case, so let's disable
netpoll on the device being enslaved. I think it is too harsh to prevent
the device being ensalved if it is running netconsole.

By the way, this patch also removes the NETDEV_GOING_DOWN from netconsole
netdev notifier, because netpoll will check if the device is running or not
and we don't handle NETDEV_PRE_UP neither.

Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: Neil Horman <nhorman@redhat.com>

---
 drivers/net/bonding/bond_main.c |    2 ++
 drivers/net/netconsole.c        |   26 +++++++++++++++++---------
 include/linux/notifier.h        |    1 +
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 088fd84..b9c70c5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1640,6 +1640,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		}
 	}
 
+	netdev_bonding_change(slave_dev, NETDEV_ENSLAVE);
+
 	/* If this is the first slave, then we need to set the master's hardware
 	 * address to be the same as the slave's. */
 	if (is_zero_ether_addr(bond->dev->dev_addr))
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index a83e101..c2a8230 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -621,11 +621,10 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	bool stopped = false;
 
 	if (!(event == NETDEV_CHANGENAME || event == NETDEV_UNREGISTER ||
-	      event == NETDEV_BONDING_DESLAVE || event == NETDEV_GOING_DOWN))
+	      event == NETDEV_BONDING_DESLAVE || event == NETDEV_ENSLAVE))
 		goto done;
 
 	spin_lock_irqsave(&target_list_lock, flags);
-restart:
 	list_for_each_entry(nt, &target_list, list) {
 		netconsole_target_get(nt);
 		if (nt->np.dev == dev) {
@@ -633,6 +632,8 @@ restart:
 			case NETDEV_CHANGENAME:
 				strlcpy(nt->np.dev_name, dev->name, IFNAMSIZ);
 				break;
+			case NETDEV_BONDING_DESLAVE:
+			case NETDEV_ENSLAVE:
 			case NETDEV_UNREGISTER:
 				/*
 				 * rtnl_lock already held
@@ -647,11 +648,7 @@ restart:
 					dev_put(nt->np.dev);
 					nt->np.dev = NULL;
 					netconsole_target_put(nt);
-					goto restart;
 				}
-				/* Fall through */
-			case NETDEV_GOING_DOWN:
-			case NETDEV_BONDING_DESLAVE:
 				nt->enabled = 0;
 				stopped = true;
 				break;
@@ -660,10 +657,21 @@ restart:
 		netconsole_target_put(nt);
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
-	if (stopped && (event == NETDEV_UNREGISTER || event == NETDEV_BONDING_DESLAVE))
+	if (stopped) {
 		printk(KERN_INFO "netconsole: network logging stopped on "
-			"interface %s as it %s\n",  dev->name,
-			event == NETDEV_UNREGISTER ? "unregistered" : "released slaves");
+		       "interface %s as it ", dev->name);
+		switch (event) {
+		case NETDEV_UNREGISTER:
+			printk(KERN_CONT "unregistered\n");
+			break;
+		case NETDEV_BONDING_DESLAVE:
+			printk(KERN_CONT "released slaves\n");
+			break;
+		case NETDEV_ENSLAVE:
+			printk(KERN_CONT "is enslaved\n");
+			break;
+		}
+	}
 
 done:
 	return NOTIFY_DONE;
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 621dfa1..3d82867 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -211,6 +211,7 @@ static inline int notifier_to_errno(int ret)
 #define NETDEV_UNREGISTER_BATCH 0x0011
 #define NETDEV_BONDING_DESLAVE  0x0012
 #define NETDEV_NOTIFY_PEERS	0x0013
+#define NETDEV_ENSLAVE		0x0014
 
 #define SYS_DOWN	0x0001	/* Notify of system down */
 #define SYS_RESTART	SYS_DOWN

^ permalink raw reply related

* Re: ip_vs_ftp causing ip_vs oops on module load.
From: Hans Schillstrom @ 2011-05-19  8:52 UTC (permalink / raw)
  To: Simon Horman
  Cc: Julian Anastasov, Dave Jones, netdev@vger.kernel.org,
	Wensong Zhang, Hans Schillstrom
In-Reply-To: <20110519081706.GE3922@verge.net.au>

On Thursday 19 May 2011 10:17:07 Simon Horman wrote:
> On Thu, May 19, 2011 at 09:58:55AM +0200, Hans Schillstrom wrote:
> > On Thursday 19 May 2011 08:33:55 Julian Anastasov wrote:
> > > 
> > > 	Hello,
> > > 
> > [snip]
> > > 
> > > 	One unregister_netdevice_notifier(&ip_vs_dst_notifier);
> > > is missing in ip_vs_control_cleanup for sure.
> > > 
> > Oops, 
> > Should I prepare a patch for that one ?
> 
> Could you test the one I posted?
> (Or send another one if I got it wrong? :-)
> 
Tested,
it works fine :-)
-- 
Regards
Hans Schillstrom <hans.schillstrom@ericsson.com>

^ 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