| Message ID | 20221213225430.1892940-1-arne@rfc2549.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,1/3] Improve debug logging of DCO swap key message and Linux dco_new_peer | expand |
On 13/12/2022 23:54, Arne Schwabe wrote: > Signed-off-by: Arne Schwabe <arne@rfc2549.org> > --- > src/openvpn/dco.c | 18 ++++++++++++++---- > src/openvpn/dco_linux.c | 10 ++++++++-- > 2 files changed, 22 insertions(+), 6 deletions(-) > > diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c > index feb38cd02..2396bcbf0 100644 > --- a/src/openvpn/dco.c > +++ b/src/openvpn/dco.c > @@ -55,8 +55,8 @@ dco_install_key(struct tls_multi *multi, struct key_state *ks, > const char *ciphername) > > { > - msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d", __func__, multi->dco_peer_id, > - ks->key_id); > + msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d, currently installed %d", I'd make this "num. installed" because "currently installed" makes me think as if what follows is the ID of something that is installed. While we should make it explicit that this is the number of installed keys. > + __func__, multi->dco_peer_id, ks->key_id, multi->dco_keys_installed); > > /* Install a key in the PRIMARY slot only when no other key exist. > * From that moment on, any new key will be installed in the SECONDARY > @@ -181,8 +181,18 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi) > */ > if (primary->dco_status == DCO_INSTALLED_SECONDARY) > { > - msg(D_DCO_DEBUG, "Swapping primary and secondary keys, now: id1=%d id2=%d", > - primary->key_id, secondary ? secondary->key_id : -1); > + if (secondary) > + { > + msg(D_DCO_DEBUG, "Swapping primary and secondary keys to " > + "primary-id=%d secondary-id=%d", > + primary->key_id, secondary->key_id); > + } > + else > + { > + msg(D_DCO_DEBUG, "Swapping primary and secondary keys to" > + "primary-id=%d secondary-id=(to be deleted)", > + primary->key_id); > + } > > int ret = dco_swap_keys(dco, multi->dco_peer_id); > if (ret < 0) > diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c > index 109358205..fbd940c28 100644 > --- a/src/openvpn/dco_linux.c > +++ b/src/openvpn/dco_linux.c > @@ -216,9 +216,15 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, > struct sockaddr *localaddr, struct sockaddr *remoteaddr, > struct in_addr *remote_in4, struct in6_addr *remote_in6) > { > - msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd); > - > struct gc_arena gc = gc_new(); > + const char *remotestr = "[undefined]"; > + if (remoteaddr) > + { > + remotestr = print_sockaddr(remoteaddr, &gc); > + } > + msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d, remote addr: %s", __func__, > + peerid, sd, remotestr); > + > struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_NEW_PEER); > struct nlattr *attr = nla_nest_start(nl_msg, OVPN_ATTR_NEW_PEER); > int ret = -EMSGSIZE; The rest makes sense: Acked-by: Antonio Quartulli <a@unstable.cc>
As instructed I have changed the wording a bit, it's now
"... currently %d keys installed"
which should make it clear that we are talking about the number of keys,
and not a key ID. "currently installed %d" is ambiguous here.
I have not tested this beyond "does it compile?", but since this is all
just messages (and a bit of string formatting with an existing gc) the
risk for "it will explode on platform X or with compile-time option Y"
is minimal.
Your patch has been applied to the master and release/2.6 branch.
commit 63f838e384df3fb644bdeae6ede777b54dc968db (master)
commit 8910b5b6028ee0feb9c324b1baf137cae5b4d9bd (release/2.6)
Author: Arne Schwabe
Date: Tue Dec 13 23:54:28 2022 +0100
Improve debug logging of DCO swap key message and Linux dco_new_peer
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20221213225430.1892940-1-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25680.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index feb38cd02..2396bcbf0 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -55,8 +55,8 @@ dco_install_key(struct tls_multi *multi, struct key_state *ks, const char *ciphername) { - msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d", __func__, multi->dco_peer_id, - ks->key_id); + msg(D_DCO_DEBUG, "%s: peer_id=%d keyid=%d, currently installed %d", + __func__, multi->dco_peer_id, ks->key_id, multi->dco_keys_installed); /* Install a key in the PRIMARY slot only when no other key exist. * From that moment on, any new key will be installed in the SECONDARY @@ -181,8 +181,18 @@ dco_update_keys(dco_context_t *dco, struct tls_multi *multi) */ if (primary->dco_status == DCO_INSTALLED_SECONDARY) { - msg(D_DCO_DEBUG, "Swapping primary and secondary keys, now: id1=%d id2=%d", - primary->key_id, secondary ? secondary->key_id : -1); + if (secondary) + { + msg(D_DCO_DEBUG, "Swapping primary and secondary keys to " + "primary-id=%d secondary-id=%d", + primary->key_id, secondary->key_id); + } + else + { + msg(D_DCO_DEBUG, "Swapping primary and secondary keys to" + "primary-id=%d secondary-id=(to be deleted)", + primary->key_id); + } int ret = dco_swap_keys(dco, multi->dco_peer_id); if (ret < 0) diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 109358205..fbd940c28 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -216,9 +216,15 @@ dco_new_peer(dco_context_t *dco, unsigned int peerid, int sd, struct sockaddr *localaddr, struct sockaddr *remoteaddr, struct in_addr *remote_in4, struct in6_addr *remote_in6) { - msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d", __func__, peerid, sd); - struct gc_arena gc = gc_new(); + const char *remotestr = "[undefined]"; + if (remoteaddr) + { + remotestr = print_sockaddr(remoteaddr, &gc); + } + msg(D_DCO_DEBUG, "%s: peer-id %d, fd %d, remote addr: %s", __func__, + peerid, sd, remotestr); + struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_NEW_PEER); struct nlattr *attr = nla_nest_start(nl_msg, OVPN_ATTR_NEW_PEER); int ret = -EMSGSIZE;
Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- src/openvpn/dco.c | 18 ++++++++++++++---- src/openvpn/dco_linux.c | 10 ++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-)