@@ -38,8 +38,9 @@
#include "memdbg.h"
-/* calculates test - base while allowing for base or test wraparound. test is
- * assumed to be higher than base */
+/* calculates test - base while allowing for base or test wraparound. The
+ * result is only meaningful compared against a bound: on its own it does not
+ * say which id came first. */
static inline packet_id_type
subtract_pid(const packet_id_type test, const packet_id_type base)
{
@@ -356,12 +357,11 @@
return true;
}
-int
-validate_packet_id_window(struct reliable *rel, packet_id_type pid)
+bool
+reliable_pid_in_send_window(const struct reliable *rel, packet_id_type pid)
{
- const packet_id_type dist = subtract_pid(rel->packet_id, pid);
-
- return dist > 0 && dist < RELIABLE_CAPACITY;
+ return reliable_pid_in_range(pid, subtract_pid(rel->packet_id, RELIABLE_CAPACITY),
+ RELIABLE_CAPACITY);
}
/* del acknowledged items from send buf */
@@ -376,7 +376,7 @@
packet_id_type pid = ack->packet_id[i];
- if (!validate_packet_id_window(rel, pid))
+ if (!reliable_pid_in_send_window(rel, pid))
{
if (out_of_window == 0)
{
@@ -566,7 +566,7 @@
}
int
-reliable_get_num_output_sequenced_available(struct reliable *rel)
+reliable_get_num_output_sequenced_available(const struct reliable *rel)
{
return rel->size - (int)reliable_oldest_active_distance(rel);
}
@@ -190,15 +190,20 @@
}
/**
- * check that pid is inside the window of possible outstanding packets
- * of size RELIABLE_CAPACITY, ie inside the range
- * [rel->packet_id - RELIABLE_CAPACITY, rel->packet_id).
+ * Check whether pid is one of the RELIABLE_CAPACITY ids below rel->packet_id,
+ * and so could still be outstanding. rel->packet_id is the *next* id to be
+ * sent, so it is not itself in the window.
*
- * rel->packet is the *next* packet id to be sent out, so it is not
- * included in the valid range.
+ * pid comes off the wire; all 2^32 values are handled.
+ *
+ * @param rel The reliable structure holding this tunnel's sent packets.
+ * @param pid A packet ID from a received acknowledgment.
+ *
+ * @return
+ * @li True, if pid is inside the window.
+ * @li False, otherwise.
*/
-int
-validate_packet_id_window(struct reliable *rel, packet_id_type pid);
+bool reliable_pid_in_send_window(const struct reliable *rel, packet_id_type pid);
/**
* Returns the number of packets that need to be acked.
@@ -305,14 +310,15 @@
* Check that a received packet's ID can safely be stored in
* the reliable structure's processing window.
*
- * This function checks the difference between the received packet's ID
- * and the lowest non-acknowledged packet ID in the given reliable
- * structure. If that difference is larger than the total number of
- * packets which can be stored, then this packet cannot be stored safely,
- * because the reliable structure could possibly fill up without leaving
- * room for all intervening packets. In that case, this received packet
- * could break the reliable structure's sequentiality, and must therefore
- * be discarded.
+ * Checks the received packet's ID against rel->packet_id, the next ID the
+ * reliability layer expects to hand upwards. One too far ahead could fill the
+ * structure without leaving room for the intervening packets, so it is
+ * discarded.
+ *
+ * IDs numerically below rel->packet_id are accepted here, an absolute
+ * comparison that an ID from before a wraparound does not pass.
+ * reliable_not_replay() rejects them, and they are still acknowledged so a
+ * peer whose ACK was lost stops retransmitting.
*
* @param rel The reliable structure for handling this VPN tunnel's
* received packets.
@@ -354,6 +360,10 @@
* Mark the %reliable entry associated with the given buffer as active
* incoming.
*
+ * pid must be less than rel->size ahead of rel->packet_id. This is asserted,
+ * so run reliable_wont_break_sequentiality() and reliable_not_replay()
+ * first.
+ *
* @param rel The reliable structure associated with this packet.
* @param buf The buffer into which the packet has been copied.
* @param pid The packet's packet ID.
@@ -446,7 +456,7 @@
* @return the number of buffer that are available for sending without
* breaking ack sequence
* */
-int reliable_get_num_output_sequenced_available(struct reliable *rel);
+int reliable_get_num_output_sequenced_available(const struct reliable *rel);
/**
* Mark the reliable entry associated with the given buffer as
@@ -331,44 +331,50 @@
struct reliable rel = { 0 };
rel.packet_id = 1;
- assert_true(validate_packet_id_window(&rel, 0));
+ assert_true(reliable_pid_in_send_window(&rel, 0));
/* packet id 1 is outside the window as it is the *next* packet id */
- assert_false(validate_packet_id_window(&rel, 1));
+ assert_false(reliable_pid_in_send_window(&rel, 1));
/* wrapped around packet id, "-2" */
- assert_true(validate_packet_id_window(&rel, 0xFFFFFFFD));
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFFD));
/* wrapped around packet id, "-10" */
- assert_true(validate_packet_id_window(&rel, 0xFFFFFFF6));
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFF6));
- /* wrapped around packet id, "-11" */
- assert_false(validate_packet_id_window(&rel, 0xFFFFFFF5));
- assert_false(validate_packet_id_window(&rel, 0x80000000));
+ /* wrapped around packet id, "-11": the oldest id still in the window */
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFF5));
+
+ /* one further back is outside it */
+ assert_false(reliable_pid_in_send_window(&rel, 0xFFFFFFF4));
+ assert_false(reliable_pid_in_send_window(&rel, 0x80000000));
rel.packet_id = 0x80000000;
/* near the signed/usigned integer area */
- assert_false(validate_packet_id_window(&rel, 0x80000001));
- assert_true(validate_packet_id_window(&rel, 0x7fffffff));
- assert_true(validate_packet_id_window(&rel, 0x7ffffff5));
- assert_false(validate_packet_id_window(&rel, 0x7ffffff4));
+ assert_false(reliable_pid_in_send_window(&rel, 0x80000001));
+ assert_true(reliable_pid_in_send_window(&rel, 0x7fffffff));
+ assert_true(reliable_pid_in_send_window(&rel, 0x7ffffff5));
+ assert_true(reliable_pid_in_send_window(&rel, 0x7ffffff4));
+ assert_false(reliable_pid_in_send_window(&rel, 0x7ffffff3));
rel.packet_id = 0xFFFFFFFD;
- assert_false(validate_packet_id_window(&rel, 0xFFFFFFFD));
- assert_false(validate_packet_id_window(&rel, 0));
- assert_false(validate_packet_id_window(&rel, 1));
- assert_false(validate_packet_id_window(&rel, 0xFFFFFFFE));
- assert_false(validate_packet_id_window(&rel, 0xFFFFFFFF));
- assert_true(validate_packet_id_window(&rel, 0xFFFFFFF3));
- assert_true(validate_packet_id_window(&rel, 0xFFFFFFF2));
- assert_false(validate_packet_id_window(&rel, 0xFFFFFFF1));
+ assert_false(reliable_pid_in_send_window(&rel, 0xFFFFFFFD));
+ assert_false(reliable_pid_in_send_window(&rel, 0));
+ assert_false(reliable_pid_in_send_window(&rel, 1));
+ assert_false(reliable_pid_in_send_window(&rel, 0xFFFFFFFE));
+ assert_false(reliable_pid_in_send_window(&rel, 0xFFFFFFFF));
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFF3));
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFF2));
+ assert_true(reliable_pid_in_send_window(&rel, 0xFFFFFFF1));
+ assert_false(reliable_pid_in_send_window(&rel, 0xFFFFFFF0));
rel.packet_id = 500;
- assert_false(validate_packet_id_window(&rel, 501));
- assert_true(validate_packet_id_window(&rel, 497));
- assert_true(validate_packet_id_window(&rel, 500 - (RELIABLE_CAPACITY - 1)));
- assert_false(validate_packet_id_window(&rel, 500 - RELIABLE_CAPACITY));
+ assert_false(reliable_pid_in_send_window(&rel, 501));
+ assert_true(reliable_pid_in_send_window(&rel, 497));
+ assert_true(reliable_pid_in_send_window(&rel, 500 - (RELIABLE_CAPACITY - 1)));
+ assert_true(reliable_pid_in_send_window(&rel, 500 - RELIABLE_CAPACITY));
+ assert_false(reliable_pid_in_send_window(&rel, 500 - (RELIABLE_CAPACITY + 1)));
}
@@ -533,21 +539,19 @@
return (packet_id_type)(p1 - p2) >= 0x80000000u;
}
-/* one short of RELIABLE_CAPACITY, which is what the code accepts today */
-#define CHARACTERIZED_SEND_WINDOW 11
-
/* TLS_RELIABLE_N_SEND_BUFFERS and P_CONTROL_V1, from ssl_pkt.h, which this
* test binary does not pull in */
#define CHAR_N_SEND_BUFFERS 6
#define CHAR_OPCODE_CONTROL_V1 4
-/* preserved: the ids validate_packet_id_window() accepts */
+/* introduced: the RELIABLE_CAPACITY ids below rel->packet_id.
+ * validate_packet_id_window() stopped one short of that. */
static bool
ref_pid_in_send_window(const struct reliable *rel, packet_id_type pid)
{
const packet_id_type dist = (packet_id_type)(rel->packet_id - pid);
- return dist >= 1 && dist <= CHARACTERIZED_SEND_WINDOW;
+ return dist >= 1 && dist <= RELIABLE_CAPACITY;
}
/* preserved: reliable_pid_in_range2() */
@@ -798,7 +802,7 @@
rel.packet_id = anchors[a];
assert_int_equal(ref_pid_in_send_window(&rel, ids[i]),
- validate_packet_id_window(&rel, ids[i]) != 0);
+ reliable_pid_in_send_window(&rel, ids[i]));
}
}
}