Re: [PATCH net v2 1/3] macsec: introduce dedicated workqueue for SA crypto cleanup
From: alexjlzheng
Date: Fri May 08 2026 - 22:01:18 EST
From: Jinliang Zheng <alexjlzheng@xxxxxxxxxxx>
On Fri, 8 May 2026 15:42:43 -0700, Jakub Kicinski wrote:
> > +wq:
> > + destroy_workqueue(macsec_wq);
> > + return err;
>
> nit: err_destroy_wq: would be a better label name
>
> rcu_barrier() is needed before this
Thanks for the review.
Regarding the label name: "wq:" follows the existing naming convention
in macsec_init(), where the other error labels are also named after the
resource they clean up ("rtnl:", "notifier:") rather than using the
"err_xxx:" style. I kept it consistent with that local convention, but
happy to switch to "err_destroy_wq:" if you prefer uniformity with the
broader codebase.
Regarding rcu_barrier(): in the error path of macsec_init(), the
workqueue has just been created and no MACsec interface has been
registered yet. queue_rcu_work(macsec_wq, ...) is only reachable
from macsec_rxsa_put() and macsec_txsa_put(), which require an SA
object to exist. No SA can be created before macsec_init() succeeds,
so macsec_wq is guaranteed to be empty at this point and
rcu_barrier() is not needed here.
The rcu_barrier() in macsec_exit() is necessary for a different
reason: at that point live interfaces may have been destroyed and
their SAs may be in-flight through the rcu_work mechanism, so we
must wait for all rcu_work_rcufn callbacks to finish enqueuing their
work items before destroy_workqueue() drains them.
Does that reasoning make sense, or am I missing a path where work
could be queued onto macsec_wq during the init error unwind?
Thanks,
Jinliang