From 779c87b0266adb815d27d3e4dfcc35433ede0fcb Mon Sep 17 00:00:00 2001 From: Antonio Quartulli Date: Sat, 26 Jul 2025 01:24:23 +0200 Subject: [PATCH] dco_linux: clean up PEER_GET trigger and parser This patch is intended to reduce code duplication and cleanup the DCO code around the PEER_GET command. Specifically it: * unified PEER_GET reply parser for `multi` and `non-multi` case * unified PEER_GET request trigger for `multi` and `non-multi` case * dropped struct multi_context from the argument list of dco_get_peer_stats_multi() Github: closes OpenVPN/openvpn#800 Change-Id: Icbc70225d53ca678b8c22ed437b424c16e199d66 Signed-off-by: Antonio Quartulli --- src/openvpn/dco.h | 7 +- src/openvpn/dco_freebsd.c | 5 +- src/openvpn/dco_linux.c | 142 +++++++++++--------------------------- src/openvpn/dco_win.c | 3 +- src/openvpn/multi.c | 4 +- 5 files changed, 49 insertions(+), 112 deletions(-) diff --git a/src/openvpn/dco.h b/src/openvpn/dco.h index 90784177c..2ce0eb1d1 100644 --- a/src/openvpn/dco.h +++ b/src/openvpn/dco.h @@ -230,11 +230,9 @@ void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi); * Update traffic statistics for all peers * * @param dco DCO device context - * @param m the server context * @param raise_sigusr1_on_err whether to raise SIGUSR1 on error **/ -int dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, - const bool raise_sigusr1_on_err); +int dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err); /** * Update traffic statistics for single peer @@ -374,8 +372,7 @@ dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi) } static inline int -dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, - const bool raise_sigusr1_on_err) +dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) { return 0; } diff --git a/src/openvpn/dco_freebsd.c b/src/openvpn/dco_freebsd.c index 98d8fb54c..e55e58d2f 100644 --- a/src/openvpn/dco_freebsd.c +++ b/src/openvpn/dco_freebsd.c @@ -713,8 +713,7 @@ dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *n } int -dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, - const bool raise_sigusr1_on_err) +dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) { struct ifdrv drv; @@ -774,7 +773,7 @@ retry: const nvlist_t *peer = nvpeers[i]; uint32_t peerid = nvlist_get_number(peer, "peerid"); - dco_update_peer_stat(m, peerid, nvlist_get_nvlist(peer, "bytes")); + dco_update_peer_stat(dco->c->multi, peerid, nvlist_get_nvlist(peer, "bytes")); } nvlist_destroy(nvl); diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c index 728fb7e7a..9ad3d5179 100644 --- a/src/openvpn/dco_linux.c +++ b/src/openvpn/dco_linux.c @@ -876,54 +876,9 @@ dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id) } } -static int -ovpn_handle_peer_multi(dco_context_t *dco, struct nlattr *attrs[]) -{ - msg(D_DCO_DEBUG, "%s: parsing message...", __func__); - - /* this function assumes openvpn is running in multipeer mode as - * it accesses c->multi - */ - if (dco->ifmode != OVPN_MODE_MP) - { - msg(M_WARN, "%s: can't parse 'multi-peer' message on P2P instance", __func__); - return NL_SKIP; - } - - if (!attrs[OVPN_A_PEER]) - { - return NL_SKIP; - } - - struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1]; - nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL); - - if (!tb_peer[OVPN_A_PEER_ID]) - { - msg(M_WARN, "ovpn-dco: no peer-id provided in (MULTI) PEER_GET reply"); - return NL_SKIP; - } - - struct multi_context *m = dco->c->multi; - uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]); - - if (peer_id >= m->max_clients || !m->instances[peer_id]) - { - msg(M_WARN, "%s: cannot store DCO stats for peer %u", __func__, - peer_id); - return NL_SKIP; - } - - dco_update_peer_stat(&m->instances[peer_id]->context.c2, tb_peer, peer_id); - - return NL_OK; -} - static int ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) { - msg(D_DCO_DEBUG, "%s: parsing message...", __func__); - if (!attrs[OVPN_A_PEER]) { msg(D_DCO_DEBUG, "%s: malformed reply", __func__); @@ -942,12 +897,25 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]); struct context_2 *c2; + msg(D_DCO_DEBUG, "%s: parsing message for peer %u...", __func__, peer_id); + if (dco->ifmode == OVPN_MODE_P2P) { c2 = &dco->c->c2; + if (c2->tls_multi->dco_peer_id != peer_id) + { + return NL_SKIP; + } } else { + if (peer_id >= dco->c->multi->max_clients) + { + msg(M_WARN, "%s: received out of bound peer_id %u (max=%u)", __func__, peer_id, + dco->c->multi->max_clients); + return NL_SKIP; + } + struct multi_instance *mi = dco->c->multi->instances[peer_id]; if (!mi) { @@ -958,14 +926,6 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) c2 = &mi->context.c2; } - /* at this point this check should never fail for MP mode, - * but it's still fully valid for P2P mode - */ - if (c2->tls_multi->dco_peer_id != peer_id) - { - return NL_SKIP; - } - dco_update_peer_stat(c2, tb_peer, peer_id); return NL_OK; @@ -1176,17 +1136,7 @@ ovpn_handle_msg(struct nl_msg *msg, void *arg) { case OVPN_CMD_PEER_GET: { - /* this message is part of a peer list dump, hence triggered - * by a MP/server instance - */ - if (nlh->nlmsg_flags & NLM_F_MULTI) - { - return ovpn_handle_peer_multi(dco, attrs); - } - else - { - return ovpn_handle_peer(dco, attrs); - } + return ovpn_handle_peer(dco, attrs); } case OVPN_CMD_PEER_DEL_NTF: @@ -1221,52 +1171,32 @@ dco_do_read(dco_context_t *dco) return ovpn_nl_recvmsgs(dco, __func__); } -int -dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, - const bool raise_sigusr1_on_err) -{ - msg(D_DCO_DEBUG, "%s", __func__); - - struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_PEER_GET); - - nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP; - - int ret = ovpn_nl_msg_send(dco, nl_msg, __func__); - - nlmsg_free(nl_msg); - - if (raise_sigusr1_on_err && ret < 0) - { - msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer" - "may have been deleted from the kernel without notifying " - "userspace. Restarting the session"); - register_signal(m->top.sig, SIGUSR1, "dco peer stats error"); - } - return ret; -} - -int -dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err) +static int +dco_get_peer(dco_context_t *dco, int peer_id, const bool raise_sigusr1_on_err) { - int peer_id = c->c2.tls_multi->dco_peer_id; - if (peer_id == -1) + /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. + * If it happens in P2P mode it means that the DCO peer was deleted and we + * can simply bail out + */ + if (peer_id == -1 && dco->ifmode == OVPN_MODE_P2P) { return 0; } msg(D_DCO_DEBUG, "%s: peer-id %d", __func__, peer_id); - if (!c->c1.tuntap) - { - return 0; - } - - dco_context_t *dco = &c->c1.tuntap->dco; struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_PEER_GET); struct nlattr *attr = nla_nest_start(nl_msg, OVPN_A_PEER); int ret = -EMSGSIZE; - NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id); + if (peer_id != -1) + { + NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id); + } + else + { + nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP; + } nla_nest_end(nl_msg, attr); ret = ovpn_nl_msg_send(dco, nl_msg, __func__); @@ -1279,11 +1209,23 @@ nla_put_failure: msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer" "may have been deleted from the kernel without notifying " "userspace. Restarting the session"); - register_signal(c->sig, SIGUSR1, "dco peer stats error"); + register_signal(dco->c->sig, SIGUSR1, "dco peer stats error"); } return ret; } +int +dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err) +{ + return dco_get_peer(&c->c1.tuntap->dco, c->c2.tls_multi->dco_peer_id, raise_sigusr1_on_err); +} + +int +dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) +{ + return dco_get_peer(dco, -1, raise_sigusr1_on_err); +} + bool dco_available(int msglevel) { diff --git a/src/openvpn/dco_win.c b/src/openvpn/dco_win.c index e5a33a0f3..995b121d0 100644 --- a/src/openvpn/dco_win.c +++ b/src/openvpn/dco_win.c @@ -715,8 +715,7 @@ dco_do_read(dco_context_t *dco) } int -dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, - const bool raise_sigusr1_on_err) +dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) { /* Not implemented. */ return 0; diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index a62c57afc..c5691ffeb 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -551,7 +551,7 @@ setenv_stats(struct multi_context *m, struct context *c) { if (dco_enabled(&m->top.options)) { - if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, false) < 0) + if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, false) < 0) { return; } @@ -862,7 +862,7 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int if (dco_enabled(&m->top.options)) { - if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, true) < 0) + if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, true) < 0) { return; } -- 2.47.2