Re: [PATCH net-next v2 1/3] ipv4: icmp: Add RFC 5837 support

[David 'equinox' Lamparter <equinox@diac24.net>]

On Mon, Oct 27, 2025 at 10:22:30AM +0200, Ido Schimmel wrote:
> +/* ICMP Extension Object Classes */
> +#define ICMP_EXT_OBJ_CLASS_IIO		2	/* RFC 5837 */
> +
> +/* Interface Information Object - RFC 5837 */
> +enum {
> +	ICMP_EXT_CTYPE_IIO_ROLE_IIF,
> +};

...

> +static __be32 icmp_ext_iio_addr4_find(const struct net_device *dev)
> +{
> +	struct in_device *in_dev;
> +	struct in_ifaddr *ifa;
> +
> +	in_dev = __in_dev_get_rcu(dev);
> +	if (!in_dev)
> +		return 0;
> +
> +	/* It is unclear from RFC 5837 which IP address should be chosen, but
> +	 * it makes sense to choose a global unicast address.
> +	 */
> +	in_dev_for_each_ifa_rcu(ifa, in_dev) {
> +		if (READ_ONCE(ifa->ifa_flags) & IFA_F_SECONDARY)
> +			continue;
> +		if (ifa->ifa_scope != RT_SCOPE_UNIVERSE ||
> +		    ipv4_is_multicast(ifa->ifa_address))
> +			continue;
> +		return ifa->ifa_address;

For 5837, this should be an address identifying the interface.  This
sets up a rather tricky situation if there's a /32 configured on the
interface in the context of unnumbered operation.  Arguably, in that
case class 5 (node info) should be used rather than class 2 (interface
info).  Class 5 also allows sticking an IPv6 address in an ICMPv4 reply.

I would argue the logic here should be an order of preference:

1. any global non-/32 address on the interface, in a class 2 object
2. any global /32 on the interface, in a class 5 object
3. any global IPv6 on the interface, in a class 5 object
4. any global address from any interface in the VRF, preferring
   loopback, in a class 5 object (addrsel logic, really)

[class 5 is draft-ietf-intarea-extended-icmp-nodeid]

+ analog for IPv6

(cf. my other mail in the thread)

Cheers,


-equi