On 10/31/24 17:29, Ilya Maximets wrote:
> On 10/31/24 17:18, Eelco Chaudron wrote:
>>
>>
>> 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.
>
> Ack. Will change.
Hrm. Unfortunately, we can't actually do that, because the
default value object is created at the moment of function
definition, so the value is always the value it was when the
script is loaded, i.e. it will be None.
I'll keep this part as is for now in this and the previous
patches.
>
>>
>>>>
>>>>> 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
