Re: [linux-yocto][v5.15/standard/base][PATCH] netfilter: conntrack: avoid useless indirection during conntrack destruction

2022-04-20 Thread Bruce Ashfield
In message: [linux-yocto][v5.15/standard/base][PATCH] netfilter: conntrack: 
avoid useless indirection during conntrack destruction
on 18/04/2022 He Zhe wrote:

> From: Florian Westphal 
> 
> nf_ct_put() results in a usesless indirection:
> 
> nf_ct_put -> nf_conntrack_put -> nf_conntrack_destroy -> rcu readlock +
> indirect call of ct_hooks->destroy().
> 
> There are two _put helpers:
> nf_ct_put and nf_conntrack_put.  The latter is what should be used in
> code that MUST NOT cause a linker dependency on the conntrack module
> (e.g. calls from core network stack).
> 
> Everyone else should call nf_ct_put() instead.
> 
> A followup patch will convert a few nf_conntrack_put() calls to
> nf_ct_put(), in particular from modules that already have a conntrack
> dependency such as act_ct or even nf_conntrack itself.
> 
> Signed-off-by: Florian Westphal 
> Signed-off-by: Pablo Neira Ayuso 
> 
> Backport
> 6ae7989c9af0 ("netfilter: conntrack: avoid useless indirection during")
> which is not going to stable tree from mainline to fix the following warning,
> with some necessary context tweaks.

Thanks for the extra explanation about the -stable status of
the patch.

It makes the decision on merging much easier.

This is now merged to v5.15 and I'll send SRCREV updates after
my next round of -stable updates.

Bruce

> 
> root@intel-x86-64:~# ovs-vsctl add-br br0
> device ovs-system entered promiscuous mode
> [ cut here ]
> WARNING: CPU: 2 PID: 711 at include/net/netfilter/nf_conntrack.h:175 
> __ovs_ct_lookup+0x819/0x920 [openvswitch]
> CPU: 2 PID: 711 Comm: ovs-vswitchd Not tainted 5.15.33-rt34-yocto-preempt-rt 
> #1
> Hardware name: Intel(R) Client Systems NUC7i5DNKE/NUC7i5DNB, BIOS 
> DNKBLi5v.86A.0064.2019.0523.1933 05/23/2019
> RIP: 0010:__ovs_ct_lookup+0x819/0x920 [openvswitch]
> Code: b8 fe ff ff ff e9 69 f9 ff ff 41 0f b7 84 24 b0 00 00 00 49 8b 94 24 c0 
> 00 00 00 0f b6 1c 02 83 e3 0f c1 e3 02 e9 c5 fc ff ff <0f> 0b e9 86 f8 ff ff 
> 4c 89 f7 44 89 95 40 ff ff ff e8 61 30 fc ff
> RSP: 0018:914c80a77638 EFLAGS: 00010246
> RAX:  RBX: 903b139fa528 RCX: 
> RDX: 0002 RSI: 914c80a77660 RDI: 
> RBP: 914c80a77710 R08: 903b04819128 R09: aa605c40
> R10: 914c80a779c0 R11:  R12: 903b05dd7000
> R13: 0001 R14: 903b0a603c00 R15: 903b04819120
> FS:  7f9256c9ba80() GS:903c65d0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 7ffe2fad9008 CR3: 00010a5e0002 CR4: 003706e0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> Call Trace:
>  
>  ? nf_ct_get_tuple+0x144/0x1f0 [nf_conntrack]
>  ? nf_ct_get_tuplepr+0x5f/0x90 [nf_conntrack]
>  ovs_ct_execute+0x3e1/0x5e0 [openvswitch]
>  do_execute_actions+0xed/0x1ab0 [openvswitch]
>  ? ovs_ct_copy_action+0x17b/0x8a0 [openvswitch]
>  ? __ovs_nla_copy_actions+0x884/0xf30 [openvswitch]
>  ? migrate_enable+0xd3/0x150
>  ovs_execute_actions+0x62/0x140 [openvswitch]
>  ? ovs_execute_actions+0x62/0x140 [openvswitch]
>  ovs_packet_cmd_execute+0x294/0x310 [openvswitch]
>  genl_family_rcv_msg_doit+0xe6/0x140
>  genl_rcv_msg+0xde/0x1d0
>  ? ovs_vport_cmd_del+0x200/0x200 [openvswitch]
>  ? genl_get_cmd+0xd0/0xd0
>  netlink_rcv_skb+0x55/0x100
>  genl_rcv+0x29/0x40
>  netlink_unicast+0x234/0x340
>  netlink_sendmsg+0x226/0x470
>  ? netlink_unicast+0x340/0x340
>  sys_sendmsg+0x273/0x2b0
>  ? sendmsg_copy_msghdr+0x7b/0xa0
>  ___sys_sendmsg+0x81/0xc0
>  ? do_futex+0x1c4/0xb70
>  ? rt_spin_unlock+0x18/0x40
>  ? __fget_light+0xa3/0x120
>  __sys_sendmsg+0x62/0xb0
>  ? fpregs_assert_state_consistent+0x27/0x50
>  __x64_sys_sendmsg+0x1d/0x20
>  do_syscall_64+0x43/0x90
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f9256dab82d
> Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 1a 97 f7 ff 8b 54 24 1c 48 
> 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 
> 33 44 89 c7 48 89 44 24 08 e8 5e 97 f7 ff 48
> RSP: 002b:7ffe2fae90a0 EFLAGS: 0293 ORIG_RAX: 002e
> RAX: ffda RBX: 0001 RCX: 7f9256dab82d
> RDX:  RSI: 7ffe2fae9130 RDI: 0011
> RBP: 7ffe2fae9f30 R08:  R09: 0001
> R10: 0006 R11: 0293 R12: 55fab0216e90
> R13:  R14: 55fab0216e90 R15: 7ffe2fae9130
>  
> ---[ end trace 0002 ]---
> 
> Signed-off-by: He Zhe 
> ---
>  include/linux/netfilter/nf_conntrack_common.

[linux-yocto][v5.15/standard/base][PATCH] netfilter: conntrack: avoid useless indirection during conntrack destruction

2022-04-18 Thread He Zhe
From: Florian Westphal 

nf_ct_put() results in a usesless indirection:

nf_ct_put -> nf_conntrack_put -> nf_conntrack_destroy -> rcu readlock +
indirect call of ct_hooks->destroy().

There are two _put helpers:
nf_ct_put and nf_conntrack_put.  The latter is what should be used in
code that MUST NOT cause a linker dependency on the conntrack module
(e.g. calls from core network stack).

Everyone else should call nf_ct_put() instead.

A followup patch will convert a few nf_conntrack_put() calls to
nf_ct_put(), in particular from modules that already have a conntrack
dependency such as act_ct or even nf_conntrack itself.

Signed-off-by: Florian Westphal 
Signed-off-by: Pablo Neira Ayuso 

Backport
6ae7989c9af0 ("netfilter: conntrack: avoid useless indirection during")
which is not going to stable tree from mainline to fix the following warning,
with some necessary context tweaks.

root@intel-x86-64:~# ovs-vsctl add-br br0
device ovs-system entered promiscuous mode
[ cut here ]
WARNING: CPU: 2 PID: 711 at include/net/netfilter/nf_conntrack.h:175 
__ovs_ct_lookup+0x819/0x920 [openvswitch]
CPU: 2 PID: 711 Comm: ovs-vswitchd Not tainted 5.15.33-rt34-yocto-preempt-rt #1
Hardware name: Intel(R) Client Systems NUC7i5DNKE/NUC7i5DNB, BIOS 
DNKBLi5v.86A.0064.2019.0523.1933 05/23/2019
RIP: 0010:__ovs_ct_lookup+0x819/0x920 [openvswitch]
Code: b8 fe ff ff ff e9 69 f9 ff ff 41 0f b7 84 24 b0 00 00 00 49 8b 94 24 c0 
00 00 00 0f b6 1c 02 83 e3 0f c1 e3 02 e9 c5 fc ff ff <0f> 0b e9 86 f8 ff ff 4c 
89 f7 44 89 95 40 ff ff ff e8 61 30 fc ff
RSP: 0018:914c80a77638 EFLAGS: 00010246
RAX:  RBX: 903b139fa528 RCX: 
RDX: 0002 RSI: 914c80a77660 RDI: 
RBP: 914c80a77710 R08: 903b04819128 R09: aa605c40
R10: 914c80a779c0 R11:  R12: 903b05dd7000
R13: 0001 R14: 903b0a603c00 R15: 903b04819120
FS:  7f9256c9ba80() GS:903c65d0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7ffe2fad9008 CR3: 00010a5e0002 CR4: 003706e0
DR0:  DR1:  DR2: 
DR3:  DR6: fffe0ff0 DR7: 0400
Call Trace:
 
 ? nf_ct_get_tuple+0x144/0x1f0 [nf_conntrack]
 ? nf_ct_get_tuplepr+0x5f/0x90 [nf_conntrack]
 ovs_ct_execute+0x3e1/0x5e0 [openvswitch]
 do_execute_actions+0xed/0x1ab0 [openvswitch]
 ? ovs_ct_copy_action+0x17b/0x8a0 [openvswitch]
 ? __ovs_nla_copy_actions+0x884/0xf30 [openvswitch]
 ? migrate_enable+0xd3/0x150
 ovs_execute_actions+0x62/0x140 [openvswitch]
 ? ovs_execute_actions+0x62/0x140 [openvswitch]
 ovs_packet_cmd_execute+0x294/0x310 [openvswitch]
 genl_family_rcv_msg_doit+0xe6/0x140
 genl_rcv_msg+0xde/0x1d0
 ? ovs_vport_cmd_del+0x200/0x200 [openvswitch]
 ? genl_get_cmd+0xd0/0xd0
 netlink_rcv_skb+0x55/0x100
 genl_rcv+0x29/0x40
 netlink_unicast+0x234/0x340
 netlink_sendmsg+0x226/0x470
 ? netlink_unicast+0x340/0x340
 sys_sendmsg+0x273/0x2b0
 ? sendmsg_copy_msghdr+0x7b/0xa0
 ___sys_sendmsg+0x81/0xc0
 ? do_futex+0x1c4/0xb70
 ? rt_spin_unlock+0x18/0x40
 ? __fget_light+0xa3/0x120
 __sys_sendmsg+0x62/0xb0
 ? fpregs_assert_state_consistent+0x27/0x50
 __x64_sys_sendmsg+0x1d/0x20
 do_syscall_64+0x43/0x90
 entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7f9256dab82d
Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 1a 97 f7 ff 8b 54 24 1c 48 
8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33 
44 89 c7 48 89 44 24 08 e8 5e 97 f7 ff 48
RSP: 002b:7ffe2fae90a0 EFLAGS: 0293 ORIG_RAX: 002e
RAX: ffda RBX: 0001 RCX: 7f9256dab82d
RDX:  RSI: 7ffe2fae9130 RDI: 0011
RBP: 7ffe2fae9f30 R08:  R09: 0001
R10: 0006 R11: 0293 R12: 55fab0216e90
R13:  R14: 55fab0216e90 R15: 7ffe2fae9130
 
---[ end trace 0002 ]---

Signed-off-by: He Zhe 
---
 include/linux/netfilter/nf_conntrack_common.h |  2 ++
 include/net/netfilter/nf_conntrack.h  |  8 ++--
 net/netfilter/nf_conntrack_core.c | 12 ++--
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/include/linux/netfilter/nf_conntrack_common.h 
b/include/linux/netfilter/nf_conntrack_common.h
index 700ea077ce2d..a0a587ffa021 100644
--- a/include/linux/netfilter/nf_conntrack_common.h
+++ b/include/linux/netfilter/nf_conntrack_common.h
@@ -29,6 +29,8 @@ struct nf_conntrack {
 };
 
 void nf_conntrack_destroy(struct nf_conntrack *nfct);
+
+/* like nf_ct_put, but without module dependency on nf_conntrack */
 static inline void nf_conntrack_put(struct nf_conntrack *nfct)
 {
if (nfct && atomic_dec_and_test(>use))
diff --git a/include/net/netfilter/nf_conntrack.h 
b/include/net/netfilter/nf_conntrack.h
index d24b0a34c8f0..8fc256af3df2 100644
--- a/include/net/netfilter/nf_conntrack.h
+++