On 31 Oct 2024, at 16:07, Ilya Maximets wrote:

> On 10/31/24 12:03, Eelco Chaudron wrote:
>>
>>
>> On 30 Oct 2024, at 14:50, Ilya Maximets wrote:
>>
>>> Add a new command line option --command-timeout that controls the
>>> command timeout.  It is important to have this configurable, because
>>> the retransmit-timeout is configurable in Libreswan.  Also, users
>>> may prefer the monitor to be more responsive.
>>>
>>> ovs-monitor-ipsec options are not documented anywhere, so not
>>> trying to address that here.
>>>
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>
>> See comment below.
>>
>>> ---
>>>  ipsec/ovs-monitor-ipsec.in | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in
>>> index dba855af5..bf7b9c6ad 100755
>>> --- a/ipsec/ovs-monitor-ipsec.in
>>> +++ b/ipsec/ovs-monitor-ipsec.in
>>> @@ -83,6 +83,7 @@ vlog = ovs.vlog.Vlog("ovs-monitor-ipsec")
>>>  exiting = False
>>>  monitor = None
>>>  xfrm = None
>>> +command_timeout = None
>>>  RECONCILIATION_INTERVAL = 15  # seconds
>>>  TIEMOUT_EXPIRED = 37
>>>
>>> @@ -98,7 +99,7 @@ def run_command(args, description=None):
>>>      proc = subprocess.Popen(args, stdout=subprocess.PIPE,
>>>                              stderr=subprocess.PIPE)
>>>      try:
>>> -        pout, perr = proc.communicate(timeout=120)
>>> +        pout, perr = proc.communicate(timeout=command_timeout)
>>
>> Using a global variable inside a function looks wrong :)
>> Probably still wrong, but moving it in the definition might look better 
>> ¯\_(ツ)_/¯
>>
>>   def run_command(args, description=None, timeout=command_timeout):
>
>
> Yeah, there is no much difference, IMO.  But I can change that.

It both looks ugly, but at least it’s a bit more clear from a function entry 
point, rather than in function.

>>
>>>          ret = proc.returncode
>>>      except subprocess.TimeoutExpired:
>>>          vlog.warn("Command timed out trying to %s." % description)
>>> @@ -1387,6 +1388,10 @@ def main():
>>>      parser.add_argument("--ipsec-ctl", metavar="IPSEC-CTL",
>>>                          help="Use DIR/IPSEC-CTL as location for "
>>>                          " pluto ctl socket (libreswan only).")
>>> +    parser.add_argument("--command-timeout", metavar="TIMEOUT",
>>> +                        type=int, default=120,
>>> +                        help="Timeout for external commands called by the "
>>> +                        "ovs-monitor-ipsec daemon, e.g. ipsec --start.")
>>>
>>>      ovs.vlog.add_args(parser)
>>>      ovs.daemon.add_args(parser)
>>> @@ -1396,11 +1401,13 @@ def main():
>>>
>>>      global monitor
>>>      global xfrm
>>> +    global command_timeout
>>>
>>>      root_prefix = args.root_prefix if args.root_prefix else ""
>>>      xfrm = XFRM(root_prefix)
>>>      monitor = IPsecMonitor(root_prefix, args.ike_daemon,
>>>                             not args.no_restart_ike_daemon, args)
>>> +    command_timeout = args.command_timeout
>>>
>>>      remote = args.database
>>>      schema_helper = ovs.db.idl.SchemaHelper()
>>> -- 
>>> 2.46.0
>>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to