* [PATCH nf-next 9/9] netfilter: conntrack: consider ct netns in early_drop logic
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
When iterating, skip conntrack entries living in a different netns.
We could ignore netns and kill some other non-assured one, but it
has two problems:
- a netns can kill non-assured conntracks in other namespace
- we would start to 'over-subscribe' the affected/overlimit netns.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 43 +++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index d58b597..418e4bc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -763,18 +763,20 @@ static noinline int early_drop(struct net *net, unsigned int _hash)
{
/* Use oldest entry, which is roughly LRU */
struct nf_conntrack_tuple_hash *h;
- struct nf_conn *ct = NULL, *tmp;
+ struct nf_conn *tmp;
struct hlist_nulls_node *n;
- unsigned int i = 0, cnt = 0;
- int dropped = 0;
- unsigned int hash, sequence;
+ unsigned int i, hash, sequence;
+ struct nf_conn *ct = NULL;
spinlock_t *lockp;
+ bool ret = false;
+
+ i = 0;
local_bh_disable();
restart:
sequence = read_seqcount_begin(&nf_conntrack_generation);
- hash = scale_hash(_hash);
- for (; i < nf_conntrack_htable_size; i++) {
+ for (; i < NF_CT_EVICTION_RANGE; i++) {
+ hash = scale_hash(_hash++);
lockp = &nf_conntrack_locks[hash % CONNTRACK_LOCKS];
nf_conntrack_lock(lockp);
if (read_seqcount_retry(&nf_conntrack_generation, sequence)) {
@@ -784,35 +786,40 @@ restart:
hlist_nulls_for_each_entry_rcu(h, n, &nf_conntrack_hash[hash],
hnnode) {
tmp = nf_ct_tuplehash_to_ctrack(h);
- if (!test_bit(IPS_ASSURED_BIT, &tmp->status) &&
- !nf_ct_is_dying(tmp) &&
- atomic_inc_not_zero(&tmp->ct_general.use)) {
+
+ if (test_bit(IPS_ASSURED_BIT, &tmp->status) ||
+ !net_eq(nf_ct_net(tmp), net) ||
+ nf_ct_is_dying(tmp))
+ continue;
+
+ if (atomic_inc_not_zero(&tmp->ct_general.use)) {
ct = tmp;
break;
}
- cnt++;
}
- hash = (hash + 1) % nf_conntrack_htable_size;
spin_unlock(lockp);
-
- if (ct || cnt >= NF_CT_EVICTION_RANGE)
+ if (ct)
break;
-
}
+
local_bh_enable();
if (!ct)
- return dropped;
+ return false;
- if (del_timer(&ct->timeout)) {
+ /* kill only if in same netns -- might have moved due to
+ * SLAB_DESTROY_BY_RCU rules
+ */
+ if (net_eq(nf_ct_net(ct), net) && del_timer(&ct->timeout)) {
if (nf_ct_delete(ct, 0, 0)) {
- dropped = 1;
NF_CT_STAT_INC_ATOMIC(net, early_drop);
+ ret = true;
}
}
+
nf_ct_put(ct);
- return dropped;
+ return ret;
}
static struct nf_conn *
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 6/9] netfilter: conntrack: check netns when comparing conntrack objects
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
Once we place all conntracks in the same hash table we must also compare
the netns pointer to skip conntracks that belong to a different namespace.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 8 ++++++--
net/netfilter/nf_conntrack_core.c | 23 ++++++++++++----------
net/netfilter/nf_conntrack_netlink.c | 3 +++
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 483cf79..171aba1 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -115,6 +115,7 @@ static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
#endif
static bool ct_seq_should_skip(const struct nf_conn *ct,
+ const struct net *net,
const struct nf_conntrack_tuple_hash *hash)
{
/* we only want to print DIR_ORIGINAL */
@@ -124,6 +125,9 @@ static bool ct_seq_should_skip(const struct nf_conn *ct,
if (nf_ct_l3num(ct) != AF_INET)
return true;
+ if (!net_eq(nf_ct_net(ct), net))
+ return true;
+
return false;
}
@@ -136,7 +140,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
int ret = 0;
NF_CT_ASSERT(ct);
- if (ct_seq_should_skip(ct, hash))
+ if (ct_seq_should_skip(ct, seq_file_net(s), hash))
return 0;
if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
@@ -144,7 +148,7 @@ static int ct_seq_show(struct seq_file *s, void *v)
/* check if we raced w. object reuse */
if (!nf_ct_is_confirmed(ct) ||
- ct_seq_should_skip(ct, hash))
+ ct_seq_should_skip(ct, seq_file_net(s), hash))
goto release;
l3proto = __nf_ct_l3proto_find(nf_ct_l3num(ct));
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 3b9c302..10ae2ee 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -447,7 +447,8 @@ static void death_by_timeout(unsigned long ul_conntrack)
static inline bool
nf_ct_key_equal(struct nf_conntrack_tuple_hash *h,
const struct nf_conntrack_tuple *tuple,
- const struct nf_conntrack_zone *zone)
+ const struct nf_conntrack_zone *zone,
+ const struct net *net)
{
struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
@@ -456,7 +457,8 @@ nf_ct_key_equal(struct nf_conntrack_tuple_hash *h,
*/
return nf_ct_tuple_equal(tuple, &h->tuple) &&
nf_ct_zone_equal(ct, zone, NF_CT_DIRECTION(h)) &&
- nf_ct_is_confirmed(ct);
+ nf_ct_is_confirmed(ct) &&
+ net_eq(net, nf_ct_net(ct));
}
/*
@@ -481,7 +483,7 @@ begin:
} while (read_seqcount_retry(&nf_conntrack_generation, sequence));
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[bucket], hnnode) {
- if (nf_ct_key_equal(h, tuple, zone)) {
+ if (nf_ct_key_equal(h, tuple, zone, net)) {
NF_CT_STAT_INC_ATOMIC(net, found);
return h;
}
@@ -517,7 +519,7 @@ begin:
!atomic_inc_not_zero(&ct->ct_general.use)))
h = NULL;
else {
- if (unlikely(!nf_ct_key_equal(h, tuple, zone))) {
+ if (unlikely(!nf_ct_key_equal(h, tuple, zone, net))) {
nf_ct_put(ct);
goto begin;
}
@@ -573,12 +575,12 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
/* See if there's one in the list already, including reverse */
hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
- zone))
+ zone, net))
goto out;
hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
- zone))
+ zone, net))
goto out;
add_timer(&ct->timeout);
@@ -663,12 +665,12 @@ __nf_conntrack_confirm(struct sk_buff *skb)
not in the hash. If there is, we lost race. */
hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
- zone))
+ zone, net))
goto out;
hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
- zone))
+ zone, net))
goto out;
/* Timer relative to confirmation time, not original
@@ -740,7 +742,7 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[hash], hnnode) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (ct != ignored_conntrack &&
- nf_ct_key_equal(h, tuple, zone)) {
+ nf_ct_key_equal(h, tuple, zone, net)) {
NF_CT_STAT_INC_ATOMIC(net, found);
rcu_read_unlock();
return 1;
@@ -1383,7 +1385,8 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
continue;
ct = nf_ct_tuplehash_to_ctrack(h);
- if (iter(ct, data))
+ if (net_eq(nf_ct_net(ct), net) &&
+ iter(ct, data))
goto found;
}
}
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 294a8e2..f6bbcb2 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -837,6 +837,9 @@ restart:
if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
continue;
ct = nf_ct_tuplehash_to_ctrack(h);
+ if (!net_eq(net, nf_ct_net(ct)))
+ continue;
+
/* Dump entries of a given L3 protocol number.
* If it is not specified, ie. l3proto == 0,
* then dump everything. */
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 8/9] netfilter: conntrack: use a single hashtable for all namespaces
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
We already include netns address in the hash and compare the netns pointers
during lookup, so even if namespaces have overlapping addresses entries
will be spread across the table.
Assuming 64k bucket size, this change saves 0.5 mbyte per namespace on a
64bit system.
NAT bysrc and expectation hash is still per namespace, those will
changed too soon.
Future patch will also make conntrack object slab cache global again.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
checkpatch complains about 'WARNING: line over 80 characters' but
forcing line breaks looked even worse to me.
include/net/netfilter/nf_conntrack_core.h | 1 +
include/net/netns/conntrack.h | 2 -
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 2 +-
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 10 ++-
net/netfilter/nf_conntrack_core.c | 78 +++++++++++-----------
net/netfilter/nf_conntrack_helper.c | 6 +-
net/netfilter/nf_conntrack_netlink.c | 8 +--
net/netfilter/nf_conntrack_standalone.c | 13 ++--
net/netfilter/nf_nat_core.c | 2 +-
net/netfilter/nfnetlink_cttimeout.c | 6 +-
10 files changed, 60 insertions(+), 68 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 389e6da..e8ad0ad 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -82,6 +82,7 @@ print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple,
#define CONNTRACK_LOCKS 1024
+extern struct hlist_nulls_head *nf_conntrack_hash;
extern spinlock_t nf_conntrack_locks[CONNTRACK_LOCKS];
void nf_conntrack_lock(spinlock_t *lock);
diff --git a/include/net/netns/conntrack.h b/include/net/netns/conntrack.h
index b052785..251c435 100644
--- a/include/net/netns/conntrack.h
+++ b/include/net/netns/conntrack.h
@@ -93,9 +93,7 @@ struct netns_ct {
int sysctl_tstamp;
int sysctl_checksum;
- unsigned int htable_size;
struct kmem_cache *nf_conntrack_cachep;
- struct hlist_nulls_head *hash;
struct hlist_head *expect_hash;
struct ct_pcpu __percpu *pcpu_lists;
struct ip_conntrack_stat __percpu *stat;
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
index e3c46e8..ae1a71a 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c
@@ -360,7 +360,7 @@ static int ipv4_init_net(struct net *net)
in->ctl_table[0].data = &nf_conntrack_max;
in->ctl_table[1].data = &net->ct.count;
- in->ctl_table[2].data = &net->ct.htable_size;
+ in->ctl_table[2].data = &nf_conntrack_htable_size;
in->ctl_table[3].data = &net->ct.sysctl_checksum;
in->ctl_table[4].data = &net->ct.sysctl_log_invalid;
#endif
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index 171aba1..f8fc7ab 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -31,15 +31,14 @@ struct ct_iter_state {
static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
{
- struct net *net = seq_file_net(seq);
struct ct_iter_state *st = seq->private;
struct hlist_nulls_node *n;
for (st->bucket = 0;
- st->bucket < net->ct.htable_size;
+ st->bucket < nf_conntrack_htable_size;
st->bucket++) {
n = rcu_dereference(
- hlist_nulls_first_rcu(&net->ct.hash[st->bucket]));
+ hlist_nulls_first_rcu(&nf_conntrack_hash[st->bucket]));
if (!is_a_nulls(n))
return n;
}
@@ -49,17 +48,16 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
static struct hlist_nulls_node *ct_get_next(struct seq_file *seq,
struct hlist_nulls_node *head)
{
- struct net *net = seq_file_net(seq);
struct ct_iter_state *st = seq->private;
head = rcu_dereference(hlist_nulls_next_rcu(head));
while (is_a_nulls(head)) {
if (likely(get_nulls_value(head) == st->bucket)) {
- if (++st->bucket >= net->ct.htable_size)
+ if (++st->bucket >= nf_conntrack_htable_size)
return NULL;
}
head = rcu_dereference(
- hlist_nulls_first_rcu(&net->ct.hash[st->bucket]));
+ hlist_nulls_first_rcu(&nf_conntrack_hash[st->bucket]));
}
return head;
}
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index c29b929..d58b597 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -68,6 +68,9 @@ EXPORT_SYMBOL_GPL(nf_conntrack_locks);
__cacheline_aligned_in_smp DEFINE_SPINLOCK(nf_conntrack_expect_lock);
EXPORT_SYMBOL_GPL(nf_conntrack_expect_lock);
+struct hlist_nulls_head *nf_conntrack_hash __read_mostly;
+EXPORT_SYMBOL_GPL(nf_conntrack_hash);
+
static __read_mostly spinlock_t nf_conntrack_locks_all_lock;
static __read_mostly seqcount_t nf_conntrack_generation;
static __read_mostly bool nf_conntrack_locks_all;
@@ -163,9 +166,9 @@ static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple,
tuple->dst.protonum));
}
-static u32 hash_bucket(u32 hash, const struct net *net)
+static u32 scale_hash(u32 hash)
{
- return reciprocal_scale(hash, net->ct.htable_size);
+ return reciprocal_scale(hash, nf_conntrack_htable_size);
}
static u32 __hash_conntrack(const struct net *net,
@@ -178,7 +181,7 @@ static u32 __hash_conntrack(const struct net *net,
static u32 hash_conntrack(const struct net *net,
const struct nf_conntrack_tuple *tuple)
{
- return __hash_conntrack(net, tuple, net->ct.htable_size);
+ return scale_hash(hash_conntrack_raw(tuple, net));
}
bool
@@ -477,8 +480,8 @@ ____nf_conntrack_find(struct net *net, const struct nf_conntrack_zone *zone,
begin:
do {
sequence = read_seqcount_begin(&nf_conntrack_generation);
- bucket = hash_bucket(hash, net);
- ct_hash = net->ct.hash;
+ bucket = scale_hash(hash);
+ ct_hash = nf_conntrack_hash;
} while (read_seqcount_retry(&nf_conntrack_generation, sequence));
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[bucket], hnnode) {
@@ -542,12 +545,10 @@ static void __nf_conntrack_hash_insert(struct nf_conn *ct,
unsigned int hash,
unsigned int reply_hash)
{
- struct net *net = nf_ct_net(ct);
-
hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode,
- &net->ct.hash[hash]);
+ &nf_conntrack_hash[hash]);
hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_REPLY].hnnode,
- &net->ct.hash[reply_hash]);
+ &nf_conntrack_hash[reply_hash]);
}
int
@@ -572,12 +573,12 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
} while (nf_conntrack_double_lock(net, hash, reply_hash, sequence));
/* See if there's one in the list already, including reverse */
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
zone, net))
goto out;
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[reply_hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
zone, net))
goto out;
@@ -632,7 +633,7 @@ __nf_conntrack_confirm(struct sk_buff *skb)
sequence = read_seqcount_begin(&nf_conntrack_generation);
/* reuse the hash saved before */
hash = *(unsigned long *)&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev;
- hash = hash_bucket(hash, net);
+ hash = scale_hash(hash);
reply_hash = hash_conntrack(net,
&ct->tuplehash[IP_CT_DIR_REPLY].tuple);
@@ -662,12 +663,12 @@ __nf_conntrack_confirm(struct sk_buff *skb)
/* See if there's one in the list already, including reverse:
NAT could have grabbed it without realizing, since we're
not in the hash. If there is, we lost race. */
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
zone, net))
goto out;
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[reply_hash], hnnode)
if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
zone, net))
goto out;
@@ -735,7 +736,7 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
do {
sequence = read_seqcount_begin(&nf_conntrack_generation);
hash = hash_conntrack(net, tuple);
- ct_hash = net->ct.hash;
+ ct_hash = nf_conntrack_hash;
} while (read_seqcount_retry(&nf_conntrack_generation, sequence));
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[hash], hnnode) {
@@ -772,16 +773,16 @@ static noinline int early_drop(struct net *net, unsigned int _hash)
local_bh_disable();
restart:
sequence = read_seqcount_begin(&nf_conntrack_generation);
- hash = hash_bucket(_hash, net);
- for (; i < net->ct.htable_size; i++) {
+ hash = scale_hash(_hash);
+ for (; i < nf_conntrack_htable_size; i++) {
lockp = &nf_conntrack_locks[hash % CONNTRACK_LOCKS];
nf_conntrack_lock(lockp);
if (read_seqcount_retry(&nf_conntrack_generation, sequence)) {
spin_unlock(lockp);
goto restart;
}
- hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash],
- hnnode) {
+ hlist_nulls_for_each_entry_rcu(h, n, &nf_conntrack_hash[hash],
+ hnnode) {
tmp = nf_ct_tuplehash_to_ctrack(h);
if (!test_bit(IPS_ASSURED_BIT, &tmp->status) &&
!nf_ct_is_dying(tmp) &&
@@ -792,7 +793,7 @@ restart:
cnt++;
}
- hash = (hash + 1) % net->ct.htable_size;
+ hash = (hash + 1) % nf_conntrack_htable_size;
spin_unlock(lockp);
if (ct || cnt >= NF_CT_EVICTION_RANGE)
@@ -1375,12 +1376,12 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
int cpu;
spinlock_t *lockp;
- for (; *bucket < net->ct.htable_size; (*bucket)++) {
+ for (; *bucket < nf_conntrack_htable_size; (*bucket)++) {
lockp = &nf_conntrack_locks[*bucket % CONNTRACK_LOCKS];
local_bh_disable();
nf_conntrack_lock(lockp);
- if (*bucket < net->ct.htable_size) {
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket], hnnode) {
+ if (*bucket < nf_conntrack_htable_size) {
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[*bucket], hnnode) {
if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
continue;
ct = nf_ct_tuplehash_to_ctrack(h);
@@ -1527,7 +1528,6 @@ i_see_dead_people:
}
list_for_each_entry(net, net_exit_list, exit_list) {
- nf_ct_free_hashtable(net->ct.hash, net->ct.htable_size);
nf_conntrack_proto_pernet_fini(net);
nf_conntrack_helper_pernet_fini(net);
nf_conntrack_ecache_pernet_fini(net);
@@ -1598,10 +1598,10 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
* though since that required taking the locks.
*/
- for (i = 0; i < init_net.ct.htable_size; i++) {
- while (!hlist_nulls_empty(&init_net.ct.hash[i])) {
- h = hlist_nulls_entry(init_net.ct.hash[i].first,
- struct nf_conntrack_tuple_hash, hnnode);
+ for (i = 0; i < nf_conntrack_htable_size; i++) {
+ while (!hlist_nulls_empty(&nf_conntrack_hash[i])) {
+ h = hlist_nulls_entry(nf_conntrack_hash[i].first,
+ struct nf_conntrack_tuple_hash, hnnode);
ct = nf_ct_tuplehash_to_ctrack(h);
hlist_nulls_del_rcu(&h->hnnode);
bucket = __hash_conntrack(nf_ct_net(ct),
@@ -1609,11 +1609,11 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
hlist_nulls_add_head_rcu(&h->hnnode, &hash[bucket]);
}
}
- old_size = init_net.ct.htable_size;
- old_hash = init_net.ct.hash;
+ old_size = nf_conntrack_htable_size;
+ old_hash = nf_conntrack_hash;
- init_net.ct.htable_size = nf_conntrack_htable_size = hashsize;
- init_net.ct.hash = hash;
+ nf_conntrack_hash = hash;
+ nf_conntrack_htable_size = hashsize;
write_seqcount_end(&nf_conntrack_generation);
nf_conntrack_all_unlock();
@@ -1669,6 +1669,11 @@ int nf_conntrack_init_start(void)
* entries. */
max_factor = 4;
}
+
+ nf_conntrack_hash = nf_ct_alloc_hashtable(&nf_conntrack_htable_size, 1);
+ if (!nf_conntrack_hash)
+ return -ENOMEM;
+
nf_conntrack_max = max_factor * nf_conntrack_htable_size;
printk(KERN_INFO "nf_conntrack version %s (%u buckets, %d max)\n",
@@ -1747,6 +1752,7 @@ err_tstamp:
err_acct:
nf_conntrack_expect_fini();
err_expect:
+ nf_ct_free_hashtable(nf_conntrack_hash, nf_conntrack_htable_size);
return ret;
}
@@ -1799,12 +1805,6 @@ int nf_conntrack_init_net(struct net *net)
goto err_cache;
}
- net->ct.htable_size = nf_conntrack_htable_size;
- net->ct.hash = nf_ct_alloc_hashtable(&net->ct.htable_size, 1);
- if (!net->ct.hash) {
- printk(KERN_ERR "Unable to create nf_conntrack_hash\n");
- goto err_hash;
- }
ret = nf_conntrack_expect_pernet_init(net);
if (ret < 0)
goto err_expect;
@@ -1836,8 +1836,6 @@ err_tstamp:
err_acct:
nf_conntrack_expect_pernet_fini(net);
err_expect:
- nf_ct_free_hashtable(net->ct.hash, net->ct.htable_size);
-err_hash:
kmem_cache_destroy(net->ct.nf_conntrack_cachep);
err_cache:
kfree(net->ct.slabname);
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index 498bf74..cb48e6a 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -424,10 +424,10 @@ static void __nf_conntrack_helper_unregister(struct nf_conntrack_helper *me,
spin_unlock_bh(&pcpu->lock);
}
local_bh_disable();
- for (i = 0; i < net->ct.htable_size; i++) {
+ for (i = 0; i < nf_conntrack_htable_size; i++) {
nf_conntrack_lock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
- if (i < net->ct.htable_size) {
- hlist_nulls_for_each_entry(h, nn, &net->ct.hash[i], hnnode)
+ if (i < nf_conntrack_htable_size) {
+ hlist_nulls_for_each_entry(h, nn, &nf_conntrack_hash[i], hnnode)
unhelp(h, me);
}
spin_unlock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index f6bbcb2..e00f178 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -824,16 +824,16 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb)
last = (struct nf_conn *)cb->args[1];
local_bh_disable();
- for (; cb->args[0] < net->ct.htable_size; cb->args[0]++) {
+ for (; cb->args[0] < nf_conntrack_htable_size; cb->args[0]++) {
restart:
lockp = &nf_conntrack_locks[cb->args[0] % CONNTRACK_LOCKS];
nf_conntrack_lock(lockp);
- if (cb->args[0] >= net->ct.htable_size) {
+ if (cb->args[0] >= nf_conntrack_htable_size) {
spin_unlock(lockp);
goto out;
}
- hlist_nulls_for_each_entry(h, n, &net->ct.hash[cb->args[0]],
- hnnode) {
+ hlist_nulls_for_each_entry(h, n, &nf_conntrack_hash[cb->args[0]],
+ hnnode) {
if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
continue;
ct = nf_ct_tuplehash_to_ctrack(h);
diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index 0f1a45b..f87e84e 100644
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -54,14 +54,13 @@ struct ct_iter_state {
static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
{
- struct net *net = seq_file_net(seq);
struct ct_iter_state *st = seq->private;
struct hlist_nulls_node *n;
for (st->bucket = 0;
- st->bucket < net->ct.htable_size;
+ st->bucket < nf_conntrack_htable_size;
st->bucket++) {
- n = rcu_dereference(hlist_nulls_first_rcu(&net->ct.hash[st->bucket]));
+ n = rcu_dereference(hlist_nulls_first_rcu(&nf_conntrack_hash[st->bucket]));
if (!is_a_nulls(n))
return n;
}
@@ -71,18 +70,17 @@ static struct hlist_nulls_node *ct_get_first(struct seq_file *seq)
static struct hlist_nulls_node *ct_get_next(struct seq_file *seq,
struct hlist_nulls_node *head)
{
- struct net *net = seq_file_net(seq);
struct ct_iter_state *st = seq->private;
head = rcu_dereference(hlist_nulls_next_rcu(head));
while (is_a_nulls(head)) {
if (likely(get_nulls_value(head) == st->bucket)) {
- if (++st->bucket >= net->ct.htable_size)
+ if (++st->bucket >= nf_conntrack_htable_size)
return NULL;
}
head = rcu_dereference(
hlist_nulls_first_rcu(
- &net->ct.hash[st->bucket]));
+ &nf_conntrack_hash[st->bucket]));
}
return head;
}
@@ -458,7 +456,7 @@ static struct ctl_table nf_ct_sysctl_table[] = {
},
{
.procname = "nf_conntrack_buckets",
- .data = &init_net.ct.htable_size,
+ .data = &nf_conntrack_htable_size,
.maxlen = sizeof(unsigned int),
.mode = 0444,
.proc_handler = proc_dointvec,
@@ -512,7 +510,6 @@ static int nf_conntrack_standalone_init_sysctl(struct net *net)
goto out_kmemdup;
table[1].data = &net->ct.count;
- table[2].data = &net->ct.htable_size;
table[3].data = &net->ct.sysctl_checksum;
table[4].data = &net->ct.sysctl_log_invalid;
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index 3d52271..d74e716 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -824,7 +824,7 @@ nfnetlink_parse_nat_setup(struct nf_conn *ct,
static int __net_init nf_nat_net_init(struct net *net)
{
/* Leave them the same for the moment. */
- net->ct.nat_htable_size = net->ct.htable_size;
+ net->ct.nat_htable_size = nf_conntrack_htable_size;
net->ct.nat_bysource = nf_ct_alloc_hashtable(&net->ct.nat_htable_size, 0);
if (!net->ct.nat_bysource)
return -ENOMEM;
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 2671b9d..3c84f14 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -306,10 +306,10 @@ static void ctnl_untimeout(struct net *net, struct ctnl_timeout *timeout)
int i;
local_bh_disable();
- for (i = 0; i < net->ct.htable_size; i++) {
+ for (i = 0; i < nf_conntrack_htable_size; i++) {
nf_conntrack_lock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
- if (i < net->ct.htable_size) {
- hlist_nulls_for_each_entry(h, nn, &net->ct.hash[i], hnnode)
+ if (i < nf_conntrack_htable_size) {
+ hlist_nulls_for_each_entry(h, nn, &nf_conntrack_hash[i], hnnode)
untimeout(h, timeout);
}
spin_unlock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
--
2.7.3
^ permalink raw reply related
* RE: VRF_DEVICE integration plan
From: Elluru, Krishna Mohan @ 2016-04-28 17:16 UTC (permalink / raw)
To: David Ahern, netdev@vger.kernel.org
Cc: Kumara, Shantha (HP Networking), Govindan Nair, Anoop
In-Reply-To: <571D5720.7000908@cumulusnetworks.com>
HI David,
Thanks a lot for your response. It clarifies few of my questions. Please see inline for with tag MOHAN> for my response.
Thanks
Krishna Mohan.
-----Original Message-----
From: David Ahern [mailto:dsa@cumulusnetworks.com]
Sent: Monday, April 25, 2016 5:01 AM
To: Elluru, Krishna Mohan <elluru.kri.mohan@hpe.com>; netdev@vger.kernel.org
Subject: Re: VRF_DEVICE integration plan
On 4/23/16 10:07 PM, Elluru, Krishna Mohan wrote:
> HI Netdev team,
>
> Greetings. We have been monitoring the vrf device approach for l3 isolation from cumulus networks and we are currently interested in validating it. We have following questions on them and hoping to get answers from you/concerned team.
>
> 1. As per the linux documentation, there are known limits on if_index lookup, as the incoming if_index is changed to vrf_device index and thus an application receiving this packet will perceive this as a vrf_device packet, than right if_index. I saw you mentioned about a special flag to identify the origin, but didn't see the same in the latest linux 4.4.2 version code. Is there a patch expected for it?
you are referring to IP{6}_PKTINFO? I have patches from our 4.1 kernel tree that I have rebased to top of tree. I hope to send those out in the next few weeks.
MOHAN> Yes. Sure. Thanks.
>
> 2. What are the future additions planned for this approach? Are there any ipv4 and ipv6 known bugs with vrf_device model?
We have about 20 patches in our tree that I have not sent upstream yet.
Those patches fix PKTINFO, allow local traffic (e.g, ping in a VRF to a
local address in a VRF), allow IPv6 multicast and linklocal traffic, and
the cgroup implementation which has been sent as an RFC.
I posted a few bug fix patches a week or two ago. Not sure what the
status is with respect to 4.3 - 4.5 trees.
MOHAN> Sure. Are those patches sent over netdev mailer list?
>
> 3. It has been said in the documentation that, with addition of cgroup functionality for vrf device, with net_admin capabilities, we should be able to add an interface to vrf_device, currently it is not so. Any timelines on these?
I don't understand that question. The current implementation allows
adding interfaces (netdev's) to a VRF. The cgroup allows running a
process in a VRF context such that AF_INET{6} sockets are automatically
bound to the VRF device.
MOHAN> sorry for not being clear. My ask was, to create a namespace we need cap_admin privileges currently, but your earlier mails suggested that we should be able to configure/create vrf device with net_admin capabilities. Is this support present /expected to be added soon?
>
> 4. Currently the changes are available and portable from 4.3.x onwards. Is there a plan to port them to previous kernel versions?
no. Anyone wanting to use the vrf patches on other kernel versions will
need to port them.
MOHAN> Sure.
>
> 5. Is there a possibility of enabling secondary level lookup, to give a leak functionality to parent route table from device local route table? I tested with veth pair, configured one as default gateway, it is possible to forward traffic b/w the interfaces, looking for cleaner method.
Are you referring to inter-vrf routing? See slide 27
http://www.netdevconf.org/1.1/proceedings/slides/ahern-vrf-tutorial.pdf
Full lookup in VRF table
▪ ip route add table vrf-red 1.1.1.0/24 dev vrf-green
MOHAN> In slide 27 above shows inter vrf routing, requirement is to use current namespace global route table if the ip lookup fails in vrf-device routing table.
Reference: https://www.juniper.net/techpubs/en_US/junose16.1/topics/task/configuration/mbgp-secondary-routing-table-search.html
>
> 6. With "VRF Device" in place, please confirm if there are any plans to add VRF support for applications like
>
> 1. Ping
no need. ping{6} -I <vrf device> ...
> 2. Traceroute
no need. traceroute{6} -i <vrf device> ...
> 3. DNS-Client [glibc]
>
> In case of DNS-Client, most of the name resolution APIs will have to consider the VRF to do the lookup in and the way the domain-name/name-server configuration is stored.
I have looked into it but no patches worth distributing at the moment.
MOHAN> Okay, thanks for the inputs.
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Alexei Starovoitov @ 2016-04-28 17:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet
In-Reply-To: <1461821152-23200-1-git-send-email-edumazet@google.com>
On Wed, Apr 27, 2016 at 10:25:46PM -0700, Eric Dumazet wrote:
> Most of TCP stack assumed it was running from BH handler.
>
> This is great for most things, as TCP behavior is very sensitive
> to scheduling artifacts.
>
> However, the prequeue and backlog processing are problematic,
> as they need to be flushed with BH being blocked.
>
> To cope with modern needs, TCP sockets have big sk_rcvbuf values,
> in the order of 16 MB.
> This means that backlog can hold thousands of packets, and things
> like TCP coalescing or collapsing on this amount of packets can
> lead to insane latency spikes, since BH are blocked for too long.
>
> It is time to make UDP/TCP stacks preemptible.
>
> Note that fast path still runs from BH handler.
this looks pretty awesome.
the change will make the backlog run in bh enabled, so that one
large flow reciever will not penalize the rest of the system, right?
but you're saying that prequeue is also expensive, but not touched
by this patchset? was it addressed by your eariler patch?
Or more work still tbd?
I'm just trying to understand more about tcp stack.
^ permalink raw reply
* [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: Wang Shanker @ 2016-04-28 17:29 UTC (permalink / raw)
To: netdev; +Cc: James Chapman, Tom Herbert, David S. Miller
[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]
This patch fixes a bug which causes the behavior of whether to ignore
udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
userspace program requests.
When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
expected that udp6 checksums of received packets of the l2tp tunnel
to create should be ignored. In `l2tp_netlink.c`:
`l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
according to the flag, and then passed to `l2tp_core.c`:
`l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
`l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
to `false`, i.e. be set to the contrary. Similarly, the same should be
done to `udp_conf.use_udp6_tx_checksums`.
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
net/l2tp/l2tp_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index afca2eb..6edfa99 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
sizeof(udp_conf.peer_ip6));
udp_conf.use_udp6_tx_checksums =
- cfg->udp6_zero_tx_checksums;
+ ! cfg->udp6_zero_tx_checksums;
udp_conf.use_udp6_rx_checksums =
- cfg->udp6_zero_rx_checksums;
+ ! cfg->udp6_zero_rx_checksums;
} else
#endif
{
--
2.5.2
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4130 bytes --]
^ permalink raw reply related
* [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: Wang Shanker @ 2016-04-28 17:32 UTC (permalink / raw)
To: netdev; +Cc: James Chapman, Tom Herbert, David S. Miller
This patch fixes a bug which causes the behavior of whether to ignore
udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
userspace program requests.
When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
expected that udp6 checksums of received packets of the l2tp tunnel
to create should be ignored. In `l2tp_netlink.c`:
`l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
according to the flag, and then passed to `l2tp_core.c`:
`l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
`l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
to `false`, i.e. be set to the contrary. Similarly, the same should be
done to `udp_conf.use_udp6_tx_checksums`.
Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
net/l2tp/l2tp_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index afca2eb..6edfa99 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
sizeof(udp_conf.peer_ip6));
udp_conf.use_udp6_tx_checksums =
- cfg->udp6_zero_tx_checksums;
+ ! cfg->udp6_zero_tx_checksums;
udp_conf.use_udp6_rx_checksums =
- cfg->udp6_zero_rx_checksums;
+ ! cfg->udp6_zero_rx_checksums;
} else
#endif
{
--
2.5.2
^ permalink raw reply related
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Eric Dumazet @ 2016-04-28 17:41 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <20160428172320.GA83199@ast-mbp.thefacebook.com>
On Thu, 2016-04-28 at 10:23 -0700, Alexei Starovoitov wrote:
> On Wed, Apr 27, 2016 at 10:25:46PM -0700, Eric Dumazet wrote:
> > Most of TCP stack assumed it was running from BH handler.
> >
> > This is great for most things, as TCP behavior is very sensitive
> > to scheduling artifacts.
> >
> > However, the prequeue and backlog processing are problematic,
> > as they need to be flushed with BH being blocked.
> >
> > To cope with modern needs, TCP sockets have big sk_rcvbuf values,
> > in the order of 16 MB.
> > This means that backlog can hold thousands of packets, and things
> > like TCP coalescing or collapsing on this amount of packets can
> > lead to insane latency spikes, since BH are blocked for too long.
> >
> > It is time to make UDP/TCP stacks preemptible.
> >
> > Note that fast path still runs from BH handler.
>
> this looks pretty awesome.
Yes, I am pretty excited ;)
> the change will make the backlog run in bh enabled, so that one
> large flow reciever will not penalize the rest of the system, right?
Not only large flows, but flows with losses/reorders.
Typically many flows are in this case when a congestion collapse
happens.
> but you're saying that prequeue is also expensive, but not touched
> by this patchset? was it addressed by your eariler patch?
> Or more work still tbd?
prequeue is handled by "[2/6] tcp: do not block bh during prequeue
processing"
Note that I also sent a patch earlier (("tcp: give prequeue mode some
care")) to control max size of prequeue to 32 packets.
> I'm just trying to understand more about tcp stack.
>
Sure ;)
I also have a patch to add scheduling point in sendmsg() (ie : draining
the backlog if not empty) for each new skb added to the write queue.
Since each skb is about 64KB (with GSO/TSO), it means an application no
longer will hold the socket lock too long, even when doing a
write()/sendmsg() of say 8 MB at once ;)
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Alexei Starovoitov @ 2016-04-28 17:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <1461865270.5535.109.camel@edumazet-glaptop3.roam.corp.google.com>
On Thu, Apr 28, 2016 at 10:41:10AM -0700, Eric Dumazet wrote:
> On Thu, 2016-04-28 at 10:23 -0700, Alexei Starovoitov wrote:
> > On Wed, Apr 27, 2016 at 10:25:46PM -0700, Eric Dumazet wrote:
> > > Most of TCP stack assumed it was running from BH handler.
> > >
> > > This is great for most things, as TCP behavior is very sensitive
> > > to scheduling artifacts.
> > >
> > > However, the prequeue and backlog processing are problematic,
> > > as they need to be flushed with BH being blocked.
> > >
> > > To cope with modern needs, TCP sockets have big sk_rcvbuf values,
> > > in the order of 16 MB.
> > > This means that backlog can hold thousands of packets, and things
> > > like TCP coalescing or collapsing on this amount of packets can
> > > lead to insane latency spikes, since BH are blocked for too long.
> > >
> > > It is time to make UDP/TCP stacks preemptible.
> > >
> > > Note that fast path still runs from BH handler.
> >
> > this looks pretty awesome.
>
> Yes, I am pretty excited ;)
>
> > the change will make the backlog run in bh enabled, so that one
> > large flow reciever will not penalize the rest of the system, right?
>
> Not only large flows, but flows with losses/reorders.
> Typically many flows are in this case when a congestion collapse
> happens.
>
>
>
> > but you're saying that prequeue is also expensive, but not touched
> > by this patchset? was it addressed by your eariler patch?
> > Or more work still tbd?
>
> prequeue is handled by "[2/6] tcp: do not block bh during prequeue
> processing"
got it. It applies to both v4 and v6, right?
> Note that I also sent a patch earlier (("tcp: give prequeue mode some
> care")) to control max size of prequeue to 32 packets.
>
> > I'm just trying to understand more about tcp stack.
> >
> Sure ;)
>
> I also have a patch to add scheduling point in sendmsg() (ie : draining
> the backlog if not empty) for each new skb added to the write queue.
>
> Since each skb is about 64KB (with GSO/TSO), it means an application no
> longer will hold the socket lock too long, even when doing a
> write()/sendmsg() of say 8 MB at once ;)
that would be awesome. I think map/reduce type jobs typically
do large sendmsg, so it should help p99 latency.
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 17:56 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428163207.GP29024@lunn.ch>
On Thu, Apr 28, 2016 at 06:32:07PM +0200, Andrew Lunn wrote:
> > Hmm, are AT91 platforms special in this regard? As far as I can tell, this
> > driver (macb) and Marvell PXA are the only ethernet drivers that call
> > mdiobus_scan directly, and PXA does it on a known address. I do see that there
> > are trees that use macb and don't have a phy listed, which is unfortunate.
>
> How it is supposed to work is that you do one of two things:
>
> 1) Your device tree does not have an mdio node. In this case, you call
> mdiobus_register() and it will perform a scan of the bus, and find the
> phys.
>
> 2) Your device tree does have an MDIO node, and you list your PHYs.
>
> Having an MDIO node and not listing the PHYs is broken...
>
> There are however, a few broken device trees around, and a few drivers
> have workarounds. e.g. davinci_mdio.c
>
> /* register the mii bus
> * Create PHYs from DT only in case if PHY child nodes are explicitly
> * defined to support backward compatibility with DTs which assume that
> * Davinci MDIO will always scan the bus for PHYs detection.
> */
> if (dev->of_node && of_get_child_count(dev->of_node)) {
> data->skip_scan = true;
> ret = of_mdiobus_register(data->bus, dev->of_node);
> } else {
> ret = mdiobus_register(data->bus);
> }
>
> You probably need to do the same for AT91, count the number of
> children, and it if it zero, fall back to the non-DT way. It would
> also be good to print a warning to get people to fix their device
> tree.
>
> Andrew
I agree that is a valid fix for AT91, however it won't solve our problem, since
we have no children on the second ethernet MAC in our devices' device trees. I'm
starting to feel like our second MAC shouldn't even really register the MDIO bus
since it isn't being used - maybe adding a DT property to not have a bus is a
better option?
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: make TCP preemptible
From: Eric Dumazet @ 2016-04-28 18:04 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Eric Dumazet, David S . Miller, netdev
In-Reply-To: <20160428175154.GA83737@ast-mbp.thefacebook.com>
On Thu, 2016-04-28 at 10:51 -0700, Alexei Starovoitov wrote:
> got it. It applies to both v4 and v6, right?
You mean IPv6 ?
Well, you know, I decided to skip IPv6 and go to IPv8 anyway ;)
Yes, these changes apply for both IPv4 and IPv6 in the meantime.
> that would be awesome. I think map/reduce type jobs typically
> do large sendmsg, so it should help p99 latency.
Note that it should even help to send faster, since the ACK packets or
TCP Small Queue handlers are blocked until the sendmsg() was complete.
ACK packets usually open the window for more sends.
Many applications are using a loop and multiple sendmsg( ) with ~64KB
chunks to work around these latency issues.
^ permalink raw reply
* Re: [RFC 07/20] net: dsa: list ports in switch\\
From: Vivien Didelot @ 2016-04-28 18:18 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Jiri Pirko
In-Reply-To: <572241BB.1070304@gmail.com>
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 27/04/16 16:15, Andrew Lunn wrote:
>> On Wed, Apr 27, 2016 at 06:30:04PM -0400, Vivien Didelot wrote:
>>> List DSA port structures in their switch structure, so that drivers can
>>> iterate on them to retrieve information such as their ports membership.
>>
>> And this would be so much easier using a plan array.
>
> Agreed, I do not see much value in doing this at the moment. Even if you
> have unused ports in a switch, allocating an array is a small price to
> pay compared to directly indexing by port number.
>
> NAK from me unless there is a compelling reason for doing so.
The point of having a list is 1) get rid of the DSA_MAX_PORTS and have
variable number of ports 2) lists make iteration easier with variable
number of switchs/ports, e.g.:
dsa_tree_for_each_switch(dst, ds)
dsa_switch_for_each_port(ds, dp)
/* do something with the port */;
Anyway, I'm writing a proposal for a new design of DSA, in order to
support the D in DSA. That way, we'll avoid reviewing details of the
implementation and have a big picture of the necessary API changes.
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCH net] RDMA/nes: don't leak skb if carrier down
From: David Miller @ 2016-04-28 18:20 UTC (permalink / raw)
To: fw-HFFVJYpyMKqzQB+pC5nmwQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1461529139-28582-1-git-send-email-fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
From: Florian Westphal <fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
Date: Sun, 24 Apr 2016 22:18:59 +0200
> Alternatively one could free the skb, OTOH I don't think this test is
> useful so just remove it.
>
> Cc: <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Florian Westphal <fw-HFFVJYpyMKqzQB+pC5nmwQ@public.gmane.org>
> ---
> Noticed this while working on the TX_LOCKED removal.
Assuming Doug will take this.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 30/41] Documentation: networking: fix spelling mistakes
From: David Miller @ 2016-04-28 18:21 UTC (permalink / raw)
To: eric; +Cc: linux-kernel, corbet, dsa, shm, brouer, ast, linux-doc, netdev
In-Reply-To: <1461566229-4717-4-git-send-email-eric@engestrom.ch>
From: Eric Engestrom <eric@engestrom.ch>
Date: Mon, 25 Apr 2016 07:36:56 +0100
> Signed-off-by: Eric Engestrom <eric@engestrom.ch>
Applied.
^ permalink raw reply
* Re: myri10ge: fix sleeping with bh disabled
From: David Miller @ 2016-04-28 18:21 UTC (permalink / raw)
To: sgruszka; +Cc: netdev, hykim
In-Reply-To: <20160425085918.GB2608@redhat.com>
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Mon, 25 Apr 2016 10:59:19 +0200
> napi_disable() can not be called with bh disabled, move locking just
> around myri10ge_ss_lock_napi() .
>
> Patches fixes following bug:
>
> [ 114.278378] BUG: sleeping function called from invalid context at net/core/dev.c:4383
> <snip>
> [ 114.313712] Call Trace:
> [ 114.314943] [<ffffffff817010ce>] dump_stack+0x19/0x1b
> [ 114.317673] [<ffffffff810ce7f3>] __might_sleep+0x173/0x230
> [ 114.320566] [<ffffffff815b3117>] napi_disable+0x27/0x90
> [ 114.323254] [<ffffffffa01e437f>] myri10ge_close+0xbf/0x3f0 [myri10ge]
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] sfc: disable RSS when unsupported
From: David Miller @ 2016-04-28 18:22 UTC (permalink / raw)
To: bkenward; +Cc: netdev, jcooper, linux-net-drivers
In-Reply-To: <571E3CE4.6010808@solarflare.com>
From: Bert Kenward <bkenward@solarflare.com>
Date: Mon, 25 Apr 2016 16:51:00 +0100
> From: Jon Cooper <jcooper@solarflare.com>
>
> When certain firmware variants are selected (via the sfboot utility) the
> SFC7000 and SFC8000 series NICs don't support RSS. The driver still
> tries (and fails) to insert filters with the RSS flag, and the NIC fails
> to pass traffic.
>
> When the firmware reports RSS_LIMITED suppress allocating a default RSS
> context. The absence of an RSS context is picked up in filter insertion
> and RSS flags are discarded.
>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] MAINTAINERS: net: update sfc maintainers
From: David Miller @ 2016-04-28 18:22 UTC (permalink / raw)
To: bkenward; +Cc: linux-kernel, netdev, ecree, sshah, linux-net-drivers
In-Reply-To: <571E48E4.4050106@solarflare.com>
From: Bert Kenward <bkenward@solarflare.com>
Date: Mon, 25 Apr 2016 17:42:12 +0100
> Add myself and Edward Cree as maintainers.
> Remove Shradha Shah, who is on extended leave.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Edward Cree <ecree@solarflare.com>
> Cc: Shradha Shah <sshah@solarflare.com>
> Signed-off-by: Bert Kenward <bkenward@solarflare.com>
Applied.
^ permalink raw reply
* Re: pull-request: wireless-drivers 2016-04-25
From: David Miller @ 2016-04-28 18:23 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <87a8kh3bvj.fsf@kamboji.qca.qualcomm.com>
From: Kalle Valo <kvalo@codeaurora.org>
Date: Mon, 25 Apr 2016 19:13:20 +0300
> few fixes for 4.6, more info in the signed tag below. I'm hoping this to
> be the final pull request for 4.6 but let's see how it goes. Please let
> me know if there are any problems.
Pulled, thanks.
^ permalink raw reply
* Re: [RFC 07/20] net: dsa: list ports in switch\\
From: Florian Fainelli @ 2016-04-28 18:29 UTC (permalink / raw)
To: Vivien Didelot, Andrew Lunn
Cc: netdev, linux-kernel, kernel, David S. Miller, Jiri Pirko
In-Reply-To: <871t5pr3zm.fsf@ketchup.mtl.sfl>
On 28/04/16 11:18, Vivien Didelot wrote:
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 27/04/16 16:15, Andrew Lunn wrote:
>>> On Wed, Apr 27, 2016 at 06:30:04PM -0400, Vivien Didelot wrote:
>>>> List DSA port structures in their switch structure, so that drivers can
>>>> iterate on them to retrieve information such as their ports membership.
>>>
>>> And this would be so much easier using a plan array.
>>
>> Agreed, I do not see much value in doing this at the moment. Even if you
>> have unused ports in a switch, allocating an array is a small price to
>> pay compared to directly indexing by port number.
>>
>> NAK from me unless there is a compelling reason for doing so.
>
> The point of having a list is 1) get rid of the DSA_MAX_PORTS and have
> variable number of ports 2) lists make iteration easier with variable
> number of switchs/ports, e.g.:
You could get rid of the DSA_MAX_PORTS by asking switch drivers how many
ports they support and allocate that dynamically.
>
> dsa_tree_for_each_switch(dst, ds)
> dsa_switch_for_each_port(ds, dp)
> /* do something with the port */;
This is not more compact or efficient than an array walk, but at this
point this becoming preference over anything.
>
> Anyway, I'm writing a proposal for a new design of DSA, in order to
> support the D in DSA. That way, we'll avoid reviewing details of the
> implementation and have a big picture of the necessary API changes.
Quite frankly, I think your set of changes are submitted at a terrible
time, I would very much prefer to allow Andrew to complete his work on
re-designing the DSA layer to allow different kinds of switches, thus
allowing other people to support more HW in a blink of an eye, and
therefore allowing us all to get a clearer picture of what these little
switches are capable, rather than some patches that produce a lot of
churn with little documented benefits outside of the cross-chip operations.
Don't get me wrong, I think we should get to the point where you want us
to go, and work in that area is very much appreciated!
Thanks
--
Florian
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-28 18:43 UTC (permalink / raw)
To: Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428175619.GA8791@nathan3500-linux-VM>
> I agree that is a valid fix for AT91, however it won't solve our problem, since
> we have no children on the second ethernet MAC in our devices' device trees. I'm
> starting to feel like our second MAC shouldn't even really register the MDIO bus
> since it isn't being used - maybe adding a DT property to not have a bus is a
> better option?
status = "disabled"
would be the unusual way.
Andrew
^ permalink raw reply
* Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: James Chapman @ 2016-04-28 18:46 UTC (permalink / raw)
To: Wang Shanker; +Cc: netdev, Tom Herbert, David S. Miller
In-Reply-To: <9A325D67-FED1-4AF6-8ED0-6C93FFE8DC67@gmail.com>
Some additional background on this: Wang found this when configuring
l2tp tunnels using "ip l2tp" between two systems and then one system
was upgraded. The tunnel failed to pass data because one side had UDP
checksums enabled and the other now had them disabled. It seems kernel
changes related to UDP checksums resulted in a change to the default
UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
iproute2 doesn't let the user configure L2TP UDP checksum settings, so
without this fix, some users may see problems depending on the kernel
version differences on the L2TP peers. One for stable?
Acked-by: James Chapman <jchapman@katalix.com>
On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
> This patch fixes a bug which causes the behavior of whether to ignore
> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
> userspace program requests.
>
> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
> expected that udp6 checksums of received packets of the l2tp tunnel
> to create should be ignored. In `l2tp_netlink.c`:
> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
> according to the flag, and then passed to `l2tp_core.c`:
> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
> to `false`, i.e. be set to the contrary. Similarly, the same should be
> done to `udp_conf.use_udp6_tx_checksums`.
>
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
> net/l2tp/l2tp_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index afca2eb..6edfa99 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
> memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
> sizeof(udp_conf.peer_ip6));
> udp_conf.use_udp6_tx_checksums =
> - cfg->udp6_zero_tx_checksums;
> + ! cfg->udp6_zero_tx_checksums;
> udp_conf.use_udp6_rx_checksums =
> - cfg->udp6_zero_rx_checksums;
> + ! cfg->udp6_zero_rx_checksums;
> } else
> #endif
> {
> --
> 2.5.2
>
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 18:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428184303.GR29024@lunn.ch>
On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
> > I agree that is a valid fix for AT91, however it won't solve our problem, since
> > we have no children on the second ethernet MAC in our devices' device trees. I'm
> > starting to feel like our second MAC shouldn't even really register the MDIO bus
> > since it isn't being used - maybe adding a DT property to not have a bus is a
> > better option?
>
> status = "disabled"
>
> would be the unusual way.
>
> Andrew
Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
and so it does not have any PHYs under its DT node. It would be nice if there
were a way to tell macb not to bother with MDIO for the second MAC, since that's
handled by the first MAC.
I guess a good longer-term solution to all these problems would be to treat the
MAC and MDIO as seperate devices, like davinci seems to be doing.
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-28 18:59 UTC (permalink / raw)
To: Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428185527.GA8851@nathan3500-linux-VM>
On Thu, Apr 28, 2016 at 01:55:27PM -0500, Nathan Sullivan wrote:
> On Thu, Apr 28, 2016 at 08:43:03PM +0200, Andrew Lunn wrote:
> > > I agree that is a valid fix for AT91, however it won't solve our problem, since
> > > we have no children on the second ethernet MAC in our devices' device trees. I'm
> > > starting to feel like our second MAC shouldn't even really register the MDIO bus
> > > since it isn't being used - maybe adding a DT property to not have a bus is a
> > > better option?
> >
> > status = "disabled"
> >
> > would be the unusual way.
> >
> > Andrew
>
> Oh, sorry, I meant we use both MACs on Zynq, however the PHYs are on the MDIO
> bus of the first MAC. So, the second MAC is used for ethernet but not for MDIO,
> and so it does not have any PHYs under its DT node. It would be nice if there
> were a way to tell macb not to bother with MDIO for the second MAC, since that's
> handled by the first MAC.
Yes, exactly, add support for status = "disabled" in the mdio node.
> I guess a good longer-term solution to all these problems would be to treat the
> MAC and MDIO as seperate devices, like davinci seems to be doing.
A few others do this as well, e.g. most Marvell devices.
Andrew
^ permalink raw reply
* Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags
From: Wang Shanker @ 2016-04-28 19:25 UTC (permalink / raw)
To: James Chapman; +Cc: netdev, Tom Herbert, David S. Miller
In-Reply-To: <CAEwTi7SejvT35H0S0nBArEZFwoFAcCJ5twTKUmTxHcNx8az7Rg@mail.gmail.com>
I think this is a logic error, rather than a change to the default
UDP checksum setting. As expected, take rx for example, the flag
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` is not set by default, and udp6
checksum will be checked by default. The fact is that, not setting
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` leads to ignoring udp6 checksum. Such
a behavior does not correspond to the name
“L2TP_ATTR_UDP_ZERO_CSUM6_RX”. As a result, I call it a logic error.
> 在 2016年4月29日，02:46，James Chapman <jchapman@katalix.com> 写道：
>
> Some additional background on this: Wang found this when configuring
> l2tp tunnels using "ip l2tp" between two systems and then one system
> was upgraded. The tunnel failed to pass data because one side had UDP
> checksums enabled and the other now had them disabled. It seems kernel
> changes related to UDP checksums resulted in a change to the default
> UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
> iproute2 doesn't let the user configure L2TP UDP checksum settings, so
> without this fix, some users may see problems depending on the kernel
> version differences on the L2TP peers. One for stable?
>
> Acked-by: James Chapman <jchapman@katalix.com>
>
> On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>> This patch fixes a bug which causes the behavior of whether to ignore
>> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
>> userspace program requests.
>>
>> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
>> expected that udp6 checksums of received packets of the l2tp tunnel
>> to create should be ignored. In `l2tp_netlink.c`:
>> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
>> according to the flag, and then passed to `l2tp_core.c`:
>> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
>> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
>> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
>> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
>> to `false`, i.e. be set to the contrary. Similarly, the same should be
>> done to `udp_conf.use_udp6_tx_checksums`.
>>
>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
>> ---
>> net/l2tp/l2tp_core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
>> index afca2eb..6edfa99 100644
>> --- a/net/l2tp/l2tp_core.c
>> +++ b/net/l2tp/l2tp_core.c
>> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
>> memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
>> sizeof(udp_conf.peer_ip6));
>> udp_conf.use_udp6_tx_checksums =
>> - cfg->udp6_zero_tx_checksums;
>> + ! cfg->udp6_zero_tx_checksums;
>> udp_conf.use_udp6_rx_checksums =
>> - cfg->udp6_zero_rx_checksums;
>> + ! cfg->udp6_zero_rx_checksums;
>> } else
>> #endif
>> {
>> --
>> 2.5.2
>>
^ permalink raw reply
* Re: [PATCH] net: davinci_mdio: Set of_node in the mdio bus
From: David Miller @ 2016-04-28 19:44 UTC (permalink / raw)
To: Linux.HWI
Cc: linux-kernel, netdev, Grygorii.Strashko, jay.schroeder,
ben.mccauley
In-Reply-To: <1461595571-11438-1-git-send-email-Linux.HWI@garmin.com>
From: "J.D. Schroeder" <Linux.HWI@garmin.com>
Date: Mon, 25 Apr 2016 09:46:11 -0500
> From: "J.D. Schroeder" <jay.schroeder@garmin.com>
>
> Assigns the of_node from the platform device to the of_node of the
> mdio bus so that it can be used in the mdio driver to properly match
> a bus in the DT with a phandle in of_mdio_find_bus().
>
> Signed-off-by: J.D. Schroeder <jay.schroeder@garmin.com>
> Signed-off-by: Ben McCauley <ben.mccauley@garmin.com>
> ---
> drivers/net/ethernet/ti/davinci_mdio.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index 4e7c9b9..b5e5f37 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -343,6 +343,7 @@ static int davinci_mdio_probe(struct platform_device *pdev)
> if (davinci_mdio_probe_dt(&data->pdata, pdev))
> data->pdata = default_pdata;
> snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s", pdev->name);
> + data->bus->dev.of_node = dev->of_node;
> } else {
> data->pdata = pdata ? (*pdata) : default_pdata;
> snprintf(data->bus->id, MII_BUS_ID_SIZE, "%s-%x",
You can't do this.
First of all, of_node objects are reference counted. So even if this was a
legal thing to do you would have to drop the reference to the existing of_node
pointer and gain a reference to dev->of_node.
But even more importantly, it is the job of the bus driver to set that
bus->dev.of_node correctly, you should never override it in a driver like
this.
I'm not applying this, sorry.
^ 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