* Re: [PATCH] xfrm: export xfrm garbage collector thresholds via sysctl
From: Joe Nall @ 2009-07-27 20:02 UTC (permalink / raw)
To: David Miller
Cc: nhorman, netdev, herbert, kuznet, pekkas, jmorris, yoshfuji,
kaber
In-Reply-To: <20090727.124011.05728493.davem@davemloft.net>
On Jul 27, 2009, at 2:40 PM, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon, 27 Jul 2009 15:36:25 -0400
>
>> I think that makes sense, since it means we only keep cache entries
>> for active connections, and clean them up as soon as they close
>> (e.g. I don't really see the advantage to unhashing a xfrm cache
>> entry only to recreate it on the next packet sent).
>
> How is this related to the user's problem?
>
> My impression was that they were forwarding IPSEC traffic when
> running up against these limits, and that has no socket based
> assosciation at all.
>
> Making the XFRM GC limits get computed similarly to how the
> ipv4/ipv6 one's do probably makes sense.
The problem was seen serving TCP connections over IPSec when the
server (a 24 core machine w 32GB RAM) and the IPSec host were the same
(transport not tunnel).
The problem was originally identified in an MLS ipsec thin client
stress test with 25 clients and 200+ windows per client.
We then duplicated the issue with single level xclocks on the same
hardware.
I then duplicated the problem with two bone stock (not MLS) F10 (and
later F11) boxes running ab (apache benchmark tool) with 10k requests
over 2k concurrent connections. See https://bugzilla.redhat.com/show_bug.cgi?id=503124
With the gc_thresh raised to 8k, my ab test passes with 5k+
connections and 100k requests. Our test guys ran 11 workstations with
about 2200 concurrent X connections over the weekend and I'm hoping
for some MLS results before we lose the test suite on Wednesday.
joe
^ permalink raw reply
* Re: [PATCH] xfrm: export xfrm garbage collector thresholds via sysctl
From: David Miller @ 2009-07-27 19:40 UTC (permalink / raw)
To: nhorman; +Cc: netdev, joe, herbert, kuznet, pekkas, jmorris, yoshfuji, kaber
In-Reply-To: <20090727193625.GD15823@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 27 Jul 2009 15:36:25 -0400
> I think that makes sense, since it means we only keep cache entries
> for active connections, and clean them up as soon as they close
> (e.g. I don't really see the advantage to unhashing a xfrm cache
> entry only to recreate it on the next packet sent).
How is this related to the user's problem?
My impression was that they were forwarding IPSEC traffic when
running up against these limits, and that has no socket based
assosciation at all.
Making the XFRM GC limits get computed similarly to how the
ipv4/ipv6 one's do probably makes sense.
^ permalink raw reply
* Re: [PATCH NEXT 12/12] netxen: update version to 4.0.41
From: David Miller @ 2009-07-27 19:38 UTC (permalink / raw)
To: dhananjay.phadke; +Cc: netdev
In-Reply-To: <7608421F3572AB4292BB2532AE89D56581877E22A4@AVEXMB1.qlogic.org>
From: Dhananjay Phadke <dhananjay.phadke@qlogic.com>
Date: Mon, 27 Jul 2009 12:26:58 -0700
> Ok, the idea for module params was to set global defaults
> (configured in modprobe.conf) vs. per interface ethtool calls.
> The ethtool support is anyway added by # 12 patch.
>
> Is it not good to have both options available?
No, it is not good.
With module parameters we:
1) duplicate functionality
2) every device driver will name the module parms differently,
therefore the interface for the user is different for every device
type
We discuss this all the time, and this is a pretty deep rooted
belief and requirement for all network drivers.
^ permalink raw reply
* Re: [PATCH] xfrm: export xfrm garbage collector thresholds via sysctl
From: Neil Horman @ 2009-07-27 19:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, joe, herbert, kuznet, pekkas, jmorris, yoshfuji, kaber
In-Reply-To: <20090727.113755.260927092.davem@davemloft.net>
On Mon, Jul 27, 2009 at 11:37:55AM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon, 27 Jul 2009 14:22:46 -0400
>
> > Export garbage collector thresholds for xfrm[4|6]_dst_ops
> >
> > Had a problem reported to me recently in which a high volume of ipsec
> > connections on a system began reporting ENOBUFS for new connections eventually.
> > It seemed that after about 2000 connections we started being unable to create
> > more. A quick look revealed that the xfrm code used a dst_ops structure that
> > limited the gc_thresh value to 1024, and alaways dropped route cache entries
> > after 2x the gc_thresh. It seems the most direct solution is to export the
> > gc_thresh values in the xfrm[4|6] dst_ops as sysctls, like the main routing
> > table does, so that higher volumes of connections can be supported. This patch
> > has been tested and allows the reporter to increase their ipsec connection
> > volume successfully.
> >
> > Reported-by: Joe Nall <joe@nall.com>
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>
> Applied, but this suggests that either:
>
Thanks!
> 1) we pick a horrible default
>
> 2) our IPSEC machinery holds onto dst entries too tightly and that's
> the true cause of this problem
>
> I'd like to ask that you investigate this, because with defaults
> we should be able to handle IPSEC loads as high as the routing
> loads we could handle.
>
I'll gladly look into this further. Compared to the main routing table, the
ipsec default selection is pretty bad. Its statcially set despite the size of
the larger routing table. Looking at the garbage collection algorithm, we do
keep a pretty tight leash on freeing entries, but I think its warrented. We
create 1 dst_entry for each open socket on an ipsec tunnel, and don't release it
until its __refcnt drops to zero. I think that makes sense, since it means
we only keep cache entries for active connections, and clean them up as soon as
they close (e.g. I don't really see the advantage to unhashing a xfrm cache
entry only to recreate it on the next packet sent). I think the most sensible
first step is to dynamically choose a gc threshold based on the size of memory
or the main routing table. I'll write this up and post later this week after I
do some testing. Thanks!
Neil
> Thanks.
>
^ permalink raw reply
* RE: [PATCH NEXT 12/12] netxen: update version to 4.0.41
From: Dhananjay Phadke @ 2009-07-27 19:26 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20090727.105403.202219074.davem@davemloft.net>
Ok, the idea for module params was to set global defaults
(configured in modprobe.conf) vs. per interface ethtool calls.
The ethtool support is anyway added by # 12 patch.
Is it not good to have both options available?
I will sync up my tree and see if the merge is functionally
correct.
Thanks,
Dhananjay
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, July 27, 2009 10:54
> To: Dhananjay Phadke
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH NEXT 12/12] netxen: update version to 4.0.41
>
>
> Ok, in order to make this build I'm going to have to insert
> some parts of patch #6 which I rejected because that adds
> ring size defaults/max/etc. defines which get used by later
> patches.
>
> No virus found in this incoming message.
> Checked by AVG - www.avg.com
> Version: 8.5.392 / Virus Database: 270.13.25/2256 - Release
> Date: 07/27/09 05:58:00
>
^ permalink raw reply
* Re: [PATCH v2] IPVS: logging sizeof(struct ip_vs_conn) on startup
From: David Miller @ 2009-07-27 18:48 UTC (permalink / raw)
To: heder; +Cc: netdev, eric.dumazet, lvs-users, malcolm, linux-kernel, horms
In-Reply-To: <20090727131357.9561.39720.stgit@jazzy.zrh.corp.google.com>
From: Hannes Eder <heder@google.com>
Date: Mon, 27 Jul 2009 15:13:57 +0200
> No more guessing, how much memory used by IPVS for a connection.
>
> [ The number printed is a lower bound, as ip_vs_conn_cache uses
> SLAB_HWCACHE_ALIGN. ]
>
> Signed-off-by: Hannes Eder <heder@google.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
Who needs to know this? A developer?
They can run tools such as 'pahole' et al. to fish out this
information even when only a kernel binary is available.
For the user, it's just noise.
I'm not applying this, sorry.
^ permalink raw reply
* Re: [RFC][PATCH 5/5] libxt_ipvs: user space lib for netfilter matcher xt_ipvs
From: Jan Engelhardt @ 2009-07-27 18:40 UTC (permalink / raw)
To: Hannes Eder; +Cc: lvs-devel, netdev, netfilter-devel, linux-kernel
In-Reply-To: <20090727134852.13319.39035.stgit@jazzy.zrh.corp.google.com>
On Monday 2009-07-27 15:48, Hannes Eder wrote:
>+
>+ switch (c) {
>+ case '0': /* --ipvs */
>+ /* Nothing to do here. */
Then why add it?
>+ char buf[BUFSIZ];
>+
>+ if (family == NFPROTO_IPV4) {
>+ if (!numeric && addr->ip == 0) {
>+ printf("anywhere ");
>+ return;
>+ }
>+ if (numeric)
>+ strcpy(buf, xtables_ipaddr_to_numeric(&addr->in));
>+ else
>+ strcpy(buf, xtables_ipaddr_to_anyname(&addr->in));
>+ strcat(buf, xtables_ipmask_to_numeric(&mask->in));
>+ printf("%s ", buf);
There is no need to use the strcpy/strcat hacks. Just directly printf it.
>--- /dev/null
>+++ b/extensions/libxt_ipvs.man
>@@ -0,0 +1,7 @@
>+ipvs tests where the packet was modified by IPVS, i.e. is the
>+skb_buff->ipvs_property set.
>+.TP
>+[\fB!\fP] \fB--ipvs
>+Does the packet have to IPVS property?
>+
>+TODO: Write proper documentation.
Yes.
^ permalink raw reply
* Re: [PATCH] xfrm: export xfrm garbage collector thresholds via sysctl
From: David Miller @ 2009-07-27 18:37 UTC (permalink / raw)
To: nhorman; +Cc: netdev, joe, herbert, kuznet, pekkas, jmorris, yoshfuji, kaber
In-Reply-To: <20090727182246.GC15823@hmsreliant.think-freely.org>
From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 27 Jul 2009 14:22:46 -0400
> Export garbage collector thresholds for xfrm[4|6]_dst_ops
>
> Had a problem reported to me recently in which a high volume of ipsec
> connections on a system began reporting ENOBUFS for new connections eventually.
> It seemed that after about 2000 connections we started being unable to create
> more. A quick look revealed that the xfrm code used a dst_ops structure that
> limited the gc_thresh value to 1024, and alaways dropped route cache entries
> after 2x the gc_thresh. It seems the most direct solution is to export the
> gc_thresh values in the xfrm[4|6] dst_ops as sysctls, like the main routing
> table does, so that higher volumes of connections can be supported. This patch
> has been tested and allows the reporter to increase their ipsec connection
> volume successfully.
>
> Reported-by: Joe Nall <joe@nall.com>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Applied, but this suggests that either:
1) we pick a horrible default
2) our IPSEC machinery holds onto dst entries too tightly and that's
the true cause of this problem
I'd like to ask that you investigate this, because with defaults
we should be able to handle IPSEC loads as high as the routing
loads we could handle.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] net: ethtool_op_get_rx_csum() should be public and exported
From: David Miller @ 2009-07-27 18:36 UTC (permalink / raw)
To: eric.dumazet; +Cc: sri, netdev
In-Reply-To: <4A6D70D3.8030408@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Jul 2009 11:18:11 +0200
> [PATCH] net: ethtool_op_get_rx_csum() should be public and exported
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Fair enough, applied, but now you are on the hook to add at
least one user of this :-)
^ permalink raw reply
* Re: [PATCH 11/12] net/netlabel: Correct redundant test
From: David Miller @ 2009-07-27 18:36 UTC (permalink / raw)
To: paul.moore; +Cc: julia, netdev, linux-kernel, kernel-janitors
In-Reply-To: <200907271337.30401.paul.moore@hp.com>
From: Paul Moore <paul.moore@hp.com>
Date: Mon, 27 Jul 2009 13:37:29 -0400
> On Monday 27 July 2009 12:15:43 pm Julia Lawall wrote:
>> From: Julia Lawall <julia@diku.dk>
>>
>> entry was tested for NULL near the beginning of the function, followed by a
>> return, and there is no intervening modification of its value.
>>
>> A simplified version of the semantic match that finds this problem is as
>> follows: (http://www.emn.fr/x-info/coccinelle/)
...
>> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> Good catch, thanks.
>
> Acked-by: Paul Moore <paul.moore@hp.com>
Applied to net-next-2.6
^ permalink raw reply
* Re: [PATCH 4/12] drivers/net: Correct redundant test
From: David Miller @ 2009-07-27 18:36 UTC (permalink / raw)
To: julia; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.0907271813120.2682@ask.diku.dk>
From: Julia Lawall <julia@diku.dk>
Date: Mon, 27 Jul 2009 18:13:30 +0200 (CEST)
> From: Julia Lawall <julia@diku.dk>
>
> res has already been tested. It seems that this test should be on the
> recently returned value mmio.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://www.emn.fr/x-info/coccinelle/)
...
> Signed-off-by: Julia Lawall <julia@diku.dk>
Applied to net-next-2.6
^ permalink raw reply
* Re: [RFC][PATCH 2/5] netfilter: xt_ipvs (netfilter matcher for ipvs)
From: Jan Engelhardt @ 2009-07-27 18:35 UTC (permalink / raw)
To: Hannes Eder; +Cc: lvs-devel, netdev, netfilter-devel, linux-kernel
In-Reply-To: <20090727134621.12897.75558.stgit@jazzy.zrh.corp.google.com>
On Monday 2009-07-27 15:46, Hannes Eder wrote:
>--- /dev/null
>+++ b/include/linux/netfilter/xt_ipvs.h
>@@ -0,0 +1,32 @@
>+#ifndef _XT_IPVS_H
>+#define _XT_IPVS_H 1
>+
>+#ifndef _IP_VS_H
Do the definitions (should probably be enums) exist in
very old <linux/ip_vs.h>? Then maybe you can get rid of them
from xt_ipvs.h.
>+#define IP_VS_CONN_F_FWD_MASK 0x0007 /* mask for the fwd methods */
>+#define IP_VS_CONN_F_MASQ 0x0000 /* masquerading/NAT */
>+#define IP_VS_CONN_F_LOCALNODE 0x0001 /* local node */
>+#define IP_VS_CONN_F_TUNNEL 0x0002 /* tunneling */
>+#define IP_VS_CONN_F_DROUTE 0x0003 /* direct routing */
>+#define IP_VS_CONN_F_BYPASS 0x0004 /* cache bypass */
>+#endif
>+
>+#define XT_IPVS_IPVS 0x01 /* this is implied by all other options */
>+#define XT_IPVS_PROTO 0x02
>+#define XT_IPVS_VADDR 0x04
>+#define XT_IPVS_VPORT 0x08
>+#define XT_IPVS_DIR 0x10
>+#define XT_IPVS_METHOD 0x20
>+#define XT_IPVS_MASK (0x40 - 1)
>+#define XT_IPVS_ONCE_MASK (XT_IPVS_MASK & ~XT_IPVS_IPVS)
>+
>+struct xt_ipvs {
>+ union nf_inet_addr vaddr, vmask;
>+ __be16 vport;
>+ u_int16_t l4proto;
>+ u_int16_t fwd_method;
>+
>+ u_int8_t invert;
>+ u_int8_t bitmask;
>+};
As per the "Writing Netfilter modules" e-book, __u16/__u8 is required.
>+config NETFILTER_XT_MATCH_IPVS
>+ tristate '"ipvs" match support'
>+ depends on NETFILTER_ADVANCED
>+ help
>+ This option allows you to match against ipvs properties of a packet.
>+
>+ To compile it as a module, choos M here. If unsure, say N.
>+
IMHO the "to compile it as a module, choos[e] M" is a relict from
old times and should just be dropped. These days, I stupilate,
everyone knows that M makes modules. And if it doesnot, then
it's not allowed by Kconfig :>
>+MODULE_AUTHOR("Hannes Eder <heder@google.com>");
>+MODULE_DESCRIPTION("Xtables: match ipvs");
"Match IPVS connection properties" is what you previously stated,
and which I think is good. Use it here, too.
>+bool ipvs_mt(const struct sk_buff *skb, const struct xt_match_param *par)
>+{
>+ const struct xt_ipvs *data = par->matchinfo;
>+ const u_int8_t family = par->family;
>+ struct ip_vs_iphdr iph;
>+ struct ip_vs_protocol *pp;
>+ struct ip_vs_conn *cp;
>+ int af;
>+ bool match = true;
>+
>+ if (data->bitmask == XT_IPVS_IPVS) {
>+ match = !!(skb->ipvs_property)
>+ ^ !!(data->invert & XT_IPVS_IPVS);
>+ goto out;
>+ }
XT_IPVS_IPVS? What's that supposed to tell me...
Anyway, this can be made much shorter, given that all following
the "out" label is a return:
if (data->bitmask == XT_IPVS_IPVS)
return skb->ipvs_property ^ !!(data->invert & XT_IPVS_IPVS);
>+ /* other flags than XT_IPVS_IPVS are set */
>+ if (!skb->ipvs_property) {
>+ match = false;
>+ goto out;
>+ }
if (!skb->ipvs_property)
return false;
>+ ip_vs_fill_iphdr(af, skb_network_header(skb), &iph);
>+
>+ if (data->bitmask & XT_IPVS_PROTO)
>+ if ((iph.protocol == data->l4proto)
>+ ^ !(data->invert & XT_IPVS_PROTO)) {
>+ match = false;
>+ goto out;
>+ }
if (iph.protocol == data->l4proto ^
!(data->invert & XT_IPVS_PROTO))
return false;
>+ pp = ip_vs_proto_get(iph.protocol);
>+ if (unlikely(!pp)) {
>+ match = false;
>+ goto out;
>+ }
if (unlikely(pp == NULL))
return false;
Only after here we really need goto (to put pp).
>+ /*
>+ * Check if the packet belongs to an existing entry
>+ */
>+ /* TODO: why does it only match in inverse? */
FIXME: Figure it out :)
>+ cp = pp->conn_out_get(af, skb, pp, &iph, iph.len, 1 /* inverse */);
>+ if (unlikely(!cp)) {
>+ match = false;
>+ goto out;
>+ }
>+
>+ /*
>+ * We found a connection, i.e. ct != 0, make sure to call
>+ * __ip_vs_conn_put before returning. In our case jump to out_put_con.
>+ */
>+
>+ if (data->bitmask & XT_IPVS_VPORT)
>+ if ((cp->vport == data->vport)
>+ ^ !(data->invert & XT_IPVS_VPORT)) {
>+ match = false;
>+ goto out_put_ct;
>+ }
>+
>+ if (data->bitmask & XT_IPVS_DIR) {
>+ /* TODO: implement */
/* Yes please */
>+ }
>+
>+ if (data->bitmask & XT_IPVS_METHOD)
>+ if (((cp->flags & IP_VS_CONN_F_FWD_MASK) == data->fwd_method)
>+ ^ !(data->invert & XT_IPVS_METHOD)) {
>+ match = false;
>+ goto out_put_ct;
>+ }
>+
>+ if (data->bitmask & XT_IPVS_VADDR) {
>+ if (af != family) {
>+ match = false;
>+ goto out_put_ct;
>+ }
>+
>+ if (ipvs_mt_addrcmp(&cp->vaddr, &data->vaddr, &data->vmask, af)
>+ ^ !(data->invert & XT_IPVS_VADDR)) {
I think the operator (^ in this case) always goes onto the same line as the ')'
((a) ^\n(b), not ((a)\n ^(b)), though some legacy code seems to do it
random so-so.)
>+ return match;
>+}
>+EXPORT_SYMBOL(ipvs_mt);
What will be using ipvs_mt?
>+static struct xt_match xt_ipvs_mt_reg[] __read_mostly = {
>+ {
>+ .name = "ipvs",
>+ .revision = 0,
>+ .family = NFPROTO_UNSPEC,
>+ .match = ipvs_mt,
>+ .matchsize = sizeof(struct xt_ipvs),
>+ .me = THIS_MODULE,
>+ },
>+};
There is just one element, so no strict need for the [] array.
^ permalink raw reply
* Re: [PULL REQ][PATCH 0/3] ieee 802.15.4 documentation updates
From: David Miller @ 2009-07-27 18:30 UTC (permalink / raw)
To: dbaryshkov; +Cc: netdev, linux-zigbee-devel, slapin
In-Reply-To: <1248703695-7043-1-git-send-email-dbaryshkov@gmail.com>
From: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Date: Mon, 27 Jul 2009 18:08:12 +0400
>
> Please pull 3 small updates for IEEE 802.15.4 code (documentation and headers move).
>
> The following changes since commit 72dccb01e8632aa5ffe58070003d0fa19d007116:
> Eric Dumazet (1):
> bnx2: Update vlan_features
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/lowpan/lowpan.git for-linus
Pulled into net-next-2.6, thanks.
^ permalink raw reply
* Re: [PATCH] cnic: Fix ISCSI_KEVENT_IF_DOWN message handling.
From: David Miller @ 2009-07-27 18:26 UTC (permalink / raw)
To: mchan; +Cc: James.Bottomley, michaelc, netdev, linux-scsi, open-iscsi
In-Reply-To: <1248318115-14214-1-git-send-email-mchan@broadcom.com>
From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 22 Jul 2009 20:01:55 -0700
> When a net device goes down or when the bnx2i driver is unloaded,
> the code was not generating the ISCSI_KEVENT_IF_DOWN message
> properly and this could cause the userspace driver to crash.
>
> This is fixed by sending the message properly in the shutdown path.
> cnic_uio_stop() is also added to send the message when bnx2i is
> unregistering.
>
> Signed-off-by: Michael Chan <mchan@broadcom.com>
Applied.
^ permalink raw reply
* Re: [PATCH] Add mac driver for w90p910
From: David Miller @ 2009-07-27 18:24 UTC (permalink / raw)
To: mcuos.com; +Cc: linux, soni.trilok, netdev, linux-arm-kernel, eric.y.miao
In-Reply-To: <4A67EC88.8010001@gmail.com>
From: Wan ZongShun <mcuos.com@gmail.com>
Date: Thu, 23 Jul 2009 12:52:24 +0800
> I fixed up previous mac driver and make a patch relative to it.
This patch does not apply to the code actually in the tree,
for example:
> ether->tdesc = (struct tran_pdesc *)
> - dma_alloc_coherent(NULL, sizeof(struct tran_pdesc),
> - (dma_addr_t *)ðer->tdesc_phys, GFP_KERNEL);
> + dma_alloc_coherent(&pdev->dev, sizeof(struct tran_pdesc),
> + ðer->tdesc_phys, GFP_KERNEL);
> +
in the tree there is a space between '(dma_addr_t *)' and
'ðer->tdesc_phys', but the code you are patching against
does not have that space.
Please do two things:
1) Generate your patch properly against the code actually in
the tree.
2) Create a coherent, verbose, commit message and proper subject
line which describes one-by-one the bug fixes you are making
to the driver.
Thanks.
^ permalink raw reply
* [PATCH] xfrm: export xfrm garbage collector thresholds via sysctl
From: Neil Horman @ 2009-07-27 18:22 UTC (permalink / raw)
To: netdev
Cc: joe, nhorman, davem, herbert, kuznet, pekkas, jmorris, yoshfuji,
kaber
Export garbage collector thresholds for xfrm[4|6]_dst_ops
Had a problem reported to me recently in which a high volume of ipsec
connections on a system began reporting ENOBUFS for new connections eventually.
It seemed that after about 2000 connections we started being unable to create
more. A quick look revealed that the xfrm code used a dst_ops structure that
limited the gc_thresh value to 1024, and alaways dropped route cache entries
after 2x the gc_thresh. It seems the most direct solution is to export the
gc_thresh values in the xfrm[4|6] dst_ops as sysctls, like the main routing
table does, so that higher volumes of connections can be supported. This patch
has been tested and allows the reporter to increase their ipsec connection
volume successfully.
Reported-by: Joe Nall <joe@nall.com>
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
ipv4/xfrm4_policy.c | 18 ++++++++++++++++++
ipv6/xfrm6_policy.c | 18 ++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 0071ee6..018ac8b 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -264,6 +264,20 @@ static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
.fill_dst = xfrm4_fill_dst,
};
+static struct ctl_table xfrm4_policy_table[] = {
+ {
+ .ctl_name = CTL_UNNUMBERED,
+ .procname = "xfrm4_gc_thresh",
+ .data = &xfrm4_dst_ops.gc_thresh,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ { }
+};
+
+static struct ctl_table_header *sysctl_hdr;
+
static void __init xfrm4_policy_init(void)
{
xfrm_policy_register_afinfo(&xfrm4_policy_afinfo);
@@ -271,6 +285,8 @@ static void __init xfrm4_policy_init(void)
static void __exit xfrm4_policy_fini(void)
{
+ if (sysctl_hdr)
+ unregister_net_sysctl_table(sysctl_hdr);
xfrm_policy_unregister_afinfo(&xfrm4_policy_afinfo);
}
@@ -278,5 +294,7 @@ void __init xfrm4_init(void)
{
xfrm4_state_init();
xfrm4_policy_init();
+ sysctl_hdr = register_net_sysctl_table(&init_net, net_ipv4_ctl_path,
+ xfrm4_policy_table);
}
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 3a3c677..4acc308 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -306,6 +306,20 @@ static void xfrm6_policy_fini(void)
xfrm_policy_unregister_afinfo(&xfrm6_policy_afinfo);
}
+static struct ctl_table xfrm6_policy_table[] = {
+ {
+ .ctl_name = CTL_UNNUMBERED,
+ .procname = "xfrm6_gc_thresh",
+ .data = &xfrm6_dst_ops.gc_thresh,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
+ { }
+};
+
+static struct ctl_table_header *sysctl_hdr;
+
int __init xfrm6_init(void)
{
int ret;
@@ -317,6 +331,8 @@ int __init xfrm6_init(void)
ret = xfrm6_state_init();
if (ret)
goto out_policy;
+ sysctl_hdr = register_net_sysctl_table(&init_net, net_ipv6_ctl_path,
+ xfrm6_policy_table);
out:
return ret;
out_policy:
@@ -326,6 +342,8 @@ out_policy:
void xfrm6_fini(void)
{
+ if (sysctl_hdr)
+ unregister_net_sysctl_table(sysctl_hdr);
//xfrm6_input_fini();
xfrm6_policy_fini();
xfrm6_state_fini();
^ permalink raw reply related
* Re: [RFC][PATCH 1/5] IPVS: prefix EnterFunction and LeaveFunction msg with "IPVS:"
From: Jan Engelhardt @ 2009-07-27 18:14 UTC (permalink / raw)
To: Hannes Eder; +Cc: lvs-devel, netdev, netfilter-devel, linux-kernel
In-Reply-To: <20090727134616.12897.7639.stgit@jazzy.zrh.corp.google.com>
On Monday 2009-07-27 15:46, Hannes Eder wrote:
>
>Now all printk messages from IPVS are prefixed with "IPVS:".
>
>+#define EnterFunction(level) \
>+ do { \
>+ if (level <= ip_vs_get_debug_level()) \
>+ printk(KERN_DEBUG "IPVS: Enter: %s, %s line %i\n", \
>+ __func__, __FILE__, __LINE__); \
>+ } while (0)
>+#define LeaveFunction(level) \
>+ do { \
>+ if (level <= ip_vs_get_debug_level()) \
>+ printk(KERN_DEBUG "IPVS: Leave: %s, %s line %i\n", \
>+ __func__, __FILE__, __LINE__); \
>+ } while (0)
I think you should rather make use of pr_fmt:
<before any #includes>
#define pr_fmt(x) "IPVS: " x
And then use pr_<level>("Elvis has left the building") in code. This
will add IPVS: automatically to all pr_* calls, alleviating the need
to manually type it into all printks.
Of course, if you only want it for the two defines here, scrap
my idea :)
^ permalink raw reply
* Re: [PATCH] USB host CDC Phonet network interface driver
From: Marcel Holtmann @ 2009-07-27 17:59 UTC (permalink / raw)
To: Dan Williams
Cc: Oliver Neukum, Alan Cox, Rémi Denis-Courmont,
netdev@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <1248716166.17189.25.camel@localhost.localdomain>
Hi Dan,
> > > > > No, I was thinking of having two full devices, a data channel and a
> > > > > control channel for devices that really talk AT commands natively.
> > > >
> > > > If the hardware does it great, however for things like a 3G modem you
> > > > have the problem that the PPP is over the AT command channel which may
> > > > itself be multiplexed. And the muxing in question is *ugly* - sort of
> > > > HDLC and LAP-B done wrong.
> > >
> > > Well, yes, but we would really like a separate control channel, so we
> > > can query parameters like signal strength, while we do PPP over the data
> > > channel.
> >
> > we don't want PPP at all. It is just plain stupid and a total braindead
> > idea. Non of the GSM/UMTS networks talk PPP over the air interface or
> > actually anywhere in their stack. The PPP is just between the host OS
> > and the card. It is a pointless encapsulation of IP packets that comes
> > from the POTS stuff where PPP over a telephone line made sense.
>
> Feel free to convince all the hardware vendors to move over to that
> model. Many of them are, but there are still *boatloads* of devices
> that do PPP. It'll be at least 3 or 4 years before you can even think
> about ignoring PPP entirely.
>
> This is totally a firmware thing and not something under our control at
> all. Either the vendor implements a non-PPP data channel, or they
> don't. We have to live with that, or ignore devices that use PPP. Most
> of the devices out there use PPP. Maybe 80 or 90%.
I know that and it is sad. It is also sad that a firmware upgrade could
bring most cards away from PPP, but besides Option I haven't seen it
from anybody.
> The only reason vendors ditched PPP was that it was too much overhead to
> achieve full speed on HSDPA 7.2 networks. Guess how many operators have
> actually deployed HSDPA 7.2? Count them on your hands. Yes, over the
> next year or two we'll see a lot more HSDPA 7.2-capable networks, and
> that means more devices will show up that ditch PPP. But at the moment,
> PPP can't be ignored.
Actually HSDPA 7.2 is deployed more than you think. At least in Europe
the base stations in the big cities are 7.2 capable. Some of them even
with proper HSUPA support. It is impressive what you can get through
these networks.
> The next problem is that not all vendors implement the non-PPP data
> channel using cdc-ether, or provide specs/drivers for that channel. So
> just because a vendor ditches PPP doesn't automatically mean it's better
> for Linux and a driver is available.
The CDC Ethernet support of the Ericsson MBM cards is kinda a nice idea.
You get proper IFF_LOWER_UP and can just run DHCP on it. I am not such a
big fan of the DHCP part, but it is the same for WiMAX, so I assume that
is what they will be settling for. Nice would to have just plain and
simple IPv6 on UMTS networks.
Regards
Marcel
^ permalink raw reply
* Re: netfilter -stable 08/08: nf_conntrack: nf_conntrack_alloc() fixes
From: Paul E. McKenney @ 2009-07-27 17:55 UTC (permalink / raw)
To: Patrick McHardy; +Cc: stable, netdev, netfilter-devel
In-Reply-To: <20090723141533.19029.28754.sendpatchset@x2.localnet>
On Thu, Jul 23, 2009 at 04:15:34PM +0200, Patrick McHardy wrote:
> commit 2b12e9634d9676c55f576195a7f950956179d881
> Author: Patrick McHardy <kaber@trash.net>
> Date: Thu Jul 23 16:02:18 2009 +0200
>
> netfilter: nf_conntrack: nf_conntrack_alloc() fixes
>
> Upstream commit 941297f4:
>
> When a slab cache uses SLAB_DESTROY_BY_RCU, we must be careful when allocating
> objects, since slab allocator could give a freed object still used by lockless
> readers.
>
> In particular, nf_conntrack RCU lookups rely on ct->tuplehash[xxx].hnnode.next
> being always valid (ie containing a valid 'nulls' value, or a valid pointer to next
> object in hash chain.)
>
> kmem_cache_zalloc() setups object with NULL values, but a NULL value is not valid
> for ct->tuplehash[xxx].hnnode.next.
>
> Fix is to call kmem_cache_alloc() and do the zeroing ourself.
>
> As spotted by Patrick, we also need to make sure lookup keys are committed to
> memory before setting refcount to 1, or a lockless reader could get a reference
> on the old version of the object. Its key re-check could then pass the barrier.
The atomic_inc() works, but only if ->refcnt is set to zero on the
initial allocation. That said, this approach seems safer, so:
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
> diff --git a/Documentation/RCU/rculist_nulls.txt b/Documentation/RCU/rculist_nulls.txt
> index 6389dec..d0c017e 100644
> --- a/Documentation/RCU/rculist_nulls.txt
> +++ b/Documentation/RCU/rculist_nulls.txt
> @@ -83,11 +83,12 @@ not detect it missed following items in original chain.
> obj = kmem_cache_alloc(...);
> lock_chain(); // typically a spin_lock()
> obj->key = key;
> -atomic_inc(&obj->refcnt);
> /*
> * we need to make sure obj->key is updated before obj->next
> + * or obj->refcnt
> */
> smp_wmb();
> +atomic_set(&obj->refcnt, 1);
> hlist_add_head_rcu(&obj->obj_node, list);
> unlock_chain(); // typically a spin_unlock()
>
> @@ -159,6 +160,10 @@ out:
> obj = kmem_cache_alloc(cachep);
> lock_chain(); // typically a spin_lock()
> obj->key = key;
> +/*
> + * changes to obj->key must be visible before refcnt one
> + */
> +smp_wmb();
> atomic_set(&obj->refcnt, 1);
> /*
> * insert obj in RCU way (readers might be traversing chain)
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 2d3584f..0d961ee 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -525,22 +525,37 @@ struct nf_conn *nf_conntrack_alloc(struct net *net,
> }
> }
>
> - ct = kmem_cache_zalloc(nf_conntrack_cachep, gfp);
> + /*
> + * Do not use kmem_cache_zalloc(), as this cache uses
> + * SLAB_DESTROY_BY_RCU.
> + */
> + ct = kmem_cache_alloc(nf_conntrack_cachep, gfp);
> if (ct == NULL) {
> pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n");
> atomic_dec(&net->ct.count);
> return ERR_PTR(-ENOMEM);
> }
> -
> - atomic_set(&ct->ct_general.use, 1);
> + /*
> + * Let ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.next
> + * and ct->tuplehash[IP_CT_DIR_REPLY].hnnode.next unchanged.
> + */
> + memset(&ct->tuplehash[IP_CT_DIR_MAX], 0,
> + sizeof(*ct) - offsetof(struct nf_conn, tuplehash[IP_CT_DIR_MAX]));
> ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
> + ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
> ct->tuplehash[IP_CT_DIR_REPLY].tuple = *repl;
> + ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev = NULL;
> /* Don't set timer yet: wait for confirmation */
> setup_timer(&ct->timeout, death_by_timeout, (unsigned long)ct);
> #ifdef CONFIG_NET_NS
> ct->ct_net = net;
> #endif
>
> + /*
> + * changes to lookup keys must be done before setting refcnt to 1
> + */
> + smp_wmb();
> + atomic_set(&ct->ct_general.use, 1);
> return ct;
> }
> EXPORT_SYMBOL_GPL(nf_conntrack_alloc);
> --
> To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH NEXT 12/12] netxen: update version to 4.0.41
From: David Miller @ 2009-07-27 17:54 UTC (permalink / raw)
To: dhananjay; +Cc: netdev
In-Reply-To: <20090727.103842.52201957.davem@davemloft.net>
Ok, in order to make this build I'm going to have to insert
some parts of patch #6 which I rejected because that adds
ring size defaults/max/etc. defines which get used by later
patches.
^ permalink raw reply
* Re: [PATCH] net: irda: init spinlock after memcpy
From: David Miller @ 2009-07-27 17:49 UTC (permalink / raw)
To: tglx; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.0907271407590.2936@localhost.localdomain>
From: Thomas Gleixner <tglx@linutronix.de>
Date: Mon, 27 Jul 2009 14:12:24 +0200 (CEST)
> From: Deepak Saxena <dsaxena@mvista.com>
>
> irttp_dup() copies a tsap_cb struct, but does not initialize the
> spinlock in the new structure, which confuses lockdep.
>
> Signed-off-by: Deepak Saxena <dsaxena@mvista.com>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 12/12] netxen: update version to 4.0.41
From: David Miller @ 2009-07-27 17:38 UTC (permalink / raw)
To: dhananjay; +Cc: netdev
In-Reply-To: <1248674867-18821-13-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Sun, 26 Jul 2009 23:07:47 -0700
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 11/12] netxen: support for ethtool set ringparam
From: David Miller @ 2009-07-27 17:38 UTC (permalink / raw)
To: dhananjay; +Cc: netdev, amit
In-Reply-To: <1248674867-18821-12-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Sun, 26 Jul 2009 23:07:46 -0700
> Add support for ethtool -G to tune rx and tx ring sizes
> per interface basis.
>
> This is only supported for NX3031 based cards.
>
> Signed-off-by: Amit Kumar Salecha <amit@netxen.com>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 10/12] netxen: add vlan tx acceleration support
From: David Miller @ 2009-07-27 17:38 UTC (permalink / raw)
To: dhananjay; +Cc: netdev, amit
In-Reply-To: <1248674867-18821-11-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Sun, 26 Jul 2009 23:07:45 -0700
> Enable vlan tx acceleration for NX3031 if firmware advertises
> capability.
>
> Signed-off-by: Amit Kumar Salecha <amit@netxen.com>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
* Re: [PATCH NEXT 09/12] netxen: fix skb alloc size for legacy firmware
From: David Miller @ 2009-07-27 17:38 UTC (permalink / raw)
To: dhananjay; +Cc: netdev
In-Reply-To: <1248674867-18821-10-git-send-email-dhananjay@netxen.com>
From: Dhananjay Phadke <dhananjay@netxen.com>
Date: Sun, 26 Jul 2009 23:07:44 -0700
> Request 1532 bytes skb data size for NX3031. NX2031 firmware
> needs 1760 sized buffers.
>
> Signed-off-by: Dhananjay Phadke <dhananjay@netxen.com>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox