Re: [PATCH net-next] selftests: net: exit cleanly on SIGTERM / timeout

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 26 Apr 2025 11:15:34 -0400 Willem de Bruijn wrote:
> > @@ -193,6 +198,19 @@ KSFT_DISRUPTIVE = True
> >      return env
> >  
> >  
> > +term_cnt = 0
> > +  
> 
> A bit ugly to initialize this here. Also, it already is initialized
> below.

We need a global so that the signal handler can access it.
Python doesn't have syntax to define a variable without a value.
Or do you suggest term_cnt = None ?

The whole term_cnt dance is super ugly, couldn't think of a cleaner way.
It's really annoying that ksft infra sends 2 terminating signals one
immediately after the other :|

> > +def _ksft_intr(signum, frame):
> > +    # ksft runner.sh sends 2 SIGTERMs in a row on a timeout
> > +    # if we don't ignore the second one it will stop us from handling cleanup
> > +    global term_cnt
> > +    term_cnt += 1
> > +    if term_cnt == 1:
> > +        raise KsftTerminate()
> > +    else:
> > +        ksft_pr(f"Ignoring SIGTERM (cnt: {term_cnt}), already exiting...")
> > +
> > +
> >  def ksft_run(cases=None, globs=None, case_pfx=None, args=()):
> >      cases = cases or []
> >  
> > @@ -205,6 +223,10 @@ KSFT_DISRUPTIVE = True
> >                      cases.append(value)
> >                      break
> >  
> > +    global term_cnt
> > +    term_cnt = 0
> > +    prev_sigterm = signal.signal(signal.SIGTERM, _ksft_intr)
> > +
> >      totals = {"pass": 0, "fail": 0, "skip": 0, "xfail": 0}
> >  
> >      print("TAP version 13")
> > @@ -229,11 +251,12 @@ KSFT_DISRUPTIVE = True
> >              cnt_key = 'xfail'
> >          except BaseException as e:
> >              stop |= isinstance(e, KeyboardInterrupt)
> > +            stop |= isinstance(e, KsftTerminate)
> >              tb = traceback.format_exc()
> >              for line in tb.strip().split('\n'):
> >                  ksft_pr("Exception|", line)
> >              if stop:
> > -                ksft_pr("Stopping tests due to KeyboardInterrupt.")
> > +                ksft_pr(f"Stopping tests due to {type(e).__name__}.")
> >              KSFT_RESULT = False
> >              cnt_key = 'fail'
> >  
> > @@ -248,6 +271,8 @@ KSFT_DISRUPTIVE = True
> >          if stop:
> >              break
> >  
> > +    signal.signal(signal.SIGTERM, prev_sigterm)
> > +  
> 
> Why is prev_sigterm saved and reassigned as handler here?

Because we ignore all signals when cnt > 2 I didn't want to keep our
handler installed. Just in case something after ksft_run() hangs.
It should be equivalent to

	signal.signal(signal.SIGTERM, signal.SIG_DLF)

if the prev is of concern. Then again keeping prev doesn't change #LOC