On Tue, 21 Jun 2022, Antony Antony wrote:

Hi Paul,
Here is a new iteration sa-expire branch. I cherry picked changes from
https://github.com/paulwouters/libreswan/tree/sa-expire-2022-01-06

and rebased to origin/main.

I have created a PR to make it easy to review my branch.
https://github.com/libreswan/libreswan/pull/777

Thanks. I'm reviewing it now.

I ignored "<unset>" change.
I am not in favor of "<unset>" :  for 2^64 or the default. Currently it look
ipsec_max_bytes: 16EiB
ipsec_max_packets: 16Ei

Also there are ciphers which only allow 2^32 bytes and packets as default.
So it is better to print the default value in abbreviated form than unset
based on values.  Also another concern is if a user actually set to 16Ei or
16EiB in the config, your proposal will show that as "unset"?
We don't print unset when using other defaults! So it feels odd to me.
I undderstand 18446744073709551615 is very confusing, and I feel 16Ei and
16EiB is better. Would that work for you?

I prefer <unset> but it is fine. Anything but 18446744073709551615 is in
improvement but I do still think people won't know what 16EiB is.

I am presently surprised at your proposal to rename salifetime ->
ipsec-max-time. I think that is greate, and good for consistency. However,
lot of changes to keep track of on seperate branch before merge, ie.
variable names output. changes whack command ..
So I propose we change the those right after merge of sa-expire-2022*. i
As an atomic operation change config option, whack command and test output.

This is unfortunate. You could have fixed or told me the issues to fix.
It basically doubles my work because I have to do those from scratch
again after the merge.

Paul
_______________________________________________
Swan-dev mailing list
Swan-dev@lists.libreswan.org
https://lists.libreswan.org/mailman/listinfo/swan-dev

Reply via email to