unreachables: also consider local 'via' address

Knot.includes

 src
 src/knot/common
+tests
 tests/knot

src/knot/common/unreachable.c

@@ -14,12 +14,12 @@
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-#include "unreachable.h"
-
 #include <assert.h>
 #include <stdlib.h>
 #include <time.h>
 
+#include "unreachable.h"
+
 knot_unreachables_t *global_unreachables = NULL;
 
 static uint32_t get_timestamp(void)
@@ -60,7 +60,8 @@ static void clear_old(knot_unreachable_t *ur, uint32_t now, uint32_t ttl)
 // also clears up (some) expired unreachables
 // returns either match or free space
 static knot_unreachable_t *get_ur(knot_unreachables_t *urs,
-                                  const struct sockaddr_storage *addr)
+                                  const struct sockaddr_storage *addr,
+                                  const struct sockaddr_storage *via)
 {
 	assert(urs != NULL);
 
@@ -75,7 +76,7 @@ static knot_unreachable_t *get_ur(knot_unreachables_t *urs,
 			if (clear == NULL) {
 				clear = ur;
 			}
-		} else if (sockaddr_cmp(&ur->addr, addr, false) == 0) {
+		} else if (sockaddr_cmp_two(&ur->addr, &ur->via, addr, via) == 0) {
 			return ur;
 		} else if (oldest == NULL || ur->time < oldest->time) {
 			oldest = ur;
@@ -91,7 +92,8 @@ static knot_unreachable_t *get_ur(knot_unreachables_t *urs,
 }
 
 bool knot_unreachable_is(knot_unreachables_t *urs,
-                         const struct sockaddr_storage *addr)
+                         const struct sockaddr_storage *addr,
+                         const struct sockaddr_storage *via)
 {
 	if (urs == NULL) {
 		return false;
@@ -99,7 +101,7 @@ bool knot_unreachable_is(knot_unreachables_t *urs,
 
 	pthread_mutex_lock(&urs->mutex);
 
-	bool res = (get_ur(urs, addr)->time != 0);
+	bool res = (get_ur(urs, addr, via)->time != 0);
 
 	pthread_mutex_unlock(&urs->mutex);
 
@@ -107,7 +109,8 @@ bool knot_unreachable_is(knot_unreachables_t *urs,
 }
 
 void knot_unreachable_add(knot_unreachables_t *urs,
-                          const struct sockaddr_storage *addr)
+                          const struct sockaddr_storage *addr,
+                          const struct sockaddr_storage *via)
 {
 
 	if (urs == NULL) {
@@ -116,9 +119,10 @@ void knot_unreachable_add(knot_unreachables_t *urs,
 
 	pthread_mutex_lock(&urs->mutex);
 
-	knot_unreachable_t *ur = get_ur(urs, addr);
+	knot_unreachable_t *ur = get_ur(urs, addr, via);
 	if (ur->time == 0) {
 		memcpy(&ur->addr, addr, sizeof(ur->addr));
+		memcpy(&ur->via, via, sizeof(ur->via));
 	}
 	ur->time = get_timestamp();
 

src/knot/common/unreachable.h

@@ -26,6 +26,7 @@
 
 typedef struct {
 	struct sockaddr_storage addr;
+	struct sockaddr_storage via;
 	uint32_t time;
 } knot_unreachable_t;
 
@@ -56,17 +57,21 @@ void knot_unreachables_deinit(knot_unreachables_t **urs);
  *
  * \param urs     Unreachables structure.
  * \param addr    Address and port in question.
+ * \param via     Local outgoing address.
  *
  * \return True iff unreachable within TTL.
  */
 bool knot_unreachable_is(knot_unreachables_t *urs,
-                         const struct sockaddr_storage *addr);
+                         const struct sockaddr_storage *addr,
+                         const struct sockaddr_storage *via);
 
 /*!
  * \brief Add an unreachable into Unreachables structure.
  *
  * \param urs     Unreachables structure.
  * \param addr    Address and port being unreachable.
+ * \param via     Local outgoing address.
  */
 void knot_unreachable_add(knot_unreachables_t *urs,
-                          const struct sockaddr_storage *addr);
+                          const struct sockaddr_storage *addr,
+                          const struct sockaddr_storage *via);

src/knot/query/requestor.c

@@ -52,7 +52,8 @@ static int request_ensure_connected(knot_request_t *request)
 			return KNOT_EOK;
 		}
 
-		if (knot_unreachable_is(global_unreachables, &request->remote)) {
+		if (knot_unreachable_is(global_unreachables, &request->remote,
+		                        &request->source)) {
 			return KNOT_EUNREACH;
 		}
 	}
@@ -62,8 +63,9 @@ static int request_ensure_connected(knot_request_t *request)
 	                                   &request->source,
 	                                   request->flags & KNOT_REQUEST_TFO);
 	if (request->fd < 0) {
-		if (request->fd == KNOT_ETIMEOUT) { // this never happens when I try
-			knot_unreachable_add(global_unreachables, &request->remote);
+		if (request->fd == KNOT_ETIMEOUT) {
+			knot_unreachable_add(global_unreachables, &request->remote,
+			                     &request->source);
 		}
 		return request->fd;
 	}
@@ -90,8 +92,9 @@ static int request_send(knot_request_t *request, int timeout_ms)
 	if (use_tcp(request)) {
 		ret = net_dns_tcp_send(request->fd, wire, wire_len, timeout_ms,
 		                       tfo_addr);
-		if (ret == KNOT_ETIMEOUT) { // this includes the case when establishing the conn times out
-			knot_unreachable_add(global_unreachables, &request->remote);
+		if (ret == KNOT_ETIMEOUT) { // Includes establishing conn which times out.
+			knot_unreachable_add(global_unreachables, &request->remote,
+			                     &request->source);
 		}
 	} else {
 		ret = net_dgram_send(request->fd, wire, wire_len, NULL);

tests/knot/test_unreachable.c

@@ -14,12 +14,15 @@
     along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
+#include <tap/basic.h>
+
 #include "knot/common/unreachable.h"
 
-#include <tap/basic.h>
+#include "contrib/sockaddr.h"
 
 #define UR_TEST_ADDRS 32
 struct sockaddr_storage ur_test_addrs[UR_TEST_ADDRS] = { 0 };
+struct sockaddr_storage ur_test_via[2] = { 0 };
 
 int main(int argc, char *argv[])
 {
@@ -28,33 +31,29 @@ int main(int argc, char *argv[])
 	global_unreachables = knot_unreachables_init(1000);
 	ok(global_unreachables != NULL, "unreachables: init");
 
+	// ur_test_via[0] left empty - AF_UNSPEC
+	sockaddr_set(&ur_test_via[1], AF_INET6, "::1", 0);
+
 	for (int i = 0; i < UR_TEST_ADDRS; i++) {
 		struct sockaddr_storage *s = &ur_test_addrs[i];
-		struct sockaddr_in6 *s6 = (struct sockaddr_in6 *)s;
-		s6->sin6_family = AF_INET6;
-		s6->sin6_port = i + 1;
+		sockaddr_set(s, AF_INET6, "::2", i + 1);
+		struct sockaddr_storage *via = &ur_test_via[i % 2];
+		struct sockaddr_storage *not_via = &ur_test_via[1 - i % 2];
 
-		ok(!knot_unreachable_is(global_unreachables, s), "unreachables: pre[%d]", i);
-		knot_unreachable_add(global_unreachables, s);
-		ok(knot_unreachable_is(global_unreachables, s), "unreachables: post[%d]", i);
+		ok(!knot_unreachable_is(global_unreachables, s, via), "unreachables: pre[%d]", i);
+		knot_unreachable_add(global_unreachables, s, via);
+		ok(knot_unreachable_is(global_unreachables, s, via), "unreachables: post[%d]", i);
+		ok(!knot_unreachable_is(global_unreachables, s, not_via), "unreachables: via[%d]", i);
 
 		usleep(100);
 		if (i >= 10) {
-			ok(!knot_unreachable_is(global_unreachables, &ur_test_addrs[i - 10]), "unreachables: expired[%d]", i - 10);
-		}
-	}
-	usleep(1000);
-
-	for (int i = 0; i < UR_TEST_ADDRS; i++) {
-		knot_unreachable_add(global_unreachables, &ur_test_addrs[i]);
-
-		usleep(10);
-		if (i >= KNOT_UNREACHABLE_COUNT) {
-			ok(!knot_unreachable_is(global_unreachables, &ur_test_addrs[i - KNOT_UNREACHABLE_COUNT]), "unreachables: overfill[%d]", i - 10);
+			ok(!knot_unreachable_is(global_unreachables, &ur_test_addrs[i - 10], via),
+			   "unreachables: expired[%d]", i - 10);
 		}
 	}
 
 	knot_unreachables_deinit(&global_unreachables);
 	ok(global_unreachables == NULL, "unreachables: deinit");
+
 	return 0;
 }