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 <antonio@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20250727102245.24931-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg32361.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
| ... | ... |
@@ -230,11 +230,9 @@ void dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi); |
| 230 | 230 |
* Update traffic statistics for all peers |
| 231 | 231 |
* |
| 232 | 232 |
* @param dco DCO device context |
| 233 |
- * @param m the server context |
|
| 234 | 233 |
* @param raise_sigusr1_on_err whether to raise SIGUSR1 on error |
| 235 | 234 |
**/ |
| 236 |
-int dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, |
|
| 237 |
- const bool raise_sigusr1_on_err); |
|
| 235 |
+int dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err); |
|
| 238 | 236 |
|
| 239 | 237 |
/** |
| 240 | 238 |
* Update traffic statistics for single peer |
| ... | ... |
@@ -374,8 +372,7 @@ dco_delete_iroutes(struct multi_context *m, struct multi_instance *mi) |
| 374 | 374 |
} |
| 375 | 375 |
|
| 376 | 376 |
static inline int |
| 377 |
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, |
|
| 378 |
- const bool raise_sigusr1_on_err) |
|
| 377 |
+dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) |
|
| 379 | 378 |
{
|
| 380 | 379 |
return 0; |
| 381 | 380 |
} |
| ... | ... |
@@ -167,6 +167,8 @@ close_fd(dco_context_t *dco) |
| 167 | 167 |
bool |
| 168 | 168 |
ovpn_dco_init(struct context *c) |
| 169 | 169 |
{
|
| 170 |
+ c->c1.tuntap->dco.c = c; |
|
| 171 |
+ |
|
| 170 | 172 |
if (open_fd(&c->c1.tuntap->dco) < 0) |
| 171 | 173 |
{
|
| 172 | 174 |
msg(M_ERR, "Failed to open socket"); |
| ... | ... |
@@ -713,8 +715,7 @@ dco_update_peer_stat(struct multi_context *m, uint32_t peerid, const nvlist_t *n |
| 713 | 713 |
} |
| 714 | 714 |
|
| 715 | 715 |
int |
| 716 |
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, |
|
| 717 |
- const bool raise_sigusr1_on_err) |
|
| 716 |
+dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) |
|
| 718 | 717 |
{
|
| 719 | 718 |
|
| 720 | 719 |
struct ifdrv drv; |
| ... | ... |
@@ -774,7 +775,7 @@ retry: |
| 774 | 774 |
const nvlist_t *peer = nvpeers[i]; |
| 775 | 775 |
uint32_t peerid = nvlist_get_number(peer, "peerid"); |
| 776 | 776 |
|
| 777 |
- dco_update_peer_stat(m, peerid, nvlist_get_nvlist(peer, "bytes")); |
|
| 777 |
+ dco_update_peer_stat(dco->c->multi, peerid, nvlist_get_nvlist(peer, "bytes")); |
|
| 778 | 778 |
} |
| 779 | 779 |
|
| 780 | 780 |
nvlist_destroy(nvl); |
| ... | ... |
@@ -877,53 +877,8 @@ dco_update_peer_stat(struct context_2 *c2, struct nlattr *tb[], uint32_t id) |
| 877 | 877 |
} |
| 878 | 878 |
|
| 879 | 879 |
static int |
| 880 |
-ovpn_handle_peer_multi(dco_context_t *dco, struct nlattr *attrs[]) |
|
| 881 |
-{
|
|
| 882 |
- msg(D_DCO_DEBUG, "%s: parsing message...", __func__); |
|
| 883 |
- |
|
| 884 |
- /* this function assumes openvpn is running in multipeer mode as |
|
| 885 |
- * it accesses c->multi |
|
| 886 |
- */ |
|
| 887 |
- if (dco->ifmode != OVPN_MODE_MP) |
|
| 888 |
- {
|
|
| 889 |
- msg(M_WARN, "%s: can't parse 'multi-peer' message on P2P instance", __func__); |
|
| 890 |
- return NL_SKIP; |
|
| 891 |
- } |
|
| 892 |
- |
|
| 893 |
- if (!attrs[OVPN_A_PEER]) |
|
| 894 |
- {
|
|
| 895 |
- return NL_SKIP; |
|
| 896 |
- } |
|
| 897 |
- |
|
| 898 |
- struct nlattr *tb_peer[OVPN_A_PEER_MAX + 1]; |
|
| 899 |
- nla_parse_nested(tb_peer, OVPN_A_PEER_MAX, attrs[OVPN_A_PEER], NULL); |
|
| 900 |
- |
|
| 901 |
- if (!tb_peer[OVPN_A_PEER_ID]) |
|
| 902 |
- {
|
|
| 903 |
- msg(M_WARN, "ovpn-dco: no peer-id provided in (MULTI) PEER_GET reply"); |
|
| 904 |
- return NL_SKIP; |
|
| 905 |
- } |
|
| 906 |
- |
|
| 907 |
- struct multi_context *m = dco->c->multi; |
|
| 908 |
- uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]); |
|
| 909 |
- |
|
| 910 |
- if (peer_id >= m->max_clients || !m->instances[peer_id]) |
|
| 911 |
- {
|
|
| 912 |
- msg(M_WARN, "%s: cannot store DCO stats for peer %u", __func__, |
|
| 913 |
- peer_id); |
|
| 914 |
- return NL_SKIP; |
|
| 915 |
- } |
|
| 916 |
- |
|
| 917 |
- dco_update_peer_stat(&m->instances[peer_id]->context.c2, tb_peer, peer_id); |
|
| 918 |
- |
|
| 919 |
- return NL_OK; |
|
| 920 |
-} |
|
| 921 |
- |
|
| 922 |
-static int |
|
| 923 | 880 |
ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) |
| 924 | 881 |
{
|
| 925 |
- msg(D_DCO_DEBUG, "%s: parsing message...", __func__); |
|
| 926 |
- |
|
| 927 | 882 |
if (!attrs[OVPN_A_PEER]) |
| 928 | 883 |
{
|
| 929 | 884 |
msg(D_DCO_DEBUG, "%s: malformed reply", __func__); |
| ... | ... |
@@ -942,12 +897,25 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) |
| 942 | 942 |
uint32_t peer_id = nla_get_u32(tb_peer[OVPN_A_PEER_ID]); |
| 943 | 943 |
struct context_2 *c2; |
| 944 | 944 |
|
| 945 |
+ msg(D_DCO_DEBUG, "%s: parsing message for peer %u...", __func__, peer_id); |
|
| 946 |
+ |
|
| 945 | 947 |
if (dco->ifmode == OVPN_MODE_P2P) |
| 946 | 948 |
{
|
| 947 | 949 |
c2 = &dco->c->c2; |
| 950 |
+ if (c2->tls_multi->dco_peer_id != peer_id) |
|
| 951 |
+ {
|
|
| 952 |
+ return NL_SKIP; |
|
| 953 |
+ } |
|
| 948 | 954 |
} |
| 949 | 955 |
else |
| 950 | 956 |
{
|
| 957 |
+ if (peer_id >= dco->c->multi->max_clients) |
|
| 958 |
+ {
|
|
| 959 |
+ msg(M_WARN, "%s: received out of bound peer_id %u (max=%u)", __func__, peer_id, |
|
| 960 |
+ dco->c->multi->max_clients); |
|
| 961 |
+ return NL_SKIP; |
|
| 962 |
+ } |
|
| 963 |
+ |
|
| 951 | 964 |
struct multi_instance *mi = dco->c->multi->instances[peer_id]; |
| 952 | 965 |
if (!mi) |
| 953 | 966 |
{
|
| ... | ... |
@@ -958,14 +926,6 @@ ovpn_handle_peer(dco_context_t *dco, struct nlattr *attrs[]) |
| 958 | 958 |
c2 = &mi->context.c2; |
| 959 | 959 |
} |
| 960 | 960 |
|
| 961 |
- /* at this point this check should never fail for MP mode, |
|
| 962 |
- * but it's still fully valid for P2P mode |
|
| 963 |
- */ |
|
| 964 |
- if (c2->tls_multi->dco_peer_id != peer_id) |
|
| 965 |
- {
|
|
| 966 |
- return NL_SKIP; |
|
| 967 |
- } |
|
| 968 |
- |
|
| 969 | 961 |
dco_update_peer_stat(c2, tb_peer, peer_id); |
| 970 | 962 |
|
| 971 | 963 |
return NL_OK; |
| ... | ... |
@@ -1176,17 +1136,7 @@ ovpn_handle_msg(struct nl_msg *msg, void *arg) |
| 1176 | 1176 |
{
|
| 1177 | 1177 |
case OVPN_CMD_PEER_GET: |
| 1178 | 1178 |
{
|
| 1179 |
- /* this message is part of a peer list dump, hence triggered |
|
| 1180 |
- * by a MP/server instance |
|
| 1181 |
- */ |
|
| 1182 |
- if (nlh->nlmsg_flags & NLM_F_MULTI) |
|
| 1183 |
- {
|
|
| 1184 |
- return ovpn_handle_peer_multi(dco, attrs); |
|
| 1185 |
- } |
|
| 1186 |
- else |
|
| 1187 |
- {
|
|
| 1188 |
- return ovpn_handle_peer(dco, attrs); |
|
| 1189 |
- } |
|
| 1179 |
+ return ovpn_handle_peer(dco, attrs); |
|
| 1190 | 1180 |
} |
| 1191 | 1181 |
|
| 1192 | 1182 |
case OVPN_CMD_PEER_DEL_NTF: |
| ... | ... |
@@ -1221,52 +1171,32 @@ dco_do_read(dco_context_t *dco) |
| 1221 | 1221 |
return ovpn_nl_recvmsgs(dco, __func__); |
| 1222 | 1222 |
} |
| 1223 | 1223 |
|
| 1224 |
-int |
|
| 1225 |
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, |
|
| 1226 |
- const bool raise_sigusr1_on_err) |
|
| 1227 |
-{
|
|
| 1228 |
- msg(D_DCO_DEBUG, "%s", __func__); |
|
| 1229 |
- |
|
| 1230 |
- struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_PEER_GET); |
|
| 1231 |
- |
|
| 1232 |
- nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP; |
|
| 1233 |
- |
|
| 1234 |
- int ret = ovpn_nl_msg_send(dco, nl_msg, __func__); |
|
| 1235 |
- |
|
| 1236 |
- nlmsg_free(nl_msg); |
|
| 1237 |
- |
|
| 1238 |
- if (raise_sigusr1_on_err && ret < 0) |
|
| 1239 |
- {
|
|
| 1240 |
- msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer" |
|
| 1241 |
- "may have been deleted from the kernel without notifying " |
|
| 1242 |
- "userspace. Restarting the session"); |
|
| 1243 |
- register_signal(m->top.sig, SIGUSR1, "dco peer stats error"); |
|
| 1244 |
- } |
|
| 1245 |
- return ret; |
|
| 1246 |
-} |
|
| 1247 |
- |
|
| 1248 |
-int |
|
| 1249 |
-dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err) |
|
| 1224 |
+static int |
|
| 1225 |
+dco_get_peer(dco_context_t *dco, int peer_id, const bool raise_sigusr1_on_err) |
|
| 1250 | 1226 |
{
|
| 1251 |
- int peer_id = c->c2.tls_multi->dco_peer_id; |
|
| 1252 |
- if (peer_id == -1) |
|
| 1227 |
+ /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only. |
|
| 1228 |
+ * If it happens in P2P mode it means that the DCO peer was deleted and we |
|
| 1229 |
+ * can simply bail out |
|
| 1230 |
+ */ |
|
| 1231 |
+ if (peer_id == -1 && dco->ifmode == OVPN_MODE_P2P) |
|
| 1253 | 1232 |
{
|
| 1254 | 1233 |
return 0; |
| 1255 | 1234 |
} |
| 1256 | 1235 |
|
| 1257 | 1236 |
msg(D_DCO_DEBUG, "%s: peer-id %d", __func__, peer_id); |
| 1258 | 1237 |
|
| 1259 |
- if (!c->c1.tuntap) |
|
| 1260 |
- {
|
|
| 1261 |
- return 0; |
|
| 1262 |
- } |
|
| 1263 |
- |
|
| 1264 |
- dco_context_t *dco = &c->c1.tuntap->dco; |
|
| 1265 | 1238 |
struct nl_msg *nl_msg = ovpn_dco_nlmsg_create(dco, OVPN_CMD_PEER_GET); |
| 1266 | 1239 |
struct nlattr *attr = nla_nest_start(nl_msg, OVPN_A_PEER); |
| 1267 | 1240 |
int ret = -EMSGSIZE; |
| 1268 | 1241 |
|
| 1269 |
- NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id); |
|
| 1242 |
+ if (peer_id != -1) |
|
| 1243 |
+ {
|
|
| 1244 |
+ NLA_PUT_U32(nl_msg, OVPN_A_PEER_ID, peer_id); |
|
| 1245 |
+ } |
|
| 1246 |
+ else |
|
| 1247 |
+ {
|
|
| 1248 |
+ nlmsg_hdr(nl_msg)->nlmsg_flags |= NLM_F_DUMP; |
|
| 1249 |
+ } |
|
| 1270 | 1250 |
nla_nest_end(nl_msg, attr); |
| 1271 | 1251 |
|
| 1272 | 1252 |
ret = ovpn_nl_msg_send(dco, nl_msg, __func__); |
| ... | ... |
@@ -1279,11 +1209,23 @@ nla_put_failure: |
| 1279 | 1279 |
msg(M_WARN, "Error retrieving DCO peer stats: the underlying DCO peer" |
| 1280 | 1280 |
"may have been deleted from the kernel without notifying " |
| 1281 | 1281 |
"userspace. Restarting the session"); |
| 1282 |
- register_signal(c->sig, SIGUSR1, "dco peer stats error"); |
|
| 1282 |
+ register_signal(dco->c->sig, SIGUSR1, "dco peer stats error"); |
|
| 1283 | 1283 |
} |
| 1284 | 1284 |
return ret; |
| 1285 | 1285 |
} |
| 1286 | 1286 |
|
| 1287 |
+int |
|
| 1288 |
+dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err) |
|
| 1289 |
+{
|
|
| 1290 |
+ return dco_get_peer(&c->c1.tuntap->dco, c->c2.tls_multi->dco_peer_id, raise_sigusr1_on_err); |
|
| 1291 |
+} |
|
| 1292 |
+ |
|
| 1293 |
+int |
|
| 1294 |
+dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) |
|
| 1295 |
+{
|
|
| 1296 |
+ return dco_get_peer(dco, -1, raise_sigusr1_on_err); |
|
| 1297 |
+} |
|
| 1298 |
+ |
|
| 1287 | 1299 |
bool |
| 1288 | 1300 |
dco_available(int msglevel) |
| 1289 | 1301 |
{
|
| ... | ... |
@@ -715,8 +715,7 @@ dco_do_read(dco_context_t *dco) |
| 715 | 715 |
} |
| 716 | 716 |
|
| 717 | 717 |
int |
| 718 |
-dco_get_peer_stats_multi(dco_context_t *dco, struct multi_context *m, |
|
| 719 |
- const bool raise_sigusr1_on_err) |
|
| 718 |
+dco_get_peer_stats_multi(dco_context_t *dco, const bool raise_sigusr1_on_err) |
|
| 720 | 719 |
{
|
| 721 | 720 |
/* Not implemented. */ |
| 722 | 721 |
return 0; |
| ... | ... |
@@ -551,7 +551,7 @@ setenv_stats(struct multi_context *m, struct context *c) |
| 551 | 551 |
{
|
| 552 | 552 |
if (dco_enabled(&m->top.options)) |
| 553 | 553 |
{
|
| 554 |
- if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, false) < 0) |
|
| 554 |
+ if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, false) < 0) |
|
| 555 | 555 |
{
|
| 556 | 556 |
return; |
| 557 | 557 |
} |
| ... | ... |
@@ -862,7 +862,7 @@ multi_print_status(struct multi_context *m, struct status_output *so, const int |
| 862 | 862 |
|
| 863 | 863 |
if (dco_enabled(&m->top.options)) |
| 864 | 864 |
{
|
| 865 |
- if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, m, true) < 0) |
|
| 865 |
+ if (dco_get_peer_stats_multi(&m->top.c1.tuntap->dco, true) < 0) |
|
| 866 | 866 |
{
|
| 867 | 867 |
return; |
| 868 | 868 |
} |