Re: [RFC net-next 1/2] ipv4: Honor prohibit when oif specified

[liamsmith827]

On 10/25/26 17:13, Ido Schimmel wrote:
> Given the above, I think that the patch is too specific to your use
> case. It doesn't make sense to only honor prohibit rules / routes. How
> about checking for 'err == -ENETUNREACH' [1]?
>
> It doesn't allow us to distinguish between "no route" and "unreachable
> rule", but we can probably ignore it given that it has always been the
> case and nobody complained. It needs to be explained in the commit
> message.

I was trying to sneak in just enough functionality for us to do what we 
needed, but I'll gladly expand the scope.

I think if we were checking for -ENETUNREACH we would want to also check 
for -EHOSTUNREACH to keep the semantics of unreachable consistent 
between both the rule and route variants. However, I think we should 
differentiate between a routing miss and an unreachable rule/route, 
allowing the fallback only in the former case. Not only is this, as 
you've said, the intention of the fallback, but it also has a concrete 
benefit for Android's current networking implementation. Android 
includes a final unreachable rule, so a routing miss is not possible. If 
treated the same, this unreachable rule would not prevent apps from 
being able to use the fallback to send traffic over interfaces which the 
routing rules do not allow. This is a real cross-user leak which we were 
going to fix by changing the final rule to prohibit (alongside the 
original patch), but that would likely not have reached upstream.

This is easy to implement. In v2, I've added a FIB_LOOKUP_REPORT_MISS 
flag that returns -ESRCH instead of -ENETUNREACH on an IPv4 lookup miss.

> Note that multicast needs to be exempted because of VRF. The lookup
> fails on the unreachable default route in the VRF table (-EHOSTUNREACH)
> and we construct the route anyway. It is also consistent with IPv6 where
> these routes / rules do not affect multicast traffic since the
> per-interface ff00::/8 routes in the local table are matched first.

I do agree that multicast needs to be exempted, and that's not 
exclusively because of VRF. That same -EHOSTUNREACH can be encountered 
with multicast outside VRF. In addition to the IPv6 reasoning you've 
provided, there are 2 instances within ip_route_output_key_hash_rcu() 
where multicast traffic goes directly to make_route without a 
fib_lookup(). Whether fib_lookup() should be consulted for all multicast 
is another discussion altogether.

> Why modify this hunk? Looks like an unrelated change.

This comment does need to be updated because it has multiple 
inaccuracies, both with and without the new changes. I'll split it out.