All of lore.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: netdev@vger.kernel.org, Florian Westphal <fw@strlen.de>,
	Phil Sutter <phil@nwl.cc>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	Victor Nogueira <victor@mojatatu.com>,
	Jiri Pirko <jiri@resnulli.us>,
	sashiko-bot@kernel.org
Subject: Re: [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
Date: Wed, 11 Nov 2026 22:33:54 +0100	[thread overview]
Message-ID: <avTfQjFQYR2NNdRg@chamomile> (raw)
In-Reply-To: <CORE-9NOG.v1.r0.20261111073528-1@mojatatu.com>

On Wed, Nov 11, 2026 at 09:16:37AM -0500, Jamal Hadi Salim wrote:
> This is a followup of the review of the teql arp-queue fix as reported
> by Sashiko.
> 
> The issue: the neighbour variant of fwd rewrites skb->dev to the
> configured output device and calls neigh_xmit(), but leaves the skb's
> dst untouched. If that device's neighbour for the configured nexthop is
> incomplete, the packet is parked on the neighbour's arp_queue while the
> stale dst still describes the device the packet was routed through.
> Draining the queue re-looks the neighbour up from that dst, so the frame
> is built with the routed device's link-layer header and then transmitted
> on the configured device. The packet leaves with another device's
> addresses.
> 
> The Fix: drop the dst before the device switch. Both lookup inputs the
> neighbour transmission needs, the device and the nexthop, are supplied
> by the rule, so the route dst is never consulted. Without it the queue
> drain keeps the parking neighbour and the frame carries the configured
> device's addresses.
> 
> Conditions to recreate the bug: two network namespaces, A (10.10.0.1/24)
> and B (10.1.0.1/24) in one, their veth peers in the other with only A's
> peer addressed (10.10.0.2/24) so B's nexthop cannot be resolved by ARP.
> Resolve A's neighbour for 10.10.0.2 as permanent, then load this ruleset
> on A:
> 
>     table netdev t {
>             chain c {
>                     type filter hook egress device va priority 0
>                     policy accept
>                     fwd ip to 10.1.0.2 device vb
>             }
>     }
> 
> Send one packet to 10.10.0.2. It is parked on B's incomplete neighbour
> for 10.1.0.2; once that neighbour becomes valid the frame leaves B
> carrying A's source address and A's neighbour as the destination.
> 
> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
> Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  net/netfilter/nft_fwd_netdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
> index a48c2f765bba..ad4d61d64cdd 100644
> --- a/net/netfilter/nft_fwd_netdev.c
> +++ b/net/netfilter/nft_fwd_netdev.c
> @@ -175,6 +175,7 @@ static void nft_fwd_neigh_eval(const struct nft_expr *expr,
>  		}
>  	}
>  

Does this need a check for skb_dst_valid()? I think it is valid to
attach a template dst here for tunneling?

> +	skb_dst_drop(skb);
>  	skb->dev = dev;
>  	skb_clear_tstamp(skb);
>  
> -- 
> 2.43.0
> 

  reply	other threads:[~2026-11-11 21:33 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11 14:16 [PATCH net-next 0/2] net: drop the stale dst when steering to another device Jamal Hadi Salim
2026-11-11 14:16 ` [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device Jamal Hadi Salim
2026-11-11 21:33   ` Pablo Neira Ayuso [this message]
2026-11-12 10:49     ` Jamal Hadi Salim
2026-11-11 14:16 ` [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop Jamal Hadi Salim
2026-11-12 14:16   ` sashiko-bot

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=avTfQjFQYR2NNdRg@chamomile \
    --to=pablo@netfilter.org \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=fw@strlen.de \
    --cc=horms@kernel.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=phil@nwl.cc \
    --cc=sashiko-bot@kernel.org \
    --cc=victor@mojatatu.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.