[Patch net-next 0/2] net: some minor optimization for netns id

[Patch net-next 0/2] net: some minor optimization for netns id

[Cong Wang]

Cong Wang (2):
  vxlan: call peernet2id() in fdb notification
  netns: avoid disabling irq for netns id

 drivers/net/vxlan.c      |  2 +-
 net/core/net_namespace.c | 37 ++++++++++++++++---------------------
 2 files changed, 17 insertions(+), 22 deletions(-)

-- 
2.1.0

[Patch net-next 1/2] vxlan: call peernet2id() in fdb notification

[Cong Wang]

netns id should be already allocated each time we change
netns, that is, in dev_change_net_namespace() (more precisely
in rtnl_fill_ifinfo()). It is safe to just call peernet2id() here.

Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 drivers/net/vxlan.c      | 2 +-
 net/core/net_namespace.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index f605a36..9735059 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -287,7 +287,7 @@ static int vxlan_fdb_info(struct sk_buff *skb, struct vxlan_dev *vxlan,
 
 	if (!net_eq(dev_net(vxlan->dev), vxlan->net) &&
 	    nla_put_s32(skb, NDA_LINK_NETNSID,
-			peernet2id_alloc(dev_net(vxlan->dev), vxlan->net)))
+			peernet2id(dev_net(vxlan->dev), vxlan->net)))
 		goto nla_put_failure;
 
 	if (send_eth && nla_put(skb, NDA_LLADDR, ETH_ALEN, &fdb->eth_addr))
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 7a77dca..f3fa435 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -227,7 +227,6 @@ int peernet2id_alloc(struct net *net, struct net *peer)
 		rtnl_net_notifyid(net, RTM_NEWNSID, id);
 	return id;
 }
-EXPORT_SYMBOL(peernet2id_alloc);
 
 /* This function returns, if assigned, the id of a peer netns. */
 int peernet2id(struct net *net, struct net *peer)
@@ -240,6 +239,7 @@ int peernet2id(struct net *net, struct net *peer)
 	spin_unlock_irqrestore(&net->nsid_lock, flags);
 	return id;
 }
+EXPORT_SYMBOL(peernet2id);
 
 /* This function returns true is the peer netns has an id assigned into the
  * current netns.
-- 
2.1.0

[Patch net-next 2/2] netns: avoid disabling irq for netns id

[Cong Wang]

We never read or change netns id in hardirq context,
the only place we read netns id in softirq context
is in vxlan_xmit(). So, it should be enough to just
disable BH.

Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/core/net_namespace.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index f3fa435..42bdda0 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -215,14 +215,13 @@ static void rtnl_net_notifyid(struct net *net, int cmd, int id);
  */
 int peernet2id_alloc(struct net *net, struct net *peer)
 {
-	unsigned long flags;
 	bool alloc;
 	int id;
 
-	spin_lock_irqsave(&net->nsid_lock, flags);
+	spin_lock_bh(&net->nsid_lock);
 	alloc = atomic_read(&peer->count) == 0 ? false : true;
 	id = __peernet2id_alloc(net, peer, &alloc);
-	spin_unlock_irqrestore(&net->nsid_lock, flags);
+	spin_unlock_bh(&net->nsid_lock);
 	if (alloc && id >= 0)
 		rtnl_net_notifyid(net, RTM_NEWNSID, id);
 	return id;
@@ -231,12 +230,11 @@ int peernet2id_alloc(struct net *net, struct net *peer)
 /* This function returns, if assigned, the id of a peer netns. */
 int peernet2id(struct net *net, struct net *peer)
 {
-	unsigned long flags;
 	int id;
 
-	spin_lock_irqsave(&net->nsid_lock, flags);
+	spin_lock_bh(&net->nsid_lock);
 	id = __peernet2id(net, peer);
-	spin_unlock_irqrestore(&net->nsid_lock, flags);
+	spin_unlock_bh(&net->nsid_lock);
 	return id;
 }
 EXPORT_SYMBOL(peernet2id);
@@ -251,18 +249,17 @@ bool peernet_has_id(struct net *net, struct net *peer)
 
 struct net *get_net_ns_by_id(struct net *net, int id)
 {
-	unsigned long flags;
 	struct net *peer;
 
 	if (id < 0)
 		return NULL;
 
 	rcu_read_lock();
-	spin_lock_irqsave(&net->nsid_lock, flags);
+	spin_lock_bh(&net->nsid_lock);
 	peer = idr_find(&net->netns_ids, id);
 	if (peer)
 		get_net(peer);
-	spin_unlock_irqrestore(&net->nsid_lock, flags);
+	spin_unlock_bh(&net->nsid_lock);
 	rcu_read_unlock();
 
 	return peer;
@@ -406,17 +403,17 @@ static void cleanup_net(struct work_struct *work)
 		for_each_net(tmp) {
 			int id;
 
-			spin_lock_irq(&tmp->nsid_lock);
+			spin_lock_bh(&tmp->nsid_lock);
 			id = __peernet2id(tmp, net);
 			if (id >= 0)
 				idr_remove(&tmp->netns_ids, id);
-			spin_unlock_irq(&tmp->nsid_lock);
+			spin_unlock_bh(&tmp->nsid_lock);
 			if (id >= 0)
 				rtnl_net_notifyid(tmp, RTM_DELNSID, id);
 		}
-		spin_lock_irq(&net->nsid_lock);
+		spin_lock_bh(&net->nsid_lock);
 		idr_destroy(&net->netns_ids);
-		spin_unlock_irq(&net->nsid_lock);
+		spin_unlock_bh(&net->nsid_lock);
 
 	}
 	rtnl_unlock();
@@ -544,7 +541,6 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tb[NETNSA_MAX + 1];
-	unsigned long flags;
 	struct net *peer;
 	int nsid, err;
 
@@ -565,15 +561,15 @@ static int rtnl_net_newid(struct sk_buff *skb, struct nlmsghdr *nlh)
 	if (IS_ERR(peer))
 		return PTR_ERR(peer);
 
-	spin_lock_irqsave(&net->nsid_lock, flags);
+	spin_lock_bh(&net->nsid_lock);
 	if (__peernet2id(net, peer) >= 0) {
-		spin_unlock_irqrestore(&net->nsid_lock, flags);
+		spin_unlock_bh(&net->nsid_lock);
 		err = -EEXIST;
 		goto out;
 	}
 
 	err = alloc_netid(net, peer, nsid);
-	spin_unlock_irqrestore(&net->nsid_lock, flags);
+	spin_unlock_bh(&net->nsid_lock);
 	if (err >= 0) {
 		rtnl_net_notifyid(net, RTM_NEWNSID, err);
 		err = 0;
@@ -695,11 +691,10 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
 		.idx = 0,
 		.s_idx = cb->args[0],
 	};
-	unsigned long flags;
 
-	spin_lock_irqsave(&net->nsid_lock, flags);
+	spin_lock_bh(&net->nsid_lock);
 	idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
-	spin_unlock_irqrestore(&net->nsid_lock, flags);
+	spin_unlock_bh(&net->nsid_lock);
 
 	cb->args[0] = net_cb.idx;
 	return skb->len;
-- 
2.1.0

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Nicolas Dichtel]

Le 02/09/2016 à 06:53, Cong Wang a écrit :
> We never read or change netns id in hardirq context,
> the only place we read netns id in softirq context
> is in vxlan_xmit(). So, it should be enough to just
> disable BH.

Are you sure? Did you audit all part of the code?
peernet2id() is called from netlink core system (do_one_broadcast()). Are you
sure that no driver call this function from an hard irq context?

I think that NETLINK_LISTEN_ALL_NSID is largely untested, so it will be hard to
detect a bug introduced in this feature.

Re: [Patch net-next 1/2] vxlan: call peernet2id() in fdb notification

[Nicolas Dichtel]

Le 02/09/2016 à 06:53, Cong Wang a écrit :
> netns id should be already allocated each time we change
> netns, that is, in dev_change_net_namespace() (more precisely
> in rtnl_fill_ifinfo()). It is safe to just call peernet2id() here.
> 
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Cong Wang]

On Fri, Sep 2, 2016 at 1:12 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>> We never read or change netns id in hardirq context,
>> the only place we read netns id in softirq context
>> is in vxlan_xmit(). So, it should be enough to just
>> disable BH.
>
> Are you sure? Did you audit all part of the code?

I did audit all the callers, and I didn't find any of them in IRQ context.

> peernet2id() is called from netlink core system (do_one_broadcast()). Are you
> sure that no driver call this function from an hard irq context?

I audit all callers of netlink_broadcast(), and I don't see any of
them in hardirq context.

>
> I think that NETLINK_LISTEN_ALL_NSID is largely untested, so it will be hard to
> detect a bug introduced in this feature.

This patch passed my netns stress tests, I have LOCKDEP enabled,
and I don't get any warning or crash etc.

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Cong Wang]

On Fri, Sep 2, 2016 at 9:39 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Sep 2, 2016 at 1:12 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>>> We never read or change netns id in hardirq context,
>>> the only place we read netns id in softirq context
>>> is in vxlan_xmit(). So, it should be enough to just
>>> disable BH.
>>
>> Are you sure? Did you audit all part of the code?
>
> I did audit all the callers, and I didn't find any of them in IRQ context.
>
>> peernet2id() is called from netlink core system (do_one_broadcast()). Are you
>> sure that no driver call this function from an hard irq context?
>
> I audit all callers of netlink_broadcast(), and I don't see any of
> them in hardirq context.

Note, you can rule out most of them by checking GFP_KERNEL,
which indicates a process context. ;) For GFP_ATOMIC cases,
I don't see any of them in hardirq context either, but I am definitely
not familiar with drivers like infiniband.

Re: [Patch net-next 0/2] net: some minor optimization for netns id

[David Miller]

Series applied.

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Nicolas Dichtel]

Le 02/09/2016 à 19:24, Cong Wang a écrit :
> On Fri, Sep 2, 2016 at 9:39 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Fri, Sep 2, 2016 at 1:12 AM, Nicolas Dichtel
>> <nicolas.dichtel@6wind.com> wrote:
>>> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>>>> We never read or change netns id in hardirq context,
>>>> the only place we read netns id in softirq context
>>>> is in vxlan_xmit(). So, it should be enough to just
>>>> disable BH.
>>>
>>> Are you sure? Did you audit all part of the code?
>>
>> I did audit all the callers, and I didn't find any of them in IRQ context.
>>
>>> peernet2id() is called from netlink core system (do_one_broadcast()). Are you
>>> sure that no driver call this function from an hard irq context?
>>
>> I audit all callers of netlink_broadcast(), and I don't see any of
>> them in hardirq context.
> 
> Note, you can rule out most of them by checking GFP_KERNEL,
> which indicates a process context. ;) For GFP_ATOMIC cases,
> I don't see any of them in hardirq context either, but I am definitely
> not familiar with drivers like infiniband.
> 
Yes, I was thinking to that one. But I'm also not familiar with it ;-)

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Elad Raz]

On Fri, Sep 2, 2016 at 11:12 AM, Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>> We never read or change netns id in hardirq context,
>> the only place we read netns id in softirq context
>> is in vxlan_xmit(). So, it should be enough to just
>> disable BH.
>
> Are you sure? Did you audit all part of the code?
> peernet2id() is called from netlink core system (do_one_broadcast()). Are you
> sure that no driver call this function from an hard irq context?
>
> I think that NETLINK_LISTEN_ALL_NSID is largely untested, so it will be hard to
> detect a bug introduced in this feature.

I'm seeing strange things on our systems on boot time when trying to
mount autofs.
I bisected and got this patch as the bad one.
I can see that only when I'm using "debug" config file.

CONFIG_LOCKDEP =y I can see on boot:

[  OK  ] Started OpenSSH server daemon.
        Starting OpenSSH server daemon...
[   23.498577] ------------[ cut here ]------------
[   23.503247] WARNING: CPU: 0 PID: 994 at kernel/softirq.c:150
__local_bh_enable_ip+0x9d/0xc0
[   23.511665] Modules linked in: xt_conntrack ebtable_nat
ebtable_broute bridge stp llc ebtable_filter ebtables iptable_nat
nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack
iptable_mangle iptable_security iptable_raw intel_rapl
x86_pkg_temp_thermal iTCO
_wdt coretemp iTCO_vendor_support kvm_intel kvm irqbypass
crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcspkr i2c_i801
i2c_smbus lpc_ich mfd_core mei_me mei video tpm_tis tpm_tis_core tpm
nfsd auth_rpcgss nfs_acl lockd grace sunrpc xfs libcrc32c crc32c_intel
e1000e
ptp pps_core
[   23.561125] CPU: 0 PID: 994 Comm: setsebool Not tainted 4.8.0-rc4eladr+ #51
[   23.568152] Hardware name: Mellanox Technologies Ltd. Mellanox
switch/Mellanox switch, BIOS 4.6.5 05/21/2015
[   23.578066]  0000000000000000 ffff8801161d3bb0 ffffffff81411b73
0000000000000000
[   23.585594]  0000000000000000 ffff8801161d3bf0 ffffffff810aa69b
00000096816dcda7
[   23.593118]  0000000000000200 ffffffff816dcdc5 ffffffff81f0fb00
ffff8801147a0000
[   23.600639] Call Trace:
[   23.603114]  [<ffffffff81411b73>] dump_stack+0x85/0xc2
[   23.608302]  [<ffffffff810aa69b>] __warn+0xcb/0xf0
[   23.613138]  [<ffffffff816dcdc5>] ? peernet2id+0x45/0x60
[   23.618502]  [<ffffffff810aa78d>] warn_slowpath_null+0x1d/0x20
[   23.624394]  [<ffffffff810b0a4d>] __local_bh_enable_ip+0x9d/0xc0
[   23.630453]  [<ffffffff8183e845>] _raw_spin_unlock_bh+0x35/0x40
[   23.636432]  [<ffffffff816dcdc5>] peernet2id+0x45/0x60
[   23.641626]  [<ffffffff81727ac5>] netlink_broadcast_filtered+0x265/0x3d0
[   23.648385]  [<ffffffff81727c4d>] netlink_broadcast+0x1d/0x20
[   23.654188]  [<ffffffff81165b21>] audit_log_end+0x2b1/0x2c0
[   23.659820]  [<ffffffff811658a0>] ? audit_log_end+0x30/0x2c0
[   23.665532]  [<ffffffff8116631a>] audit_log+0x5a/0x70
[   23.670631]  [<ffffffff813ab7ce>] security_set_bools+0xee/0x200
[   23.676602]  [<ffffffff8139cbc8>] sel_commit_bools_write+0xb8/0x100
[   23.682928]  [<ffffffff8126c1d8>] __vfs_write+0x28/0x120
[   23.688291]  [<ffffffff810fcc17>] ? update_fast_ctr+0x17/0x30
[   23.694087]  [<ffffffff810fcca9>] ? percpu_down_read+0x49/0x90
[   23.699980]  [<ffffffff81270567>] ? __sb_start_write+0xb7/0xf0
[   23.705872]  [<ffffffff81270567>] ? __sb_start_write+0xb7/0xf0
[   23.711758]  [<ffffffff8126d338>] vfs_write+0xb8/0x1b0
[   23.716949]  [<ffffffff81100e09>] ? trace_hardirqs_on_caller+0x129/0x1b0
[   23.723717]  [<ffffffff8126e689>] SyS_write+0x49/0xa0
[   23.728828]  [<ffffffff8183ef3c>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[   23.735337] ---[ end trace 576457efb89c3ea5 ]---

and

        Starting NIS/YP (Network Informatio... Clients to NIS Domain Binder...
[   24.182895] FS-Cache: Netfs 'nfs' registered for caching
[  OK  ] Started rolekit - role server.
[   24.210512] Key type dns_resolver registered
[   24.243061] NFS: Registering the id_resolver key type
[   24.253951] Key type id_resolver registered
[   24.258215] Key type id_legacy registered
[   24.272025]
[   24.273545] =================================
[   24.277919] [ INFO: inconsistent lock state ]
[   24.282348] 4.8.0-rc4eladr+ #51 Tainted: G        W
[   24.287806] ---------------------------------
[   24.292234] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-R} usage.
[   24.298301] dbus-daemon/673 [HC0[0]:SC1[1]:HE1:SE0] takes:
[   24.303898]  (policy_rwlock){+++?..}, at: [<ffffffff813ac742>]
security_netlbl_sid_to_secattr+0x32/0xc0
[   24.313456] {SOFTIRQ-ON-W} state was registered at:
[   24.318388]   [<ffffffff81100caf>] mark_held_locks+0x6f/0xa0
[   24.324134]   [<ffffffff81100e09>] trace_hardirqs_on_caller+0x129/0x1b0
[   24.330900]   [<ffffffff81100e9d>] trace_hardirqs_on+0xd/0x10
[   24.336708]   [<ffffffff810b0a20>] __local_bh_enable_ip+0x70/0xc0
[   24.342912]   [<ffffffff8183e845>] _raw_spin_unlock_bh+0x35/0x40
[   24.349005]   [<ffffffff816dcdc5>] peernet2id+0x45/0x60
[   24.354361]   [<ffffffff81727ac5>] netlink_broadcast_filtered+0x265/0x3d0
[   24.361285]   [<ffffffff81727c4d>] netlink_broadcast+0x1d/0x20
[   24.367222]   [<ffffffff81165b21>] audit_log_end+0x2b1/0x2c0
[   24.372957]   [<ffffffff8116631a>] audit_log+0x5a/0x70
[   24.378174]   [<ffffffff813ab7ce>] security_set_bools+0xee/0x200
[   24.384283]   [<ffffffff8139cbc8>] sel_commit_bools_write+0xb8/0x100
[   24.390733]   [<ffffffff8126c1d8>] __vfs_write+0x28/0x120
[   24.396208]   [<ffffffff8126d338>] vfs_write+0xb8/0x1b0
[   24.401537]   [<ffffffff8126e689>] SyS_write+0x49/0xa0
[   24.406746]   [<ffffffff8183ef3c>] entry_SYSCALL_64_fastpath+0x1f/0xbd
[   24.413375] irq event stamp: 73908
[   24.416878] hardirqs last  enabled at (73908): [<ffffffff812433be>]
__slab_alloc+0x5e/0x90
[   24.425215] hardirqs last disabled at (73907): [<ffffffff8124339c>]
__slab_alloc+0x3c/0x90
[   24.433621] softirqs last  enabled at (73890): [<ffffffff8173f89d>]
ip_finish_output2+0x1fd/0x610
[   24.442634] softirqs last disabled at (73891): [<ffffffff8183ffcc>]
do_softirq_own_stack+0x1c/0x30
[   24.451776]
[   24.451776] other info that might help us debug this:
[   24.458345]  Possible unsafe locking scenario:
[   24.458345]
[   24.464357]        CPU0
[   24.466854]        ----
[   24.469324]   lock(policy_rwlock);
[   24.472817]   <Interrupt>
[   24.475450]     lock(policy_rwlock);
[   24.479108]
[   24.479108]  *** DEADLOCK ***
[   24.479108]
[   24.485081] 4 locks held by dbus-daemon/673:
[   24.489386]  #0:  (sk_lock-AF_INET){+.+.+.}, at:
[<ffffffff8177dcb7>] inet_stream_connect+0x27/0x50
[   24.498659]  #1:  (rcu_read_lock){......}, at: [<ffffffff81740d65>]
ip_queue_xmit+0x5/0x560
[   24.507169]  #2:  (rcu_read_lock){......}, at: [<ffffffff816e949d>]
process_backlog+0xcd/0x230
[   24.515921]  #3:  (rcu_read_lock){......}, at: [<ffffffff8173a8af>]
ip_local_deliver_finish+0x2f/0x390
                       [29/1805]
[   24.525419]
[   24.525419] stack backtrace:
[   24.529833] CPU: 0 PID: 673 Comm: dbus-daemon Tainted: G        W
    4.8.0-rc4eladr+ #51
[   24.538323] Hardware name: Mellanox Technologies Ltd. Mellanox
switch/Mellanox switch, BIOS 4.6.5 05/21/2015
[   24.548246]  0000000000000000 ffff88011dc037d0 ffffffff81411b73
ffff880114955700
[   24.555777]  ffffffff828623a0 ffff88011dc03820 ffffffff81100605
0000000000000001
[   24.563308]  0000000000000001 ffff880100000000 0000000000000006
0000000000000005
[   24.570811] Call Trace:
[   24.573284]  <IRQ>  [<ffffffff81411b73>] dump_stack+0x85/0xc2
[   24.579093]  [<ffffffff81100605>] print_usage_bug+0x215/0x240
[   24.584889]  [<ffffffff81100b7a>] mark_lock+0x54a/0x610
[   24.590165]  [<ffffffff810ffa80>] ?
print_shortest_lock_dependencies+0x1a0/0x1a0
[   24.597655]  [<ffffffff8110174d>] __lock_acquire+0x59d/0x1490
[   24.603469]  [<ffffffff81241b0c>] ? new_slab+0x35c/0x670
[   24.608860]  [<ffffffff81102ab2>] lock_acquire+0xf2/0x1e0
[   24.614363]  [<ffffffff813ac742>] ? security_netlbl_sid_to_secattr+0x32/0xc0
[   24.621521]  [<ffffffff8183e924>] _raw_read_lock+0x34/0x50
[   24.627058]  [<ffffffff813ac742>] ? security_netlbl_sid_to_secattr+0x32/0xc0
[   24.634154]  [<ffffffff813ac742>] security_netlbl_sid_to_secattr+0x32/0xc0
[   24.641070]  [<ffffffff813af520>] selinux_netlbl_inet_conn_request+0x40/0xe0
[   24.648178]  [<ffffffff81393302>] selinux_inet_conn_request+0x62/0x90
[   24.654686]  [<ffffffff8138bf53>] security_inet_conn_request+0x43/0x60
[   24.661255]  [<ffffffff8175298a>] tcp_conn_request+0x32a/0xa50
[   24.667154]  [<ffffffff8139543e>] ? selinux_peerlbl_enabled+0x1e/0x40
[   24.673707]  [<ffffffff81397f89>] ? selinux_socket_sock_rcv_skb+0x89/0x200
[   24.680675]  [<ffffffff81762726>] tcp_v4_conn_request+0x86/0xb0
[   24.686638]  [<ffffffff81757db4>] tcp_rcv_state_process+0x194/0xef0
[   24.692980]  [<ffffffff81763832>] tcp_v4_do_rcv+0xb2/0x200
[   24.698507]  [<ffffffff81764f50>] tcp_v4_rcv+0xba0/0xbf0
[   24.703884]  [<ffffffff8173a95a>] ip_local_deliver_finish+0xda/0x390
[   24.710303]  [<ffffffff8173a8af>] ? ip_local_deliver_finish+0x2f/0x390
[   24.716933]  [<ffffffff8173ae50>] ip_local_deliver+0x60/0xd0
[   24.722662]  [<ffffffff8173a880>] ? ip_rcv_finish+0x620/0x620
[   24.728476]  [<ffffffff8173a3ef>] ip_rcv_finish+0x18f/0x620
[   24.734118]  [<ffffffff8173b12c>] ip_rcv+0x26c/0x390
[   24.739129]  [<ffffffff8173a260>] ? inet_del_offload+0x40/0x40
[   24.745029]  [<ffffffff816e8a8a>] __netif_receive_skb_core+0x22a/0xb10
[   24.751630]  [<ffffffff816e949d>] ? process_backlog+0xcd/0x230
[   24.757525]  [<ffffffff816e9388>] __netif_receive_skb+0x18/0x60
[   24.763513]  [<ffffffff816e9442>] process_backlog+0x72/0x230
[   24.769213]  [<ffffffff816e949d>] ? process_backlog+0xcd/0x230
[   24.775090]  [<ffffffff816eb38f>] net_rx_action+0x21f/0x430
[   24.780699]  [<ffffffff81100caf>] ? mark_held_locks+0x6f/0xa0
[   24.786504]  [<ffffffff81841f29>] __do_softirq+0xc9/0x44d
[   24.791946]  [<ffffffff8173f89d>] ? ip_finish_output2+0x1fd/0x610
[   24.798091]  [<ffffffff8183ffcc>] do_softirq_own_stack+0x1c/0x30
[   24.804164]  <EOI>  [<ffffffff810b09a9>] do_softirq.part.17+0x59/0x60
[   24.810699]  [<ffffffff810b0a69>] __local_bh_enable_ip+0xb9/0xc0
[   24.816817]  [<ffffffff8173f8c6>] ip_finish_output2+0x226/0x610
[   24.822804]  [<ffffffff8174083e>] ? ip_finish_output+0x1ae/0x340
[   24.828897]  [<ffffffff8174083e>] ip_finish_output+0x1ae/0x340
[   24.834800]  [<ffffffff8172db12>] ? nf_hook_slow+0xd2/0x170
[   24.840433]  [<ffffffff81741614>] ip_output+0x74/0x120
[   24.845633]  [<ffffffff81740690>] ? ip_fragment.constprop.53+0x80/0x80
[   24.852236]  [<ffffffff81740b1d>] ip_local_out+0x3d/0x80
[   24.857607]  [<ffffffff81740f3d>] ip_queue_xmit+0x1dd/0x560
[   24.863259]  [<ffffffff81740d65>] ? ip_queue_xmit+0x5/0x560
[   24.868894]  [<ffffffff8175aeb4>] tcp_transmit_skb+0x4a4/0x950
[   24.874776]  [<ffffffff8175ceab>] tcp_connect+0x60b/0xa60
[   24.880287]  [<ffffffff816de8d4>] ? secure_tcp_sequence_number+0x74/0xc0
[   24.887099]  [<ffffffff817623e3>] tcp_v4_connect+0x323/0x520
[   24.892836]  [<ffffffff8177da35>] __inet_stream_connect+0x95/0x2f0
[   24.899104]  [<ffffffff81100e9d>] ? trace_hardirqs_on+0xd/0x10
[   24.904976]  [<ffffffff810b0a20>] ? __local_bh_enable_ip+0x70/0xc0
[   24.911217]  [<ffffffff8177dcc8>] inet_stream_connect+0x38/0x50
[   24.917259]  [<ffffffff816c8ec7>] SYSC_connect+0xb7/0xf0
[   24.922649]  [<ffffffff816c6357>] ? sock_alloc_file+0x97/0x110
[   24.928557]  [<ffffffff81100e09>] ? trace_hardirqs_on_caller+0x129/0x1b0
[   24.935309]  [<ffffffff8100301a>] ? trace_hardirqs_on_thunk+0x1a/0x1c
[   24.941819]  [<ffffffff816c9dfe>] SyS_connect+0xe/0x10
[   24.947050]  [<ffffffff8183ef3c>] entry_SYSCALL_64_fastpath+0x1f/0xbd

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Cong Wang]

On Wed, Oct 19, 2016 at 8:21 AM, Elad Raz <e@eladraz.com> wrote:
> On Fri, Sep 2, 2016 at 11:12 AM, Nicolas Dichtel
> <nicolas.dichtel@6wind.com> wrote:
>> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>>> We never read or change netns id in hardirq context,
>>> the only place we read netns id in softirq context
>>> is in vxlan_xmit(). So, it should be enough to just
>>> disable BH.
>>
>> Are you sure? Did you audit all part of the code?
>> peernet2id() is called from netlink core system (do_one_broadcast()). Are you
>> sure that no driver call this function from an hard irq context?
>>
>> I think that NETLINK_LISTEN_ALL_NSID is largely untested, so it will be hard to
>> detect a bug introduced in this feature.
>
> I'm seeing strange things on our systems on boot time when trying to
> mount autofs.
> I bisected and got this patch as the bad one.
> I can see that only when I'm using "debug" config file.

Yeah, I saw the same report from SELinux developers, I am working
on a fix.

Thanks.

RE: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Elluru, Krishna Mohan]

HI Cong,
	Have you posted any patch for the same? I am looking for single netlink socket to handle multiple network namespace events using NETLINK_LISTEN_ALL_NSID. I don't see detection of new namespaces by this socket yet and causing updates to be missed.


Thanks
Krishna Mohan.

-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Cong Wang
Sent: Thursday, October 20, 2016 1:37 AM
To: Elad Raz <e@eladraz.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>; David Miller <davem@davemloft.net>; Linux Netdev List <netdev@vger.kernel.org>; Jiri Pirko <jiri@resnulli.us>; Ido Schimmel <idosch@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>
Subject: Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

On Wed, Oct 19, 2016 at 8:21 AM, Elad Raz <e@eladraz.com> wrote:
> On Fri, Sep 2, 2016 at 11:12 AM, Nicolas Dichtel 
> <nicolas.dichtel@6wind.com> wrote:
>> Le 02/09/2016 à 06:53, Cong Wang a écrit :
>>> We never read or change netns id in hardirq context, the only place 
>>> we read netns id in softirq context is in vxlan_xmit(). So, it 
>>> should be enough to just disable BH.
>>
>> Are you sure? Did you audit all part of the code?
>> peernet2id() is called from netlink core system (do_one_broadcast()). 
>> Are you sure that no driver call this function from an hard irq context?
>>
>> I think that NETLINK_LISTEN_ALL_NSID is largely untested, so it will 
>> be hard to detect a bug introduced in this feature.
>
> I'm seeing strange things on our systems on boot time when trying to 
> mount autofs.
> I bisected and got this patch as the bad one.
> I can see that only when I'm using "debug" config file.

Yeah, I saw the same report from SELinux developers, I am working on a fix.

Thanks.

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Cong Wang]

On Mon, Jan 30, 2017 at 10:47 PM, Elluru, Krishna Mohan
<elluru.kri.mohan@hpe.com> wrote:
> HI Cong,
>         Have you posted any patch for the same? I am looking for single netlink socket to handle multiple network namespace events using NETLINK_LISTEN_ALL_NSID. I don't see detection of new namespaces by this socket yet and causing updates to be missed.

The patch in $subject was reposted by Paul after fixing the audit part:

commit fba143c66abb81307a450679f38ab953fe96a413
Author: Paul Moore <paul@paul-moore.com>
Date:   Tue Nov 29 16:57:48 2016 -0500

    netns: avoid disabling irq for netns id

Which version of kernel are you testing? Does it have this commit?

Thanks.

RE: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Elluru, Krishna Mohan]

I am of 4.4.18. I see the commit is not present. I will recompile my kernel with below patch.

Thanks for the quick sha reference.

Thanks
Krishna Mohan.

-----Original Message-----
From: Cong Wang [mailto:xiyou.wangcong@gmail.com] 
Sent: Tuesday, January 31, 2017 12:23 PM
To: Elluru, Krishna Mohan <elluru.kri.mohan@hpe.com>
Cc: Elad Raz <e@eladraz.com>; Nicolas Dichtel <nicolas.dichtel@6wind.com>; David Miller <davem@davemloft.net>; Linux Netdev List <netdev@vger.kernel.org>; Jiri Pirko <jiri@resnulli.us>; Ido Schimmel <idosch@mellanox.com>; Yotam Gigi <yotamg@mellanox.com>
Subject: Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

On Mon, Jan 30, 2017 at 10:47 PM, Elluru, Krishna Mohan <elluru.kri.mohan@hpe.com> wrote:
> HI Cong,
>         Have you posted any patch for the same? I am looking for single netlink socket to handle multiple network namespace events using NETLINK_LISTEN_ALL_NSID. I don't see detection of new namespaces by this socket yet and causing updates to be missed.

The patch in $subject was reposted by Paul after fixing the audit part:

commit fba143c66abb81307a450679f38ab953fe96a413
Author: Paul Moore <paul@paul-moore.com>
Date:   Tue Nov 29 16:57:48 2016 -0500

    netns: avoid disabling irq for netns id

Which version of kernel are you testing? Does it have this commit?

Thanks.

Re: [Patch net-next 2/2] netns: avoid disabling irq for netns id

[Nicolas Dichtel]

Le 31/01/2017 à 07:47, Elluru, Krishna Mohan a écrit :
> HI Cong,
> 	Have you posted any patch for the same? I am looking for single netlink socket to handle multiple network namespace events using NETLINK_LISTEN_ALL_NSID. I don't see detection of new namespaces by this socket yet and causing updates to be missed.
New namespaces are not automatically detected. You have to assign them a nsid
(e.g. ip netns set).
NETLINK_LISTEN_ALL_NSID enables notifications only for peer netns that have a
nsid assign in the netns where the socket has been opened.


Regards,
Nicolas