A patch you wrote some time ago (aka: "[patch 41/54] ICMP: Fix icmp_errors_use_inbound_ifaddr sysctl")

[Alexander Hoogerhuis <alexh@boxed.no>]

I hope you (or anyone else) can spare half a minute to have a quick look 
at a patch you wrote a few years ago:

> http://lkml.org/lkml/2007/6/8/124

I've been tracking down a case of ICMP Redirects originating from the 
wrong IPs, and as far I can tell, you patch is the last to touch this 
code (net/ipv4/icmp.c:507):

>  if (rt->fl.iif && net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
>         dev = dev_get_by_index_rcu(net, rt->fl.iif);
>
> if (dev)
>         saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
> else
>         saddr = 0;

In a plain world this would work, but I have come across a case that 
seems to be not handled by this.

I have two machines set up with VRRP to act as routers out of a subnet, 
and they have IPs x.x.x.13/28 and x.x.x.14/28, with VRRP holding on to 
x.x.x.1/28.

If a node in x.x.x.0/28 needs to get a ICMP redirect from x.x.x.1/28 (to 
reach another subnet behind a  different gateway in x.x.x.0/28), then 
the source IP on the ICMP redirect is chosen as the primary IP on the 
interface that the packet arrived at.

This is as far as I can tell from RFCs and colleagues fine for most 
things after you're routed one hop or more, but in the case of ICMP 
redirect it means that the redirect is not adhered to by the client, as 
it will get the reidrect from x.x.x.13/28, not x.x.x.1/28.

inet_select_addr seems to be explicitly looking for the primary IP in 
all cases (./net/ipv4/devinet.c:875), and in the case of sending ICMP 
recdirect when in an VRRP setup, that would not work well. It should try 
to match the actual inbound IP.

Judging by the comments from your patch I am not sure if the source IP 
that triggers the ICMP redirect is available at this point any more.

The way I understand it should pick adress is this way:

 >  if (rt->fl.iif && net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
 >         dev = dev_get_by_index_rcu(net, rt->fl.iif);
 >
 > if (dev == fl.iif)
 >         saddr = iph->daddr;
 >
 > if (dev != fl.iif)
 >         saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
 > else
 >         saddr = 0;

I.e. if we are replying to something that is from a local network 
segment, then iph->daddr would be a more correct source. My C skill is 
prehistoric so what I've written likely is far from correct, but the 
general gist is that there is a special case for replying to something 
local.

As it stands today (I'm on 2.6.35.11), ICMP redirects when using VRRP 
are broken, and I'm hoping I may have found out why. :)

mvh,
A
-- 
Alexander Hoogerhuis | http://no.linkedin.com/in/alexh
Boxed Solutions AS   | +47 908 21 485 - alexh@boxed.no
"Given enough eyeballs, all bugs are shallow." -Eric S. Raymond