Hi Emeric, > Please find attached some of the patches we have applied. > The goal is mainly to ease integration with our build environment
Please find my feedback and questions on the patches below. > patch-0000-ha-flush-segment -> Add a command to flush all the SA of a HA > segment What exactly is the use case here? > patch-0001-alert-initial-contact -> Add an alert to listen to INITIAL CONTACT > events Same question, what is the use case for this? And can't you just use the message hook to check for INITIAL_CONTACT notifies? I'm mainly concerned with polluting the alert space with new events that are rarely used but increase traffic on the bus. > patch-0002-libhydra-flush-only-managed-sas -> do not flush everything but > only managed SA types (AH, ESP, IPCOMP) -> Leave intact SA inserted by other > daemons like bird. Makes sense, but should probably also be done for the kernel-netlink plugin. One problem is that we can't control which policies to flush. Luckily with the changes I merged to master this morning, already existing policies are not treated as errors anymore, so we can probably get rid of the flush_policies() call in starter (if installpolicies=no is used it wouldn't make sense to flush them anyway - I think the main reason to flush them was to avoid later conflicts). I wonder if we could also remove the flush_sas() call (but not all SAs actually have a lifetime set, in particular IPComp SAs, but neither have policies for that matter). Anyway, if this is required something went wrong anyway. I pushed some changes to the kernel-flush branch. > patch-0003-ha-fifo-sanity-checks -> Add some checks on the HA ctl file I took some of the ideas and pushed them to the ha-ctl-checks branch. > patch-0004-ha-sync-delay -> Already in a branch on github. The goal is to > request a sync only once the configuration has been completely reloaded. Can't say I like this. It is specific to stroke, adds additional alerts, won't work if multiple config backends are loaded etc. Perhaps Martin can chime in and comment on why he created the branch. > patch-0005-disable-backtrace-use -> Condition "backtrace" since we do not > have backtrace available on our systems As far as I can tell, if backtrace() isn't available and no alternatives are used (like libunwind) backtrace_t.log() is basically a noop that will just log "no support for capturing backtraces". Is that a problem? > patch-0006-fix-compile-warnings -> Fix some of the warnings we got on our > toolchain Applied most of them to master. I don't get the cast in openssl_asn1_obj2chunk(), though. Why is that triple cast needed to simply cast the const qualifier away (the `data` member seems to be a const u_char*)? I haven't applied the cas_ptr() casts, as these are only required when no atomic built-ins are available (we could probably define the fallback with void* as first argument to avoid the warnings, however, in the apidoc void** clarifies that a pointer to a pointer has to be passed). > patch-0007-kernel_pfroute-virtual-ip-option -> implement the > install_virtual_ip option for the kernel_pfroute plugin Applied to master. Regards, Tobias _______________________________________________ Dev mailing list [email protected] https://lists.strongswan.org/mailman/listinfo/dev
