[S] Change in openvpn[master]: dco_linux: set ifindex also when adopting a pre-existing interface

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1896?usp=email

to review the following change.


Change subject: dco_linux: set ifindex also when adopting a pre-existing interface
......................................................................

dco_linux: set ifindex also when adopting a pre-existing interface

When the DCO interface is created by the user before the daemon is
started, net_iface_new() gets -EEXIST back from the kernel.
At this point open_tun_dco() returns right away and never reaches
the assignment of tt->dco.ifindex.

Missing the ifindex assignment makes all subsequent netlink call fail,
as they require a proper ifindex to identify the target device (ENODEV).

The breakage is easy to miss, because the error is reported at D_DCO,
which is not printed at the default verb, while the reassuring "re-using
existing device" message is.

Resolve the ifindex for the -EEXIST case too and pass -EEXIST on to the
caller, which already knows how to deal with it. This also brings the
Linux implementation in line with the FreeBSD one, which has always
completed the interface setup on -EEXIST as well.

The dynamic device name path is not affected: it accepts a return value
of 0 only and keeps scanning the next name on -EEXIST.

Change-Id: I1cd2fcb363b6ee95f25c5cba72f74f85e43ab66d
Signed-off-by: Antonio Quartulli <[email protected]>
---
M src/openvpn/dco.h
M src/openvpn/dco_linux.c
2 files changed, 7 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/96/1896/1

diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h
index 4e5aad5..1898bcf 100644
--- a/src/openvpn/dco.h
+++ b/src/openvpn/dco.h
@@ -114,7 +114,8 @@
  * @param tt        the tuntap context
  * @param ctx       the networking API context
  * @param dev       the name of the interface to create
- * @return          0 on success or a negative error code otherwise
+ * @return          0 on success, -EEXIST if the interface already existed
+ *                  and was adopted, or another negative error code on failure
  */
 int open_tun_dco(struct tuntap *tt, openvpn_net_ctx_t *ctx, const char *dev);
 
diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index 56f6259..40b4781 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -509,7 +509,10 @@
     ASSERT(tt->type == DEV_TYPE_TUN);
 
     int ret = net_iface_new(ctx, dev, OVPN_FAMILY_NAME, &tt->dco);
-    if (ret < 0)
+    /* don't bail out on -EEXIST, as we need to resolve the ifindex also
+     * in this case (the interface was pre-created and we may want to use it)
+     */
+    if (ret < 0 && ret != -EEXIST)
     {
         msg(D_DCO_DEBUG, "Cannot create DCO interface %s: %d", dev, ret);
         return ret;
@@ -521,7 +524,7 @@
         msg(M_FATAL, "DCO: cannot retrieve ifindex for interface %s", dev);
     }
 
-    return 0;
+    return ret;
 }
 
 void

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1896?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I1cd2fcb363b6ee95f25c5cba72f74f85e43ab66d
Gerrit-Change-Number: 1896
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel