Attention is currently required from: plaisthos, ralf_lici, stipa.
Hello plaisthos, ralf_lici,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1748?usp=email
to look at the new patch set (#22).
The following approvals got outdated and were removed:
Code-Review-1 by ralf_lici
Change subject: oob: Measure probe RTT for candidate-band selection
......................................................................
oob: Measure probe RTT for candidate-band selection
Record each probe's round-trip time (per-target send timestamp vs reply
arrival) and feed it to oob_rank_probe_results(), where RTT forms the
candidate band: responders within the effective margin of the fastest
compete on weight, slower ones fall behind. Log the measured RTT and the
effective margin (with its source: client or server-advertised) at verb
4.
Before this the client never measured RTT, so every responder sat in the
band and selection was purely weighted-random; with real measurements
the latency-based candidate selection takes effect.
The timestamp is taken at the first send. A reply carries nothing that
ties it to a send round, so measuring from a resend would report a
server slower than one round as fast, and could rank it ahead of a
genuinely faster one. Entries that resolve to the same address share the
send time of the single probe.
The RTT therefore runs from the first send to the moment the client
reads the reply: it compares servers, it is not a network measurement. A
probe that needed a resend reads high; the resend exists so that a
dropped probe does not make a live server look dead, not to refine the
measurement.
Change-Id: I375a41749060b7d006647bc27002c55436c8a8bb
Signed-off-by: Lev Stipakov <[email protected]>
---
M doc/man-sections/client-options.rst
M src/openvpn/oob.h
M src/openvpn/oob_client.c
3 files changed, 62 insertions(+), 17 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/48/1748/22
diff --git a/doc/man-sections/client-options.rst
b/doc/man-sections/client-options.rst
index 6eff520..766e3bc 100644
--- a/doc/man-sections/client-options.rst
+++ b/doc/man-sections/client-options.rst
@@ -608,9 +608,14 @@
remote, and each answering server replies with its advertised priority
and weight. Remotes are then reordered following DNS SRV (RFC 2782)
semantics: servers that answered are tried before those that did not,
- grouped by priority (lowest first); within a priority group, servers are
- picked by weighted-random selection. Round-trip time is not yet taken
- into account, so ``max-latency-diff`` has no effect for now.
+ grouped by priority (lowest first); within a priority group, servers
+ whose measured round-trip time is within ``max-latency-diff``
+ milliseconds of the fastest one are picked by weighted-random
+ selection, the others follow in round-trip-time order. When
+ ``max-latency-diff`` is not given, each answering server's own
+ advertised margin decides whether that server stays a candidate, so a
+ server advertising :code:`0` asks to be considered only while it ties
+ the fastest of its priority group, without constraining the others.
The probe is currently sent without control-channel wrapping, so it only
works against a server configured without ``--tls-auth``,
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 2eb8d4f..ebdcd3f 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -196,6 +196,7 @@
struct openvpn_sockaddr dest;
socklen_t destlen;
bool sent;
+ struct timeval sent_at; /* when the first probe went out, for the RTT */
};
/**
diff --git a/src/openvpn/oob_client.c b/src/openvpn/oob_client.c
index 16217c7..61e2602 100644
--- a/src/openvpn/oob_client.c
+++ b/src/openvpn/oob_client.c
@@ -60,9 +60,11 @@
struct probe_ctx
{
socket_descriptor_t sd[PROBE_AF_COUNT]; /* SOCKET_UNDEFINED if that AF is
unavailable */
- /* Addresses probed so far in the current send round: an address several
- * entries resolve to is probed once, and its reply is credited to each. */
+ /* Addresses probed so far in the current send round, with the time each
+ * went out: an address several entries resolve to is probed once, and its
+ * reply is credited to each of them with the same RTT. */
struct openvpn_sockaddr *probed;
+ struct timeval *probed_at;
int n_probed;
#ifdef TARGET_ANDROID
bool fd_protected[PROBE_AF_COUNT]; /* VPNService protect(), once per
socket */
@@ -277,10 +279,18 @@
/* Credit the reply to every still-unanswered remote probed at its source
* address: several entries can resolve to the same address, and each takes
* the first reply for it. */
+ struct timeval rcv;
+ openvpn_gettimeofday(&rcv, NULL);
int i = oob_probe_next_target_at(from, targets, results, n, 0);
while (i >= 0)
{
+ /* From this round's send to now, i.e. including any time the reply
+ * waited in the receive queue while we were still sending. */
+ 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].rtt_ms = (ms > 0) ? (unsigned int)ms : 0;
results[i].reply = reply;
i = oob_probe_next_target_at(from, targets, results, n, i + 1);
@@ -368,23 +378,34 @@
/* Resend the probe to every remote that we probed but that has not answered.
*/
static void
oob_probe_resend_unanswered(struct probe_ctx *pc, const struct buffer *probe,
- const struct oob_probe_target *targets,
+ struct oob_probe_target *targets,
const struct oob_probe_result *results, int n)
{
- pc->n_probed = 0; /* a new round: probe each unanswered address once again
*/
+ /* A new round: every unanswered address is probed once more. The send time
+ * is left alone, so the RTT keeps measuring from the first send. */
+ pc->n_probed = 0;
for (int i = 0; i < n; i++)
{
- if (!targets[i].sent || results[i].responded
- || oob_addr_list_find(pc->probed, pc->n_probed, &targets[i].dest)
>= 0)
+ if (!targets[i].sent || results[i].responded)
+ {
+ continue;
+ }
+ if (oob_addr_list_find(pc->probed, pc->n_probed, &targets[i].dest) >=
0)
{
continue;
}
socket_descriptor_t sd =
pc->sd[probe_af_index(targets[i].dest.addr.sa.sa_family)];
- if (sd != SOCKET_UNDEFINED
- && sendto(sd, (const char *)CBPTR(probe), (int)BLEN(probe), 0,
- (const struct sockaddr *)&targets[i].dest,
targets[i].destlen)
- >= 0)
+ if (sd == SOCKET_UNDEFINED)
{
+ continue;
+ }
+ struct timeval sent_at;
+ openvpn_gettimeofday(&sent_at, NULL);
+ if (sendto(sd, (const char *)CBPTR(probe), (int)BLEN(probe), 0,
+ (const struct sockaddr *)&targets[i].dest,
targets[i].destlen)
+ >= 0)
+ {
+ pc->probed_at[pc->n_probed] = sent_at;
pc->probed[pc->n_probed++] = targets[i].dest;
}
}
@@ -397,7 +418,7 @@
* once the window elapses or every sent probe has been answered. */
static void
oob_probe_collect(struct probe_ctx *pc, const struct buffer *probe,
- const struct session_id *client_sid, const struct
oob_probe_target *targets,
+ const struct session_id *client_sid, struct oob_probe_target
*targets,
struct oob_probe_result *results, int n,
const struct signal_info *sig)
{
@@ -572,6 +593,7 @@
* An address several entries resolve to is probed once; its reply is
* credited to each of them. */
pc.probed = gc_malloc(sizeof(*pc.probed) * l->len, false, &gc);
+ pc.probed_at = gc_malloc(sizeof(*pc.probed_at) * l->len, false, &gc);
pc.n_probed = 0;
int sent_count = 0;
@@ -598,8 +620,16 @@
pc.fd_protected[af] = true;
}
#endif
- if (oob_addr_list_find(pc.probed, pc.n_probed, &targets[i].dest) < 0)
+ const int seen = oob_addr_list_find(pc.probed, pc.n_probed,
&targets[i].dest);
+ if (seen >= 0)
{
+ /* Already probed this round by an earlier entry: reuse that send
+ * time, so both entries report the same RTT for the one reply. */
+ targets[i].sent_at = pc.probed_at[seen];
+ }
+ else
+ {
+ openvpn_gettimeofday(&targets[i].sent_at, NULL);
if (sendto(sd, (const char *)BPTR(&probe), (int)BLEN(&probe), 0,
(struct sockaddr *)&targets[i].dest, targets[i].destlen)
< 0)
@@ -607,6 +637,7 @@
msg(D_LOW, "server-probe: %s:%s: probe send failed",
ce->remote, ce->remote_port);
continue;
}
+ pc.probed_at[pc.n_probed] = targets[i].sent_at;
pc.probed[pc.n_probed++] = targets[i].dest;
}
targets[i].sent = true;
@@ -632,8 +663,16 @@
if (results[i].responded)
{
responded++;
- msg(D_LOW, "server-probe: %s:%s answered (priority %d, weight
%d)", ce->remote,
- ce->remote_port, results[i].reply.priority,
results[i].reply.weight);
+ /* Effective candidate-band margin and where it came from: the
+ * client's own setting wins, else the server's advertised value.
*/
+ int client_margin = c->options.server_probe_latency_margin;
+ int margin = oob_effective_margin(&results[i], client_margin);
+ const char *margin_src = client_margin >= 0 ? "client" :
"server-advertised";
+ msg(D_LOW,
+ "server-probe: %s:%s answered (priority %d, weight %d, rtt %u
ms;"
+ " latency margin %d ms [%s])",
+ ce->remote, ce->remote_port, results[i].reply.priority,
results[i].reply.weight,
+ results[i].rtt_ms, margin, margin_src);
}
else
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1748?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I375a41749060b7d006647bc27002c55436c8a8bb
Gerrit-Change-Number: 1748
Gerrit-PatchSet: 22
Gerrit-Owner: stipa <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-Reviewer: ralf_lici <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: ralf_lici <[email protected]>
Gerrit-Attention: stipa <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
