Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1751?usp=email
to review the following change.
Change subject: oob: Probe every resolved address of a remote
......................................................................
oob: Probe every resolved address of a remote
A remote can resolve to several A/AAAA records; --server-probe previously
sent a SERVER_PROBE only to the first one, so a remote whose first address
was unreachable looked unresponsive even if another address answered, and
the connection could later try an address that was never probed.
Probe every resolved address of each remote in parallel (per-remote storage
is sized to the resolved address count, no fixed cap), accept a reply from
any of them (first reply for a remote wins), and resend to all of a remote's
addresses while it is unanswered.
Change-Id: I00208610e0ce4f3ed9a87233e68ece1b3b8a768a
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/oob_client.c
1 file changed, 85 insertions(+), 33 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/51/1751/1
diff --git a/src/openvpn/oob_client.c b/src/openvpn/oob_client.c
index 5f22650..7391d1e 100644
--- a/src/openvpn/oob_client.c
+++ b/src/openvpn/oob_client.c
@@ -52,11 +52,15 @@
#define OOB_PROBE_RETRIES 1
/* Where we sent a probe, so a reply's source address can be matched back to
the
- * connection-list entry it belongs to. */
+ * connection-list entry it belongs to. A remote may resolve to several
+ * addresses, all of which are probed; a reply from any of them counts. The
+ * @p dests / @p destlens arrays are gc-allocated, sized to the remote's
resolved
+ * address count. */
struct probe_target
{
- struct sockaddr_storage dest;
- socklen_t destlen;
+ struct sockaddr_storage *dests;
+ socklen_t *destlens;
+ int n_dests;
bool sent;
struct timeval sent_at; /* when the probe was sent, for RTT measurement */
};
@@ -173,6 +177,25 @@
return 0;
}
+/* Send the probe to every resolved address of @p t. Returns true if at least
+ * one send succeeded. */
+static bool
+oob_probe_send_target(socket_descriptor_t sd, const struct buffer *probe,
+ const struct probe_target *t)
+{
+ bool any_sent = false;
+ for (int k = 0; k < t->n_dests; k++)
+ {
+ if (sendto(sd, (const char *)BPTR(probe), (int)BLEN(probe), 0,
+ (const struct sockaddr *)&t->dests[k], t->destlens[k])
+ >= 0)
+ {
+ any_sent = true;
+ }
+ }
+ return any_sent;
+}
+
/* Parse one received datagram as a PROBE_REPLY and, if valid and matching one
* of the probes we sent, record the reply in @p results. */
static void
@@ -213,25 +236,38 @@
return;
}
- /* Match the reply's source address to the remote we probed. */
+ /* Match the reply's source address to one of the addresses we probed for a
+ * remote. The first reply for a remote wins (a remote with several
addresses
+ * may answer from more than one). */
for (int i = 0; i < n; i++)
{
- if (targets[i].sent
- && addr_port_match((const struct openvpn_sockaddr *)(const void
*)from,
- (const struct openvpn_sockaddr *)(const void
*)&targets[i].dest))
+ if (!targets[i].sent || results[i].responded)
{
- struct timeval rcv;
- openvpn_gettimeofday(&rcv, NULL);
- long ms = (long)(rcv.tv_sec - targets[i].sent_at.tv_sec) * 1000
- + (rcv.tv_usec - targets[i].sent_at.tv_usec) / 1000;
-
- results[i].responded = true;
- results[i].priority = reply.priority;
- results[i].weight = reply.weight;
- results[i].max_latency_diff = reply.max_latency_diff;
- results[i].rtt_ms = (ms > 0) ? (unsigned int)ms : 0;
- break;
+ continue;
}
+
+ bool match = false;
+ for (int k = 0; k < targets[i].n_dests && !match; k++)
+ {
+ match = addr_port_match((const struct openvpn_sockaddr *)(const
void *)from,
+ (const struct openvpn_sockaddr *)(const
void *)&targets[i].dests[k]);
+ }
+ if (!match)
+ {
+ continue;
+ }
+
+ struct timeval rcv;
+ openvpn_gettimeofday(&rcv, NULL);
+ long ms = (long)(rcv.tv_sec - targets[i].sent_at.tv_sec) * 1000
+ + (rcv.tv_usec - targets[i].sent_at.tv_usec) / 1000;
+
+ results[i].responded = true;
+ results[i].priority = reply.priority;
+ results[i].weight = reply.weight;
+ results[i].max_latency_diff = reply.max_latency_diff;
+ results[i].rtt_ms = (ms > 0) ? (unsigned int)ms : 0;
+ break;
}
}
@@ -306,8 +342,7 @@
{
if (targets[i].sent && !results[i].responded)
{
- sendto(sd, (const char *)BPTR(probe), (int)BLEN(probe), 0,
- (const struct sockaddr *)&targets[i].dest,
targets[i].destlen);
+ oob_probe_send_target(sd, probe, &targets[i]);
}
}
}
@@ -476,26 +511,43 @@
continue;
}
- socklen_t destlen = oob_probe_make_dest(ai, sock_af, &targets[i].dest);
- if (destlen == 0)
+ /* Collect every address this remote resolves to that the probe socket
+ * can reach. Storage is sized to the resolved address count. */
+ struct probe_target *t = &targets[i];
+ int n_addr = 0;
+ for (const struct addrinfo *a = ai; a; a = a->ai_next)
+ {
+ n_addr++;
+ }
+ t->dests = gc_malloc(sizeof(*t->dests) * n_addr, true, &gc);
+ t->destlens = gc_malloc(sizeof(*t->destlens) * n_addr, true, &gc);
+ for (const struct addrinfo *a = ai; a; a = a->ai_next)
+ {
+ socklen_t destlen = oob_probe_make_dest(a, sock_af,
&t->dests[t->n_dests]);
+ if (destlen > 0)
+ {
+ t->destlens[t->n_dests] = destlen;
+ t->n_dests++;
+ }
+ }
+ freeaddrinfo(ai);
+
+ if (t->n_dests == 0)
{
msg(D_LOW, "server-probe: %s:%s: not reachable by the probe
socket", ce->remote,
ce->remote_port);
+ continue;
}
- else if (sendto(sd, (const char *)BPTR(&probe), (int)BLEN(&probe), 0,
- (struct sockaddr *)&targets[i].dest, destlen)
- < 0)
+
+ if (!oob_probe_send_target(sd, &probe, t))
{
msg(D_LOW, "server-probe: %s:%s: probe send failed", ce->remote,
ce->remote_port);
+ continue;
}
- else
- {
- openvpn_gettimeofday(&targets[i].sent_at, NULL);
- targets[i].destlen = destlen;
- targets[i].sent = true;
- sent_count++;
- }
- freeaddrinfo(ai);
+
+ openvpn_gettimeofday(&t->sent_at, NULL);
+ t->sent = true;
+ sent_count++;
}
if (sent_count > 0)
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1751?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I00208610e0ce4f3ed9a87233e68ece1b3b8a768a
Gerrit-Change-Number: 1751
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
