* [PATCH net-next 0/2] net: drop the stale dst when steering to another device
@ 2026-11-11 14:16 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 14:16 ` [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop Jamal Hadi Salim
0 siblings, 2 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-11 14:16 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netfilter-devel, coreteam,
Daniel Borkmann, John Fastabend, Stanislav Fomichev,
Martin KaFai Lau, Alexei Starovoitov, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai, bpf,
Victor Nogueira, Jiri Pirko, sashiko-bot
This patchset is created to address constant whining by both sashikos
about this being an issue when fixing a UAF on a teql issue (entering
v4 now), see the v3 here fixing the teql consumer:
https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
Two producers steer a packet to a network device of their choosing and
then hand it to the neighbour layer for link-layer resolution:
1/2 netfilter: the neighbour variant of nft `fwd`, on the netdev egress
hook
2/2 bpf: `bpf_redirect_neigh()` when the program supplies the nexthop
Both rewrite `skb->dev` to the target device and leave the skb's dst
pointing at the route the packet arrived through. When the target
device's neighbour for the nexthop is incomplete the packet is parked on
that neighbour's arp_queue, and draining the queue re-looks the neighbour
up from the stale dst. The frame is therefore built with the original
device's link-layer header and then transmitted on the target device: the
packet leaves with another device's addresses, and the sender never
learns that its nexthop was unreachable.
This is explicit: the steer-to-another-device path never worked in this
respect, so there is nothing here to regress. Both producers shipped with
`skb->dev` swapped to the target device and the route's dst still
attached, and the arp_queue drain has re-derived the neighbour from that
dst for as long as either producer has existed; the invariant "the skb's
dst describes where the skb is going", which the routing path keeps when
it steers, was never established for these two entry points. With no
commit to anchor with a `Fixes:` tag and no crash, UAF or lockup
involved, the series targets net-next and does not nominate `Cc: stable`:
a backport would change long-standing redirect behaviour for no
crash-class benefit.
More context: this came out of the review of the fix for a parked skb
being resolved against the wrong device in sch_teql, posted separately and
not part of this series. The neighbour code's queue drain is doing the
right thing there, it re-derives the neighbour from the skb, so the repair
belongs with the producers rather than with the drain path. Both
producers are fixed in the same series so neither is left behind; each
patch stands on its own and carries its own reproduction recipe.
The reproduction for both arms is a pair of veth devices, one of them
with an unresolved neighbour, plus a capture on the target's peer: the
captured frame's source and destination addresses name the device whose
neighbour was used to build it.
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com
Link: https://lore.kernel.org/netdev/20261026160747.283702-1-horms@kernel.org/
Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
Jamal Hadi Salim (2):
netfilter: nft_fwd_netdev: drop the stale dst when steering to a
device
bpf: drop the stale dst when redirecting to an explicit nexthop
net/core/filter.c | 4 ++++
net/netfilter/nft_fwd_netdev.c | 1 +
2 files changed, 5 insertions(+)
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
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 ` Jamal Hadi Salim
2026-11-11 21:33 ` Pablo Neira Ayuso
2026-11-15 14:42 ` netdev-bot+sashiko
2026-11-11 14:16 ` [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop Jamal Hadi Salim
1 sibling, 2 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-11 14:16 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Pablo Neira Ayuso, Florian Westphal,
Phil Sutter, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, netfilter-devel, coreteam,
Victor Nogueira, Jiri Pirko, sashiko-bot
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,
}
}
+ skb_dst_drop(skb);
skb->dev = dev;
skb_clear_tstamp(skb);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop
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 14:16 ` Jamal Hadi Salim
2026-11-15 14:42 ` netdev-bot+sashiko
1 sibling, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-11 14:16 UTC (permalink / raw)
To: netdev
Cc: Jamal Hadi Salim, Daniel Borkmann, John Fastabend,
Stanislav Fomichev, Martin KaFai Lau, Alexei Starovoitov,
Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
Ihor Solodrai, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, bpf, Victor Nogueira, Jiri Pirko,
sashiko-bot
This is a followup of the review of the teql arp-queue fix as reported
by Sashiko.
The issue: __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6()
install a dst for the redirect target only when the program lets the
helper do the FIB lookup. With BPF_F_NEXTHOP the program supplies the
nexthop, the lookup is skipped, and whatever dst the packet arrived
with stays attached while bpf_out_neigh_v{4,6}() switch skb->dev to the
target device. If that device's neighbour is incomplete the packet is
parked on its arp_queue with the original route's dst; draining the
queue re-looks the neighbour up from that dst and builds the frame with
the original device's link-layer header, then transmits it on the target
device.
The Fix: drop the dst when the nexthop is supplied. That branch never
consults the route dst, so nothing is lost, and the queue drain keeps
the target device's parking neighbour.
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 and attach a cls_bpf
egress program to A, with vb the index of B:
struct bpf_redir_neigh nh = { .nh_family = AF_INET,
.ipv4_nh = htonl(0x0a010002) };
return bpf_redirect_neigh(vb, &nh, sizeof(nh), 0);
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/core/filter.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/filter.c b/net/core/filter.c
index aca8160b26a1..2af24f371648 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2318,6 +2318,8 @@ static int __bpf_redirect_neigh_v6(struct sk_buff *skb, struct net_device *dev,
skb_dst_set(skb, dst);
} else if (nh->nh_family != AF_INET6) {
goto out_drop;
+ } else {
+ skb_dst_drop(skb);
}
err = bpf_out_neigh_v6(net, skb, dev, nh);
@@ -2433,6 +2435,8 @@ static int __bpf_redirect_neigh_v4(struct sk_buff *skb, struct net_device *dev,
skb_dst_drop(skb);
skb_dst_set(skb, &rt->dst);
+ } else {
+ skb_dst_drop(skb);
}
err = bpf_out_neigh_v4(net, skb, dev, nh);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
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
2026-11-12 10:49 ` Jamal Hadi Salim
2026-11-15 14:42 ` netdev-bot+sashiko
1 sibling, 1 reply; 7+ messages in thread
From: Pablo Neira Ayuso @ 2026-11-11 21:33 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: netdev, Florian Westphal, Phil Sutter, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
netfilter-devel, coreteam, Victor Nogueira, Jiri Pirko,
sashiko-bot
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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
2026-11-11 21:33 ` Pablo Neira Ayuso
@ 2026-11-12 10:49 ` Jamal Hadi Salim
0 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-11-12 10:49 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: netdev, Florian Westphal, Phil Sutter, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
netfilter-devel, coreteam, Victor Nogueira, Jiri Pirko,
sashiko-bot
On Wed, Nov 11, 2026 at 4:33 PM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> 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?
>
You are correct (I think you mean skb_valid_dst()). Same guard needed
with the second (ebpf) patch.
pw-bot: cr
cheers,
jamal
> > + skb_dst_drop(skb);
> > skb->dev = dev;
> > skb_clear_tstamp(skb);
> >
> > --
> > 2.43.0
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 1/2] netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
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
@ 2026-11-15 14:42 ` netdev-bot+sashiko
1 sibling, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-15 14:42 UTC (permalink / raw)
To: jhs
Cc: netdev, pablo, fw, phil, davem, edumazet, kuba, pabeni, horms,
netfilter-devel, coreteam, victor, jiri, sashiko-bot
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The commit fixes a real wire-level bug in nft_fwd_neigh_eval()
(net/netfilter/nft_fwd_netdev.c) but has no Fixes: tag and no Cc:…
- [Low] The commit message says the rule supplies both the device and the
nexthop "so the route dst is never consulted", which implies dropping…
--
netfilter: nft_fwd_netdev: drop the stale dst when steering to a device
This patch makes nft_fwd_neigh_eval() drop the skb's dst before it
switches skb->dev to the configured output device and calls neigh_xmit().
Without the drop, a packet parked on an incomplete neighbour's arp_queue
can be re-resolved through the stale dst when the queue drains. It then
leaves the configured device carrying another device's link-layer
addresses.
Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
> 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>
[Severity: Low]
Should this have a Fixes: tag and Cc: stable?
The skb->dev = dev; followed by neigh_xmit() in nft_fwd_neigh_eval(),
with the skb's dst still attached, came in with:
d32de98ea70f ("netfilter: nft_fwd_netdev: allow to forward packets via neighbour layer")
The bug became much easier to hit once nft_fwd_validate() allowed
NF_NETDEV_EGRESS, because egress skbs always carry the output route's
dst:
f87b9464d152 ("netfilter: nft_fwd_netdev: Support egress hook")
The reproducer in the commit message uses an egress chain. Without a
Fixes: tag, stable backporters can't tell which trees need this fix.
> diff --git a/net/netfilter/nft_fwd_netdev.c b/net/netfilter/nft_fwd_netdev.c
> index a48c2f765bbaa..ad4d61d64cdde 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,
> }
> }
>
> + skb_dst_drop(skb);
[Severity: Low]
The commit message says:
Both lookup inputs the neighbour transmission needs, the device and
the nexthop, are supplied by the rule, so the route dst is never
consulted.
Is that accurate? The arp_queue drain is not the only place that reads
the dst. If the target neighbour never resolves, the neighbour's
error_report hook also reads it:
neigh_xmit()
neigh->output()
... resolution fails ...
arp_error_report() / ndisc_error_report()
dst_link_failure(skb)
include/net/dst.h:dst_link_failure() {
...
if (dst && dst->ops && dst->ops->link_failure)
dst->ops->link_failure(skb);
...
}
Before this patch, an egress skb handled by "fwd ip to <nh> device <dev>"
whose nexthop never resolved still had its route dst. So
ipv4_link_failure() or ip6_link_failure() sent ICMP host unreachable or
ICMPv6 address unreachable back to the sender, and expired or
invalidated the original route.
Once the dst is dropped, dst_link_failure() returns without doing
anything. __icmp_send() also needs skb_rtable(skb_in), so the packet is
now dropped silently. Local senders no longer get EHOSTUNREACH feedback,
and the route is not expired.
The new behaviour is arguably more correct. The old ICMP and route
expiry applied to a route and device the packet was no longer using,
and the new behaviour matches the ingress hook, where skbs normally have
no dst.
Could the commit message mention this change in user-visible behaviour?
The follow-up patch "bpf: drop the stale dst when redirecting to an
explicit nexthop" makes the same claim for bpf_redirect_neigh(), so the
same question may apply there.
> skb->dev = dev;
> skb_clear_tstamp(skb);
>
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/CORE-9NOG.v1.r0.20261111073528%40mojatatu.com
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next 2/2] bpf: drop the stale dst when redirecting to an explicit nexthop
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-15 14:42 ` netdev-bot+sashiko
0 siblings, 0 replies; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-11-15 14:42 UTC (permalink / raw)
To: jhs
Cc: netdev, pablo, fw, phil, davem, edumazet, kuba, pabeni, horms,
netfilter-devel, coreteam, victor, jiri, sashiko-bot
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 1
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] The new `else { skb_dst_drop(skb); }` branches in
__bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6()…
- [Low] The commit fixes a user-visible bug but has no Fixes: tag (and no
stable Cc).
--
bpf: drop the stale dst when redirecting to an explicit nexthop
With BPF_F_NEXTHOP, __bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6()
skip the FIB lookup. They leave the packet's original dst attached while
skb->dev is switched to the target device. If the packet is parked on the
target's arp_queue, the drain looks the neighbour up again from that stale
dst and builds the frame with the wrong link-layer header, so the patch
drops the dst on the explicit-nexthop branch.
Link: https://lore.kernel.org/netdev/QDISC-6HS9.v3.r0.20261103062649-2@mojatatu.com/
> 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>
[Severity: Low]
Should this patch carry a Fixes: tag? The explicit-nexthop branches in
__bpf_redirect_neigh_v4() and __bpf_redirect_neigh_v6() that keep the
original route dst came from:
Fixes: ba452c9e996d ("bpf: Fix bpf_redirect_neigh helper api to support supplying nexthop")
The related fix for the non-nexthop branch, commit 23f3770e1a53 ("bpf: Fix
metadata_dst leak __bpf_redirect_neigh_v{4,6}"), had a Fixes: tag.
Without one here, stable kernels that contain ba452c9e996d may not get
this fix.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index aca8160b26a1a..2af24f371648a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2318,6 +2318,8 @@ static int __bpf_redirect_neigh_v6(struct sk_buff *skb, struct net_device *dev,
> skb_dst_set(skb, dst);
> } else if (nh->nh_family != AF_INET6) {
> goto out_drop;
> + } else {
> + skb_dst_drop(skb);
> }
[Severity: High]
Can this drop a metadata dst that was attached by
bpf_skb_set_tunnel_key()?
The new else branch drops any dst on the skb and does not check whether
it is a route dst. bpf_skb_set_tunnel_key() stores the TX tunnel info in
a DST_METADATA dst:
net/core/filter.c:bpf_skb_set_tunnel_key() {
...
skb_dst_drop(skb);
dst_hold((struct dst_entry *) md);
skb_dst_set(skb, (struct dst_entry *) md);
...
}
Take a tc program that calls bpf_skb_set_tunnel_key() and then
bpf_redirect_neigh() to a collect_md vxlan or geneve device, using an nh
from bpf_fib_lookup().
Before this patch the metadata dst stayed on the skb. vxlan and geneve
call netif_keep_dst(), so __dev_queue_xmit() keeps it. vxlan_xmit() then
finds skb_tunnel_info() with IP_TUNNEL_INFO_TX and encapsulates.
After this patch skb_tunnel_info() returns NULL. In collect_md mode,
vxlan_xmit() then drops the packet:
drivers/net/vxlan/vxlan_core.c:vxlan_xmit() {
...
if (info && info->mode & IP_TUNNEL_INFO_TX)
vxlan_xmit_one(skb, dev, cfg, vni, NULL, false);
else
kfree_skb_reason(skb, SKB_DROP_REASON_TUNNEL_TXINFO);
...
}
Geneve behaves the same way.
If the overlay neighbour is not yet resolved, arp_solicit() also reads
the tunnel metadata for the ARP request from the parked skb's dst:
net/ipv4/arp.c:arp_solicit() {
...
if (skb && !(dev->priv_flags & IFF_XMIT_DST_RELEASE))
dst = skb_dst(skb);
...
}
With that dst now NULL, would the solicitation be dropped too, so the
neighbour never resolves?
The commit message says:
That branch never consults the route dst, so nothing is lost
That is true for a routing dst. The target device's xmit path does read
a metadata dst, though.
Metadata dsts use dst_blackhole_ops, and dst_blackhole_neigh_lookup()
returns NULL. For those dsts, the arp_queue drain already falls back to
the parking neighbour.
Would skipping the drop when skb_metadata_dst(skb) is set fix the
reported bug without this regression?
The same question applies to the new else branch in
__bpf_redirect_neigh_v4().
No selftest covers this case. test_tc_neigh_fib.c only redirects with a
FIB-derived nexthop and no tunnel key.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/CORE-9NOG.v1.r0.20261111073528%40mojatatu.com
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-15 14:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-11-12 10:49 ` Jamal Hadi Salim
2026-11-15 14:42 ` netdev-bot+sashiko
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-15 14:42 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox