On 8 Mar 2022, at 15:23, Antonio Quartulli wrote: > On 24/02/2022 17:55, Kristof Provost via Openvpn-devel wrote: >> I've had to add a lot of '|| defined(TARGET_FREEBSD)', and I think the >> code could be a bit cleaner if we'd make these calls conditional only on >> defined(ENABLE_DCO), and instead expect every DCO implementation to >> provide them, if only as stubs. I've not done that here. > > Can you name what new internal APIs would you want to add? > I am also in favor of less ifdefs and have APIs that can be defined as stub > or not based on the platform. > We’ve got a significant number of cases where we call DCO functions for Linux & FreeBSD but not for Windows. One example is the call to dco_event_state() in multi_tcp_wait() and io_wait_doword(). If windows provided an implementation for that, even if it did no work, we could replace ‘#if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)’ with an ‘#ifdef ENABLE_DCO’. If we provided a stub implementation for the DCO functions even for non-DCO builds we wouldn’t need any ‘#ifdef’ at all.
> A comment for your kernel API: > I have seen you created an API named "OVPN_SET_TIMEOUT", however I'd suggest > to name it "OVPN_SET_PEER" (like on Linux and Windows), because the idea is > that this call will be used to set any additional/optional per-peer param. > > For example, we have been recently working on mssfix, which we may want to > set on a per-peer basis (more params may come later). > > The SET_PEER call will allow to set all these params (if specified). > > What do you think? > I think I renamed it because I found the ‘SET_PEER’ name to be easy to confuse with ‘NEW_PEER’, but I’m happy to align to other platforms. Best regards, Kristof _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
