[Swan-dev] WIP: supporting xfrm SA expire

[Antony Antony]

On Fri, Jun 24, 2022 at 02:22:14PM -0400, Paul Wouters wrote:
> 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.

we are making progress. Good review. Let me know I how to access it as 
branch. I commented on the review message on github.

> > 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 vote for 16EiB, May be we could add entry to the man page.

> > 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.

Your work is here and it is not lost!
Your commit
https://github.com/paulwouters/libreswan/commit/c4c36e3e1dd92fd30a1267fb511

I rebased commit to May 2022,
https://github.com/antonyantony/libreswan/commit/e482cac16a3ce4f8309d86750585fdb5207975c7 

After I rebased, to May 2022 commit, I realized your commit is partial. Many 
internal variables are not renamed. Renmaing more variables is huge work.
Sorry I don't have time for that now!
If I do that, the whole sa-exoire branch would become complex to rebase and 
merge into main. Then I risk not able merge sa-expire to main any time soon.
I felt it is a huge moving target and demand too much time from me.

e.g
there were still
IKE_SA_LIFETIME_DEFAULT
sa_ipsec_life_seconds
sa_ike_life_seconds

or look at:
git reset --hard  c4c36e3e1dd92fd30a1267fb511
git grep -i sa_life
git grep -i life

Renaming ike-lifetime to ike-max-time, and ipsec-lifetime to ipsec-max-time 
would be a big code change! It should hapen atomiclly, directly in main and  
not a seperate branch for long like sa-expire.

-antony