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

[Ido Schimmel <idosch@nvidia.com>]

On Fri, Nov 07, 2025 at 06:33:30PM +0100, David 'equinox' Lamparter wrote:
> 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.

This patchset does not add support for class 5 objects and the above
code is strictly about choosing an address for the "IP Address
Sub-Object" in a class 2 object. Support for class 5 objects can be
added in a different patchset. An administrator can then choose to
include both objects in ICMP error messages.

The kernel does not avoid using a /32 as the source IP of ICMP error
messages, so I don't see a reason to avoid using such an address in the
"IP Address Sub-Object".

> 
> 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