RE: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors
-Original Message- From: ext Du, Fan [mailto:fan...@intel.com] Sent: Tuesday, July 14, 2015 9:53 AM To: Giuseppe Cantavenera; netdev@vger.kernel.org Cc: Steffen Klassert; David S. Miller; Sverdlin, Alexander (Nokia - DE/Ulm); Glavinic-Pecotic, Matija (EXT-Other - DE/Ulm); Cantavenera, Giuseppe (EXT-Other - DE/Ulm); Faustini, Nicholas (EXT-Other - DE/Ulm); Du, Fan Subject: RE: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors -Original Message- From: Giuseppe Cantavenera [mailto:giuseppe.cantaven...@azcom.it] Sent: Tuesday, July 7, 2015 3:43 PM To: netdev@vger.kernel.org Cc: Giuseppe Cantavenera; Steffen Klassert; David S. Miller; Du, Fan; Alexander Sverdlin; Matija Glavinic Pecotic; Giuseppe Cantavenera; Nicholas Faustini Subject: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors Hello, we also meet the same bug Fan Du did a while ago. Two solutions were proposed in the past: either forcibly mark as expired all of the keys every time the clock is set, or replace the existing timers with relative ones. The former would introduce unexpected behaviour (the keys would keep expiring when they shouldn't) and does not address the real problem: THE COUPLING between the SA scheduling and the wall timer. Actually it introduces even more of that. The latter is robust, extremly lightweight and maintanable, and preserves the expected behaviour, that's why we preferred it. Any feedback or any other idea is greatly appreciated. Thanks for keep working this issue as I did 2 years ago. Objection against the original approach from the maintainers is that it complicates the logic to the degree which involving extra maintenance effort, that is the effort it's not worthwhile against the trouble it might introduce in the future. Another approach you can try is using monotonic boot time(counting in suspend time also) to mark the life time of SA, then the timer handler logic will be quite easier and smaller than now, sure it will be robust naturally. The cost is that SA lifetime displaying by setkey and SA migration has to be taken care of as SA life time is boot time now, not the wall time. Hi Fan Du, Thanks for your feedback. It can probably work that way. I think we can wait if someone else has any further comments, then maybe try to see how it works. Thanks, Giuseppe -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors
-Original Message- From: Giuseppe Cantavenera [mailto:giuseppe.cantaven...@azcom.it] Sent: Tuesday, July 7, 2015 3:43 PM To: netdev@vger.kernel.org Cc: Giuseppe Cantavenera; Steffen Klassert; David S. Miller; Du, Fan; Alexander Sverdlin; Matija Glavinic Pecotic; Giuseppe Cantavenera; Nicholas Faustini Subject: [RFC net-next] xfrm: refactory to avoid state tasklet scheduling errors The SA state is managed by a tasklet scheduled relying on the wall clock. Previous changes have already tried to address bugs when the system time is changed but some error conditions still exist, because the logic is still coupled with the wall time. If the time is changed in between the SA is created and the tasklet timer is started for the first time, the SA scheduling will be broken: either the SA will expire and never be recreated, or it will expire at an unexpected time. The reason is that x-curlft.add_time will not be valid when the next variable is computed for the very first time in xfrm_timer_handler(). Fix this behaviour by avoiding to rely on the system time. Stick to relative time intervals and realise a total decoupling from the wall time. Based on another patch written and published by Fan Du (fan...@intel.com) in 2013 but never merged: part of the code preserved, some rewritten and improved. Changes to the logic accounting for the use_time expiration. Here we allow both add_time and use_time expirations to be set. Cc: Steffen Klassert steffen.klass...@secunet.com Cc: David S. Miller da...@davemloft.net Cc: Fan Du fan...@intel.com Cc: Alexander Sverdlin alexander.sverd...@nokia.com Cc: Matija Glavinic Pecotic matija.glavinic-pecotic@nokia.com Signed-off-by: Giuseppe Cantavenera giuseppe.cantavenera@nokia.com Signed-off-by: Nicholas Faustini nicholas.faustini@nokia.com --- Hello, we also meet the same bug Fan Du did a while ago. Two solutions were proposed in the past: either forcibly mark as expired all of the keys every time the clock is set, or replace the existing timers with relative ones. The former would introduce unexpected behaviour (the keys would keep expiring when they shouldn't) and does not address the real problem: THE COUPLING between the SA scheduling and the wall timer. Actually it introduces even more of that. The latter is robust, extremly lightweight and maintanable, and preserves the expected behaviour, that's why we preferred it. Any feedback or any other idea is greatly appreciated. Thanks for keep working this issue as I did 2 years ago. Objection against the original approach from the maintainers is that it complicates the logic to the degree which involving extra maintenance effort, that is the effort it's not worthwhile against the trouble it might introduce in the future. Another approach you can try is using monotonic boot time(counting in suspend time also) to mark the life time of SA, then the timer handler logic will be quite easier and smaller than now, sure it will be robust naturally. The cost is that SA lifetime displaying by setkey and SA migration has to be taken care of as SA life time is boot time now, not the wall time. Thanks, Regards, Giuseppe -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
