Your patch has been applied to the master branch.

I'm (sorry) again not totally happy with it - it introduces the configure
changes for sitnl, namely, breaking the "do route and ifconfig command
exist?" check on all Linux systems (have_sitnl=yes is unconditionally
set for Linux, and that will then always skip the check), without 
actually providing calls to sitnl yet.  A better granularity would have
introduced the configure checks only at the very end, when the actual
HAVE_SITNL code is not only "it is there and compiles" but also functional,
and route/ifconfig are kicked out.

Additonally, it introduces "sitnl.h" which is not included in 
openvpn_SOURCES and not referenced from anywhere... which is also an
extra burden on me (will this disappear in a future patch?  will it
be included?  will it appear in openvpn_SOURCES eventually?).

The code in networking_sitnl.* looks mostly reasonable, though I 
wonder if it wasn't more elegant to do the sitnl_socket() dance only
once in the init, and store the file descriptor in the context
afterwards... right now, you're doing the whole open/bind/../close
for every function called.

The upcoming networking_we_are_done() function doing the gc_free()
calls for non-linux implementations could then do the close() for
sitnl...

Besides this, there are a few warts...

+    if (up)
+        req.i.ifi_flags |= IFF_UP;
+    else
+        req.i.ifi_flags &= ~IFF_UP;

..
+    ifindex = if_nametoindex(iface);
+    if (ifindex == 0) {

.. which you might want to run through uncrustify eventually...


commit c6542257019ba66a98b817d3b851621dd553f80a
Author: Antonio Quartulli
Date:   Wed Dec 19 15:01:13 2018 +1000

     introduce sitnl: Simplified Interface To NetLink

     Signed-off-by: Antonio Quartulli <a...@unstable.cc>
     Acked-by: Arne Schwabe <a...@rfc2549.org>
     Message-Id: <20181219050118.6568-...@unstable.cc>
     URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18030.html
     Signed-off-by: Gert Doering <g...@greenie.muc.de>


--
kind regards,

Gert Doering



_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to