lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <953f4a8c-1b17-cf22-9cbf-151ba4d39656@gmail.com> Date: Fri, 8 Jul 2022 01:14:32 +0300 From: Tariq Toukan <ttoukan.linux@...il.com> To: Jakub Kicinski <kuba@...nel.org>, Saeed Mahameed <saeed@...nel.org> Cc: "David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org, Tariq Toukan <tariqt@...dia.com>, Maxim Mikityanskiy <maximmi@...dia.com> Subject: Re: [net-next 11/15] net/tls: Multi-threaded calls to TX tls_dev_del On 7/7/2022 5:37 AM, Jakub Kicinski wrote: > On Wed, 6 Jul 2022 16:24:17 -0700 Saeed Mahameed wrote: >> diff --git a/include/net/tls.h b/include/net/tls.h >> index 4fc16ca5f469..c4be74635502 100644 >> --- a/include/net/tls.h >> +++ b/include/net/tls.h >> @@ -163,6 +163,11 @@ struct tls_record_info { >> skb_frag_t frags[MAX_SKB_FRAGS]; >> }; >> >> +struct destruct_work { >> + struct work_struct work; >> + struct tls_context *ctx; > > Pretty strange to bundle the back-pointer with the work. > Why not put it directly in struct tls_offload_context_tx? > I can put them directly under struct tls_offload_context_tx. No strong reason, I just followed the code reference in include/net/tls.h :: struct tx_work. I'll change it and respin. > Also now that we have the backpointer, can we move the list member of > struct tls_context to the offload context? (I haven't checked if its > used in other places) I'll check and move if possible. > >> >> up_write(&device_offload_lock); >> >> - flush_work(&tls_device_gc_work); >> - >> return NOTIFY_DONE; >> } >> >> @@ -1435,6 +1416,5 @@ void __init tls_device_init(void) >> void __exit tls_device_cleanup(void) >> { >> unregister_netdevice_notifier(&tls_dev_notifier); >> - flush_work(&tls_device_gc_work); >> clean_acked_data_flush(); >> } > > Why don't we need the flush any more? The module reference is gone as > soon as destructor runs (i.e. on ULP cleanup), the work can still be > pending, no? So this garbage collector work does not exist anymore. Replaced by per-context works, with no accessibility to them from this function. It seems that we need to guarantee completion of these works. Maybe by queuing them to a new dedicated queue, flushing it here.
Powered by blists - more mailing lists