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 <[email protected]> > > 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 <[email protected]> > Signed-off-by: Pablo Neira Ayuso <[email protected]> > > 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:ffff914c80a77638 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: ffff903b139fa528 RCX: 0000000000000000 > RDX: 0000000000000002 RSI: ffff914c80a77660 RDI: 0000000000000000 > RBP: ffff914c80a77710 R08: ffff903b04819128 R09: ffffffffaa605c40 > R10: ffff914c80a779c0 R11: 0000000000000000 R12: ffff903b05dd7000 > R13: 0000000000000001 R14: ffff903b0a603c00 R15: ffff903b04819120 > FS: 00007f9256c9ba80(0000) GS:ffff903c65d00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007ffe2fad9008 CR3: 000000010a5e0002 CR4: 00000000003706e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > ? 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:00007ffe2fae90a0 EFLAGS: 00000293 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f9256dab82d > RDX: 0000000000000000 RSI: 00007ffe2fae9130 RDI: 0000000000000011 > RBP: 00007ffe2fae9f30 R08: 0000000000000000 R09: 0000000000000001 > R10: 0000000000000006 R11: 0000000000000293 R12: 000055fab0216e90 > R13: 0000000000000000 R14: 000055fab0216e90 R15: 00007ffe2fae9130 > </TASK> > ---[ end trace 0000000000000002 ]--- > > Signed-off-by: He Zhe <[email protected]> > --- > 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(&nfct->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 > +++ b/include/net/netfilter/nf_conntrack.h > @@ -76,6 +76,8 @@ struct nf_conn { > * Hint, SKB address this struct and refcnt via skb->_nfct and > * helpers nf_conntrack_get() and nf_conntrack_put(). > * Helper nf_ct_put() equals nf_conntrack_put() by dec refcnt, > + * except that the latter uses internal indirection and does not > + * result in a conntrack module dependency. > * beware nf_ct_get() is different and don't inc refcnt. > */ > struct nf_conntrack ct_general; > @@ -169,11 +171,13 @@ nf_ct_get(const struct sk_buff *skb, enum > ip_conntrack_info *ctinfo) > return (struct nf_conn *)(nfct & NFCT_PTRMASK); > } > > +void nf_ct_destroy(struct nf_conntrack *nfct); > + > /* decrement reference count on a conntrack */ > static inline void nf_ct_put(struct nf_conn *ct) > { > - WARN_ON(!ct); > - nf_conntrack_put(&ct->ct_general); > + if (ct && atomic_dec_and_test(&ct->ct_general.use)) > + nf_ct_destroy(&ct->ct_general); > } > > /* Protocol module loading */ > diff --git a/net/netfilter/nf_conntrack_core.c > b/net/netfilter/nf_conntrack_core.c > index 917e708a4561..b3c61705bdea 100644 > --- a/net/netfilter/nf_conntrack_core.c > +++ b/net/netfilter/nf_conntrack_core.c > @@ -558,7 +558,7 @@ static void > nf_ct_del_from_dying_or_unconfirmed_list(struct nf_conn *ct) > > #define NFCT_ALIGN(len) (((len) + NFCT_INFOMASK) & ~NFCT_INFOMASK) > > -/* Released via destroy_conntrack() */ > +/* Released via nf_ct_destroy() */ > struct nf_conn *nf_ct_tmpl_alloc(struct net *net, > const struct nf_conntrack_zone *zone, > gfp_t flags) > @@ -612,12 +612,11 @@ static void destroy_gre_conntrack(struct nf_conn *ct) > #endif > } > > -static void > -destroy_conntrack(struct nf_conntrack *nfct) > +void nf_ct_destroy(struct nf_conntrack *nfct) > { > struct nf_conn *ct = (struct nf_conn *)nfct; > > - pr_debug("destroy_conntrack(%p)\n", ct); > + pr_debug("%s(%p)\n", __func__, ct); > WARN_ON(atomic_read(&nfct->use) != 0); > > if (unlikely(nf_ct_is_template(ct))) { > @@ -643,9 +642,10 @@ destroy_conntrack(struct nf_conntrack *nfct) > if (ct->master) > nf_ct_put(ct->master); > > - pr_debug("destroy_conntrack: returning ct=%p to slab\n", ct); > + pr_debug("%s: returning ct=%p to slab\n", __func__, ct); > nf_conntrack_free(ct); > } > +EXPORT_SYMBOL(nf_ct_destroy); > > static void nf_ct_delete_from_lists(struct nf_conn *ct) > { > @@ -2774,7 +2774,7 @@ int nf_conntrack_init_start(void) > > static struct nf_ct_hook nf_conntrack_hook = { > .update = nf_conntrack_update, > - .destroy = destroy_conntrack, > + .destroy = nf_ct_destroy, > .get_tuple_skb = nf_conntrack_get_tuple_skb, > }; > > -- > 2.32.0 >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#11211): https://lists.yoctoproject.org/g/linux-yocto/message/11211 Mute This Topic: https://lists.yoctoproject.org/mt/90535883/21656 Group Owner: [email protected] Unsubscribe: https://lists.yoctoproject.org/g/linux-yocto/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
