* [PATCH net v3 v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
2026-05-09 3:33 [PATCH net v3 0/3] macsec: use rcu_work to fix crypto cleanup in softirq context alexjlzheng
@ 2026-05-09 3:33 ` alexjlzheng
2026-05-11 13:32 ` Sabrina Dubroca
2026-05-09 3:33 ` [PATCH net v3 v3 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq alexjlzheng
2026-05-09 3:33 ` [PATCH net v3 v3 3/3] macsec: use rcu_work to defer TX " alexjlzheng
2 siblings, 1 reply; 10+ messages in thread
From: alexjlzheng @ 2026-05-09 3:33 UTC (permalink / raw)
To: sd, andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu
Cc: netdev, linux-kernel, Jinliang Zheng
From: Jinliang Zheng <alexjlzheng@tencent.com>
Introduce a dedicated ordered workqueue, macsec_wq, which will be used
by subsequent patches to defer SA crypto cleanup (crypto_free_aead and
related teardown) out of softirq context.
Using a dedicated workqueue instead of system_wq allows macsec_exit()
to drain exactly the work items belonging to this module via
destroy_workqueue(), without interfering with unrelated work items on
system_wq or causing unexpected delays elsewhere.
rcu_barrier() in macsec_exit() ensures all in-flight rcu_work callbacks
have enqueued their work items before destroy_workqueue() drains and
destroys the queue, making the two-step teardown correct and complete.
The same sequence is kept in the error path of macsec_init() as a
precaution, to mirror macsec_exit() and stay safe if work ever becomes
queueable before this point in the future.
While at it, rename the error labels in macsec_init() from the
resource-named style (rtnl:, notifier:, wq:) to the err_xxx: style
(err_rtnl:, err_notifier:, err_destroy_wq:) to align with the broader
kernel convention.
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
drivers/net/macsec.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index f6cad0746a02..52d6dd03ac92 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -26,6 +26,8 @@
#include <uapi/linux/if_macsec.h>
+static struct workqueue_struct *macsec_wq;
+
/* SecTAG length = macsec_eth_header without the optional SCI */
#define MACSEC_TAG_LEN 6
@@ -4450,25 +4452,35 @@ static int __init macsec_init(void)
{
int err;
+ macsec_wq = alloc_ordered_workqueue("macsec", 0);
+ if (!macsec_wq)
+ return -ENOMEM;
+
pr_info("MACsec IEEE 802.1AE\n");
err = register_netdevice_notifier(&macsec_notifier);
if (err)
- return err;
+ goto err_destroy_wq;
err = rtnl_link_register(&macsec_link_ops);
if (err)
- goto notifier;
+ goto err_notifier;
err = genl_register_family(&macsec_fam);
if (err)
- goto rtnl;
+ goto err_rtnl;
return 0;
-rtnl:
+err_rtnl:
rtnl_link_unregister(&macsec_link_ops);
-notifier:
+err_notifier:
unregister_netdevice_notifier(&macsec_notifier);
+err_destroy_wq:
+ /* Precautionary, mirrors macsec_exit() to stay safe if work
+ * ever becomes queueable before this point in the future.
+ */
+ rcu_barrier();
+ destroy_workqueue(macsec_wq);
return err;
}
@@ -4478,6 +4490,7 @@ static void __exit macsec_exit(void)
rtnl_link_unregister(&macsec_link_ops);
unregister_netdevice_notifier(&macsec_notifier);
rcu_barrier();
+ destroy_workqueue(macsec_wq);
}
module_init(macsec_init);
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net v3 v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
2026-05-09 3:33 ` [PATCH net v3 v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup alexjlzheng
@ 2026-05-11 13:32 ` Sabrina Dubroca
2026-05-11 14:00 ` [PATCH net " Jinliang Zheng
0 siblings, 1 reply; 10+ messages in thread
From: Sabrina Dubroca @ 2026-05-11 13:32 UTC (permalink / raw)
To: alexjlzheng
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu, netdev, linux-kernel,
Jinliang Zheng
Thanks for the updated patch. A few minor comments:
2026-05-09, 11:33:45 +0800, alexjlzheng@gmail.com wrote:
> @@ -4450,25 +4452,35 @@ static int __init macsec_init(void)
> {
> int err;
>
> + macsec_wq = alloc_ordered_workqueue("macsec", 0);
> + if (!macsec_wq)
> + return -ENOMEM;
Why did you pick an ordered workqueue instead of a normal one?
[...]
> -rtnl:
> +err_rtnl:
> rtnl_link_unregister(&macsec_link_ops);
> -notifier:
> +err_notifier:
> unregister_netdevice_notifier(&macsec_notifier);
> +err_destroy_wq:
> + /* Precautionary, mirrors macsec_exit() to stay safe if work
> + * ever becomes queueable before this point in the future.
> + */
I don't think this comment is really necessary, but ok.
(I missed the discussion on v2, it's not possible to create SAs at
this point, but it's reasonable to add the rcu_barrier here)
> + rcu_barrier();
> + destroy_workqueue(macsec_wq);
> return err;
> }
--
Sabrina
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
2026-05-11 13:32 ` Sabrina Dubroca
@ 2026-05-11 14:00 ` Jinliang Zheng
2026-05-11 14:37 ` Sabrina Dubroca
0 siblings, 1 reply; 10+ messages in thread
From: Jinliang Zheng @ 2026-05-11 14:00 UTC (permalink / raw)
To: sd
Cc: albinwyang, alexjlzheng, alexjlzheng, andrew+netdev, davem,
edumazet, hannes, horms, kuba, kuniyu, linux-kernel, netdev,
pabeni, shenyangyang4
On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > + macsec_wq = alloc_ordered_workqueue("macsec", 0);
>
> Why did you pick an ordered workqueue instead of a normal one?
I chose an ordered workqueue for its serialized execution, which
avoids potential ordering issues between concurrent SA destructions.
In hindsight, there is no actual ordering dependency between SA
cleanups, so a normal workqueue is sufficient here.
Should I send a v4 with that switched to alloc_workqueue()?
Thanks,
Jinliang
> > +err_destroy_wq:
> > + /* Precautionary, mirrors macsec_exit() to stay safe if work
> > + * ever becomes queueable before this point in the future.
> > + */
>
> I don't think this comment is really necessary, but ok.
>
> (I missed the discussion on v2, it's not possible to create SAs at
> this point, but it's reasonable to add the rcu_barrier here)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
2026-05-11 14:00 ` [PATCH net " Jinliang Zheng
@ 2026-05-11 14:37 ` Sabrina Dubroca
2026-05-11 14:49 ` Jinliang Zheng
0 siblings, 1 reply; 10+ messages in thread
From: Sabrina Dubroca @ 2026-05-11 14:37 UTC (permalink / raw)
To: Jinliang Zheng
Cc: albinwyang, alexjlzheng, andrew+netdev, davem, edumazet, hannes,
horms, kuba, kuniyu, linux-kernel, netdev, pabeni, shenyangyang4
2026-05-11, 22:00:05 +0800, Jinliang Zheng wrote:
> On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > > + macsec_wq = alloc_ordered_workqueue("macsec", 0);
> >
> > Why did you pick an ordered workqueue instead of a normal one?
>
> I chose an ordered workqueue for its serialized execution, which
> avoids potential ordering issues between concurrent SA destructions.
> In hindsight, there is no actual ordering dependency between SA
> cleanups, so a normal workqueue is sufficient here.
>
> Should I send a v4 with that switched to alloc_workqueue()?
Probably doesn't really matter, but since your cover letter mentions
"unnecessary latency", maybe? Would it make a difference in your use
case? I think it would mostly help when we try to unload the module
with lots of SAs, otherwise the only difference will be that actual
freeing of resources will be a bit slower. (OTOH it would avoid giving
the "wrong impression", that the ordered variant of the WQ is needed,
when it's not)
Either way, the patch looks ok to me, so for this version:
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
--
Sabrina
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH net v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
2026-05-11 14:37 ` Sabrina Dubroca
@ 2026-05-11 14:49 ` Jinliang Zheng
0 siblings, 0 replies; 10+ messages in thread
From: Jinliang Zheng @ 2026-05-11 14:49 UTC (permalink / raw)
To: sd
Cc: albinwyang, alexjlzheng, alexjlzheng, andrew+netdev, davem,
edumazet, hannes, horms, kuba, kuniyu, linux-kernel, netdev,
pabeni, shenyangyang4
On Sun, 11 May 2026, Sabrina Dubroca wrote:
> > In hindsight, there is no actual ordering dependency between SA
> > cleanups, so a normal workqueue is sufficient here. Should I send
> > a v4 with that switched to alloc_workqueue()?
>
> It probably doesn't matter a lot, but since your cover letter
> mentions "unnecessary latency", switching to a regular workqueue
> would help avoid giving the wrong impression that the ordered
> variant is needed.
>
> Either way, the patch looks ok to me
>
> Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Great, I'll switch to alloc_workqueue() in v4. Thanks a lot for
the review and the Reviewed-by!
Thanks,
Jinliang
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net v3 v3 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq
2026-05-09 3:33 [PATCH net v3 0/3] macsec: use rcu_work to fix crypto cleanup in softirq context alexjlzheng
2026-05-09 3:33 ` [PATCH net v3 v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup alexjlzheng
@ 2026-05-09 3:33 ` alexjlzheng
2026-05-11 13:50 ` Sabrina Dubroca
2026-05-09 3:33 ` [PATCH net v3 v3 3/3] macsec: use rcu_work to defer TX " alexjlzheng
2 siblings, 1 reply; 10+ messages in thread
From: alexjlzheng @ 2026-05-09 3:33 UTC (permalink / raw)
To: sd, andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu
Cc: netdev, linux-kernel, Jinliang Zheng
From: Jinliang Zheng <alexjlzheng@tencent.com>
crypto_free_aead() can internally invoke vunmap() (e.g. via
dma_free_attrs() in hardware crypto drivers such as hisi_sec2).
vunmap() must not be called from softirq context, but free_rxsa()
is an RCU callback that runs in softirq, leading to a kernel crash:
vunmap+0x4c/0x70
__iommu_dma_free+0xd0/0x138
dma_free_attrs+0xf4/0x100
sec_aead_exit+0x64/0xb8 [hisi_sec2]
crypto_destroy_tfm+0x98/0x110
free_rxsa+0x28/0x50 [macsec]
rcu_do_batch+0x184/0x460
rcu_core+0xf4/0x1f8
handle_softirqs+0x118/0x330
Use rcu_work to defer the cleanup to a workqueue. rcu_work dispatches
the worker asynchronously after the RCU grace period, so no thread
blocks waiting, and concurrent releases of multiple SAs naturally
share the same grace period.
Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
drivers/net/macsec.c | 8 +++++---
include/net/macsec.h | 4 +++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 52d6dd03ac92..67e4e3f439c7 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -176,9 +176,10 @@ static void macsec_rxsc_put(struct macsec_rx_sc *sc)
call_rcu(&sc->rcu_head, free_rx_sc_rcu);
}
-static void free_rxsa(struct rcu_head *head)
+static void free_rxsa_work(struct work_struct *work)
{
- struct macsec_rx_sa *sa = container_of(head, struct macsec_rx_sa, rcu);
+ struct macsec_rx_sa *sa =
+ container_of(to_rcu_work(work), struct macsec_rx_sa, destroy_work);
crypto_free_aead(sa->key.tfm);
free_percpu(sa->stats);
@@ -188,7 +189,7 @@ static void free_rxsa(struct rcu_head *head)
static void macsec_rxsa_put(struct macsec_rx_sa *sa)
{
if (refcount_dec_and_test(&sa->refcnt))
- call_rcu(&sa->rcu, free_rxsa);
+ queue_rcu_work(macsec_wq, &sa->destroy_work);
}
static struct macsec_tx_sa *macsec_txsa_get(struct macsec_tx_sa __rcu *ptr)
@@ -1409,6 +1410,7 @@ static int init_rx_sa(struct macsec_rx_sa *rx_sa, char *sak, int key_len,
rx_sa->next_pn = 1;
refcount_set(&rx_sa->refcnt, 1);
spin_lock_init(&rx_sa->lock);
+ INIT_RCU_WORK(&rx_sa->destroy_work, free_rxsa_work);
return 0;
}
diff --git a/include/net/macsec.h b/include/net/macsec.h
index bc7de5b53e54..0980ef36fbf0 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -9,6 +9,7 @@
#include <linux/u64_stats_sync.h>
#include <linux/if_vlan.h>
+#include <linux/workqueue.h>
#include <uapi/linux/if_link.h>
#include <uapi/linux/if_macsec.h>
@@ -123,6 +124,7 @@ struct macsec_dev_stats {
* @key: key structure
* @ssci: short secure channel identifier
* @stats: per-SA stats
+ * @destroy_work: deferred work to free the SA in process context after RCU grace period
*/
struct macsec_rx_sa {
struct macsec_key key;
@@ -136,7 +138,7 @@ struct macsec_rx_sa {
bool active;
struct macsec_rx_sa_stats __percpu *stats;
struct macsec_rx_sc *sc;
- struct rcu_head rcu;
+ struct rcu_work destroy_work;
};
struct pcpu_rx_sc_stats {
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net v3 v3 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq
2026-05-09 3:33 ` [PATCH net v3 v3 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq alexjlzheng
@ 2026-05-11 13:50 ` Sabrina Dubroca
0 siblings, 0 replies; 10+ messages in thread
From: Sabrina Dubroca @ 2026-05-11 13:50 UTC (permalink / raw)
To: alexjlzheng
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu, netdev, linux-kernel,
Jinliang Zheng
2026-05-09, 11:33:46 +0800, alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
>
> crypto_free_aead() can internally invoke vunmap() (e.g. via
> dma_free_attrs() in hardware crypto drivers such as hisi_sec2).
> vunmap() must not be called from softirq context, but free_rxsa()
> is an RCU callback that runs in softirq, leading to a kernel crash:
>
> vunmap+0x4c/0x70
> __iommu_dma_free+0xd0/0x138
> dma_free_attrs+0xf4/0x100
> sec_aead_exit+0x64/0xb8 [hisi_sec2]
> crypto_destroy_tfm+0x98/0x110
> free_rxsa+0x28/0x50 [macsec]
> rcu_do_batch+0x184/0x460
> rcu_core+0xf4/0x1f8
> handle_softirqs+0x118/0x330
>
> Use rcu_work to defer the cleanup to a workqueue. rcu_work dispatches
> the worker asynchronously after the RCU grace period, so no thread
> blocks waiting, and concurrent releases of multiple SAs naturally
> share the same grace period.
>
> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> ---
> drivers/net/macsec.c | 8 +++++---
> include/net/macsec.h | 4 +++-
> 2 files changed, 8 insertions(+), 4 deletions(-)
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
--
Sabrina
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net v3 v3 3/3] macsec: use rcu_work to defer TX SA crypto cleanup out of softirq
2026-05-09 3:33 [PATCH net v3 0/3] macsec: use rcu_work to fix crypto cleanup in softirq context alexjlzheng
2026-05-09 3:33 ` [PATCH net v3 v3 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup alexjlzheng
2026-05-09 3:33 ` [PATCH net v3 v3 2/3] macsec: use rcu_work to defer RX SA crypto cleanup out of softirq alexjlzheng
@ 2026-05-09 3:33 ` alexjlzheng
2026-05-11 13:50 ` Sabrina Dubroca
2 siblings, 1 reply; 10+ messages in thread
From: alexjlzheng @ 2026-05-09 3:33 UTC (permalink / raw)
To: sd, andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu
Cc: netdev, linux-kernel, Jinliang Zheng
From: Jinliang Zheng <alexjlzheng@tencent.com>
free_txsa() is an RCU callback running in softirq context, but calls
crypto_free_aead() which can invoke vunmap() internally on hardware
crypto drivers (e.g. hisi_sec2), triggering a kernel crash.
Use rcu_work to defer the cleanup to a workqueue, for the same reasons
as the analogous fix to free_rxsa() in the previous patch.
Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
drivers/net/macsec.c | 8 +++++---
include/net/macsec.h | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 67e4e3f439c7..e91d853c8d45 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -205,9 +205,10 @@ static struct macsec_tx_sa *macsec_txsa_get(struct macsec_tx_sa __rcu *ptr)
return sa;
}
-static void free_txsa(struct rcu_head *head)
+static void free_txsa_work(struct work_struct *work)
{
- struct macsec_tx_sa *sa = container_of(head, struct macsec_tx_sa, rcu);
+ struct macsec_tx_sa *sa =
+ container_of(to_rcu_work(work), struct macsec_tx_sa, destroy_work);
crypto_free_aead(sa->key.tfm);
free_percpu(sa->stats);
@@ -217,7 +218,7 @@ static void free_txsa(struct rcu_head *head)
static void macsec_txsa_put(struct macsec_tx_sa *sa)
{
if (refcount_dec_and_test(&sa->refcnt))
- call_rcu(&sa->rcu, free_txsa);
+ queue_rcu_work(macsec_wq, &sa->destroy_work);
}
static struct macsec_cb *macsec_skb_cb(struct sk_buff *skb)
@@ -1510,6 +1511,7 @@ static int init_tx_sa(struct macsec_tx_sa *tx_sa, char *sak, int key_len,
tx_sa->active = false;
refcount_set(&tx_sa->refcnt, 1);
spin_lock_init(&tx_sa->lock);
+ INIT_RCU_WORK(&tx_sa->destroy_work, free_txsa_work);
return 0;
}
diff --git a/include/net/macsec.h b/include/net/macsec.h
index 0980ef36fbf0..d962093ee923 100644
--- a/include/net/macsec.h
+++ b/include/net/macsec.h
@@ -176,6 +176,7 @@ struct macsec_rx_sc {
* @key: key structure
* @ssci: short secure channel identifier
* @stats: per-SA stats
+ * @destroy_work: deferred work to free the SA in process context after RCU grace period
*/
struct macsec_tx_sa {
struct macsec_key key;
@@ -188,7 +189,7 @@ struct macsec_tx_sa {
refcount_t refcnt;
bool active;
struct macsec_tx_sa_stats __percpu *stats;
- struct rcu_head rcu;
+ struct rcu_work destroy_work;
};
/**
--
2.39.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH net v3 v3 3/3] macsec: use rcu_work to defer TX SA crypto cleanup out of softirq
2026-05-09 3:33 ` [PATCH net v3 v3 3/3] macsec: use rcu_work to defer TX " alexjlzheng
@ 2026-05-11 13:50 ` Sabrina Dubroca
0 siblings, 0 replies; 10+ messages in thread
From: Sabrina Dubroca @ 2026-05-11 13:50 UTC (permalink / raw)
To: alexjlzheng
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, horms, hannes,
albinwyang, shenyangyang4, kuniyu, netdev, linux-kernel,
Jinliang Zheng
2026-05-09, 11:33:47 +0800, alexjlzheng@gmail.com wrote:
> From: Jinliang Zheng <alexjlzheng@tencent.com>
>
> free_txsa() is an RCU callback running in softirq context, but calls
> crypto_free_aead() which can invoke vunmap() internally on hardware
> crypto drivers (e.g. hisi_sec2), triggering a kernel crash.
>
> Use rcu_work to defer the cleanup to a workqueue, for the same reasons
> as the analogous fix to free_rxsa() in the previous patch.
>
> Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
> Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
> ---
> drivers/net/macsec.c | 8 +++++---
> include/net/macsec.h | 3 ++-
> 2 files changed, 7 insertions(+), 4 deletions(-)
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
--
Sabrina
^ permalink raw reply [flat|nested] 10+ messages in thread