Changes between Version 3 and Version 4 of Topics-2011-08-18


Ignore:
Timestamp:
08/17/11 09:49:25 (13 years ago)
Author:
Samuli Seppänen
Comment:

--

Legend:

Unmodified
Added
Removed
Modified
  • Topics-2011-08-18

    v3 v4  
    88= Development =
    99
    10 == SVN merger ==
    11 
    12 Dazo made a [http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=commitdiff;h=e47fb603ed721bb718495e6f8ed42ec134da2f98 heroic merge] of James' SVN branch to "master". We need to discuss this in more detail.
    13 
    1410== Next releases ==
    1511
    1612 * OpenVPN 2.2.1
    1713 * OpenVPN 2.3
     14
     15== SVN merger ==
     16
     17Dazo made a [http://openvpn.git.sourceforge.net/git/gitweb.cgi?p=openvpn/openvpn-testing.git;a=commitdiff;h=e47fb603ed721bb718495e6f8ed42ec134da2f98 heroic merge] of James' SVN branch to "master". We need to discuss this in more detail. Here are comments from cron2:
     18
     19{{{
     20
     21Random ramblings in the order I go through things...
     22
     23   - I git-clone'd openvpn-testing.git, went to the svn-merger branch, and
     24     ran "make check" (on Gentoo Linux), with my full-featured t_client.rc
     25     setup.
     26
     27     Test ran:
     28      - p2mp tun udp (ipv4 + ipv6 ok)
     29      - p2mp tun tcp (ipv4 + ipv6 ok)
     30      - p2mp tun udp, "topology subnet" (ipv4 + ipv6 ok)
     31      - p2mp tap udp (ipv4 ok, ipv6 fails, known issue with IPv6 auto-conf
     32                      on TAP, not related to the svn-merger)
     33
     34     so the client code (at least) is still working as well as my tests
     35     cover the code.
     36
     37   - code alignment needed: for IPv4, the "did_redirect_default_gateway"
     38     and "spec.remote_endpoint_defined" have been converted to flag bits in
     39     route_list-iflags, but for IPv6, the old structure elements
     40     remain - so to make the code more "in-line" for IPv4 and IPv6, this
     41     needs code adjustments in the IPv6 code.
     42
     43   - I'm not overly happy about the "default-gateway block-local" changes -
     44     this is less a code issue (the code might be fine) but a procedural
     45     issue, with a huge change to route.c coming in without any sort of
     46     review or discussion.  Gah.  (No response needed).
     47
     48   - I'm somewhat more annoyed by this one (route.c, line 1280):
     49
     50 #if defined(TARGET_LINUX)
     51 #ifdef CONFIG_FEATURE_IPROUTE
     52    /* FIXME -- add LR_MATCH support for CONFIG_FEATURE_IPROUTE */
     53
     54     this is implemented only for the "non-iproute2" case, so we have
     55     differing behaviour for iproute2/non-iproute2 compiles now.
     56     This MUST be fixed for 2.3
     57
     58   - implementations of LR_MATCH for most other platforms are missing,
     59     but this is something that can be documented in the release notes,
     60     and if someone thinks they need this, they can add it - but having
     61     support-or-not for Linux, depending on --enable-iproute2, is a no-go
     62
     63   - the merger has PF_INET6 blocks, and I currently don't run tests over
     64     IPv6 transport.  So maybe jjo could also take a look at this branch
     65     and see whether his stuff is still working.
     66
     67   - the web view of route.c, "-887,13 - +894,10" looks a bit weird,
     68     with "++add_routes (...", but the code in the branch is fine.
     69
     70   - there's a functional and potentially-fatal change here:
     71
     72    void
     73   -delete_routes (struct route_list *rl, const struct tuntap *tt, unsigned int flags, const struct env_set *es)
     74   +delete_routes (struct route_list *rl, struct route_ipv6_list *rl6,
     75   +             const struct tuntap *tt, unsigned int flags, const struct env_set *es)
     76    {
     77 -   if (rl&&  rl-routes_added)
     78 +   if (rl-iflags&  RL_ROUTES_ADDED)
     79
     80
     81     this new code does not check whether "rl" is non-NULL, but in theory
     82     it could very well be NULL if we only have IPv6 routes.
     83
     84     So route.c line 1034 should really be:
     85
     86       if ( rl&&  rl-iflags&  RL_ROUTES_ADDED)
     87
     88     and the corresponding code in add_routes (route.c, line 990) should
     89     read:
     90
     91       if ( rl&&  !(rl-iflags&  RL_ROUTES_ADDED))
     92
     93     ... enhancing my tests to add a "--route-nopull --route-ipv6 test"...
     94     and indeed:
     95
     96 ./t_client.sh: Zeile 200:  8787 Speicherzugriffsfehler  ./openvpn $openvpn_conf  $LOGDIR/$SUF:openvpn.log
     97
     98     *bang*
     99
     100 redirect_default_route_to_vpn (rl=0x0, rl6=0x80f801c, tt=0x8101f90, flags=0,
     101      es=0x80dc860) at route.c:811
     102 811       if (rl-flags&  RG_ENABLE)
     103
     104     a proper patch is attached...
     105
     106   - ssl.c: it would be useful if andj or d12fk could review that - I'm not
     107     actually sure I understand what changed, but it seems to be some
     108     shuffling around of code and #ifdef ENABLE_CLIENT_CR, without actually
     109     changing much.
     110
     111
     112 the rest looks ok-ish to me...  (but yes, I can understand that it took
     113 you a heroic effort to merge that).
     114
     115}}}