All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Eric Dumazet <edumazet@google.com>
Cc: "David S . Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	Ido Schimmel <idosch@nvidia.com>,
	David Ahern <dsahern@kernel.org>,
	netdev@vger.kernel.org, eric.dumazet@gmail.com,
	Muhammad Ziad <muhzi100@gmail.com>
Subject: Re: [PATCH net] ipv4: icmp: fill flow parameters in icmp_route_lookup decoy lookup
Date: Tue, 21 Jul 2026 18:38:00 -0700	[thread overview]
Message-ID: <20260721183800.3bb3fc5e@kernel.org> (raw)
In-Reply-To: <20260716021049.2124921-1-edumazet@google.com>

On Thu, 16 Jul 2026 02:10:49 +0000 Eric Dumazet wrote:
> When Linux forwards a packet and needs to generate an ICMP error,
> icmp_route_lookup() performs a reverse-path relookup. For non-local
> destinations, it performs a decoy lookup via ip_route_output_key() to
> find the expected egress interface (rt2->dst.dev) before validating the
> path with ip_route_input().
> 
> Currently, the decoy flow structure (fl4_2) only sets .daddr = fl4_dec.saddr,
> leaving .saddr, .flowi4_dscp, .flowi4_proto, .flowi4_mark, and
> .flowi4_uid zeroed out.
> 
> When policy routing rules (such as ip rule add from $SRC lookup 100, or
> dscp/fwmark/ipproto rules) are configured:
> 1. ip_route_output_key() fails to match the policy rule because saddr and
>    other key flow selectors are missing in fl4_2.
> 2. It resolves a route using the default table instead, returning an incorrect
>    egress netdev.
> 3. Passing the wrong netdev to ip_route_input() causes strict reverse-path
>    filtering (rp_filter=1) to fail, logging false-positive "martian source"
>    warnings and causing the relookup to fail.
> 
> Fix this by populating fl4_2 with .saddr = fl4_dec.daddr, .flowi4_dscp,
> .flowi4_proto = IPPROTO_ICMP, .flowi4_mark, and .flowi4_uid so that
> ip_route_output_key() selects the correct egress interface matching policy
> routing rules.

Hi Eric!

This got auto-discarded due to bad interaction with the xfrm test:

https://netdev-ctrl.bots.linux.dev/logview.html?f=/logs/vmksft/net/results/736183/28-xfrm-state-sh/stdout

AI bot says:

The NIPA CI system flagged a regression in the selftests/net/xfrm_state.sh
test on both the normal and debug kernel configurations after this patch was
applied. The failures are consistent across retries.

Two IPv4 sub-tests fail; the IPv6 equivalents pass:

  TEST: IPv4 unreachable from IPsec gateway s2   [ FAIL ]  unreachable_gw_ipv4
    ip netns exec ns_a ping -W 5 -w 4 -c 1 10.1.5.3
    4 packets transmitted, 0 received, 100% packet loss

  TEST: IPv4 MTU exceeded from IPsec gateway s2  [ FAIL ]  mtu_ipv4_s2
    ip netns exec ns_a ping -M do -s 1350 -W 5 -w 4 -c 1 10.1.5.2
    4 packets transmitted, 0 received, 100% packet loss

The test topology uses an IPsec (ESP tunnel) gateway that should generate and
return IPv4 ICMP error messages (host unreachable / fragmentation needed) to
the originating host.

The regression is in the IPv4 path only, which points to the change in
icmp_route_lookup() in net/ipv4/icmp.c.

Specifically, the newly added `.flowi4_proto = IPPROTO_ICMP` field in the
decoy `fl4_2` structure may be causing ip_route_output_key() to select a
different (possibly IPsec-encapsulated) egress interface than expected in
the xfrm gateway scenario. When the wrong interface is passed to
ip_route_input(), reverse-path filtering appears to reject the lookup,
causing the ICMP error reply to be silently dropped rather than forwarded
back to the sender.

The patch's stated intent — fixing policy routing scenarios where .saddr and
other selectors are needed — is correct in principle, but the addition of
.flowi4_proto = IPPROTO_ICMP seems to interact poorly with xfrm policy
matching during the decoy lookup.

Could you take a look?

      reply	other threads:[~2026-07-22  1:38 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16  2:10 [PATCH net] ipv4: icmp: fill flow parameters in icmp_route_lookup decoy lookup Eric Dumazet
2026-07-22  1:38 ` Jakub Kicinski [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260721183800.3bb3fc5e@kernel.org \
    --to=kuba@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=eric.dumazet@gmail.com \
    --cc=horms@kernel.org \
    --cc=idosch@nvidia.com \
    --cc=muhzi100@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.