[Openvpn-devel,4/4] Set (DCO) timeouts as well for p2p mode

Message ID 20220516185621.6182-5-kprovost@netgate.com
State Superseded
Delegated to: Antonio Quartulli
Headers show
Series
  • [Openvpn-devel,1/4] mtcp: Handle multi_create_instance() returning NULL
Related show

Commit Message

Kristof Provost via Openvpn-devel May 16, 2022, 6:56 p.m.
From: Kristof Provost <kp@FreeBSD.org>

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/init.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Antonio Quartulli May 17, 2022, 1:25 p.m. | #1
Hi,

On 16/05/2022 20:56, Kristof Provost via Openvpn-devel wrote:
> From: Kristof Provost <kp@FreeBSD.org>
> 
> Signed-off-by: Kristof Provost <kprovost@netgate.com>
> ---
>   src/openvpn/init.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 0d991ba4..701749cd 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2232,6 +2232,21 @@ do_deferred_p2p_ncp(struct context *c)
>           msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
>           return false;
>       }
> +
> +    if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
> +    {
> +        int ret = dco_set_peer(&c->c1.tuntap->dco,
> +                c->c2.tls_multi->peer_id,
> +                c->options.ping_send_timeout,
> +                c->options.ping_rec_timeout,
> +                c->c2.frame.mss_fix);
> +        if (ret < 0)
> +        {
> +            msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret));
> +            return false;
> +        }
> +    }
> +

unless I am missing something, I don't think we need another 
dco_set_peer call.

It should be enough to move the invocation of dco_set_peer in 
finish_options at the very top, before any attempt of bailing out.

Regards,
Kristof Provost via Openvpn-devel May 17, 2022, 4:28 p.m. | #2
On 17 May 2022, at 15:25, Antonio Quartulli wrote:
> Hi,
>
> On 16/05/2022 20:56, Kristof Provost via Openvpn-devel wrote:
>> From: Kristof Provost <kp@FreeBSD.org>
>>
>> Signed-off-by: Kristof Provost <kprovost@netgate.com>
>> ---
>>   src/openvpn/init.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
>> index 0d991ba4..701749cd 100644
>> --- a/src/openvpn/init.c
>> +++ b/src/openvpn/init.c
>> @@ -2232,6 +2232,21 @@ do_deferred_p2p_ncp(struct context *c)
>>           msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
>>           return false;
>>       }
>> +
>> +    if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
>> +    {
>> +        int ret = dco_set_peer(&c->c1.tuntap->dco,
>> +                c->c2.tls_multi->peer_id,
>> +                c->options.ping_send_timeout,
>> +                c->options.ping_rec_timeout,
>> +                c->c2.frame.mss_fix);
>> +        if (ret < 0)
>> +        {
>> +            msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret));
>> +            return false;
>> +        }
>> +    }
>> +
>
> unless I am missing something, I don't think we need another dco_set_peer call.
>
> It should be enough to move the invocation of dco_set_peer in finish_options at the very top, before any attempt of bailing out.
>
Good point. I’ll update my patchset and include this in the next update.

Br,
Kristof

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0d991ba4..701749cd 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2232,6 +2232,21 @@  do_deferred_p2p_ncp(struct context *c)
         msg(D_TLS_ERRORS, "ERROR: failed to set crypto cipher");
         return false;
     }
+
+    if (dco_enabled(&c->options) && (c->options.ping_send_timeout || c->c2.frame.mss_fix))
+    {
+        int ret = dco_set_peer(&c->c1.tuntap->dco,
+                c->c2.tls_multi->peer_id,
+                c->options.ping_send_timeout,
+                c->options.ping_rec_timeout,
+                c->c2.frame.mss_fix);
+        if (ret < 0)
+        {
+            msg(D_DCO, "Cannot set DCO peer: %s", strerror(-ret));
+            return false;
+        }
+    }
+
     return true;
 }