* Re: [RFC PATCH 00/29] net: VRF support
From: Nicolas Dichtel @ 2015-02-06 15:10 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: ebiederm, Roopa Prabhu
In-Reply-To: <1423100070-31848-1-git-send-email-dsahern@gmail.com>
Le 05/02/2015 02:34, David Ahern a écrit :
[snip]
>
> Design
> ------
> Namespaces provide excellent separation of the networking stack from the
> netdevices and up. The intent of VRFs is to provide an additional,
> logical separation at the L3 layer within a namespace.
>
> +----------------------------------------------------------+
> | Namespace foo |
> | +---------------+ |
> | +------+ | L3/L4 service | |
> | | lldp | | (VRF any) | |
> | +------+ +---------------+ |
> | |
> | +-------------------------+ |
> | | VRF M | |
> | +---------------------+ +-------------------------+ | |
> | | VRF 1 (default) | | VRF N | | |
> | | +---------------+ | | +---------------+ | | |
> | | | L3/L4 service | | | | L3/L4 service | | | |
> | | | (VRF unaware) | | | | (VRF unaware) | | | |
> | | +---------------+ | | +---------------+ | | |
> | | | | | | |
> | |+-----+ +----------+ | | +-----+ +----------+ | | |
> | || FIB | | neighbor | | | | FIB | | neighbor | | | |
> | |+-----+ +----------+ | | +-----+ +----------+ | | |
> | | | | |-+ |
> | | {dev 1} {dev 2} | | {dev 3} {dev 4} {dev 5} | |
> | +---------------------+ +-------------------------+ |
> +----------------------------------------------------------+
>
Another question: how is the loopback managed?
If I understand well, there is only one loopback for all vrf.
If I add an address on this loopback, is this address visible from all VRF?
Can an app in VRF1 talks to an app in VRF2 via the loopback?
^ permalink raw reply
* [PATCH] ethtool: return non-zero from do_sset() when any of ETHTOOL_{G,S}* fails
From: Ivan Vecera @ 2015-02-06 15:24 UTC (permalink / raw)
To: netdev; +Cc: Ben Hutchings
Function do_sset returns unconditionally zero error code without regard
to return value of ETHTOOL_{G,S}* ioctls.
Cc: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
ethtool.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index bf583f3..c004d41 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2357,7 +2357,7 @@ static int do_sset(struct cmd_context *ctx)
int argc = ctx->argc;
char **argp = ctx->argp;
int i;
- int err;
+ int err, result = 0;
for (i = 0; i < ARRAY_SIZE(flags_msglvl); i++)
flag_to_cmdline_info(flags_msglvl[i].name,
@@ -2617,6 +2617,7 @@ static int do_sset(struct cmd_context *ctx)
fprintf(stderr, " not setting transceiver\n");
if (mdix_wanted != -1)
fprintf(stderr, " not setting mdix\n");
+ result = err;
}
}
@@ -2650,6 +2651,7 @@ static int do_sset(struct cmd_context *ctx)
fprintf(stderr, " not setting wol\n");
if (sopass_change)
fprintf(stderr, " not setting sopass\n");
+ result = err;
}
}
@@ -2668,9 +2670,10 @@ static int do_sset(struct cmd_context *ctx)
if (err < 0)
perror("Cannot set new msglvl");
}
+ result = err;
}
- return 0;
+ return result;
}
static int do_gregs(struct cmd_context *ctx)
--
2.0.5
^ permalink raw reply related
* [PATCH net-next] rhashtable: Fix remove logic to avoid cross references between buckets
From: Thomas Graf @ 2015-02-06 16:08 UTC (permalink / raw)
To: Ying Xue; +Cc: davem, netdev, herbert
In-Reply-To: <54D42892.3030808@windriver.com>
The remove logic properly searched the remaining chain for a matching
entry with an identical hash but it did this while searching from both
the old and new table. Instead in order to not leave stale references
behind we need to:
1. When growing and searching from the new table:
Search remaining chain for entry with same hash to avoid having
the new table directly point to a entry with a different hash.
2. When shrinking and searching from the old table:
Check if the element after the removed would create a cross
reference and avoid it if so.
These bugs were present from the beginning in nft_hash.
Also, both insert functions calculated the hash based on the mask of
the new table. This worked while growing. Wwhile shrinking, the mask
of the inew table is smaller than the mask of the old table. This lead
to a bit not being taken into account when selecting the bucket lock
and thus caused the wrong bucket to be locked eventually.
Fixes: 7e1e77636e36 ("lib: Resizable, Scalable, Concurrent Hash Table")
Fixes: 97defe1ecf86 ("rhashtable: Per bucket locks & deferred expansion/shrinking")
Reported-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Thomas Graf <tgraf@suug.ch>
---
lib/rhashtable.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 5919d63..e96fc00 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -552,8 +552,10 @@ static void rhashtable_wakeup_worker(struct rhashtable *ht)
static void __rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj,
struct bucket_table *tbl, u32 hash)
{
- struct rhash_head *head = rht_dereference_bucket(tbl->buckets[hash],
- tbl, hash);
+ struct rhash_head *head;
+
+ hash = rht_bucket_index(tbl, hash);
+ head = rht_dereference_bucket(tbl->buckets[hash], tbl, hash);
ASSERT_BUCKET_LOCK(ht, tbl, hash);
@@ -593,7 +595,7 @@ void rhashtable_insert(struct rhashtable *ht, struct rhash_head *obj)
tbl = rht_dereference_rcu(ht->future_tbl, ht);
old_tbl = rht_dereference_rcu(ht->tbl, ht);
- hash = head_hashfn(ht, tbl, obj);
+ hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
lock_buckets(tbl, old_tbl, hash);
__rhashtable_insert(ht, obj, tbl, hash);
@@ -627,8 +629,8 @@ bool rhashtable_remove(struct rhashtable *ht, struct rhash_head *obj)
bool ret = false;
rcu_read_lock();
- tbl = old_tbl = rht_dereference_rcu(ht->tbl, ht);
- new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
+ old_tbl = rht_dereference_rcu(ht->tbl, ht);
+ tbl = new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
new_hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
lock_buckets(new_tbl, old_tbl, new_hash);
@@ -643,15 +645,19 @@ restart:
ASSERT_BUCKET_LOCK(ht, tbl, hash);
- if (unlikely(new_tbl != tbl)) {
- rht_for_each_continue(he2, he->next, tbl, hash) {
+ if (old_tbl->size > new_tbl->size && tbl == old_tbl &&
+ !rht_is_a_nulls(obj->next) &&
+ head_hashfn(ht, tbl, obj->next) != hash) {
+ rcu_assign_pointer(*pprev, (struct rhash_head *) rht_marker(ht, hash));
+ } else if (unlikely(old_tbl->size < new_tbl->size && tbl == new_tbl)) {
+ rht_for_each_continue(he2, obj->next, tbl, hash) {
if (head_hashfn(ht, tbl, he2) == hash) {
rcu_assign_pointer(*pprev, he2);
goto found;
}
}
- INIT_RHT_NULLS_HEAD(*pprev, ht, hash);
+ rcu_assign_pointer(*pprev, (struct rhash_head *) rht_marker(ht, hash));
} else {
rcu_assign_pointer(*pprev, obj->next);
}
@@ -666,8 +672,8 @@ found:
* resizing. Thus traversing both is fine and the added cost is
* very rare.
*/
- if (tbl != new_tbl) {
- tbl = new_tbl;
+ if (tbl != old_tbl) {
+ tbl = old_tbl;
goto restart;
}
@@ -835,7 +841,7 @@ bool rhashtable_lookup_compare_insert(struct rhashtable *ht,
rcu_read_lock();
old_tbl = rht_dereference_rcu(ht->tbl, ht);
new_tbl = rht_dereference_rcu(ht->future_tbl, ht);
- new_hash = head_hashfn(ht, new_tbl, obj);
+ new_hash = obj_raw_hashfn(ht, rht_obj(ht, obj));
lock_buckets(new_tbl, old_tbl, new_hash);
--
1.9.3
^ permalink raw reply related
* [PATCH net-next] gre/ipip: use be16 variants of netlink functions
From: Sabrina Dubroca @ 2015-02-06 16:22 UTC (permalink / raw)
To: davem; +Cc: netdev, Sabrina Dubroca
encap.sport and encap.dport are __be16, use nla_{get,put}_be16 instead
of nla_{get,put}_u16.
Fixes the sparse warnings:
warning: incorrect type in assignment (different base types)
expected restricted __be32 [addressable] [usertype] o_key
got restricted __be16 [addressable] [usertype] i_flags
warning: incorrect type in assignment (different base types)
expected restricted __be16 [usertype] sport
got unsigned short
warning: incorrect type in assignment (different base types)
expected restricted __be16 [usertype] dport
got unsigned short
warning: incorrect type in argument 3 (different base types)
expected unsigned short [unsigned] [usertype] value
got restricted __be16 [usertype] sport
warning: incorrect type in argument 3 (different base types)
expected unsigned short [unsigned] [usertype] value
got restricted __be16 [usertype] dport
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
net/ipv4/ip_gre.c | 12 ++++++------
net/ipv4/ipip.c | 12 ++++++------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 6e7727f27393..6207275fc749 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -659,12 +659,12 @@ static bool ipgre_netlink_encap_parms(struct nlattr *data[],
if (data[IFLA_GRE_ENCAP_SPORT]) {
ret = true;
- ipencap->sport = nla_get_u16(data[IFLA_GRE_ENCAP_SPORT]);
+ ipencap->sport = nla_get_be16(data[IFLA_GRE_ENCAP_SPORT]);
}
if (data[IFLA_GRE_ENCAP_DPORT]) {
ret = true;
- ipencap->dport = nla_get_u16(data[IFLA_GRE_ENCAP_DPORT]);
+ ipencap->dport = nla_get_be16(data[IFLA_GRE_ENCAP_DPORT]);
}
return ret;
@@ -786,10 +786,10 @@ static int ipgre_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_u16(skb, IFLA_GRE_ENCAP_TYPE,
t->encap.type) ||
- nla_put_u16(skb, IFLA_GRE_ENCAP_SPORT,
- t->encap.sport) ||
- nla_put_u16(skb, IFLA_GRE_ENCAP_DPORT,
- t->encap.dport) ||
+ nla_put_be16(skb, IFLA_GRE_ENCAP_SPORT,
+ t->encap.sport) ||
+ nla_put_be16(skb, IFLA_GRE_ENCAP_DPORT,
+ t->encap.dport) ||
nla_put_u16(skb, IFLA_GRE_ENCAP_FLAGS,
t->encap.flags))
goto nla_put_failure;
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index b58d6689874c..915d215a7d14 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -366,12 +366,12 @@ static bool ipip_netlink_encap_parms(struct nlattr *data[],
if (data[IFLA_IPTUN_ENCAP_SPORT]) {
ret = true;
- ipencap->sport = nla_get_u16(data[IFLA_IPTUN_ENCAP_SPORT]);
+ ipencap->sport = nla_get_be16(data[IFLA_IPTUN_ENCAP_SPORT]);
}
if (data[IFLA_IPTUN_ENCAP_DPORT]) {
ret = true;
- ipencap->dport = nla_get_u16(data[IFLA_IPTUN_ENCAP_DPORT]);
+ ipencap->dport = nla_get_be16(data[IFLA_IPTUN_ENCAP_DPORT]);
}
return ret;
@@ -460,10 +460,10 @@ static int ipip_fill_info(struct sk_buff *skb, const struct net_device *dev)
if (nla_put_u16(skb, IFLA_IPTUN_ENCAP_TYPE,
tunnel->encap.type) ||
- nla_put_u16(skb, IFLA_IPTUN_ENCAP_SPORT,
- tunnel->encap.sport) ||
- nla_put_u16(skb, IFLA_IPTUN_ENCAP_DPORT,
- tunnel->encap.dport) ||
+ nla_put_be16(skb, IFLA_IPTUN_ENCAP_SPORT,
+ tunnel->encap.sport) ||
+ nla_put_be16(skb, IFLA_IPTUN_ENCAP_DPORT,
+ tunnel->encap.dport) ||
nla_put_u16(skb, IFLA_IPTUN_ENCAP_FLAGS,
tunnel->encap.flags))
goto nla_put_failure;
--
2.3.0
^ permalink raw reply related
* Re: Throughput regression with `tcp: refine TSO autosizing`
From: Rick Jones @ 2015-02-06 17:48 UTC (permalink / raw)
To: Eric Dumazet, Michal Kazior
Cc: Neal Cardwell, linux-wireless, Network Development, Eyal Perry
In-Reply-To: <1423233346.31870.136.camel@edumazet-glaptop2.roam.corp.google.com>
> If you increase ability to flood on one flow, then you need to make sure
> receiver has big rcvbuf as well.
>
> echo 2000000 >/proc/sys/net/core/rmem_default
>
> Otherwise it might drop bursts.
>
> This is the kind of things that TCP does automatically, not UDP.
An alternative, if the application involved can make explicit
setsockopt() calls to set SO_SNDBUF and/or SO_RCVBUF, is to tweak
rmem_max and wmem_max and then let the application make the setsockopt()
calls.
Which path one would take would depend on circumstances I suspect.
rick jones
^ permalink raw reply
* [PATCH net-next] net: fix a typo in skb_checksum_validate_zero_check
From: Sabrina Dubroca @ 2015-02-06 17:54 UTC (permalink / raw)
To: davem; +Cc: netdev, Sabrina Dubroca
Remove trailing underscore.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
include/linux/skbuff.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 59ad1c95182d..970305e40558 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3072,7 +3072,7 @@ static inline __wsum null_compute_pseudo(struct sk_buff *skb, int proto)
#define skb_checksum_validate_zero_check(skb, proto, check, \
compute_pseudo) \
- __skb_checksum_validate_(skb, proto, true, true, check, compute_pseudo)
+ __skb_checksum_validate(skb, proto, true, true, check, compute_pseudo)
#define skb_checksum_simple_validate(skb) \
__skb_checksum_validate(skb, 0, true, false, 0, null_compute_pseudo)
--
2.3.0
^ permalink raw reply related
* Re: [RFC: add openvswitch actions using BPF 4/9] autoconf: support -with-llc options
From: Joe Stringer @ 2015-02-06 18:12 UTC (permalink / raw)
To: Andy Zhou; +Cc: dev@openvswitch.com, Linux Netdev List
In-Reply-To: <1423090163-19902-5-git-send-email-azhou@nicira.com>
On 4 February 2015 at 14:49, Andy Zhou <azhou@nicira.com> wrote:
> Add a configuration option to allow specifing the LLVM backend module
> for compiling BPF C source code into BPF instruction sets.
>
> See INSTALL.BPF.md for usgae information.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> Signed-off-by: Andy Zhou <azhou@nicira.com>
> ---
> INSTALL.BPF.md | 42 ++++++++++++++++++++++++++++++++++++++++++
> Makefile.am | 1 +
> acinclude.m4 | 16 ++++++++++++++++
> configure.ac | 2 ++
> 4 files changed, 61 insertions(+)
> create mode 100644 INSTALL.BPF.md
>
> diff --git a/INSTALL.BPF.md b/INSTALL.BPF.md
> new file mode 100644
> index 0000000..bc1f5ad
> --- /dev/null
> +++ b/INSTALL.BPF.md
> @@ -0,0 +1,42 @@
> +# Dependencies
> +
> +- Need Clang + llvm-dev version 3.X for any (2 <= X <= 4)
> +- http://llvm.org/apt/
> +
> +apt-get install libelf-dev clang-3.4 llvm-3.4-dev
> +
> +# Build LLVM plugin
> +
> +*[Upstream] git://git.kernel.org/pub/scm/linux/kernel/git/ast/bpf*
> +
> +git clone git://github.com/joestringer/linux
> +NN=$PWD/linux
> +
> +cd $NN/tools/bpf/llvm/bld
> +make LLVM_CONFIG=`which llvm-config-3.4`
> +
> +# Configure OVS to use BPF LLC
> +
> +OVS=/path/to/openvswitch
> +cd $OVS
> +./configure --with-llc=$NN/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
Naturally we'd need to update these instructions, this becomes much
simpler if we just require clang-3.7.
^ permalink raw reply
* Re: [PATCH net-next] openvswitch: Initialize unmasked key and uid len
From: Pravin Shelar @ 2015-02-06 18:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Or Gerlitz, Joe Stringer
In-Reply-To: <1423161437.31870.88.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Feb 5, 2015 at 10:37 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2015-02-05 at 09:56 -0800, Pravin B Shelar wrote:
>> Flow alloc needs to initialize unmasked key pointer. Otherwise
>> it can crash kernel trying to free random unmasked-key pointer.
>
>> CC: Joe Stringer <joestringer@nicira.com>
>> Reported-by: Or Gerlitz <ogerlitz@mellanox.com>
>> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
>> ---
>> net/openvswitch/flow_table.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
>> index 5e57628..d58447d 100644
>> --- a/net/openvswitch/flow_table.c
>> +++ b/net/openvswitch/flow_table.c
>> @@ -85,6 +85,8 @@ struct sw_flow *ovs_flow_alloc(void)
>>
>> flow->sf_acts = NULL;
>> flow->mask = NULL;
>> + flow->id.unmasked_key = NULL;
>> + flow->id.ufid_len = 0;
>> flow->stats_last_writer = NUMA_NO_NODE;
>
> At this stage, one has to wonder why not using kmem_cache_zalloc()
> and reduce icache pressure right ?
>
struct flow is still big struct, about 536 bytes. Most of the struct
is still not cleared. I do not want to clear it then initialize it
again. This helps in OVS flow setup performance.
> And please please add :
>
> Fixes: e64457191a259 ("openvswitch: Restructure datapath.c and flow.c")
>
> or whatever commit was the bug origin.
>
ok, I will send updated patch.
> Thanks
>
>
^ permalink raw reply
* Re: [RFC 0/3] Slab allocator array operations
From: Christoph Lameter @ 2015-02-06 18:39 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Andrew Morton, linux-kernel, linux-mm, penberg,
netdev@vger.kernel.org, Joonsoo Kim
In-Reply-To: <20150204001922.5650ca4b@redhat.com>
On Wed, 4 Feb 2015, Jesper Dangaard Brouer wrote:
> I promised Christoph that I will performance benchmark this. I'll start
> by writing/performing some micro benchmarks, but it first starts to get
> really interesting once we plug it into e.g. the networking stack, as
> effects as instruction-cache misses due to code size starts to play a
> role.
Ok I got a patchset here with the options removed. Just the basic ops.
Should I repost that?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC: add openvswitch actions using BPF 7/9] ofproto-dpif: Add eBPF program loader and runtime infrasturcure.
From: Joe Stringer @ 2015-02-06 18:49 UTC (permalink / raw)
To: Andy Zhou; +Cc: dev@openvswitch.com, Linux Netdev List
In-Reply-To: <1423090163-19902-8-git-send-email-azhou@nicira.com>
On 4 February 2015 at 14:49, Andy Zhou <azhou@nicira.com> wrote:
> Added a eBPF program loader in ofproto-dpif layer. It only loades
> the OVS programs.
>
> Signed-off-by: Andy Zhou <azhou@nicira.com>
> ---
A little more precise: It only loads BPF programs with sections that
begin with the 'ovs' prefix.
There's a few scattered OVS style things like using ovs_scan(),
removing commented lines.
> +#define BPF_UNIXCTL_CHECK(conn) \
> + if (bpf_unixctl_check_enable(conn) == -1) return
It seems unusual for OVS code to skimp on some "if(foo()) return"
logic by using a macro.
> +static int
> +bpf_unixctl_check_enable(struct unixctl_conn *conn)
Another minor style thing here, I think "check_enable" is ambiguous.
It's actually checking for datapath support for the feature. (I
realise there are other places in OVS that we do this, and I've been
meaning to clean them up).
> diff --git a/vswitchd/automake.mk b/vswitchd/automake.mk
> index 80affe9..a94720b 100644
> --- a/vswitchd/automake.mk
> +++ b/vswitchd/automake.mk
> @@ -19,6 +19,11 @@ vswitchd_ovs_vswitchd_LDFLAGS = $(AM_LDFLAGS) $(DPDK_vswitchd_LDFLAGS)
> EXTRA_DIST += vswitchd/INTERNALS
> MAN_ROOTS += vswitchd/ovs-vswitchd.8.in
>
> +if LINUX
> +vswitchd_ovs_vswitchd_LDADD += lib/libbpf.la
> +vswitchd_ovs_vswitchd_LDADD += -lelf
> +endif
> +
Do we perform a check for the libelf dev headers and library anywhere
in the build process?
^ permalink raw reply
* Re: [PATCH v3] net: openvswitch: Support masked set actions.
From: Pravin Shelar @ 2015-02-06 19:10 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: netdev, dev@openvswitch.org
In-Reply-To: <1423172449-3654-1-git-send-email-jrajahalme@nicira.com>
On Thu, Feb 5, 2015 at 1:40 PM, Jarno Rajahalme <jrajahalme@nicira.com> wrote:
> OVS userspace already probes the openvswitch kernel module for
> OVS_ACTION_ATTR_SET_MASKED support. This patch adds the kernel module
> implementation of masked set actions.
>
> The existing set action sets many fields at once. When only a subset
> of the IP header fields, for example, should be modified, all the IP
> fields need to be exact matched so that the other field values can be
> copied to the set action. A masked set action allows modification of
> an arbitrary subset of the supported header bits without requiring the
> rest to be matched.
>
> Masked set action is now supported for all writeable key types, except
> for the tunnel key. The set tunnel action is an exception as any
> input tunnel info is cleared before action processing starts, so there
> is no tunnel info to mask.
>
> The kernel module converts all (non-tunnel) set actions to masked set
> actions. This makes action processing more uniform, and results in
> less branching and duplicating the action processing code. When
> returning actions to userspace, the fully masked set actions are
> converted back to normal set actions. We use a kernel internal action
> code to be able to tell the userspace provided and converted masked
> set actions apart.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
> ---
> v3: Clean up code and remove flow key transport port checks from
> validate_set(), as suggested by Jesse Gross.
>
Acked-by: Pravin B Shelar <pshelar@nicira.com>
^ permalink raw reply
* [PATCH net-next v2] openvswitch: Initialize unmasked key and uid len
From: Pravin B Shelar @ 2015-02-06 19:17 UTC (permalink / raw)
To: davem; +Cc: netdev, Pravin B Shelar, Joe Stringer
Flow alloc needs to initialize unmasked key pointer. Otherwise
it can crash kernel trying to free random unmasked-key pointer.
general protection fault: 0000 [#1] SMP
3.19.0-rc6-net-next+ #457
Hardware name: Supermicro X7DWU/X7DWU, BIOS 1.1 04/30/2008
RIP: 0010:[<ffffffff8111df0e>] [<ffffffff8111df0e>] kfree+0xac/0x196
Call Trace:
[<ffffffffa060bd87>] flow_free+0x21/0x59 [openvswitch]
[<ffffffffa060bde0>] ovs_flow_free+0x21/0x23 [openvswitch]
[<ffffffffa0605b4a>] ovs_packet_cmd_execute+0x2f3/0x35f [openvswitch]
[<ffffffffa0605995>] ? ovs_packet_cmd_execute+0x13e/0x35f [openvswitch]
[<ffffffff811fe6fb>] ? nla_parse+0x4f/0xec
[<ffffffff8139a2fc>] genl_family_rcv_msg+0x26d/0x2c9
[<ffffffff8107620f>] ? __lock_acquire+0x90e/0x9aa
[<ffffffff8139a3be>] genl_rcv_msg+0x66/0x89
[<ffffffff8139a358>] ? genl_family_rcv_msg+0x2c9/0x2c9
[<ffffffff81399591>] netlink_rcv_skb+0x3e/0x95
[<ffffffff81399898>] ? genl_rcv+0x18/0x37
[<ffffffff813998a7>] genl_rcv+0x27/0x37
[<ffffffff81399033>] netlink_unicast+0x103/0x191
[<ffffffff81399382>] netlink_sendmsg+0x2c1/0x310
[<ffffffff811007ad>] ? might_fault+0x50/0xa0
[<ffffffff8135c773>] do_sock_sendmsg+0x5f/0x7a
[<ffffffff8135c799>] sock_sendmsg+0xb/0xd
[<ffffffff8135cacf>] ___sys_sendmsg+0x1a3/0x218
[<ffffffff8113e54b>] ? get_close_on_exec+0x86/0x86
[<ffffffff8115a9d0>] ? fsnotify+0x32c/0x348
[<ffffffff8115a720>] ? fsnotify+0x7c/0x348
[<ffffffff8113e5f5>] ? __fget+0xaa/0xbf
[<ffffffff8113e54b>] ? get_close_on_exec+0x86/0x86
[<ffffffff8135cccd>] __sys_sendmsg+0x3d/0x5e
[<ffffffff8135cd02>] SyS_sendmsg+0x14/0x16
[<ffffffff81411852>] system_call_fastpath+0x12/0x17
Fixes: 74ed7ab9264("openvswitch: Add support for unique flow IDs.")
CC: Joe Stringer <joestringer@nicira.com>
Reported-by: Or Gerlitz <ogerlitz@mellanox.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/openvswitch/flow_table.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c
index 5e57628..d58447d 100644
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -85,6 +85,8 @@ struct sw_flow *ovs_flow_alloc(void)
flow->sf_acts = NULL;
flow->mask = NULL;
+ flow->id.unmasked_key = NULL;
+ flow->id.ufid_len = 0;
flow->stats_last_writer = NUMA_NO_NODE;
/* Initialize the default stat node. */
--
1.7.1
^ permalink raw reply related
* Re: [PATCH net-next v2] openvswitch: Initialize unmasked key and uid len
From: Eric Dumazet @ 2015-02-06 19:44 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: davem, netdev, Joe Stringer
In-Reply-To: <1423250233-1529-1-git-send-email-pshelar@nicira.com>
On Fri, 2015-02-06 at 11:17 -0800, Pravin B Shelar wrote:
> Flow alloc needs to initialize unmasked key pointer. Otherwise
> it can crash kernel trying to free random unmasked-key pointer.
> Fixes: 74ed7ab9264("openvswitch: Add support for unique flow IDs.")
> CC: Joe Stringer <joestringer@nicira.com>
> Reported-by: Or Gerlitz <ogerlitz@mellanox.com>
> Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
> ---
Thanks Pravin
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* [PATCH] cipso: don't use IPCB() to locate the CIPSO IP option
From: Paul Moore @ 2015-02-06 19:57 UTC (permalink / raw)
To: linux-security-module, selinux; +Cc: casey, netdev
Using the IPCB() macro to get the IPv4 options is convenient, but
unfortunately NetLabel often needs to examine the CIPSO option outside
of the scope of the IP layer in the stack. While historically IPCB()
worked above the IP layer, due to the inclusion of the inet_skb_param
struct at the head of the {tcp,udp}_skb_cb structs, recent commit
971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses")
reordered the tcp_skb_cb struct and invalidated this IPCB() trick.
This patch fixes the problem by creating a new function,
cipso_v4_optptr(), which locates the CIPSO option inside the IP header
without calling IPCB(). Unfortunately, this isn't as fast as a simple
lookup so some additional tweaks were made to limit the use of this
new function.
Cc: <stable@vger.kernel.org> # 3.18
Reported-by: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Paul Moore <pmoore@redhat.com>
---
include/net/cipso_ipv4.h | 25 +++++++++++++--------
net/ipv4/cipso_ipv4.c | 51 +++++++++++++++++++++++++-----------------
net/netlabel/netlabel_kapi.c | 15 ++++++++----
3 files changed, 56 insertions(+), 35 deletions(-)
diff --git a/include/net/cipso_ipv4.h b/include/net/cipso_ipv4.h
index a6fd939..3ebb168 100644
--- a/include/net/cipso_ipv4.h
+++ b/include/net/cipso_ipv4.h
@@ -121,13 +121,6 @@ extern int cipso_v4_rbm_strictvalid;
#endif
/*
- * Helper Functions
- */
-
-#define CIPSO_V4_OPTEXIST(x) (IPCB(x)->opt.cipso != 0)
-#define CIPSO_V4_OPTPTR(x) (skb_network_header(x) + IPCB(x)->opt.cipso)
-
-/*
* DOI List Functions
*/
@@ -190,7 +183,7 @@ static inline int cipso_v4_doi_domhsh_remove(struct cipso_v4_doi *doi_def,
#ifdef CONFIG_NETLABEL
void cipso_v4_cache_invalidate(void);
-int cipso_v4_cache_add(const struct sk_buff *skb,
+int cipso_v4_cache_add(const unsigned char *cipso_ptr,
const struct netlbl_lsm_secattr *secattr);
#else
static inline void cipso_v4_cache_invalidate(void)
@@ -198,7 +191,7 @@ static inline void cipso_v4_cache_invalidate(void)
return;
}
-static inline int cipso_v4_cache_add(const struct sk_buff *skb,
+static inline int cipso_v4_cache_add(const unsigned char *cipso_ptr,
const struct netlbl_lsm_secattr *secattr)
{
return 0;
@@ -211,6 +204,8 @@ static inline int cipso_v4_cache_add(const struct sk_buff *skb,
#ifdef CONFIG_NETLABEL
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway);
+int cipso_v4_getattr(const unsigned char *cipso,
+ struct netlbl_lsm_secattr *secattr);
int cipso_v4_sock_setattr(struct sock *sk,
const struct cipso_v4_doi *doi_def,
const struct netlbl_lsm_secattr *secattr);
@@ -226,6 +221,7 @@ int cipso_v4_skbuff_setattr(struct sk_buff *skb,
int cipso_v4_skbuff_delattr(struct sk_buff *skb);
int cipso_v4_skbuff_getattr(const struct sk_buff *skb,
struct netlbl_lsm_secattr *secattr);
+unsigned char *cipso_v4_optptr(const struct sk_buff *skb);
int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option);
#else
static inline void cipso_v4_error(struct sk_buff *skb,
@@ -235,6 +231,12 @@ static inline void cipso_v4_error(struct sk_buff *skb,
return;
}
+static inline int cipso_v4_getattr(const unsigned char *cipso,
+ struct netlbl_lsm_secattr *secattr)
+{
+ return -ENOSYS;
+}
+
static inline int cipso_v4_sock_setattr(struct sock *sk,
const struct cipso_v4_doi *doi_def,
const struct netlbl_lsm_secattr *secattr)
@@ -282,6 +284,11 @@ static inline int cipso_v4_skbuff_getattr(const struct sk_buff *skb,
return -ENOSYS;
}
+static inline unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
+{
+ return NULL;
+}
+
static inline int cipso_v4_validate(const struct sk_buff *skb,
unsigned char **option)
{
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 5160c71..e361ea6 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -378,20 +378,18 @@ static int cipso_v4_cache_check(const unsigned char *key,
* negative values on failure.
*
*/
-int cipso_v4_cache_add(const struct sk_buff *skb,
+int cipso_v4_cache_add(const unsigned char *cipso_ptr,
const struct netlbl_lsm_secattr *secattr)
{
int ret_val = -EPERM;
u32 bkt;
struct cipso_v4_map_cache_entry *entry = NULL;
struct cipso_v4_map_cache_entry *old_entry = NULL;
- unsigned char *cipso_ptr;
u32 cipso_ptr_len;
if (!cipso_v4_cache_enabled || cipso_v4_cache_bucketsize <= 0)
return 0;
- cipso_ptr = CIPSO_V4_OPTPTR(skb);
cipso_ptr_len = cipso_ptr[1];
entry = kzalloc(sizeof(*entry), GFP_ATOMIC);
@@ -1579,6 +1577,33 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
}
/**
+ * cipso_v4_optptr - Find the CIPSO option in the packet
+ * @skb: the packet
+ *
+ * Description:
+ * Parse the packet's IP header looking for a CIPSO option. Returns a pointer
+ * to the start of the CIPSO option on success, NULL if one if not found.
+ *
+ */
+unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
+{
+ const struct iphdr *iph = ip_hdr(skb);
+ unsigned char *optptr = (unsigned char *)&(ip_hdr(skb)[1]);
+ int optlen;
+ int taglen;
+
+ for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) {
+ if (optptr[0] == IPOPT_CIPSO)
+ return optptr;
+ taglen = optptr[1];
+ optlen -= taglen;
+ optptr += taglen;
+ }
+
+ return NULL;
+}
+
+/**
* cipso_v4_validate - Validate a CIPSO option
* @option: the start of the option, on error it is set to point to the error
*
@@ -2119,8 +2144,8 @@ void cipso_v4_req_delattr(struct request_sock *req)
* on success and negative values on failure.
*
*/
-static int cipso_v4_getattr(const unsigned char *cipso,
- struct netlbl_lsm_secattr *secattr)
+int cipso_v4_getattr(const unsigned char *cipso,
+ struct netlbl_lsm_secattr *secattr)
{
int ret_val = -ENOMSG;
u32 doi;
@@ -2305,22 +2330,6 @@ int cipso_v4_skbuff_delattr(struct sk_buff *skb)
return 0;
}
-/**
- * cipso_v4_skbuff_getattr - Get the security attributes from the CIPSO option
- * @skb: the packet
- * @secattr: the security attributes
- *
- * Description:
- * Parse the given packet's CIPSO option and return the security attributes.
- * Returns zero on success and negative values on failure.
- *
- */
-int cipso_v4_skbuff_getattr(const struct sk_buff *skb,
- struct netlbl_lsm_secattr *secattr)
-{
- return cipso_v4_getattr(CIPSO_V4_OPTPTR(skb), secattr);
-}
-
/*
* Setup Functions
*/
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index a845cd4..28cddc8 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -1065,10 +1065,12 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb,
u16 family,
struct netlbl_lsm_secattr *secattr)
{
+ unsigned char *ptr;
+
switch (family) {
case AF_INET:
- if (CIPSO_V4_OPTEXIST(skb) &&
- cipso_v4_skbuff_getattr(skb, secattr) == 0)
+ ptr = cipso_v4_optptr(skb);
+ if (ptr && cipso_v4_getattr(ptr, secattr) == 0)
return 0;
break;
#if IS_ENABLED(CONFIG_IPV6)
@@ -1094,7 +1096,7 @@ int netlbl_skbuff_getattr(const struct sk_buff *skb,
*/
void netlbl_skbuff_err(struct sk_buff *skb, int error, int gateway)
{
- if (CIPSO_V4_OPTEXIST(skb))
+ if (cipso_v4_optptr(skb))
cipso_v4_error(skb, error, gateway);
}
@@ -1126,11 +1128,14 @@ void netlbl_cache_invalidate(void)
int netlbl_cache_add(const struct sk_buff *skb,
const struct netlbl_lsm_secattr *secattr)
{
+ unsigned char *ptr;
+
if ((secattr->flags & NETLBL_SECATTR_CACHE) == 0)
return -ENOMSG;
- if (CIPSO_V4_OPTEXIST(skb))
- return cipso_v4_cache_add(skb, secattr);
+ ptr = cipso_v4_optptr(skb);
+ if (ptr)
+ return cipso_v4_cache_add(ptr, secattr);
return -ENOMSG;
}
^ permalink raw reply related
* Re: [PATCH] cipso: don't use IPCB() to locate the CIPSO IP option
From: Paul Moore @ 2015-02-06 20:03 UTC (permalink / raw)
To: davem; +Cc: linux-security-module, selinux, casey, netdev
In-Reply-To: <20150206195728.28733.12118.stgit@localhost>
On Friday, February 06, 2015 02:57:28 PM Paul Moore wrote:
> Using the IPCB() macro to get the IPv4 options is convenient, but
> unfortunately NetLabel often needs to examine the CIPSO option outside
> of the scope of the IP layer in the stack. While historically IPCB()
> worked above the IP layer, due to the inclusion of the inet_skb_param
> struct at the head of the {tcp,udp}_skb_cb structs, recent commit
> 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses")
> reordered the tcp_skb_cb struct and invalidated this IPCB() trick.
>
> This patch fixes the problem by creating a new function,
> cipso_v4_optptr(), which locates the CIPSO option inside the IP header
> without calling IPCB(). Unfortunately, this isn't as fast as a simple
> lookup so some additional tweaks were made to limit the use of this
> new function.
>
> Cc: <stable@vger.kernel.org> # 3.18
> Reported-by: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Paul Moore <pmoore@redhat.com>
DaveM, I'd prefer this go upstream via the SELinux/security tree so we don't
have to worry about syncing up with the netdev tree to get this fix. Any
objections on your part (this patch only touches NetLabel/CIPSO)?
--
paul moore
security @ redhat
^ permalink raw reply
* Re: [patch] net: sxgbe: fix error handling in init_rx_ring()
From: David Miller @ 2015-02-06 20:50 UTC (permalink / raw)
To: dan.carpenter
Cc: bh74.an, kgene, ks.giri, vipul.pandya, netdev, kernel-janitors
In-Reply-To: <20150205080042.GC27855@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Thu, 5 Feb 2015 11:00:42 +0300
> There are a couple bugs with the error handling in this function.
>
> 1) If we can't allocate "rx_ring->rx_skbuff" then we should call
> dma_free_coherent() but we don't.
> 2) free_rx_ring() frees "rx_ring->rx_skbuff_dma" and "rx_ring->rx_skbuff"
> so calling it in a loop causes a double free.
>
> Also it was a bit confusing how we sometimes freed things before doing
> the goto. I've cleaned it up so it does error handling in normal kernel
> style.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Looks good, applied, thanks Dan.
^ permalink raw reply
* Re: [PATCH net] ipv6: addrconf: add missing validate_link_af handler
From: David Miller @ 2015-02-06 20:52 UTC (permalink / raw)
To: daniel; +Cc: netdev, jiri
In-Reply-To: <0f0921b794041725c882546a614defde18fba7ea.1423143235.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Thu, 5 Feb 2015 14:39:11 +0100
> We still need a validate_link_af() handler with an appropriate nla policy,
> similarly as we have in IPv4 case, otherwise size validations are not being
> done properly in that case.
>
> Fixes: f53adae4eae5 ("net: ipv6: add tokenized interface identifier support")
> Fixes: bc91b0f07ada ("ipv6: addrconf: implement address generation modes")
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [RFC PATCH 00/29] net: VRF support
From: Eric W. Biederman @ 2015-02-06 20:50 UTC (permalink / raw)
To: David Ahern
Cc: Stephen Hemminger, netdev, Nicolas Dichtel, roopa, hannes,
Dinesh Dutt, Vipin Kumar, Nicolas Dichtel, Shmulik Ladkani
In-Reply-To: <1423100070-31848-1-git-send-email-dsahern@gmail.com>
David looking at your patches and reading through your code I think I
understand what you are proposing, let me see if I can sum it up.
Semantics:
- The same as a network namespace.
- Addition of a VRF-ANY context
- No per VRF settings
- No creation or removal operation.
Implementation:
- Add a VRF-id to every affected data structure.
This implies that you see the current implementation of network
namespaces to be space inefficient, and that you think you can remove
this inefficiency by simply removing the settings and the associated
proc files.
Given that you have chosen to keep the same semantics as network
namespaces for everything except for settings this raises the questions:
- Are the settings and their associated proc files what actually cause
the size cost you see in network namespaces?
- Can we instead of reimplementing network namespaces instead optimize
the current implementation?
We need measurements to answer either of those questions and I think
before proceeding we need to answer those questions.
Beyond that I want to point out that in general a data structure that
has a tag on every member is going to have a larger memory foot print
per entry, contain more entries, and by virtue of both of those be less
memory efficient and less time efficient to use. So it is not clear
that a implementation that tags everything with a vrf-id will actually
be lighter weight.
Also there is a concern that placing tags in every data structure may be
significantly more error prone to implement (as it is more more thing to
keep trace of), and that can impact the maintainability and the
correctness of the code for everyone.
The standard that was applied to the network namespace was that
it did not have any measurable performance impact when enabled. The
measurments taken at the time did not show a slow down when a 1Gig
interface was place in a network namespace. Compared to running an
unpatched kernel.
I suspect your extra layer of indirection to get to struct net in
addition to touching struct skb will give you a noticable performance
impact.
I have another concern. I don't think it is wise to have a data
structure modified two different ways to deal with network namespaces
and vrfs. For maintainability and our own sanity we should pick which
version that we judge to be the most efficient implementation and go
with it.
The architecture I imagine for using network namespaces as vrfs for
devices that perform layer 2 bridging and layer 3 routing.
port1 port2 port3 port4 port5 port6 port7 port8 port9 port10
| | | | | | | | | |
+-----+-----+-----+-----+-----+-----+-----+-----+-----+
/ Link Aggregation \
+ +
| Bridging |
+----------------------------+----------------------------+
|
cpu port
|
+---------------------+---------------------+
/ +---------------/ \---------------+ \
/ / +---------/ \---------+ \ \
/ / / +---/ \---+ \ \ \
/ / / / | | \ \ \ \
| | | | | | | | | |
vlan1 vlan2 vlan3 vlan4 vlan5 vlan6 vlan7 vlan8 vlan9 vlan10
| | | | | | | | | |
+-+-----+-----+-----+-----+-+ +-+-----+-----+-----+-----+-+
| network namespace 1 | | network namespace2 |
+---------------------------+ +---------------------------+
Traffic to and from the rest of the world comes through the
external ports.
The traffic is then processed at layer two including link
aggregation, bridging and classifying which vlan the traffic
belongs in.
If the traffic needs to be routed it then comes up to the cpu port.
The cpu port looks at the tags on the traffic and places it into
the appropriate vlan device.
>From the various vlans the traffic is then routed according
to the routing table of whichever network namespace the vlan
device is in.
There are stateless offloads to this in modern hardware but this is a
reasonable model how all of this works semantically.
As such the vlan devices can be moved between network namespaces without
affecting any layer two monitoring or work that happens on the lower
level devices. The practical restriction is that L2 and L3 need to be
handled on different network devices.
This split of network devices ensures that L2 code that works today
should not need any changes or in any way be concerned about network
namespaces or that the parent devices are in.
Eric
^ permalink raw reply
* [PATCH net-next] net: rfs: add hash collision detection
From: Eric Dumazet @ 2015-02-06 20:59 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Tom Herbert, Ying Cai, Willem de Bruijn
From: Eric Dumazet <edumazet@google.com>
Receive Flow Steering is a nice solution but suffers from
hash collisions when a mix of connected and unconnected traffic
is received on the host, when flow hash table is populated.
Also, clearing flow in inet_release() makes RFS not very good
for short lived flows, as many packets can follow close().
(FIN , ACK packets, ...)
This patch extends the information stored into global hash table
to not only include cpu number, but upper part of the hash value.
I use a 32bit value, and dynamically split it in two parts.
For host with less than 64 possible cpus, this gives 6 bits for the
cpu number, and 26 (32-6) bits for the upper part of the hash.
Since hash bucket selection use low order bits of the hash, we have
a full hash match, if /proc/sys/net/core/rps_sock_flow_entries is big
enough.
If the hash found in flow table does not match, we fallback to RPS (if
it is enabled for the rxqueue).
This means that a packet for an non connected flow can avoid the
IPI through a unrelated/victim CPU.
This also means we no longer have to clear the table at socket
close time, and this helps short lived flows performance.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/tun.c | 5 ---
include/linux/netdevice.h | 34 ++++++++++++------------
include/net/sock.h | 24 -----------------
net/core/dev.c | 48 +++++++++++++++++++----------------
net/core/sysctl_net_core.c | 2 -
net/ipv4/af_inet.c | 2 -
6 files changed, 47 insertions(+), 68 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ad7d3d5f3ee5..857dca47bf80 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -256,7 +256,6 @@ static void tun_flow_delete(struct tun_struct *tun, struct tun_flow_entry *e)
{
tun_debug(KERN_INFO, tun, "delete flow: hash %u index %u\n",
e->rxhash, e->queue_index);
- sock_rps_reset_flow_hash(e->rps_rxhash);
hlist_del_rcu(&e->hash_link);
kfree_rcu(e, rcu);
--tun->flow_count;
@@ -373,10 +372,8 @@ unlock:
*/
static inline void tun_flow_save_rps_rxhash(struct tun_flow_entry *e, u32 hash)
{
- if (unlikely(e->rps_rxhash != hash)) {
- sock_rps_reset_flow_hash(e->rps_rxhash);
+ if (unlikely(e->rps_rxhash != hash))
e->rps_rxhash = hash;
- }
}
/* We try to identify a flow through its rxhash first. The reason that
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ce784d5018e0..ab3b7cef4638 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -644,39 +644,39 @@ struct rps_dev_flow_table {
/*
* The rps_sock_flow_table contains mappings of flows to the last CPU
* on which they were processed by the application (set in recvmsg).
+ * Each entry is a 32bit value. Upper part is the high order bits
+ * of flow hash, lower part is cpu number.
+ * rps_cpu_mask is used to partition the space, depending on number of
+ * possible cpus : rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1
+ * For example, if 64 cpus are possible, rps_cpu_mask = 0x3f,
+ * meaning we use 32-6=26 bits for the hash.
*/
struct rps_sock_flow_table {
- unsigned int mask;
- u16 ents[0];
+ u32 mask;
+ u32 ents[0];
};
-#define RPS_SOCK_FLOW_TABLE_SIZE(_num) (sizeof(struct rps_sock_flow_table) + \
- ((_num) * sizeof(u16)))
+#define RPS_SOCK_FLOW_TABLE_SIZE(_num) (offsetof(struct rps_sock_flow_table, ents[_num]))
#define RPS_NO_CPU 0xffff
+extern u32 rps_cpu_mask;
+extern struct rps_sock_flow_table __rcu *rps_sock_flow_table;
+
static inline void rps_record_sock_flow(struct rps_sock_flow_table *table,
u32 hash)
{
if (table && hash) {
- unsigned int cpu, index = hash & table->mask;
+ unsigned int index = hash & table->mask;
+ u32 val = hash & ~rps_cpu_mask;
/* We only give a hint, preemption can change cpu under us */
- cpu = raw_smp_processor_id();
+ val |= raw_smp_processor_id();
- if (table->ents[index] != cpu)
- table->ents[index] = cpu;
+ if (table->ents[index] != val)
+ table->ents[index] = val;
}
}
-static inline void rps_reset_sock_flow(struct rps_sock_flow_table *table,
- u32 hash)
-{
- if (table && hash)
- table->ents[hash & table->mask] = RPS_NO_CPU;
-}
-
-extern struct rps_sock_flow_table __rcu *rps_sock_flow_table;
-
#ifdef CONFIG_RFS_ACCEL
bool rps_may_expire_flow(struct net_device *dev, u16 rxq_index, u32 flow_id,
u16 filter_id);
diff --git a/include/net/sock.h b/include/net/sock.h
index d28b8fededd6..e13824570b0f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -857,18 +857,6 @@ static inline void sock_rps_record_flow_hash(__u32 hash)
#endif
}
-static inline void sock_rps_reset_flow_hash(__u32 hash)
-{
-#ifdef CONFIG_RPS
- struct rps_sock_flow_table *sock_flow_table;
-
- rcu_read_lock();
- sock_flow_table = rcu_dereference(rps_sock_flow_table);
- rps_reset_sock_flow(sock_flow_table, hash);
- rcu_read_unlock();
-#endif
-}
-
static inline void sock_rps_record_flow(const struct sock *sk)
{
#ifdef CONFIG_RPS
@@ -876,28 +864,18 @@ static inline void sock_rps_record_flow(const struct sock *sk)
#endif
}
-static inline void sock_rps_reset_flow(const struct sock *sk)
-{
-#ifdef CONFIG_RPS
- sock_rps_reset_flow_hash(sk->sk_rxhash);
-#endif
-}
-
static inline void sock_rps_save_rxhash(struct sock *sk,
const struct sk_buff *skb)
{
#ifdef CONFIG_RPS
- if (unlikely(sk->sk_rxhash != skb->hash)) {
- sock_rps_reset_flow(sk);
+ if (unlikely(sk->sk_rxhash != skb->hash))
sk->sk_rxhash = skb->hash;
- }
#endif
}
static inline void sock_rps_reset_rxhash(struct sock *sk)
{
#ifdef CONFIG_RPS
- sock_rps_reset_flow(sk);
sk->sk_rxhash = 0;
#endif
}
diff --git a/net/core/dev.c b/net/core/dev.c
index a3a96ffc67f4..8be38675e1a8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3030,6 +3030,8 @@ static inline void ____napi_schedule(struct softnet_data *sd,
/* One global table that all flow-based protocols share. */
struct rps_sock_flow_table __rcu *rps_sock_flow_table __read_mostly;
EXPORT_SYMBOL(rps_sock_flow_table);
+u32 rps_cpu_mask __read_mostly;
+EXPORT_SYMBOL(rps_cpu_mask);
struct static_key rps_needed __read_mostly;
@@ -3086,16 +3088,17 @@ set_rps_cpu(struct net_device *dev, struct sk_buff *skb,
static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
struct rps_dev_flow **rflowp)
{
- struct netdev_rx_queue *rxqueue;
- struct rps_map *map;
+ const struct rps_sock_flow_table *sock_flow_table;
+ struct netdev_rx_queue *rxqueue = dev->_rx;
struct rps_dev_flow_table *flow_table;
- struct rps_sock_flow_table *sock_flow_table;
+ struct rps_map *map;
int cpu = -1;
- u16 tcpu;
+ u32 tcpu;
u32 hash;
if (skb_rx_queue_recorded(skb)) {
u16 index = skb_get_rx_queue(skb);
+
if (unlikely(index >= dev->real_num_rx_queues)) {
WARN_ONCE(dev->real_num_rx_queues > 1,
"%s received packet on queue %u, but number "
@@ -3103,39 +3106,40 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
dev->name, index, dev->real_num_rx_queues);
goto done;
}
- rxqueue = dev->_rx + index;
- } else
- rxqueue = dev->_rx;
+ rxqueue += index;
+ }
+ /* Avoid computing hash if RFS/RPS is not active for this rxqueue */
+
+ flow_table = rcu_dereference(rxqueue->rps_flow_table);
map = rcu_dereference(rxqueue->rps_map);
- if (map) {
- if (map->len == 1 &&
- !rcu_access_pointer(rxqueue->rps_flow_table)) {
- tcpu = map->cpus[0];
- if (cpu_online(tcpu))
- cpu = tcpu;
- goto done;
- }
- } else if (!rcu_access_pointer(rxqueue->rps_flow_table)) {
+ if (!flow_table && !map)
goto done;
- }
skb_reset_network_header(skb);
hash = skb_get_hash(skb);
if (!hash)
goto done;
- flow_table = rcu_dereference(rxqueue->rps_flow_table);
sock_flow_table = rcu_dereference(rps_sock_flow_table);
if (flow_table && sock_flow_table) {
- u16 next_cpu;
struct rps_dev_flow *rflow;
+ u32 next_cpu;
+ u32 ident;
+
+ /* First check into global flow table if there is a match */
+ ident = sock_flow_table->ents[hash & sock_flow_table->mask];
+ if ((ident ^ hash) & ~rps_cpu_mask)
+ goto try_rps;
+ next_cpu = ident & rps_cpu_mask;
+
+ /* OK, now we know there is a match,
+ * we can look at the local (per receive queue) flow table
+ */
rflow = &flow_table->flows[hash & flow_table->mask];
tcpu = rflow->cpu;
- next_cpu = sock_flow_table->ents[hash & sock_flow_table->mask];
-
/*
* If the desired CPU (where last recvmsg was done) is
* different from current CPU (one in the rx-queue flow
@@ -3162,6 +3166,8 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
}
}
+try_rps:
+
if (map) {
tcpu = map->cpus[reciprocal_scale(hash, map->len)];
if (cpu_online(tcpu)) {
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index fde21d19e61b..7a31be5e361f 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -65,7 +65,7 @@ static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
mutex_unlock(&sock_flow_mutex);
return -ENOMEM;
}
-
+ rps_cpu_mask = roundup_pow_of_two(nr_cpu_ids) - 1;
sock_table->mask = size - 1;
} else
sock_table = orig_sock_table;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index a44773c8346c..d2e49baaff63 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -395,8 +395,6 @@ int inet_release(struct socket *sock)
if (sk) {
long timeout;
- sock_rps_reset_flow(sk);
-
/* Applications forget to leave groups before exiting */
ip_mc_drop_socket(sk);
^ permalink raw reply related
* panic on boot with latest net-next
From: Josh Hunt @ 2015-02-06 21:02 UTC (permalink / raw)
To: herbert, tgraf; +Cc: netdev
I'm hitting the following crash on boot with the latest net-next
(2ca292d968ef20cb04f31192d1f626bd8d782960):
[ 14.550948] BUG: unable to handle kernel paging request at
fffffffffffffef0
[ 14.558955] IP: [<ffffffff815657fa>] netlink_compare+0xa/0x30
[ 14.565258] PGD 191f067 PUD 1921067 PMD 0
[ 14.570118] Oops: 0000 [#1] SMP
[ 14.573997] Modules linked in: ext4 jbd2 crc16 raid10 raid456
async_raid6_recov async_pq raid6_pq async_xor xor async_memcpy async_tx
raid1 raid0 linear md_mod dm_mod ahci libahci mpt2sas scsi_transport_sas
raid_class libata
[ 14.597480] CPU: 0 PID: 1828 Comm: udevd Not tainted 3.19.0-rc7+ #7
[ 14.604226] Hardware name: CIARA TECHNOLOGIES 1X8-X6 SSD 16G
10GE/S5530WG2NR-LE-2T-AKA, BIOS 7.008 14/04/2014
[ 14.614973] task: ffff88040a63e6d0 ti: ffff88040d340000 task.ti:
ffff88040d340000
[ 14.623304] RIP: 0010:[<ffffffff815657fa>] [<ffffffff815657fa>]
netlink_compare+0xa/0x30
[ 14.632421] RSP: 0018:ffff88040d343db0 EFLAGS: 00010246
[ 14.638188] RAX: 0000000000000000 RBX: ffff88040d2b5158 RCX:
00000000c74f856c
[ 14.645783] RDX: 0000000000000724 RSI: ffff88040d343e10 RDI:
fffffffffffffc50
[ 14.653378] RBP: ffff88040d343df8 R08: 00000000e684c87d R09:
ffff88040d343e24
[ 14.660972] R10: 00007fffed830500 R11: 0000000000000202 R12:
ffffffff815657f0
[ 14.668564] R13: ffff88040d343e10 R14: ffff88040a5d9040 R15:
0000000000000000
[ 14.676161] FS: 00007fb45823a7c0(0000) GS:ffff88041fc00000(0000)
knlGS:0000000000000000
[ 14.685094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 14.691304] CR2: fffffffffffffef0 CR3: 000000040a241000 CR4:
00000000001407f0
[ 14.698898] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 14.706492] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 14.714086] Stack:
[ 14.716555] ffffffff8135a254 0000000000000009 ffff88040a5d9040
ffff88040d343e08
[ 14.725179] 0000000000000724 ffff88040d2b5158 ffffffff819c3a80
00000000ffffefff
[ 14.733804] ffff88040cc66000 ffff88040d343e58 ffffffff8156781e
ffff88040d343e18
[ 14.742428] Call Trace:
[ 14.745351] [<ffffffff8135a254>] ?
rhashtable_lookup_compare+0x74/0xc0
[ 14.752425] [<ffffffff8156781e>] netlink_autobind.isra.26+0x9e/0xd0
[ 14.759240] [<ffffffff81567f4f>] netlink_bind+0xcf/0x250
[ 14.765103] [<ffffffff815219e4>] SyS_bind+0xb4/0xd0
[ 14.770530] [<ffffffff81011170>] ? syscall_trace_enter_phase1+0xf0/0x150
[ 14.777778] [<ffffffff81616bfd>] ? int_check_syscall_exit_work+0x34/0x3d
[ 14.785026] [<ffffffff816169d2>] system_call_fastpath+0x12/0x17
[ 14.791495] Code: ff 93 88 02 00 00 f0 ff 83 d8 00 00 00 48 83 c4 08
5b 5d c3 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 31 c0 8b
56 08 <39> 97 a0 02 00 00 55 48 89 e5 74 0a 5d c3 0f 1f 84 00 00 00 00
[ 14.818000] RIP [<ffffffff815657fa>] netlink_compare+0xa/0x30
[ 14.824391] RSP <ffff88040d343db0>
[ 14.828340] CR2: fffffffffffffef0
[ 14.832115] ---[ end trace 2c31c5b4adad18c3 ]---
[ 14.979678] Kernel panic - not syncing: Fatal exception
[ 14.985392] Kernel Offset: 0x0 from 0xffffffff81000000 (relocation
range: 0xffffffff80000000-0xffffffff9fffffff)
[ 15.138889] Rebooting in 30 seconds..
(gdb) list *(netlink_compare+0xa)
0xffffffff815657fa is in netlink_compare (net/netlink/af_netlink.c:982).
977 static bool netlink_compare(void *ptr, void *arg)
978 {
979 struct netlink_compare_arg *x = arg;
980 struct sock *sk = ptr;
981
982 return nlk_sk(sk)->portid == x->portid &&
983 net_eq(sock_net(sk), x->net);
984 }
985
986 static struct sock *__netlink_lookup(struct netlink_table *table,
u32 portid,
I assumed this was rhashtable-related and so sending to Thomas and
Herbert, but if not please let me know and I get it to the right person.
I'm happy to test any patches you may have to resolve this. Also, let me
know if you need my config file to reproduce.
Thanks
Josh
^ permalink raw reply
* [PATCH net-next 0/4] tcp: mitigate TCP ACK loops due to out-of-window validation dupacks
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell
This patch series mitigates "ack loop" DoS scenarios by rate-limiting
outgoing duplicate ACKs sent in response to incoming "out of window"
segments.
Background
-----------
There are several cases in which the TCP RFCs specify that a TCP
endpoint should send a pure duplicate ACK in response to a pure
duplicate ACK that appears to be invalid due to being "out of window":
(1) RFC 793 (section 3.9, page 69) specifies that endpoints should
send a duplicate ACK in response to an ACK when the incoming
sequence number is invalid due to being outside the receive
window: "If an incoming segment is not acceptable, an
acknowledgment should be sent in reply".
(2) RFC 793 (section 3.9, page 72) says: "If the ACK acknowledges
something not yet sent (SEG.ACK > SND.NXT) then send an ACK".
(3) RFC 1323 (section 4.2.1, page 18) specifies that endpoints should
send a duplicate ACK in response to an ACK when the PAWS check for
the incoming timestamp value fails: "If .... SEG.TSval < TS.Recent
and if TS.Recent is valid ... Send an acknowledgement in reply"
The problem
------------
Normally, this is not a problem. However, a buggy middlebox or
malicious man-in-the-middle can inject a few packets into the
conversation that advance each endpoint's notion of the current window
(sequence, ACK, or timestamp), without either side noticing. In this
case, from then on each side can think the other is sending invalid
segments. Thus an infinite feedback loop of duplicate ACKs can ensue,
as each endpoint receives a duplicate ACK, decides that it is invalid
(due to sequence number, ACK number, or timestamp), and then sends a
dupack in reply, which the other side decides is invalid, responding
with a dupack... ad infinitum. This ping-pong feedback loop can happen
at a very high rate.
This phenomenon can and does happen in practice. It has been seen in
datacenter and Internet contexts at Google, and has been documented by
Anil Agarwal in the Nov 2013 tcpm thread "TCP mismatched sequence
numbers issue", and Avery Fay in the Feb 2015 Linux netdev thread
"Invalid timestamp? causing tight ack loop (hundreds of thousands of
packets / sec)".
This patch series
------------------
This patch series mitigates such ack loops by rate-limiting outgoing
duplicate ACKs sent in response to incoming TCP packets that are for
an existing connection but that are invalid due to any of the reasons
mentioned above: sequence number (1), ACK field (2), or timestamp
value (3). The rate limit for such duplicate ACKs is specified by a
new sysctl, tcp_invalid_ratelimit, which specifies the minimal space
between such outbound duplicate ACKs, in milliseconds. The default is
500 (500ms), and 0 disables the mechanism.
We rate-limit these duplicate ACK responses rather than blocking them
entirely or resetting the connection, because legitimate connections
can rely on dupacks in response to some out-of-window segments. For
example, zero window probes are typically sent with a sequence number
that is below the current window, and ZWPs thus expect to thus elicit
a dupack in response.
Testing: this approach has been in use at Google for a while.
Neal Cardwell (4):
tcp: helpers to mitigate ACK loops by rate-limiting out-of-window
dupacks
tcp: mitigate ACK loops for connections as tcp_request_sock
tcp: mitigate ACK loops for connections as tcp_sock
tcp: mitigate ACK loops for connections as tcp_timewait_sock
Documentation/networking/ip-sysctl.txt | 22 +++++++++++++++++++++
include/linux/tcp.h | 6 ++++++
include/net/tcp.h | 33 +++++++++++++++++++++++++++++++
include/uapi/linux/snmp.h | 6 ++++++
net/ipv4/proc.c | 6 ++++++
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv4/tcp_input.c | 30 +++++++++++++++++++++-------
net/ipv4/tcp_minisocks.c | 36 ++++++++++++++++++++++++++++------
8 files changed, 133 insertions(+), 13 deletions(-)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* [PATCH net-next 1/4] tcp: helpers to mitigate ACK loops by rate-limiting out-of-window dupacks
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1423256681-31716-1-git-send-email-ncardwell@google.com>
Helpers for mitigating ACK loops by rate-limiting dupacks sent in
response to incoming out-of-window packets.
This patch includes:
- rate-limiting logic
- sysctl to control how often we allow dupacks to out-of-window packets
- SNMP counter for cases where we rate-limited our dupack sending
The rate-limiting logic in this patch decides to not send dupacks in
response to out-of-window segments if (a) they are SYNs or pure ACKs
and (b) the remote endpoint is sending them faster than the configured
rate limit.
We rate-limit our responses rather than blocking them entirely or
resetting the connection, because legitimate connections can rely on
dupacks in response to some out-of-window segments. For example, zero
window probes are typically sent with a sequence number that is below
the current window, and ZWPs thus expect to thus elicit a dupack in
response.
We allow dupacks in response to TCP segments with data, because these
may be spurious retransmissions for which the remote endpoint wants to
receive DSACKs. This is safe because segments with data can't
realistically be part of ACK loops, which by their nature consist of
each side sending pure/data-less ACKs to each other.
The dupack interval is controlled by a new sysctl knob,
tcp_invalid_ratelimit, given in milliseconds, in case an administrator
needs to dial this upward in the face of a high-rate DoS attack. The
name and units are chosen to be analogous to the existing analogous
knob for ICMP, icmp_ratelimit.
The default value for tcp_invalid_ratelimit is 500ms, which allows at
most one such dupack per 500ms. This is chosen to be 2x faster than
the 1-second minimum RTO interval allowed by RFC 6298 (section 2, rule
2.4). We allow the extra 2x factor because network delay variations
can cause packets sent at 1 second intervals to be compressed and
arrive much closer.
Reported-by: Avery Fay <avery@mixpanel.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 22 ++++++++++++++++++++++
include/net/tcp.h | 32 ++++++++++++++++++++++++++++++++
include/uapi/linux/snmp.h | 6 ++++++
net/ipv4/proc.c | 6 ++++++
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv4/tcp_input.c | 1 +
6 files changed, 74 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index a5e4c81..1b8c964 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -290,6 +290,28 @@ tcp_frto - INTEGER
By default it's enabled with a non-zero value. 0 disables F-RTO.
+tcp_invalid_ratelimit - INTEGER
+ Limit the maximal rate for sending duplicate acknowledgments
+ in response to incoming TCP packets that are for an existing
+ connection but that are invalid due to any of these reasons:
+
+ (a) out-of-window sequence number,
+ (b) out-of-window acknowledgment number, or
+ (c) PAWS (Protection Against Wrapped Sequence numbers) check failure
+
+ This can help mitigate simple "ack loop" DoS attacks, wherein
+ a buggy or malicious middlebox or man-in-the-middle can
+ rewrite TCP header fields in manner that causes each endpoint
+ to think that the other is sending invalid TCP segments, thus
+ causing each side to send an unterminating stream of duplicate
+ acknowledgments for invalid segments.
+
+ Using 0 disables rate-limiting of dupacks in response to
+ invalid segments; otherwise this value specifies the minimal
+ space between sending such dupacks, in milliseconds.
+
+ Default: 500 (milliseconds).
+
tcp_keepalive_time - INTEGER
How often TCP sends out keepalive messages when keepalive is enabled.
Default: 2hours.
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 28e9bd3..b81f45c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -274,6 +274,7 @@ extern int sysctl_tcp_challenge_ack_limit;
extern unsigned int sysctl_tcp_notsent_lowat;
extern int sysctl_tcp_min_tso_segs;
extern int sysctl_tcp_autocorking;
+extern int sysctl_tcp_invalid_ratelimit;
extern atomic_long_t tcp_memory_allocated;
extern struct percpu_counter tcp_sockets_allocated;
@@ -1236,6 +1237,37 @@ static inline bool tcp_paws_reject(const struct tcp_options_received *rx_opt,
return true;
}
+/* Return true if we're currently rate-limiting out-of-window ACKs and
+ * thus shouldn't send a dupack right now. We rate-limit dupacks in
+ * response to out-of-window SYNs or ACKs to mitigate ACK loops or DoS
+ * attacks that send repeated SYNs or ACKs for the same connection. To
+ * do this, we do not send a duplicate SYNACK or ACK if the remote
+ * endpoint is sending out-of-window SYNs or pure ACKs at a high rate.
+ */
+static inline bool tcp_oow_rate_limited(struct net *net,
+ const struct sk_buff *skb,
+ int mib_idx, u32 *last_oow_ack_time)
+{
+ /* Data packets without SYNs are not likely part of an ACK loop. */
+ if ((TCP_SKB_CB(skb)->seq != TCP_SKB_CB(skb)->end_seq) &&
+ !tcp_hdr(skb)->syn)
+ goto not_rate_limited;
+
+ if (*last_oow_ack_time) {
+ s32 elapsed = (s32)(tcp_time_stamp - *last_oow_ack_time);
+
+ if (0 <= elapsed && elapsed < sysctl_tcp_invalid_ratelimit) {
+ NET_INC_STATS_BH(net, mib_idx);
+ return true; /* rate-limited: don't send yet! */
+ }
+ }
+
+ *last_oow_ack_time = tcp_time_stamp;
+
+not_rate_limited:
+ return false; /* not rate-limited: go ahead, send dupack now! */
+}
+
static inline void tcp_mib_init(struct net *net)
{
/* See RFC 2012 */
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index b222241..6a6fb74 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -270,6 +270,12 @@ enum
LINUX_MIB_TCPHYSTARTTRAINCWND, /* TCPHystartTrainCwnd */
LINUX_MIB_TCPHYSTARTDELAYDETECT, /* TCPHystartDelayDetect */
LINUX_MIB_TCPHYSTARTDELAYCWND, /* TCPHystartDelayCwnd */
+ LINUX_MIB_TCPACKSKIPPEDSYNRECV, /* TCPACKSkippedSynRecv */
+ LINUX_MIB_TCPACKSKIPPEDPAWS, /* TCPACKSkippedPAWS */
+ LINUX_MIB_TCPACKSKIPPEDSEQ, /* TCPACKSkippedSeq */
+ LINUX_MIB_TCPACKSKIPPEDFINWAIT2, /* TCPACKSkippedFinWait2 */
+ LINUX_MIB_TCPACKSKIPPEDTIMEWAIT, /* TCPACKSkippedTimeWait */
+ LINUX_MIB_TCPACKSKIPPEDCHALLENGE, /* TCPACKSkippedChallenge */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 8f9cd20..d8953ef 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -292,6 +292,12 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPHystartTrainCwnd", LINUX_MIB_TCPHYSTARTTRAINCWND),
SNMP_MIB_ITEM("TCPHystartDelayDetect", LINUX_MIB_TCPHYSTARTDELAYDETECT),
SNMP_MIB_ITEM("TCPHystartDelayCwnd", LINUX_MIB_TCPHYSTARTDELAYCWND),
+ SNMP_MIB_ITEM("TCPACKSkippedSynRecv", LINUX_MIB_TCPACKSKIPPEDSYNRECV),
+ SNMP_MIB_ITEM("TCPACKSkippedPAWS", LINUX_MIB_TCPACKSKIPPEDPAWS),
+ SNMP_MIB_ITEM("TCPACKSkippedSeq", LINUX_MIB_TCPACKSKIPPEDSEQ),
+ SNMP_MIB_ITEM("TCPACKSkippedFinWait2", LINUX_MIB_TCPACKSKIPPEDFINWAIT2),
+ SNMP_MIB_ITEM("TCPACKSkippedTimeWait", LINUX_MIB_TCPACKSKIPPEDTIMEWAIT),
+ SNMP_MIB_ITEM("TCPACKSkippedChallenge", LINUX_MIB_TCPACKSKIPPEDCHALLENGE),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index e0ee384..82601a6 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -729,6 +729,13 @@ static struct ctl_table ipv4_table[] = {
.extra2 = &one,
},
{
+ .procname = "tcp_invalid_ratelimit",
+ .data = &sysctl_tcp_invalid_ratelimit,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_ms_jiffies,
+ },
+ {
.procname = "icmp_msgs_per_sec",
.data = &sysctl_icmp_msgs_per_sec,
.maxlen = sizeof(int),
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d3dfff7..9401aa4 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -100,6 +100,7 @@ int sysctl_tcp_thin_dupack __read_mostly;
int sysctl_tcp_moderate_rcvbuf __read_mostly = 1;
int sysctl_tcp_early_retrans __read_mostly = 3;
+int sysctl_tcp_invalid_ratelimit __read_mostly = HZ/2;
#define FLAG_DATA 0x01 /* Incoming frame contained data. */
#define FLAG_WIN_UPDATE 0x02 /* Incoming ACK was a window update. */
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 2/4] tcp: mitigate ACK loops for connections as tcp_request_sock
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1423256681-31716-1-git-send-email-ncardwell@google.com>
In the SYN_RECV state, where the TCP connection is represented by
tcp_request_sock, we now rate-limit SYNACKs in response to a client's
retransmitted SYNs: we do not send a SYNACK in response to client SYN
if it has been less than sysctl_tcp_invalid_ratelimit (default 500ms)
since we last sent a SYNACK in response to a client's retransmitted
SYN.
This allows the vast majority of legitimate client connections to
proceed unimpeded, even for the most aggressive platforms, iOS and
MacOS, which actually retransmit SYNs 1-second intervals for several
times in a row. They use SYN RTO timeouts following the progression:
1,1,1,1,1,2,4,8,16,32.
Reported-by: Avery Fay <avery@mixpanel.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 1 +
include/net/tcp.h | 1 +
net/ipv4/tcp_minisocks.c | 6 +++++-
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 67309ec..bcc828d 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -115,6 +115,7 @@ struct tcp_request_sock {
u32 rcv_isn;
u32 snt_isn;
u32 snt_synack; /* synack sent time */
+ u32 last_oow_ack_time; /* last SYNACK */
u32 rcv_nxt; /* the ack # by SYNACK. For
* FastOpen it's the seq#
* after data-in-SYN.
diff --git a/include/net/tcp.h b/include/net/tcp.h
index b81f45c..da4196fb 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1145,6 +1145,7 @@ static inline void tcp_openreq_init(struct request_sock *req,
tcp_rsk(req)->rcv_isn = TCP_SKB_CB(skb)->seq;
tcp_rsk(req)->rcv_nxt = TCP_SKB_CB(skb)->seq + 1;
tcp_rsk(req)->snt_synack = tcp_time_stamp;
+ tcp_rsk(req)->last_oow_ack_time = 0;
req->mss = rx_opt->mss_clamp;
req->ts_recent = rx_opt->saw_tstamp ? rx_opt->rcv_tsval : 0;
ireq->tstamp_ok = rx_opt->tstamp_ok;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index bc9216d..131aa49 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -605,7 +605,11 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
* Reset timer after retransmitting SYNACK, similar to
* the idea of fast retransmit in recovery.
*/
- if (!inet_rtx_syn_ack(sk, req))
+ if (!tcp_oow_rate_limited(sock_net(sk), skb,
+ LINUX_MIB_TCPACKSKIPPEDSYNRECV,
+ &tcp_rsk(req)->last_oow_ack_time) &&
+
+ !inet_rtx_syn_ack(sk, req))
req->expires = min(TCP_TIMEOUT_INIT << req->num_timeout,
TCP_RTO_MAX) + jiffies;
return NULL;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 3/4] tcp: mitigate ACK loops for connections as tcp_sock
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1423256681-31716-1-git-send-email-ncardwell@google.com>
Ensure that in state ESTABLISHED, where the connection is represented
by a tcp_sock, we rate limit dupacks in response to incoming packets
(a) with TCP timestamps that fail PAWS checks, or (b) with sequence
numbers or ACK numbers that are out of the acceptable window.
We do not send a dupack in response to out-of-window packets if it has
been less than sysctl_tcp_invalid_ratelimit (default 500ms) since we
last sent a dupack in response to an out-of-window packet.
There is already a similar (although global) rate-limiting mechanism
for "challenge ACKs". When deciding whether to send a challence ACK,
we first consult the new per-connection rate limit, and then the
global rate limit.
Reported-by: Avery Fay <avery@mixpanel.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 1 +
net/ipv4/tcp_input.c | 29 ++++++++++++++++++++++-------
net/ipv4/tcp_minisocks.c | 1 +
3 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index bcc828d..66d85a8 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -153,6 +153,7 @@ struct tcp_sock {
u32 snd_sml; /* Last byte of the most recently transmitted small packet */
u32 rcv_tstamp; /* timestamp of last received ACK (for keepalives) */
u32 lsndtime; /* timestamp of last sent data packet (for restart window) */
+ u32 last_oow_ack_time; /* timestamp of last out-of-window ACK */
u32 tsoffset; /* timestamp offset */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9401aa4..8fdd27b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3322,13 +3322,22 @@ static int tcp_ack_update_window(struct sock *sk, const struct sk_buff *skb, u32
}
/* RFC 5961 7 [ACK Throttling] */
-static void tcp_send_challenge_ack(struct sock *sk)
+static void tcp_send_challenge_ack(struct sock *sk, const struct sk_buff *skb)
{
/* unprotected vars, we dont care of overwrites */
static u32 challenge_timestamp;
static unsigned int challenge_count;
- u32 now = jiffies / HZ;
+ struct tcp_sock *tp = tcp_sk(sk);
+ u32 now;
+
+ /* First check our per-socket dupack rate limit. */
+ if (tcp_oow_rate_limited(sock_net(sk), skb,
+ LINUX_MIB_TCPACKSKIPPEDCHALLENGE,
+ &tp->last_oow_ack_time))
+ return;
+ /* Then check the check host-wide RFC 5961 rate limit. */
+ now = jiffies / HZ;
if (now != challenge_timestamp) {
challenge_timestamp = now;
challenge_count = 0;
@@ -3424,7 +3433,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (before(ack, prior_snd_una)) {
/* RFC 5961 5.2 [Blind Data Injection Attack].[Mitigation] */
if (before(ack, prior_snd_una - tp->max_window)) {
- tcp_send_challenge_ack(sk);
+ tcp_send_challenge_ack(sk, skb);
return -1;
}
goto old_ack;
@@ -4993,7 +5002,10 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
tcp_paws_discard(sk, skb)) {
if (!th->rst) {
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_PAWSESTABREJECTED);
- tcp_send_dupack(sk, skb);
+ if (!tcp_oow_rate_limited(sock_net(sk), skb,
+ LINUX_MIB_TCPACKSKIPPEDPAWS,
+ &tp->last_oow_ack_time))
+ tcp_send_dupack(sk, skb);
goto discard;
}
/* Reset is accepted even if it did not pass PAWS. */
@@ -5010,7 +5022,10 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
if (!th->rst) {
if (th->syn)
goto syn_challenge;
- tcp_send_dupack(sk, skb);
+ if (!tcp_oow_rate_limited(sock_net(sk), skb,
+ LINUX_MIB_TCPACKSKIPPEDSEQ,
+ &tp->last_oow_ack_time))
+ tcp_send_dupack(sk, skb);
}
goto discard;
}
@@ -5026,7 +5041,7 @@ static bool tcp_validate_incoming(struct sock *sk, struct sk_buff *skb,
if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt)
tcp_reset(sk);
else
- tcp_send_challenge_ack(sk);
+ tcp_send_challenge_ack(sk, skb);
goto discard;
}
@@ -5040,7 +5055,7 @@ syn_challenge:
if (syn_inerr)
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_INERRS);
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPSYNCHALLENGE);
- tcp_send_challenge_ack(sk);
+ tcp_send_challenge_ack(sk, skb);
goto discard;
}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 131aa49..98a8405 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -467,6 +467,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
tcp_enable_early_retrans(newtp);
newtp->tlp_high_seq = 0;
newtp->lsndtime = treq->snt_synack;
+ newtp->last_oow_ack_time = 0;
newtp->total_retrans = req->num_retrans;
/* So many TCP implementations out there (incorrectly) count the
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 4/4] tcp: mitigate ACK loops for connections as tcp_timewait_sock
From: Neal Cardwell @ 2015-02-06 21:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Eric Dumazet
In-Reply-To: <1423256681-31716-1-git-send-email-ncardwell@google.com>
Ensure that in state FIN_WAIT2 or TIME_WAIT, where the connection is
represented by a tcp_timewait_sock, we rate limit dupacks in response
to incoming packets (a) with TCP timestamps that fail PAWS checks, or
(b) with sequence numbers that are out of the acceptable window.
We do not send a dupack in response to out-of-window packets if it has
been less than sysctl_tcp_invalid_ratelimit (default 500ms) since we
last sent a dupack in response to an out-of-window packet.
Reported-by: Avery Fay <avery@mixpanel.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 4 ++++
net/ipv4/tcp_minisocks.c | 29 ++++++++++++++++++++++++-----
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 66d85a8..1a7adb4 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -342,6 +342,10 @@ struct tcp_timewait_sock {
u32 tw_rcv_wnd;
u32 tw_ts_offset;
u32 tw_ts_recent;
+
+ /* The time we sent the last out-of-window ACK: */
+ u32 tw_last_oow_ack_time;
+
long tw_ts_recent_stamp;
#ifdef CONFIG_TCP_MD5SIG
struct tcp_md5sig_key *tw_md5_key;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 98a8405..dd11ac7 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -58,6 +58,25 @@ static bool tcp_in_window(u32 seq, u32 end_seq, u32 s_win, u32 e_win)
return seq == e_win && seq == end_seq;
}
+static enum tcp_tw_status
+tcp_timewait_check_oow_rate_limit(struct inet_timewait_sock *tw,
+ const struct sk_buff *skb, int mib_idx)
+{
+ struct tcp_timewait_sock *tcptw = tcp_twsk((struct sock *)tw);
+
+ if (!tcp_oow_rate_limited(twsk_net(tw), skb, mib_idx,
+ &tcptw->tw_last_oow_ack_time)) {
+ /* Send ACK. Note, we do not put the bucket,
+ * it will be released by caller.
+ */
+ return TCP_TW_ACK;
+ }
+
+ /* We are rate-limiting, so just release the tw sock and drop skb. */
+ inet_twsk_put(tw);
+ return TCP_TW_SUCCESS;
+}
+
/*
* * Main purpose of TIME-WAIT state is to close connection gracefully,
* when one of ends sits in LAST-ACK or CLOSING retransmitting FIN
@@ -116,7 +135,8 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb,
!tcp_in_window(TCP_SKB_CB(skb)->seq, TCP_SKB_CB(skb)->end_seq,
tcptw->tw_rcv_nxt,
tcptw->tw_rcv_nxt + tcptw->tw_rcv_wnd))
- return TCP_TW_ACK;
+ return tcp_timewait_check_oow_rate_limit(
+ tw, skb, LINUX_MIB_TCPACKSKIPPEDFINWAIT2);
if (th->rst)
goto kill;
@@ -250,10 +270,8 @@ kill:
inet_twsk_schedule(tw, &tcp_death_row, TCP_TIMEWAIT_LEN,
TCP_TIMEWAIT_LEN);
- /* Send ACK. Note, we do not put the bucket,
- * it will be released by caller.
- */
- return TCP_TW_ACK;
+ return tcp_timewait_check_oow_rate_limit(
+ tw, skb, LINUX_MIB_TCPACKSKIPPEDTIMEWAIT);
}
inet_twsk_put(tw);
return TCP_TW_SUCCESS;
@@ -289,6 +307,7 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
tcptw->tw_ts_recent = tp->rx_opt.ts_recent;
tcptw->tw_ts_recent_stamp = tp->rx_opt.ts_recent_stamp;
tcptw->tw_ts_offset = tp->tsoffset;
+ tcptw->tw_last_oow_ack_time = 0;
#if IS_ENABLED(CONFIG_IPV6)
if (tw->tw_family == PF_INET6) {
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
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