ip_vs_ftp causing ip_vs oops on module load.

ip_vs_ftp causing ip_vs oops on module load.

[Dave Jones]

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 ?

	Dave

Re: ip_vs_ftp causing ip_vs oops on module load.

[Simon Horman]

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 ?

Thanks Dave, I will look into this.

Re: ip_vs_ftp causing ip_vs oops on module load.

[Simon Horman]

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 ?
> 
> 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?

Re: ip_vs_ftp causing ip_vs oops on module load.

[Julian Anastasov]

	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.

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Hans Schillstrom]

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>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Simon Horman]

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

Re: ip_vs_ftp causing ip_vs oops on module load.

[Hans Schillstrom]

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>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Simon Horman]

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
> 

Re: ip_vs_ftp causing ip_vs oops on module load.

[Hans Schillstrom]

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>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Julian Anastasov]

	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

Re: ip_vs_ftp causing ip_vs oops on module load.

[Simon Horman]

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? :-)

Re: ip_vs_ftp causing ip_vs oops on module load.

[Hans Schillstrom]

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>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Julian Anastasov]

	Hello,

On Thu, 19 May 2011, Hans Schillstrom wrote:

> 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 ?

	This patch is a fast fix but may be it is too late for it,
after 2.6.39 is out. It seems we overlooked the apps when
migrating to netns. I think, the apps do not need to be pernet.
If one day application needs pernet context we can add such
fields in the ipvs structure.

	While the protocols have controls that manipulate pernet
timeouts, the apps do not have such controls about app->timeouts.
May be we can remove app->timeouts to avoid confusion because
it was never implemented in user space. May be instead of this
fix we should restore the global ip_vs_app_list and all things in
ip_vs_app.c and ip_vs_ftp.c as before the netns changes?

> 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

--
Julian Anastasov <ja@ssi.bg>

Re: ip_vs_ftp causing ip_vs oops on module load.

[Hans Schillstrom]

On Thursday 19 May 2011 10:55:07 Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Thu, 19 May 2011, Hans Schillstrom wrote:
> 
> > 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 ?
> 
> 	This patch is a fast fix but may be it is too late for it,
> after 2.6.39 is out. It seems we overlooked the apps when
> migrating to netns. I think, the apps do not need to be pernet.
> If one day application needs pernet context we can add such
> fields in the ipvs structure.

If we talk about the long term solution,
applications that affects other netns should have their own data.
ex. like the ftp, ports should be per netns
    ipvsadm --appftp [port list]

> 
> 	While the protocols have controls that manipulate pernet
> timeouts, the apps do not have such controls about app->timeouts.
> May be we can remove app->timeouts to avoid confusion because
> it was never implemented in user space. May be instead of this
> fix we should restore the global ip_vs_app_list and all things in
> ip_vs_app.c and ip_vs_ftp.c as before the netns changes?

Doesn't matter, just keep the patch as simple as possible
while we discuss the long term solution.

> 
> > 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
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 

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

Re: ip_vs_ftp causing ip_vs oops on module load.

[Julian Anastasov]

	Hello,

On Thu, 19 May 2011, Hans Schillstrom wrote:

> If we talk about the long term solution,
> applications that affects other netns should have their own data.
> ex. like the ftp, ports should be per netns
>     ipvsadm --appftp [port list]

	Yes, it is possible, if we find solution to
trigger register_ip_vs_app_inc() calls with such controls...
May be app have to export some method(s) for this to work.

> > 	While the protocols have controls that manipulate pernet
> > timeouts, the apps do not have such controls about app->timeouts.
> > May be we can remove app->timeouts to avoid confusion because
> > it was never implemented in user space. May be instead of this
> > fix we should restore the global ip_vs_app_list and all things in
> > ip_vs_app.c and ip_vs_ftp.c as before the netns changes?
> 
> Doesn't matter, just keep the patch as simple as possible
> while we discuss the long term solution.

	OK, then let's use this solution but with some changes...

> > > --- 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);

	No need to init a_list and p_list here. As for
INIT_LIST_HEAD(&app->incs_list), may be it is better to
be moved into register_ip_vs_app() before the mutex_lock
to avoid further problems.

> > > +	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);


	Add here kfree(ipvs->ftp_app);


> > >  }
> > >  
> > >  static struct pernet_operations ip_vs_ftp_ops = {

	If you agree with these changes then you have my ack.

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