[Openvpn-devel] Adjust Linux+FreeBSD DCO device name handling to 'non DCO linux style'

Message ID 20220829190124.2636045-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel] Adjust Linux+FreeBSD DCO device name handling to 'non DCO linux style' | expand

Commit Message

Gert Doering Aug. 29, 2022, 9:01 a.m. UTC
On Linux, tun devices are created according to the following algorithm
  --dev tun    -> try tun0, tun1, ... tun255, use first free
  --dev anything -> create a TUN device named "anything"
(as long as "anything" is not "null" or "tap[N]")

DCO was following the "other platform convention", where everything
not having a digit was iterated ("--dev tun-home" -> "tun-home0") -
which does not work for classic tun/tap devices on the BSDs anyway,
so is not the best model.

Adjust open_tun_dco_generic() to document expected behaviour and
do the thing.

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/tun.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Kristof Provost Aug. 29, 2022, 9:16 p.m. UTC | #1
On 29 Aug 2022, at 21:01, Gert Doering wrote:
> On Linux, tun devices are created according to the following algorithm
>   --dev tun    -> try tun0, tun1, ... tun255, use first free
>   --dev anything -> create a TUN device named "anything"
> (as long as "anything" is not "null" or "tap[N]")
>
> DCO was following the "other platform convention", where everything
> not having a digit was iterated ("--dev tun-home" -> "tun-home0") -
> which does not work for classic tun/tap devices on the BSDs anyway,
> so is not the best model.
>
> Adjust open_tun_dco_generic() to document expected behaviour and
> do the thing.
>
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Kristof Provost <kprovost@netgate.com>


Kristof
Antonio Quartulli Aug. 30, 2022, 2:20 a.m. UTC | #2
Hi,

On 29/08/2022 21:01, Gert Doering wrote:
> On Linux, tun devices are created according to the following algorithm
>    --dev tun    -> try tun0, tun1, ... tun255, use first free
>    --dev anything -> create a TUN device named "anything"
> (as long as "anything" is not "null" or "tap[N]")
> 
> DCO was following the "other platform convention", where everything
> not having a digit was iterated ("--dev tun-home" -> "tun-home0") -
> which does not work for classic tun/tap devices on the BSDs anyway,
> so is not the best model.
> 
> Adjust open_tun_dco_generic() to document expected behaviour and
> do the thing.
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>   src/openvpn/tun.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 94803acd..94b0d322 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -1943,12 +1943,14 @@ open_tun_dco_generic(const char *dev, const char *dev_type,
>       }
>   
>       /*
> -     * dynamic open is indicated by --dev specified without
> -     * explicit unit number.  Try opening DCO device named "[dev]n"
> -     * where n = [0, 255].
> +     * unlike "open_tun_generic()", DCO on Linux and FreeBSD follows
> +     * the device naming model of "non-DCO linux", that is:
> +     *   --dev tun         -> try tun0, tun1, ... tun255, use first free
> +     *   --dev <anything>  -> (try to) create a tun device named "anything"
> +     * ("--dev tap" and "--dev null" are caught earlier and not handled here)
>        */
>   
> -    if (!tun_name_is_fixed(dev))
> +    if (strcmp(dev,"tun") == 0)

you need to add a space after the ','.

>       {
>           for (int i = 0; i < 256; ++i)
>           {


Other than that you get a 2nd ACK:

Acked-by: Antonio Quartulli <a@unstable.cc>

I wonder if we should document this behaviour somewhere else? i.e. manpage?

It seems that even us developers are puzzled by what it is supposed to 
happen.

Cheers,
Gert Doering Aug. 30, 2022, 9:57 a.m. UTC | #3
Thanks for the review.  strcmp() fixed (space) as instructed.

I'll send a manpage patch next to (try to...) document the expected behaviour
across platforms.

Your patch has been applied to the master branch.

commit a5cf4cfb77f74588759d96258e7435e54db12b2c
Author: Gert Doering
Date:   Mon Aug 29 21:01:24 2022 +0200

     Adjust Linux+FreeBSD DCO device name handling to 'non DCO linux style'

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Kristof Provost <kprovost@netgate.com>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20220829190124.2636045-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25134.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 94803acd..94b0d322 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1943,12 +1943,14 @@  open_tun_dco_generic(const char *dev, const char *dev_type,
     }
 
     /*
-     * dynamic open is indicated by --dev specified without
-     * explicit unit number.  Try opening DCO device named "[dev]n"
-     * where n = [0, 255].
+     * unlike "open_tun_generic()", DCO on Linux and FreeBSD follows
+     * the device naming model of "non-DCO linux", that is:
+     *   --dev tun         -> try tun0, tun1, ... tun255, use first free
+     *   --dev <anything>  -> (try to) create a tun device named "anything"
+     * ("--dev tap" and "--dev null" are caught earlier and not handled here)
      */
 
-    if (!tun_name_is_fixed(dev))
+    if (strcmp(dev,"tun") == 0)
     {
         for (int i = 0; i < 256; ++i)
         {