-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi,
Here's the summary of the previous IRC meeting. - --- COMMUNITY MEETING Place: #openvpn-devel on irc.freenode.net List-Post: openvpn-devel@lists.sourceforge.net Date: Monday 12th Jan 2015 Time: 20:00 CET (19:00 UTC) Planned meeting topics for this meeting were on this page: <https://community.openvpn.net/openvpn/wiki/Topics-2015-01-12> Next meeting has not been scheduled but will be on the same weekday and time. Your local meeting time is easy to check from services such as <http://www.timeanddate.com/worldclock> SUMMARY cron2, d12fk, mattock. plaisthos and syzzer participated in this meeting. - --- Discussed the "Make OpenVPN set routes on Windows Vista and later" patch: <http://sourceforge.net/p/openvpn-gui/openvpn/ci/8195efc0d8841b52217643a43d486cda2e171fea> The changes that affect non-Windows platforms look good, but some modifications were proposed and will be implemented. The actual worked code needs to be reviewed in another meeting, which is scheduled for next Monday. Merging this patch into OpenVPN 2.4 will break Windows XP service support. However, this is not considered a big loss for several reasons: - - Windows XP is end of life already - - Windows XP does not support NDIS6 either (=would be more work to maintain) - - OpenVPN 2.3.x will be available for the remaining Windows XP users - --- Full chatlog is attached. - -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlS0N1MACgkQwp2X7RmNIqMy0ACZAcUMOLEFRefFIBTh/LqnJsrK 7fwAn1foSUXpZav/uDnoqAJwh7cPzW/2 =qbL/ -----END PGP SIGNATURE-----
(21.00.58) mattock: meeting time (21.01.32) d12fk: yes (21.01.32) mattock: who do we have here today? (21.02.19) cron2: moh (21.02.22) mattock: hi! (21.03.41) syzzer: hi :) (21.04.00) d12fk: hey all (21.04.06) d12fk: here's the commit (21.04.10) d12fk: http://sourceforge.net/p/openvpn-gui/openvpn/ci/8195efc0d8841b52217643a43d486cda2e171fea/ (21.04.10) mattock: hi guys! (21.04.11) vpnHelper: Title: OpenVPN GUI / OpenVPN / Commit [8195ef] (at sourceforge.net) (21.04.56) syzzer: anything changed since you sent it to the ml? (21.05.02) d12fk: nope (21.06.26) cron2: is plaisthos here? (21.07.49) plaisthos: yes (21.08.50) cron2: I'm looking at "add_route" for android, and I see (21.08.53) cron2: management_android_control (management, "ROUTE", buf_bptr(&out)); (21.08.58) cron2: is "management" a global variable? (21.09.09) plaisthos: yes (21.09.32) cron2: so that's how you managed to do that without carrying an extra "options" variable... (21.09.44) plaisthos: :) (21.10.01) plaisthos: there are other instances in the code that use this global variable (21.10.10) plaisthos: iirc the proxy-query stuff does it too (21.12.00) cron2: d12fk: I'm not really happy with the "route_ipv6_clear_host_bits()" change - add_route_ipv6() should not overwrite data passed to it (21.13.05) cron2: (and I'm not really happy with all the extra "const struct options *o", but whether introducing a new global is *better*...? phew...) (21.14.59) syzzer: ^^ yes, that was partly my reaction too, see (1) from my 'preliminary review' mail (21.15.18) d12fk: i did not change the semantics of route_ipv6_clear_host_bits iirc (21.16.16) cron2: d12fk: well, you introduced that function :) (21.16.28) d12fk: o_O (21.16.29) cron2: my function was called "route_ipv6_print_network_bits()" (21.16.57) d12fk: all too long ago is seems, checking (21.17.01) cron2: but I see why you wanted that - I only had to deal with ASCII "netbits", not with a cleared binary representation (21.20.27) ***cron2 wonders (21.20.47) cron2: add_route() (etc.) carries a "flag" parameter today that is used to encode ROUTE_OPTION_FLAGS (options) (21.22.28) d12fk: ok checked and will only modify a copy (21.22.38) ***d12fk takes note (21.22.38) cron2: we cheat in tun.c and do not pass it on non-windows platforms (all others do not have it) (21.24.41) cron2: only bits 0+1 of "flags" are ever used (flags & ROUTE_METHOD_MASK). But then, overloading this with the message channel isn't *exactly* clean either... (21.25.59) cron2: uh... is_on_link() checks "flags & ROUTE_REF_GW" but I have the suspicion that this wants to be rgi->flags (21.26.10) cron2: argh (21.26.10) cron2: no (21.26.20) cron2: my head spins (21.26.52) cron2: add_bypass_routes() calls add_route3( ..., flags | ROUTE_REF_GW, ...) (21.29.20) plaisthos: :) (21.29.32) plaisthos: that bypass_route is really head spinning stuff (21.30.08) d12fk: wonder why i changed the function in the first place (21.30.50) cron2: d12fk: so you can get rid of it in delete_ipv6_route(), I'd say, because that now nicely has a clean binary structure :) (21.31.00) cron2: plaisthos: can you explain ROUTE_DELETE_FIRST to me? (21.34.43) ***syzzer is fighting virtualbox to get my windows vm started... (21.36.43) plaisthos: cron2: yes (21.36.44) plaisthos: it is a hack instead of def1 (21.36.56) plaisthos: you first delete the old default route and then add a new default route (21.36.58) plaisthos: for example (21.37.55) cron2: plaisthos: that is the theory, but the only place ROUTE_DELETE_FIRST is ever used is in add_routes()/delete_routes(), and if it is set, it is used for *all* routes - and as far as I can see, nobody ever *sets* it... (21.38.15) plaisthos: cron2: yeah (21.38.16) cron2: add_routes() only (21.38.21) plaisthos: probably old/dead code (21.38.48) cron2: lemme check something... (21.40.48) cron2: definitely not a merge accident, it's like that in james' svn... (21.41.25) harshar ha abbandonato la stanza (quit: Ping timeout: 252 seconds). (21.46.28) cron2: there is an amazing number of /* GLOBAL */ comments in our code (21.47.03) trispace [~trispace@unaffiliated/trispace] è entrato nella stanza. (21.47.37) ***cron2 throws "what about 'static HANDLE msg_pipe; /* GLOBAL */' in win32.c in the discussion (21.47.48) cron2: just trying the waters (21.48.14) d12fk: global vars considered lazy (21.48.30) plaisthos: (windows something, no idea ;)) (21.48.41) cron2: indeed, but "passing everything around everywhere" is not exactly pretty either (21.50.20) cron2: (especially as this is something only a single platform uses today. Now, OTOH, we might see more use of that on other platforms...) (21.53.38) syzzer: yeah, this is tricky... (21.57.24) syzzer: finally, VM booting (21.57.55) syzzer: my first thought was to put it in struct tuntap (21.58.56) plaisthos: I already put is_utun as member on osx only in there (21.59.01) cron2: hacky :-) but also less intrusive, and available everyhwere (21.59.41) cron2: otoh quite in line with what is there today... (21.59.44) cron2: struct tuntap_options { /* --ip-win32 options */ bool ip_win32_defined; (22.00.34) syzzer: yes, it is not very pretty either, but 'struct tuntap' is basically the network stuff context anyway (22.00.49) cron2: oh, this is so brilliant actually :-) - tuntap_options is fully inside #if defined(WIN32), and all other platforms have a MUCH simpler variant (22.01.51) cron2: (tun.h) (22.03.20) d12fk: || android ... (22.03.27) d12fk: strange (22.04.16) ***cron2 leaves that to plaisthos, but I think the underlying reason might be similar - lots of pushed dhcp-* etc. options work on android as well (22.04.46) cron2: ah, specifically, the pushed dhcp-option stuff, which is stored in there (22.04.46) plaisthos: d12fk: well, only windows parses the dhcp-options (22.04.54) cron2: #if defined(WIN32) || defined(TARGET_ANDROID) else if (streq (p[0], "dhcp-option") && p[1]) (22.05.09) plaisthos: all other platforms relay on script and env stuff and parse the options in the script (22.06.23) plaisthos: On Android I let OpenVPN parse that stuff and let OpenVPN decide on dns server (22.06.23) plaisthos: just like on Windows (22.07.16) d12fk: ah ok (22.08.39) plaisthos: windows and android are simmilar in the regard that some strange alien thing (tap driver/android gui) configure the network :) (22.09.17) d12fk: the HANDLE could be ifdef'd WIN32 in there again (22.09.58) d12fk: syzzer: where's that mail you were talking about (22.10.07) d12fk: must have missed it (22.12.00) syzzer: it's from 14 dec (22.13.45) syzzer: http://thread.gmane.org/gmane.network.openvpn.devel/9237 (22.13.48) vpnHelper: Title: Gmane Loom (at thread.gmane.org) (22.14.15) cron2: that mail was actually a bit cryptic IIRC :) (22.16.05) cron2: (and does not mention the tuntap idea) (22.16.23) syzzer: yeah, I tend to do that... but that one wasn't that bad, was it? (22.16.40) syzzer: no, I think I thought of that later on (22.17.11) ***cron2 is absolutely thrilled by that idea - it will perfectly well transport the handle *and* avoids global *and* is quite in line with what we currently have :-) (22.19.41) d12fk: ok i'll change the code accordingly (22.20.39) mattock: how far into the patch are we now? (22.21.02) cron2: mattock: we covered the bits that affect "common" code (22.21.10) mattock: ah, nice! (22.21.13) cron2: we didn't touch the actual windows specific bits at all yet :) (22.21.18) mattock: ok :) (22.21.35) mattock: just wondering if we should not even try to review all of it today, the patch is pretty huge (22.21.40) mattock: we = you :) (22.22.19) cron2: but that stuff is really quite modularized, so even if it were buggy (of course we assume its perfect) it will not affect non-windows builds or even windows without service handle (22.23.19) mattock: windows without service handle = ? (22.23.28) syzzer: yeah, I want to at least have it running, and figure out how the permission stuff is working, but otherwise I'd actually vote to just get it in and improve it along the way if needed (22.24.01) roentgen ha abbandonato la stanza (quit: Remote host closed the connection). (22.24.13) mattock: if we merge it into "master" then nobody will see it in practice until 2.4 release (22.24.32) mattock: and I can start building "master" snapshot for Windows, which will include the interactive service (22.25.02) syzzer: yes, that :) (22.25.10) mattock: meaning we get some amount of live testing before the actual 2.4 release (22.25.32) cron2: mattock: "classic windows - gui, no interactive service" (22.25.38) mattock: cron2: ok (22.25.50) mattock: how does one activate the interactive service? (22.26.00) cron2: hehe (22.26.05) pekster ha abbandonato la stanza (quit: Ping timeout: 265 seconds). (22.26.09) cron2: I was just typing "some installer work will be needed to cover the service" :) (22.26.23) mattock: achso (22.26.34) mattock: "some" = "tons"? :P (22.26.49) roentgen [~none@openvpn/community/support/roentgen] è entrato nella stanza. (22.26.49) modalità (+v roentgen) da ChanServ (22.26.50) mattock: some is no problem (22.28.06) d12fk: mattock: only thing is you have to start the interactive service instead of the classic one (22.28.16) d12fk: installation is done by the current command (22.28.27) d12fk: it the two services in one binary (22.28.50) d12fk: when the interactive one is started it is used by the gui (22.28.54) d12fk: if not, not (22.29.07) syzzer: but how do I start it? how do I know what to pass to --msg-channel? (22.29.12) mattock: is the currently available GUI patched for interactive service? (22.29.22) d12fk: mattock: yes (22.29.27) mattock: nice! (22.29.53) d12fk: syzzer: just start it and the gui will take care, the service itself will pass the respective option to openvpn (22.30.05) cron2: d12fk: in tun.c, do_address_service(), AF_INET - this confuses me a bit, why is netbits always 32? (22.30.24) syzzer: d12fk: ah, good. so I need to figure out how to build the gui then :p (22.30.41) syzzer: d12fk: could you maybe add --msg-channel to the man page, and the option descriptions in options.c ? (22.31.36) d12fk: cron2: because this one sets the adapter address (22.32.03) cron2: d12fk: how is the subnet mask set? (22.32.05) mattock: syzzer: you can build the gui with openvpn-build, if you have that setup (22.32.24) d12fk: syzzer: should be document it there? it's nothing users really use (22.32.37) ***cron2 can't find the call to do_address_service() for AF_INET anyway (22.34.28) d12fk: cron2: ah subnet mode probably didnt have that on my screen while writing the code (22.35.15) syzzer: d12fk: you do have a point there, but I think it should be documented somewhere (22.35.16) cron2: d12fk: I could argue that on windows, there is nothing but subnet mode :) (22.35.51) cron2: d12fk: seems the "set ipv4 address + netmask" bits are not there yet :-) - relying on DHCP always (22.36.12) d12fk: it's on my list now anyways (22.36.20) cron2: thanks (22.37.40) ***cron2 has not looked at the changes to the service yet, but the other changes to the openvpn src tree look good to me - so "ACK with the discussed changes" :-) (22.38.15) d12fk: i'll commit the canges to the repo, do you insist on one single commit while reviewing (22.38.22) mattock: so the consensus is: "common bits with proposed modifications -> ACK" (22.38.38) mattock: and also "merge into master and improve if bugs are found"? (22.38.55) d12fk: i think we should not merge into master yet (22.39.30) d12fk: if you need to build i can rebase to master until the feature is totally acked (22.39.48) d12fk: but then your clone breaks of course (22.39.48) mattock: ah, ok, so it does not even merge cleanly at this point (22.40.04) d12fk: it might (22.40.16) cron2: d12fk: for reviewing the impact on existing openvpn code, actually a single commit would be good (because otherwise we have one commit adding tons of *o and then another getting rid of them again) (22.40.21) d12fk: think i based it on a recent commit before our meeting (22.41.02) d12fk: well for that we can produce a single one when we're satisfied with the branch (22.41.06) pekster [~rewt@openvpn/community/support/pekster] è entrato nella stanza. (22.41.06) modalità (+o pekster) da ChanServ (22.41.06) syzzer: I did not had much issues when applying iirc (22.41.24) syzzer: I have a version based on master in my tree, which I could publish to github if that makes sense (22.41.25) cron2: chances are good that it will merge nicely as we didn't change anything in route.* or tun.* recently (22.43.01) d12fk: i think the way options are passed to the service could use some eyeballs (22.43.26) harshar [~harsh@202.3.77.215] è entrato nella stanza. (22.43.39) cron2: d12fk: we could use a primer on how this works in general, and what to look for :) (22.43.40) d12fk: cureently it's a \0 terminated list due to lack of other ideas (22.43.51) ***cron2 has NO ideas how windows services work (22.44.32) d12fk: that's not the interesting part really. once it runs it listens for commands on a named pipe (22.45.26) d12fk: so for reviewing the actual worker code knowledge of threads, asyc IO and IPC are needed rather (22.46.03) d12fk: so everything in interactive.c is rather service agnostic code (22.46.48) d12fk: do we support XP in 2.4? (22.47.02) cron2: not with the iservice (22.47.21) syzzer: d12fk: I think we settles for now, because it would lead to nasty code in the interactive service (22.47.23) cron2: XP users can run gui+openvpn.exe without service (22.47.30) syzzer: *settled for no (22.47.31) mattock: maybe we should get James to review the worker code? (22.47.58) mattock: next week or so maybe (22.48.46) ***cron2 has no time next week, but as I'm not a windows/thread/... expert, don't let that stop you? (22.50.02) syzzer: I have time next week, but my calender it quite full, so I might not find the time to look into it further before then (22.50.43) mattock: I'll ask James if he could attend next Monday (22.50.47) mattock: d12fk: what about you? (22.51.18) d12fk: problem is that the service will not even start if there are symbols linked that XP doesn't have (22.51.45) d12fk: mattock: should work out for me (22.51.52) mattock: d12fk: ok (22.51.56) syzzer: d12fk: but the service is not needed for the gui to work, right/ (22.52.07) cron2: d12fk: "we don't care". If someone insists on using service+xp, they can use 2.3 (22.52.10) d12fk: nope, but if the installer tries to start it... (22.52.17) mattock: cron2: +1 (22.52.32) d12fk: then the installer need to check if XP and not start/install the service there (22.52.33) cron2: (or use the other service alternative that was just proposed, which actually seems to work more robust anyway) (22.52.35) mattock: winxp has been obsolete for a while, and 2.3 will still work (22.52.42) syzzer: I guess the installer should just not install the service on XP (22.52.58) cron2: XP -> old tap driver, no service (22.53.03) d12fk: ok (22.53.05) mattock: another good point (22.53.06) cron2: Vista and up -> new tap driver, new service (22.53.15) d12fk: i'll remove the XP cruft then (22.53.20) mattock: let's forget about Windows XP for 2.4 (22.53.22) cron2: +1 (22.53.26) syzzer: ack (22.53.49) d12fk: in XP you don't need the servie anyway (22.53.53) cron2: it will not be a major loss anyway, as XP has no privilege separation anyway (22.53.57) cron2: what he said :) (22.54.05) d12fk: =) (22.54.18) d12fk: ok i'll drop out then (22.54.29) cron2: good night :) (22.54.30) cron2: (I (22.54.31) d12fk: cu next monday latest (22.54.41) cron2: I'll be sitting in a bus next monday, let's see how good their Internet works! (22.54.48) mattock: good night guys! (22.54.54) syzzer: good night! (22.54.55) d12fk: night (22.56.10) segoon: now http://thread.gmane.org/gmane.network.openvpn.devel/9392 i guess :) (22.56.11) vpnHelper: Title: Gmane Loom (at thread.gmane.org) (22.56.31) syzzer: yes, let's review that too :p (22.58.19) syzzer: segoon: you've been hiding good until now (22.58.35) syzzer: just to make sure - you are the author, right? (22.58.37) harshar ha abbandonato la stanza (quit: Ping timeout: 264 seconds). (22.58.41) segoon: yep (22.58.55) syzzer: I'm not sure if plaisthos is still around (23.00.34) segoon: uh, stupid mistake (23.01.03) segoon: I returned from afk when 1st question was discussed, didn't want to break it (23.01.07) plaisthos: oh sorry (23.01.10) plaisthos: was afk (23.01.51) syzzer: segoon: yeah, that first question took a bit longer than expected... (23.02.50) mattock: segoon: the first topic was a bit heavy
openvpn_irc_meeting_chatlog_2015-01-12.txt.sig
Description: PGP signature
