[Openvpn-devel] Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode

Message ID 20230109200011.2525342-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel] Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode | expand

Commit Message

Gert Doering Jan. 9, 2023, 8 p.m. UTC
p2p --tls-server with no active client/peer logs once per second

  "dco_update_keys: peer_id=-1"

which does exactly nothing, except fill the disk.  So skip the call to
dco_update_keys() if peer_id == -1.

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

Comments

Antonio Quartulli Jan. 10, 2023, 1:59 p.m. UTC | #1
Hi,

On 09/01/2023 21:00, Gert Doering wrote:
> p2p --tls-server with no active client/peer logs once per second
> 
>    "dco_update_keys: peer_id=-1"
> 
> which does exactly nothing, except fill the disk.  So skip the call to
> dco_update_keys() if peer_id == -1.
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>   src/openvpn/forward.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index ae0512fc..2ba8b0fa 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -151,6 +151,12 @@ check_dco_key_status(struct context *c)
>           return;
>       }
>   
> +    /* no active peer (p2p tls-server mode) */
> +    if (c->c2.tls_multi->dco_peer_id == -1 )

Please remove the space after -1 (not sure why uncrustify hasn't caught it).

> +    {
> +        return;
> +    }
> +
>       if (!dco_update_keys(&c->c1.tuntap->dco, c->c2.tls_multi))
>       {
>           /* Something bad happened. Kill the connection to


Rest looks good. Thanks!

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

However, as discussed on IRC: *why* are we running the check_tls code is 
the peer has gone away and we have switched the peer-id to -1?

This is the real question.
Gert Doering Jan. 10, 2023, 2:06 p.m. UTC | #2
Antonio, thanks for the review.  Fixed the whitespace.  (Uncrustify
did not see it since the patch was ad-hoc written on a system that
does not have the hook - but my pre-merge hook would have caught it).

Patch has been applied to the master and release/2.6 branch.

commit 85e0df6b493396d9d1d9030c4018f67037d2f12b (master)
commit 9bc2dfea0e1f643b2a02b61455e1845ae83f7518 (release/2.6)
Author: Gert Doering
Date:   Mon Jan 9 21:00:11 2023 +0100

     Reduce logspam about 'dco_update_keys: peer_id=-1' in p2p server mode

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index ae0512fc..2ba8b0fa 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -151,6 +151,12 @@  check_dco_key_status(struct context *c)
         return;
     }
 
+    /* no active peer (p2p tls-server mode) */
+    if (c->c2.tls_multi->dco_peer_id == -1 )
+    {
+        return;
+    }
+
     if (!dco_update_keys(&c->c1.tuntap->dco, c->c2.tls_multi))
     {
         /* Something bad happened. Kill the connection to