diff mbox series

[Openvpn-devel] Avoid management log loop with verb >= 6

Message ID 20230217122156.541-1-lstipakov@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] Avoid management log loop with verb >= 6 | expand

Commit Message

Lev Stipakov Feb. 17, 2023, 12:21 p.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

This log message is printed within check_tls(),
which is called by pre_select(), which is called
on every iteration of event loop.

When management is attached (and doesn't use own event loop),
this message sets management state to "wait write",
which arms event loop. When on the next iteration iowait
returns with "management write event is set", we call
pre_select() and print that message again, causing the loop.

Fix by simply removing this log message.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/dco.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Antonio Quartulli Feb. 26, 2023, 8:55 p.m. UTC | #1 
Hi,

On 17/02/2023 13:21, Lev Stipakov wrote:
> From: Lev Stipakov <lev@openvpn.net>
> 
> This log message is printed within check_tls(),
> which is called by pre_select(), which is called
> on every iteration of event loop.
> 
> When management is attached (and doesn't use own event loop),
> this message sets management state to "wait write",
> which arms event loop. When on the next iteration iowait
> returns with "management write event is set", we call
> pre_select() and print that message again, causing the loop.
> 
> Fix by simply removing this log message.
> 
> Signed-off-by: Lev Stipakov <lev@openvpn.net>

As discussed on IRC, removing this message is a good thing as it serves 
no real purpose.

On top of that this message is causing this log infinite recursion, 
therefore it should just go.

OTOH we may still have *some* recursion due to other messages printed by 
this function. However, this messages will print only once, therefore 
they won't cause the recursion to continue indefinitely.

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

> ---
>   src/openvpn/dco.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
> index 3087a0df..b53332a8 100644
> --- a/src/openvpn/dco.c
> +++ b/src/openvpn/dco.c
> @@ -133,8 +133,6 @@ dco_get_secondary_key(struct tls_multi *multi, const struct key_state *primary)
>   bool
>   dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
>   {
> -    msg(D_DCO_DEBUG, "%s: peer_id=%d", __func__, multi->dco_peer_id);
> -
>       /* this function checks if keys have to be swapped or erased, therefore it
>        * can't do much if we don't have any key installed
>        */
Gert Doering Feb. 27, 2023, 7:17 a.m. UTC | #2 
Trivial enough :-)

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

commit b8eddda8524bf6f164361667bfce6bbb3fac846b (master)
commit c26609bdcd75da5702befbdfa5d0136a7787e52e (release/2.6)
Author: Lev Stipakov
Date:   Fri Feb 17 14:21:55 2023 +0200

     Avoid management log loop with verb >= 6

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 3087a0df..b53332a8 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -133,8 +133,6 @@  dco_get_secondary_key(struct tls_multi *multi, const struct key_state *primary)
 bool
 dco_update_keys(dco_context_t *dco, struct tls_multi *multi)
 {
-    msg(D_DCO_DEBUG, "%s: peer_id=%d", __func__, multi->dco_peer_id);
-
     /* this function checks if keys have to be swapped or erased, therefore it
      * can't do much if we don't have any key installed
      */