Re: [PATCH net] xfrm: Fix work re-schedule after cancel in xfrm_nat_keepalive_net_fini()

[Sabrina Dubroca <sd@queasysnail.net>]

2026-03-10, 17:14:19 -0700, Eyal Birger wrote:
> Hi,
> 
> On Tue, Mar 10, 2026 at 3:57 PM Sabrina Dubroca <sd@queasysnail.net> wrote:
> >
> > Please also CC the author, and maybe additional contributors, of the
> > patch that introduced the problem you're fixing.
> >
> > 2026-03-11, 03:16:29 +0900, Hyunwoo Kim wrote:
> > > After cancel_delayed_work_sync() is called from
> > > xfrm_nat_keepalive_net_fini(), xfrm_state_fini() flushes remaining
> > > states via __xfrm_state_delete(), which calls
> > > xfrm_nat_keepalive_state_updated() to re-schedule nat_keepalive_work.
> >
> > Eyal, I'm wondering why __xfrm_state_delete() calls
> > xfrm_nat_keepalive_state_updated(). At this point the state has been
> > removed from the walk list so nat_keepalive_work() won't do
> > anything. Am I missing something?
> 
> I don't remember for sure, but I think the idea was to have the work
> run "now" so that when deleting the last nat-keepalive state it
> won't run in the future, and in general to refresh the interval and
> not wait for the next iteration.
> 
> Eyal.

Ok. I thought about this, but I'm not seeing the benefit of doing
that. Assuming we're deleting just this one state, the next run will
process all the remaining states in the same way, whether it happens
right now or at the previously scheduled time:

 - if the next run was needed by the peer we're deleting, not much
   changes except that we're recomputing the delay earlier than
   otherwise (right now instead of when deleted_state's interval runs
   out)

 - if some other state was the first to need a keepalive, we do a run
   for nothing

So I think we could drop the xfrm_nat_keepalive_state_updated call
from __xfrm_state_delete.


@Hyunwoo here again I'm not opposed to s/cancel/disable/, it makes
sense to use disable_ in a "destruct" operation where we don't plan to
need the work again. But AFAICT this schedule_delayed_work isn't
really useful.

-- 
Sabrina