Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25  9:52 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <1335346710.3274.24.camel@edumazet-glaptop>

> Hmm, but what if we change linux to choice a) instead of b) ?
>
> That is, not cap mtu to minimum value 1280 (and not use anymore
> RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.
>
> In this case, do we still need to send the frag header ?

Yeah, I was wondering about that myself.

By my reading of the relevant RFC it's not quite clear whether you
truly must include the frag header even if you choose to obey the
lower than 1280 mtu.
Although I don't see any reason why you would need to...
So long as there's a decent minimum pmtu we're willing to obey.

> I ask this because some TSO6 implementations probably dont cope very
> well with this added header (untested path)

Yes, I was thinking the same thing, hence why I mentioned GSO.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25  9:51 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <1335346710.3274.24.camel@edumazet-glaptop>

* Eric Dumazet

> Hmm, but what if we change linux to choice a) instead of b) ?
>
> That is, not cap mtu to minimum value 1280 (and not use anymore
> RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.

Yep.

> In this case, do we still need to send the frag header ?

No - the (translated) IPv4 packet should then fit onto the small-MTU
IPv4 link without any fragmentation (by the IPv4 router) required. So
you end up with perfectly regular end-to-end Path MTU Discovery.

Tore

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25  9:48 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <f88609a3e80bbe53233e62dec2699a3e@greed.fud.no>

>> I get that we _choose_ to behave such, and I agree this adheres to
>> specs.
>
> "Chose" (past), not "choose" (present). ;-)

Details, but until we 'choose' to change it we continuously 'choose'
to have the current behaviour. ;-)

> This patch does not make this choice. This patch merely fixes a bug in
> the implementation of the choice that was made a long time ago.

yes, I wasn't saying this patch was bad, I was just wanting to point
that we should perhaps revisit the design choice.

>> But I'm not convinced that (even though this is allowed per RFC) this
>> is the right choice.
>
> That is a different issue entirely, but I don't disagree with you. A
> "min_pmtu" sysctl or something like that would be useful.

I don't really know what the default value should be?  Something around 500?
[to handle IPv4s min mtu of 576?]

Do we have any idea what values of small mtu actually show up in practice?

> Actually, in IPv6, fragmentation *must* be performed by end hosts,
> routers (including tunnel end points) *cannot* fragment.

Yes, I miss-phrased that, that's what I meant.

> However, the use case for the allfrag feature is not handling tunnels,
> but IPv4<->IPv6 translation. The issue is that a IPv6 host may very well
> receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
> originally transmitted by an IPv4 router (as an ICMPv4 Need To Fragment)
> and underwent translation to IPv6.

Very good point, although that's basically kind of like half a tunnel ;-)

> In case you're interested, I have a slide deck below that explains the
> use case for IPv4<->IPv6 translation. Slide 25 is about the particular
> corner case where the allfrag feature is necessary. URL:
>
> http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf

Yes, I've seen this slide set a couple days ago - very good.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Tore Anderson @ 2012-04-25  9:20 UTC (permalink / raw)
  To: Maciej Żenczykowski; +Cc: Eric Dumazet, David Miller, netdev, Tom Herbert
In-Reply-To: <CANP3RGdhs8s_RytR=f8ismSZdGs91bpVq=ZAjb0EOm-gCsDPAw@mail.gmail.com>

* Maciej Żenczykowski

>> But we chose to _not_ decrease mtu and adhere to the specs.
>
> I get that we _choose_ to behave such, and I agree this adheres to
> specs.

"Chose" (past), not "choose" (present). ;-)

This patch does not make this choice. This patch merely fixes a bug in
the implementation of the choice that was made a long time ago.

> But I'm not convinced that (even though this is allowed per RFC) this
> is the right choice.

That is a different issue entirely, but I don't disagree with you. A
"min_pmtu" sysctl or something like that would be useful.

> Also note that IPv6 prefers to see fragmentation happen at the end
> hosts, and not at the routers.
> Although of course it doesn't treat a tunnel end point as a router.

Actually, in IPv6, fragmentation *must* be performed by end hosts,
routers (including tunnel end points) *cannot* fragment.

However, the use case for the allfrag feature is not handling tunnels,
but IPv4<->IPv6 translation. The issue is that a IPv6 host may very 
well
receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
originally transmitted by an IPv4 router (as an ICMPv4 Need To 
Fragment)
and underwent translation to IPv6.

In this case, the IPv6 node does not need to reduce the PMTU to <1280
(Linux does not), but it is not invalid to have a <1280 MTU link in the
IPv4 internet either, so something else must be done for the
communication to work. The solution is then to include the IPv6 
Fragment
extension header, so that the translator have a suitable Identification
value to copy into the translated IPv4 header, and may therefore clear
the Don't Fragment flag, so that the IPv4 router will fragment the
packet as it is forwarded onto the low-MTU link.

In case you're interested, I have a slide deck below that explains the
use case for IPv4<->IPv6 translation. Slide 25 is about the particular
corner case where the allfrag feature is necessary. URL:

http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf

Tore

^ permalink raw reply

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Eric Dumazet @ 2012-04-25  9:38 UTC (permalink / raw)
  To: Tore Anderson; +Cc: Maciej Żenczykowski, David Miller, netdev, Tom Herbert
In-Reply-To: <f88609a3e80bbe53233e62dec2699a3e@greed.fud.no>

On Wed, 2012-04-25 at 11:20 +0200, Tore Anderson wrote:
> * Maciej Żenczykowski
> 
> >> But we chose to _not_ decrease mtu and adhere to the specs.
> >
> > I get that we _choose_ to behave such, and I agree this adheres to
> > specs.
> 
> "Chose" (past), not "choose" (present). ;-)
> 
> This patch does not make this choice. This patch merely fixes a bug in
> the implementation of the choice that was made a long time ago.
> 
> > But I'm not convinced that (even though this is allowed per RFC) this
> > is the right choice.
> 
> That is a different issue entirely, but I don't disagree with you. A
> "min_pmtu" sysctl or something like that would be useful.
> 
> > Also note that IPv6 prefers to see fragmentation happen at the end
> > hosts, and not at the routers.
> > Although of course it doesn't treat a tunnel end point as a router.
> 
> Actually, in IPv6, fragmentation *must* be performed by end hosts,
> routers (including tunnel end points) *cannot* fragment.
> 
> However, the use case for the allfrag feature is not handling tunnels,
> but IPv4<->IPv6 translation. The issue is that a IPv6 host may very 
> well
> receive an ICMPv6 Packet Too Big indicating a PMTU of <1280 that was
> originally transmitted by an IPv4 router (as an ICMPv4 Need To 
> Fragment)
> and underwent translation to IPv6.
> 
> In this case, the IPv6 node does not need to reduce the PMTU to <1280
> (Linux does not), but it is not invalid to have a <1280 MTU link in the
> IPv4 internet either, so something else must be done for the
> communication to work. The solution is then to include the IPv6 
> Fragment
> extension header, so that the translator have a suitable Identification
> value to copy into the translated IPv4 header, and may therefore clear
> the Don't Fragment flag, so that the IPv4 router will fragment the
> packet as it is forwarded onto the low-MTU link.
> 
> In case you're interested, I have a slide deck below that explains the
> use case for IPv4<->IPv6 translation. Slide 25 is about the particular
> corner case where the allfrag feature is necessary. URL:
> 
> http://fud.no/talks/20120417-RIPE64-The_Case_for_IPv6_Only_Data_Centres.pdf


Hmm, but what if we change linux to choice a) instead of b) ?

That is, not cap mtu to minimum value 1280 (and not use anymore
RTAX_FEATURE_ALLFRAG) : dst_allfrag() would be always false.

In this case, do we still need to send the frag header ?

I ask this because some TSO6 implementations probably dont cope very
well with this added header (untested path)

^ permalink raw reply

* Re: [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Hans Schillstrom @ 2012-04-25  9:36 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: horms@verge.net.au, ja@ssi.bg, wensong@linux-vs.org,
	lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org, hans@schillstrom.com
In-Reply-To: <1335344761.6337.42.camel@t520>

On Wednesday 25 April 2012 11:06:01 Jesper Dangaard Brouer wrote:
> Hi Hans,
> 
> Thank you for your work.
> Just some whitespace nitpicks below.

OK thanks I forgott to run checkpatch ...
I'll send a new patch  

> I have not been able to reproduce the bug in:
>   https://bugzilla.redhat.com/show_bug.cgi?id=806704
> 
> Can you recommend a (better) way to reproduce this bug?
> I just want to be able to verify that this fixes the bug in question.

I think you need two threads or procs.
Start a modprobe in thread one then issue a ipvs ioctl in the other.
The timing is not that easy here :-)

> 
> 
> On Wed, 2012-04-25 at 09:44 +0200, Hans Schillstrom wrote:
> > Change order of init so netns init is ready
> > when register ioctl and netlink.
> > 
> > Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> > Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> > ---
> >  include/net/ip_vs.h             |    2 +
> >  net/netfilter/ipvs/ip_vs_core.c |    9 ++++++
> >  net/netfilter/ipvs/ip_vs_ctl.c  |   52 ++++++++++++++++++++++----------------
> >  3 files changed, 41 insertions(+), 22 deletions(-)
> > 
> > diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> > index f967395..93b81aa 100644
> > --- a/include/net/ip_vs.h
> > +++ b/include/net/ip_vs.h
> > @@ -1201,6 +1201,8 @@ ip_vs_lookup_real_service(struct net *net, int af, __u16 protocol,
> >  
> >  extern int ip_vs_use_count_inc(void);
> >  extern void ip_vs_use_count_dec(void);
> > +extern int ip_vs_register_nl_ioctl(void);
> > +extern void ip_vs_unregister_nl_ioctl(void);
> >  extern int ip_vs_control_init(void);
> >  extern void ip_vs_control_cleanup(void);
> >  extern struct ip_vs_dest *
> > diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> > index d8b1d30..c8f36b9 100644
> > --- a/net/netfilter/ipvs/ip_vs_core.c
> > +++ b/net/netfilter/ipvs/ip_vs_core.c
> > @@ -1995,10 +1995,18 @@ static int __init ip_vs_init(void)
> >  		goto cleanup_dev;
> >  	}
> >  
> > +	ret = ip_vs_register_nl_ioctl();
> > +	if (ret < 0) {
> > +		pr_err("can't register netlink/ioctl.\n");
> > +		goto cleanup_hooks;
> > +	}
> > +
> >  	pr_info("ipvs loaded.\n");
> >  
> >  	return ret;
> >  
> > +cleanup_hooks:
> > +	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> >  cleanup_dev:
> >  	unregister_pernet_device(&ipvs_core_dev_ops);
> >  cleanup_sub:
> > @@ -2014,6 +2022,7 @@ exit:
> >  
> >  static void __exit ip_vs_cleanup(void)
> >  {
> > +	ip_vs_unregister_nl_ioctl();
> >  	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
> >  	unregister_pernet_device(&ipvs_core_dev_ops);
> >  	unregister_pernet_subsys(&ipvs_core_ops);	/* free ip_vs struct */
> > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> > index 7131417..efaf484 100644
> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
> >  	free_percpu(ipvs->tot_stats.cpustats);
> >  }
> >  
> > -int __init ip_vs_control_init(void)
> > +int ip_vs_register_nl_ioctl(void)
> >  {
> > -	int idx;
> >  	int ret;
> >  
> > -	EnterFunction(2);
> > -
> > -	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> > -	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
> > -		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> > -		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> > -	}
> > -
> > -	smp_wmb();	/* Do we really need it now ? */
> > -
> >  	ret = nf_register_sockopt(&ip_vs_sockopts);
> >  	if (ret) {
> >  		pr_err("cannot register sockopt.\n");
> > @@ -3776,28 +3765,47 @@ int __init ip_vs_control_init(void)
> >  		pr_err("cannot register Generic Netlink interface.\n");
> >  		goto err_genl;
> >  	}
> > -
> > -	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> > -	if (ret < 0)
> > -		goto err_notf;
> > -
> > -	LeaveFunction(2);
> >  	return 0;
> >  
> > -err_notf:
> > -	ip_vs_genl_unregister();
> >  err_genl:
> >  	nf_unregister_sockopt(&ip_vs_sockopts);
> >  err_sock:
> >  	return ret;
> >  }
> >  
> > +void ip_vs_unregister_nl_ioctl(void) 
> 
> There is a extra trailing whitespace behind
> ip_vs_unregister_nl_ioctl(void)
> 
> 
> > +{
> > +	ip_vs_genl_unregister();
> > +	nf_unregister_sockopt(&ip_vs_sockopts);
> > +}
> > +
> > +int __init ip_vs_control_init(void)
> > +{
> > +	int idx;
> > +	int ret;
> > +
> > +	EnterFunction(2);
> > +
> > +	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> > +	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
> 
> The for loop is funny spaced...
> 
> 
> > +		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> > +		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> > +	}
> > +
> > +	smp_wmb();	/* Do we really need it now ? */
> > +
> > +	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	LeaveFunction(2);
> > +	return 0;
> > +}
> > +
> >  
> >  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: [PATCH 2/2] ipvs: fix crash in ip_vs_control_net_cleanup on unload
From: Pablo Neira Ayuso @ 2012-04-25  9:15 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Sasha Levin, stable, Hans Schillstrom
In-Reply-To: <1335186732-16002-3-git-send-email-horms@verge.net.au>

On Mon, Apr 23, 2012 at 10:12:12PM +0900, Simon Horman wrote:
> From: Julian Anastasov <ja@ssi.bg>
> 
> 	commit 14e405461e664b777e2a5636e10b2ebf36a686ec (2.6.39)
> ("Add __ip_vs_control_{init,cleanup}_sysctl()")
> introduced regression due to wrong __net_init for
> __ip_vs_control_cleanup_sysctl. This leads to crash when
> the ip_vs module is unloaded.
> 
> 	Fix it by changing __net_init to __net_exit for
> the function that is already renamed to ip_vs_control_net_cleanup_sysctl.
> 
> Cc: stable@vger.kernel.org

Removed this above.

> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Hans Schillstrom <hans@schillstrom.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>

And applied, thanks Simon.

^ permalink raw reply

* Re: [PATCH 1/2] netfilter: ipvs: Verify that IP_VS protocol has been registered
From: Pablo Neira Ayuso @ 2012-04-25  9:14 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
	Julian Anastasov, Sasha Levin
In-Reply-To: <1335186732-16002-2-git-send-email-horms@verge.net.au>

On Mon, Apr 23, 2012 at 10:12:11PM +0900, Simon Horman wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
> 
> The registration of a protocol might fail, there were no checks
> and all registrations were assumed to be correct. This lead to
> NULL ptr dereferences when apps tried registering.
> 
> For example:
> 
> [ 1293.226051] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
> [ 1293.227038] IP: [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] PGD 391de067 PUD 6c20b067 PMD 0
> [ 1293.227038] Oops: 0000 [#1] PREEMPT SMP
> [ 1293.227038] CPU 1
> [ 1293.227038] Pid: 19609, comm: trinity Tainted: G        W    3.4.0-rc1-next-20120405-sasha-dirty #57
> [ 1293.227038] RIP: 0010:[<ffffffff822aacb0>]  [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038] RSP: 0018:ffff880038c1dd18  EFLAGS: 00010286
> [ 1293.227038] RAX: ffffffffffffffc0 RBX: 0000000000001500 RCX: 0000000000010000
> [ 1293.227038] RDX: 0000000000000000 RSI: ffff88003a2d5888 RDI: 0000000000000282
> [ 1293.227038] RBP: ffff880038c1dd48 R08: 0000000000000000 R09: 0000000000000000
> [ 1293.227038] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88003a2d5668
> [ 1293.227038] R13: ffff88003a2d5988 R14: ffff8800696a8ff8 R15: 0000000000000000
> [ 1293.227038] FS:  00007f01930d9700(0000) GS:ffff88007ce00000(0000) knlGS:0000000000000000
> [ 1293.227038] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 1293.227038] CR2: 0000000000000018 CR3: 0000000065dfc000 CR4: 00000000000406e0
> [ 1293.227038] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1293.227038] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 1293.227038] Process trinity (pid: 19609, threadinfo ffff880038c1c000, task ffff88002dc73000)
> [ 1293.227038] Stack:
> [ 1293.227038]  ffff880038c1dd48 00000000fffffff4 ffff8800696aada0 ffff8800694f5580
> [ 1293.227038]  ffffffff8369f1e0 0000000000001500 ffff880038c1dd98 ffffffff822a716b
> [ 1293.227038]  0000000000000000 ffff8800696a8ff8 0000000000000015 ffff8800694f5580
> [ 1293.227038] Call Trace:
> [ 1293.227038]  [<ffffffff822a716b>] ip_vs_app_inc_new+0xdb/0x180
> [ 1293.227038]  [<ffffffff822a7258>] register_ip_vs_app_inc+0x48/0x70
> [ 1293.227038]  [<ffffffff822b2fea>] __ip_vs_ftp_init+0xba/0x140
> [ 1293.227038]  [<ffffffff821c9060>] ops_init+0x80/0x90
> [ 1293.227038]  [<ffffffff821c90cb>] setup_net+0x5b/0xe0
> [ 1293.227038]  [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> [ 1293.227038]  [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> [ 1293.227038]  [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> [ 1293.227038]  [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> [ 1293.227038]  [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 1293.227038]  [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> [ 1293.227038] Code: 89 c7 e8 34 91 3b 00 89 de 66 c1 ee 04 31 de 83 e6 0f 48 83 c6 22 48 c1 e6 04 4a 8b 14 26 49 8d 34 34 48 8d 42 c0 48 39 d6 74 13 <66> 39 58 58 74 22 48 8b 48 40 48 8d 41 c0 48 39 ce 75 ed 49 8d
> [ 1293.227038] RIP  [<ffffffff822aacb0>] tcp_register_app+0x60/0xb0
> [ 1293.227038]  RSP <ffff880038c1dd18>
> [ 1293.227038] CR2: 0000000000000018
> [ 1293.379284] ---[ end trace 364ab40c7011a009 ]---
> [ 1293.381182] Kernel panic - not syncing: Fatal exception in interrupt
> 
> Cc: stable@vger.kernel.org

I've removed this above from the patch. I prefer not to add confusing
information (this means you'll have to rebase, sorry).

> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> Acked-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: Simon Horman <horms@verge.net.au>

Applied. Thanks.

BTW, did you consider using an array for ipvs->ip_vs_proto_table?

I think a hash table for only 5 protocols is overkill. I know, we'll
consume more memory but lookups will definitely be faster, but it will
consume a bit more memory (the space-time tradeoff again).

^ permalink raw reply

* Re: My e1000e GBE card is eating all port 623 pks
From: Joakim Tjernlund @ 2012-04-25  9:06 UTC (permalink / raw)
  To: Jesse Brandeburg; +Cc: netdev
In-Reply-To: <20120424145128.000070a8@unknown>

Jesse Brandeburg <jesse.brandeburg@intel.com> wrote on 2012/04/24 23:51:28:
>
> On Sat, 21 Apr 2012 15:31:09 +0200
> Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:
>
> > Looks like port 623 is some mgmt protocol and our e1000e boards are eating these
> > pkgs and this trips NIS, finger and yptest hangs for a long time before timing out and
> > moving on.
> >
> > Is there a way to tell the network stack not to eat port 623 pkgs or
> > have NIS not to use port 623?
>
> I think you might be looking for something like portreserve
> (see man portreserve)

Seen portreserve on the web(gentoo does not have it) but it seems like a workaround.
Should not eating port 623 be something one should turn on? Now it is default on
and I cannot find a way to turn it off.

 Jocke

^ permalink raw reply

* Re: [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Jesper Dangaard Brouer @ 2012-04-25  9:06 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: horms, ja, wensong, lvs-devel, netdev, netfilter-devel, hans
In-Reply-To: <1335339884-29825-1-git-send-email-hans.schillstrom@ericsson.com>

Hi Hans,

Thank you for your work.
Just some whitespace nitpicks below.

I have not been able to reproduce the bug in:
  https://bugzilla.redhat.com/show_bug.cgi?id=806704

Can you recommend a (better) way to reproduce this bug?
I just want to be able to verify that this fixes the bug in question.


On Wed, 2012-04-25 at 09:44 +0200, Hans Schillstrom wrote:
> Change order of init so netns init is ready
> when register ioctl and netlink.
> 
> Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---
>  include/net/ip_vs.h             |    2 +
>  net/netfilter/ipvs/ip_vs_core.c |    9 ++++++
>  net/netfilter/ipvs/ip_vs_ctl.c  |   52 ++++++++++++++++++++++----------------
>  3 files changed, 41 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index f967395..93b81aa 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -1201,6 +1201,8 @@ ip_vs_lookup_real_service(struct net *net, int af, __u16 protocol,
>  
>  extern int ip_vs_use_count_inc(void);
>  extern void ip_vs_use_count_dec(void);
> +extern int ip_vs_register_nl_ioctl(void);
> +extern void ip_vs_unregister_nl_ioctl(void);
>  extern int ip_vs_control_init(void);
>  extern void ip_vs_control_cleanup(void);
>  extern struct ip_vs_dest *
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index d8b1d30..c8f36b9 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1995,10 +1995,18 @@ static int __init ip_vs_init(void)
>  		goto cleanup_dev;
>  	}
>  
> +	ret = ip_vs_register_nl_ioctl();
> +	if (ret < 0) {
> +		pr_err("can't register netlink/ioctl.\n");
> +		goto cleanup_hooks;
> +	}
> +
>  	pr_info("ipvs loaded.\n");
>  
>  	return ret;
>  
> +cleanup_hooks:
> +	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
>  cleanup_dev:
>  	unregister_pernet_device(&ipvs_core_dev_ops);
>  cleanup_sub:
> @@ -2014,6 +2022,7 @@ exit:
>  
>  static void __exit ip_vs_cleanup(void)
>  {
> +	ip_vs_unregister_nl_ioctl();
>  	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
>  	unregister_pernet_device(&ipvs_core_dev_ops);
>  	unregister_pernet_subsys(&ipvs_core_ops);	/* free ip_vs struct */
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7131417..efaf484 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
>  	free_percpu(ipvs->tot_stats.cpustats);
>  }
>  
> -int __init ip_vs_control_init(void)
> +int ip_vs_register_nl_ioctl(void)
>  {
> -	int idx;
>  	int ret;
>  
> -	EnterFunction(2);
> -
> -	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> -	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
> -		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> -		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> -	}
> -
> -	smp_wmb();	/* Do we really need it now ? */
> -
>  	ret = nf_register_sockopt(&ip_vs_sockopts);
>  	if (ret) {
>  		pr_err("cannot register sockopt.\n");
> @@ -3776,28 +3765,47 @@ int __init ip_vs_control_init(void)
>  		pr_err("cannot register Generic Netlink interface.\n");
>  		goto err_genl;
>  	}
> -
> -	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> -	if (ret < 0)
> -		goto err_notf;
> -
> -	LeaveFunction(2);
>  	return 0;
>  
> -err_notf:
> -	ip_vs_genl_unregister();
>  err_genl:
>  	nf_unregister_sockopt(&ip_vs_sockopts);
>  err_sock:
>  	return ret;
>  }
>  
> +void ip_vs_unregister_nl_ioctl(void) 

There is a extra trailing whitespace behind
ip_vs_unregister_nl_ioctl(void)


> +{
> +	ip_vs_genl_unregister();
> +	nf_unregister_sockopt(&ip_vs_sockopts);
> +}
> +
> +int __init ip_vs_control_init(void)
> +{
> +	int idx;
> +	int ret;
> +
> +	EnterFunction(2);
> +
> +	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
> +	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {

The for loop is funny spaced...


> +		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
> +		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
> +	}
> +
> +	smp_wmb();	/* Do we really need it now ? */
> +
> +	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
> +	if (ret < 0)
> +		return ret;
> +
> +	LeaveFunction(2);
> +	return 0;
> +}
> +
>  
>  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);
>  }

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply

* Re: [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Julian Anastasov @ 2012-04-25  8:56 UTC (permalink / raw)
  To: Hans Schillstrom; +Cc: horms, wensong, lvs-devel, netdev, netfilter-devel, hans
In-Reply-To: <1335339884-29825-1-git-send-email-hans.schillstrom@ericsson.com>


	Hello,

On Wed, 25 Apr 2012, Hans Schillstrom wrote:

> Change order of init so netns init is ready
> when register ioctl and netlink.
> 
> Reported-by: "Ryan O'Hara" <rohara@redhat.com>
> Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---
>  include/net/ip_vs.h             |    2 +
>  net/netfilter/ipvs/ip_vs_core.c |    9 ++++++
>  net/netfilter/ipvs/ip_vs_ctl.c  |   52 ++++++++++++++++++++++----------------
>  3 files changed, 41 insertions(+), 22 deletions(-)
> 

> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7131417..efaf484 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
>  	free_percpu(ipvs->tot_stats.cpustats);
>  }
>  
> -int __init ip_vs_control_init(void)
> +int ip_vs_register_nl_ioctl(void)

	Can we add __init here. Everything else looks good.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* re: wireless: mwifiex: initial commit for Marvell mwifiex driver
From: Dan Carpenter @ 2012-04-25  8:44 UTC (permalink / raw)
  To: bzhao; +Cc: netdev

Hi Bing,

The patch 5e6e3a92b9a4: "wireless: mwifiex: initial commit for
Marvell mwifiex driver" from Mar 21, 2011, leads to the following
static checker warning:

drivers/net/wireless/mwifiex/sta_ioctl.c:1410
mwifiex_set_gen_ie_helper()
	 error: memcmp() 'pvendor_ie->oui' too small (3 vs 4)

  1390  mwifiex_set_gen_ie_helper(struct mwifiex_private *priv, u8 *ie_data_ptr,
  1391                            u16 ie_len)
  1392  {
  1393          int ret = 0;
  1394          struct ieee_types_vendor_header *pvendor_ie;
  1395          const u8 wpa_oui[] = { 0x00, 0x50, 0xf2, 0x01 };
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
4 byte array.

  1396          const u8 wps_oui[] = { 0x00, 0x50, 0xf2, 0x04 };
  1397  
  1398          /* If the passed length is zero, reset the buffer */
  1399          if (!ie_len) {
  1400                  priv->gen_ie_buf_len = 0;
  1401                  priv->wps.session_enable = false;
  1402  
  1403                  return 0;
  1404          } else if (!ie_data_ptr) {
  1405                  return -1;
  1406          }
  1407          pvendor_ie = (struct ieee_types_vendor_header *) ie_data_ptr;
  1408          /* Test to see if it is a WPA IE, if not, then it is a gen IE */
  1409          if (((pvendor_ie->element_id == WLAN_EID_WPA) &&
  1410               (!memcmp(pvendor_ie->oui, wpa_oui, sizeof(wpa_oui)))) ||
                              ^^^^^^^^^^^^^^^
->oui is only a 3 byte array so we're reading past the end for this
comparison.

  1411              (pvendor_ie->element_id == WLAN_EID_RSN)) {


There are a couple other similar warnings as well:

drivers/net/wireless/mwifiex/sta_ioctl.c:1435 mwifiex_set_gen_ie_helper()
	error: memcmp() 'pvendor_ie->oui' too small (3 vs 4)
drivers/net/wireless/mwifiex/scan.c:1177 mwifiex_update_bss_desc_with_ie()
	error: memcmp() 'vendor_ie->vend_hdr.oui' too small (3 vs 4)
drivers/net/wireless/mwifiex/scan.c:1185 mwifiex_update_bss_desc_with_ie()
	error: memcmp() 'vendor_ie->vend_hdr.oui' too small (3 vs 4)

regards,
dan carpenter

^ permalink raw reply

* Re: [RFC v4] Add TCP encap_rcv hook (repost)
From: Simon Horman @ 2012-04-25  8:39 UTC (permalink / raw)
  To: Kyle Mestery (kmestery)
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, jhs-jkUAjuhPggJWk0Htik3J/w,
	stephen.hemminger-ZtmgI6mnKB3QT0dZR+AlfA,
	shemminger-ZtmgI6mnKB3QT0dZR+AlfA, David Miller
In-Reply-To: <807AC914-2F33-46C7-99DC-E2F8F0F97531-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

On Tue, Apr 24, 2012 at 04:02:41PM +0000, Kyle Mestery (kmestery) wrote:
> On Apr 23, 2012, at 9:25 PM, Simon Horman wrote:
> > On Mon, Apr 23, 2012 at 03:59:24PM -0700, Jesse Gross wrote:
> >> On Mon, Apr 23, 2012 at 3:32 PM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
> >>> On Mon, Apr 23, 2012 at 02:38:07PM -0700, Jesse Gross wrote:
> >>>> On Mon, Apr 23, 2012 at 2:08 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> >>>>> From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> >>>>> Date: Mon, 23 Apr 2012 13:53:42 -0700
> >>>>> 
> >>>>>> On Mon, Apr 23, 2012 at 1:13 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> >>>>>>> From: Jesse Gross <jesse-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> >>>>>>> Date: Mon, 23 Apr 2012 13:08:49 -0700
> >>>>>>> 
> >>>>>>>> Assuming that the TCP stack generates large TSO frames on transmit
> >>>>>>>> (which could be the local stack; something sent by a VM; or packets
> >>>>>>>> received, coalesced by GRO and then encapsulated by STT) then you can
> >>>>>>>> just prepend the STT header (possibly slightly adjusting things like
> >>>>>>>> requested MSS, number of segments, etc. slightly).  After that it's
> >>>>>>>> possible to just output the resulting frame through the IP stack like
> >>>>>>>> all tunnels do today.
> >>>>>>> 
> >>>>>>> Which seems to potentially suggest a stronger intergration of the STT
> >>>>>>> tunnel transmit path into our IP stack rather than the approach Simon
> >>>>>>> is taking
> >>>>>> 
> >>>>>> Did you have something in mind?
> >>>>> 
> >>>>> A normal bonafide tunnel netdevice driver like GRE instead of the
> >>>>> openvswitch approach Simon is using.
> >>>> 
> >>>> Ahh, yes, that I agree with.  Independent of this, there's work being
> >>>> done to make it so that OVS can use the normal in-tree tunneling code
> >>>> and not need its own.  Once that's done I expect that STT will follow
> >>>> the same model.
> >>> 
> >>> Hi Jesse,
> >>> 
> >>> I am wondering how firm the plans to on allowing OVS to use in-tree tunnel
> >>> code are. I'm happy to move my efforts over to an in-tree STT implementation
> >>> but ultimately I would like to get STT running in conjunction with OVS.
> >> 
> >> I would say that it's a firm goal but the implementation probably
> >> still has a ways to go.  Kyle Mestery (CC'ed) has volunteered to work
> >> on this in support of adding VXLAN, which needs some additional
> >> flexibility that this approach would also provide.  You might want to
> >> talk to him to see if there are ways that you guys can work together
> >> on it if you are interested.  Having better integration with upstream
> >> tunneling is definitely a step that OVS needs to make and sooner would
> >> be better than later.
> > 
> > Hi Jesse, Hi Kyle,
> > 
> > that sounds like an excellent plan.
> > 
> > Kyle, do you have any thoughts on how we might best work together on this?
> > Perhaps there are some patches floating around that I could take a look at?
> > 
> 
> Hi Simon:
> 
> The VXLAN work has been slow going for me at this point. What I have works, but is far from complete. It's available here:
> 
> https://github.com/mestery/ovs-vxlan/tree/vxlan
> 
> This is based on a fairly recent version of OVS. I'm currently working to allow tunnels to be flow-based rather than port-based, as they currently exist.
> As Jesse may have mentioned, doing this allows us to move most tunnel state into user space. The outer header can now be part of the flow lookup and can
> be passed to user space, so things like multicast learning for VXLAN become possible.
> 
> With regards to working together, ping me off-list and we can work something out, I'm very much in favor of this!

Hi Kyle,

the component that is of most interest to me is enabling OVS to use in-tree
tunnelling code - as it seems that makes most sense for an implementation
of STT. I have taken a brief look over your vxlan work and it isn't clear
to me if it is moving towards being an in-tree implementation.  Moreover,
I'm a rather unclear on what changes need to be made to OVS in order for
in-tree tunneling to be used.

My recollection is that OVS did make use of in-tree tunnelling code
but this was removed in favour of the current implementation for various
reasons (performance being one IIRC). I gather that revisiting in-tree
tunnelling won't revisit the previous set of problems. But I'm unclear how.

Jesse, is it possible for you to describe that in a little detail
or point me to some information?

^ permalink raw reply

* Re: [PATCH 2/4] ipvs: take care of return value from protocol init_netns
From: Julian Anastasov @ 2012-04-25  8:35 UTC (permalink / raw)
  To: Hans Schillstrom; +Cc: horms, wensong, lvs-devel, netdev, netfilter-devel, hans
In-Reply-To: <1335339821-29684-3-git-send-email-hans.schillstrom@ericsson.com>


	Hello,

On Wed, 25 Apr 2012, Hans Schillstrom wrote:

> ip_vs_create_timeout_table() can return NULL
> All functions protocol init_netns is affected of this patch.
> 
> Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
> ---

> diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
> index 6a8d176..0b74795 100644
> --- a/net/netfilter/ipvs/ip_vs_proto.c
> +++ b/net/netfilter/ipvs/ip_vs_proto.c
> @@ -79,7 +79,7 @@ register_ip_vs_proto_netns(struct net *net, struct ip_vs_protocol *pp)
>  	atomic_set(&pd->appcnt, 0);	/* Init app counter */
>  
>  	if (pp->init_netns != NULL)
> -		pp->init_netns(net, pd);
> +		return pp->init_netns(net, pd);

	May be it is better to unlink and free the pd here
if init_netns fails. Currently, protocols attach only
allocated pointer (pd->timeout_table) and it is not a
big problem to call exit_netns on failure but lets do
it safely, every init handler should release its data on
failure and then we should not call exit_netns.

	Patch 1 looks ok and I'll ack it next time.

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [PATCH v2 4/5] don't take cgroup_mutex in destroy()
From: Li Zefan @ 2012-04-25  8:01 UTC (permalink / raw)
  To: Glauber Costa
  Cc: KAMEZAWA Hiroyuki, Tejun Heo, netdev-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, David Miller,
	devel-GEFAQzZX7r8dnm+yROfE0A, Vivek Goyal
In-Reply-To: <4F9691A8.1070106-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

Glauber Costa wrote:

> On 04/23/2012 11:31 PM, KAMEZAWA Hiroyuki wrote:
>> (2012/04/24 4:37), Glauber Costa wrote:
>>
>>> Most of the destroy functions are only doing very simple things
>>> like freeing memory.
>>>
>>> The ones who goes through lists and such, already use its own
>>> locking for those.
>>>
>>> * The cgroup itself won't go away until we free it, (after destroy)
>>> * The parent won't go away because we hold a reference count
>>> * There are no more tasks in the cgroup, and the cgroup is declared
>>>    dead (cgroup_is_removed() == true)
>>>
>>> [v2: don't cgroup_lock the freezer and blkcg ]
>>>
>>> Signed-off-by: Glauber Costa<glommer-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
>>> CC: Tejun Heo<tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> CC: Li Zefan<lizefan-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>> CC: Kamezawa Hiroyuki<kamezawa.hiroyu-+CUm20s59erQFUHtdCDX3A@public.gmane.org>
>>> CC: Vivek Goyal<vgoyal-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>   kernel/cgroup.c |    9 ++++-----
>>>   1 files changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
>>> index 932c318..976d332 100644
>>> --- a/kernel/cgroup.c
>>> +++ b/kernel/cgroup.c
>>> @@ -869,13 +869,13 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode)
>>>   		 * agent */
>>>   		synchronize_rcu();
>>>
>>> -		mutex_lock(&cgroup_mutex);
>>>   		/*
>>>   		 * Release the subsystem state objects.
>>>   		 */
>>>   		for_each_subsys(cgrp->root, ss)
>>>   			ss->destroy(cgrp);
>>>
>>> +		mutex_lock(&cgroup_mutex);
>>>   		cgrp->root->number_of_cgroups--;
>>>   		mutex_unlock(&cgroup_mutex);
>>>
>>> @@ -3994,13 +3994,12 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
>>>
>>>    err_destroy:
>>>
>>> +	mutex_unlock(&cgroup_mutex);
>>>   	for_each_subsys(root, ss) {
>>>   		if (cgrp->subsys[ss->subsys_id])
>>>   			ss->destroy(cgrp);
>>>   	}
>>>
>>> -	mutex_unlock(&cgroup_mutex);
>>> -
>>>   	/* Release the reference count that we took on the superblock */
>>>   	deactivate_super(sb);
>>>
>>> @@ -4349,9 +4348,9 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss)
>>>   		int ret = cgroup_init_idr(ss, css);
>>>   		if (ret) {
>>>   			dummytop->subsys[ss->subsys_id] = NULL;
>>> +			mutex_unlock(&cgroup_mutex);
>>>   			ss->destroy(dummytop);
>>>   			subsys[i] = NULL;
>>> -			mutex_unlock(&cgroup_mutex);
>>>   			return ret;
>>>   		}
>>>   	}
>>> @@ -4447,10 +4446,10 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss)
>>>   	 * pointer to find their state. note that this also takes care of
>>>   	 * freeing the css_id.
>>>   	 */
>>> +	mutex_unlock(&cgroup_mutex);
>>>   	ss->destroy(dummytop);
>>>   	dummytop->subsys[ss->subsys_id] = NULL;
>>>
>>
>> I'm not fully sure but...dummytop->subsys[] update can be done without locking ?
>>
> I don't see a reason why updates to subsys[] after destruction shouldn't
> be safe. But maybe I am wrong.
> 
> Tejun? Li?
> 


It's safe for dummpytop->subsys[], but it makes the code a bit subtle.

The worst part is, it's not safe to NULLify subsys[i] without cgroup_mutex. It should be
ok to do that before calling ->destroy(), but again the code becomes a bit subtler.

^ permalink raw reply

* [PATCH 1/1] ipvs: kernel oops - do_ip_vs_get_ctl
From: Hans Schillstrom @ 2012-04-25  7:44 UTC (permalink / raw)
  To: horms, ja, wensong, lvs-devel, netdev, netfilter-devel
  Cc: hans, Hans Schillstrom

Change order of init so netns init is ready
when register ioctl and netlink.

Reported-by: "Ryan O'Hara" <rohara@redhat.com>
Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
 include/net/ip_vs.h             |    2 +
 net/netfilter/ipvs/ip_vs_core.c |    9 ++++++
 net/netfilter/ipvs/ip_vs_ctl.c  |   52 ++++++++++++++++++++++----------------
 3 files changed, 41 insertions(+), 22 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index f967395..93b81aa 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1201,6 +1201,8 @@ ip_vs_lookup_real_service(struct net *net, int af, __u16 protocol,
 
 extern int ip_vs_use_count_inc(void);
 extern void ip_vs_use_count_dec(void);
+extern int ip_vs_register_nl_ioctl(void);
+extern void ip_vs_unregister_nl_ioctl(void);
 extern int ip_vs_control_init(void);
 extern void ip_vs_control_cleanup(void);
 extern struct ip_vs_dest *
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index d8b1d30..c8f36b9 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1995,10 +1995,18 @@ static int __init ip_vs_init(void)
 		goto cleanup_dev;
 	}
 
+	ret = ip_vs_register_nl_ioctl();
+	if (ret < 0) {
+		pr_err("can't register netlink/ioctl.\n");
+		goto cleanup_hooks;
+	}
+
 	pr_info("ipvs loaded.\n");
 
 	return ret;
 
+cleanup_hooks:
+	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
 cleanup_dev:
 	unregister_pernet_device(&ipvs_core_dev_ops);
 cleanup_sub:
@@ -2014,6 +2022,7 @@ exit:
 
 static void __exit ip_vs_cleanup(void)
 {
+	ip_vs_unregister_nl_ioctl();
 	nf_unregister_hooks(ip_vs_ops, ARRAY_SIZE(ip_vs_ops));
 	unregister_pernet_device(&ipvs_core_dev_ops);
 	unregister_pernet_subsys(&ipvs_core_ops);	/* free ip_vs struct */
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7131417..efaf484 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -3750,21 +3750,10 @@ void __net_exit ip_vs_control_net_cleanup(struct net *net)
 	free_percpu(ipvs->tot_stats.cpustats);
 }
 
-int __init ip_vs_control_init(void)
+int ip_vs_register_nl_ioctl(void)
 {
-	int idx;
 	int ret;
 
-	EnterFunction(2);
-
-	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
-	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
-		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
-		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
-	}
-
-	smp_wmb();	/* Do we really need it now ? */
-
 	ret = nf_register_sockopt(&ip_vs_sockopts);
 	if (ret) {
 		pr_err("cannot register sockopt.\n");
@@ -3776,28 +3765,47 @@ int __init ip_vs_control_init(void)
 		pr_err("cannot register Generic Netlink interface.\n");
 		goto err_genl;
 	}
-
-	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
-	if (ret < 0)
-		goto err_notf;
-
-	LeaveFunction(2);
 	return 0;
 
-err_notf:
-	ip_vs_genl_unregister();
 err_genl:
 	nf_unregister_sockopt(&ip_vs_sockopts);
 err_sock:
 	return ret;
 }
 
+void ip_vs_unregister_nl_ioctl(void) 
+{
+	ip_vs_genl_unregister();
+	nf_unregister_sockopt(&ip_vs_sockopts);
+}
+
+int __init ip_vs_control_init(void)
+{
+	int idx;
+	int ret;
+
+	EnterFunction(2);
+
+	/* Initialize svc_table, ip_vs_svc_fwm_table, rs_table */
+	for(idx = 0; idx < IP_VS_SVC_TAB_SIZE; idx++)  {
+		INIT_LIST_HEAD(&ip_vs_svc_table[idx]);
+		INIT_LIST_HEAD(&ip_vs_svc_fwm_table[idx]);
+	}
+
+	smp_wmb();	/* Do we really need it now ? */
+
+	ret = register_netdevice_notifier(&ip_vs_dst_notifier);
+	if (ret < 0)
+		return ret;
+
+	LeaveFunction(2);
+	return 0;
+}
+
 
 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.2.3


^ permalink raw reply related

* [PATCH 1/2] ipvs: null check of net->ipvs in lblc(r) shedulers
From: Hans Schillstrom @ 2012-04-25  7:43 UTC (permalink / raw)
  To: horms, ja, wensong, lvs-devel, netdev, netfilter-devel
  Cc: hans, Hans Schillstrom
In-Reply-To: <1335339821-29684-1-git-send-email-hans.schillstrom@ericsson.com>

Avoid crash when registering shedulers after
the IPVS core initialization for netns fails. Do this by
checking for present core (net->ipvs).

Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
 net/netfilter/ipvs/ip_vs_lblc.c  |    3 +++
 net/netfilter/ipvs/ip_vs_lblcr.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_lblc.c b/net/netfilter/ipvs/ip_vs_lblc.c
index 222b6cb..df646cc 100644
--- a/net/netfilter/ipvs/ip_vs_lblc.c
+++ b/net/netfilter/ipvs/ip_vs_lblc.c
@@ -551,6 +551,9 @@ static int __net_init __ip_vs_lblc_init(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
+	if (!ipvs)
+		return -ENOENT;
+
 	if (!net_eq(net, &init_net)) {
 		ipvs->lblc_ctl_table = kmemdup(vs_vars_table,
 						sizeof(vs_vars_table),
diff --git a/net/netfilter/ipvs/ip_vs_lblcr.c b/net/netfilter/ipvs/ip_vs_lblcr.c
index 30c476f..570e31e 100644
--- a/net/netfilter/ipvs/ip_vs_lblcr.c
+++ b/net/netfilter/ipvs/ip_vs_lblcr.c
@@ -745,6 +745,9 @@ static int __net_init __ip_vs_lblcr_init(struct net *net)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
+	if (!ipvs)
+		return -ENOENT;
+
 	if (!net_eq(net, &init_net)) {
 		ipvs->lblcr_ctl_table = kmemdup(vs_vars_table,
 						sizeof(vs_vars_table),
-- 
1.7.2.3

^ permalink raw reply related

* [PATCH 0/2] ipvs: netns avoid crash at netns init
From: Hans Schillstrom @ 2012-04-25  7:43 UTC (permalink / raw)
  To: horms, ja, wensong, lvs-devel, netdev, netfilter-devel
  Cc: hans, Hans Schillstrom

Two simple patches that avoid crashes for no memory at netns init.

- Check return value of protocoll init.
- Null check of net-ipvs in shedulers.

Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
-- 
1.7.2.3

^ permalink raw reply

* [PATCH 2/4] ipvs: take care of return value from protocol init_netns
From: Hans Schillstrom @ 2012-04-25  7:43 UTC (permalink / raw)
  To: horms, ja, wensong, lvs-devel, netdev, netfilter-devel
  Cc: hans, Hans Schillstrom
In-Reply-To: <1335339821-29684-1-git-send-email-hans.schillstrom@ericsson.com>

ip_vs_create_timeout_table() can return NULL
All functions protocol init_netns is affected of this patch.

Signed-off-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
---
 include/net/ip_vs.h                   |    2 +-
 net/netfilter/ipvs/ip_vs_proto.c      |    2 +-
 net/netfilter/ipvs/ip_vs_proto_sctp.c |    5 ++++-
 net/netfilter/ipvs/ip_vs_proto_tcp.c  |    5 ++++-
 net/netfilter/ipvs/ip_vs_proto_udp.c  |    5 ++++-
 5 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index 990216d..f967395 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -392,7 +392,7 @@ struct ip_vs_protocol {
 
 	void (*exit)(struct ip_vs_protocol *pp);
 
-	void (*init_netns)(struct net *net, struct ip_vs_proto_data *pd);
+	int (*init_netns)(struct net *net, struct ip_vs_proto_data *pd);
 
 	void (*exit_netns)(struct net *net, struct ip_vs_proto_data *pd);
 
diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
index 6a8d176..0b74795 100644
--- a/net/netfilter/ipvs/ip_vs_proto.c
+++ b/net/netfilter/ipvs/ip_vs_proto.c
@@ -79,7 +79,7 @@ register_ip_vs_proto_netns(struct net *net, struct ip_vs_protocol *pp)
 	atomic_set(&pd->appcnt, 0);	/* Init app counter */
 
 	if (pp->init_netns != NULL)
-		pp->init_netns(net, pd);
+		return pp->init_netns(net, pd);
 
 	return 0;
 }
diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
index 1fbf7a2..9f3fb75 100644
--- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
@@ -1090,7 +1090,7 @@ out:
  *   timeouts is netns related now.
  * ---------------------------------------------
  */
-static void __ip_vs_sctp_init(struct net *net, struct ip_vs_proto_data *pd)
+static int __ip_vs_sctp_init(struct net *net, struct ip_vs_proto_data *pd)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
@@ -1098,6 +1098,9 @@ static void __ip_vs_sctp_init(struct net *net, struct ip_vs_proto_data *pd)
 	spin_lock_init(&ipvs->sctp_app_lock);
 	pd->timeout_table = ip_vs_create_timeout_table((int *)sctp_timeouts,
 							sizeof(sctp_timeouts));
+	if (!pd->timeout_table)
+		return -ENOMEM;
+	return 0;
 }
 
 static void __ip_vs_sctp_exit(struct net *net, struct ip_vs_proto_data *pd)
diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
index ef8641f..cd609cc 100644
--- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
@@ -677,7 +677,7 @@ void ip_vs_tcp_conn_listen(struct net *net, struct ip_vs_conn *cp)
  *   timeouts is netns related now.
  * ---------------------------------------------
  */
-static void __ip_vs_tcp_init(struct net *net, struct ip_vs_proto_data *pd)
+static int __ip_vs_tcp_init(struct net *net, struct ip_vs_proto_data *pd)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
@@ -685,7 +685,10 @@ static void __ip_vs_tcp_init(struct net *net, struct ip_vs_proto_data *pd)
 	spin_lock_init(&ipvs->tcp_app_lock);
 	pd->timeout_table = ip_vs_create_timeout_table((int *)tcp_timeouts,
 							sizeof(tcp_timeouts));
+	if (!pd->timeout_table)
+		return -ENOMEM;
 	pd->tcp_state_table =  tcp_states;
+	return 0;
 }
 
 static void __ip_vs_tcp_exit(struct net *net, struct ip_vs_proto_data *pd)
diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
index f4b7262..2fedb2d 100644
--- a/net/netfilter/ipvs/ip_vs_proto_udp.c
+++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
@@ -467,7 +467,7 @@ udp_state_transition(struct ip_vs_conn *cp, int direction,
 	cp->timeout = pd->timeout_table[IP_VS_UDP_S_NORMAL];
 }
 
-static void __udp_init(struct net *net, struct ip_vs_proto_data *pd)
+static int __udp_init(struct net *net, struct ip_vs_proto_data *pd)
 {
 	struct netns_ipvs *ipvs = net_ipvs(net);
 
@@ -475,6 +475,9 @@ static void __udp_init(struct net *net, struct ip_vs_proto_data *pd)
 	spin_lock_init(&ipvs->udp_app_lock);
 	pd->timeout_table = ip_vs_create_timeout_table((int *)udp_timeouts,
 							sizeof(udp_timeouts));
+	if (!pd->timeout_table)
+		return -ENOMEM;
+	return 0;
 }
 
 static void __udp_exit(struct net *net, struct ip_vs_proto_data *pd)
-- 
1.7.2.3


^ permalink raw reply related

* Re: [PATCH net-next] ipv6: RTAX_FEATURE_ALLFRAG causes inefficient TCP segment sizing
From: Maciej Żenczykowski @ 2012-04-25  7:34 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Tore Anderson, David Miller, netdev, Tom Herbert
In-Reply-To: <1335331960.5205.180.camel@edumazet-glaptop>

> But we chose to _not_ decrease mtu and adhere to the specs.

I get that we _choose_ to behave such, and I agree this adheres to specs.

But I'm not convinced that (even though this is allowed per RFC) this
is the right choice.

> Current linux chose to implement the allfragfeature, so match RFC 2460
> specs.
>
> If you want to reduce size of subsequent packets, its a lot more work in
> linux stack, for small gain, if any.

True, but this is work at the source host (and indeed might possibly
even be done by tso hardware or at least by gso),
which is reducing work at whatever the actual tunneling point is.
It's not actually affecting how much
work the receiver has to do (indeed it might even decrease it, imagine
sending 3840 bytes, getting that
fragmented into 6 packets: 1000, 280, 1000, 280, 1000, 280 instead of
just 4 packets: 1000, 1000,
1000, 840 [obviously would need to correct for header overhead, but
the basis stands])

[side note: indeed 1280 should never be split 1000 + 280, but should
rather be split into even pieces 640 + 640, to help prevent need for
further fragmentation further down the line]

Tunneling points are traditionally cpu starved.

Also note that IPv6 prefers to see fragmentation happen at the end
hosts, and not at the routers.
Although of course it doesn't treat a tunnel end point as a router.

Also packet loss is better handled by tcp losing segments than by the
tunnel losing a fragment and thus losing the entire packet.

I guess I simply think we're making the wrong trade off here.

> We only need to properly generate the fragment header, that means
> reducing the _payload_ by 8 bytes. Not reducing the _mtu_ that still is
> 1280, as allowed.
>
> IPv6 must cohabit with IPv4 for the next years, there is no hope
> thinking it can ignore tunnelings issues, since tunneling is part of the
> global IPv6 transition that is currently happening.

To be fair this probably isn't super-related to the vast majority of
IPv4, the vast majority of the IPv4 world out there has mtu's easily
in excess of 1280+tunneling overhead bytes.
I wonder if anyone has any statistics, but I'd guess 99.99% of the
IPv4 internet is 1400+ MTU.

^ permalink raw reply

* [PATCH 2/2] ss: implement -M option to get all memory information
From: Shan Wei @ 2012-04-25  7:15 UTC (permalink / raw)
  To: Stephen Hemminger, xemul, NetDev

From: Shan Wei <davidshan@tencent.com>

INET_DIAG_SKMEMINFO is used to monitor socket memory information
which contains more information than INET_DIAG_MEMINFO.

-m option is retained for old kernel that don't surpport INET_DIAG_SKMEMINFO.


Signed-off-by: Shan Wei <davidshan@tencent.com>
---
 misc/ss.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 5f70a26..3c8befd 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -45,6 +45,7 @@ int show_options = 0;
 int show_details = 0;
 int show_users = 0;
 int show_mem = 0;
+int show_skmem = 0;
 int show_tcpinfo = 0;
 
 int netid_width;
@@ -1410,6 +1411,19 @@ static void tcp_show_info(const struct nlmsghdr *nlh, struct inet_diag_msg *r)
 			printf(" rcv_space:%d", info->tcpi_rcv_space);
 
 	}
+
+	if (tb[INET_DIAG_SKMEMINFO]) {
+		const unsigned int *skmeminfo =  RTA_DATA(tb[INET_DIAG_SKMEMINFO]);
+		printf(" skmem:(r%u,rb%u,t%u,tb%u,f%u,w%u,o%u)",
+			skmeminfo[SK_MEMINFO_RMEM_ALLOC],
+			skmeminfo[SK_MEMINFO_RCVBUF],
+			skmeminfo[SK_MEMINFO_WMEM_ALLOC],
+			skmeminfo[SK_MEMINFO_SNDBUF],
+			skmeminfo[SK_MEMINFO_FWD_ALLOC],
+			skmeminfo[SK_MEMINFO_WMEM_QUEUED],
+			skmeminfo[SK_MEMINFO_OPTMEM]);
+	}
+
 }
 
 static int tcp_show_sock(struct nlmsghdr *nlh, struct filter *f)
@@ -1466,7 +1480,7 @@ static int tcp_show_sock(struct nlmsghdr *nlh, struct filter *f)
 			printf("%08x", r->id.idiag_cookie[1]);
  		printf("%08x", r->id.idiag_cookie[0]);
 	}
-	if (show_mem || show_tcpinfo) {
+	if (show_mem || show_tcpinfo || show_skmem) {
 		printf("\n\t");
 		tcp_show_info(nlh, r);
 	}
@@ -1508,6 +1522,9 @@ static int tcp_show_netlink(struct filter *f, FILE *dump_fp, int socktype)
 	if (show_mem)
 		req.r.idiag_ext |= (1<<(INET_DIAG_MEMINFO-1));
 
+	if (show_skmem)
+		req.r.idiag_ext |= (1<<(INET_DIAG_SKMEMINFO-1));
+
 	if (show_tcpinfo) {
 		req.r.idiag_ext |= (1<<(INET_DIAG_INFO-1));
 		req.r.idiag_ext |= (1<<(INET_DIAG_VEGASINFO-1));
@@ -2581,6 +2598,8 @@ static void _usage(FILE *dest)
 "   -o, --options       show timer information\n"
 "   -e, --extended      show detailed socket information\n"
 "   -m, --memory        show socket memory usage\n"
+"   -M, --skmem         show socket memory information.\n"
+"                       this option which will replace -m, is valid since Linux 3.3.\n"
 "   -p, --processes	show process using socket\n"
 "   -i, --info		show internal TCP information\n"
 "   -s, --summary	show socket usage summary\n"
@@ -2652,6 +2671,7 @@ static const struct option long_opts[] = {
 	{ "options", 0, 0, 'o' },
 	{ "extended", 0, 0, 'e' },
 	{ "memory", 0, 0, 'm' },
+	{ "skmem", 0, 0, 'M'}
 	{ "info", 0, 0, 'i' },
 	{ "processes", 0, 0, 'p' },
 	{ "dccp", 0, 0, 'd' },
@@ -2690,7 +2710,7 @@ int main(int argc, char *argv[])
 
 	current_filter.states = default_filter.states;
 
-	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spf:miA:D:F:vV",
+	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spf:mMiA:D:F:vV",
 				 long_opts, NULL)) != EOF) {
 		switch(ch) {
 		case 'n':
@@ -2709,6 +2729,9 @@ int main(int argc, char *argv[])
 		case 'm':
 			show_mem = 1;
 			break;
+		case 'M':
+			show_skmem = 1;
+			break;
 		case 'i':
 			show_tcpinfo = 1;
 			break;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 1/2] ss: fix the incorrect value of total UNIX_DIAG_* number
From: Shan Wei @ 2012-04-25  7:15 UTC (permalink / raw)
  To: xemul, Stephen Hemminger; +Cc: NetDev

From: Shan Wei <davidshan@tencent.com>

UNIX_DIAG_MAX is included in enum type.
It is equal to the total number of enum element.

But lots of enum MAX value is defined as the max enum element, e.g. INET_DIAG_MAX, XFRMA_MAX. 
The right fixing way seems to define UNIX_DIAG_MAX as UNIX_DIAG_MEMINFO,
but this way will break other user application.

So, just fix it on user application.


Signed-off-by: Shan Wei <davidshan@tencent.com>
---
 misc/ss.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 4017918..5f70a26 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2011,12 +2011,12 @@ void unix_list_print(struct unixstat *list, struct filter *f)
 static int unix_show_sock(struct nlmsghdr *nlh, struct filter *f)
 {
 	struct unix_diag_msg *r = NLMSG_DATA(nlh);
-	struct rtattr *tb[UNIX_DIAG_MAX+1];
+	struct rtattr *tb[UNIX_DIAG_MAX];
 	char name[128];
 	int peer_ino;
 	int rqlen;
 
-	parse_rtattr(tb, UNIX_DIAG_MAX, (struct rtattr*)(r+1),
+	parse_rtattr(tb, UNIX_DIAG_MAX-1, (struct rtattr*)(r+1),
 		     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
 
 	if (netid_width)
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH] isdn/eicon: use standard __init,__exit function markup
From: Armin Schindler @ 2012-04-25  6:48 UTC (permalink / raw)
  To: H Hartley Sweeten; +Cc: Linux Kernel, netdev, isdn
In-Reply-To: <201204241556.03516.hartleys@visionengravers.com>

What is the reason for this change?

Armin

On 04/25/2012 12:56 AM, H Hartley Sweeten wrote:
> Remove the custom DIVA_{INIT,EXIT}_FUNCTION defines and use
> the standard __init,__exit markup.
>
> Signed-off-by: H Hartley Sweeten<hsweeten@visionengravers.com>
> Cc: Armin Schindler<mac@melware.de>
> Cc: Karsten Keil<isdn@linux-pingi.de>
>
> ---
>
> diff --git a/drivers/isdn/hardware/eicon/capifunc.c b/drivers/isdn/hardware/eicon/capifunc.c
> index a576f32..7a0bdbd 100644
> --- a/drivers/isdn/hardware/eicon/capifunc.c
> +++ b/drivers/isdn/hardware/eicon/capifunc.c
> @@ -1120,7 +1120,7 @@ int fax_head_line_time(char *buffer)
>   /*
>    * init (alloc) main structures
>    */
> -static int DIVA_INIT_FUNCTION init_main_structs(void)
> +static int __init init_main_structs(void)
>   {
>   	if (!(mapped_msg = (CAPI_MSG *) diva_os_malloc(0, MAX_MSG_SIZE))) {
>   		DBG_ERR(("init: failed alloc mapped_msg."))
> @@ -1181,7 +1181,7 @@ static void do_api_remove_start(void)
>   /*
>    * init
>    */
> -int DIVA_INIT_FUNCTION init_capifunc(void)
> +int __init init_capifunc(void)
>   {
>   	diva_os_initialize_spin_lock(&api_lock, "capifunc");
>   	memset(ControllerMap, 0, MAX_DESCRIPTORS + 1);
> @@ -1209,7 +1209,7 @@ int DIVA_INIT_FUNCTION init_capifunc(void)
>   /*
>    * finit
>    */
> -void DIVA_EXIT_FUNCTION finit_capifunc(void)
> +void __exit finit_capifunc(void)
>   {
>   	do_api_remove_start();
>   	divacapi_disconnect_didd();
> diff --git a/drivers/isdn/hardware/eicon/capimain.c b/drivers/isdn/hardware/eicon/capimain.c
> index eabe0fa..997d46a 100644
> --- a/drivers/isdn/hardware/eicon/capimain.c
> +++ b/drivers/isdn/hardware/eicon/capimain.c
> @@ -118,7 +118,7 @@ void diva_os_set_controller_struct(struct capi_ctr *ctrl)
>   /*
>    * module init
>    */
> -static int DIVA_INIT_FUNCTION divacapi_init(void)
> +static int __init divacapi_init(void)
>   {
>   	char tmprev[32];
>   	int ret = 0;
> @@ -144,7 +144,7 @@ static int DIVA_INIT_FUNCTION divacapi_init(void)
>   /*
>    * module exit
>    */
> -static void DIVA_EXIT_FUNCTION divacapi_exit(void)
> +static void __exit divacapi_exit(void)
>   {
>   	finit_capifunc();
>   	printk(KERN_INFO "%s: module unloaded.\n", DRIVERLNAME);
> diff --git a/drivers/isdn/hardware/eicon/diddfunc.c b/drivers/isdn/hardware/eicon/diddfunc.c
> index c4c8220..b0b23ed 100644
> --- a/drivers/isdn/hardware/eicon/diddfunc.c
> +++ b/drivers/isdn/hardware/eicon/diddfunc.c
> @@ -47,7 +47,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
>   /*
>    * connect to didd
>    */
> -static int DIVA_INIT_FUNCTION connect_didd(void)
> +static int __init connect_didd(void)
>   {
>   	int x = 0;
>   	int dadapter = 0;
> @@ -79,7 +79,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
>   /*
>    * disconnect from didd
>    */
> -static void DIVA_EXIT_FUNCTION disconnect_didd(void)
> +static void __exit disconnect_didd(void)
>   {
>   	IDI_SYNC_REQ req;
>
> @@ -92,7 +92,7 @@ static void DIVA_EXIT_FUNCTION disconnect_didd(void)
>   /*
>    * init
>    */
> -int DIVA_INIT_FUNCTION diddfunc_init(void)
> +int __init diddfunc_init(void)
>   {
>   	diva_didd_load_time_init();
>
> @@ -107,7 +107,7 @@ int DIVA_INIT_FUNCTION diddfunc_init(void)
>   /*
>    * finit
>    */
> -void DIVA_EXIT_FUNCTION diddfunc_finit(void)
> +void __exit diddfunc_finit(void)
>   {
>   	DbgDeregister();
>   	disconnect_didd();
> diff --git a/drivers/isdn/hardware/eicon/diva_didd.c b/drivers/isdn/hardware/eicon/diva_didd.c
> index d1d3de0..fab6ccf 100644
> --- a/drivers/isdn/hardware/eicon/diva_didd.c
> +++ b/drivers/isdn/hardware/eicon/diva_didd.c
> @@ -91,7 +91,7 @@ static const struct file_operations divadidd_proc_fops = {
>   	.release	= single_release,
>   };
>
> -static int DIVA_INIT_FUNCTION create_proc(void)
> +static int __init create_proc(void)
>   {
>   	proc_net_eicon = proc_mkdir("eicon", init_net.proc_net);
>
> @@ -109,7 +109,7 @@ static void remove_proc(void)
>   	remove_proc_entry("eicon", init_net.proc_net);
>   }
>
> -static int DIVA_INIT_FUNCTION divadidd_init(void)
> +static int __init divadidd_init(void)
>   {
>   	char tmprev[32];
>   	int ret = 0;
> @@ -141,7 +141,7 @@ out:
>   	return (ret);
>   }
>
> -static void DIVA_EXIT_FUNCTION divadidd_exit(void)
> +static void __exit divadidd_exit(void)
>   {
>   	diddfunc_finit();
>   	remove_proc();
> diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c
> index ffa0c31..48db08d 100644
> --- a/drivers/isdn/hardware/eicon/divamnt.c
> +++ b/drivers/isdn/hardware/eicon/divamnt.c
> @@ -184,7 +184,7 @@ static void divas_maint_unregister_chrdev(void)
>   	unregister_chrdev(major, DEVNAME);
>   }
>
> -static int DIVA_INIT_FUNCTION divas_maint_register_chrdev(void)
> +static int __init divas_maint_register_chrdev(void)
>   {
>   	if ((major = register_chrdev(0, DEVNAME,&divas_maint_fops))<  0)
>   	{
> @@ -207,7 +207,7 @@ void diva_maint_wakeup_read(void)
>   /*
>    *  Driver Load
>    */
> -static int DIVA_INIT_FUNCTION maint_init(void)
> +static int __init maint_init(void)
>   {
>   	char tmprev[50];
>   	int ret = 0;
> @@ -245,7 +245,7 @@ out:
>   /*
>   **  Driver Unload
>   */
> -static void DIVA_EXIT_FUNCTION maint_exit(void)
> +static void __exit maint_exit(void)
>   {
>   	divas_maint_unregister_chrdev();
>   	mntfunc_finit();
> diff --git a/drivers/isdn/hardware/eicon/divasfunc.c b/drivers/isdn/hardware/eicon/divasfunc.c
> index 60aaf95..4be5f88 100644
> --- a/drivers/isdn/hardware/eicon/divasfunc.c
> +++ b/drivers/isdn/hardware/eicon/divasfunc.c
> @@ -153,7 +153,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
>   /*
>    * connect to didd
>    */
> -static int DIVA_INIT_FUNCTION connect_didd(void)
> +static int __init connect_didd(void)
>   {
>   	int x = 0;
>   	int dadapter = 0;
> @@ -209,7 +209,7 @@ static void disconnect_didd(void)
>   /*
>    * init
>    */
> -int DIVA_INIT_FUNCTION divasfunc_init(int dbgmask)
> +int __init divasfunc_init(int dbgmask)
>   {
>   	char *version;
>
> diff --git a/drivers/isdn/hardware/eicon/divasi.c b/drivers/isdn/hardware/eicon/divasi.c
> index a5c8f90..4103a8c 100644
> --- a/drivers/isdn/hardware/eicon/divasi.c
> +++ b/drivers/isdn/hardware/eicon/divasi.c
> @@ -114,7 +114,7 @@ static const struct file_operations um_idi_proc_fops = {
>   	.release	= single_release,
>   };
>
> -static int DIVA_INIT_FUNCTION create_um_idi_proc(void)
> +static int __init create_um_idi_proc(void)
>   {
>   	um_idi_proc_entry = proc_create(DRIVERLNAME, S_IRUGO, proc_net_eicon,
>   					&um_idi_proc_fops);
> @@ -146,7 +146,7 @@ static void divas_idi_unregister_chrdev(void)
>   	unregister_chrdev(major, DEVNAME);
>   }
>
> -static int DIVA_INIT_FUNCTION divas_idi_register_chrdev(void)
> +static int __init divas_idi_register_chrdev(void)
>   {
>   	if ((major = register_chrdev(0, DEVNAME,&divas_idi_fops))<  0)
>   	{
> @@ -161,7 +161,7 @@ static int DIVA_INIT_FUNCTION divas_idi_register_chrdev(void)
>   /*
>   ** Driver Load
>   */
> -static int DIVA_INIT_FUNCTION divasi_init(void)
> +static int __init divasi_init(void)
>   {
>   	char tmprev[50];
>   	int ret = 0;
> @@ -202,7 +202,7 @@ out:
>   /*
>   ** Driver Unload
>   */
> -static void DIVA_EXIT_FUNCTION divasi_exit(void)
> +static void __exit divasi_exit(void)
>   {
>   	idifunc_finit();
>   	remove_um_idi_proc();
> diff --git a/drivers/isdn/hardware/eicon/divasmain.c b/drivers/isdn/hardware/eicon/divasmain.c
> index 7eaab06..ca6d276 100644
> --- a/drivers/isdn/hardware/eicon/divasmain.c
> +++ b/drivers/isdn/hardware/eicon/divasmain.c
> @@ -673,7 +673,7 @@ static void divas_unregister_chrdev(void)
>   	unregister_chrdev(major, DEVNAME);
>   }
>
> -static int DIVA_INIT_FUNCTION divas_register_chrdev(void)
> +static int __init divas_register_chrdev(void)
>   {
>   	if ((major = register_chrdev(0, DEVNAME,&divas_fops))<  0)
>   	{
> @@ -767,7 +767,7 @@ static void __devexit divas_remove_one(struct pci_dev *pdev)
>   /* --------------------------------------------------------------------------
>      Driver Load / Startup
>      -------------------------------------------------------------------------- */
> -static int DIVA_INIT_FUNCTION divas_init(void)
> +static int __init divas_init(void)
>   {
>   	char tmprev[50];
>   	int ret = 0;
> @@ -831,7 +831,7 @@ out:
>   /* --------------------------------------------------------------------------
>      Driver Unload
>      -------------------------------------------------------------------------- */
> -static void DIVA_EXIT_FUNCTION divas_exit(void)
> +static void __exit divas_exit(void)
>   {
>   	pci_unregister_driver(&diva_pci_driver);
>   	remove_divas_proc();
> diff --git a/drivers/isdn/hardware/eicon/idifunc.c b/drivers/isdn/hardware/eicon/idifunc.c
> index d153e3c..fef6586 100644
> --- a/drivers/isdn/hardware/eicon/idifunc.c
> +++ b/drivers/isdn/hardware/eicon/idifunc.c
> @@ -133,7 +133,7 @@ static void um_remove_card(DESCRIPTOR *d)
>   /*
>    * remove all adapter
>    */
> -static void DIVA_EXIT_FUNCTION remove_all_idi_proc(void)
> +static void __exit remove_all_idi_proc(void)
>   {
>   	udiva_card *card;
>   	diva_os_spin_lock_magic_t old_irql;
> @@ -181,7 +181,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
>   /*
>    * connect DIDD
>    */
> -static int DIVA_INIT_FUNCTION connect_didd(void)
> +static int __init connect_didd(void)
>   {
>   	int x = 0;
>   	int dadapter = 0;
> @@ -225,7 +225,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
>   /*
>    *  Disconnect from DIDD
>    */
> -static void DIVA_EXIT_FUNCTION disconnect_didd(void)
> +static void __exit disconnect_didd(void)
>   {
>   	IDI_SYNC_REQ req;
>
> @@ -240,7 +240,7 @@ static void DIVA_EXIT_FUNCTION disconnect_didd(void)
>   /*
>    * init
>    */
> -int DIVA_INIT_FUNCTION idifunc_init(void)
> +int __init idifunc_init(void)
>   {
>   	diva_os_initialize_spin_lock(&ll_lock, "idifunc");
>
> @@ -260,7 +260,7 @@ int DIVA_INIT_FUNCTION idifunc_init(void)
>   /*
>    * finit
>    */
> -void DIVA_EXIT_FUNCTION idifunc_finit(void)
> +void __exit idifunc_finit(void)
>   {
>   	diva_user_mode_idi_finit();
>   	disconnect_didd();
> diff --git a/drivers/isdn/hardware/eicon/mntfunc.c b/drivers/isdn/hardware/eicon/mntfunc.c
> index d607260..1cd9aff 100644
> --- a/drivers/isdn/hardware/eicon/mntfunc.c
> +++ b/drivers/isdn/hardware/eicon/mntfunc.c
> @@ -72,7 +72,7 @@ static void *didd_callback(void *context, DESCRIPTOR *adapter,
>   /*
>    * connect to didd
>    */
> -static int DIVA_INIT_FUNCTION connect_didd(void)
> +static int __init connect_didd(void)
>   {
>   	int x = 0;
>   	int dadapter = 0;
> @@ -114,7 +114,7 @@ static int DIVA_INIT_FUNCTION connect_didd(void)
>   /*
>    * disconnect from didd
>    */
> -static void DIVA_EXIT_FUNCTION disconnect_didd(void)
> +static void __exit disconnect_didd(void)
>   {
>   	IDI_SYNC_REQ req;
>
> @@ -300,7 +300,7 @@ int maint_read_write(void __user *buf, int count)
>   /*
>    *  init
>    */
> -int DIVA_INIT_FUNCTION mntfunc_init(int *buffer_length, void **buffer,
> +int __init mntfunc_init(int *buffer_length, void **buffer,
>   				    unsigned long diva_dbg_mem)
>   {
>   	if (*buffer_length<  64) {
> @@ -348,7 +348,7 @@ int DIVA_INIT_FUNCTION mntfunc_init(int *buffer_length, void **buffer,
>   /*
>    *  exit
>    */
> -void DIVA_EXIT_FUNCTION mntfunc_finit(void)
> +void __exit mntfunc_finit(void)
>   {
>   	void *buffer;
>   	int i = 100;
> diff --git a/drivers/isdn/hardware/eicon/platform.h b/drivers/isdn/hardware/eicon/platform.h
> index 7331c3b..b2edb75 100644
> --- a/drivers/isdn/hardware/eicon/platform.h
> +++ b/drivers/isdn/hardware/eicon/platform.h
> @@ -38,9 +38,6 @@
>   #define DIVA_NO_DEBUGLIB
>   #endif
>
> -#define DIVA_INIT_FUNCTION  __init
> -#define DIVA_EXIT_FUNCTION  __exit
> -
>   #define DIVA_USER_MODE_CARD_CONFIG 1
>   #define	USE_EXTENDED_DEBUGS 1
>


-- 
Mit freundlichen Grüßen / Best regards
Armin Schindler

Cytronics & Melware
Weinbergstrasse 39, 55296 Loerzweiler / Germany
Tel: +49 6138 99998-100
Fax: +49 6138 99998-109
VoIP: sip:info@melware.net
mailto:info@melware.de
http://www.melware.de

^ permalink raw reply

* [PATCH 2/4] mISDN: Using FLG_ACTIVE flag to determine if layer 1 is active or not.
From: Karsten Keil @ 2012-04-25  6:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Andreas Eversberg
In-Reply-To: <1335336734-25181-1-git-send-email-kkeil@linux-pingi.de>

From: Andreas Eversberg <andreas@eversberg.eu>

We already have the flag for L1 active, so we should use it.
L2 will be solved in a later patch.

Signed-off-by: Andreas Eversberg <andreas@eversberg.eu>
Signed-off-by: Karsten Keil <keil@b1-systems.de>
---
 drivers/isdn/hardware/mISDN/hfcmulti.c |   25 ++++++++++---------------
 1 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/hfcmulti.c b/drivers/isdn/hardware/mISDN/hfcmulti.c
index c4e7fda..cc978e8 100644
--- a/drivers/isdn/hardware/mISDN/hfcmulti.c
+++ b/drivers/isdn/hardware/mISDN/hfcmulti.c
@@ -1619,10 +1619,16 @@ hfcmulti_leds(struct hfc_multi *hc)
 		 * 2 red steady:   TE mode deactivate
 		 * left green:     L1 active
 		 * left red:       frame sync, but no L1
-		 * right green:    L2 active
+		 * todo right green:    L2 active
 		 */
-		if (hc->chan[hc->dslot].sync != 2) { /* no frame sync */
-			if (hc->chan[hc->dslot].dch->dev.D.protocol
+		dch = hc->chan[hc->dslot].dch;
+		if (test_bit(FLG_ACTIVE, &dch->Flags)) {
+			led[0] = 0;
+			led[1] = 0;
+			led[2] = 0;
+			led[3] = 1;
+		} else {
+			if (dch->dev.D.protocol
 			    != ISDN_P_NT_E1) {
 				led[0] = 1;
 				led[1] = 1;
@@ -1635,12 +1641,6 @@ hfcmulti_leds(struct hfc_multi *hc)
 			}
 			led[2] = 0;
 			led[3] = 0;
-		} else { /* with frame sync */
-			/* TODO make it work */
-			led[0] = 0;
-			led[1] = 0;
-			led[2] = 0;
-			led[3] = 1;
 		}
 		leds = (led[0] | (led[1]<<2) | (led[2]<<1) | (led[3]<<3))^0xF;
 		/* leds are inverted */
@@ -4062,14 +4062,9 @@ open_dchannel(struct hfc_multi *hc, struct dchannel *dch,
 		hfcmulti_initmode(dch);
 		spin_unlock_irqrestore(&hc->lock, flags);
 	}
-
-	if (((rq->protocol == ISDN_P_NT_S0) && (dch->state == 3)) ||
-	    ((rq->protocol == ISDN_P_TE_S0) && (dch->state == 7)) ||
-	    ((rq->protocol == ISDN_P_NT_E1) && (dch->state == 1)) ||
-	    ((rq->protocol == ISDN_P_TE_E1) && (dch->state == 1))) {
+	if (test_bit(FLG_ACTIVE, &dch->Flags))
 		_queue_data(&dch->dev.D, PH_ACTIVATE_IND, MISDN_ID_ANY,
 			    0, NULL, GFP_KERNEL);
-	}
 	rq->ch = &dch->dev.D;
 	if (!try_module_get(THIS_MODULE))
 		printk(KERN_WARNING "%s:cannot get module\n", __func__);
-- 
1.7.3.4

^ permalink raw reply related

* [PATCH 1/4] mISDN: Fixed false interruption of audio during bridging change.
From: Karsten Keil @ 2012-04-25  6:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Andreas Eversberg
In-Reply-To: <1335336734-25181-1-git-send-email-kkeil@linux-pingi.de>

From: Andreas Eversberg <jolly@eversberg.eu>

Transmitted audio data was interrupted if a bridge was enabled or disabled.
Now transmission seamlessly continues during that action.
Fix in hfcmulti.ko

Signed-off-by: Andreas Eversberg <jolly@eversberg.eu>
Signed-off-by: Karsten Keil <keil@b1-systems.de>
---
 drivers/isdn/hardware/mISDN/hfcmulti.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/isdn/hardware/mISDN/hfcmulti.c b/drivers/isdn/hardware/mISDN/hfcmulti.c
index 0332231..c4e7fda 100644
--- a/drivers/isdn/hardware/mISDN/hfcmulti.c
+++ b/drivers/isdn/hardware/mISDN/hfcmulti.c
@@ -3018,8 +3018,10 @@ mode_hfcmulti(struct hfc_multi *hc, int ch, int protocol, int slot_tx,
 			HFC_outb(hc, A_CON_HDLC, 0x20 | V_HDLC_TRP | V_IFF);
 			HFC_outb(hc, A_SUBCH_CFG, 0);
 			HFC_outb(hc, A_IRQ_MSK, 0);
-			HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
-			HFC_wait(hc);
+			if (hc->chan[ch].protocol != protocol) {
+				HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
+				HFC_wait(hc);
+			}
 			HFC_outb(hc, R_SLOT, ((((ch / 4) * 8) +
 					       ((ch % 4) * 4) + 1) << 1) | 1);
 			HFC_outb(hc, A_SL_CFG, 0x80 | 0x20 | (ch << 1) | 1);
@@ -3039,8 +3041,10 @@ mode_hfcmulti(struct hfc_multi *hc, int ch, int protocol, int slot_tx,
 			HFC_outb(hc, A_CON_HDLC, 0x20 | V_HDLC_TRP | V_IFF);
 			HFC_outb(hc, A_SUBCH_CFG, 0);
 			HFC_outb(hc, A_IRQ_MSK, 0);
-			HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
-			HFC_wait(hc);
+			if (hc->chan[ch].protocol != protocol) {
+				HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
+				HFC_wait(hc);
+			}
 			/* tx silence */
 			HFC_outb_nodebug(hc, A_FIFO_DATA0_NOINC, hc->silence);
 			HFC_outb(hc, R_SLOT, (((ch / 4) * 8) +
@@ -3059,8 +3063,10 @@ mode_hfcmulti(struct hfc_multi *hc, int ch, int protocol, int slot_tx,
 					 V_HDLC_TRP | V_IFF);
 			HFC_outb(hc, A_SUBCH_CFG, 0);
 			HFC_outb(hc, A_IRQ_MSK, 0);
-			HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
-			HFC_wait(hc);
+			if (hc->chan[ch].protocol != protocol) {
+				HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
+				HFC_wait(hc);
+			}
 			/* tx silence */
 			HFC_outb_nodebug(hc, A_FIFO_DATA0_NOINC, hc->silence);
 			/* enable RX fifo */
@@ -3075,8 +3081,10 @@ mode_hfcmulti(struct hfc_multi *hc, int ch, int protocol, int slot_tx,
 					 V_HDLC_TRP);
 			HFC_outb(hc, A_SUBCH_CFG, 0);
 			HFC_outb(hc, A_IRQ_MSK, 0);
-			HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
-			HFC_wait(hc);
+			if (hc->chan[ch].protocol != protocol) {
+				HFC_outb(hc, R_INC_RES_FIFO, V_RES_F);
+				HFC_wait(hc);
+			}
 		}
 		if (hc->ctype != HFC_TYPE_E1) {
 			hc->hw.a_st_ctrl0[hc->chan[ch].port] |=
-- 
1.7.3.4

^ permalink raw reply related


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