ICMPv6: Recognise ND option 38 (PREF64)

[bonktree]

This option, defined in RFC 8781, allows a router administrator to pass NAT64 prefix information for the network to end hosts together with other routing and prefix information in the RA message, getting rid of the need for DNS64 service in the network.

As of today the option is widely supported by software routers, including radvd-project/radvd#179, BIRD and systemd/systemd#28565.

We implement the printer and test some valid and broken option contents.

[fenner]

Just for a data point of "can this decode the PREF64 attribute in some random network", I captured an RA packet on the ietf-ipv6-mostly network, which is advertising the default 6to4 in the PREF64 field, and it decoded as follows:

09:56:27.552274 IP6 (hlim 255, next-header ICMPv6 (58), payload length 144) fe80::1998:1 > ff02::1: [icmp6 sum ok] ICMP6, router advertisement, length 144
	hop limit 64, Flags [other stateful], pref high, router lifetime 9000s, reachable time 0ms, retrans timer 0ms
	  source link-address option (1), length 8 (1): 00:00:5e:00:02:c6
	  rdnss option (25), length 40 (5):  lifetime 9000s, addr: 2001:67c:370:229::9 addr: 2001:67c:370:229::10
	  prefix info option (3), length 32 (4): 2001:67c:370:1998::/64, Flags [onlink, auto], valid time 2592000s, pref. time 604800s
	  prefix info option (3), length 32 (4): 2001:67c:370:1998::/64, Flags [onlink, auto], valid time 2592000s, pref. time 604800s
	  pref64 option (38), length 16 (2): 64:ff9b::/96 (plc 0), lifetime 18000s

[fenner]

It feels a little odd to have the w & 0x0007 both inside get_pref64_len_repr() and in the ND_PRINT args. It might be more clear to use a temp variable, maybe named plc, and then pass that to both get_pref64_len_repr() and in the ND_PRINT arg list. Obviously this is pretty minor.

[bonktree]

The arg of get_pref64_len_repr() is eventually used to index memory, so IMHO the mask should not be removed from get_pref64_len_repr() anyway; it should be local to the array member access. Otherwise, in the case where the compiler does not inline the call, we would not be far from introducing a gadget to read garbage memory; it would take a single human error. In my view, the resulting code would be harder to maintain.

A temporary variable declaration five screens away, only useful for opt 38, does not address the point above.

One way to reduce repetition would be to abstract "%s/%u (plc %u)" into its own function instead of get_pref64_len_repr():

static void
print_pref64(netdissect_options *ndo, const u_char *in6, size_t w)
{
	static const char *lengths[] = {
		"96", "64", "56", "48", "40", "32"
	};
	const char *plen;
	uint16_t plc = w & 0x0007;

	if (plc < 6)
		plen = lengths[plc];
	else
		plen = "??";
	ND_PRINT("%s/%s (plc %u)",
	         ip6addr_string(ndo, in6),
	         plen, plc);
}

..., and then use it in icmp6_opt_print(). Or, maybe, even print the lifetime in here.
The mask is local to the memory access, so it is hard to make a mistake. We do not have to write w & 0x0007 twice.
That might or might not be easier to read.

[fenner]

Sometimes the cure is worse than the disease. While the two masks jumped out at me as unusual, I don't think it's a blocker. Thanks for humoring me.

[fxlb]

Thank you!