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/+/1752?usp=email
to review the following change.
Change subject: oob: Add --server-probe-reply to advertise probe priority/weight
......................................................................
oob: Add --server-probe-reply to advertise probe priority/weight
Add a server option, --server-probe-reply <priority> <weight>, that sets
the DNS-SRV-style priority and weight the server returns in its OOB
PROBE_REPLY. Both default to 0 (the previous hardcoded behaviour), and are
range-checked to 0..65535.
The values are plumbed through oob_build_probe_reply() into the probe_reply
TLV; the client already reads and ranks remotes by them.
Change-Id: Id74cfae7e9d69029d2ddbf635ee85a2a6cedc3d8
Signed-off-by: Lev Stipakov <[email protected]>
---
M src/openvpn/mudp.c
M src/openvpn/oob.c
M src/openvpn/oob.h
M src/openvpn/options.c
M src/openvpn/options.h
M tests/unit_tests/openvpn/test_oob.c
6 files changed, 58 insertions(+), 15 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/52/1752/1
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 9bc2d9c..0e9414d 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -236,7 +236,11 @@
* tls-auth/tls-crypt wrapping). Answer it without creating a session.
*/
struct oob_probe_reply reply;
if (!oob_build_probe_reply(&state->newbuf, (uint64_t)now,
(uint64_t)handwindow,
- &state->peer_session_id, &reply))
+ &state->peer_session_id,
+
(uint16_t)m->top.options.server_probe_reply_priority,
+
(uint16_t)m->top.options.server_probe_reply_weight,
+
(uint16_t)m->top.options.server_probe_reply_max_latency_diff,
+ &reply))
{
/* malformed or replayed/stale probe: silently drop */
return false;
diff --git a/src/openvpn/oob.c b/src/openvpn/oob.c
index 9b359dd..42a618d 100644
--- a/src/openvpn/oob.c
+++ b/src/openvpn/oob.c
@@ -222,7 +222,8 @@
bool
oob_build_probe_reply(struct buffer *probe_payload, uint64_t now, uint64_t
window_secs,
- const struct session_id *peer_sid, struct
oob_probe_reply *reply)
+ const struct session_id *peer_sid, uint16_t priority,
uint16_t weight,
+ uint16_t max_latency_diff, struct oob_probe_reply *reply)
{
struct oob_probe_parameter param;
if (!oob_server_probe_read(probe_payload, ¶m))
@@ -238,7 +239,10 @@
memset(reply, 0, sizeof(*reply));
reply->peer_session_id = *peer_sid;
- /* priority/weight/connect_lifetime/flags left at 0 for now */
+ reply->priority = priority;
+ reply->weight = weight;
+ reply->max_latency_diff = max_latency_diff;
+ /* connect_lifetime/flags left at 0 for now */
return true;
}
diff --git a/src/openvpn/oob.h b/src/openvpn/oob.h
index 65e84bd..fcefb26 100644
--- a/src/openvpn/oob.h
+++ b/src/openvpn/oob.h
@@ -200,8 +200,8 @@
* answer it. Combines oob_server_probe_read() and oob_timestamp_in_window():
* the probe is dropped (false returned) if it has no valid probe_parameter or
* its timestamp is outside the acceptable window. On success @p reply is
- * populated with the peer's session id echoed back and the remaining fields
- * zeroed, ready to be wrapped and sent.
+ * populated with the peer's session id echoed back and the given priority and
+ * weight (other fields zeroed), ready to be wrapped and sent.
*
* This is the transport-agnostic decision step; the caller performs the send.
*
@@ -209,11 +209,15 @@
* @param now current time (UNIX seconds)
* @param window_secs acceptable timestamp skew, in either direction
* @param peer_sid session id of the requesting peer (echoed in the
reply)
+ * @param priority priority to advertise (DNS-SRV semantics; lower
preferred)
+ * @param weight weight to advertise (DNS-SRV semantics; higher
preferred)
+ * @param max_latency_diff candidate-band margin (ms) to advertise; 0 = defer
to client
* @param reply filled with the reply to send on success
* @return true if a reply should be sent, false to silently drop the probe
*/
bool oob_build_probe_reply(struct buffer *probe_payload, uint64_t now,
uint64_t window_secs,
- const struct session_id *peer_sid, struct
oob_probe_reply *reply);
+ const struct session_id *peer_sid, uint16_t
priority, uint16_t weight,
+ uint16_t max_latency_diff, struct oob_probe_reply
*reply);
/* Candidate-band margin (ms) used when neither the client nor the server
* specifies one. */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index d54e3ab..f86b438 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -804,10 +804,14 @@
o->ce.proto = PROTO_UDP;
o->ce.af = AF_UNSPEC;
- /* The client latency margin is -1 = "not set": the client's value is
- * authoritative when given, otherwise each server's advertised margin (or
- * the built-in default) applies. */
+ /* server-probe defaults. The client latency margin is -1 = "not set": the
+ * client's value is authoritative when given, otherwise each server's
+ * advertised margin (or the built-in default) applies. An (unconfigured)
+ * server advertises weight 50 / priority 100 and no margin (0). */
o->server_probe_latency_margin = -1;
+ o->server_probe_reply_weight = 50;
+ o->server_probe_reply_priority = 100;
+ o->server_probe_reply_max_latency_diff = 0;
o->ce.bind_ipv6_only = false;
o->ce.connect_retry_seconds = 1;
o->ce.connect_retry_seconds_max = 300;
@@ -6526,6 +6530,26 @@
options->server_probe_latency_margin = margin;
}
}
+ else if (streq(p[0], "server-probe-reply") && !p[4])
+ {
+ VERIFY_PERMISSION(OPT_P_GENERAL);
+ /* --server-probe-reply [max-latency-diff] [weight] [prio]; each
optional */
+ int vals[3] = { options->server_probe_reply_max_latency_diff,
+ options->server_probe_reply_weight,
+ options->server_probe_reply_priority };
+ for (int i = 0; i < 3 && p[i + 1]; i++)
+ {
+ vals[i] = positive_atoi(p[i + 1], msglevel);
+ if (vals[i] > 0xffff)
+ {
+ msg(msglevel, "--server-probe-reply: values must be 0 to
65535");
+ goto err;
+ }
+ }
+ options->server_probe_reply_max_latency_diff = vals[0];
+ options->server_probe_reply_weight = vals[1];
+ options->server_probe_reply_priority = vals[2];
+ }
else if (streq(p[0], "nice") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_NICE);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 6b1120a..6e4829d 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -339,6 +339,12 @@
/* client: default candidate-band margin in ms (--server-probe
[max-latency-diff]):
* servers within this RTT of the fastest are treated as equally fast */
int server_probe_latency_margin;
+ /* server: values advertised in the OOB PROBE_REPLY (--server-probe-reply).
+ * priority/weight follow DNS-SRV semantics; max_latency_diff overrides the
+ * client's margin for this server (0 = defer to the client). */
+ int server_probe_reply_priority;
+ int server_probe_reply_weight;
+ int server_probe_reply_max_latency_diff;
bool mlock;
diff --git a/tests/unit_tests/openvpn/test_oob.c
b/tests/unit_tests/openvpn/test_oob.c
index b66ade2..c3efbdf 100644
--- a/tests/unit_tests/openvpn/test_oob.c
+++ b/tests/unit_tests/openvpn/test_oob.c
@@ -285,7 +285,7 @@
}
/* A valid, in-window SERVER_PROBE yields a reply that echoes the peer's
- * session id and zeroes the remaining fields. */
+ * session id and carries the configured priority and weight. */
static void
test_build_probe_reply_valid(void **state)
{
@@ -300,12 +300,13 @@
memcpy(peer.id, "PEER1234", SID_SIZE);
struct oob_probe_reply reply;
- assert_true(oob_build_probe_reply(&buf, now, 30, &peer, &reply));
+ assert_true(oob_build_probe_reply(&buf, now, 30, &peer, 5, 50, 25,
&reply));
assert_memory_equal(reply.peer_session_id.id, peer.id, SID_SIZE);
- assert_int_equal(reply.priority, 0);
- assert_int_equal(reply.weight, 0);
+ assert_int_equal(reply.priority, 5);
+ assert_int_equal(reply.weight, 50);
assert_int_equal(reply.connect_lifetime, 0);
assert_int_equal(reply.flags, 0);
+ assert_int_equal(reply.max_latency_diff, 25);
gc_free(&gc);
}
@@ -323,7 +324,7 @@
struct session_id peer = { 0 };
struct oob_probe_reply reply;
- assert_false(oob_build_probe_reply(&buf, now, 30, &peer, &reply));
+ assert_false(oob_build_probe_reply(&buf, now, 30, &peer, 0, 0, 0, &reply));
gc_free(&gc);
}
@@ -341,7 +342,7 @@
struct session_id peer = { 0 };
struct oob_probe_reply reply;
- assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, &reply));
+ assert_false(oob_build_probe_reply(&buf, 1000000, 30, &peer, 0, 0, 0,
&reply));
gc_free(&gc);
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1752?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: Id74cfae7e9d69029d2ddbf635ee85a2a6cedc3d8
Gerrit-Change-Number: 1752
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
