Specify which address turned out untranslatable

The code reuses the translation function for both source and
destination, and since the error strings were constant, specifying
the offending address during the logs would have led to significant
clutter.

Or so I thought. But the situation looks different now that I'm reading
the code with fresher eyes.

Helps debug for troubleshooting along the lines of #411.

src/mod/common/address_xlat.c

@@ -27,7 +27,7 @@ static struct addrxlat_result programming_error(void)
 {
 	struct addrxlat_result result;
 	result.verdict = ADDRXLAT_DROP;
-	result.reason = "Programming error";
+	result.reason = "Programming error.";
 	return result;
 }
 
@@ -40,7 +40,7 @@ struct addrxlat_result addrxlat_siit64(struct xlator *instance,
 
 	if (is_illegal_source(in)) {
 		result.verdict = ADDRXLAT_ACCEPT;
-		result.reason = "IPv6 source address (::1) is illegal (according to RFC 7915)";
+		result.reason = "IPv6 source address (::1) is illegal (according to RFC 7915).";
 		return result;
 	}
 
@@ -52,22 +52,22 @@ struct addrxlat_result addrxlat_siit64(struct xlator *instance,
 
 	if (!instance->globals.pool6.set || rfc6052_6to4(&instance->globals.pool6.prefix, in, out)) {
 		result.verdict = ADDRXLAT_TRY_SOMETHING_ELSE;
-		result.reason = "The input address lacks both pool6 prefix and EAM";
+		result.reason = "Address lacks both pool6 prefix and EAM.";
 		return result;
 	}
 
 	if (enable_denylists && denylist4_contains(instance->siit.denylist4,
 			&out->addr)) {
 		result.verdict = ADDRXLAT_ACCEPT;
 		/* No, that's not a typo. */
-		result.reason = "The resulting address (%pI4) is denylist4ed";
+		result.reason = "Address is denylist4ed.";
 		return result;
 	}
 
 success:
 	if (enable_denylists && must_not_translate(&out->addr, instance->ns)) {
 		result.verdict = ADDRXLAT_ACCEPT;
-		result.reason = "The resulting address is subnet-scoped or belongs to a local interface";
+		result.reason = "Address is subnet-scoped or belongs to a local interface.";
 		return result;
 	}
 
@@ -86,7 +86,7 @@ struct addrxlat_result addrxlat_siit46(struct xlator *instance,
 
 	if (enable_denylists && must_not_translate(&tmp, instance->ns)) {
 		result.verdict = ADDRXLAT_ACCEPT;
-		result.reason = "The address is subnet-scoped or belongs to a local interface";
+		result.reason = "Address is subnet-scoped or belongs to a local interface.";
 		return result;
 	}
 
@@ -100,13 +100,13 @@ struct addrxlat_result addrxlat_siit46(struct xlator *instance,
 
 	if (denylist4_contains(instance->siit.denylist4, &tmp)) {
 		result.verdict = ADDRXLAT_ACCEPT;
-		result.reason = "The address lacks EAMT entry and is denylist4ed";
+		result.reason = "Address lacks EAMT entry and is denylist4ed.";
 		return result;
 	}
 
 	if (!instance->globals.pool6.set) {
 		result.verdict = ADDRXLAT_TRY_SOMETHING_ELSE;
-		result.reason = "The address lacks EAMT entry and there's no pool6 prefix";
+		result.reason = "Address lacks EAMT entry and there's no pool6 prefix.";
 		return result;
 	}
 

src/mod/common/nl/address.c

@@ -54,7 +54,7 @@ int handle_address_query64(struct sk_buff *skb, struct genl_info *info)
 	/* Perform query */
 	verdict = addrxlat_siit64(&jool, &request, &result, true);
 	if (verdict.verdict != ADDRXLAT_CONTINUE) {
-		log_err("%s.", verdict.reason);
+		log_err("Unable to translate %pI6c: %s", &request, verdict.reason);
 		error = -EINVAL;
 		goto revert_start;
 	}
@@ -107,7 +107,7 @@ int handle_address_query46(struct sk_buff *skb, struct genl_info *info)
 	/* Perform query */
 	verdict = addrxlat_siit46(&jool, request.s_addr, &result, true, true);
 	if (verdict.verdict != ADDRXLAT_CONTINUE) {
-		log_err("%s.", verdict.reason);
+		log_err("Unable to translate %pI4: %s", &request, verdict.reason);
 		error = -EINVAL;
 		goto revert_start;
 	}

src/mod/common/steps/compute_outgoing_tuple_siit.c

@@ -15,8 +15,10 @@ verdict translate_addrs64_siit(struct xlation *state, __be32 *src_out,
 
 	/* Dst address. (SRC DEPENDS CON DST, SO WE NEED TO XLAT DST FIRST!) */
 	addr_result = addrxlat_siit64(&state->jool, &hdr6->daddr, &dst, true);
-	if (addr_result.reason)
-		log_debug(state, "%s.", addr_result.reason);
+	if (addr_result.reason) {
+		log_debug(state, "Unable to translate %pI6c: %s", &hdr6->daddr,
+				addr_result.reason);
+	}
 
 	switch (addr_result.verdict) {
 	case ADDRXLAT_CONTINUE:
@@ -33,8 +35,10 @@ verdict translate_addrs64_siit(struct xlation *state, __be32 *src_out,
 	/* Src address. */
 	addr_result = addrxlat_siit64(&state->jool, &hdr6->saddr, &src,
 			!pkt_is_icmp6_error(&state->in));
-	if (addr_result.reason)
-		log_debug(state, "%s.", addr_result.reason);
+	if (addr_result.reason) {
+		log_debug(state, "Unable to translate %pI6c: %s", &hdr6->saddr,
+				addr_result.reason);
+	}
 
 	switch (addr_result.verdict) {
 	case ADDRXLAT_CONTINUE:
@@ -113,8 +117,10 @@ verdict translate_addrs46_siit(struct xlation *state, struct in6_addr *src_out,
 
 	addr_result = addrxlat_siit46(&state->jool, hdr4->daddr, &addr6,
 			!disable_dst_eam(in, is_hairpin), true);
-	if (addr_result.reason)
-		log_debug(state, "%s.", addr_result.reason);
+	if (addr_result.reason) {
+		log_debug(state, "Unable to translate %pI4: %s", &hdr4->daddr,
+				addr_result.reason);
+	}
 
 	switch (addr_result.verdict) {
 	case ADDRXLAT_CONTINUE:
@@ -131,8 +137,10 @@ verdict translate_addrs46_siit(struct xlation *state, struct in6_addr *src_out,
 	addr_result = addrxlat_siit46(&state->jool, hdr4->saddr, &addr6,
 			!disable_src_eam(in, is_hairpin),
 			!pkt_is_icmp4_error(in));
-	if (addr_result.reason)
-		log_debug(state, "%s.", addr_result.reason);
+	if (addr_result.reason) {
+		log_debug(state, "Unable to translate %pI4: %s", &hdr4->saddr,
+				addr_result.reason);
+	}
 
 	switch (addr_result.verdict) {
 	case ADDRXLAT_CONTINUE: